From a1e0bc7469a0774bcc16ec43c043a9cce641e9f7 Mon Sep 17 00:00:00 2001 From: NomenAK Date: Sat, 30 May 2026 14:35:52 +0000 Subject: [PATCH] fix(claude): harden tool cloak + schema sanitizer (adversarial review round) Addresses confirmed findings from an adversarial review of the prior commits: - schema sanitizer: a truncation placeholder in a SCALAR annotation keyword (description/title/pattern/format) was coerced to {}, which is itself invalid draft-2020-12 and re-triggered the exact "input_schema is invalid" 400 the sanitizer exists to prevent. Placeholders are now only coerced to {} in subschema-expecting positions; scalar keywords are left untouched. - schema sanitizer: numeric-string coercion is folded into stripInvalidSchemaConstructs so it also covers contains / propertyNames / additionalItems (which coerceSchemaNumericFields never visited). - schema sanitizer: stop stripping the valid `default` keyword on the Claude native/passthrough surface (the #1782 default-strip is a translator concern; tool schemas here were previously forwarded verbatim). sanitizeClaudeToolSchema is now a single stripInvalidSchemaConstructs pass. - tool-name cloak: consult TOOL_RENAME_MAP / EXTRA_TOOL_RENAME_MAP before the generic PascalCase fallback, so the CLIProxyAPI path uses the established fingerprint-evasion aliases (subagents->SubDispatch, session_status->CheckStatus, webfetch->WebFetch, ...) identically to the native path instead of weaker first-letter casing. - kill-switch: CLAUDE_DISABLE_TOOL_NAME_CLOAK is now honoured inside cloakThirdPartyToolNames, so BOTH the native and CLIProxyAPI executor paths respect it (previously only base.ts did); .env.example + ENVIRONMENT.md updated. Regression tests added for each. Verified end-to-end through the live CPA path: mixture_of_agents, subagents, and a tool carrying placeholder descriptions and `default` values all return 200 with original names restored on the response. Co-Authored-By: Claude Opus 4.8 --- .env.example | 5 +- docs/reference/ENVIRONMENT.md | 2 +- open-sse/executors/base.ts | 4 +- open-sse/services/claudeCodeToolRemapper.ts | 13 +++- open-sse/translator/helpers/schemaCoercion.ts | 24 +++++-- tests/unit/claude-oauth-tool-cloak.test.ts | 63 +++++++++++++++++++ 6 files changed, 99 insertions(+), 12 deletions(-) diff --git a/.env.example b/.env.example index c9153e8091..c57d416049 100644 --- a/.env.example +++ b/.env.example @@ -684,8 +684,9 @@ GITHUB_OAUTH_CLIENT_ID=Iv1.b507a08c87ecfe98 CLAUDE_USER_AGENT="claude-cli/2.1.146 (external, cli)" -# Disable the deterministic tool-name cloak applied on the native Claude OAuth -# path (executors/base.ts) — third-party-harness tool names are aliased to +# Disable the deterministic tool-name cloak applied on both Anthropic-bound paths +# (executors/base.ts native OAuth + executors/cliproxyapi.ts CLIProxyAPI) — +# third-party-harness tool names are aliased to # Claude Code canonical or PascalCase forms so Anthropic does not refuse the # stream with a misleading 400 out-of-extra-usage placeholder. Set to true to # forward the original names verbatim (debugging only). diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index 5b371aa5b7..629e30a20b 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -435,7 +435,7 @@ process.env[`${PROVIDER_ID}_USER_AGENT`] | Variable | Default Value | When to Update | | ------------------------ | --------------------------------------------- | ------------------------------------------------------------- | | `CLAUDE_USER_AGENT` | `claude-cli/2.1.145 (external, cli)` | When Anthropic releases a new CLI version | -| `CLAUDE_DISABLE_TOOL_NAME_CLOAK` | `false` | `open-sse/executors/base.ts` | Set to `1`/`true` to forward third-party harness tool names verbatim to Anthropic on the native Claude OAuth path. By default the executor deterministically aliases non-Claude-Code tool names (Claude Code canonical mapping where one exists, otherwise PascalCase) and reverses them on the response via `_toolNameMap`, so harnesses with snake_case tools are not refused as fingerprinted third-party clients. Debugging only. | +| `CLAUDE_DISABLE_TOOL_NAME_CLOAK` | `false` | `executors/base.ts` + `executors/cliproxyapi.ts` | Set to `1`/`true` to forward third-party harness tool names verbatim to Anthropic on both Anthropic-bound paths (native OAuth and CLIProxyAPI). By default the executor deterministically aliases non-Claude-Code tool names (Claude Code canonical mapping where one exists, otherwise PascalCase) and reverses them on the response via `_toolNameMap`, so harnesses with snake_case tools are not refused as fingerprinted third-party clients. Debugging only. | | `CODEX_USER_AGENT` | `codex-cli/0.132.0 (Windows 10.0.26200; x64)` | When OpenAI updates the Codex CLI | | `CODEX_CLIENT_VERSION` | `0.131.0` | Override Codex client version independently of full UA string | | `GITHUB_USER_AGENT` | `GitHubCopilotChat/0.45.1` | When GitHub Copilot Chat updates | diff --git a/open-sse/executors/base.ts b/open-sse/executors/base.ts index f5a7e2829c..b04b6a073c 100644 --- a/open-sse/executors/base.ts +++ b/open-sse/executors/base.ts @@ -779,9 +779,7 @@ export class BaseExecutor { // Cloak third-party tool names + sanitize invalid tool schemas so // Anthropic does not refuse native Claude OAuth traffic with a // misleading "out of extra usage" placeholder. See Spec E. - if (process.env.CLAUDE_DISABLE_TOOL_NAME_CLOAK !== "true") { - cloakThirdPartyToolNames(tb); - } + cloakThirdPartyToolNames(tb); if (Array.isArray(tb.tools)) { tb.tools = sanitizeClaudeToolSchemas(tb.tools); } diff --git a/open-sse/services/claudeCodeToolRemapper.ts b/open-sse/services/claudeCodeToolRemapper.ts index 76b1f9f85a..16c97a7b3b 100644 --- a/open-sse/services/claudeCodeToolRemapper.ts +++ b/open-sse/services/claudeCodeToolRemapper.ts @@ -211,6 +211,12 @@ export function cloakThirdPartyToolNames( body: Record, options?: CloakOptions ): Map { + // Operator kill-switch (documented in .env.example / ENVIRONMENT.md). Checked + // here so every call site — native base.ts AND the CLIProxyAPI executor — + // honours it, rather than each caller having to remember to guard. + if (process.env.CLAUDE_DISABLE_TOOL_NAME_CLOAK === "true") { + return new Map(); + } const shouldCloak = (name: string): boolean => needsThirdPartyCloak(name) && !(options?.skip ? options.skip(name) : false); const tools = body.tools as Array> | undefined; @@ -235,7 +241,12 @@ export function cloakThirdPartyToolNames( const aliasFor = (original: string): string => { const existing = assigned.get(original); if (existing) return existing; - const base = HARNESS_CANONICAL_MAP[original] ?? toPascalCaseToolName(original); + // Prefer the established Claude Code rename maps (TOOL_RENAME_MAP spreads + // EXTRA_TOOL_RENAME_MAP) so the CPA path matches the native path exactly: + // subagents->SubDispatch, session_status->CheckStatus, webfetch->WebFetch, … + // Then harness-canonical (read_file->Read), then a generic PascalCase. + const base = + TOOL_RENAME_MAP[original] ?? HARNESS_CANONICAL_MAP[original] ?? toPascalCaseToolName(original); let alias = base; let suffix = 2; while (alias !== original && used.has(alias)) { diff --git a/open-sse/translator/helpers/schemaCoercion.ts b/open-sse/translator/helpers/schemaCoercion.ts index efcd5dccb5..28f9b27cf5 100644 --- a/open-sse/translator/helpers/schemaCoercion.ts +++ b/open-sse/translator/helpers/schemaCoercion.ts @@ -288,6 +288,14 @@ export function stripInvalidSchemaConstructs(schema: unknown): unknown { const result: JsonRecord = {}; for (const [key, value] of Object.entries(schema)) { + // Coerce string-encoded numeric constraints (e.g. minimum: "5") to numbers — + // Anthropic rejects the string form. Done here so the Claude sanitizer covers + // every slot this function recurses into (incl. contains / propertyNames / + // additionalItems, which coerceSchemaNumericFields does not visit). + if ((NUMERIC_SCHEMA_FIELDS as readonly string[]).includes(key)) { + result[key] = coerceNumericString(value); + continue; + } if (ARRAY_SCHEMA_KEYS.includes(key)) { const array = coerceIndexedObjectToArray(value); if (array === null) continue; // drop invalid non-array keyword (e.g. enum: "[MaxDepth]") @@ -350,10 +358,11 @@ export function stripInvalidSchemaConstructs(schema: unknown): unknown { result[key] = defs; continue; } - if (isSchemaPlaceholder(value)) { - result[key] = {}; - continue; - } + // Placeholders are only coerced to {} in subschema-expecting positions + // (handled in the branches above). A placeholder in a scalar annotation + // keyword (description / title / pattern / format) must stay scalar — + // turning it into {} is itself invalid draft-2020-12 and would re-trigger + // the very 400 this sanitizer prevents. result[key] = isPlainObject(value) || Array.isArray(value) ? stripInvalidSchemaConstructs(value) : value; } @@ -361,7 +370,12 @@ export function stripInvalidSchemaConstructs(schema: unknown): unknown { } export function sanitizeClaudeToolSchema(schema: unknown): unknown { - return stripInvalidSchemaConstructs(coerceSchemaNumericFields(schema)); + // stripInvalidSchemaConstructs now also coerces numeric-string constraints, so + // it is the single pass for the Claude path. We deliberately do NOT compose + // coerceSchemaNumericFields: it strips the valid `default` keyword (Fix #1782, + // a translator concern) which on the native / passthrough surface would + // silently alter tool schemas that were previously forwarded verbatim. + return stripInvalidSchemaConstructs(schema); } export function sanitizeClaudeToolSchemas(tools: unknown): unknown { diff --git a/tests/unit/claude-oauth-tool-cloak.test.ts b/tests/unit/claude-oauth-tool-cloak.test.ts index 1d4295359b..cdf1b25420 100644 --- a/tests/unit/claude-oauth-tool-cloak.test.ts +++ b/tests/unit/claude-oauth-tool-cloak.test.ts @@ -207,3 +207,66 @@ describe("cloakThirdPartyToolNames — non-mutating + skip option", () => { assert.deepEqual((body.tools as AnyRecord[]).map((t) => t.name), ["mcp_call", "Read"]); }); }); + +describe("review fixes — schema sanitizer scalar / default / numeric", () => { + it("keeps a placeholder in a scalar annotation keyword as a scalar (not {})", () => { + const s = sanitizeClaudeToolSchema({ + type: "object", + description: "[Object]", + properties: { a: { type: "string", description: "[Truncated]" } }, + }) as AnyRecord; + assert.equal(s.description, "[Object]"); + assert.equal(((s.properties as AnyRecord).a as AnyRecord).description, "[Truncated]"); + }); + + it("preserves the valid `default` keyword on the Claude path", () => { + const s = sanitizeClaudeToolSchema({ + type: "object", + properties: { mode: { type: "string", default: "replace" }, all: { type: "boolean", default: false } }, + }) as AnyRecord; + const p = s.properties as AnyRecord; + assert.equal((p.mode as AnyRecord).default, "replace"); + assert.equal((p.all as AnyRecord).default, false); + }); + + it("coerces numeric-string constraints inside contains (not only items)", () => { + const s = sanitizeClaudeToolSchema({ + type: "array", + contains: { type: "object", properties: { n: { type: "integer", minimum: "5" } } }, + }) as AnyRecord; + const n = ((s.contains as AnyRecord).properties as AnyRecord).n as AnyRecord; + assert.equal(n.minimum, 5); + }); + + it("still coerces a placeholder to {} in a real subschema slot", () => { + const s = sanitizeClaudeToolSchema({ type: "object", additionalProperties: "[MaxDepth]" }) as AnyRecord; + assert.deepEqual(s.additionalProperties, {}); + }); +}); + +describe("review fixes — established aliases + kill-switch", () => { + it("uses the established Claude Code aliases on the cloak path", () => { + const body: AnyRecord = { + tools: [{ name: "subagents" }, { name: "session_status" }, { name: "webfetch" }, { name: "todowrite" }], + }; + cloakThirdPartyToolNames(body); + assert.deepEqual( + (body.tools as AnyRecord[]).map((t) => t.name), + ["SubDispatch", "CheckStatus", "WebFetch", "TodoWrite"] + ); + }); + + it("CLAUDE_DISABLE_TOOL_NAME_CLOAK=true disables the cloak at the function level", () => { + const prev = process.env.CLAUDE_DISABLE_TOOL_NAME_CLOAK; + process.env.CLAUDE_DISABLE_TOOL_NAME_CLOAK = "true"; + try { + const body: AnyRecord = { tools: [{ name: "mixture_of_agents" }] }; + const map = cloakThirdPartyToolNames(body); + assert.equal((body.tools as AnyRecord[])[0].name, "mixture_of_agents"); + assert.equal(map.size, 0); + } finally { + if (prev === undefined) delete process.env.CLAUDE_DISABLE_TOOL_NAME_CLOAK; + else process.env.CLAUDE_DISABLE_TOOL_NAME_CLOAK = prev; + } + }); +});