siftlode

2,116 Commits 1 Branch 73 Tags

3 Commits

Include renames

Author	SHA1	Message	Date
peter	3dc14402b0	test(auth): pin what rounds 1 and 2 left unpinned (E9.5 review round 3) Three low findings, all in what the review rounds themselves wrote. The comment on `test_approving_an_unverified_account_keeps_its_password` still asserted the argument rounds 1 and 2 refuted — that dropping the approval-time revoke was safe because `password_login` reads `email_verified` first — while the docstrings on the two functions it describes now say the opposite. Two contradictory "why" records for one rule is how the round-1 takeover primitive got reasoned into existence in the first place, so the test comment now carries the corrected account and names SEC-16. `test_a_later_registration_leaves_a_link_already_in_flight_alive` asserted a subset relation that is vacuously true on an empty set, and the guard that made it non-empty had gone out with the revert. It now pins both halves: the old link survives, and exactly one new one appears. Round 2's own fix — activation moved to after a successful send — was unpinned; putting it back above the send left the suite green. The failed-invitation test now also asserts the account was not activated by a request the UI reported as a failure.	2026-09-04 23:04:09 +02:00
peter	540ca244c8	revert(auth): the newest registration does not own an unverified account (E9.5 review round 2) Round 1 fixed the address pre-emption chain by letting a later registration replace the password on an unverified account and delete its outstanding verify tokens. Round 2 read that code and showed it was worse than what it fixed, so it goes back out. It closes nothing. The rule is symmetric and an attacker can simply register last — on a timer, inside the five-per-five-minutes budget — while the owner registers once. What it does add is two primitives an unauthenticated caller did not have: overwrite a real registrant's chosen password, and delete the confirmation link already sitting in their inbox (the approval mail's 7-day token is an ordinary verify token, so the blanket delete took that too). Worst of all it fired on the rows SEC-10 left stripped — password_hash NULL, password_revoked_at set, active — installing an attacker's password where the entire state means "there is none", and clearing the stamp that is the only reason password-reset still accepts them. That is a straight takeover which did not exist before, in the code written to prevent one. The owner's recourse needed no new primitive and already worked: password-reset accepts a pre-empted account, proves the mailbox, rewrites the hash and bumps the session epoch. What remains is the bounded window between a confirmation click and that reset, which is SEC-16 — logged with this attempt written down, so the next person does not reach for it again. Two smaller round-2 findings go with it. `upsert_pending_invite`'s new exact-spelling tie-break ran before any status preference, so it could deterministically pick a DENIED sibling over a pending one and reopen it — the very outcome its comment claimed to prevent; the status preference now comes first. And `send_invitation` activated the account before the send it might have to report as failed, so a request the UI called a failure had quietly let someone in; activation now follows a mail that actually left. The verify token minted to render that mail still commits either way — it is single-use, expires on its own and was mailed nowhere, and the note says so rather than leaving the next reader to wonder whether it was missed.	2026-09-04 22:54:52 +02:00
peter	563fa0513d	fix(auth): close what the epic's own review found (E9.5 review round 1) Eleven findings survived adversarial verification; these are the fixes. Address pre-emption. S1 dropped the approval-time password revoke, arguing `password_login` refuses an unverified account before it reads `is_active`. Two verifiers showed that holds only while the address stays unverified — and S6's approval mail exists to get the real owner to end that state, at which point the squatter's password works. While an address is unverified the NEWEST registration now owns it: the password is replaced and every outstanding verify token deleted, so the owner takes the row back simply by registering, and a link already in flight cannot confirm it for anyone else. The residual (an owner who confirms and neither registers nor resets) is logged as SEC-16 beside SEC-15, where the re-attestation path it needs already lives. The comments that asserted the old, false invariant now say what actually guards this. The entry URL. S2 moved `clearEntryUrl()` out of the signed-out auth card into a hook App mounts unconditionally, and it blanket-stripped the whole query string, the fragment and `history.state` on the FIRST commit. That broke first-run setup outright — the wizard mounts only after its query resolves and then finds no `?token=` — and silently ate `?link=`/`?oauth=` (no toast, no refetch after connecting Google), `?v=`/`?chan=` deep links, and scroll restoration. It now clears only the fragment and the two one-time flags it owns, and preserves `history.state`. The env allow-list. S3 canonicalised the Invite branch of `is_allowed` and left the ALLOWED_EMAILS/ADMIN_EMAILS fallback matching literally — the 2026-09-03 lockout with a different source of truth, plus a shadowing trick where an unauthenticated request under a sibling spelling permanently masked an env grant. Canonical on both sides now, and the same for the delete dialog's warning and the ADMIN_EMAILS role bootstrap. Honest states. `send_invitation` stamped "I already wrote to them" before a fail-soft background send; it sends inline and records only what left. The admin-side resend answered `{ok: true}` for a dropped mail; it now says so. Both also activate/serialize consistently with the other decide paths. Correctness sweep. The 7-day approval link said "expires in 168 hours" — durations render in days once there is more than one. `_register_account`, `upsert_pending_invite` and `add_invite` picked among canonical siblings with no ORDER BY and no exact-spelling tie-break, so which account got the re-send, and whether a denied row was reopened over a pending one, was up to Postgres. `purge_user` deleted every canonically-equal invite, revoking a surviving sibling account's access. `ConfirmProvider` wrapped its ReactNode message in a `<p>`. Approving no longer leaves the stuck-sign-ups list stale. The refusal panels announce and take focus. Tests. Three mailers rewritten this epic never rendered in any test; every `send_*` now renders in both languages. The orphan-key guard could not fail (it scanned the catalog that defines the keys) and had no reverse direction. Newly pinned: canonical activation, the surviving sibling's grant, a failed invitation and resend, an expired reset link, a throttled login's silence, the per-language admin fan-out, and that the entry URL keeps what it does not own.	2026-09-04 22:42:15 +02:00

Author

SHA1

Message

Date

peter

3dc14402b0

test(auth): pin what rounds 1 and 2 left unpinned (E9.5 review round 3)

Three low findings, all in what the review rounds themselves wrote.

The comment on `test_approving_an_unverified_account_keeps_its_password` still asserted the argument
rounds 1 and 2 refuted — that dropping the approval-time revoke was safe because `password_login`
reads `email_verified` first — while the docstrings on the two functions it describes now say the
opposite. Two contradictory "why" records for one rule is how the round-1 takeover primitive got
reasoned into existence in the first place, so the test comment now carries the corrected account
and names SEC-16.

`test_a_later_registration_leaves_a_link_already_in_flight_alive` asserted a subset relation that is
vacuously true on an empty set, and the guard that made it non-empty had gone out with the revert.
It now pins both halves: the old link survives, and exactly one new one appears.

Round 2's own fix — activation moved to after a successful send — was unpinned; putting it back
above the send left the suite green. The failed-invitation test now also asserts the account was not
activated by a request the UI reported as a failure.

2026-09-04 23:04:09 +02:00

peter

540ca244c8

revert(auth): the newest registration does not own an unverified account (E9.5 review round 2)

Round 1 fixed the address pre-emption chain by letting a later registration replace the password on
an unverified account and delete its outstanding verify tokens. Round 2 read that code and showed it
was worse than what it fixed, so it goes back out.

It closes nothing. The rule is symmetric and an attacker can simply register last — on a timer,
inside the five-per-five-minutes budget — while the owner registers once. What it does add is two
primitives an unauthenticated caller did not have: overwrite a real registrant's chosen password,
and delete the confirmation link already sitting in their inbox (the approval mail's 7-day token is
an ordinary verify token, so the blanket delete took that too). Worst of all it fired on the rows
SEC-10 left stripped — password_hash NULL, password_revoked_at set, active — installing an
attacker's password where the entire state means "there is none", and clearing the stamp that is the
only reason password-reset still accepts them. That is a straight takeover which did not exist
before, in the code written to prevent one.

The owner's recourse needed no new primitive and already worked: password-reset accepts a
pre-empted account, proves the mailbox, rewrites the hash and bumps the session epoch. What remains
is the bounded window between a confirmation click and that reset, which is SEC-16 — logged with
this attempt written down, so the next person does not reach for it again.

Two smaller round-2 findings go with it. `upsert_pending_invite`'s new exact-spelling tie-break ran
before any status preference, so it could deterministically pick a DENIED sibling over a pending one
and reopen it — the very outcome its comment claimed to prevent; the status preference now comes
first. And `send_invitation` activated the account before the send it might have to report as
failed, so a request the UI called a failure had quietly let someone in; activation now follows a
mail that actually left. The verify token minted to render that mail still commits either way — it
is single-use, expires on its own and was mailed nowhere, and the note says so rather than leaving
the next reader to wonder whether it was missed.

2026-09-04 22:54:52 +02:00

peter

563fa0513d

fix(auth): close what the epic's own review found (E9.5 review round 1)

Eleven findings survived adversarial verification; these are the fixes.

**Address pre-emption.** S1 dropped the approval-time password revoke, arguing `password_login`
refuses an unverified account before it reads `is_active`. Two verifiers showed that holds only
while the address stays unverified — and S6's approval mail exists to get the real owner to end that
state, at which point the squatter's password works. While an address is unverified the NEWEST
registration now owns it: the password is replaced and every outstanding verify token deleted, so
the owner takes the row back simply by registering, and a link already in flight cannot confirm it
for anyone else. The residual (an owner who confirms and neither registers nor resets) is logged as
SEC-16 beside SEC-15, where the re-attestation path it needs already lives. The comments that
asserted the old, false invariant now say what actually guards this.

**The entry URL.** S2 moved `clearEntryUrl()` out of the signed-out auth card into a hook App mounts
unconditionally, and it blanket-stripped the whole query string, the fragment and `history.state` on
the FIRST commit. That broke first-run setup outright — the wizard mounts only after its query
resolves and then finds no `?token=` — and silently ate `?link=`/`?oauth=` (no toast, no refetch
after connecting Google), `?v=`/`?chan=` deep links, and scroll restoration. It now clears only the
fragment and the two one-time flags it owns, and preserves `history.state`.

**The env allow-list.** S3 canonicalised the Invite branch of `is_allowed` and left the
ALLOWED_EMAILS/ADMIN_EMAILS fallback matching literally — the 2026-09-03 lockout with a different
source of truth, plus a shadowing trick where an unauthenticated request under a sibling spelling
permanently masked an env grant. Canonical on both sides now, and the same for the delete dialog's
warning and the ADMIN_EMAILS role bootstrap.

**Honest states.** `send_invitation` stamped "I already wrote to them" before a fail-soft background
send; it sends inline and records only what left. The admin-side resend answered `{ok: true}` for a
dropped mail; it now says so. Both also activate/serialize consistently with the other decide paths.

**Correctness sweep.** The 7-day approval link said "expires in 168 hours" — durations render in
days once there is more than one. `_register_account`, `upsert_pending_invite` and `add_invite`
picked among canonical siblings with no ORDER BY and no exact-spelling tie-break, so which account
got the re-send, and whether a denied row was reopened over a pending one, was up to Postgres.
`purge_user` deleted every canonically-equal invite, revoking a surviving sibling account's access.
`ConfirmProvider` wrapped its ReactNode message in a `<p>`. Approving no longer leaves the
stuck-sign-ups list stale. The refusal panels announce and take focus.

**Tests.** Three mailers rewritten this epic never rendered in any test; every `send_*` now renders
in both languages. The orphan-key guard could not fail (it scanned the catalog that defines the
keys) and had no reverse direction. Newly pinned: canonical activation, the surviving sibling's
grant, a failed invitation and resend, an expired reset link, a throttled login's silence, the
per-language admin fan-out, and that the entry URL keeps what it does not own.

2026-09-04 22:42:15 +02:00