diff --git a/open-sse/services/combo/shadowRouting.ts b/open-sse/services/combo/shadowRouting.ts index 2fe214df77cf..5483b4320dab 100644 --- a/open-sse/services/combo/shadowRouting.ts +++ b/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, diff --git a/open-sse/services/combo/targetSorters.ts b/open-sse/services/combo/targetSorters.ts index 7829e7512b34..3432b639fe5a 100644 --- a/open-sse/services/combo/targetSorters.ts +++ b/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(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]; diff --git a/src/shared/utils/secureRandom.ts b/src/shared/utils/secureRandom.ts new file mode 100644 index 000000000000..2e3440f7d5af --- /dev/null +++ b/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; +} diff --git a/src/shared/utils/shuffleDeck.ts b/src/shared/utils/shuffleDeck.ts index 39d43713924d..76d67d81363c 100644 --- a/src/shared/utils/shuffleDeck.ts +++ b/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>(); export function fisherYatesShuffle(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; diff --git a/tests/unit/combo-routing-engine.test.ts b/tests/unit/combo-routing-engine.test.ts index 2f3972f785e8..ed07c9ac1ed4 100644 --- a/tests/unit/combo-routing-engine.test.ts +++ b/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); } }); diff --git a/tests/unit/secure-random-routing.test.ts b/tests/unit/secure-random-routing.test.ts new file mode 100644 index 000000000000..b865c34fa244 --- /dev/null +++ b/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(); + 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`); + } +});