fix(plugin-auth): 2FA re-enrollment must not inherit verified from the previous enrollment

[os-warren]

Part of #10700 — the integrity half is discharged here; one design question is left open below, so this deliberately does not carry a closing keyword.

⚠️Draft, and it stays draft. Clause ② applies: this changes the accept/reject behaviour of a published auth endpoint. needs:contract-review hangs on the card as the compensating control; this seat does not clear it, does not mark ready, and does not arm auto-merge.

🔒 Auth/authz disclosure carve-out. Mechanism and verdict only — no reproduction recipe here, on the card, or in any commit message. The withheld detail lives in QA session #10663.

What was wrong

better-auth's /two-factor/enable handler computes the row it is about to write as

verified: existingTwoFactor!=null&&existingTwoFactor.verified===true||!!options?.skipVerificationOnEnable

and updates that onto the existing row when one is there (dist/plugins/two-factor/index.mjs, measured against the installed better-auth@1.7.1 on this branch). sys_two_factor declares user_id unique, so that is an in-place rewrite of the one row the account has: the secret is brand new, the flag is inherited from the enrollment before it. verified stopped describing the secret stored beside it.

Why the fix is one flag and not a new gate

The vendor already gates the sign-in challenge on this flag, in both places that matter — totp/index.mjs refuses an unconfirmed factor with TOTP_NOT_ENABLED before any lockout bookkeeping, and the post-sign-in hook offers totp in twoFactorMethods only when the flag is not false. That gate is exactly what makes a first enrollment inert until it is confirmed. Re-enrollment was the one path that slipped past it, and not because the gate was missing — because the value handed to the gate was inherited.

So this restores the flag rather than adding a second owner of the same decision (AGENTS.md · Route & surface ownership #1). After a successful method: 'totp' enable, verified is set to false; the secret the endpoint just handed out becomes live only once the caller proves possession of it through /two-factor/verify-totp.

Both strict === false comparisons hold here because verified is declared type: 'boolean' in the plugin's own schema and this repo's adapter runs supportsBooleans: false, so better-auth's factory converts the stored 0/1 back to a real boolean on read. That is measured, not assumed — reading the engine table directly returns 0 where the gate sees false, and the tests assert both spellings for that reason.

skipVerificationOnEnable is honoured rather than overridden: under it, verified: true beside a fresh secret is the operator's declared intent. AuthManager never sets it.

Tightening only

Same request body, same response shape, same status. Nothing widens. A first-time enrollment is untouched (better-auth already wrote false there), and a rotation is still reachable and still completes — it now takes the same confirmation step a first enrollment takes.

⚠️ What this does NOT do — the open design question

The card asks for two things. This closes the first and does not close the second, and the PR says so rather than implying otherwise:

• ✅ Integrity — verified describes the stored secret at every point in the flow.
• ❌ "The old secret goes dead only when the new one is live and confirmed."enable rewrites the account's single row unconditionally, so the previously confirmed secret stops working when the call returns. That is true before this change and after it.

What does change on the availability axis is where the caller finds out: with a live session in hand, at the confirmation step, instead of at the next sign-in with none. And the floor that survives the window is pinned — the backup codes the same enable response issues still complete a sign-in.

Making the strict property true needs somewhere to park an unconfirmed secret, and sys_two_factor has no room for one (user_id is unique). Every route to it is a call this lane may not make alone: staging the pending secret elsewhere widens what /two-factor/verify-totp accepts (a code from a secret that is not on the row), and adding a column adds persisted state to a managedBy: 'better-auth', lock: 'full' table. The remaining alternative — refusing enable while already enabled — is the card's remedy 2, which triage ruled needs a maintainer decision. Reviewer's call, not this seat's.

Out of scope: #10681 (generate_backup_codes re-provisioning, same family, deliberately held back). Nothing here touches it.

Evidence

Ablation — signature predicted in writing before mutating, then observed:

	predicted	observed
totals	2 failed | 12 passed (14)	2 failed | 12 passed (14)
fail 1	verified must describe the secret stored beside it, not the enrollment before it: expected true to be false	identical
fail 2	expected { status: 401, code: 'INVALID_CODE' } to deeply equal { status: 400, code: 'TOTP_NOT_ENABLED' }	identical
pass 1	first enrollment still completes	passed
pass 2	rotation still completes (still-works leg, expected green in both arms)	passed

Mutation was the single wiring line in the after-hook. No build sits between the edit and the run: packages/plugins/plugin-auth has no dist/ directory at all and its vitest.config.ts declares no alias, and the suite imports ./auth-manager relatively — so the file edited is the file executed, and the verdict flipped with no build step. Restore proved byte-identical: git hash-object reads ce4e2711ed503b8aa608bee7a146e0aa222b4565 before the mutation and after the restore. Restore leg re-run: 14 passed (14).

Suites (b3c9ea3db): pnpm --filter @objectstack/plugin-auth test → Test Files 68 passed (68) · Tests 1414 passed (1414). pnpm --filter @objectstack/plugin-auth typecheck → exit 0.

Gate union derived on this final commit with a clean tree, node scripts/pm/dispatch-gates.mjs with no path arguments (6 path(s) → 26 matched families + 5 convention-triggered). All green, exit codes captured before any pipe. Class #10309: check:route-envelope, check:dispatcher-error-vocabulary and check:error-code-casing were run explicitly and the derivation named none of the three.

check:type-check-debt --re-measure (built closure, 33 ledger entr(ies) re-measured in 332.6s, 1908 raw tsc error(s) total, none above its recorded number) reports plugin-auth at TEST_DEBT records 109, tsc now reports 97 (-12) — a pre-existing surplus, already tracked by the gate's own reference to #6376, not lowered here because it is unrelated to this diff.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 8 documentable anchor(s).

23 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 7ab286e44aa6626e91e87f9d7d4a7ce881fec34a.

⛔ 5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

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 7ab286e44aa6626e91e87f9d7d4a7ce881fec34a → packageMentionDocs.

Which tree this was computed on

This run read content/docs from d75727a28cfbf824d894f27cabc39d0fd646f85c — the merge of head b3c9ea3dbdcd1b0ba89b55372e3df692bc465a9a into base 7ab286e44aa6626e91e87f9d7d4a7ce881fec34a, 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 d75727a28cfbf824d894f27cabc39d0fd646f85c && git checkout d75727a28cfbf824d894f27cabc39d0fd646f85c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7ab286e44aa6626e91e87f9d7d4a7ce881fec34a b3c9ea3dbdcd1b0ba89b55372e3df692bc465a9a && git checkout -B drift-repro 7ab286e44aa6626e91e87f9d7d4a7ce881fec34a && git merge --no-ff b3c9ea3dbdcd1b0ba89b55372e3df692bc465a9a
node scripts/docs-audit/affected-docs.mjs --json 7ab286e44aa6626e91e87f9d7d4a7ce881fec34a

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 7ab286e44aa6626e91e87f9d7d4a7ce881fec34a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[github-actions]

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

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

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

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

  ✗ Build failed in 5.99s
  

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

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

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

历史信号：

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

分诊清单：

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

---

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