fix: prevent Anthropic-specific fields from leaking into 3P requests#268
fix: prevent Anthropic-specific fields from leaking into 3P requests#268auriti wants to merge 5 commits intoGitlawb:mainfrom
Conversation
Four bugs where Anthropic-internal data was unconditionally included in requests to third-party OpenAI-compatible providers: 1. cache_control blocks sent to all providers — getPromptCachingEnabled() now returns false for openai/gemini/github/codex providers. Prevents 400 errors on strict backends like Azure. 2. Internal session headers (X-Claude-Code-Session-Id, x-anthropic-*, and potentially Anthropic API keys) forwarded to 3P endpoints — now filtered before passing to createOpenAIShimClient(). 3. Image content in tool results silently dropped — the shim only extracted text fields, losing screenshots and vision results. Now converts images to [Image](url) placeholders matching codexShim. 4. Session resume thinking blocks corrupt tool pairing — stripThinkingBlocks now runs before filterUnresolvedToolUses for 3P providers, preventing orphaned tool_result blocks that cause 400 on resume. Addresses Gitlawb#267.
gnanam1990
left a comment
There was a problem hiding this comment.
Thanks for addressing this. I don't think the leakage problem is fully closed yet.
The fixes appear to cover the env-driven shim path, but there are still gaps for providerOverride-based routing. Those paths can still carry Anthropic-specific headers and fields because the filtering and cache-control guards are not consistently applied there too.
Please extend the fix to the providerOverride path and add focused tests covering header stripping and cache-control suppression on overridden third-party requests.
Address CHANGES_REQUESTED on Gitlawb#268. The previous fix filtered Anthropic-specific headers only in client.ts (env-driven path). This adds defense-in-depth inside openaiShim itself so the guard applies regardless of how the shim client is created: - filterAnthropicHeaders() applied at OpenAIShimMessages construction time - Same filter applied to per-request options.headers in _doOpenAIRequest and the Codex path in _doRequest - Two new tests: header stripping via direct shim creation, cache_control suppression in message body forwarded to OpenAI-compatible endpoints
|
Thanks for the thorough review — you're right that the previous fix only covered the env-driven path in `client.ts`. This follow-up commit extends the guard with defense-in-depth inside `openaiShim.ts` itself, so Anthropic-specific headers are stripped regardless of how the shim client is created:
All 7 tests pass. |
|
fix conflicts @auriti |
|
Conflicts are now resolved by merging the latest I kept the leakage fix aligned with the current |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Rechecked the current head afc58c293c7b038c5da95a83b2d3e651a6f31980 against current origin/main.
The follow-up commits do close the earlier blocker from the old review thread:
- providerOverride-based routing now goes through the same Anthropic-header scrubber in
openaiShim - providerOverride requests also suppress
cache_control - the image tool-result regression is covered and passing
I still can't approve because Anthropic-specific headers can still leak through a real supported path.
-
src/services/api/client.ts:110-122, 389-412plussrc/services/api/openaiShim.ts:95-110
ANTHROPIC_CUSTOM_HEADERSis merged directly intodefaultHeadersinclient.ts. The new scrubber inopenaiShim.tsonly removes:x-anthropic*x-claude*- auth headers (
authorization,x-api-key,api-key)
It does not remove canonical Anthropic headers like:
anthropic-versionanthropic-beta
Direct repro I verified locally on this head:
- create the shim with
defaultHeaderscontaining:anthropic-version: 2023-06-01anthropic-beta: prompt-caching-2024-07-31
- send a normal 3P OpenAI-compatible request
- the outbound request still contains both headers unchanged
This matters in the real client path because
ANTHROPIC_CUSTOM_HEADERSis parsed intodefaultHeadersbefore routing (client.ts:110-122). So the branch still does not fully satisfy its own goal of preventing Anthropic-specific fields from leaking into third-party requests.
Non-blocking note:
src/utils/conversationRecovery.tschanges the 3P resume preprocessing order, but there is still no targeted regression test for the thinking/tool_result pairing case described in the new comment.
What I verified on this head:
bun test src/services/api/openaiShim.test.ts src/utils/context.test.ts-> 40 passbun run test:provider-> 141 passbun run build-> successbun run smoke-> success- direct runtime repro that providerOverride strips
x-anthropic-*headers and suppressescache_control - direct runtime repro that
anthropic-versionandanthropic-betastill reach the third-party request
I would not merge this until canonical Anthropic headers are scrubbed too, not just the x-anthropic* variants.
The remaining blocker from PR Gitlawb#268 was that canonical Anthropic headers such as `anthropic-version` and `anthropic-beta` could still ride through supported 3P paths even after the earlier x-anthropic/x-claude scrubber work. This tightens header filtering inside the shim itself so direct defaultHeaders, env-driven client setup, providerOverride routing, and per-request header injection all share the same scrubber. Constraint: Preserve non-Anthropic custom headers and provider auth while stripping only Anthropic/OpenClaude-internal headers from 3P requests Rejected: Rely on client.ts filtering alone | direct shim construction and per-request headers would still leave gaps Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep header scrubbing centralized in the shim so new call paths do not reopen 3P leakage bugs Tested: bun test src/services/api/openaiShim.test.ts src/services/api/client.test.ts src/utils/context.test.ts Tested: bun run test:provider Tested: bun run build && node dist/cli.mjs --version Not-tested: bun run typecheck (repository baseline currently fails in many unrelated files)
The remaining blocker from PR Gitlawb#268 was that canonical Anthropic headers such as `anthropic-version` and `anthropic-beta` could still ride through supported 3P paths even after the earlier x-anthropic/x-claude scrubber work. This tightens header filtering inside the shim itself so direct defaultHeaders, env-driven client setup, providerOverride routing, and per-request header injection all share the same scrubber. Constraint: Preserve non-Anthropic custom headers and provider auth while stripping only Anthropic/OpenClaude-internal headers from 3P requests Rejected: Rely on client.ts filtering alone | direct shim construction and per-request headers would still leave gaps Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep header scrubbing centralized in the shim so new call paths do not reopen 3P leakage bugs Tested: bun test src/services/api/openaiShim.test.ts src/services/api/client.test.ts src/utils/context.test.ts Tested: bun run test:provider Tested: bun run build && node dist/cli.mjs --version Not-tested: bun run typecheck (repository baseline currently fails in many unrelated files)
Filter anthropic-version and anthropic-beta alongside the existing Anthropic header scrubber so third-party providers never receive canonical Anthropic headers through shim defaults or per-request overrides. Co-Authored-By: Claude Opus 4.6 <noreply@openclaude.dev>
|
Addressed the remaining header-leak path. This follow-up now strips canonical Anthropic headers in addition to the existing
Applied in both places that matter for 3P routing:
I also extended the focused shim test so both headers are asserted to be removed from:
Local validation:
This should close the remaining leakage case called out in review where |
Bring the branch up to date with main so the canonical Anthropic header scrubber can merge cleanly with the latest conversation recovery changes and re-open review on the current head. Co-Authored-By: Claude Opus 4.6 <noreply@openclaude.dev>
|
Rebased/updated the branch with the latest The canonical header scrubber follow-up is still included on top of current
Current branch head also keeps the earlier providerOverride/cache-control coverage. The focused shim test still covers the canonical-header case that prompted the latest review round. If there are no new issues on the updated head, this should be ready for a fresh re-review. |
* Stop canonical Anthropic headers from leaking into 3P shim requests The remaining blocker from PR #268 was that canonical Anthropic headers such as `anthropic-version` and `anthropic-beta` could still ride through supported 3P paths even after the earlier x-anthropic/x-claude scrubber work. This tightens header filtering inside the shim itself so direct defaultHeaders, env-driven client setup, providerOverride routing, and per-request header injection all share the same scrubber. Constraint: Preserve non-Anthropic custom headers and provider auth while stripping only Anthropic/OpenClaude-internal headers from 3P requests Rejected: Rely on client.ts filtering alone | direct shim construction and per-request headers would still leave gaps Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep header scrubbing centralized in the shim so new call paths do not reopen 3P leakage bugs Tested: bun test src/services/api/openaiShim.test.ts src/services/api/client.test.ts src/utils/context.test.ts Tested: bun run test:provider Tested: bun run build && node dist/cli.mjs --version Not-tested: bun run typecheck (repository baseline currently fails in many unrelated files) * Keep OpenAI client tests from restoring undefined env as strings The new header-leak regression tests in client.test.ts restored environment variables via direct assignment, which can leave literal "undefined" strings in process.env when the original value was unset. This switches the teardown over to the same restore helper pattern already used in openaiShim.test.ts. Constraint: Keep the fix limited to test hygiene without altering runtime behavior Rejected: Restore only the two env vars Copilot called out | using one helper for all test env restores is simpler and less error-prone Confidence: high Scope-risk: narrow Reversibility: clean Directive: Use restore helpers for env teardown in tests so unset values stay deleted instead of becoming the string "undefined" Tested: bun test src/services/api/client.test.ts src/services/api/openaiShim.test.ts src/utils/context.test.ts Not-tested: Full provider suite (unchanged runtime path) * Prevent GitHub Codex requests from forwarding unsanitized Anthropic headers A base-sync with upstream exposed a separate GitHub+Codex transport branch that still merged per-request headers raw before adding Copilot headers. This keeps the filter aligned across Codex-family paths and adds explicit regression tests for GitHub Codex routing, including providerOverride. Constraint: Must not push or modify GitHub state while validating the reviewer concern Rejected: Leave the GitHub Codex path unchanged | runtime repro showed anthropic-* headers still leaked after the upstream sync Confidence: high Scope-risk: narrow Directive: Keep header scrubbing consistent across every Codex-family transport branch when provider routing changes Tested: bun test src/services/api/openaiShim.test.ts Tested: bun test src/services/api/client.test.ts src/services/api/codexShim.test.ts src/services/api/providerConfig.github.test.ts Tested: bun run build Not-tested: Full repository test suite
…#499) * Stop canonical Anthropic headers from leaking into 3P shim requests The remaining blocker from PR Gitlawb#268 was that canonical Anthropic headers such as `anthropic-version` and `anthropic-beta` could still ride through supported 3P paths even after the earlier x-anthropic/x-claude scrubber work. This tightens header filtering inside the shim itself so direct defaultHeaders, env-driven client setup, providerOverride routing, and per-request header injection all share the same scrubber. Constraint: Preserve non-Anthropic custom headers and provider auth while stripping only Anthropic/OpenClaude-internal headers from 3P requests Rejected: Rely on client.ts filtering alone | direct shim construction and per-request headers would still leave gaps Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep header scrubbing centralized in the shim so new call paths do not reopen 3P leakage bugs Tested: bun test src/services/api/openaiShim.test.ts src/services/api/client.test.ts src/utils/context.test.ts Tested: bun run test:provider Tested: bun run build && node dist/cli.mjs --version Not-tested: bun run typecheck (repository baseline currently fails in many unrelated files) * Keep OpenAI client tests from restoring undefined env as strings The new header-leak regression tests in client.test.ts restored environment variables via direct assignment, which can leave literal "undefined" strings in process.env when the original value was unset. This switches the teardown over to the same restore helper pattern already used in openaiShim.test.ts. Constraint: Keep the fix limited to test hygiene without altering runtime behavior Rejected: Restore only the two env vars Copilot called out | using one helper for all test env restores is simpler and less error-prone Confidence: high Scope-risk: narrow Reversibility: clean Directive: Use restore helpers for env teardown in tests so unset values stay deleted instead of becoming the string "undefined" Tested: bun test src/services/api/client.test.ts src/services/api/openaiShim.test.ts src/utils/context.test.ts Not-tested: Full provider suite (unchanged runtime path) * Prevent GitHub Codex requests from forwarding unsanitized Anthropic headers A base-sync with upstream exposed a separate GitHub+Codex transport branch that still merged per-request headers raw before adding Copilot headers. This keeps the filter aligned across Codex-family paths and adds explicit regression tests for GitHub Codex routing, including providerOverride. Constraint: Must not push or modify GitHub state while validating the reviewer concern Rejected: Leave the GitHub Codex path unchanged | runtime repro showed anthropic-* headers still leaked after the upstream sync Confidence: high Scope-risk: narrow Directive: Keep header scrubbing consistent across every Codex-family transport branch when provider routing changes Tested: bun test src/services/api/openaiShim.test.ts Tested: bun test src/services/api/client.test.ts src/services/api/codexShim.test.ts src/services/api/providerConfig.github.test.ts Tested: bun run build Not-tested: Full repository test suite
- Strip store field from request body for local providers (Ollama, vLLM) that reject unknown JSON fields with 400 errors - Add Gemini 3.x model context windows and output token limits (gemini-3-flash-preview, gemini-3.1-pro-preview, google/ OpenRouter variants) - Preserve reasoning_content on assistant tool-call message replays for providers that require it (Kimi k2.5, DeepSeek reasoner) - Use conservative max_output_tokens fallback (4096/16384) for unknown 3P models to prevent vLLM/Ollama 400 errors from exceeding max_model_len Consolidates fixes from: #258, #268, #237, #643, #666, #677 Co-authored-by: auriti <auriti@users.noreply.github.com> Co-authored-by: Gustavo-Falci <Gustavo-Falci@users.noreply.github.com> Co-authored-by: lttlin <lttlin@users.noreply.github.com> Co-authored-by: Durannd <Durannd@users.noreply.github.com>
Summary
Fixes 4 bugs where Anthropic-internal data was unconditionally included in requests to third-party OpenAI-compatible providers. All findings verified against the current codebase.
Root Causes
claude.ts:333getPromptCachingEnabled()returnstruefor all providers — no provider checkclient.ts:163defaultHeaders(session IDs,x-anthropic-*, potentially Anthropic API key) passed directly to shimopenaiShim.ts:186textfields —imageblocks produce empty stringsconversationRecovery.ts:186-196stripThinkingBlocksruns afterfilterUnresolvedToolUses— removing a thinking-only message orphans its pairedtool_resultChanges
src/services/api/claude.tsgetPromptCachingEnabled()returnsfalseforopenai/gemini/github/codexproviderssrc/services/api/client.tsx-anthropic-*,x-claude-*,authorization,api-keyheaders before passing to shimsrc/services/api/openaiShim.tsimageblocks in tool result content — convert to[Image](url)placeholders (aligned withcodexShim.ts)src/utils/conversationRecovery.tsstripThinkingBlocksbeforefilterUnresolvedToolUsesfor 3P providerssrc/services/api/openaiShim.test.tsIssues addressed
Test plan
bun test src/services/api/openaiShim.test.ts— 5/5 pass (includes new image regression test)bun test src/utils/context.test.ts— 6/6 passcache_controlin 3P request payloadx-anthropic-*headers sent to 3P endpoint[Image](url)in conversation