Description

handleCodexAppServerApprovalRequest() attempts to ensure an approval request is for the current Codex thread/turn via matchesCurrentTurn(), but the matching logic is fail-open when identifiers are missing.

• If requestParams is missing or not a JSON object, matchesCurrentTurn() returns true.
• If threadId/turnId are absent from requestParams, the function also returns true.
• As a result, an out-of-order/stale approval request (or a malicious/compromised app-server) can submit an approval request without IDs and have it treated as belonging to the current turn, potentially tricking an operator into approving an unrelated command/file/permission action.

Vulnerable code:

function matchesCurrentTurn(requestParams: JsonObject | undefined, threadId: string, turnId: string): boolean {
  if (!requestParams) {
    return true;
  }
  const requestThreadId = readString(requestParams, "threadId") ?? readString(requestParams, "conversationId");
  const requestTurnId = readString(requestParams, "turnId");
  if (requestThreadId && requestThreadId !== threadId) return false;
  if (requestTurnId && requestTurnId !== turnId) return false;
  return true;
}

Recommendation

Fail closed and require turn binding for approval requests.

Suggested approach:

• Require requestParams to be a JSON object.
• Require both threadId (or conversationId) and turnId to be present and match the active turn.
• If the protocol legitimately omits these fields for some methods, consider:
  • rejecting those methods entirely, or
  • requiring an alternative unforgeable binding (e.g., a per-turn nonce established when turn/start returns).

Example fix:

function matchesCurrentTurn(
  requestParams: JsonObject | undefined,
  threadId: string,
  turnId: string,
): boolean {
  if (!requestParams) return false;

  const requestThreadId =
    readString(requestParams, "threadId") ?? readString(requestParams, "conversationId");
  const requestTurnId = readString(requestParams, "turnId");

  if (!requestThreadId || !requestTurnId) return false;
  return requestThreadId === threadId && requestTurnId === turnId;
}

This ensures approvals cannot be processed unless they are explicitly associated with the active thread/turn.

Description

runCodexAppServerAttempt() starts Codex app-server threads/turns with an approval policy resolved from environment, but defaults to "never" when unset.

• The approvalPolicy value is sent to Codex app-server in both thread/start and turn/start.
• When set to "never", Codex app-server may proceed with native command execution / file change / permission requests without ever issuing */requestApproval RPCs, meaning OpenClaw’s handleCodexAppServerApprovalRequest() path is bypassed.
• This creates a risky insecure-by-default posture: simply enabling/auto-selecting the Codex app-server harness can result in actions occurring without user approval prompts.

Vulnerable code (defaulting to no approvals):

function resolveAppServerApprovalPolicy(): "never" | "on-request" | "on-failure" | "untrusted" {
  const raw = process.env.OPENCLAW_CODEX_APP_SERVER_APPROVAL_POLICY?.trim();
  if (raw === "on-request" || raw === "on-failure" || raw === "untrusted") {
    return raw;
  }
  return "never";
}

And it is used here:

turn = await client.request("turn/start", {
  ...,
  approvalPolicy: resolveAppServerApprovalPolicy(),
});

Recommendation

Make the default approval policy safe-by-default (typically "on-request"), and require an explicit opt-out to disable approvals.

Suggested change:

function resolveAppServerApprovalPolicy(): "never" | "on-request" | "on-failure" | "untrusted" {
  const raw = process.env.OPENCLAW_CODEX_APP_SERVER_APPROVAL_POLICY?.trim();
  if (raw === "never" || raw === "on-request" || raw === "on-failure" || raw === "untrusted") {
    return raw;
  }
  ​// Safe default
  return "on-request";
}

Additionally:

• Consider refusing to start the Codex app-server harness unless approvals are enabled, unless a clearly-named “I understand the risk” configuration flag is set.
• Add logging/telemetry when approvals are disabled, to make the security posture explicit.

Description

buildApprovalResponse() grants permissions for item/permissions/requestApproval by directly reflecting requestParams.permissions.network and requestParams.permissions.fileSystem back to the app-server without any schema validation or allowlisting.

• Input: requestParams is app-server-provided JSON (untrusted boundary)
• Logic flaw: requestedPermissions() only checks that network / fileSystem are JSON objects, then copies them verbatim
• Result: if the user approves the request (potentially based on an approval prompt that does not enumerate requested permissions), the app-server can obtain arbitrary network / filesystem scopes encoded in those objects.

Vulnerable code:

if (method === "item/permissions/requestApproval") {
  if (outcome === "approved-session" || outcome === "approved-once") {
    return {
      permissions: requestedPermissions(requestParams),
      scope: outcome === "approved-session" ? "session" : "turn",
    };
  }
  return { permissions: {}, scope: "turn" };
}

function requestedPermissions(requestParams: JsonObject | undefined): JsonObject {
  const permissions = isJsonObject(requestParams?.permissions) ? requestParams.permissions : {};
  const granted: JsonObject = {};
  if (isJsonObject(permissions.network)) {
    granted.network = permissions.network;
  }
  if (isJsonObject(permissions.fileSystem)) {
    granted.fileSystem = permissions.fileSystem;
  }
  return granted;
}

Additionally, the approval prompt context built in buildApprovalContext() does not include the requested permission details (it only uses command, reason, or method name), which increases the risk of a user approving overly-broad permissions without understanding the scope.

Recommendation

Treat the app-server permission request as untrusted and only grant permissions that pass a strict schema + policy.

1. Validate and normalize the requested permission objects (deep validation, types, bounds):

   • Define explicit shapes for network (e.g., allowlisted hosts, ports, protocols) and fileSystem (e.g., explicit paths, read/write flags).
   • Reject unknown keys and excessively broad wildcards.

2. Enforce an allowlist / policy at the bridge (or gateway) level so even an approved request cannot exceed configured maximums.

3. Render full requested scopes in the approval UI prompt (include normalized permissions in description), so approvals are informed.

Example (sketch):

import { z } from "zod";

const PermissionRequestSchema = z.object({
  permissions: z.object({
    network: z
      .object({ allowedHosts: z.array(z.string()).max(100) })
      .strict()
      .optional(),
    fileSystem: z
      .object({ allowedPaths: z.array(z.string()).max(100), mode: z.enum(["read","write","readwrite"]) })
      .strict()
      .optional(),
  }).strict(),
}).strict();

function requestedPermissions(requestParams: JsonObject | undefined): JsonObject {
  const parsed = PermissionRequestSchema.safeParse(requestParams);
  if (!parsed.success) return {};
  return parsed.data.permissions;
}

If permissive behavior is required for compatibility, at minimum log/deny requests that contain unknown keys or wildcards, and show the full permission JSON in the approval description.

Description

mergeAttemptToolMediaPayloads() merges attempt.toolMediaUrls (collected from tool results) into outbound reply payloads without validation. These payload mediaUrls are later consumed by the webchat path (buildWebchatAudioContentBlocksFromReplyPayloads() in src/gateway/server-methods/chat-webchat-media.ts), which will treat absolute paths / file: URLs as local files and fs.readFileSync() them when they look like audio.

Impact:

• A malicious/compromised tool can return mediaUrl(s) like file:/.../secret.mp3 or /.../secret.mp3.
• The runner will propagate these into outbound payloads.
• Webchat will read and base64-embed the referenced local file into the assistant message content, disclosing server-local files to the client.

Vulnerable code (propagation):

const mediaUrls = Array.from(
  new Set(params.toolMediaUrls?.map((url) => url.trim()).filter(Boolean) ?? []),
);
...
const mergedMediaUrls = Array.from(new Set([...(payload.mediaUrls ?? []), ...mediaUrls]));
...
mediaUrls: mergedMediaUrls.length ? mergedMediaUrls : undefined,
mediaUrl: payload.mediaUrl ?? mergedMediaUrls[0],

Downstream sink (for context, not in this diff): src/gateway/server-methods/chat-webchat-media.ts reads absolute paths / file: URLs and calls fs.readFileSync() for audio.

Recommendation

Treat tool-returned media URLs as untrusted input and enforce a strict allowlist before merging them into outbound payloads.

Suggested fixes (choose one or combine):

1. Allow only safe URL schemes (e.g. https: and optionally data:) and explicitly reject file: and absolute/relative paths:

function isSafeOutboundMediaUrl(raw: string): boolean {
  const u = raw.trim();
  if (!u) return false;
  if (u.startsWith("file:")) return false;
  if (u.startsWith("/")) return false; // blocks absolute paths
  if (/^[a-zA-Z]:\\/.test(u)) return false; // blocks Windows paths
  try {
    const parsed = new URL(u);
    return parsed.protocol === "https:"; // or allow "http:" only if you have SSRF protections
  } catch {
    return false;
  }
}

const mediaUrls = Array.from(new Set(
  (params.toolMediaUrls ?? []).filter(isSafeOutboundMediaUrl)
));

2. If local media is required, gate it behind an explicit capability/allowlist (e.g. only allow files within a dedicated media directory) and validate path traversal with path.resolve().

3. Add a second line of defense at the sink (buildWebchatAudioContentBlocksFromReplyPayloads) to require an allowlisted directory or a signed token rather than arbitrary paths/file: URLs.

Description

When running with OPENCLAW_AGENT_RUNTIME=auto (default) and PI fallback enabled, failures in the Codex agent harness can fall back to the embedded PI backend. Because the Codex provider advertises an external baseUrl (https://chatgpt.com/backend-api) and uses the openai-codex-responses transport, this fallback path can cause the PI transport to make outbound HTTPS requests to chatgpt.com and include the full prompt/messages in the request.

Key points:

• The Codex provider config sets baseUrl to an external service (chatgpt.com/backend-api).
• runAgentHarnessAttemptWithFallback() falls back to PI on any harness error when runtime is auto.
• PI's OpenAI Responses transport uses model.baseUrl for the OpenAI client base URL, so it will send the request body (prompt/messages) to that host.
• The provider also defines resolveSyntheticAuth() returning a non-secret placeholder token; if this (or some other resolved token) is used in the fallback request, it may create confusing/unsafe authentication behavior, but more importantly the request still contains sensitive prompt content.

Vulnerable code (fallback to PI on harness failure):

try {
  return await harness.runAttempt(params);
} catch (error) {
  if (policy.runtime !== "auto" || policy.fallback === "none") {
    throw error;
  }
  log.warn(`${harness.label} failed; falling back to embedded PI backend`, { error });
  return createPiAgentHarness().runAttempt(params);
}

This can result in unintended external network calls and potential disclosure of user prompts, tool outputs, or other sensitive session data to chatgpt.com.

Recommendation

Prevent Codex runs from ever using the generic PI provider transport unless explicitly opted-in.

Options:

1. Disable PI fallback when provider is Codex (recommended default):

​// before falling back
if (params.provider.trim().toLowerCase() === "codex") {
  throw error; // or require fallback:"none" for codex
}

2. Introduce a provider-level guard in the PI embedded runner to refuse using openai-codex-responses (or baseUrl containing chatgpt.com) unless an explicit allowlist flag is enabled.

3. Change Codex provider catalog to not expose an external baseUrl/transport when it is intended to be harness-only (e.g., mark as harness-only, or set a dummy baseUrl and ensure transport cannot run).

Also consider setting the default embedded harness policy for Codex agents to fallback: "none" so harness errors fail closed instead of falling back to an outbound network path.

Description

CodexAppServerClient spawns an app-server and processes newline-delimited JSON-RPC messages from stdout using readline.

There are no limits or backpressure safeguards:

• Unbounded message size / line length: readline.createInterface({ input: child.stdout }) will buffer until a newline is seen. A malicious or buggy app-server can emit an extremely long line (or never emit a newline), causing large memory usage and high CPU during line.trim() and JSON.parse().
• Unbounded in-flight requests: request() inserts an entry into pending but there is no per-request timeout and no cap on pending size. If the app-server stops responding (or responses are withheld), the pending map can grow without bound (e.g., repeated turn/steer calls), leading to memory growth.

Vulnerable code:

this.lines = createInterface({ input: child.stdout });
this.lines.on("line", (line) => this.handleLine(line));
...
parsed = JSON.parse(trimmed);
...
this.pending.set(id, { ... });

Because the app-server binary and args can be overridden via environment variables (OPENCLAW_CODEX_APP_SERVER_BIN, OPENCLAW_CODEX_APP_SERVER_ARGS), this is reachable in deployments where environment/config is not fully trusted, and it is also a robustness issue even when trusted (buggy server).

Recommendation

Add explicit resource limits and timeouts at the transport/client layer:

1. Enforce a maximum message size (and optionally a maximum JSON nesting depth):

• Replace readline with a small framing parser that buffers data up to MAX_MESSAGE_BYTES and rejects/terminates the child if exceeded.
• Alternatively, wrap child.stdout with a Transform that enforces a maximum line length.

2. Add per-request timeouts and/or a maximum pending cap:

• Allow request(method, params, { timeoutMs }) and reject (and delete) the pending entry on timeout.
• Consider a global cap such as MAX_PENDING_REQUESTS; if exceeded, reject new requests or close the client.

Example (sketch):

request<T>(method: string, params?: JsonValue, opts: { timeoutMs?: number } = {}): Promise<T> {
  const id = this.nextId++;
  return new Promise<T>((resolve, reject) => {
    const timeout = opts.timeoutMs
      ? setTimeout(() => {
          this.pending.delete(id);
          reject(new Error(`${method} timed out`));
        }, opts.timeoutMs)
      : undefined;

    this.pending.set(id, {
      method,
      resolve: (v) => { if (timeout) clearTimeout(timeout); resolve(v as T); },
      reject: (e) => { if (timeout) clearTimeout(timeout); reject(e); },
    });

    this.writeMessage({ id, method, params });
  });
}

3. Consider rate-limiting / truncating stderr logging to avoid log flooding.

These changes ensure a misbehaving app-server cannot force unbounded memory/CPU growth in the host process.

Description

The Codex app-server child process stderr stream is logged verbatim at debug level. stderr output from external processes commonly contains sensitive information (API tokens, auth headers, file paths, user prompts, command lines, stack traces).

• Input/source: chunk data from child.stderr (external process output)
• Sink: embeddedAgentLog.debug(...) without redaction or length limiting
• If debug logging is enabled (e.g., via config/env), these values may be written to console and/or persisted to log files, causing information disclosure.

Vulnerable code:

child.stderr.on("data", (chunk: Buffer | string) => {
  const text = chunk.toString("utf8").trim();
  if (text) {
    embeddedAgentLog.debug(`codex app-server stderr: ${text}`);
  }
});

Recommendation

Avoid logging external process stderr verbatim, or ensure it is sanitized and bounded.

Options:

• Log only a short, redacted summary (and/or only on non-zero exit).
• Apply a secret redaction function before logging.
• Add a configuration flag to enable raw stderr logging only for local debugging.

Example (bounded + redaction):

import { redact } from "../../../logging/redact.js";

child.stderr.on("data", (chunk) => {
  const raw = chunk.toString("utf8");
  const bounded = raw.slice(0, 2000); // prevent huge log lines
  const safe = redact(bounded).trim();
  if (safe) {
    embeddedAgentLog.debug("codex app-server stderr (redacted): " + safe);
  }
});

Also consider logging to trace and/or gating with an explicit env var like OPENCLAW_LOG_CODEX_STDERR=1.

Description

mirrorCodexAppServerTranscript() calls readTranscriptIdempotencyKeys() which reads the entire session transcript JSONL file into memory and parses it line-by-line on every mirror attempt.

• Input/control: sessionFile is provided via EmbeddedRunAttemptParams.sessionFile (passed through from run orchestration), and its contents are whatever is on disk.
• Issue: fs.readFile(sessionFile, "utf8") loads the whole file, then raw.split(/\r?\n/) duplicates memory and iterates JSON.parse for each line.
• Impact: A very large transcript (or a transcript intentionally bloated with many lines / large lines) can cause high memory usage and significant CPU time each time transcript mirroring runs, potentially leading to process slowdown or out-of-memory termination.

Vulnerable code:

raw = await fs.readFile(sessionFile, "utf8");
for (const line of raw.split(/\r?\n/)) {
  ...
  const parsed = JSON.parse(line)
  ...
}

Recommendation

Avoid loading and splitting the entire transcript file on every run.

Options:

1. Stream the JSONL file line-by-line using fs.createReadStream + readline to keep memory bounded.
2. Add a maximum file size / maximum lines processed for idempotency scanning (e.g., only scan the last N MB/lines).
3. Persist idempotency keys separately (sidecar index file) so you don't need to re-scan the full transcript.

Example (streaming):

import fs from "node:fs";
import readline from "node:readline";

async function readTranscriptIdempotencyKeys(sessionFile: string): Promise<Set<string>> {
  const keys = new Set<string>();
  try {
    const stream = fs.createReadStream(sessionFile, { encoding: "utf8" });
    const rl = readline.createInterface({ input: stream, crlfDelay: Infinity });
    for await (const line of rl) {
      if (!line.trim()) continue;
      try {
        const parsed = JSON.parse(line) as { message?: { idempotencyKey?: unknown } };
        if (typeof parsed.message?.idempotencyKey === "string") keys.add(parsed.message.idempotencyKey);
      } catch {
        ​// ignore malformed lines
      }
    }
  } catch (e: any) {
    if (e?.code !== "ENOENT") throw e;
  }
  return keys;
}