fix(plugin-auth): a 2FA verification echoes the session it installed, not the one it deleted

[os-warren]

Fixes#10701

A successful 2FA verification handed the caller a session token it had just
deleted, and a client that believed the echo locked itself out of a session it
still held.

What was measured

POST /api/v1/auth/two-factor/verify-totp answers 200 carrying two
credentials that disagree. The Set-Cookie names the caller's rotated session;
the JSON token names the session row the same request deleted.

Reproduced from the mechanism description (the full reproduction is withheld
under the auth/authz disclosure carve-out and stays in the QA session). All
three cases from the report were measured side by side in one tree, one run,
one arrangement — before the fix:

credentials presented	get-session	get-totp-uri
rotated cookie alone	resolves to the user	200
echoed token as bearer alone	200 and empty	401
rotated cookie + echoed token	200 and empty	401
set-auth-token bearer alone (control)	resolves to the user	200

The third row is the finding. It is not that the bearer is useless — it is that
a useless bearer destroys an otherwise valid cookie session. The fourth row
is the control that says so: the bearer seam itself works fine, so nothing about
credential resolution is broken. Only the echoed value is wrong.

What the token turned out to be

Not an internal identifier, and not a malformed credential: it is the caller's
pre-rotation session token — a credential that was valid until moments
earlier in the same request.

better-auth's verifyTwoFactor resolves the caller's session once, at entry,
and closes over it:

valid: async(ctx)=>ctx.json({token: session.session.token, ... })

On the enrolment lane, /two-factor/verify-totp rotates that session before it
answers — mints a new session, installs it with setSessionCookie, then deletes
the caller's original row — and only afterwards calls valid(ctx), which still
holds the pre-rotation session. Measured on the installed better-auth 1.7.1
(dist/plugins/two-factor/verify-two-factor.mjs,
dist/plugins/two-factor/totp/index.mjs).

The reason a dead echo is worse than no echo is bearer(): its before-hook
overwrites the request's session cookie with whatever the Authorization
header carries, so presenting the dead token does not merely fail — it discards
the good cookie and drops the request to anonymous.

The fix

Direction (a), the "make the echoed token an actually-accepted bearer" arm. The
value is now read back out of the response's own session cookie, so token
names the session the response actually installed.

The arm was chosen on measurement, not preference: every other auth response in
this repo echoes token as the unsigned token of a live session, and
bearer() accepts exactly that (presented without a signature it signs the
value itself before verifying). Measured on /sign-up/email, that body's
token resolves to the user as a bearer. So verify-totp is the only endpoint
breaking a contract the rest of the surface already keeps — which makes this a
restoration rather than a change, and made "stop echoing the token" the wrong
arm: the field is right, the value was wrong.

Keyed on the mechanism rather than the enrolment branch: the echo is repaired
only when the response staged a session cookie whose token differs from the one
being echoed. Nothing is invented — the corrected value comes out of the
response's own cookie, so this cannot hand a caller a credential the request did
not already grant it.

/two-factor/verify-otp carries the byte-identical rotate-then-answer block and
is covered by the same guard rather than left as a known-identical hole; it is
not pinned by a test here, because exercising it needs OTP transport
configuration this package's harness does not wire.
/two-factor/verify-backup-code does not rotate and is unaffected.

Resolver precedence is untouched

Direction (b) from the report — the resolver falling back to the cookie when the
bearer is unusable — was ruled out of scope because it stops an invalid
credential from failing loud, and this change does not go near it. Two pins hold
that line so a later attempt to loosen it reddens this suite: anonymous is still
refused, and a bogus bearer still overrides a valid cookie and still fails
closed.

Pins

#	direction	pin
①	works	the echoed token is an accepted bearer — resolves to the exact user id, and get-totp-uri answers 200
②	still works	the rotated cookie alone still authenticates
⭐	the card	cookie + echoed token together resolve to the user (was anonymous)
③	still refused	anonymous resolves to nobody and gets 401
③	still refused	a bogus bearer still overrides a valid cookie and still 401s
④	genuinely accepted	the echoed token is a live session row, is not the deleted pre-enrolment token, and equals the token in the response's own Set-Cookie
—	untouched	completing a 2FA sign-in still echoes a token that authenticates

Assertions end at which principal does the next request resolve to, never at
status alone: get-session answers 200 for anonymous, which is exactly how
this defect read in the field.

Ablation

Predicted signature written down before mutating; both directions run.
Neutralising the repair predicted 4 failed / 13 passed of 17, naming the four
tests and their first failing assertions. Measured: 4 failed / 13 passed,
the same four, with the predicted messages — AssertionError: expected null to be the user id, and AssertionError: expected 401 to be 200. The load-bearing
prediction held too: the sign-in lane test passed under ablation,
independently confirming the repair is a genuine no-op there rather than an
untested claim.

Restore proved byte-identical by git hash-object
(6b352697eabe4b4320cd61c84696cf6cbfce98be before and after), and the restored
tree re-ran 17/17 green.

Positive control for src vs dist, settled rather than asserted: the package
has no dist/ directory at all and its vitest config declares no alias,
while the suite ran 1394 tests green — a suite resolving the subject through a
built dist/ could not have run.

Gates

Union derived after the final commit on a clean tree, with no path arguments
(node scripts/pm/dispatch-gates.mjs, 4 paths vs merge base 376b3dc0b), at
d5330290f. Exit codes captured before any pipe. All 18 exit 0; none returned
254, printed Command not found, or refused with PREREQUISITE NOT MET.

Selected verdict lines, as each gate printed them:

• ✓ check-adr-0087-registration: this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).
• ✓ check-changeset-no-major: "This diff introduces no major bump."
• ✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
• check-engine-double-contract: OK — 376 pinned, 133 in the DEBT ledger, 2 exempt.
• ✓ where-matcher conformance holds: 275 matcher(s) discovered, 275 answer the combinator battery correctly or refuse it loudly (165 refuse).
• ✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new, and every file in the population parsed.
• check-nul-bytes: OK (scanned 6316 text file(s) ... no raw ASCII control bytes).
• ✓ check-route-envelope self-test passed
• check-dispatcher-error-vocabulary --self-test: 8 shapes + 102 assertions OK (vocabulary + #9098 door typing)

The last two were not named by the path derivation; they were run explicitly
with --self-test because this change sits in the route/envelope class.

Package-scoped: @objectstack/plugin-authpnpm test 66 files / 1394 tests
passed; pnpm typecheck (tsc --noEmit) clean.

Declared narrowing.check:type-check-debt --re-measure was not run — it
needs the whole workspace closure built. What matters for that ratchet was
measured directly instead: with the package's test files included in the tsc
program, the new test file contributes 0 raw errors, so it cannot drift the
frozen TEST_DEBT count. The ledger entry was not edited and --lower was not
run. CI runs the gate itself.

Versioning note for review

Shipped as patch, deliberately, and this is the call most worth a second pair
of eyes. The dispatch expected minor on the assumption that the credential
semantics of a published endpoint change. What the measurement shows is a
contract restoration: the field keeps its shape (the unsigned session token)
and its meaning ("the session you now hold"), only the value moves from a
deleted row to the live one, and the previous value was not a usable credential
for anything — so no consumer expression has to be rewritten and nothing could
have depended on it. No breaking declaration is made, and consequently no
ADR-0087 disposition marker is required. If review reads it the other way, the
flip is a one-file edit.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

7 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see

• the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 11 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json c2b97c2a188d3a5798f5ee224a943ccb413c6396 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from d41ee399eae6b87de6b5ca6898f3556d7b04b40d — the merge of head d5330290f12f7c9b9fc57257e20d40ffc7a22927 into base c2b97c2a188d3a5798f5ee224a943ccb413c6396, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d41ee399eae6b87de6b5ca6898f3556d7b04b40d && git checkout d41ee399eae6b87de6b5ca6898f3556d7b04b40d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c2b97c2a188d3a5798f5ee224a943ccb413c6396 d5330290f12f7c9b9fc57257e20d40ffc7a22927 && git checkout -B drift-repro c2b97c2a188d3a5798f5ee224a943ccb413c6396 && git merge --no-ff d5330290f12f7c9b9fc57257e20d40ffc7a22927
node scripts/docs-audit/affected-docs.mjs --json c2b97c2a188d3a5798f5ee224a943ccb413c6396

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

[github-actions]

⛔ merge queue 构建失败 — 先分诊，再决定要不要重排

队列构建 32543625194 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集），
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job（日志抽取，best effort）：

• Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

  ✗ Build failed in 4.04s
  

↳ 失败原因 是判读的关键：超时（Test timed out in … / Hook timed out in …）多半是负载/时序，不是本 PR 的回归；
断言（AssertionError: …）才指向真实的行为改变。两者的 FAIL 行长得一模一样，只有这一行能区分。

跨 PR 相同签名（24h，按失败测试文件聚合）：

• ⚠️本次没有可用的聚合签名（日志里没有能解析出测试文件名的 FAIL 行）—— 这不是「没有同签名的其他 PR」，是这一轮没测到。跨 PR 聚合本次不可用，请手工比对其他 PR 的同类评论。
• ⚠️ 24h 评论账本没读完（超过 5 页仍未读到窗口尽头），所以上面的「不同 PR 数」是下界，不是全量。

历史信号：

• 本 PR 过去 24h 无队列失败记录（首次）。
• 过去 24h 队列共有 61 个失败构建（不含本次）。

分诊清单：

1. 失败测试在本 PR 改动的包里 → 真回归，修 PR。
2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」；已有汇总 issue ⇒ flaky/环境问题实锤，去那张 issue 上谈，修好前重排只会再烧一轮全队列。
3. 两者都不是 → 可能与同组 PR 语义冲突；等前面的 PR 落地或失败出队后再重排一次即可，不要连续重排。

---

Generated by Claude Code · merge-queue-triage workflow (#4859)