From 1c8fe792384a9206b97a9ff59b9ba1ab3e7d7035 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?V=C3=ADctor=20R=2E=20Escobar?= <victor@vrescobar.com>
Date: Sat, 14 Feb 2026 13:48:36 +0100
Subject: [PATCH] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94=20reje?=
 =?UTF-8?q?ction=20sampling=20and=20robust=20test?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Use rejection sampling to eliminate modulo bias in generate_code().
  Values above the largest multiple of 1_000_000 in u32 are discarded
  and re-drawn (~0.02% rejection rate).
- Make generate_code_is_not_deterministic test robust against the
  1-in-10^6 collision chance by trying 10 pairs instead of one.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/security/pairing.rs | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/src/security/pairing.rs b/src/security/pairing.rs
index 814f951..5f55603 100644
--- a/src/security/pairing.rs
+++ b/src/security/pairing.rs
@@ -150,10 +150,23 @@ fn generate_code() -> String {
     // UUID v4 uses getrandom (backed by /dev/urandom on Linux, BCryptGenRandom
     // on Windows) — a CSPRNG. We extract 4 bytes from it for a uniform random
     // number in [0, 1_000_000).
-    let uuid = uuid::Uuid::new_v4();
-    let bytes = uuid.as_bytes();
-    let raw = u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]);
-    format!("{:06}", raw % 1_000_000)
+    //
+    // Rejection sampling eliminates modulo bias: values above the largest
+    // multiple of 1_000_000 that fits in u32 are discarded and re-drawn.
+    // The rejection probability is ~0.02%, so this loop almost always exits
+    // on the first iteration.
+    const UPPER_BOUND: u32 = 1_000_000;
+    const REJECT_THRESHOLD: u32 = (u32::MAX / UPPER_BOUND) * UPPER_BOUND;
+
+    loop {
+        let uuid = uuid::Uuid::new_v4();
+        let bytes = uuid.as_bytes();
+        let raw = u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]);
+
+        if raw < REJECT_THRESHOLD {
+            return format!("{:06}", raw % UPPER_BOUND);
+        }
+    }
 }
 
 /// Generate a cryptographically-adequate bearer token (hex-encoded).
@@ -314,11 +327,15 @@ mod tests {
 
     #[test]
     fn generate_code_is_not_deterministic() {
-        // Two codes generated in the same process should differ (with overwhelming
-        // probability — collision chance is 1 in 1,000,000).
-        let c1 = generate_code();
-        let c2 = generate_code();
-        assert_ne!(c1, c2, "Two consecutive codes should differ (CSPRNG)");
+        // Two codes should differ with overwhelming probability. We try
+        // multiple pairs so a single 1-in-10^6 collision doesn't cause
+        // a flaky CI failure. All 10 pairs colliding is ~1-in-10^60.
+        for _ in 0..10 {
+            if generate_code() != generate_code() {
+                return; // Pass: found a non-matching pair.
+            }
+        }
+        panic!("Generated 10 pairs of codes and all were collisions — CSPRNG failure");
     }
 
     #[test]