fix(security): random per-process self-loop admission bearer (#13679) (#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 commit is contained in:
Diego Rodrigues de Sa e Souza
2026-09-16 06:09:15 -03:00
committed by GitHub
parent 5574eab259
commit 4de71978b3
4 changed files with 113 additions and 5 deletions

View File

@@ -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)

View File

@@ -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()
);
}

View File

@@ -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<Record<(typeof SELF_LOOP_ENV_KEYS)[number], string>>) {
const saved = new Map<string, string | undefined>();
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();
}
});

View File

@@ -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();
}