fix(docs): the SEC-014 clamp does not preserve TOTP single-use — it is what costs it - #293
Merged
Merged
Conversation
…at costs it
Three places assert that the fast-clock clamp in verify_totp_step preserves
TOTP single-use: the function's own docstring (auth/totp.py), docs/SECURITY.md's
MFA section, and tests/test_totp_window.py's module docstring. At
`[auth].totp_skew_steps >= 1` that is false, and the clamp is the mechanism.
MEASURED, not reasoned. A tolerated future code is recorded against the CURRENT
step rather than its own, which leaves its own step unspent:
s = generate_secret(); t = 5000*30 + 5.0
future = totp(s, now=t+30)
verify_totp_step(s, future, now=t, window=1) -> step(t) # clamped
verify_totp_step(s, future, now=t+30, window=1) -> step(t)+1 # its own
The second is STRICTLY GREATER than the first, so a high-water store that
rejects a non-greater step accepts both: one code, two successful uses, which
ASVS 6.5.1 forbids ("only successfully usable once"). At the shipped default
window=0 single-use does hold -- the future code is refused outright rather than
clamped -- so this is the cost of the documented opt-out, not a defect in the
default posture. No verdict changes: ASVS 6.5.1 grades the shipped default.
WHY THE TEST SUITE MISSED IT. The gap was precise, not careless.
test_single_use_step_is_stable_for_the_same_code_and_now replays the same code
at the SAME now. test_optout_fast_clock_future_code_causes_no_self_lockout
replays a DIFFERENT (genuine) code at a later now. Nobody replayed the SAME code
at a LATER now -- which is the only arrangement where the clamp bites. Both
existing tests are correct; the conclusion drawn from them was not.
Adds test_optout_lets_one_tolerated_future_code_be_used_twice covering exactly
that arrangement, with the strict default as its control. Mutation-checked: with
`return min(matched, counter)` reduced to `return matched` the new test fails
alongside the two existing clamp tests, so it is load-bearing rather than
decorative.
No behaviour change. The clamp stays: it prevents a real self-inflicted lockout,
and removing it would trade a ~30 s replay window for locking users out of their
own current-step code. What changes is that the docs now state the trade instead
of denying it, and an operator reading SECURITY.md before setting
totp_skew_steps=1 is told what it costs.
Verified: 36 tests pass across test_totp_window/test_totp/test_totp_clock/
test_mfa; ruff format and ruff check clean on both touched .py files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wshallwshall
enabled auto-merge
August 8, 2026 22:04
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three places in the tree assert that the fast-clock clamp in
verify_totp_steppreserves TOTP single-use. At[auth].totp_skew_steps >= 1that is false — and the clamp is the mechanism, not the mitigation.messagefoundry/auth/totp.py— "The clamp only lowers the recorded step, so single-use is preserved."docs/SECURITY.md— "the forward step clamped to the current step so single-use holds"tests/test_totp_window.pymodule docstring — same sentenceMeasured, not reasoned
A tolerated future code is recorded against the current step rather than its own, which leaves its own step unspent:
The second resolution is strictly greater than the first, so a high-water store that rejects a non-greater step accepts both: one code, two successful uses. ASVS 6.5.1 requires TOTPs be "only successfully usable once".
At the shipped default
window=0single-use does hold — the future code is refused outright rather than clamped. So this is the cost of the documented opt-out, not a defect in the default posture. No ASVS verdict changes, since 6.5.1 grades the shipped default.Why the test suite missed it
The gap was precise rather than careless. Two neighbouring tests bracket the case without covering it:
test_single_use_step_is_stable_for_the_same_code_and_now— same code, samenowtest_optout_fast_clock_future_code_causes_no_self_lockout— different (genuine) code, laternowNobody replayed the same code at a later
now, which is the only arrangement where the clamp bites. Both existing tests are correct; the conclusion drawn from them wasn't.What changes
0.test_optout_lets_one_tolerated_future_code_be_used_twicecovering exactly that arrangement, with the strict default as its control.return min(matched, counter)toreturn matchedmakes the new test fail alongside the two existing clamp tests — so it's load-bearing, not decorative.No behaviour change. The clamp stays: it prevents a real self-inflicted lockout, and removing it would trade a ~30 s replay window for locking users out of their own current-step code. What changes is that an operator reading
SECURITY.mdbefore settingtotp_skew_steps = 1is now told what it costs.Verification
36 tests pass across
test_totp_window/test_totp/test_totp_clock/test_mfa.ruff format --checkandruff checkclean on both touched.pyfiles.Found while re-scoring ASVS 6.5.1 against this tree — the cell moved to
passon the shipped default, and this was the residual that survived it.🤖 Generated with Claude Code