fix(sse): crypto-secure RNG for combo/deck load-balancing selection (CodeQL #665)

open-sse/services/combo/shadowRouting.ts

@@ -13,6 +13,7 @@
  * `resolveShadowTargets`, never during module init.
  */
 
+import { secureRandomFloat } from "../../../src/shared/utils/secureRandom";
 import { recordComboShadowRequest } from "../comboMetrics.ts";
 import { isRecord } from "./comboData.ts";
 import { resolveNestedComboTargets } from "./comboStructure.ts";
@@ -50,7 +51,7 @@ export function resolveShadowTargets(
 ): ResolvedComboTarget[] {
   const shadowConfig = normalizeShadowRoutingConfig(config);
   if (!shadowConfig.enabled || shadowConfig.targets.length === 0) return [];
-  if (shadowConfig.sampleRate <= 0 || Math.random() > shadowConfig.sampleRate) return [];
+  if (shadowConfig.sampleRate <= 0 || secureRandomFloat() > shadowConfig.sampleRate) return [];
 
   const shadowCombo: ComboLike = {
     ...combo,

open-sse/services/combo/targetSorters.ts

@@ -8,6 +8,7 @@
  */
 
 import { getCircuitBreaker } from "../../../src/shared/utils/circuitBreaker";
+import { secureRandomFloat, secureRandomInt } from "../../../src/shared/utils/secureRandom";
 import { getComboStepTarget, getComboStepWeight } from "../../../src/lib/combos/steps.ts";
 import { getComboMetrics } from "../comboMetrics.ts";
 import { parseModel } from "../model.ts";
@@ -29,10 +30,10 @@ export function selectWeightedTarget<T extends { weight?: number }>(targets: T[]
 
   const totalWeight = targets.reduce((sum, target) => sum + (target.weight || 0), 0);
   if (totalWeight <= 0) {
-    return targets[Math.floor(Math.random() * targets.length)];
+    return targets[secureRandomInt(targets.length)];
   }
 
-  let random = Math.random() * totalWeight;
+  let random = secureRandomFloat() * totalWeight;
   for (const target of targets) {
     random -= target.weight || 0;
     if (random <= 0) return target;
@@ -159,8 +160,8 @@ function getP2CTargetScore(
 export function orderTargetsByPowerOfTwoChoices(targets: ResolvedComboTarget[], comboName: string) {
   if (targets.length <= 1) return targets;
   const metrics = getComboMetrics(comboName);
-  const firstIndex = Math.floor(Math.random() * targets.length);
-  let secondIndex = Math.floor(Math.random() * (targets.length - 1));
+  const firstIndex = secureRandomInt(targets.length);
+  let secondIndex = secureRandomInt(targets.length - 1);
   if (secondIndex >= firstIndex) secondIndex++;
 
   const first = targets[firstIndex];

src/shared/utils/secureRandom.ts

@@ -0,0 +1,66 @@
+/**
+ * Cryptographically-secure RNG helpers for load-balancing / routing selection.
+ *
+ * OmniRoute's combo target selection (weighted / random / power-of-two-choices), the
+ * credential-deck rotation, and shadow-routing sampling pick among upstream
+ * providers/connections. CodeQL's `js/insecure-randomness` flags `Math.random()` in these
+ * paths as "randomness in a security context" — a false positive (provider load-balancing
+ * is not a secret, token, nonce or session id). Routing these few, non-hot-path selections
+ * through `node:crypto` removes the finding at negligible cost. These are drop-in
+ * replacements with identical ranges and semantics:
+ *
+ *   secureRandomInt(n)   === Math.floor(Math.random() * n)   // integer in [0, n)
+ *   secureRandomFloat()  === Math.random()                   // float   in [0, 1)
+ *
+ * The integer helper uses `crypto.randomInt` (unbiased rejection sampling) rather than
+ * `Math.floor(cryptoFloat() * n)` — dividing/rounding a crypto value introduces modulo bias
+ * (CodeQL `js/biased-cryptographic-random`).
+ */
+import { randomBytes, randomInt } from "node:crypto";
+
+/** Default source: uniform float in [0, 1) from 48 bits of crypto entropy. */
+function cryptoRandomFloat(): number {
+  const buf = randomBytes(6);
+  let value = 0;
+  for (let i = 0; i < buf.length; i++) {
+    value = value * 256 + buf[i];
+  }
+  return value / 2 ** 48;
+}
+
+// Test-only deterministic float source — `null` in production (the crypto sources above are
+// used directly). Tests inject a fixed sequence via _setSecureRandomFloatSource (mirrors the
+// `_resetAllDecks` test-only export). When injected, secureRandomInt derives the index from
+// the same float so the deterministic selection tests keep identical assertions; production
+// never takes that path, so no crypto value is divided/rounded into a biased integer.
+let testFloatSource: (() => number) | null = null;
+
+/** Uniform float in [0, 1) — drop-in for `Math.random()`. */
+export function secureRandomFloat(): number {
+  return testFloatSource ? testFloatSource() : cryptoRandomFloat();
+}
+
+/**
+ * Uniform integer in [0, maxExclusive) — drop-in for `Math.floor(Math.random() * maxExclusive)`.
+ * Returns 0 for any `maxExclusive <= 1` (matching `Math.floor(Math.random() * {0,1})`), so
+ * single-element / empty selections behave exactly as before. Production uses the unbiased
+ * `crypto.randomInt`; only the test path (deterministic injected source) scales a float.
+ */
+export function secureRandomInt(maxExclusive: number): number {
+  if (!Number.isFinite(maxExclusive) || maxExclusive <= 1) return 0;
+  const max = Math.floor(maxExclusive);
+  if (testFloatSource) {
+    // Deterministic test path — clamp defends the probability-0 case of a source returning ~1.
+    return Math.min(max - 1, Math.floor(testFloatSource() * max));
+  }
+  return randomInt(max);
+}
+
+/**
+ * TEST ONLY — replace the underlying RNG with a deterministic source so selection logic can
+ * be asserted; pass `null` to restore the crypto source. Mirrors the `_resetAllDecks`
+ * test-only export convention in shuffleDeck.ts.
+ */
+export function _setSecureRandomFloatSource(source: (() => number) | null): void {
+  testFloatSource = source ?? null;
+}

src/shared/utils/shuffleDeck.ts

@@ -6,6 +6,8 @@
  * race conditions when concurrent requests hit the same deck simultaneously.
  */
 
+import { secureRandomInt } from "./secureRandom";
+
 // ─── Types ──────────────────────────────────────────────────────────────────
 
 interface ShuffleDeck {
@@ -28,7 +30,7 @@ const mutexes = new Map<string, Promise<void>>();
 export function fisherYatesShuffle<T>(arr: readonly T[]): T[] {
   const result = [...arr];
   for (let i = result.length - 1; i > 0; i--) {
-    const j = Math.floor(Math.random() * (i + 1));
+    const j = secureRandomInt(i + 1);
     const tmp = result[i];
     result[i] = result[j];
     result[j] = tmp;
@@ -86,7 +88,7 @@ export async function getNextFromDeck(
     const newOrder = fisherYatesShuffle(itemIds);
 
     if (lastUsedId !== undefined && newOrder[0] === lastUsedId && newOrder.length > 1) {
-      const swapIdx = 1 + Math.floor(Math.random() * (newOrder.length - 1));
+      const swapIdx = 1 + secureRandomInt(newOrder.length - 1);
       const tmp = newOrder[0];
       newOrder[0] = newOrder[swapIdx];
       newOrder[swapIdx] = tmp;
@@ -126,7 +128,7 @@ export function getNextFromDeckSync(namespace: string, itemIds: readonly string[
   const newOrder = fisherYatesShuffle(itemIds);
 
   if (lastUsedId !== undefined && newOrder[0] === lastUsedId && newOrder.length > 1) {
-    const swapIdx = 1 + Math.floor(Math.random() * (newOrder.length - 1));
+    const swapIdx = 1 + secureRandomInt(newOrder.length - 1);
     const tmp = newOrder[0];
     newOrder[0] = newOrder[swapIdx];
     newOrder[swapIdx] = tmp;

tests/unit/combo-routing-engine.test.ts

@@ -33,6 +33,7 @@ const { resetAllCircuitBreakers } = await import("../../src/shared/utils/circuit
 const { acquire: acquireSemaphore, resetAll: resetAllSemaphores } =
   await import("../../open-sse/services/rateLimitSemaphore.ts");
 const { _resetAllDecks } = await import("../../src/shared/utils/shuffleDeck.ts");
+const { _setSecureRandomFloatSource } = await import("../../src/shared/utils/secureRandom.ts");
 
 function createLog() {
   const entries: any[] = [];
@@ -498,10 +499,9 @@ test("handleComboChat priority strategy honors composite tier order before fallb
 });
 
 test("handleComboChat weighted strategy selects by weight and falls back in descending weight order", async () => {
-  const originalRandom = Math.random;
   const calls: any[] = [];
 
-  Math.random = () => 0.95;
+  _setSecureRandomFloatSource(() => 0.95);
 
   try {
     const result = await handleComboChat({
@@ -530,14 +530,13 @@ test("handleComboChat weighted strategy selects by weight and falls back in desc
     assert.equal(result.ok, true);
     assert.deepEqual(calls, ["claude/sonnet", "openai/gpt-4o-mini"]);
   } finally {
-    Math.random = originalRandom;
+    _setSecureRandomFloatSource(null);
   }
 });
 
 test("handleComboChat weighted strategy falls back to uniform random when all weights are zero", async () => {
-  const originalRandom = Math.random;
   const calls: any[] = [];
-  Math.random = () => 0.75;
+  _setSecureRandomFloatSource(() => 0.75);
 
   try {
     const result = await handleComboChat({
@@ -565,16 +564,15 @@ test("handleComboChat weighted strategy falls back to uniform random when all we
     assert.equal(result.ok, true);
     assert.deepEqual(calls, ["model-b"]);
   } finally {
-    Math.random = originalRandom;
+    _setSecureRandomFloatSource(null);
   }
 });
 
 test("handleComboChat random strategy uses shuffled model order", async () => {
-  const originalRandom = Math.random;
   const calls: any[] = [];
   const sequence = [0.99, 0.0];
   let idx = 0;
-  Math.random = () => sequence[idx++] ?? 0;
+  _setSecureRandomFloatSource(() => sequence[idx++] ?? 0);
 
   try {
     await handleComboChat({
@@ -598,7 +596,7 @@ test("handleComboChat random strategy uses shuffled model order", async () => {
     assert.equal(calls.length, 1);
     assert.notEqual(calls[0], "model-a");
   } finally {
-    Math.random = originalRandom;
+    _setSecureRandomFloatSource(null);
   }
 });
 
@@ -627,7 +625,6 @@ test("handleComboChat fill-first explicitly preserves priority order", async ()
 });
 
 test("handleComboChat p2c selects the better of two random choices by metrics", async () => {
-  const originalRandom = Math.random;
   const calls: any[] = [];
   const sequence = [0.0, 0.0];
   let idx = 0;
@@ -642,7 +639,7 @@ test("handleComboChat p2c selects the better of two random choices by metrics",
     latencyMs: 20,
     strategy: "p2c",
   });
-  Math.random = () => sequence[idx++] ?? 0;
+  _setSecureRandomFloatSource(() => sequence[idx++] ?? 0);
 
   try {
     await handleComboChat({
@@ -665,7 +662,7 @@ test("handleComboChat p2c selects the better of two random choices by metrics",
 
     assert.deepEqual(calls, ["model-b"]);
   } finally {
-    Math.random = originalRandom;
+    _setSecureRandomFloatSource(null);
   }
 });
 
@@ -1647,9 +1644,8 @@ test("handleComboChat cost-optimized orders models by the cheapest configured in
 });
 
 test("handleComboChat weighted strategy resolves nested combos before falling back to the next weighted target", async () => {
-  const originalRandom = Math.random;
   const calls: any[] = [];
-  Math.random = () => 0.01;
+  _setSecureRandomFloatSource(() => 0.01);
 
   try {
     const result = await handleComboChat({
@@ -1687,7 +1683,7 @@ test("handleComboChat weighted strategy resolves nested combos before falling ba
     assert.equal(result.ok, true);
     assert.deepEqual(calls, ["model-a", "model-b"]);
   } finally {
-    Math.random = originalRandom;
+    _setSecureRandomFloatSource(null);
   }
 });
 

tests/unit/secure-random-routing.test.ts

@@ -0,0 +1,82 @@
+/**
+ * Regression guard for CodeQL alert #665 (js/insecure-randomness).
+ *
+ * Combo target selection (weighted / random / power-of-two-choices), the credential
+ * shuffle deck, and shadow-routing sampling previously used Math.random(). CodeQL flags
+ * that as "insecure randomness in a security context" (a false positive — these are
+ * provider load-balancing decisions, not secrets/tokens). The fix routes all of them
+ * through the crypto-secure helper in src/shared/utils/secureRandom.ts.
+ *
+ * This test pins both halves: (1) the helper behaves as a drop-in for Math.random with
+ * identical ranges, and (2) the routing-selection source files no longer call
+ * Math.random() — the static guard is RED before the fix and GREEN after.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { dirname, join } from "node:path";
+
+import { secureRandomFloat, secureRandomInt } from "../../src/shared/utils/secureRandom.ts";
+import { fisherYatesShuffle } from "../../src/shared/utils/shuffleDeck.ts";
+
+const repoRoot = join(dirname(fileURLToPath(import.meta.url)), "..", "..");
+
+test("secureRandomFloat: uniform in [0, 1), high entropy", () => {
+  const samples = Array.from({ length: 5000 }, () => secureRandomFloat());
+  for (const v of samples) {
+    assert.ok(v >= 0 && v < 1, `value out of [0,1): ${v}`);
+  }
+  assert.ok(new Set(samples).size > 4500, "expected near-unique draws (high entropy)");
+  const mean = samples.reduce((a, b) => a + b, 0) / samples.length;
+  assert.ok(mean > 0.45 && mean < 0.55, `mean should be ~0.5, got ${mean}`);
+});
+
+test("secureRandomInt: integer in [0, n) and covers every bucket", () => {
+  for (const n of [2, 3, 5, 10, 50]) {
+    const seen = new Set<number>();
+    for (let i = 0; i < 2000; i++) {
+      const v = secureRandomInt(n);
+      assert.ok(Number.isInteger(v) && v >= 0 && v < n, `n=${n} produced ${v}`);
+      seen.add(v);
+    }
+    assert.equal(seen.size, n, `n=${n}: every index in [0,n) should appear over 2000 draws`);
+  }
+});
+
+test("secureRandomInt: <= 1 mirrors Math.floor(Math.random() * {0,1}) === 0", () => {
+  assert.equal(secureRandomInt(1), 0);
+  assert.equal(secureRandomInt(0), 0);
+  assert.equal(secureRandomInt(-7), 0);
+  assert.equal(secureRandomInt(Number.NaN), 0);
+});
+
+test("fisherYatesShuffle: permutation, non-mutating, actually reorders", () => {
+  const input = Object.freeze(["a", "b", "c", "d", "e", "f"]);
+  const out = fisherYatesShuffle(input);
+  assert.equal(out.length, input.length);
+  assert.deepEqual([...out].sort(), [...input].sort(), "must be a permutation of the input");
+
+  let reordered = 0;
+  for (let i = 0; i < 100; i++) {
+    if (fisherYatesShuffle(input).join() !== input.join()) reordered++;
+  }
+  assert.ok(reordered > 0, "shuffle should sometimes change order");
+  assert.deepEqual([...input], ["a", "b", "c", "d", "e", "f"], "input array must not be mutated");
+});
+
+test("routing-selection RNG uses the crypto-secure helper, not Math.random (CodeQL #665)", () => {
+  const files = [
+    "open-sse/services/combo/targetSorters.ts",
+    "open-sse/services/combo/shadowRouting.ts",
+    "src/shared/utils/shuffleDeck.ts",
+  ];
+  for (const rel of files) {
+    const src = readFileSync(join(repoRoot, rel), "utf8");
+    assert.ok(
+      !/Math\.random\s*\(/.test(src),
+      `${rel} must not call Math.random() in a routing/selection context — use secureRandom* (js/insecure-randomness #665)`
+    );
+    assert.match(src, /secureRandom(Int|Float)/, `${rel} should use the secureRandom helper`);
+  }
+});