fix(security): random per-process self-loop admission bearer (#13679 PR C) - #13813
Merged
diegosouzapw merged 2 commits intoSep 16, 2026
Merged
Conversation
resolveSelfLoopBearer() in chatAdmissionIdentity.ts fell back to the checked-in literal "sk_omniroute" whenever OMNIROUTE_API_KEY/ROUTER_API_KEY were unset. Anyone reading the source knew this shared secret and could send x-omniroute-admission-bypass: internal + Authorization: Bearer sk_omniroute to skip the heavyweight admission/queueing lease. Traced every caller of isInternalAdmissionBypass(): it only gates the per-connection admission lease in admitChatRequest(), not authentication/authorization, so the real-world blast radius was narrow (capacity-reservation bypass, still subject to the hard byte-size cap) but the predictable literal was still bad practice. Fix: generate a random 32-byte secret at first use, memoized in memory for the process lifetime, used only as the last-resort fallback. Both the in-process caller (audioBridgeHelpers/visionBridgeHelpers) and the verifier (isInternalAdmissionBypass) call the same resolveSelfLoopBearer(), so they still agree on the value within one process. Regression test: tests/unit/chat-admission-selfloop-random-bearer-13679.test.ts Aligned tests/unit/chat-body-admission.test.ts's existing "resolveSelfLoopBearer falls back to..." assertion to the new contract (it previously asserted the old literal). Refs #13679 (PR C of 6 — self-loop bearer default; the umbrella covers 15 other independent findings shipped as separate PRs).
diegosouzapw
force-pushed
the
fix/13679c-self-loop-bearer-random
branch
from
September 15, 2026 22:56
4bf43d9 to
becbc04
Compare
This was referenced Sep 16, 2026
QuangBlue
pushed a commit
to QuangBlue/OmniRoute
that referenced
this pull request
Sep 24, 2026
…nder REQUIRE_API_KEY The vision and audio bridges call OmniRoute's own /v1 routes with resolveSelfLoopBearer(): the env key when one is set, otherwise a random per-process secret (diegosouzapw#13813). API-key validation accepted only env keys and DB keys, so on a REQUIRE_API_KEY=true instance without OMNIROUTE_API_KEY every self-loop failed with `401 AUTH_002 Invalid API key`. The vision bridge then replaced every image with the "(unavailable — no vision-capable provider connected)" stub even though a vision model was connected, and audio transcription through the bridge failed the same way. - The generated secret lives on globalThis (Symbol.for key). The proxy (Next middleware bundle) and the route handlers load separate copies of chatAdmissionIdentity, so a module-level value differed between the side that sends it and the side that validates it. - validateApiKey accepts that secret (constant-time compare, before any hashing or cache lookup) and never creates it. getApiKeyMetadata gives it the env-key record under id "self-loop", limited to the chat and audio endpoint categories, with the scope list ["internal:self-loop"] (an empty list lets MCP fall back to client-supplied scopes) and no manage scope. - The describe call sends the self-loop bearer and the admission-bypass header only to OmniRoute's own listener (loopback host on one of its ports), not to another localhost server set as VISION_BRIDGE_BASE_URL. Any other endpoint gets only the provider key; before, a direct call without one created an OmniRoute "CLI Auto-Key" DB key and sent it as the bearer (to api.openai.com by default), and the self-loop path created such a key once per process and then overwrote it. resolveSelfLoopApiKey and its test are removed; the new tests cover self-loop authentication under REQUIRE_API_KEY.
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…uzapw#13679) (diegosouzapw#13813) Merged in the 2026-09-16 sweep of the maintainer's own open PRs, at the owner's explicit instruction. No push was made to the PR branch: the merge took the head as the owning session left it (verified OPEN, non-draft and MERGEABLE against the release tip immediately before merging).
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.
Refs #13679
Not covered here (the other 15 findings from the #13679 umbrella, shipped as separate PRs):
Root cause (short)
resolveSelfLoopBearer()insrc/shared/middleware/chatAdmissionIdentity.tsfell back to thechecked-in literal
"sk_omniroute"wheneverOMNIROUTE_API_KEY/ROUTER_API_KEYwere unset.Anyone reading the source knew this shared secret and could send
x-omniroute-admission-bypass: internal+Authorization: Bearer sk_omnirouteto skip theheavyweight admission/queueing lease.
Traced every caller of
isInternalAdmissionBypass(): it is consumed only byadmitChatRequest()inchatBodyAdmission.ts, and gates skipping the per-connectionadmission/queueing lease — not authentication/authorization. A forged header pair only skips
capacity reservation (still subject to the hard byte-size cap); it does not bypass API-key/auth
checks elsewhere in the pipeline. So the real-world blast radius is narrow, but the predictable
literal was still bad practice worth fixing (lowest priority of the 6 PRs per the owner's
ordering).
Fix
Generate a random 32-byte secret (
crypto.randomBytes(32).toString("hex")) at first use,memoized in memory for the process lifetime, used only as the last-resort fallback when neither
env var is set. Both the in-process caller (
audioBridgeHelpers.ts/visionBridgeHelpers.ts)and the verifier (
isInternalAdmissionBypass) call the sameresolveSelfLoopBearer(), so theystill agree on the value within one process — no cross-process coordination is needed since this
bearer only gates a same-process self-loop call.
Regression test
tests/unit/chat-admission-selfloop-random-bearer-13679.test.tsRED (on unfixed code):
GREEN (after fix):
Gates run
npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>→ clean, no warningsnpm run typecheck:core→ exit 0node scripts/check/check-file-size.mjs→ 1 pre-existing violation onopen-sse/utils/stream.ts(base drift, file not touched by this PR — confirmed onorigin/release/v3.8.51tip, 3114 lines vs frozen baseline 3098)node scripts/check/check-complexity.mjs→ OK (2824 violations vs baseline 3218)node scripts/check/check-cognitive-complexity.mjs→ OK (1276 violations vs baseline 1437)node scripts/check/check-test-discovery.mjs→ OK (new test file discovered)DATA_DIR=$(mktemp -d) node --import tsx/esm --test --test-force-exit tests/unit/chat-admission-selfloop-random-bearer-13679.test.ts→ 4/4 passDATA_DIR=$(mktemp -d) node --import tsx/esm --test --test-force-exit tests/unit/chat-body-admission.test.ts→ 37/37 passDATA_DIR=$(mktemp -d) node --import tsx/esm --test --test-force-exit tests/unit/timing-safe-compare.test.ts→ 6/6 pass (reads this file's source for pattern checks)Existing tests aligned
tests/unit/chat-body-admission.test.tshad a test assertingresolveSelfLoopBearer()equalsthe literal
"sk_omniroute"when no env key is set — that encoded the old (insecure) contract.Updated it to assert the fallback is NOT the literal and is memoized per-process, matching the
new corrected behavior. No assertion was weakened or removed; the other
sk_omniroute-literaltests (rejecting the sentinel once an env key is configured) were untouched since they test a
different, still-valid scenario.