backlog: amend #1131 -- an administrative unlock DOES ship, and the regex that missed it could never have found it - #357
Merged
Conversation
…egex that missed it could never have found it
Reported by the ASVS tracking lane after a builder found three things they had not.
All four load-bearing claims verified here against origin/main, not relayed.
1. THE REMEDY IS STRONGER THAN THE ITEM CREDITS. auth_routes.py:753 routes
POST /users/{user_id}/reset-password and :757 takes
Depends(require_step_up(Permission.USERS_MANAGE)) -- the admin RE-AUTHENTICATES.
That matters because #1131's own research question says an unlock route is "a
candidate with its own abuse surface, not an answer", and a step-up is a partial
answer to exactly that surface. Coverage is uniform: locked_until=NULL in all
three backends, two sites each, so this is not a partial-surface finding.
2. THE :1474 CITATION IS STALE. At 9d98f33 that line is an unrelated AD row; the
disputed parenthetical is at :1544. Cite the greppable phrase, not the line.
3. THE METHOD DEFECT, which is the part worth keeping. The filed absence claim
searched `def admin_unlock|unlock_account|clear_lockout`. Measured: 0 hits in
auth/service.py, a file containing exactly 1 `def admin_reset_password`. THE REGEX
CANNOT STRUCTURALLY MATCH THE THING THAT SHIPS. The zero was a true answer to a
question nobody asked, and the conclusion "no dedicated administrative unlock
exists" is literally true and materially misleading -- an unlock ships, as a side
effect of a forced password rotation.
PRESCRIPTION, recorded in the item: an absence claim over a CAPABILITY must be
written from the STATE, not from plausible function names. "Find every write to
locked_until" would have found it; "find every function called something-unlock"
never could.
The item STAYS OPEN and its question hardens: the adequacy and disclosure of a
step-up-gated forced-rotation reset as a remedy for a MALICIOUS lockout, rather than
whether an unlock affordance exists. Closing it on the route's existence would answer
the easy question and drop the one worth researching.
Worth naming: this is the same defect family as the day's other measurement errors --
an instrument that cannot see the class it certifies -- but it is the first instance
found INSIDE A BACKLOG ITEM'S OWN EVIDENCE rather than in tooling.
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.
Docs-only, one file. #1131 stays OPEN and its question hardens. No new number.
Reported by the ASVS tracking lane after a builder found three things they had not. All four load-bearing claims verified here against
origin/main, not relayed.1. The remedy is stronger than the item credits
api/auth_routes.py:753routesPOST /users/{user_id}/reset-password, and:757takesDepends(require_step_up(Permission.USERS_MANAGE))-- the admin re-authenticates; it is not a plain permission check.That is load-bearing because #1131's own research question says "anything that clears a lock is itself a control an attacker wants, so an unlock route is a candidate with its own abuse surface, not an answer." A step-up is a partial answer to exactly that abuse surface, and nothing in the record credited it.
Coverage is uniform --
failed_attempts=0,locked_until=NULLinstore.py,postgres.pyandsqlserver.py, two sites each -- so this is not a partial-surface finding.2. The
docs/SECURITY.md:1474citation is staleAt engine
9d98f339that line is an unrelated AD row in the comparative-properties table. The disputed parenthetical is at:1544, in the three-limiter anti-automation table. Cite the greppable phrase, not the line number.3. The method defect, which is the part worth keeping
The filed absence claim searched
def admin_unlock|unlock_account|clear_lockout. Measured onmain:The regex cannot structurally match the thing that ships. So the zero was a true answer to a question nobody asked, and the item's conclusion -- "no dedicated administrative unlock exists" -- is literally true and materially misleading: an unlock ships, as a side effect of a forced password rotation.
The re-scope
The live question is the adequacy and disclosure of a step-up-gated forced-rotation reset as a remedy for a malicious lockout -- not whether an unlock affordance exists. That is harder than the question filed, which is why this stays open rather than closing on the route's existence.
Why it is worth a commit rather than a correction in passing
This is the same defect family as the day's other measurement errors -- an instrument that cannot see the class it certifies -- but it is the first instance found inside a backlog item's own evidence rather than in tooling. A false conclusion, from a true measurement, by a regex that was never able to answer the question it was written for.