Uh oh!
There was an error while loading. Please reload this page.
fix(auth): authorize before the break-glass guard on /admin/remove-user - #12029
Conversation
The break-glass last-local-credential guard is a global better-auth `hooks.before`, which runs ahead of an endpoint's own middleware. On /admin/remove-user — served directly by better-auth's router, whose adminMiddleware establishes only a session — the guard's lookup and its target-dependent refusal were reached by any authenticated caller before either authorization layer had run. /admin/ban-user already ran the same guard AFTER authorization, because #9652 shades that path with an ObjectStack raw mount whose platform-admin gate fires first. One guard, two routes, opposite orders, nothing asserting either. /admin/remove-user now carries the same shading, converging the /admin/* family on authorization before the guard. The mount reuses the landed #9652 / #9653 gate-then-delegate pattern; no new mechanism. It DELEGATES rather than re-implementing removal, so the request re-enters better-auth's router: the path-keyed hook still fires and the guard still decides, just after authorization. A platform admin is unaffected in every respect, including the vendor's own admission decision (#9969). An ordering pin ships with the fix: one authenticated non-admin naming two different targets must receive indistinguishable responses, and — so the pin cannot be satisfied by deleting the guard — an admitted platform admin must still hit the guard's refusal and still succeed on an ordinary user. Part of the /admin/* ordering convergence; dogfood bucket for the route reclassified to `shaded-vendor-gate` (ObjectStack gate answers the refusal, better-auth still owns admission). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 11 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 1f9fe8aa27b6ba0e33cf7676876f6e5af4f3ee99 && git checkout 1f9fe8aa27b6ba0e33cf7676876f6e5af4f3ee99
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f5a7f9c8840b5e5ed909ca8601ddcd160b9287e0 a2caffa63e9990fa42dd0e8456e00e906ceb6d2e && git checkout -B drift-repro f5a7f9c8840b5e5ed909ca8601ddcd160b9287e0 && git merge --no-ff a2caffa63e9990fa42dd0e8456e00e906ceb6d2e
node scripts/docs-audit/affected-docs.mjs --json f5a7f9c8840b5e5ed909ca8601ddcd160b9287e0 |
os-sam
commented
Aug 25, 2026
⛔ CI red on |
…remove-user The #10792 pin at erasure-transaction-authorization.test.ts asserted the vendor's YOU_ARE_NOT_ALLOWED_TO_DELETE_USERS for a signed-in plain member — a literal from the pre-#11477 surface, where better-auth's router served the route unshaded. Ruled option A on #11477 shades /admin/remove-user with the #9652 pattern (gateAdmin before the break-glass guard), so through the real mount chain the member now hears the ObjectStack gate's target-independent 403 PERMISSION_DENIED. The pin's intent is unchanged and its other legs are untouched: the member still hears an authorization refusal (403, never 401), the answer still arrives fast on the pool max=1 dialect, and a refusal still erases nothing. Only the code literal moves to the converged verdict. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UQgPSniH1GFM9ZDeGyuGUa
os-trump
commented
Aug 25, 2026
Patch round (takeover continuation, authorized by the PM interim comment on #11477, 2026-08-25). Head Which pin, and why the literal moved. The pin's member leg asserted the vendor's Intent legs preserved. The patch is one expectation flip plus its comment (now citing the #11477 ruling): the status leg (403, never 401), the timing leg (answers fast on the pool max=1 dialect), the nothing-erased leg (the member's target survives), and the anonymous / admitted-admin / control tests are all untouched. Head
Diff: Generated by Claude Code Generated by Claude Code |
os-sam
commented
Aug 25, 2026
The red on |
…min-remove-user-gate-ordering
…ion carve-out Main moved under the branch: #11767 made the platform's default audience posture invite_only, so the pin's bare-AuthManager fixture had every self-serve sign-up past the first refused with 403 SELF_REGISTRATION_CLOSED during staging — all 7 pin tests died before reaching their assertions. The fixture now seeds a pending invitation per staged user via inviteForAudienceGate, the house lane the neighbouring green fixtures already use (an `open` posture would force email verification on and stop sign-up from minting the bearers the authenticated legs need). No assertion changed; the pin still asserts exactly what it asserted. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UQgPSniH1GFM9ZDeGyuGUa
os-trump
commented
Aug 25, 2026
Patch round 2 — base moved under the PR; fixture staging repaired, no assertion changed.
Fix (commit Evidence at Draft state, Generated by Claude Code Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#11477
⭐ Clause-② — YES
Declared per the maintainer ruling of 2026-08-25, which classifies this card as a
security boundary where accept/reject distinguishability for authenticated callers
changes.
CONTRACT_REVIEW_TIER;needs:contract-reviewis hung on this PR as thesecond carrier alongside the card.
⛔ This PR stays draft. The label is not to be cleared, ready is not to be flipped and
auto-merge is not to be armed from this seat.
The defect
The break-glass last-local-credential guard is registered as a global better-auth
hooks.before, keyed onctx.path. Abeforehook runs ahead of the endpoint's ownuse: [...]middleware.On
/admin/remove-user— served directly by better-auth's own router, whoseadminMiddlewareestablishes only a session, with the role decision landing laterstill inside the vendor's handler — that ordering placed the guard's lookup, and its
refusal, ahead of both authorization layers. Because the guard's answer depends on the
target named by the request, the refusal itself carried a per-record fact about a user
the caller was never entitled to ask about.
/admin/ban-userran the very same guard after authorization, because #9652 shadesthat path with an ObjectStack raw mount whose
gateAdminfires first.⇒ One guard, two routes, opposite orders, and nothing asserting either. That is the
finding, and it is why the pin below is half the deliverable rather than a garnish.
Step 1 — execution-side evidence, taken before anything was changed
The ruling required this and it is not skippable: "the ordering on this card is read from
code, not executed — reproduce the actual sequence on both routes before changing
either." Hook-vs-middleware ordering is exactly the kind of claim a framework can honour
differently from how the source reads.
Measured on the installed better-auth 1.7.1, through the real mount chain (the plugin's
Hono app in front of a real
AuthManager— raw-mount-vs-vendor-router ordering does notexist inside
AuthManageralone, so a test driving it directly would be structurally blindto this defect). One authenticated non-admin caller, two different targets, per the card's
own equality-assertion discipline:
/admin/remove-user/admin/ban-userThe code-read claim on the card is therefore confirmed by execution, in both
directions: the guard really did answer a non-admin ahead of authorization on
/admin/remove-user, and really did not on/admin/ban-user.Asserting merely "it is refused" would have been satisfied by the defective build — the
defective build refuses too; it just refuses differently depending on the named target,
and that difference is the whole finding.
The fix — ruled option A, no new mechanism
/admin/remove-usergains the raw-mount shading/admin/ban-useralready has:gateAdminfirst, guard after. This converges the whole/admin/*family onauthorization before the break-glass guard and reuses the landed #9652 / #9653
gate-then-delegate pattern.
choice and it is what keeps the blast radius at zero:
/admin/routes refuse platform admins and have no ObjectStack consumer — decide whether they are capability or just vendor surface #9969 (closednot_planned) ruled that the consumer-less vendor routes — this oneincluded — are not re-implemented, and that their 403 to a platform admin is a
recorded, intended state. Re-implementing removal here would have quietly overturned
that ruling.
trap written up in
last-local-credential.ts, and the reasonban-userhas to re-runthe guard by hand. Re-dispatching through
handleRequestputs the request back insidebetter-auth's router, so the path-keyed hook still fires and the guard keeps working,
simply later.
What changes is when the guard decides, never what it decides:
401 UNAUTHENTICATED, unchanged;the guard is now unreachable before authorization;
decision (finding: seven better-auth
/admin/routes refuse platform admins and have no ObjectStack consumer — decide whether they are capability or just vendor surface #9969) and the guard's refusal when the target really is the last localpassword login.
The ordering pin
packages/plugins/plugin-auth/src/admin-remove-user-gate-ordering.test.ts— a new pin sothe sequence is mechanically checkable instead of re-argued.
It asserts indistinguishability (one authenticated non-admin, two targets, byte-identical
responses) and then names which layer spoke, so a build that answered the guard's verdict
for both targets — indistinguishable, and strictly worse — cannot pass.
⛔ It also carries the load-bearing half, because an implementation that "fixed" the
ordering by deleting the guard would satisfy every indistinguishability assertion: an
admitted platform admin must still hit the guard's refusal, and the same admin must still
succeed on an ordinary user, so the still-refused leg cannot be satisfied by refusing
everyone.
#9652 coupling — checked, and pinned as a control
The card and the ruling both record that this card and #9652's raw mounts move each
other's ordering bidirectionally, so #9652 was re-read before the mounts were touched.
/admin/ban-user's sequence is unchanged — it is not edited, and the pin includes itas an explicit control (non-admin gets one authorization verdict for both targets; the
hand-rolled guard call site still refuses an admin removing the last local credential). If
the convergence had been bought by moving the reference implementation, that control goes
red. It does not.
Dogfood bucket reclassification (fixture triage, not a loosened assertion)
/admin/remove-usermoves out of thebetter-auth-gatebucket, because its two halves arenow owned by different layers: ObjectStack's gate answers the refusal, better-auth still
owns admission (#9969). It goes into a new single-member
shaded-vendor-gatebucketthat asserts both.
⛔ It was not forced into either neighbouring bucket. In
objectstack-gatetheadmin-is-not-refused assertion would be red (the vendor still refuses); in
better-auth-gatethe member's code assertion would be red — and that file carries anexplicit ⛔ against widening its denial vocabulary. Reclassifying the route that stopped
matching is the opposite of loosening the rule that no longer described it. The new bucket
keeps a real both-sides contrast: the member and the admin hear two different refusals,
which is what proves the member's refusal is an authorization verdict rather than a blanket
one — the job the 2xx does in the neighbouring bucket.
Ablation
Predicted in writing before running, including the pin: removing the shading must turn
the ordering pin red, and the anonymous / admin-invariant /
ban-user-control legs muststay green.
Measured: exactly the two predicted assertions failed and nothing else, with the asymmetry
itself as the failure signature — the same non-admin caller drew two different refusals for
two different targets. Every other leg held.
The mutation was proven on disk before any colour was read (anchored
grep -con thetext intended to change, 1 → 0 — an editor's exit code is not evidence), and the script
carries a
trap … EXIT INT TERMrestore so a foreground-cap kill can never leave the treemutated. Restore was verified three ways:
git status --porcelainempty and disk hash ==index hash == HEAD hash for the ablated file, then the pin re-run green on the restored
tree.
No rebuild leg was required or claimed: the pin imports the subject by relative source
path, so vitest compiles from
src/and nodist/is in the resolution path.Verification — all at
9e41285ee2Every exit code captured before any pipe (
cmd > file 2>&1; EXIT=$?), never through atail. Gate list derived, not recalled:node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(it read the changeset off the merge base itself — 6 committed paths, three-dot semantics).
pnpm --filter @objectstack/plugin-auth test→Test Files 75 passed (75) / Tests 1536 passed (1536)pnpm --filter @objectstack/plugin-auth typecheck→ exit 0, 0 error linesTest Files 1 passed (1) / Tests 17 passed (17)check-auth-mount-ledger: OK -- 18 ObjectStack auth mount(s), all accounted for (13 by a reviewed ledger row, 5 shadowing a vendor-declared path, 0 pending a disposition)(thenew mount is accounted for as a shadow, so no ledger row is owed — the route is already
in
BETTER_AUTH_MOUNTED_SURFACEand stays there, because the vendor still serves it);plus
check:route-envelope, and the ratchet family —check:slot-lookup("ratchetholds: 107 unswept site(s) in 25 file(s), none new"),
check:query-options-erasure("ratchet holds … none new"),
check:where-matcher("297 matcher(s) discovered"),check:engine-double-contract("OK — 405 pinned"),check:type-check-coverage,check:nul-bytes("scanned 6665 text file(s) … no raw ASCII control bytes").Declared narrowings (two, both stated rather than skipped):
pnpm check:type-check-debt --re-measure— needs the whole workspace closure built; CIruns it on every PR regardless. Same narrowing fix(plugin-auth): serve /admin/ban-user and /admin/unban-user with the ADR-0068 platform-admin gate #9970 declared on this package.
@objectstack/dogfoodsuites are not run here — they boot a real app. Itstypecheckfails on an unbuilt worktree, so that was measured as a baseline deltarather than left ambiguous: the two edited fixtures were swapped for their pre-change
blobs and re-run. 247 → 247 total errors, 25 → 25 in those two files: delta 0 in both
directions. Every error is a
TS2307unresolvable-workspace-import and its inferencecascade, present identically without my changes.
Generated by Claude Code
Generated by Claude Code