fix(auth): the rotating 2FA verify routes echo the live user row, not the pre-rotation snapshot - #17000
Conversation
On the enrolment lane better-auth writes `twoFactorEnabled: true`, rotates the session and only then calls the `valid(ctx)` closure it built at entry, so a successful `/two-factor/verify-totp` answered `user.twoFactorEnabled: false` to the caller who had just switched 2FA on. `two-factor-rotated-token-echo` already repaired the body's other stale member, `token`, on exactly these routes and on exactly this predicate; `user` is stale for the same reason and is now repaired under the same one. Narrowed twice: only the members the vendor already echoed are written, so the published payload shape cannot widen; and the row is re-read through `internalAdapter` by the id the response itself published, so the repair travels the same output transform that produced the echo and can never substitute a different principal. `/two-factor/verify-backup-code` does not rotate, is in neither path list, and is pinned as a negative control on both drive legs. A row read that throws degrades to the vendor's own echo, never to a failed verification and never to a lost `token` repair. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…o-factor-user-echo
📓 Docs Drift Check4 anchor(s) derived from 2 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 083f74e29175dd81bf9dd0994bdb168bcd76f026 && git checkout 083f74e29175dd81bf9dd0994bdb168bcd76f026
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2fd714f4fb21be92186cec5c0259dadbe2ffa966 6d19f483905542a0a08e5a9d0591e6b3dd2f8696 && git checkout -B drift-repro 2fd714f4fb21be92186cec5c0259dadbe2ffa966 && git merge --no-ff 6d19f483905542a0a08e5a9d0591e6b3dd2f8696
node scripts/docs-audit/affected-docs.mjs --json 2fd714f4fb21be92186cec5c0259dadbe2ffa966 |
Fixes #16535
Clause-②: no
On the enrolment lane, better-auth's
verifyTwoFactorresolves the caller's session once, at entry, and closes over it./two-factor/verify-totpthen writestwoFactorEnabled: true, mints a new session, installs it and deletes the old row — and only afterwards calls that closure, which serialisesparseUserOutput(session.user)from the pre-rotation snapshot. So a successful enrolment answereduser.twoFactorEnabled: falseto the very caller who had just switched 2FA on.two-factor-rotated-token-echo.ts(#10701) already repaired the body's other stale member,token, on exactly these routes and on exactly this predicate — the response staged a session cookie whose token differs from the one echoed.useris stale for the same reason, so it is repaired under the same predicate rather than a new one. No new decision logic.What changed
packages/plugins/plugin-auth/src/two-factor-rotated-token-echo.tsfreshEchoedUser(ctx, echoed)re-reads the row and returns the echo with its values refreshed.echoInstalledSessionTokenapplies it after thetokenrepair, inside the sametry/catch.Two narrowings, each of which is the whole safety argument for one hazard:
useris a published wire shape (AuthWireUserin@objectstack/client), and better-auth's own output filter (filterOutputFields) is a deny-list — forwarding the raw row would put every column it happens to carry on the wire. The echoed key set is the ceiling: the repair corrects values, never the shape.internalAdapter.findUserById, by the id the response already published. Same seam and same output transform that produced the echo, so a driver that stores booleans as1/0cannot change a member's wire type; and reading by the echoed id means the repair can never substitute a different principal — the mirror of [security-sensitive] 2FA verify-totp token is not accepted on the bearer seam and overrides a valid cookie (fail-closed lockout) #10701 reading its token back out of the response's own cookie.packages/client/src/index.tsdrops theAuthTwoFactorVerificationResult.userwarning that told callers to re-read the session for the live flag, and the matching note onAuthWireUser.twoFactorEnabled. The declared wire shape is unchanged.验收备注
Triage's four conditions, verbatim, each with the evidence that answers it.
1. In one successful
verify-totpresponse,user.twoFactorEnabledmust equal the real value of that row in the adapter. ⛔ Assert against the row you read back — not against a literaltrue.Two pins, one per drive leg, both reading the row at rest — the memory engine's own table, and at driver level under
SqlDriver— deliberately not through better-auth's adapter, which is the seam the fix itself reads (that would certify the fix against itself):condition ①: user.twoFactorEnabled equals the value stored for that row(in-memory engine)condition ①, at driver level: the echo equals the stored column(realObjectQL+SqlDriveron better-sqlite3)The expected value is computed from a fresh read of the row, never written as a literal; the only normalisation is on the row side, across spellings of the same boolean (SQLite
1/0vstrue/false), and the echo is never normalised. The property a literaltruecannot distinguish is measured directly byfollows the row DOWN as well as up — the echo tracks the row, not a literal: the row says OFF and the echo must say OFF, on the very lane the repair rewrites.2.
verify-otptravels the same vendor rotation block and is already in the same path table. ⛔ Do not leave a known same-shaped hole.Verified in the installed artifact:
otp/index.mjscarries the byte-identicalupdateUser→createSession→setSessionCookie→deleteSession→valid(ctx)block. The repair keys onROTATING_TWO_FACTOR_VERIFY_PATHS, so it covers both entries by construction, and the pin is table-driven —it.each([...ROTATING_TWO_FACTOR_VERIFY_PATHS])('repairs user on %s')— so a third rotating path added later is covered the day it enters the table. It is measured at the hook rather than end to end because this manager buildstwoFactor()with no OTP transport, exactly as the module header already records.3.
verify-backup-codemust be unaffected — the negative control against a blanket re-read.It does not rotate, is in neither path list, and its row is never read. Pinned on both drive legs, in the same session as the enrolment it follows:
condition ③ (negative control): verify-backup-code still echoes the live rowcondition ③, at driver level: verify-backup-code still echoes the live rowleaves a path OUTSIDE the table alone, user includedThat the pairing is doing work is measured, not asserted — see ablation A below: the
useralignment is removed and these three stay green while six others go red.4. Keep the file's failure posture — a failure while tidying the body must ⛔ never turn an already-successful verification into a failure.
Measured through the real pipeline, because the claim is about the response and not about the helper:
findUserByIdis poisoned to throw in the window betweenenableand the rotatingverify-totp(on the signed-in lane it is the only thing that reads it, so the poison isolates the repair from the request it measures). The verification still answers 200, thetokenrepair — which runs first — survives, anduserdegrades to the vendor's own pre-flip echo rather than to a 500 or a missing member. Two seam-level pins cover the other degradations: a throwing read and a row that cannot be found.Verification
Both drive legs, before and after, on a tree fetched here (
origin/main b834b48e7, merged forward tofe2b7554a):verify-totpecho vs stored rowfalsevs storedtrue— staletruevs storedtrueSqlDriver(better-sqlite3) through realObjectQL, at driver levelfalsevs stored columntrue— staletruevs storedtrueverify-backup-codeon both legs (control)The "before" column is the ablation reading, not a recollection: the assertion text under ablation A is
verify-totp echoed the PRE-rotation user snapshot: expected false to be true.Anchors, re-measured on a tree fetched here
Triage's readings reproduced against the commit object at
origin/main b834b48e7— not against a commit id copied from a comment:The zero is a real zero: the positive control answers 21 on the same file and the repo control proves the identifier is spelled that way and does exist in the package.
Ablation — three legs, one per enforcement point
Each leg: mutate, prove the mutation on disk (anchor grep counts plus
git hash-objectdiffering from the HEAD blob, with a no-op mutation refused outright), run the pins, restore withgit checkout HEAD -- ABSPATHunder atrap ... EXIT INT TERM, and prove restoration by blob equality and a zero-bytegit diff HEAD. HEAD blob throughout:f7c9df74a73ce6a4203badc5ae82a00eb1fa2e16.A — remove the
useralignment. Mutated blob49c346e836; anchorconst fresh = await freshEchoedUser1 to 0, planted marker 0 to 1. 6 failed, 27 passed. Red: condition ① on both drive legs, both table paths, the follows-the-row-down pin, the no-widening pin. Green: bothverify-backup-codecontrols, the vendor-shape parity pins, and all six #10701 pins. That pairing is the whole point — a blanket re-read would have reddened the controls too.B — remove the failure posture (
catchrethrows). Mutated blob7fb0d730b3. 2 failed, 31 passed, withAssertionError: verify-totp (enrolment): expected 500 to be 200— condition 4 measured rather than asserted: without the posture, an adapter that throws turns a completed verification into a 500.C — remove the key-set ceiling (blanket forward of the row). Mutated blob
f2062771cc. Exactly 1 failed, 32 passed: onlynever widens the echoed payload with columns the vendor did not publish. Condition ① and bothverify-backup-codecontrols stay green — which is precisely the difference between this repair and "re-read the row and forward it", and why that pin is written at the seam (the fixture rows carry no surplus column, so the end-to-end parity pins cannot see it).All three restored clean: blob back to
f7c9df74a7,git diff HEAD0 bytes.Gates
Union derived with
scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackand reconciled with--ran, on the final head after mergingorigin/main(git rev-parse --short HEADat the union run:6d19f4839):PREREQUISITE NOT METanswers (check:skill-examplesexit 1,check:dual-build-cjs-loadsexit 3,check:type-check-debtexit 3). Those are not passes and are not reported as any: the closure named by each was built (turbo run build --filter='./packages/*' --filter='./packages/*/*') and all three re-ran to a real exit 0.pnpm check:route-enveloperun explicitly (it sits in the silent bucket, dispatch-gates: a whole-tree-walk gate whose workflownames:lists only its CURRENT members is placed Silent, so it is never derived for the card that adds a new member — measured on check:route-envelope / PR #16730 #16828): exit 0. It did not ask for a module declaration — this change writes no response body, it corrects one member of the vendor's own returned object inside an after-hook, andplugin-auth's route file is already ruledexempt/vendor wire. NovendorWire, noexempt, noconsthoist: maintainer-only territory was not entered.check:auth-mount-ledger,check:error-code-casing,check:authz-resolver,check:filter-alias-parity,check-changeset-fixed— all exit 0.pnpm lintover the whole repo, unnarrowed: 6391 files, 0 errors, 0 warnings, exit 0.pnpm --filter @objectstack/plugin-auth --filter @objectstack/client typecheck— exit 0.pnpm --filter @objectstack/plugin-auth test— 106 files, 2231 tests passed.pnpm --filter @objectstack/client test— 35 files, 454 tests passed.Out of scope, noted not filed
two-factor-rotated-token-echoalthough it now repairs two members. Renaming it touches the import inauth-manager.ts, the cross-reference inpackages/client, and two sibling test headers, for no behaviour change — a move, not a fix, and not a rider on this card.arrangeCompletedEnrolmentnow returnspreEnrolmentCookie, which no pin reads yet; it is the natural handle for a future non-rotating case and costs nothing.Authored by Claude Code — session https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Generated by Claude Code