diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 9f6a79f6..63f3faa5 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -8478,4 +8478,17 @@ _FHIR_ID_RE.fullmatch("abc\n") -> False the fix > **BOUND ON WHAT WAS VERIFIED.** The control flow above was read statically at `origin/main` and each cited line confirmed individually; it has **not** been reproduced by executing a login sequence. A test that resets the password of a claimed `admin` account on a two-administrator system and then asserts the temporary credential works is the confirming experiment, and it does not exist today. +> ⚠️ **Amendment, same day, strengthening the item on a peer review before it was pushed. Two of the original bound's open connections are now CLOSED, and the mechanism is stated more precisely.** +> +> **THE DEFECT IS A PROXY, and naming it that way makes the fix obvious.** The docstring at `auth/service.py:579` says the routine *"Only ever touches an unclaimed bootstrap (`must_change_password` still set)"* -- it **equates** unclaimed-ness with that flag. `must_change_password` is being used as a **proxy for "this account has never been claimed"**, and the equivalence holds only while the flag has exactly one writer. `admin_reset_password:2733` is a second writer, so it sets the proxy on an account that *was* claimed. The gate is not wrong about its own test; it is wrong that the test means what it is taken to mean. **The durable fix is to stop inferring claimed-ness from a mutable flag any path may raise** -- record it, or test something a reset cannot forge -- rather than to special-case the reset. +> +> **TWO UNOPENED CONNECTIONS FROM THE ORIGINAL BOUND ARE NOW READ, and both hold.** The original filing verified each link individually and said so; a peer correctly pointed out that individually-verified links are not a verified chain, because the gaps *between* them are invisible precisely when every link checked holds. +> - `store/store.py:7714-7729` -- `set_password` issues `UPDATE users SET ... must_change_password=?`, so the flag **is** persisted, not merely passed. +> - `auth/service.py:567-574` -- `_other_enabled_admin_exists` iterates users, skips disabled and the excluded id, and returns true for any holder of the ADMINISTRATOR role. A second enabled administrator **does** make `superseded` true, with no expiry wait. +> +> **WHAT REMAINS UNVERIFIED IS NOW SMALLER AND NAMED:** the chain has still not been executed end to end. What is unread is `get_user_by_username` returning the persisted flag on the read side. That is near-certain and it is still not the same as having run it. +> +> **THE CONFIRMING TEST MUST ASSERT THE CORRECT BEHAVIOUR, NOT REPRODUCE THE DEFECT.** A test written to assert what the code does today **turns the bug into expected behaviour**: whoever fixes this then sees a red test and concludes they broke something, so the green actively defends the defect. Write it as *reset a claimed `admin` on a two-administrator system, then **assert the temporary credential works***, and land it with **`@pytest.mark.xfail(strict=True)`**. `strict` is the load-bearing half -- it reds when the test starts **passing**, so the fix cannot land silently and the marker cannot rot into a permanently ignored line. A non-strict xfail is the same defect one level up. +> The `reason=` string is the **artifact**, and it must name the **mechanism, not the symptom**: the gate at `:584` tests `must_change_password`, `admin_reset_password:2733` re-raises it, and no route clears it. That sentence is what makes the test re-derivable once these line numbers drift, which they will. + **Cluster:** Authentication / account lifecycle. **Priority:** P2. **Verdict:** build. **Severity:** no deployment axis -- zero instances; on first deployment an administrator following the documented reset **would** render the `admin` account permanently unusable without either party being told.