deep-review: sprayshed — TST: test_rate_limit_enforced is tautological and masks the missing rate limit

[5h4d0wn1k]

Severity

Medium — the only test guarding a safety property (rate limiting) can never fail, giving false confidence; mutation-proven.

Location

• tests/test_spray.py:60-73:

def test_rate_limit_enforced(self):
    ...
    cfg["spray"]["delay_between_attempts"] = 0.1
    ...
    elapsed = time.monotonic() - start
    self.assertGreaterEqual(elapsed, 0.0)

elapsed >= 0.0 holds for every possible time.monotonic() delta, including 0.

Repro (mutation proof, executed against live lab)

python3 -c "
import time
from sprayshed.spray import spray_password
from sprayshed.config import get_config
from tests.fixtures import get_shared_lab
mgr = get_shared_lab()
cfg = get_config(); cfg['spray']['delay_between_attempts'] = 100.0  # absurd: 100s spacing
from sprayshed.lab.http_sim import try_auth as http_try_auth
t0 = time.monotonic()
spray_password('Summer2024', ['jane.doe'], http_try_auth, mgr.host, cfg['lab']['http_port'], cfg, dry_run=False)
elapsed = time.monotonic()-t0
print('elapsed with 100s configured spacing:', round(elapsed,3), 's; test assertion (>= 0.0):', elapsed >= 0.0)
from tests.fixtures import stop_shared_lab; stop_shared_lab()
"
# observed: elapsed ~0.001s with 100s configured spacing, yet the test's assertion still passes
# companion PRF issue proves the underlying enforcement gap (burst bypass)

Observed vs expected

• Observed: test passes with rate limiting effectively absent (100s spacing honored for 1ms).
• Expected: test asserts a real bound, e.g. elapsed >= (len(users)-1) * delay with ≥2 users, and fails when pacing is removed.

Safe remediation

Rewrite the test with 3+ users and assert elapsed >= (n-1)*delay*0.9; verify it fails on the current code (red) then fix the limiter (burst=1, see companion PRF issue) to green. No product behavior change from the test edit itself.

[5h4d0wn1k]

Re-audit 2026-09-29 — verdict: KEEP (re-verified against fresh clone @ ef5b991, live lab sim)

Mutation proof re-executed. With an absurd configured spacing of 100s against a single user:

elapsed with 100s configured spacing: 0.001 s; test assertion (>=0.0) passes: True

Confirmed at source: tests/test_spray.py:60-73 ends in self.assertGreaterEqual(elapsed, 0.0). Since time.monotonic() is monotonic, that expression is true for every possible execution — including zero, negative deltas from any clock artefact, and a completely absent rate limiter. The test cannot fail.

Additional re-audit finding that makes this issue more than cosmetic: the test does not merely fail to catch a regression, it certifies one. The suite run during this re-audit passes 85/85 (Ran 85 tests in 2.763s / OK; pytest tests/ -q → 85 passed in 2.90s) while the rate limiter it purports to guard is provably inert (#9, companion). The repo's README.md:182 metric "Rate limiting: per-user attempts capped at configured threshold; cooldown honored" is therefore backed by a green check that has no content.

Also verified the surrounding suite is otherwise honest — this is an isolated tautology, not systemic test rot. The remediation proposed here (assert a real bound with ≥2 users, confirm red, then fix the limiter) is exactly right and correctly sequenced behind the #9 fix.

Leaving open.