diff --git a/changelog.d/fixes/13679-selfloop-bearer-random.md b/changelog.d/fixes/13679-selfloop-bearer-random.md new file mode 100644 index 0000000000..3b024838fd --- /dev/null +++ b/changelog.d/fixes/13679-selfloop-bearer-random.md @@ -0,0 +1 @@ +- **fix(security):** the internal self-loop admission-bypass bearer is now a random per-process secret instead of the checked-in literal `"sk_omniroute"` when no `OMNIROUTE_API_KEY`/`ROUTER_API_KEY` is configured (#13679) diff --git a/src/shared/middleware/chatAdmissionIdentity.ts b/src/shared/middleware/chatAdmissionIdentity.ts index f786eaebd4..8b6986f076 100644 --- a/src/shared/middleware/chatAdmissionIdentity.ts +++ b/src/shared/middleware/chatAdmissionIdentity.ts @@ -1,8 +1,7 @@ -import { createHmac } from "crypto"; +import { createHmac, randomBytes } from "crypto"; import { timingSafeCompare } from "@/shared/utils/timingSafeCompare"; const ADMISSION_BYPASS_VALUE = "internal"; -const SELF_LOOP_KEY = "sk_omniroute"; const FINGERPRINT_KEY = "omniroute-admission-fingerprint-v1"; export const ADMISSION_BYPASS_HEADER = "x-omniroute-admission-bypass"; @@ -19,9 +18,27 @@ export function resolveSessionId(request: Request): string { return xGoogApiKey ? fingerprint(xGoogApiKey) : "anonymous"; } +// Lazily generated, held in memory only for the lifetime of this process — never +// persisted, never logged. Used ONLY as the last-resort self-loop bearer when the +// operator hasn't set OMNIROUTE_API_KEY/ROUTER_API_KEY (#13679: the previous fallback +// was the checked-in literal "sk_omniroute", a predictable shared secret anyone reading +// the source could forge). Both the in-process caller (audioBridgeHelpers / +// visionBridgeHelpers) and the verifier (isInternalAdmissionBypass) call this same +// function, so they always agree on the value within one process. +let generatedSelfLoopSecret: string | null = null; + +function getGeneratedSelfLoopSecret(): string { + if (!generatedSelfLoopSecret) { + generatedSelfLoopSecret = randomBytes(32).toString("hex"); + } + return generatedSelfLoopSecret; +} + export function resolveSelfLoopBearer(): string { return ( - process.env.OMNIROUTE_API_KEY?.trim() || process.env.ROUTER_API_KEY?.trim() || SELF_LOOP_KEY + process.env.OMNIROUTE_API_KEY?.trim() || + process.env.ROUTER_API_KEY?.trim() || + getGeneratedSelfLoopSecret() ); } diff --git a/tests/unit/chat-admission-selfloop-random-bearer-13679.test.ts b/tests/unit/chat-admission-selfloop-random-bearer-13679.test.ts new file mode 100644 index 0000000000..1a457b8729 --- /dev/null +++ b/tests/unit/chat-admission-selfloop-random-bearer-13679.test.ts @@ -0,0 +1,85 @@ +// #13679 (PR C): the self-loop admission bypass bearer must never fall back to the +// predictable literal "sk_omniroute" when OMNIROUTE_API_KEY/ROUTER_API_KEY are unset. +// +// Root cause: `resolveSelfLoopBearer()` in chatAdmissionIdentity.ts returned the checked-in +// literal `"sk_omniroute"` as its final fallback. Anyone who read the source (or the public +// repo) knew this value and could send `x-omniroute-admission-bypass: internal` + +// `Authorization: Bearer sk_omniroute` to skip the heavyweight admission/queueing lease — +// not an auth bypass (see chatBodyAdmission.ts::admitChatRequest), but still a predictable +// shared secret that should not be a hardcoded literal. +import test from "node:test"; +import assert from "node:assert/strict"; + +const { resolveSelfLoopBearer } = + await import("../../src/shared/middleware/chatAdmissionIdentity.ts"); + +const SELF_LOOP_ENV_KEYS = ["OMNIROUTE_API_KEY", "ROUTER_API_KEY"] as const; +function withSelfLoopEnv(env: Partial>) { + const saved = new Map(); + for (const key of SELF_LOOP_ENV_KEYS) { + saved.set(key, process.env[key]); + if (env[key] === undefined) delete process.env[key]; + else process.env[key] = env[key]; + } + return () => { + for (const key of SELF_LOOP_ENV_KEYS) { + const value = saved.get(key); + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + }; +} + +test("resolveSelfLoopBearer never falls back to the predictable sk_omniroute literal", () => { + const restore = withSelfLoopEnv({}); + try { + const bearer = resolveSelfLoopBearer(); + assert.notEqual( + bearer, + "sk_omniroute", + "resolveSelfLoopBearer() fell back to the checked-in literal — a shared secret anyone " + + "reading the source knows, instead of a per-process random value" + ); + } finally { + restore(); + } +}); + +test("resolveSelfLoopBearer's generated fallback is stable within the same process", () => { + const restore = withSelfLoopEnv({}); + try { + const first = resolveSelfLoopBearer(); + const second = resolveSelfLoopBearer(); + assert.equal( + first, + second, + "the generated self-loop bearer must be memoized for the process lifetime — the same " + + "in-process caller (audioBridgeHelpers/visionBridgeHelpers) and verifier " + + "(isInternalAdmissionBypass) must agree on the value" + ); + } finally { + restore(); + } +}); + +test("resolveSelfLoopBearer's generated fallback has enough entropy to resist guessing", () => { + const restore = withSelfLoopEnv({}); + try { + const bearer = resolveSelfLoopBearer(); + assert.ok( + bearer.length >= 32, + `generated self-loop bearer is too short to be a random secret: "${bearer}" (${bearer.length} chars)` + ); + } finally { + restore(); + } +}); + +test("resolveSelfLoopBearer still prefers OMNIROUTE_API_KEY over the generated fallback", () => { + const restore = withSelfLoopEnv({ OMNIROUTE_API_KEY: "omni-key" }); + try { + assert.equal(resolveSelfLoopBearer(), "omni-key"); + } finally { + restore(); + } +}); diff --git a/tests/unit/chat-body-admission.test.ts b/tests/unit/chat-body-admission.test.ts index b7867bc069..e291d37d30 100644 --- a/tests/unit/chat-body-admission.test.ts +++ b/tests/unit/chat-body-admission.test.ts @@ -792,10 +792,15 @@ test("external clients cannot use the bypass header without a trusted self-loop // ── self-loop bearer resolution (env-key aware, #1350) ───────────────── -test("resolveSelfLoopBearer falls back to sk_omniroute when no env key is set", () => { +test("resolveSelfLoopBearer falls back to a random per-process secret when no env key is set (#13679)", () => { const restore = withSelfLoopEnv({}); try { - assert.equal(resolveSelfLoopBearer(), "sk_omniroute"); + // #13679 PR C: the fallback must NOT be the predictable checked-in literal + // "sk_omniroute" — it is a per-process random value (dedicated regression test: + // tests/unit/chat-admission-selfloop-random-bearer-13679.test.ts). + const bearer = resolveSelfLoopBearer(); + assert.notEqual(bearer, "sk_omniroute"); + assert.equal(bearer, resolveSelfLoopBearer(), "must be memoized for the process lifetime"); } finally { restore(); }