diff --git a/src/server/responses/collaboration.ts b/src/server/responses/collaboration.ts index ab6a9b55c9..a7552adb86 100644 --- a/src/server/responses/collaboration.ts +++ b/src/server/responses/collaboration.ts @@ -152,6 +152,38 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato dottedAliasOwners.set(t.name, null); } } + // Bare echo alias (`name` with no namespace spelling, #4679): some providers — observed + // on the muse family via Command Code — echo a namespaced tool by its bare name. The + // bare spelling is only a safe alias while it names ONE tool and cannot be read as + // another identity's canonical or dotted spelling. + // Code-mode helper spellings never gain a bare alias (#4679 review): admitting bare + // `exec` into the declared set would authorize the unrelated helper normalization that + // the CODE_MODE_EXEC exception exists to contain. + const BARE_ECHO_EXCLUDED_NAMES = new Set([ + "exec", "exec_command", "shell_command", "write_stdin", "apply_patch", "view_image", + ]); + const bareAliasOwners = new Map(); + for (const t of authorizedTools) { + // Bare (no-namespace) declarations participate as owners too: a namespaced tool whose + // bare name equals a bare-declared function must not gain the bare alias, mirroring how + // the tool_choice bare path refuses ambiguous owners across the whole request catalog. + const identity = JSON.stringify([t.namespace ?? null, t.name]); + const owner = bareAliasOwners.get(t.name); + if (owner === undefined) bareAliasOwners.set(t.name, identity); + else if (owner !== identity) bareAliasOwners.set(t.name, null); + } + for (const t of authorizedTools) { + const canonical = namespacedToolName(t.namespace, t.name); + const owner = bareAliasOwners.get(canonical); + if (owner !== undefined && owner !== JSON.stringify([t.namespace, t.name])) { + bareAliasOwners.set(canonical, null); + } + const dotted = dottedToolName(t.namespace, t.name); + const dottedOwner = bareAliasOwners.get(dotted); + if (dottedOwner !== undefined && dottedOwner !== JSON.stringify([t.namespace, t.name])) { + bareAliasOwners.set(dotted, null); + } + } for (const t of authorizedTools) { // Upstream output is untrusted: only restore calls for tools the caller authorized. const wireName = namespacedToolName(t.namespace, t.name); @@ -175,6 +207,23 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato toolNsMap.set(dottedName, { namespace: t.namespace, name: t.name, ...(t.freeform ? { freeform: true } : {}) }); if (t.parameters && typeof t.parameters === "object") toolParameterSchemas.set(dottedName, t.parameters); } + // Bare echo alias (`name` with no namespace spelling, #4679): same tool identity as + // the flattened wire name, so a provider that drops the namespace prefix still + // restores against this entry. Ambiguous bare names were resolved to null above; + // skipping them falls back to the spellings every provider can still echo. + // Code-mode helper spellings never gain a bare alias: admitting bare `exec` into the + // declared set would let normalizeDeclaredToolName authorize the unrelated helper + // names, the exact surface the CODE_MODE_EXEC exception exists to contain. + if ( + bareAliasOwners.get(t.name) === JSON.stringify([t.namespace, t.name]) + && !BARE_ECHO_EXCLUDED_NAMES.has(t.name) + ) { + budget?.chargeRetained(new TextEncoder().encode(t.name).byteLength, { kind: "retained_collectors" }); + declaredToolNames.add(t.name); + budget?.chargeRetained(new TextEncoder().encode(JSON.stringify([t.name, t.namespace, t.name])).byteLength, { kind: "retained_collectors" }); + toolNsMap.set(t.name, { namespace: t.namespace, name: t.name, ...(t.freeform ? { freeform: true } : {}) }); + if (t.parameters && typeof t.parameters === "object") toolParameterSchemas.set(t.name, t.parameters); + } } if (t.freeform) { budget?.chargeRetained(new TextEncoder().encode(t.name).byteLength, { kind: "retained_collectors" }); diff --git a/src/server/responses/passthrough-dispatch.ts b/src/server/responses/passthrough-dispatch.ts index f5c5b94694..6d612c9652 100644 --- a/src/server/responses/passthrough-dispatch.ts +++ b/src/server/responses/passthrough-dispatch.ts @@ -346,10 +346,13 @@ export async function preparePassthroughExchange( const declaredWireToolNames = new Set(); const declaredBareWireToolNames = new Set(); const declaredNamelessClientCallTypes = new Set(); - // `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. + // `buildToolBridgeMaps` adds each eligible bare alias to `declaredToolNames` and `toolNsMap` + // (one authorized identity claims the bare name). `refreshUndeclaredToolGuard` normally copies + // those entries into `declaredWireToolNames`, but passthrough restoration runs before the + // undeclared-tool guard, so restore that request-bounded identity here, before authorization + // checks. `exec` uses separate handling: its bridge alias is copied into the declared set only + // when the client itself declared bare `exec`, because otherwise code-mode normalization could + // authorize the unrelated code-mode helper names. const authorizedBareNamespaceToolAliases: RoutedNamespaceToolAliases = new Map( [...toolBridgeMaps.toolNsMap].flatMap(([alias, identity]) => alias === identity.name diff --git a/tests/responses/bare-echo-alias.test.ts b/tests/responses/bare-echo-alias.test.ts new file mode 100644 index 0000000000..8b9c973971 --- /dev/null +++ b/tests/responses/bare-echo-alias.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, test } from "bun:test"; +import { parseRequest } from "../../src/responses/parser"; +import { buildToolBridgeMaps } from "../../src/server/responses"; + +function collabRequest(bareName: string) { + return parseRequest({ + model: "meta/muse-spark-1.3-contributor", + input: [ + { type: "additional_tools", role: "developer", tools: [ + { type: "namespace", name: "collaboration", tools: [ + { type: "function", name: bareName, description: bareName, strict: false, parameters: { type: "object", properties: {}, required: [] } }, + ] }, + ] }, + { type: "message", role: "user", content: [{ type: "input_text", text: "run it" }] }, + ], + } as any); +} + +describe("bare echo alias for namespaced tools (#4679)", () => { + test("an unambiguous bare name is declared and restores to the namespaced identity", () => { + const maps = buildToolBridgeMaps(collabRequest("list_agents") as any); + expect(maps.declaredToolNames.has("list_agents")).toBe(true); + expect(maps.toolNsMap.get("list_agents")).toEqual({ namespace: "collaboration", name: "list_agents" }); + }); + + test("a bare name claimed by two namespaces stays undeclared (no hijack)", () => { + const parsed = parseRequest({ + model: "meta/muse-spark-1.3-contributor", + input: [ + { type: "additional_tools", role: "developer", tools: [ + { type: "namespace", name: "collaboration", tools: [ + { type: "function", name: "list_agents", description: "a", strict: false, parameters: { type: "object", properties: {}, required: [] } }, + ] }, + { type: "namespace", name: "other__ns", tools: [ + { type: "function", name: "list_agents", description: "b", strict: false, parameters: { type: "object", properties: {}, required: [] } }, + ] }, + ] }, + { type: "message", role: "user", content: [{ type: "input_text", text: "run it" }] }, + ], + } as any); + const maps = buildToolBridgeMaps(parsed as any); + expect(maps.declaredToolNames.has("list_agents")).toBe(false); + expect(maps.toolNsMap.has("list_agents")).toBe(false); + // Both canonical spellings remain declared. + expect(maps.declaredToolNames.has("collaboration__list_agents")).toBe(true); + expect(maps.declaredToolNames.has("other__ns__list_agents")).toBe(true); + }); + + test("a bare name that equals another tool's dotted spelling stays undeclared", () => { + const parsed = parseRequest({ + model: "meta/muse-spark-1.3-contributor", + input: [ + { type: "additional_tools", role: "developer", tools: [ + { type: "namespace", name: "collaboration", tools: [ + { type: "function", name: "list_agents", description: "a", strict: false, parameters: { type: "object", properties: {}, required: [] } }, + ] }, + { type: "namespace", name: "mcp__x", tools: [ + { type: "function", name: "collaboration.list_agents", description: "b", strict: false, parameters: { type: "object", properties: {}, required: [] } }, + ] }, + ] }, + { type: "message", role: "user", content: [{ type: "input_text", text: "run it" }] }, + ], + } as any); + const maps = buildToolBridgeMaps(parsed as any); + // Tool B's bare name ("collaboration.list_agents") collides with tool A's dotted + // spelling, so that bare alias is poisoned; tool A's dotted spelling is poisoned in + // return by the pre-existing dotted rule. Tool B's own distinct dotted alias does not + // collide with anything and stays declared, as do both canonical spellings. + expect(maps.declaredToolNames.has("collaboration.list_agents")).toBe(false); + expect(maps.toolNsMap.has("collaboration.list_agents")).toBe(false); + expect(maps.declaredToolNames.has("mcp__x.collaboration.list_agents")).toBe(true); + expect(maps.toolNsMap.get("mcp__x.collaboration.list_agents")).toEqual({ namespace: "mcp__x", name: "collaboration.list_agents" }); + expect(maps.declaredToolNames.has("collaboration__list_agents")).toBe(true); + expect(maps.declaredToolNames.has("mcp__x__collaboration.list_agents")).toBe(true); + }); + + test("a bare name that equals another tool's canonical spelling stays undeclared", () => { + const parsed = parseRequest({ + model: "meta/muse-spark-1.3-contributor", + input: [ + { type: "additional_tools", role: "developer", tools: [ + { type: "namespace", name: "collaboration", tools: [ + { type: "function", name: "list_agents", description: "a", strict: false, parameters: { type: "object", properties: {}, required: [] } }, + ] }, + { type: "namespace", name: "mcp__x", tools: [ + { type: "function", name: "collaboration__list_agents", description: "b", strict: false, parameters: { type: "object", properties: {}, required: [] } }, + ] }, + ] }, + { type: "message", role: "user", content: [{ type: "input_text", text: "run it" }] }, + ], + } as any); + const maps = buildToolBridgeMaps(parsed as any); + // Tool B's bare name ("collaboration__list_agents") is also tool A's declared canonical + // spelling, so the bare alias is poisoned. The canonical spelling stays declared — but as + // tool A's wire name, never as an alias of tool B — so assert the identity via toolNsMap. + // Tool B's canonical and dotted spellings remain declared. + expect(maps.toolNsMap.get("collaboration__list_agents")).toEqual({ namespace: "collaboration", name: "list_agents" }); + expect(maps.declaredToolNames.has("mcp__x__collaboration__list_agents")).toBe(true); + expect(maps.declaredToolNames.has("mcp__x.collaboration__list_agents")).toBe(true); + expect(maps.toolNsMap.get("mcp__x.collaboration__list_agents")).toEqual({ namespace: "mcp__x", name: "collaboration__list_agents" }); + }); + + test("a bare-declared function owns its name and blocks the namespaced tool's bare alias", () => { + const parsed = parseRequest({ + model: "meta/muse-spark-1.3-contributor", + input: [ + { type: "additional_tools", role: "developer", tools: [ + { type: "namespace", name: "collaboration", tools: [ + { type: "function", name: "list_agents", description: "a", strict: false, parameters: { type: "object", properties: {}, required: [] } }, + ] }, + { type: "function", name: "list_agents", description: "b", strict: false, parameters: { type: "object", properties: {}, required: [] } }, + ] }, + { type: "message", role: "user", content: [{ type: "input_text", text: "run it" }] }, + ], + } as any); + const maps = buildToolBridgeMaps(parsed as any); + // The bare-declared (no-namespace) function participates as an owner of "list_agents", + // mirroring the tool_choice bare path's whole-catalog counting, so the namespaced tool + // must not gain it as an echo alias. "list_agents" stays in declaredToolNames because the + // bare function's own wire name IS that spelling; the alias check is toolNsMap, which + // must never map the bare name to the namespaced identity. + expect(maps.toolNsMap.has("list_agents")).toBe(false); + expect(maps.declaredToolNames.has("collaboration__list_agents")).toBe(true); + }); +});