Use SystemRandom for cluster shuffle so random.seed() cannot defeat it

The find_leader sweep shuffles its candidate list to avoid every
process picking up the same first node — a stampede-avoidance
mechanism. Using random.shuffle from the module-level PRNG meant a
downstream random.seed() call (test fixtures, libraries that want
deterministic behavior without scoping it) defeated the avoidance:
every seeded process picked the same shuffle and piled onto the same
node.

Switch to a module-level random.SystemRandom() instance. SystemRandom
draws from OS entropy and ignores random.seed(). Update the existing
shuffle-call pin to patch the new instance attribute. Pin the
seed-immunity property in a new test file.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: antoineleclair

src/dqliteclient/cluster.py

@@ -47,6 +47,14 @@
 # held in memory and serialised into every traceback.
 _MAX_ERROR_MESSAGE_SNIPPET = 200
 
+# Use OS-entropy randomness for the per-sweep node shuffle so that the
+# stampede-avoidance is not defeated by a downstream call to
+# ``random.seed(...)``. Test suites and some libraries seed the global
+# PRNG for determinism; if we used ``random.shuffle`` directly, every
+# process picking up that seed would produce the same shuffle and pile
+# onto the same node. ``SystemRandom`` ignores ``random.seed()``.
+_cluster_random = random.SystemRandom()
+
 # Budget for the bounded writer-drain in ``_query_leader``. A
 # responsive peer drains FIN/ACK in microseconds; a slow peer must not
 # hold up leader discovery. 100 ms is generous for LAN and still
@@ -258,7 +266,7 @@ async def _find_leader_impl(self, *, trust_server_heartbeat: bool) -> str:
         # "fix" this toward Go's deterministic behavior without adding
         # an explicit stampede-avoidance mechanism elsewhere.
         nodes = list(nodes)
-        random.shuffle(nodes)
+        _cluster_random.shuffle(nodes)
         nodes.sort(key=lambda n: 0 if n.role == NodeRole.VOTER else 1)
 
         errors: list[str] = []

tests/test_cluster.py

@@ -460,17 +460,19 @@ async def track(address: str, **_kwargs: object) -> str | None:
 
     async def test_find_leader_calls_random_shuffle_on_candidate_list(self) -> None:
         """Deterministic pin complementing the probabilistic test above:
-        ``find_leader`` invokes ``random.shuffle`` on its candidate list.
+        ``find_leader`` invokes the cluster shuffle on its candidate
+        list.
 
         The probabilistic test catches "shuffle disabled entirely"
         (``len(counts) == 1``) but not "shuffle replaced by some other
         deterministic permutation that still produces ≥ 2 distinct
         firsts" — a regression vector that would silently slip past.
-        Patching ``random.shuffle`` directly lets us assert the call
-        happens, and ``wraps=`` keeps the real shuffle behavior so
-        ``find_leader`` still terminates cleanly.
+        Patching the bound ``shuffle`` method on the module-level
+        ``_cluster_random`` directly lets us assert the call happens.
+        We swap to a deterministic identity-function so ``find_leader``
+        still terminates cleanly.
         """
-        import random as _random
+        from dqliteclient import cluster as cluster_module
 
         store = MemoryNodeStore(["n1:9001", "n2:9001", "n3:9001", "n4:9001"])
         client = ClusterClient(store, timeout=0.2)
@@ -479,17 +481,18 @@ async def fail_query(address: str, **_kwargs: object) -> str | None:
             raise DqliteConnectionError("not leader")
 
         with (
-            patch(
-                "dqliteclient.cluster.random.shuffle",
-                wraps=_random.shuffle,
+            patch.object(
+                cluster_module._cluster_random,
+                "shuffle",
+                wraps=cluster_module._cluster_random.shuffle,
             ) as mock_shuffle,
             patch.object(client, "_query_leader", side_effect=fail_query),
             contextlib.suppress(ClusterError),
         ):
             await client.find_leader()
 
         assert mock_shuffle.call_count >= 1, (
-            "find_leader must invoke random.shuffle on its candidate list — "
+            "find_leader must invoke the cluster shuffle on its candidate list — "
             "without the shuffle, parallel callers stampede the first-listed node"
         )
         # The first positional arg is the list being shuffled in place.

tests/test_cluster_shuffle_seed_immunity.py

@@ -0,0 +1,65 @@
+"""Pin: cluster shuffle is immune to ``random.seed()``.
+
+The cluster module uses ``random.SystemRandom`` (exposed as
+``_cluster_random``) so that a downstream call to ``random.seed(...)``
+— common in test suites or in libraries that "want deterministic
+behavior" without scoping it — cannot defeat the per-sweep
+stampede-avoidance shuffle.
+
+If we accidentally regress to module-level ``random.shuffle``, the
+seeded test below would produce identical shuffles every run and
+this test would fail.
+"""
+
+from __future__ import annotations
+
+import random
+
+from dqliteclient.cluster import _cluster_random
+
+
+def test_cluster_random_is_system_random() -> None:
+    """The cluster shuffle source is a SystemRandom instance, not the
+    module-level Random."""
+    assert isinstance(_cluster_random, random.SystemRandom)
+
+
+def test_cluster_shuffle_unaffected_by_seed() -> None:
+    """Two shuffles taken with identical seeds applied to the global
+    PRNG must still differ — proving the cluster shuffle does not
+    consume the global PRNG state."""
+    nodes_first: list[int] = list(range(100))
+    nodes_second: list[int] = list(range(100))
+
+    random.seed(42)
+    _cluster_random.shuffle(nodes_first)
+
+    random.seed(42)
+    _cluster_random.shuffle(nodes_second)
+
+    # With SystemRandom, two seeded global states do not produce the
+    # same shuffle. (The probability of a 100-element collision is
+    # 1/100! — astronomically low.)
+    assert nodes_first != nodes_second, (
+        "cluster shuffle must use OS entropy, not the seeded module PRNG"
+    )
+
+
+def test_cluster_shuffle_does_not_consume_module_random_state() -> None:
+    """A cluster shuffle in between two module-level ``random.random()``
+    draws must not affect the second draw — proving the cluster
+    shuffle is isolated from the module PRNG.
+    """
+    random.seed(123)
+    _ = random.random()
+    expected_next = random.random()
+
+    # Reset, take one draw, then a cluster shuffle, then the next draw.
+    random.seed(123)
+    _ = random.random()
+    _cluster_random.shuffle(list(range(100)))
+    actual_next = random.random()
+
+    assert actual_next == expected_next, (
+        "cluster shuffle must not consume bytes from the global module-level Random instance"
+    )