Skip to content

fix(plugin-auth): run the break-glass last-local-credential guard after authentication - #11038

Merged
os-warren merged 3 commits into
mainfrom
claude/issue-10776-break-glass-guard-after-auth
Aug 22, 2026
Merged

fix(plugin-auth): run the break-glass last-local-credential guard after authentication#11038
os-warren merged 3 commits into
mainfrom
claude/issue-10776-break-glass-guard-after-auth

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#10776

What was wrong

The break-glass last-local-credential guard was registered as a better-auth
hooks.before. A before hook runs ahead of the endpoint's own
use: [adminMiddleware], and that middleware is the only layer establishing
identity on the vendor /admin/ lane. The guard therefore evaluated — and
answered — a per-record question for a caller whose identity had never been
established, while every neighbouring route on the same lane answers with the
ordinary authentication refusal. The guard's refusal is distinctive, so the
refusal itself was the answer.

Mechanism only, per the card. No request shapes, seeding steps or values appear
in this PR, in its commits, or in the card.

What changed

packages/plugins/plugin-auth/src/auth-manager.ts resolves the acting user
before the guard and runs the guard only for a caller who has an identity. An
unauthenticated caller falls through to the vendor's own session middleware and
receives the ordinary refusal — the same shape as every neighbour. This is the
pattern the /oauth2/authorize gate a few lines above already uses.

This moves when the guard decides, never what it decides. An
authenticated caller reaches the same lookup and the same CONFLICT.

Maintainer ruling, 2026-08-22 decision-inbox digest, accepted verbatim
「接受所有」: option A — authentication before the guard. Option B (keep the
guard early and disguise the pre-auth answer) was the fallback and is not taken;
nothing in the measurement establishes that the guard must run first. Option C
is not taken.

The guard's other production call site is the raw /admin/ban-user mount in
auth-plugin.ts, which already runs gateAdmin(c) before invoking
runAdminBanUser. It was already authentication-first and is unchanged.

Execution order

Triage made the order part of the ruling, so the hermetic cases landed first,
in their own commit (08ac2e7), and were measured on the pre-fix tree before
anything was changed. The fix is the second commit (07f1a57).

Measured verdicts — code AND status (ADR-0112)

All through AuthManager.handleRequest on the installed better-auth 1.7.1.

legbeforeafter
anonymous, names the break-glass holder, /admin/remove-user409 LAST_LOCAL_CREDENTIAL401 UNAUTHENTICATED
anonymous, names an ordinary user, /admin/remove-user401 UNAUTHENTICATED401 UNAUTHENTICATED
anonymous, names a user id nobody holds401 UNAUTHENTICATED401 UNAUTHENTICATED
authenticated admin, genuine last local credential409 LAST_LOCAL_CREDENTIAL409 LAST_LOCAL_CREDENTIAL
authenticated admin, ordinary user (admission)200200
anonymous, /delete-user, break-glass holder409 LAST_LOCAL_CREDENTIAL401 UNAUTHORIZED
anonymous, /delete-user, ordinary user401 UNAUTHORIZED401 UNAUTHORIZED
authenticated holder, /delete-user (self-service)409 LAST_LOCAL_CREDENTIAL409 LAST_LOCAL_CREDENTIAL

The card's second arm was an unmeasured reading and is now measured: it
already landed on 401.
The disclosure is therefore exactly as wide as the card
states, not wider — the severity does not move.

The self-service path's outcome is unchanged; only its timing moved. For an
authenticated holder it is still the same conflict. For an anonymous caller it
is now the vendor's own flat 401, which is correct: /delete-user is not an
/admin/ path, so #10349's envelope normalizer deliberately leaves it alone.

Two legs are load-bearing and exist so the disclosure fix cannot be satisfied
vacuously: an implementation that simply deleted the guard would score green on
every disclosure assertion, and fails the still-refused leg; one that refused
every caller would pass the still-refused leg, and fails the admission leg.

Proof

Ablation, signature predicted in writing before mutating. Mutation: restore
the pre-fix ordering in auth-manager.ts, both test files untouched.

  • Predicted: exactly 5 failures across exactly 2 files — arm 1, the
    indistinguishability pin, and the two anonymous /delete-user pins, each
    expected 409 to be 401; plus the unit-level "does not run at all for an
    unauthenticated caller" case rejecting instead of resolving. The legs that
    were green pre-fix predicted to stay green.
  • Observed: exactly that. Tests 5 failed | 22 passed (27), in
    break-glass-guard-authentication-order.test.ts (4) and
    break-glass-local-credential.test.ts (1), with the predicted messages.
  • Restore:git hash-object before 2e63bcada18a628de4e120572a65364122df7b61,
    after 2e63bcada18a628de4e120572a65364122df7b61 — byte-identical. The restore
    leg was re-run to a real verdict: Tests 27 passed (27).

src vs dist resolution, argued from the files (re-measured on this tree
rather than inherited): packages/plugins/plugin-auth had no dist/ at all
at ablation time, even after a full dependency-closure build; vitest.config.ts
declares no resolve/alias key and there is no root vitest workspace file;
and both test files sit inside src/ and import the subject by relative
specifier
, which bypasses package.json#exports entirely. So the subject
under test is src/auth-manager.ts and the mutation took effect without a
rebuild. The KNOWN_UNALIASED_TEST_IMPORTS entry for this package concerns its
dependencies, which do resolve through their own dist/ — that is why the
closure build was required, and it does not apply to the subject.

Zero-hit counter-check, positive control run first. The control: searching
isLastLocalCredentialHolder( and LAST_LOCAL_CREDENTIAL over
packages apps examples names the expected files, so the instrument speaks on
this corpus. The silence: the same instrument, excluding the defining module,
the two known call sites and test files, returns nothing — the guard has exactly
the two production call sites already known, and the other one is
authentication-first already.

Gates

Union derived on the final commit f3a8f0022 with a clean tree, via
node scripts/pm/dispatch-gates.mjs with no path arguments (it took the
change set from the merge base itself). Every exit code captured before any
pipe; every verdict below is the gate's own printed line, not a bare $?.

Derived (11): 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 — all green.

Convention-triggered by the new test file (5): check:query-options-erasure,
check:type-check-coverage, check:type-check-debt,
check:engine-double-contract, check:where-matcher — all green. The ratchet
ran on a built workspace closure and reached a real verdict, not a refusal:
check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 252.4s, 1908 raw tsc error(s) total, none above its recorded number.

Class #10309, run explicitly: the derivation named none of
check:route-envelope, check:dispatcher-error-vocabulary or
check:error-code-casing this time either
— the same silence reported on the
last three plugin-auth PRs. Run by hand, all three green:
✓ check-route-envelope self-test passed;
check-dispatcher-error-vocabulary: OK — 21 unregistered code-stamping site(s), all classified;
✓ no unlisted lowercase error codes in 4429 scanned file(s) (ADR-0112).

Also check:nul-bytes: OK (scanned 6391 text file(s) … no raw ASCII control bytes), plus a direct control-byte scan over the four changed files.

Package suite and typecheck on f3a8f0022:
Test Files 69 passed (69), Tests 1440 passed (1440); pnpm typecheck exit 0
for both of its tsc invocations.

Posture

Clause-② is yes — anonymous-caller accept/reject behaviour changes. This PR
stays draft, needs:contract-review stays on the card, and this seat does
not clear it, mark it ready, arm auto-merge or merge.

Scope

No packages/spec edit was needed; no content/docs/releases/** touched.
Changeset included. No files belonging to the two open sibling PRs in this
package were touched.


Generated by Claude Code

…t the real seam
Lands the hermetic cases before the fix, per the triage execution order on
#10776: both anonymous arms, the still-refused leg for an authenticated
admin, the admission direction that keeps it non-vacuous, and the
self-service path whose timing moves with the guard.
Measured on this tree (pre-fix), through AuthManager.handleRequest on the
installed better-auth 1.7.1:
- the anonymous arm naming the break-glass holder answers 409
LAST_LOCAL_CREDENTIAL, and the same request naming an ordinary user
answers 401 UNAUTHENTICATED -- the card's second arm was a reading, and
the reading holds
- an authenticated admin still gets 409 for the genuine last credential
and 200 for an ordinary user
- the self-service path shows the same split
Part of #10776
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
… before it
The last-local-credential guard is a better-auth `hooks.before`, which runs
ahead of the endpoint's own `use: [adminMiddleware]` -- the only layer that
establishes identity on that lane. It therefore read the request body, asked
the database a question about the named user, and answered it, for a caller
nobody had authenticated. Its refusal is distinctive, and every sibling route
on the same lane answers 401, so the refusal itself was a per-record answer.
Resolve the acting user first and run the guard only for a caller who has an
identity; an unauthenticated caller falls through to the vendor's own session
middleware and gets the ordinary refusal, the same shape as every neighbour.
Same pattern the /oauth2/authorize gate above already uses.
This changes WHEN the guard decides, not WHAT it decides: an authenticated
caller reaches the same lookup and the same CONFLICT. The still-refused leg
and the admission direction are both pinned so that closing the disclosure by
deleting the guard cannot pass.
Maintainer ruling 2026-08-22, decision-inbox digest, accepted verbatim
「接受所有」: option A, authentication before the guard.
Part of #10776
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 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 2866d5f97e9877a438c77ab9b2435a5043ebc0d2packageMentionDocs.

Which tree this was computed on

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    ✗ Build failed in 5.96s
    

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

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

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

历史信号:

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

分诊清单:

  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 03bdd14Aug 22, 2026
32 checks passed
@os-warren
os-warren deleted the claude/issue-10776-break-glass-guard-after-auth branch August 22, 2026 15:46
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-warren@claude