From 78fa8685d40ddaeb757a8560f5459a74a096792d Mon Sep 17 00:00:00 2001
From: diegosouzapw <diegosouza.pw@gmail.com>
Date: Sat, 20 Jun 2026 21:00:41 -0300
Subject: [PATCH] fix(sse): unbiased crypto.randomInt for combo selection
 (follow-up to #4457)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

#4457 routed combo/deck/shadow selection RNG through node:crypto to clear CodeQL
js/insecure-randomness, but built secureRandomInt as Math.floor(cryptoFloat() * n) —
dividing/rounding a cryptographic value introduces modulo bias (CodeQL
js/biased-cryptographic-random, raised on main by #4455's analysis).

Use crypto.randomInt(n) (unbiased rejection sampling) on the production integer path; the
test-only float seam still scales a float for deterministic selection tests, but no crypto
value is divided/rounded into a biased integer in production. secureRandomFloat is unchanged
(its [0,1) value is only used in a weighted threshold / sample-rate comparison, never rounded
to a bounded index). Selection behavior and all migrated test assertions are unchanged.
---
 src/shared/utils/secureRandom.ts | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/src/shared/utils/secureRandom.ts b/src/shared/utils/secureRandom.ts
index 9d3037c3027..2e3440f7d5a 100644
--- a/src/shared/utils/secureRandom.ts
+++ b/src/shared/utils/secureRandom.ts
@@ -11,8 +11,12 @@
  *
  *   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 } from "node:crypto";
+import { randomBytes, randomInt } from "node:crypto";
 
 /** Default source: uniform float in [0, 1) from 48 bits of crypto entropy. */
 function cryptoRandomFloat(): number {
@@ -24,27 +28,32 @@ function cryptoRandomFloat(): number {
   return value / 2 ** 48;
 }
 
-// Swappable source — production always uses the crypto source; tests inject a deterministic
-// sequence via _setSecureRandomFloatSource (mirrors the `_resetAllDecks` test-only export in
-// shuffleDeck.ts). secureRandomInt derives from the same float source, so a given injected
-// value picks the exact same index that `Math.floor(Math.random() * n)` would have.
-let floatSource: () => number = cryptoRandomFloat;
+// 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 floatSource();
+  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.
+ * 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);
-  // Scale [0, 1) → [0, max); clamp defends the probability-0 case of a source returning ~1.
-  return Math.min(max - 1, Math.floor(floatSource() * max));
+  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);
 }
 
 /**
@@ -53,5 +62,5 @@ export function secureRandomInt(maxExclusive: number): number {
  * test-only export convention in shuffleDeck.ts.
  */
 export function _setSecureRandomFloatSource(source: (() => number) | null): void {
-  floatSource = source ?? cryptoRandomFloat;
+  testFloatSource = source ?? null;
 }