fix: scrub canonical Anthropic headers from 3P shim requests

[ibaaaaal]

Summary

Follow-up to #268 and the remaining header-leak blocker from #267.

The current scrubber already removes x-anthropic-*, x-claude-*, and auth headers, but canonical Anthropic headers like anthropic-version and anthropic-beta can still reach third-party OpenAI-compatible requests.

This patch closes that gap.

What changed

• extend the shim scrubber to also remove canonical anthropic-* headers
• keep the filtering centralized in openaiShim.ts
• cover the same supported paths called out in review:
  • direct shim defaultHeaders
  • env-driven ANTHROPIC_CUSTOM_HEADERS
  • providerOverride
  • per-request options.headers

Tests

• bun test src/services/api/openaiShim.test.ts src/services/api/client.test.ts src/utils/context.test.ts
• bun run test:provider
• bun run build && node dist/cli.mjs --version

[Copilot]

Pull request overview

This PR closes a remaining header-leak gap in the OpenAI-compatible shim path by ensuring canonical Anthropic headers (e.g. anthropic-version, anthropic-beta) are scrubbed before third-party requests are sent.

Changes:

• Add centralized header filtering in openaiShim.ts to drop canonical anthropic-* and other Anthropic/Claude/auth headers.
• Apply filtering to both shim defaultHeaders and per-request options.headers.
• Add/extend tests covering direct shim usage and getAnthropicClient() routing paths (including ANTHROPIC_CUSTOM_HEADERS and providerOverride).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/services/api/openaiShim.ts	Introduces and applies a centralized header scrubber for OpenAI-compatible requests.
src/services/api/openaiShim.test.ts	Adds regression tests ensuring canonical Anthropic headers are removed from shim headers.
src/services/api/client.test.ts	Adds regression tests ensuring env-driven custom headers are scrubbed on shim/providerOverride requests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In this test teardown, assigning process.env.CLAUDE_CODE_USE_OPENAI = originalEnv... can leave the env var set to the literal string "undefined" when the original value was unset, which can leak state into later tests. Prefer deleting the key when the original value is undefined (e.g., via a small restoreEnv helper like in openaiShim.test.ts).

[Copilot]

Similarly, restoring ANTHROPIC_CUSTOM_HEADERS via direct assignment can set it to the string "undefined" when it was originally unset. Consider deleting the env var when the saved value is undefined to avoid cross-test contamination.

Suggested change

	process.env.ANTHROPIC_CUSTOM_HEADERS = originalEnv.ANTHROPIC_CUSTOM_HEADERS
	if (originalEnv.ANTHROPIC_CUSTOM_HEADERS === undefined) {
	delete process.env.ANTHROPIC_CUSTOM_HEADERS
	} else {
	process.env.ANTHROPIC_CUSTOM_HEADERS = originalEnv.ANTHROPIC_CUSTOM_HEADERS
	}

[gnanam1990]

Thanks for the PR! Code looks good, scope is tight, and the fix addresses the root cause. Verified locally through the focused coverage and CI is green. LGTM.

[Vasanthdev2004]

Rechecked the latest head 5751123.

One inconsistency relative to the PR's own stated goal:

Codex transport path still leaks unfiltered per-request headers (Low)

In _doRequest (openaiShim.ts), the Codex path at line ~1010 passes raw options?.headers without filtering:

\\ s
defaultHeaders: {
...this.defaultHeaders, // filtered at construction time ✓
...(options?.headers ?? {}), // NOT filtered ✗
},
\\

Both the standard OpenAI path and the non-streaming OpenAI path now call filterAnthropicHeaders(options?.headers), but the Codex path was missed. A caller passing anthropic-version or x-anthropic-* headers via options.headers into a Codex request would bypass the scrub.

Fix: change the line to use filterAnthropicHeaders(options?.headers) to match the other paths.

Severity is low since the Codex transport is specialized and callers are internal, but it's an inconsistency the PR's stated goal should cover.

The filterAnthropicHeaders function itself is solid — correct lowercase matching, undefined handling, and comprehensive prefix coverage. Test coverage is thorough across all four entry points (defaultHeaders, ANTHROPIC_CUSTOM_HEADERS, providerOverride, per-request options), with the gap noted above.

[kevincodex1]

please fix conflicts too @ibaaaaal

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/services/api/openaiShim.ts:1067

• _doRequest() treats any codex_responses request in GitHub mode as a GitHub Copilot responses request (isGithubWithCodexTransport), which means Codex-alias models (e.g. codexplan/codexspark) will bypass resolveCodexApiCredentials() and instead require a Copilot token.

To avoid breaking Codex aliases, tighten the GitHub branch condition (e.g. also require the Copilot endpoint type / baseUrl) or explicitly exclude Codex-alias routing when CLAUDE_CODE_USE_GITHUB is set.

    const githubEndpointType = getGithubEndpointType(request.baseUrl)
    const isGithubMode = isGithubModelsMode()
    const isGithubWithCodexTransport = isGithubMode && request.transport === 'codex_responses'
    const isGithubCopilotEndpoint = isGithubMode && githubEndpointType === 'copilot'

    if (isGithubWithCodexTransport) {
      const apiKey = this.providerOverride?.apiKey ?? process.env.OPENAI_API_KEY ?? ''
      if (!apiKey) {
        throw new Error(
          'GitHub Copilot auth is required. Run /onboard-github to sign in.',
        )
      }

      return performCodexRequest({
        request,
        credentials: {
          apiKey,

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Vasanthdev2004]

PR #499 Review: Scrub canonical Anthropic headers from 3P shim requests

Overview

This PR closes a header-leak gap where canonical anthropic-* headers (e.g. anthropic-version, anthropic-beta) could reach third-party OpenAI-compatible API requests. Introduces a centralized filterAnthropicHeaders() function and applies it at header injection points in the shim.

What's good

• Centralized filtering — filterAnthropicHeaders() is clean, well-structured, and handles all the right prefixes (x-anthropic, anthropic-, x-claude, x-app, x-client-app) plus auth headers. Good lowercase normalization.
• Constructor filtering — Filtering defaultHeaders at construction time is the right approach — catches it once rather than at every call site.
• Thorough test coverage — 5 new tests across both test files covering defaultHeaders, per-request options.headers, ANTHROPIC_CUSTOM_HEADERS env, providerOverride, and GitHub Codex transport. Each test verifies specific headers are stripped while non-Anthropic headers survive.
• restoreEnv helper — Fixing the "undefined" string leak in client.test.ts is a good cleanup.

Issues found

1. (Medium) Codex transport path still leaks unfiltered options.headers

At openaiShim.ts:1010, the Codex _doRequest path passes options?.headers raw:

defaultHeaders: {
  ...this.defaultHeaders,           // filtered at construction ✓
  ...(options?.headers ?? {}),      // NOT filtered ✗
},

The other three paths (streaming OpenAI at :1070, non-streaming OpenAI at :1102, and the main request at :1194) all use filterAnthropicHeaders(options?.headers), but this Codex path was missed. This is inconsistent with the PR's stated goal.

Fix: Change line 1010 to:

...filterAnthropicHeaders(options?.headers),

Severity is low-to-medium — the Codex path callers are internal and the defaultHeaders are already scrubbed at construction — but it's an inconsistency that should be closed for completeness.

2. (Low) authorization and x-api-key filtering may be overly broad for per-request headers

Stripping authorization/x-api-key/api-key from options.headers is fine for defaultHeaders at construction (those get replaced by provider-specific auth), but could interfere with callers that intentionally set a per-request auth header. That said, all current call sites set their own authorization header after merging, so this is safe in practice — just worth noting.

3. (Nit) Duplicate test boilerplate

The 5 new tests have significant boilerplate (mock fetch, Response construction, header assertions). A shared helper like createMockFetch() and expectHeadersScrubbed() would reduce ~200 lines of repetition. Not blocking, but a cleanup opportunity.

Verdict

Approve with one requested change — fix the Codex path at line 1010 to use filterAnthropicHeaders(options?.headers). The rest of the PR is solid, well-tested, and directly addresses the stated security goal.

[ibaaaaal]

hey @Vasanthdev2004, just to clarify — 5751123 isn’t the latest head; it’s an earlier test-hygiene commit. the branch was force-pushed afterward, and the current head is 1fb88c8.
In 1fb88c8, the Codex paths in openaiShim.ts already use filterAnthropicHeaders(options?.headers). let me know if you still see any gaps there👍

[gnanam1990]

Thanks for the PR. I re-checked the current head and this now looks good to merge. The scope is tight, the fix addresses a real trust-boundary issue, and the remaining Codex-path inconsistency appears resolved on the latest head.

I verified locally with a real checkout of the PR plus focused coverage and build:

1. bun test src/services/api/openaiShim.test.ts src/services/api/client.test.ts src/utils/context.test.ts src/services/api/codexShim.test.ts src/services/api/providerConfig.github.test.ts ✅
2. bun run build ✅

I’m comfortable with this as a narrow hardening fix for preventing canonical Anthropic headers from leaking into 3P shim requests. LGTM.

[Vasanthdev2004]

Rechecked the current head 1fb88c81f11332c5a58306fc9d217e9646c39207 after the earlier Codex-path concern.

The specific inconsistency I called out on the older head looks resolved now: the Codex-family _doRequest(...) branches are using filterAnthropicHeaders(options?.headers) consistently, including the GitHub Codex transport path.

So at this point the remaining shape looks tight: narrow trust-boundary hardening, focused tests, and green checks. This now looks Approve-ready on the current head.

[Vasanthdev2004]

Rechecked the current head 1fb88c81f11332c5a58306fc9d217e9646c39207.

Scope

This is a targeted re-review of the current head and the previously-raised Codex-path concern, not a full fresh re-review of every historical branch state.

Verdict

Approve-ready

What I rechecked on this head:

• the earlier missed Codex/GitHub Codex per-request header path is now using filterAnthropicHeaders(options?.headers)
• the central scrubber application is consistent across the relevant 3P shim request paths
• scope remains tight to the stated security hardening goal
• checks are green

Given that, I’m comfortable approving the current head.

[auriti]

Rechecked the current head.

The remaining Codex-path inconsistency that Vasanthdev2004 flagged now appears resolved: the Codex/GitHub Codex per-request path is using the same Anthropic-header scrubber as the other shim request paths, and the added regression coverage matches the stated trust-boundary goal.

Scope stays tight and the hardening is pointed at a real 3P leak path. I’m comfortable with this as merge-ready.