fix: scrub canonical Anthropic headers from 3P shim requests (#499)
* 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
This commit is contained in:
@@ -80,6 +80,32 @@ function isGithubModelsMode(): boolean {
|
||||
return isEnvTruthy(process.env.CLAUDE_CODE_USE_GITHUB)
|
||||
}
|
||||
|
||||
function filterAnthropicHeaders(
|
||||
headers: Record<string, string> | undefined,
|
||||
): Record<string, string> {
|
||||
if (!headers) return {}
|
||||
|
||||
const filtered: Record<string, string> = {}
|
||||
for (const [key, value] of Object.entries(headers)) {
|
||||
const lower = key.toLowerCase()
|
||||
if (
|
||||
lower.startsWith('x-anthropic') ||
|
||||
lower.startsWith('anthropic-') ||
|
||||
lower.startsWith('x-claude') ||
|
||||
lower === 'x-app' ||
|
||||
lower === 'x-client-app' ||
|
||||
lower === 'authorization' ||
|
||||
lower === 'x-api-key' ||
|
||||
lower === 'api-key'
|
||||
) {
|
||||
continue
|
||||
}
|
||||
filtered[key] = value
|
||||
}
|
||||
|
||||
return filtered
|
||||
}
|
||||
|
||||
function hasGeminiApiHost(baseUrl: string | undefined): boolean {
|
||||
if (!baseUrl) return false
|
||||
|
||||
@@ -989,7 +1015,7 @@ class OpenAIShimMessages {
|
||||
private providerOverride?: { model: string; baseURL: string; apiKey: string }
|
||||
|
||||
constructor(defaultHeaders: Record<string, string>, reasoningEffort?: 'low' | 'medium' | 'high' | 'xhigh', providerOverride?: { model: string; baseURL: string; apiKey: string }) {
|
||||
this.defaultHeaders = defaultHeaders
|
||||
this.defaultHeaders = filterAnthropicHeaders(defaultHeaders)
|
||||
this.reasoningEffort = reasoningEffort
|
||||
this.providerOverride = providerOverride
|
||||
}
|
||||
@@ -1099,7 +1125,7 @@ class OpenAIShimMessages {
|
||||
params,
|
||||
defaultHeaders: {
|
||||
...this.defaultHeaders,
|
||||
...(options?.headers ?? {}),
|
||||
...filterAnthropicHeaders(options?.headers),
|
||||
...COPILOT_HEADERS,
|
||||
},
|
||||
signal: options?.signal,
|
||||
@@ -1131,7 +1157,7 @@ class OpenAIShimMessages {
|
||||
params,
|
||||
defaultHeaders: {
|
||||
...this.defaultHeaders,
|
||||
...(options?.headers ?? {}),
|
||||
...filterAnthropicHeaders(options?.headers),
|
||||
},
|
||||
signal: options?.signal,
|
||||
})
|
||||
@@ -1223,7 +1249,7 @@ class OpenAIShimMessages {
|
||||
const headers: Record<string, string> = {
|
||||
'Content-Type': 'application/json',
|
||||
...this.defaultHeaders,
|
||||
...(options?.headers ?? {}),
|
||||
...filterAnthropicHeaders(options?.headers),
|
||||
}
|
||||
|
||||
const isGemini = isGeminiMode()
|
||||
|
||||
Reference in New Issue
Block a user