Skip to content

fix: prevent Anthropic-specific fields from leaking into 3P requests - #268

Closed
auriti wants to merge 5 commits into
Twigpine:mainfrom
auriti:fix/anthropic-fields-leak-3p
Closed

auriti wants to merge 5 commits into
Twigpine:mainfrom
auriti:fix/anthropic-fields-leak-3p

Conversation

@auriti

@auriti auriti commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

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

Finding Severity File Root Cause
cache_control leak HIGH claude.ts:333 getPromptCachingEnabled() returns true for all providers — no provider check
Header forwarding HIGH client.ts:163 defaultHeaders (session IDs, x-anthropic-*, potentially Anthropic API key) passed directly to shim
Image data loss HIGH openaiShim.ts:186 Tool result content maps only text fields — image blocks produce empty strings
Resume tool corruption MEDIUM conversationRecovery.ts:186-196 stripThinkingBlocks runs after filterUnresolvedToolUses — removing a thinking-only message orphans its paired tool_result

Changes

File Change
src/services/api/claude.ts getPromptCachingEnabled() returns false for openai/gemini/github/codex providers
src/services/api/client.ts Filter x-anthropic-*, x-claude-*, authorization, api-key headers before passing to shim
src/services/api/openaiShim.ts Handle image blocks in tool result content — convert to [Image](url) placeholders (aligned with codexShim.ts)
src/utils/conversationRecovery.ts Run stripThinkingBlocks before filterUnresolvedToolUses for 3P providers
src/services/api/openaiShim.test.ts Regression test: image content in tool results preserved as placeholder

Issues 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 pass
  • Manual: verify no cache_control in 3P request payload
  • Manual: verify no x-anthropic-* headers sent to 3P endpoint
  • Manual: tool result with image content shows [Image](url) in conversation
  • Manual: resume a 1P session against a 3P provider with thinking blocks

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 Twigpine#267.

@gnanam1990 gnanam1990 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Twigpine#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
@auriti

auriti commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

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:

  • `filterAnthropicHeaders()` is now applied at `OpenAIShimMessages` construction time (covers any `defaultHeaders` passed directly to the shim)
  • Same filter applied to per-request `options.headers` in both `_doOpenAIRequest` (OpenAI/Gemini/GitHub path) and the Codex `_doRequest` path
  • Two new focused tests added:
    • Header stripping via direct shim creation — verifies `x-anthropic-`, `x-claude-`, etc. never reach the fetch call even when bypassing `client.ts`
    • Cache-control suppression — verifies `cache_control` blocks are not forwarded in the request body to OpenAI-compatible endpoints

All 7 tests pass.

@Vasanthdev2004

Copy link
Copy Markdown
Collaborator

fix conflicts @auriti

@auriti

auriti commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

Conflicts are now resolved by merging the latest main into this branch.

I kept the leakage fix aligned with the current providerOverride flow in openaiShim.ts, and re-ran the focused src/services/api/openaiShim.test.ts suite plus bun run smoke successfully.

@Vasanthdev2004 Vasanthdev2004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

  1. src/services/api/client.ts:110-122, 389-412 plus src/services/api/openaiShim.ts:95-110
    ANTHROPIC_CUSTOM_HEADERS is merged directly into defaultHeaders in client.ts. The new scrubber in openaiShim.ts only removes:

    • x-anthropic*
    • x-claude*
    • auth headers (authorization, x-api-key, api-key)

    It does not remove canonical Anthropic headers like:

    • anthropic-version
    • anthropic-beta

    Direct repro I verified locally on this head:

    • create the shim with defaultHeaders containing:
      • anthropic-version: 2023-06-01
      • anthropic-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_HEADERS is parsed into defaultHeaders before 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.ts changes 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 pass
  • bun run test:provider -> 141 pass
  • bun run build -> success
  • bun run smoke -> success
  • direct runtime repro that providerOverride strips x-anthropic-* headers and suppresses cache_control
  • direct runtime repro that anthropic-version and anthropic-beta still reach the third-party request

I would not merge this until canonical Anthropic headers are scrubbed too, not just the x-anthropic* variants.

ibaaaaal added a commit to ibaaaaal/openclaude that referenced this pull request Apr 7, 2026
The remaining blocker from PR Twigpine#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)
ibaaaaal added a commit to ibaaaaal/openclaude that referenced this pull request Apr 7, 2026
The remaining blocker from PR Twigpine#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>
@auriti

auriti commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining header-leak path.

This follow-up now strips canonical Anthropic headers in addition to the existing x-anthropic-* / x-claude-* scrubber:

  • anthropic-version
  • anthropic-beta

Applied in both places that matter for 3P routing:

  • src/services/api/client.ts
  • src/services/api/openaiShim.ts

I also extended the focused shim test so both headers are asserted to be removed from:

  • shim defaultHeaders
  • per-request options.headers

Local validation:

  • bun test src/services/api/openaiShim.test.ts -> 34 pass, 0 fail

This should close the remaining leakage case called out in review where ANTHROPIC_CUSTOM_HEADERS could still forward canonical Anthropic headers to third-party OpenAI-compatible endpoints.

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

auriti commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased/updated the branch with the latest main and resolved the merge conflict around conversationRecovery.ts.

The canonical header scrubber follow-up is still included on top of current main:

  • anthropic-version
  • anthropic-beta

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.

kevincodex1 pushed a commit that referenced this pull request Apr 10, 2026
* 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
euxaristia pushed a commit to euxaristia/openclaude that referenced this pull request Apr 13, 2026
…e#499)

* Stop canonical Anthropic headers from leaking into 3P shim requests

The remaining blocker from PR Twigpine#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
auriti added a commit that referenced this pull request Apr 20, 2026
- 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>
C1ph3r404 pushed a commit to C1ph3r404/openclaude that referenced this pull request Apr 29, 2026
…e#499)

* Stop canonical Anthropic headers from leaking into 3P shim requests

The remaining blocker from PR Twigpine#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

@Vasanthdev2004 Vasanthdev2004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Targeted maintainer triage review of the current head ($short).

Verdict: Needs changes

Blocking issue:

  1. GitHub reports this branch as DIRTY / conflicting with main, so it cannot be merged or final-approved as-is. Please rebase or merge latest main, resolve the conflicts, and rerun the relevant checks.

I did not do a full code review because the current branch state is not mergeable. Happy to re-review once the branch is clean.

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for following up on the earlier review. The providerOverride/cache-control coverage looks addressed now, and the canonical Anthropic header scrubber covers anthropic-version / anthropic-beta in the shim tests. I found one remaining blocker below.

Findings

  • [P1] Remove the duplicate provider declarations so the branch builds
    src/utils/conversationRecovery.ts:242
    This block adds const provider and const isThirdPartyProvider in the same deserializeMessagesWithInterruptDetection scope where the existing thinking-strip block already declares both again at lines 263-264. Bun rejects the file with "provider" has already been declared, so bun run build fails and the focused test command cannot complete. Please reuse the existing provider variables, rename/scope the new ones, or consolidate the two third-party-provider checks before merge.

Recommendation: request changes.

The-FOOL-00 pushed a commit to The-FOOL-00/openclaude that referenced this pull request May 24, 2026
…e#499)

* Stop canonical Anthropic headers from leaking into 3P shim requests

The remaining blocker from PR Twigpine#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

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for following up here. I rechecked the current head against current main, and I still cannot approve this as-is.

Findings

  • [P1] Resolve the branch conflicts with current main
    src/services/api/openaiShim.ts
    GitHub currently reports the PR as dirty/not mergeable, and a local merge attempt against current origin/main conflicts in src/services/api/openaiShim.ts and src/services/api/openaiShim.test.ts. The conflicts are around the header-scrubbing code and the adjacent OpenAI shim regression tests, so this branch cannot be merged or safely final-reviewed until it is rebased/merged onto current main and the focused checks are rerun.

  • [P1] Remove the duplicate provider declarations so the branch builds
    src/utils/conversationRecovery.ts:242
    This head still declares const provider and const isThirdPartyProvider at lines 242-243, then declares the same names again in the same deserializeMessagesWithInterruptDetection scope at lines 263-264. Bun rejects the file with "provider" has already been declared, so bun run build fails and the focused test command also errors when it imports this module. Please consolidate/reuse the provider variables, or scope/rename one of the checks, before merge.

discopops pushed a commit to discopops/openclaude that referenced this pull request May 28, 2026
…e#499)

* Stop canonical Anthropic headers from leaking into 3P shim requests

The remaining blocker from PR Twigpine#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
@jatmn

jatmn commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

closing as abandoned

@jatmn jatmn closed this Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants