Repository navigation
fix(auth): accept a valid x-api-key even when an unrelated Authorization bearer is present - #16483
Merged
diegosouzapw merged 3 commits intoOct 15, 2026
Conversation
…ion bearer is present Clients such as Claude Code with an active claude.ai session or ANTHROPIC_AUTH_TOKEN send their own `Authorization: Bearer` next to a valid OmniRoute `x-api-key`. The CLIENT_API gate validated only the Bearer, so the request got a 401 with REQUIRE_API_KEY on, or was downgraded to anonymous (losing the key's policy and attribution) with it off. - add clientApiKeyResolver: extractApiKeyCandidates() (Bearer, x-api-key, x-goog-api-key, /vscode path token; deduped, precedence order) and resolveClientApiKey() returning the first candidate that validates, or the legacy extractApiKey() result when none does - clientApiPolicy accepts the request when any presented credential validates and masks THAT credential as the subject - route-level guard (enforceClientApiRouteAuth), embeddings, web fetch/map and responses input_tokens re-checks use the same resolver so they are never stricter than the gate - enforceApiKeyPolicy and the chat handler bind policy/model gates to the validated credential, so a stale Bearer cannot hide the real key's restrictions - extractApiKey() is unchanged (management auth keeps allowUrl:false) Co-authored-by: omar-nahhas <omar@innovativerealities.com> Inspired-by: decolua/9router#2926
Contributor
CI Coverage Report
Coverage artifact was not available for this run. |
# Conflicts: # src/shared/utils/apiKeyPolicy.ts
6 tasks done
diegosouzapw
added a commit
that referenced
this pull request
Oct 17, 2026
…dding routes (#16499) GHSA-3w2r: apply the API-key connection scope (allowedConnections + quota pools, combo per-target model gate, keyless-provider #9057 rule) to the direct /v1 media, embedding, retrieval and voice-proxy routes. Reconciled with #16492 (already on release/v3.8.52): the audio/video bridge parts were identical and merged clean; transcriptions/route.ts keeps #16492's scoped credential lookups but uses the shared keyConnectionScope helpers (the route-local isKeyScopeBlocked/keyScopeBlockedResponse duplicates are dropped) plus this PR's additions (quota-scope rejection, combo target key gate, keyless-provider gate). imageCombo keeps the release's runImageRequestWithAccounting dispatch (#14619) with this PR's scoped credential lookup. web/fetch keeps resolveClientApiKey (#16483) with this PR's scoped policy. Inventory test keeps the release's adobeFireflyEdit entry plus this PR's counts. Boundary: the 3 new open-sse to src edges (image/speech/video combo importing @/sse/services/keyConnectionScope) are gone. The pure helpers moved to open-sse/services/comboKeyScope.ts. src/sse/services/keyConnectionScope.ts re-exports them and keeps the DB-dependent parts (resolveKeyConnectionScope, createComboTargetKeyGate, narrowConnectionScope); the route still injects the model gate as ComboKeyScope.isTargetAllowed. allowlistPermitsSyntheticNoAuth moved next to SYNTHETIC_NOAUTH_CONNECTION_ID (noAuthAllowlist.ts re-exports it). measure-boundary --ratchet: 774/774, the same count as the tip. Validation: focused node tests 245/245 (PR tests, #16492 audio-bridge test, inventory, image/speech/video combo, noauth/allowlist, image/transcription/web-fetch route suites, new combo-key-scope-engine-module test) plus 112/112 re-run after the final base merge. Vitest autoCombo 56/56. Static checks: typecheck:core, api-typecheck and open-sse-typecheck OK; complexity, cognitive complexity, file-size and dead-code OK; changelog-integrity OK; skills generator reported Generated: 0; eslint on changed files OK; cycles ratchet 14/14. Remaining CI reds are inherited from the base: #15306.
diegosouzapw
deleted the
fix/port-pr-2926-accept-any-valid-client-credential
branch
October 18, 2026 23:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ANTHROPIC_AUTH_TOKENsends its ownAuthorization: Beareralongside a valid OmniRoutex-api-key. The CLIENT_API gate only validated the Bearer, so these requests got a 401 whenREQUIRE_API_KEYwas on. When it was off, they were silently downgraded to anonymous and lost the key's policy and attribution.extractApiKey()contract is unchanged, including management auth withallowUrl: false.Attribution
Thanks to @omar-nahhas for the original implementation.
Changes
src/sse/services/clientApiKeyResolver.ts(new):extractApiKeyCandidates()collects Bearer,x-api-key,x-goog-api-keyand the/vscode/<key>path token, deduped and in precedence order, at most 4.resolveClientApiKey()returns the first candidate that passesisValidApiKey(), or the legacyextractApiKey()result when none does. It lives in its own module becauseauth.tsis frozen at its file-size ceiling. The only change inauth.tsis thatreadNonEmptyUrlTokenis now exported.src/server/authz/policies/clientApi.ts: validates each candidate and allows on the first valid one, with that credential masked as the subject. The [BUG] invalid api key in Codex Desktop auto config #2257 behavior is unchanged: withREQUIRE_API_KEY=false, all-invalid credentials still fall back to anonymous.src/shared/utils/clientApiRouteAuth.ts,src/app/api/v1/embeddings,web/fetch,web/map,responses/input_tokens: the route-level re-checks use the same resolver, so they are never stricter than the gate.src/shared/utils/apiKeyPolicy.ts(enforceApiKeyPolicy) andsrc/sse/handlers/chat.ts: the policy and model gates use the validated credential, so a stale Bearer cannot hide the real key's allowedModels or budget. Thechat.tsedit adds no lines; the file is at its frozen size./v1/messages401s (optional in the plan, and it would change a public error shape), plus management auth, A2A, session-leases, video-bridge drilldown, combos/explain and the model catalog, which still useextractApiKey().Test plan
node --import tsx/esm --test tests/unit/authz-client-api-multi-credential.test.ts(new, 16 tests). Before the fix:ℹ tests 16 ℹ pass 11 ℹ fail 5. The 5 behavior tests (stale Bearer + valid x-api-key / x-goog-api-key, REQUIRE off keeps identity, route guard, policy binding) failed. After the fix:ℹ tests 16 ℹ pass 16 ℹ fail 0. Negative cases covered: both invalid → 401 AUTH_002, revoked x-api-key next to a stale Bearer → 401, all-invalid with REQUIRE off → anonymous, and theextractApiKeyBearer-first contract is unchanged.open-sse/mcp-server/__tests__/lazyApiKeyDb.test.ts:ℹ tests 1134 ℹ pass 1118 ℹ fail 12under concurrency. Five of the failing files pass when re-run serially, so those failures came from load. The other 3 files (integration-wiring,chat-correlation-id-exhaustion,embeddings-route-apikeymeta-6929) fail the same way on a cleanorigin/release/v3.8.52checkout, so they are inherited from the base. Vitest:1 passed.npm run typecheck:core→ exit 0npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>→ clean;npx prettier --check→ cleannode scripts/check/check-pr-test-policy.mjs(PR context) → PASS;GITHUB_BASE_REF=release/v3.8.52 npm run check:test-masking→ OK, no weakeningnpm run check:cycles:ratchet→ 14 within ceiling 14;node scripts/check/check-file-size.mjs→ the only violation isopen-sse/services/combo/executeTargetAttempt.ts, which comes from the base and is not touched here