Skip to content

fix(platform-objects): reveal 2FA backup codes on the surface a user can actually reach - #11110

Merged
os-warren merged 5 commits into
mainfrom
claude/issue-10681-2fa-backup-code-result-dialog
Aug 22, 2026
Merged

fix(platform-objects): reveal 2FA backup codes on the surface a user can actually reach#11110
os-warren merged 5 commits into
mainfrom
claude/issue-10681-2fa-backup-code-result-dialog

Conversation

@os-warren

@os-warren os-warren commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes #10681

Gate union and test evidence below were derived and run on the final commit ed96626e2, clean tree.

The defect

sys_user.generate_backup_codes is mounted at Setup → People & Organization → Users (Security tab, via record:quick_actions { location: 'record_section' } in pages/sys-user.page.ts). It declared no resultDialog: it toasted "New backup codes generated — save them somewhere safe", issued the request, and dropped the response. The previous code set is invalidated wholesale the moment that request succeeds, so the reachable path was old codes destroyed, new codes discarded into a toast.

This card is publishable in full and the reproduction is stated plainly: the actor is the account owner, using their own password, destroying their own recovery codes. It is not an auth/authz bypass.

Re-derived rather than quoted from the card (which was a day old):

  • storeBackupCodes is not something ObjectStack sets. better-auth's twoFactor() plugin defaults it to 'encrypted' (dist/plugins/two-factor/index.mjs), and auth-manager.ts passes no backupCodeOptions — so encodeBackupCodes JSON-stringifies the codes and then symmetricEncrypts that string. The column holds one opaque ciphertext.
  • auth-route-ledger.ts publishes generate-backup-codes and no route that reads codes back. There is no re-reveal endpoint.
  • Gate on the endpoint is user.twoFactorEnabled, and its documented success shape is { status, backupCodes } — which is what the declared path has to match.

So the API response is the user's one and only sight of those codes.

The fix

generate_backup_codes now declares the one-shot reveal ({ path: 'backupCodes', format: 'code-list' }) and enable_two_factor the QR equivalent (totpURI as qrcode + backupCodes). Both copy the shapes sys_two_factor.enable_two_factor / regenerate_backup_codes already carried — deliberately not a third and fourth spelling.

The successMessage on each is removed, not kept: the runtime shows the dialog instead of the toast, so a message declared alongside a resultDialog is unreachable text that still ships to every translator — and here it was the very string that made the defect look handled.

Same-file rider from triage: sys_two_factor.backup_codes was described as 'JSON-serialized backup recovery codes'. It is JSON before encryption; what the column stores is the ciphertext above.

What I pinned for the render path, stated precisely

The card's decisive question is whether the codes reach the user on the path a user actually takes. That splits across two repos, so it is pinned in two places and neither is a DOM render check — the dialog DOM lives in the sibling objectui repo (ActionRunnerActionResultDialog), which no test in this repo can drive.

1. Reachability — packages/platform-objects/src/identity/two-factor-one-shot-reveal.test.ts. Walks the chain instead of asserting a key: Setup nav contribution → sys_user → page tree → record:quick_actions at record_section → the action declaration. Coverage is held over a derived set — every identity action targeting a route known to return an unrecoverable secret must reveal it — so a fifth 2FA surface added later is held to the same rule with no edit to the test. It also keeps the card's other finding live by asserting sys_two_factor is still mounted nowhere; if that flips, the duplication becomes a question someone must answer.

2. The declaration-to-response join — packages/qa/dogfood/test/two-factor-backup-code-reveal.dogfood.test.ts. Boots a real stack, enrols, confirms the enrolment with a real TOTP, calls the real route, and resolves the real declared paths against the real response body. This is the fact a declaration test cannot reach: if the declared path stops matching better-auth's response shape, the dialog opens empty and the codes are lost just as thoroughly, while every shape assertion stays green. The "unrecoverable afterwards" leg runs its positive control first — the same matcher finds the codes in the response they came from — so its silence against the stored column is evidence of encryption rather than a broken matcher.

Render chain argued from source, held falsifiable, not executed here: record:quick_actions resolves actions through useActionEngine, which reuses the surrounding ActionProvider's runner (packages/react/src/hooks/useActionEngine.ts — its own comment names the failure mode, "its local runner has no handlers installed"), and RecordDetailView registers onResultDialog on the same provider that supplies the onToast the QA run observed firing. That is why the location difference from the sys_two_factor precedent (record_section vs list_toolbar) does not change the outcome. ⚠️ objectui has no test covering resultDialog at the quick-actions surface (packages/plugin-detail/src contains zero resultDialog references, and the shared-runner test does not pin handler inheritance) — filed as a finding, see below.

Proof

Ablation 1 — the fix (signature predicted in writing before running). Predicted: restoring sys-user.object.ts to origin/main fails exactly the two Coverage cases while all three Reachability cases and the route-table control stay green. Observed exactly that — Tests 2 failed | 4 passed (6), failing on SysUser.enable_two_factor … declares no resultDialog and the dead-successMessage case. That split is the discriminator: it shows the redness is the missing reveal, not a broken walker. Restore proved byte-identical (git hash-objectf54463fca02fb5a8173b4808881b7c184173382c) and re-run to a real verdict (6 passed), not trusted on the hash.

Resolution, argued and falsifiable: that pin imports only relative source paths; its sole package import is import type, erased at runtime. It therefore reads src, and the proof is that it went green on my edit without platform-objects ever being built, which is impossible if it resolved dist/.

Ablation 2 — the join. The dogfood test resolves through exportsdist/, so before reading any result I proved the fix reached the artifact: node scripts/ablation-dist-preflight.mjs @objectstack/platform-objects 'I have saved the new codes'marker present in 10 built files. Predicted: injecting a typo'd path fails one case with …resolves to nothing in the live response — the dialog would open EMPTY. The prediction was wrong on the count — that assertion fired as predicted, but three tests went red, because revealedCodes was assigned at the end of the first test and its abort cascaded. That is a real fragility, so the fixture extraction moved into beforeAll; each leg now stands on fixture state rather than on its predecessor having run. Re-verified green (4 passed) after the fix.

Gate union — derived on the final commit, no path arguments

node scripts/pm/dispatch-gates.mjs (exit 0), 15 path-derived families, all run, all EXIT=0, exit codes captured before any pipe:

check:changeset-gate-self-tests · check:objectui-changeset · check:slot-lookup · check:test-source-alias · check:type-source-resolution · check-adr-0087-registration · check-changeset-no-major · check-ci-filter-parity · check-empty-changeset · check-plugin-teardown-shape · check-affected-docs · spec-liveness family (check:empty-state, check:liveness, check:strictness-ledger, check:variant-docs).

Convention-triggered (the derivation named these as change-KIND, not path): check:i18n · check:i18n-coverage · check:engine-double-contract · check:where-matcher · check:query-options-erasure · check:type-check-coverage · check:type-check-debt --re-measure. Also ran check:nul-bytes plus a direct control-byte scan of every touched file.

⚠️ check:type-check-debt --re-measure first refused--re-measure cannot run: 1 workspace dependenc(ies) … no built type entry point on disk -- @objectstack/service-knowledge. That refusal is NOT MEASURED, not a pass, so I built that closure and re-ran to a real verdict: 33 ledger entr(ies) re-measured, 1908 raw tsc error(s) total, none above its recorded number.

Ratchets: nothing was baselined. check:where-matcher and check:query-options-erasure both report baseline key set verified against ab47f69: no files added.

Package suites: @objectstack/platform-objects 27 files / 432 tests passed; the new dogfood file 4/4; typecheck on both packages Scope: 2 of 79 workspace projects, exit 0 (script names verified to exist first, so a zero-match filter could not read as green).

i18n

The new reveal strings are extracted into the generated bundles. sys_two_factor's equivalents were already fully translated in zh-CN / ja-JP / es-ES, so leaving mine in English would have translated the unreachable copy and not the reachable one. The source strings are identical, so the approved translations were transplanted rather than invented; check:i18n and check:i18n-coverage are green (none new).

Housekeeping

My first git add -A swept in a zero-content file-mode change (100644100755) to packages/create-objectstack/bin/create-objectstack.js, produced by the local pnpm install. It is reverted and absent from the final diff — flagging it because #11075 is an open PR about exactly that exec bit, and it would have been a silent collision.

Deliberately not in scope

Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/platform-objects, @objectstack/dogfood, touching 8 documentable anchor(s).

26 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 b302aabc7caa7226e372728ef3b331a791635e72.

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

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 4 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 b302aabc7caa7226e372728ef3b331a791635e72packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9487a56dd74f49e746f5da1d392868430151d8c3 — the merge of head ed96626e2986bc2147001306eb6aa8e4c4e49f3a into base b302aabc7caa7226e372728ef3b331a791635e72, 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 9487a56dd74f49e746f5da1d392868430151d8c3 && git checkout 9487a56dd74f49e746f5da1d392868430151d8c3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b302aabc7caa7226e372728ef3b331a791635e72 ed96626e2986bc2147001306eb6aa8e4c4e49f3a && git checkout -B drift-repro b302aabc7caa7226e372728ef3b331a791635e72 && git merge --no-ff ed96626e2986bc2147001306eb6aa8e4c4e49f3a

node scripts/docs-audit/affected-docs.mjs --json b302aabc7caa7226e372728ef3b331a791635e72

⚠️ 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 b302aabc7caa7226e372728ef3b331a791635e72 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

Copy link
Copy Markdown
Contributor

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

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

失败的 job(日志抽取,best effort):

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

    ✗ Build failed in 4.66s
    

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

跨 PR 相同签名(24h,按失败测试文件聚合):

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

历史信号:

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

分诊清单:

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

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

Merged via the queue into main with commit 266654d Aug 22, 2026
36 checks passed
@os-warren
os-warren deleted the claude/issue-10681-2fa-backup-code-result-dialog branch August 22, 2026 18:31
@github-actions

Copy link
Copy Markdown
Contributor

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

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

失败的 job(日志抽取,best effort):

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

    ✗ Build failed in 4.68s
    

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

跨 PR 相同签名(24h,按失败测试文件聚合):

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

历史信号:

  • ⚠️ 本 PR 过去 24h 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 89 个失败构建(不含本次)。

分诊清单:

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2FA backup-code regeneration on the navigable surface is a lockout path (sys_user.generate_backup_codes has no resultDialog)

2 participants