fix(security): random per-process self-loop admission bearer (#13679 PR C)

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)

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

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

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