fix(claude): address tool-cloak PR review — preserve boolean schemas, null-guards, docs

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 <noreply@anthropic.com>
This commit is contained in:
NomenAK
2026-05-30 13:30:04 +00:00
parent 7e7faad079
commit 3b2d075402
5 changed files with 113 additions and 9 deletions

View File

@@ -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"

View File

@@ -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 |

View File

@@ -203,7 +203,7 @@ export function cloakThirdPartyToolNames(body: Record<string, unknown>): Map<str
const used = new Set<string>();
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<string, unknown>): Map<str
if (Array.isArray(tools)) {
for (const tool of tools) {
if (typeof tool.name === "string" && needsThirdPartyCloak(tool.name)) {
if (tool && typeof tool.name === "string" && needsThirdPartyCloak(tool.name)) {
tool.name = aliasFor(tool.name);
}
}
@@ -245,10 +245,14 @@ export function cloakThirdPartyToolNames(body: Record<string, unknown>): Map<str
const messages = body.messages as Array<Record<string, unknown>> | undefined;
if (Array.isArray(messages)) {
for (const message of messages) {
const content = message.content as Array<Record<string, unknown>> | undefined;
const content = message?.content as Array<Record<string, unknown>> | 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);
}
}

View File

@@ -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;

View File

@@ -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<AnyRecord | null | undefined>).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<AnyRecord>)[1].content as Array<AnyRecord>
)[0];
assert.equal(block.name, "Read");
});
});