From 3b2d0754020cd516fa56eca1a3f7b805af3359a9 Mon Sep 17 00:00:00 2001 From: NomenAK Date: Sat, 30 May 2026 13:30:04 +0000 Subject: [PATCH] =?UTF-8?q?fix(claude):=20address=20tool-cloak=20PR=20revi?= =?UTF-8?q?ew=20=E2=80=94=20preserve=20boolean=20schemas,=20null-guards,?= =?UTF-8?q?=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up commit on PR #2943 review: - Preserve boolean schemas in `sanitizeClaudeToolSchemas` (Gemini Code Assist, high severity). `additionalProperties: false` is the canonical JSON Schema lock-down for object tools; the previous coercion silently turned it into the permissive `{}`, which would invite models to hallucinate extra arguments during tool calling. Same rule now applies to per-property boolean schemas under `properties`. Placeholder strings still get the permissive `{}` slot — booleans get preserved verbatim. - Defensive null guards in `cloakThirdPartyToolNames` for `tools[]` and `messages[]` entries that might be `null`/`undefined`. Prevents a runtime `TypeError` if a malformed payload reaches the cloak. - Document `CLAUDE_DISABLE_TOOL_NAME_CLOAK` in `.env.example` and `docs/reference/ENVIRONMENT.md` (env/docs contract was failing in CI). - Regression tests covering all of the above (5 boolean preservation cases, 2 null-tolerance cases). Co-Authored-By: Claude Opus 4.8 --- .env.example | 7 ++ docs/reference/ENVIRONMENT.md | 1 + open-sse/services/claudeCodeToolRemapper.ts | 12 ++-- open-sse/translator/helpers/schemaCoercion.ts | 30 ++++++-- tests/unit/claude-oauth-tool-cloak.test.ts | 72 +++++++++++++++++++ 5 files changed, 113 insertions(+), 9 deletions(-) diff --git a/.env.example b/.env.example index 36ec68c8f6..c9153e8091 100644 --- a/.env.example +++ b/.env.example @@ -683,6 +683,13 @@ GITHUB_OAUTH_CLIENT_ID=Iv1.b507a08c87ecfe98 # Update these when providers release new CLI versions to avoid blocks. 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 +# 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). +# CLAUDE_DISABLE_TOOL_NAME_CLOAK=false CODEX_USER_AGENT="codex-cli/0.132.0 (Windows 10.0.26200; x64)" GITHUB_USER_AGENT="GitHubCopilotChat/0.45.1" ANTIGRAVITY_USER_AGENT="antigravity/2.0.1 linux/arm64 google-api-nodejs-client/10.3.0" diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index 3c03e5ac41..5b371aa5b7 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -435,6 +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. | | `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/services/claudeCodeToolRemapper.ts b/open-sse/services/claudeCodeToolRemapper.ts index 9b6cf7d9cf..688b8381b4 100644 --- a/open-sse/services/claudeCodeToolRemapper.ts +++ b/open-sse/services/claudeCodeToolRemapper.ts @@ -203,7 +203,7 @@ export function cloakThirdPartyToolNames(body: Record): Map(); if (Array.isArray(tools)) { for (const tool of tools) { - if (typeof tool.name === "string") used.add(tool.name); + if (tool && typeof tool.name === "string") used.add(tool.name); } } const existingMap = @@ -236,7 +236,7 @@ export function cloakThirdPartyToolNames(body: Record): Map): Map> | undefined; if (Array.isArray(messages)) { for (const message of messages) { - const content = message.content as Array> | undefined; + const content = message?.content as Array> | undefined; if (!Array.isArray(content)) continue; for (const block of content) { - if (block?.type === "tool_use" && typeof block.name === "string" && needsThirdPartyCloak(block.name)) { + if ( + block?.type === "tool_use" && + typeof block.name === "string" && + needsThirdPartyCloak(block.name) + ) { block.name = aliasFor(block.name); } } diff --git a/open-sse/translator/helpers/schemaCoercion.ts b/open-sse/translator/helpers/schemaCoercion.ts index e5d94100ae..efcd5dccb5 100644 --- a/open-sse/translator/helpers/schemaCoercion.ts +++ b/open-sse/translator/helpers/schemaCoercion.ts @@ -297,8 +297,19 @@ export function stripInvalidSchemaConstructs(schema: unknown): unknown { continue; } if (SCHEMA_SLOT_KEYS.includes(key)) { - result[key] = - isPlainObject(value) || Array.isArray(value) ? stripInvalidSchemaConstructs(value) : {}; + // Boolean schemas are valid in JSON Schema (e.g. `additionalProperties: false` + // locks down the object); coercing to {} would silently allow extras and + // invite the model to hallucinate arguments. Only placeholder strings + // (e.g. "[MaxDepth]") get replaced with the permissive {}. + if (isPlainObject(value) || Array.isArray(value)) { + result[key] = stripInvalidSchemaConstructs(value); + } else if (typeof value === "boolean") { + result[key] = value; + } else if (isSchemaPlaceholder(value)) { + result[key] = {}; + } else { + result[key] = value; + } continue; } if (key === "const") { @@ -309,9 +320,18 @@ export function stripInvalidSchemaConstructs(schema: unknown): unknown { if (key === "properties" && isPlainObject(value)) { const properties: JsonRecord = {}; for (const [propName, propSchema] of Object.entries(value)) { - properties[propName] = isPlainObject(propSchema) - ? stripInvalidSchemaConstructs(propSchema) - : {}; + // Same boolean-preservation rule as SCHEMA_SLOT_KEYS above: + // `{ properties: { onlyAdminCanSet: false } }` is a valid permission + // gate and must not be silently turned into the permissive {}. + if (isPlainObject(propSchema) || Array.isArray(propSchema)) { + properties[propName] = stripInvalidSchemaConstructs(propSchema); + } else if (typeof propSchema === "boolean") { + properties[propName] = propSchema; + } else if (isSchemaPlaceholder(propSchema)) { + properties[propName] = {}; + } else { + properties[propName] = propSchema; + } } result[key] = properties; continue; diff --git a/tests/unit/claude-oauth-tool-cloak.test.ts b/tests/unit/claude-oauth-tool-cloak.test.ts index 12740d0a0f..0e5a52e900 100644 --- a/tests/unit/claude-oauth-tool-cloak.test.ts +++ b/tests/unit/claude-oauth-tool-cloak.test.ts @@ -107,3 +107,75 @@ describe("cloakThirdPartyToolNames", () => { assert.equal(needsThirdPartyCloak("mixture_of_agents"), true); }); }); + +describe("sanitizeClaudeToolSchemas — boolean schema preservation", () => { + it("preserves additionalProperties: false (canonical lock-down)", () => { + const s = sanitizeClaudeToolSchema({ + type: "object", + properties: { a: { type: "string" } }, + additionalProperties: false, + }) as AnyRecord; + assert.equal(s.additionalProperties, false); + }); + + it("preserves additionalProperties: true", () => { + const s = sanitizeClaudeToolSchema({ + type: "object", + additionalProperties: true, + }) as AnyRecord; + assert.equal(s.additionalProperties, true); + }); + + it("preserves boolean property schemas under properties", () => { + const s = sanitizeClaudeToolSchema({ + type: "object", + properties: { allowed: true, denied: false }, + }) as AnyRecord; + const props = s.properties as AnyRecord; + assert.equal(props.allowed, true); + assert.equal(props.denied, false); + }); + + it("preserves boolean unevaluatedProperties", () => { + const s = sanitizeClaudeToolSchema({ + type: "object", + unevaluatedProperties: false, + }) as AnyRecord; + assert.equal(s.unevaluatedProperties, false); + }); + + it("still replaces a placeholder string in a slot key with permissive {}", () => { + const s = sanitizeClaudeToolSchema({ + type: "object", + additionalProperties: "[MaxDepth]", + }) as AnyRecord; + assert.deepEqual(s.additionalProperties, {}); + }); +}); + +describe("cloakThirdPartyToolNames — defensive null guards", () => { + it("tolerates null/undefined entries in tools[]", () => { + const body: AnyRecord = { + tools: [null, { name: "read_file" }, undefined, { name: "Bash" }], + }; + cloakThirdPartyToolNames(body); + const names = (body.tools as Array).map((t) => t?.name); + assert.deepEqual(names, [undefined, "Read", undefined, "Bash"]); + }); + + it("tolerates null/undefined entries in messages[]", () => { + const body: AnyRecord = { + tools: [{ name: "read_file" }], + messages: [ + null, + { role: "assistant", content: [{ type: "tool_use", name: "read_file" }] }, + undefined, + ], + }; + cloakThirdPartyToolNames(body); + const block = ( + (body.messages as Array)[1].content as Array + )[0]; + assert.equal(block.name, "Read"); + }); +});