fix(platform-objects): reveal 2FA backup codes on the surface a user can actually reach - #11110
Conversation
📓 Docs Drift CheckThis PR changes 2 package(s): 26 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 4 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 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
|
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32589803326 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32590210394 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
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_codesis mounted at Setup → People & Organization → Users (Security tab, viarecord:quick_actions { location: 'record_section' }inpages/sys-user.page.ts). It declared noresultDialog: 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):
storeBackupCodesis not something ObjectStack sets. better-auth'stwoFactor()plugin defaults it to'encrypted'(dist/plugins/two-factor/index.mjs), andauth-manager.tspasses nobackupCodeOptions— soencodeBackupCodesJSON-stringifies the codes and thensymmetricEncrypts that string. The column holds one opaque ciphertext.auth-route-ledger.tspublishesgenerate-backup-codesand no route that reads codes back. There is no re-reveal endpoint.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_codesnow declares the one-shot reveal ({ path: 'backupCodes', format: 'code-list' }) andenable_two_factorthe QR equivalent (totpURIasqrcode+backupCodes). Both copy the shapessys_two_factor.enable_two_factor/regenerate_backup_codesalready carried — deliberately not a third and fourth spelling.The
successMessageon each is removed, not kept: the runtime shows the dialog instead of the toast, so a message declared alongside aresultDialogis 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_codeswas 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
objectuirepo (ActionRunner→ActionResultDialog), 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_actionsatrecord_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 assertingsys_two_factoris 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:⚠️ objectui has no test covering
record:quick_actionsresolves actions throughuseActionEngine, which reuses the surroundingActionProvider's runner (packages/react/src/hooks/useActionEngine.ts— its own comment names the failure mode, "its local runner has no handlers installed"), andRecordDetailViewregistersonResultDialogon the same provider that supplies theonToastthe QA run observed firing. That is why the location difference from thesys_two_factorprecedent (record_sectionvslist_toolbar) does not change the outcome.resultDialogat the quick-actions surface (packages/plugin-detail/srccontains zeroresultDialogreferences, 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.tstoorigin/mainfails 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 onSysUser.enable_two_factor … declares no resultDialogand the dead-successMessagecase. That split is the discriminator: it shows the redness is the missing reveal, not a broken walker. Restore proved byte-identical (git hash-object→f54463fca02fb5a8173b4808881b7c184173382c) 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 readssrc, and the proof is that it went green on my edit withoutplatform-objectsever being built, which is impossible if it resolveddist/.Ablation 2 — the join. The dogfood test resolves through
exports→dist/, 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, becauserevealedCodeswas assigned at the end of the first test and its abort cascaded. That is a real fragility, so the fixture extraction moved intobeforeAll; 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, allEXIT=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 rancheck:nul-bytesplus a direct control-byte scan of every touched file.check:type-check-debt --re-measurefirst 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-matcherandcheck:query-options-erasureboth reportbaseline key set verified against ab47f69: no files added.Package suites:
@objectstack/platform-objects27 files / 432 tests passed; the new dogfood file 4/4;typecheckon both packagesScope: 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:i18nandcheck:i18n-coverageare green (none new).Housekeeping
My first
git add -Aswept in a zero-content file-mode change (100644→100755) topackages/create-objectstack/bin/create-objectstack.js, produced by the localpnpm 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
sys_two_factorinto navigation is a larger product-surface choice — raised, not taken. See the report's open question.verified=1carries over) is a separate, security-sensitive card and nothing here touches it. Correction to an earlier revision of this body: it is not still open — it was closed as completed on 2026-08-22, before this PR was raised.Generated by Claude Code