Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/server/responses-undeclared-tool-guard.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { namespacedToolName, normalizeDeclaredToolName } from "../types";
import {
CODE_MODE_EXEC_TOOL_NAME,
namespacedToolName,
normalizeDeclaredToolName,
} from "../types";
import { sseDataPayload, type SseBlockRewrite } from "./sse-payload-rewrite";

/** Item types the client executes through a request-declared wire name. */
const CLIENT_EXECUTED_CALL_TYPES = new Set(["function_call", "custom_tool_call"]);
/** Codex groups ordinary top-level tools here; unlike an MCP namespace, it has no wire prefix. */
const BUILTIN_FUNCTIONS_NAMESPACE = "functions";

/**
* Hosted declarations whose response items the PROVIDER executes, keyed by the request
Expand Down Expand Up @@ -79,11 +85,18 @@ function addWireToolName(names: Set<string>, tool: unknown, namespace?: string):
? nestedFunction.name
: undefined;
if (!name) return;
names.add(name);
// Codex routes MCP calls by an explicit `namespace` field, so the same tool is reachable
// as a bare inner name or as the flattened form; accept both rather than guess which
// coordinate system this provider echoes back.
if (namespace) names.add(namespacedToolName(namespace, name));
if (!namespace || namespace === BUILTIN_FUNCTIONS_NAMESPACE) {
names.add(name);
return;
}
names.add(namespacedToolName(namespace, name));
// `exec` is the one name that also switches on nested-helper normalization, so a bare alias
// for a namespaced MCP tool would silently authorize `exec_command`/`shell_command`/
// `apply_patch` the request never declared. Every other inner name keeps the bare alias.
if (name !== CODE_MODE_EXEC_TOOL_NAME) names.add(name);
Comment thread
luvs01 marked this conversation as resolved.
}

/**
Expand Down
47 changes: 42 additions & 5 deletions src/server/responses/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ import {
import { injectionDebugLog } from "../../lib/injection-debug-log";
import { resolveClientRetryAfter } from "../../lib/retry-after";
import { enrichOpenCodeZenRateLimitMessage } from "../../providers/opencode-zen-rate-limit";
import { modelInList, namespacedToolName } from "../../types";
import { CODE_MODE_EXEC_TOOL_NAME, modelInList, namespacedToolName } from "../../types";
import type {
AdapterEvent,
OcxConfig,
Expand Down Expand Up @@ -351,6 +351,7 @@ import { createRoutedToolSearchRestoreBlockRewrite } from "../responses-tool-sea
import {
createRoutedNamespaceCallRestoreRewrite,
NamespaceToolCollisionError,
restoreRoutedNamespaceCalls,
restoreRoutedNamespaceCallsInJson,
type RoutedNamespaceToolAliases,
} from "../../responses/namespace-tool-compat";
Expand Down Expand Up @@ -3634,6 +3635,23 @@ async function handleResponsesInner(
let outboundRequestBody: Record<string, unknown> | undefined;
const declaredWireToolNames = new Set<string>();
const declaredNamelessClientCallTypes = new Set<string>();
// `buildToolBridgeMaps` creates a bare alias only when the caller selected exactly one
// namespaced tool through a bare tool_choice. Restore that request-bounded identity before
// authorization checks instead of admitting the bare name into the declared set: for `exec`,
// the latter would also authorize the unrelated code-mode helper names.
const authorizedBareNamespaceToolAliases: RoutedNamespaceToolAliases = new Map(
[...toolBridgeMaps.toolNsMap].flatMap(([alias, identity]) =>
alias === identity.name
? [[alias, {
namespace: identity.namespace,
name: identity.name,
kind: identity.freeform ? "custom" as const : "function" as const,
}] as const]
: []
),
);
const restoreAuthorizedBareNamespaceToolCalls = (value: unknown): unknown =>
restoreRoutedNamespaceCalls(value, authorizedBareNamespaceToolAliases).value;
let undeclaredToolGuardActive = false;
const refreshUndeclaredToolGuard = (builtRequest: AdapterRequest): void => {
outboundRequestBody = parseOutboundRequestBody(builtRequest.body);
Expand Down Expand Up @@ -3677,7 +3695,19 @@ async function handleResponsesInner(
// however, the parsed maps also contain historical catalog entries, so only the bounded
// current-turn wire snapshot above may authorize a call.
if (replayedInputPrefixLength === 0) {
for (const name of toolBridgeMaps.declaredToolNames) declaredWireToolNames.add(name);
for (const name of toolBridgeMaps.declaredToolNames) {
// `buildToolBridgeMaps` also aliases a namespaced tool under its bare name when the
// caller's `tool_choice` selected it unambiguously, which the bridge needs to route the
// call back. For `exec` alone that alias would also switch on nested-helper
// normalization and re-authorize `exec_command`/`shell_command`/`apply_patch`, so it is
// admitted here only when the caller's own catalog declared a bare `exec`. Selecting an
// MCP `exec` is not a declaration of the code-mode shell tool.
if (
name === CODE_MODE_EXEC_TOOL_NAME
&& !clientDeclaredWireToolNames.has(CODE_MODE_EXEC_TOOL_NAME)
) continue;
declaredWireToolNames.add(name);
}
}
undeclaredToolGuardActive = (
declaredWireToolNames.size > 0
Expand All @@ -3703,7 +3733,7 @@ async function handleResponsesInner(
// state for exactly the passthrough traffic the guard deliberately stands down for.
if (!undeclaredToolGuardActive || inspectionSawUndeclaredTool) return;
if (undeclaredToolCallName(
payload,
restoreAuthorizedBareNamespaceToolCalls(payload),
declaredWireToolNames,
declaredNamelessClientCallTypes,
providerExecutedCallTypes,
Expand All @@ -3715,7 +3745,7 @@ async function handleResponsesInner(
? (response: { id?: unknown; output?: unknown; status?: unknown }) => {
if (inspectionSawUndeclaredTool) return;
const restoredResponse = restoreRoutedCustomCalls(
response,
restoreAuthorizedBareNamespaceToolCalls(response),
routedCustomToolNames,
routedCustomToolRepairNames,
declaredWireToolNames,
Expand Down Expand Up @@ -4449,6 +4479,9 @@ async function handleResponsesInner(
routedNamespaceToolAliases.size > 0
? createRoutedNamespaceCallRestoreRewrite(routedNamespaceToolAliases)
: undefined,
authorizedBareNamespaceToolAliases.size > 0
? createRoutedNamespaceCallRestoreRewrite(authorizedBareNamespaceToolAliases)
: undefined,
hasResponsesItemIdRepair(repairConfig)
? createResponsesItemIdPayloadRewrite(repairConfig!, translatorBudget)
: undefined,
Expand Down Expand Up @@ -4678,8 +4711,12 @@ async function handleResponsesInner(
restoreImageGenCallsInJson(text, imageGenCallAliases),
routedNamespaceToolAliases,
);
const restored = restoreRoutedCustomCallsInJson(
const restoredAuthorizedBareNamespace = restoreRoutedNamespaceCallsInJson(
restoredNamespace,
authorizedBareNamespaceToolAliases,
);
const restored = restoreRoutedCustomCallsInJson(
restoredAuthorizedBareNamespace,
routedCustomToolNames,
routedCustomToolRepairNames,
declaredWireToolNames,
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

export type { OcxTool, OcxToolChoice } from "./types/tools";
export {
CODE_MODE_EXEC_TOOL_NAME,
namespacedToolName,
normalizeDeclaredToolName,
toolChoiceAliases,
Expand Down
16 changes: 13 additions & 3 deletions src/types/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,30 @@ export function namespacedToolName(namespace: string | undefined, name: string):
const LEGACY_SHELL_BRIDGE_TOOL_NAMES = ["exec_command", "shell_command"] as const;
const CODE_MODE_HELPER_TOOL_NAMES = [...LEGACY_SHELL_BRIDGE_TOOL_NAMES, "apply_patch"] as const;

/**
* The one declared name that turns nested-helper normalization on. Declaring it is not just a
* name: it also decides whether an emitted `exec_command`/`shell_command`/`apply_patch` is
* accepted as that shell tool, so callers that build declared-name sets must add it only for a
* genuine bare declaration.
*/
export const CODE_MODE_EXEC_TOOL_NAME = "exec";

export function normalizeDeclaredToolName(
name: string,
declared: ReadonlySet<string> | undefined,
): string {
if (!declared || !declared.has("exec")) return name;
if (!declared || !declared.has(CODE_MODE_EXEC_TOOL_NAME)) return name;
if (declared.has(name)) return name;
if (name === "apply_patch") return "exec";
if (name === "apply_patch") return CODE_MODE_EXEC_TOOL_NAME;
// When the catalog explicitly declares any legacy shell bridge name, the environment
// genuinely exposes that tool — turn normalization off so a call is never mis-routed
// to `exec`.
if ((LEGACY_SHELL_BRIDGE_TOOL_NAMES as readonly string[]).some(legacy => declared.has(legacy))) {
return name;
}
return (CODE_MODE_HELPER_TOOL_NAMES as readonly string[]).includes(name) ? "exec" : name;
return (CODE_MODE_HELPER_TOOL_NAMES as readonly string[]).includes(name)
? CODE_MODE_EXEC_TOOL_NAME
: name;
}

export function toolChoiceAliases(tool: Pick<OcxTool, "namespace" | "name">): string[] {
Expand Down
Loading
Loading