Description

createCodexDynamicToolBridge now conditionally skips wrapToolWithBeforeToolCallHook() when isToolWrappedWithBeforeToolCallHook(tool) returns true. The wrapped status is determined by reading a module-private Symbol property on the tool object.

In JavaScript/TypeScript, an untrusted tool provider can supply a Proxy object that returns true for any property access (including unknown symbols). This allows the tool to claim it is wrapped while actually providing an execute() that never runs runBeforeToolCallHook(), bypassing enforcement performed by the before_tool_call hook (blocking, approval gating, parameter normalization/loop detection).

Vulnerable logic:

const tools = params.tools.map((tool) =>
  isToolWrappedWithBeforeToolCallHook(tool)
    ? tool
    : wrapToolWithBeforeToolCallHook(tool, params.hookContext),
);

Because isToolWrappedWithBeforeToolCallHook() performs a simple property read (taggedTool[BEFORE_TOOL_CALL_WRAPPED] === true), a Proxy can spoof the marker without knowing the symbol value.

Recommendation

Do not rely on a property on the tool object to decide whether to enforce before_tool_call.

Prefer one of:

1. Always wrap tools at the boundary where untrusted tools enter (simplest):

const tools = params.tools.map((tool) => wrapToolWithBeforeToolCallHook(tool, params.hookContext));

2. Track wrapped tools in a module-private WeakSet (cannot be spoofed by Proxy/property tricks):

​// in pi-tools.before-tool-call.ts
const wrappedTools = new WeakSet<object>();

export function wrapToolWithBeforeToolCallHook(tool: AnyAgentTool, ctx?: HookContext): AnyAgentTool {
  ​// ... create wrappedTool ...
  wrappedTools.add(wrappedTool as object);
  return wrappedTool;
}

export function isToolWrappedWithBeforeToolCallHook(tool: AnyAgentTool): boolean {
  return wrappedTools.has(tool as object);
}

If the intent is to avoid double wrapping only for trusted internal code paths, consider making the “already wrapped” optimization internal-only (not based on untrusted inputs), or accept double-wrapping but ensure idempotency inside the wrapper.

Description

The before_tool_call hook can return params that are merged into the tool's original arguments using object spread. Because the merge does not block special keys like __proto__, constructor, or prototype, a malicious (or compromised) plugin can inject these properties and alter the prototype of the resulting params object (or otherwise introduce surprising inherited properties).

Impact:

• A plugin can return hookResult.params containing __proto__ to change the prototype of the merged params object.
• Tool implementations that subsequently rely on property checks (e.g., in, hasOwnProperty, truthiness checks, schema validation that doesn't guard inherited props) can be bypassed.
• The polluted params object is passed directly to tool execute(...), potentially enabling unexpected argument injection into sensitive tools (e.g., exec/gateway/message tools).

Vulnerable code:

if (isPlainObject(originalParams)) {
  return { ...originalParams, ...approvalParams };
}

Recommendation

Sanitize/disallow prototype-mutating keys when incorporating hook-provided params, and ensure the merged object has a safe prototype.

Suggested fix (shallow merge with key filtering and null-prototype target):

const FORBIDDEN_KEYS = new Set(["__proto__", "prototype", "constructor"]);

function safeShallowMerge(
  base: Record<string, unknown>,
  overrides: Record<string, unknown>,
): Record<string, unknown> {
  const out: Record<string, unknown> = Object.create(null);
  for (const [k, v] of Object.entries(base)) {
    if (!FORBIDDEN_KEYS.has(k)) out[k] = v;
  }
  for (const [k, v] of Object.entries(overrides)) {
    if (!FORBIDDEN_KEYS.has(k)) out[k] = v;
  }
  return out;
}

​// ...
if (isPlainObject(originalParams) && isPlainObject(approvalParams)) {
  return safeShallowMerge(originalParams, approvalParams);
}

Additionally:

• Consider validating hookResult.params against the tool's input schema (or at least ensuring only existing/allowed keys are overridden).
• If deep merges are introduced later, use a prototype-pollution-safe merge implementation.

Description

In createCodexDynamicToolBridge.handleToolCall, tool results are passed through Codex app-server extension middleware (tool_result) before telemetry extraction/filtering and before after_tool_call hooks receive the result.

Because media URL filtering (filterToolResultMediaUrls) decides whether to allow non-HTTP(S) paths based on mutable fields in the tool result (notably details.mcpServer / details.mcpTool checked by isExternalToolResult()), a middleware extension can:

• Strip details.mcpServer / details.mcpTool (making an external/MCP result appear “non-external”)
• Inject details.media.mediaUrls or MEDIA: tokens containing local file paths (e.g. /etc/passwd, file:///..., MEDIA:/tmp/...)
• Cause extractToolResultMediaArtifact() + filterToolResultMediaUrls() to treat the tool as trusted (based on toolName) and then persist these local paths into toolTelemetry.toolMediaUrls, which are included in the turn snapshot (event-projector.ts)

This is a trust-boundary issue: extension middleware can arbitrarily rewrite security-relevant metadata in tool results in a way that bypasses the “external tool” media restriction and can lead to unsafe content being persisted/logged/telemetry’d.

Vulnerable flow:

• tool execution: rawResult = await tool.execute(...)
• extension rewrite: result = await extensionRunner.applyToolResultExtensions(...)
• media extraction/filtering uses rewritten result and rewritten result.details

Vulnerable code:

const rawResult = await tool.execute(call.callId, preparedArgs, params.signal);
const result = await extensionRunner.applyToolResultExtensions({ ... , result: rawResult });
collectToolTelemetry({ toolName: tool.name, args, result, telemetry, isError: false });

Recommendation

Make media-trust decisions based on tool identity/source, not on mutable tool-result fields that middleware can rewrite.

Concrete options:

1. Compute external/trusted status before middleware and carry it alongside the result:

const rawResult = await tool.execute(...);
const mediaTrustContext = {
  toolName: tool.name,
  isExternal: isExternalToolResult(rawResult),
};
const result = await extensionRunner.applyToolResultExtensions({ ..., result: rawResult });
collectToolTelemetry({ ..., result, mediaTrustContext });

Then in telemetry filtering, if mediaTrustContext.isExternal === true, only allow http(s) regardless of how middleware rewrites result.details.

2. Pass the run’s registered tool-name set into filterToolResultMediaUrls(..., builtinToolNames) to prevent name-collision trust inheritance, and additionally treat any result touched by extensions as untrusted for local media unless explicitly whitelisted.

3. If extensions are not fully trusted, enforce that tool_result middleware may only redact/transform content but cannot introduce new media URLs/paths; e.g., diff the extracted media URLs before/after middleware and drop any additions unless the tool is trusted.

Description

The dynamic tool bridge returns the caught exception text directly to the model/user and also forwards it to the after_tool_call hook.

• Any dynamic tool can throw an Error whose .message may contain sensitive internal details (filesystem paths, internal hostnames, command output, credentials embedded in error strings, etc.).
• Non-Error throws are stringified via String(error), which can leak serialized objects.
• The same unsanitized error string is sent to plugins via runAgentHarnessAfterToolCallHook, expanding the disclosure surface.

Vulnerable code:

error: error instanceof Error ? error.message : String(error),
...
text: error instanceof Error ? error.message : String(error),

Recommendation

Return a generic, user-safe error message, while logging the full exception server-side. If you need to expose details to trusted plugins, pass a structured error object with a sanitized message and an opaque error code.

Example:

} catch (err) {
  const errorId = crypto.randomUUID();
  log.error({ errorId, err }, "dynamic tool execution failed");

  const safeMessage = "Tool execution failed";

  void runAgentHarnessAfterToolCallHook({
    toolName: tool.name,
    toolCallId: call.callId,
    runId: params.hookContext?.runId,
    startArgs: args,
    ​// Only safe info:
    error: `${safeMessage} (id: ${errorId})`,
    startedAt,
  });

  return {
    success: false,
    contentItems: [{ type: "inputText", text: safeMessage }],
  };
}

Optionally gate detailed error forwarding behind an explicit debug/privileged mode and never include stack traces/tokens in user-visible messages.