Description

normalizeLegacyCodexHarnessModelRefs() migrates codex/* model references to openai/*. It attempts to ensure the Codex harness is selected, but only when embeddedHarness.runtime is missing or "auto".

As a result:

• If a user previously had agents.*.embeddedHarness.runtime explicitly set to a non-auto value (e.g. "pi" or some other plugin harness id) and had a codex/* primary model selected, the migration will rewrite the model ref to openai/* but will not update the harness runtime.
• After migration, execution may run through the forced non-Codex harness (not the Codex app-server harness), potentially changing authentication route and data handling (e.g., sending prompts/media to the OpenAI API via embedded PI rather than the bounded Codex harness).
• This migration runs during config read via applyRuntimeLegacyConfigMigrations() (used in src/config/io.ts), so the behavior change can occur automatically on startup.

Vulnerable code:

function ensureCodexEmbeddedHarness(raw: AgentEmbeddedHarnessPatch | undefined) {
  ...
  const runtime = normalizeOptionalLowercaseString(raw.runtime);
  if (!runtime || runtime === "auto") {
    return { value: { ...raw, runtime: "codex" }, ... };
  }
  return { value: raw, changed: false }; // non-auto runtime blocks forcing codex
}

Recommendation

When migrating a codex/* primary model to openai/*, ensure the run remains bound to the Codex harness even if an explicit non-auto embeddedHarness.runtime was set, or at minimum require explicit user confirmation / emit a high-visibility warning.

Safer options:

1. Force Codex harness on codex-selected primary, overriding any existing runtime that is not "codex" (or store the prior runtime elsewhere):

function ensureCodexEmbeddedHarness(raw: AgentEmbeddedHarnessPatch | undefined) {
  if (!isRecord(raw)) return { value: { runtime: "codex" }, changed: true };
  const runtime = normalizeOptionalLowercaseString(raw.runtime);
  if (runtime !== "codex") {
    return { value: { ...raw, runtime: "codex" }, changed: true };
  }
  return { value: raw, changed: false };
}

2. If overriding is not acceptable, do not migrate codex/* → openai/* when runtime is explicitly non-Codex; instead:

• keep the legacy ref, or
• migrate but add a blocking validation error / interactive prompt, or
• add a changes.push(...) warning stating that the harness runtime prevented selecting Codex and that traffic may go to OpenAI.

Also consider adding a regression test for embeddedHarness.runtime: "pi" + model: "codex/..." to ensure the chosen behavior is explicit and safe.

Description

normalizeLegacyCodexAllowlistModels() builds a plain object ({}) and then assigns properties using keys taken directly from a user-controlled config object (rawModels).

Because next is a normal object with Object.prototype, assigning attacker-chosen keys like __proto__, constructor, or prototype can mutate the prototype of next (or create surprising inherited properties), i.e. prototype pollution.

• Input: rawModels from cfg.agents.defaults.models / cfg.agents.list[*].models (loaded from config)
• Transformation: Object.entries(rawModels) iterates keys without filtering
• Sink: next[rawKey] = ... / next[migratedKey] = ... on {}

Vulnerable code:

const next: AgentModelsMapPatch = {};
for (const [rawKey, entry] of Object.entries(rawModels)) {
  ...
  next[rawKey] = mergeModelEntry(entry, next[rawKey]);
}

If this polluted object is later merged/spread or used in security-sensitive decisions (e.g., allowlisting models), it can cause incorrect behavior or potentially impact other parts of the process relying on object properties.

Recommendation

Harden the migration map against special keys.

Option A (recommended): use a null-prototype object and reject dangerous keys:

const next: Record<string, AgentModelsMapPatch[string]> = Object.create(null);
const isSafeKey = (k: string) => !["__proto__","prototype","constructor"].includes(k);

for (const [rawKey, entry] of Object.entries(rawModels)) {
  if (!isSafeKey(rawKey)) continue;
  const migratedKey = migrateLegacyCodexModelRef(rawKey);
  const key = migratedKey ?? rawKey;
  if (!isSafeKey(key)) continue;
  next[key] = mergeModelEntry(entry, next[key]);
}

Option B: use Map<string, ...> during normalization and convert to an object only at the boundary (also with Object.create(null)), ensuring no prototype setters are invoked.

Also consider validating that model keys match an expected pattern like /^[a-z0-9-]+\/[\w.-]+$/i before inserting them.

Description

openclaw doctor emits "changes" messages that interpolate config-controlled values (agent IDs) into terminal output without sanitization.

• Input (untrusted): entry.id comes from the user-controlled config (cfg.agents.list[].id).
• Propagation: normalizeLegacyCodexHarnessModelRefs() builds path using agentId and passes it into normalizeLegacyCodexAgentContainer(), which pushes human-readable change messages containing that path.
• Sink: loadAndMaybeMigrateDoctorConfig() prints normalized.changes via note(normalized.changes.join("\n"), "Doctor changes") without calling sanitizeDoctorNote() / sanitizeForLog().

If entry.id contains control characters (e.g., \n, \r) or ANSI escape sequences, it can:

• forge additional lines in the output (log/terminal forging)
• inject terminal control codes (hide warnings, spoof success, manipulate cursor)

Vulnerable code paths:

const agentId = normalizeOptionalString(entry.id) ?? String(index);
const agent = normalizeLegacyCodexAgentContainer(entry, `agents.list.${agentId}`, changes);
​// ...
changes.push(`Moved ${path}.model codex/* refs → openai/* ...`);

printed later as:

note(normalized.changes.join("\n"), "Doctor changes");

Recommendation

Sanitize any user/config-derived content before emitting it to terminal/log output.

Option A (preferred): sanitize the entire note payload at the sink

import { sanitizeDoctorNote } from "./doctor/emit-notes.js";

​// ...
note(sanitizeDoctorNote(normalized.changes.join("\n")), "Doctor changes");

Option B: sanitize path components where they are constructed

import { sanitizeForLog } from "../../../terminal/ansi.js";

const agentIdRaw = normalizeOptionalString(entry.id) ?? String(index);
const agentId = sanitizeForLog(agentIdRaw);

Also consider validating agents.list[].id against a safe identifier regex (e.g., /^[a-z0-9_-]+$/i) and rejecting or escaping anything else.