Uh oh!
There was an error while loading. Please reload this page.
security(plugin-auth): gate direct /sso/register on platform admin only - #10390
Conversation
The ADR-0024 before-hook on better-auth's own POST /sso/register admitted a platform admin OR an owner/admin of the caller's active org, while the four /admin/sso/* bridges have gated on the platform-admin judge since #9653. One principal therefore got two answers for the same underlying registration -- 403 at the bridge, admitted at the direct endpoint -- which made the bridge tightening honest labelling rather than a boundary. Per the 2026-08-20 maintainer ruling on #10009, ADR-0068 D4 governs: registering an identity provider is a platform-operator action. The admit set here is now platform-admin-only. - `isPlatformAdminUserId` asks the ADR-0068 D2 permission-set question on its own (org-less admin_full_access link), through `withSystemReadContext`, fail closed. It deliberately does not consult the retired `role` scalar. - `isOrgOrPlatformAdmin` lost its only production caller with this change and is removed rather than parked; the one grade ladder it delegated to (`isOrgAdminGrade`) keeps its own pins in member-role-canonical.test.ts and its other reader in last-admin-guard.ts. - The direct surface gains its first test pins, two-directional on purpose: the org owner is refused (403 + SSO_REGISTER_FORBIDDEN, code AND status) and the platform admin is still admitted. The platform admin is granted the ADR-0068 way and the fixture asserts the legacy role scalar is NOT 'admin', so the suite cannot pass by riding the retired channel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
…o-register-platform-admin-only
📓 Docs Drift CheckThis PR changes 1 package(s): 14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also name something this change touched. These are read-only:
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 f51c802c0127fa3147079122ff4b30a5733ffae7 && git checkout f51c802c0127fa3147079122ff4b30a5733ffae7
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dd8172ee223e14e3104191356fff10aa3f8abe33 c231374d9d003ebab37c101b514b920e704e8a2e && git checkout -B drift-repro dd8172ee223e14e3104191356fff10aa3f8abe33 && git merge --no-ff c231374d9d003ebab37c101b514b920e704e8a2e
node scripts/docs-audit/affected-docs.mjs --json dd8172ee223e14e3104191356fff10aa3f8abe33
|
os-warren
commented
Aug 20, 2026
⛔ 合并前置条件(PM 记录,非观察项)本 PR 已通过 PM 内容验收(见 #10009),但保持 draft、不武装 auto-merge —— Clause-② auth 接受集收窄,按维护者 2026-08-20 授权走「opus 开发 + 分诊审核」。 我把 PR 正文里的 "Serial note" 从注记升格为放行条件。本车道有过一次教训:一条被记成「残留而非返工」的余量说明,一小时内爆掉并把仓库合并队列堵了 3.5 小时。所以这条写成条件。 条件本身#10352(#9968 的 impersonate 半边)与本 PR 都在 后落地的一侧会落到两种结果之一,两种都很响:
另有一处不会变红、因而必须靠这条记录兜住的:#10352 的 docblock 写着 "Deliberately NARROWER than 指定顺序理由不是偏好:这个顺序下,两件事在同一次冲突解决里一起收掉 —— 删掉重复的
Generated by Claude Code |
…o-register-platform-admin-only # Conflicts: # packages/plugins/plugin-auth/src/auth-manager.ts
…o-register-platform-admin-only
os-warren
commented
Aug 21, 2026
Merge reconciliation with #10352 — measured, at |
| identifier | branch be985a623 | origin/main | merged c231374d9 |
|---|---|---|---|
isPlatformAdminUserId | 2 | 2 | 3 |
isOrgOrPlatformAdmin | 1 | 3 | 1 |
isOrgAdminGrade | 0 | 3 | 0 |
isPlatformAdminUserId = 3 occurrences but exactly ONE definition — grep -c 'private async isPlatformAdminUserId' = 1 (line 4387). The other two are call sites: line 1552 (this PR's /sso/register hook) and line 2625 (#10352's /admin/impersonate-user rewire). Both sides declared byte-identical bodies; one was dropped.
The surviving docblock is this branch's, which is why the {@link} is gone rather than stripped — the branch had already authored the replacement sentence:
Deliberately does NOT admit organization owners/admins. Platform-admin routes must not be reachable by whoever happens to own an org (ADR-0068). [#10009] This replaced an
isOrgOrPlatformAdminpredicate that admitted both; once/sso/registerstopped asking the org question, that wider predicate had no caller left and was removed rather than parked.
#10352's competing sentence ("Deliberately NARROWER than {@link isOrgOrPlatformAdmin}") is the one that lost, so no dangling link survives.
{@link} proof, with a positive control
grep -rn "{@link isOrgOrPlatformAdmin" src/ -> exit 1, ZERO hits
grep -rn "isPlatformAdminUserId" src/ -> exit 0, 6 hits (control: search works)
isOrgOrPlatformAdmin still appears 3 times in the package, all of it prose — last-admin-guard.ts:96, auth-manager.ts:4375, auth-manager.test.ts:3946. Each is a backticked historical mention that explicitly says the predicate is gone; none is a {@link} and none is a code reference. Deliberately left standing.
isOrgAdminGrade
Its auth-manager.ts import stayed dropped: on main all 3 occurrences lived inside isOrgOrPlatformAdmin's own docblock and body, so removing that method left no reader in this file. #10352 does not read it. The symbol is alive elsewhere in the package (invitation-role-cap.ts exports it, last-admin-guard.ts:281,827 imports and calls it, member-role-canonical.test.ts pins it) — unaffected.
Package verification on the merged tree
EXIT=0 pnpm --filter @objectstack/plugin-auth typecheck (tsc --noEmit)
EXIT=0 pnpm --filter @objectstack/plugin-auth test 62 files / 1347 tests passed
61/1323 → 62/1347: #10352 added admin-impersonate-endpoint.test.ts and modified four more plugin-auth test files. Delta accounted for, not assumed.
Gates — union derived by node scripts/pm/dispatch-gates.mjs with no path arguments, after the final commit, clean tree
EXIT=0 pnpm check:changeset-gate-self-tests EXIT=0 node scripts/check-adr-0087-registration.mjs
EXIT=0 pnpm check:objectui-changeset EXIT=0 node scripts/check-changeset-no-major.mjs
EXIT=0 pnpm check:slot-lookup EXIT=0 node scripts/check-empty-changeset.mjs
EXIT=0 pnpm check:test-source-alias EXIT=0 node scripts/docs-audit/check-affected-docs.mjs
EXIT=0 pnpm check:type-source-resolution
EXIT=0 pnpm check:query-options-erasure EXIT=0 pnpm check:engine-double-contract
EXIT=0 pnpm check:where-matcher EXIT=0 pnpm check:type-check-coverage
Class #10309 confirmed: the derivation did NOT name check:route-envelope or check:dispatcher-error-vocabulary. Both bite plugin-auth and #10352 moved both populations. Run explicitly:
EXIT=0 pnpm check:route-envelope EXIT=0 pnpm check:route-envelope --self-test
EXIT=0 pnpm check:dispatcher-error-vocabulary EXIT=0 pnpm check:dispatcher-error-vocabulary --self-test
✓ Plugin-mounted Hono routes — 12 module(s) audited, 166 hand-built body/bodies … 8 conformant, 0 ratcheted, 3 exempt, 1 vendor-wirecheck-dispatcher-error-vocabulary: OK — 21 unregistered code-stamping site(s), all classified; 1 awaiting a ledger entry (#8846).check-engine-double-contract: OK — 371 pinned, 133 in the DEBT ledger, 2 exempt.(was 338 pre-merge;mainmoved it)✓ where-matcher conformance holds: 271 matcher(s) discovered, 271 answer the combinator battery correctly or refuse it loudly (162 refuse).check-type-check-coverage: OK — 64/77 workspace packages type-checked … 13 in the DEBT ledger, 1 exempt.
Declared narrowing — check:type-check-debt --re-measure was NOT run
It needs the whole workspace closure built, and that build runs under the container-shared verification lock, which was held continuously by sibling agents (queue 3 deep). Attempted anyway: it did not refuse on the prerequisite this time, it entered the per-ledger-entry tsc sweep and was still running at a 420s cap, so it is NOT MEASURED — not a pass, not a fail. In its place tsc --noEmit for this package is clean at exit 0. CI runs the ratchet regardless.
@objectstack/plugin-auth TEST_DEBT is untouched at 109 (card #10615). No --lower was run and no ledger number was edited.
Still draft; auto-merge not armed; nothing merged.
Generated by Claude Code
Generated by Claude Code
os-warren
commented
Aug 21, 2026
PM 授权:翻 ready + 武装 auto-merge(2026-08-21T08:59Z)Clause-② 的暂扣已按流程解除,不是我自己解的:分诊席(非派发席)于 收尾按内容核对(不采信报告自述)在
那条悬空 两条我写错、被 dev 当场推翻的前提 —— 记录在案
一条显式声明的收窄
class #10309 —— 第六次坐实
正文的陈旧处,已用评论更正而非重写PR 正文 Verification 段仍引用合并前的 Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#10009
An organization owner or admin who is not a platform admin can no longer register an SSO identity provider. They previously passed the ADR-0024 before-hook on the direct
POST /api/v1/auth/sso/registerand now receive403 SSO_REGISTER_FORBIDDEN.403 SSO_REGISTER_FORBIDDENadmin_full_access)401(vendorsessionMiddleware)401(unchanged)No other surface changes: the
/admin/sso/*bridges already refused this principal since #9653, and every in-repo product path reaches SSO registration through those bridges (measured below).⛔ Not to be auto-merged. Clause-② (accept-set narrowing on an auth surface) — developed at
claude-opus-5under the maintainer's 2026-08-20 authorization while fable is exhausted, and waiting on triage review before landing.The ruling
Maintainer ruling 2026-08-20 (issue comment 5353918178) chose option (a): tighten the direct hook to platform-admin-only, aligning it with the #9653 posture on the bridges. ADR-0068 D4 — registering an identity provider is a platform-operator action. Options (b) (split by provider scope) and (c) (document the divergence) were rejected; if multi-org IdP self-serve ever becomes a product goal, (b) is a deliberate future ruling, not something inherited from the vendor default.
Premise, re-established before changing anything
Measured on one tree, one run, on this branch before the fix — the same org-owner principal at both doors:
providerId: 'credential'is permanently reserved by@better-auth/sso, and the reserved-id refusal sits after the vendor's whole authorization prologue and before any discovery fetch — so422 /reserved/proves admission with no network. The divergence #10009 describes is real.The shared judge did not fit this call site — why, rather than a fourth copy
platform-admin-gate.tsis the shared judge, and its own header argues that N copies of an authorization predicate is the shape that drifts. But it answers the session-shaped question:isPlatformAdminUser(sessionUser)readsisPlatformAdmin/positions[]/ the legacyrolescalar — fieldscustomSessioncontributes.This call site holds no session user. It resolves its actor through
resolveActor(ctx), the shared hook-order-independent resolver used by ~5 gates, which returns{ userId, activeOrgId }— an id. Feeding the session-shaped judge would need either a second session resolution (the exact "two resolution sites" defectauth-manager.ts's own #8102 comment says never to reintroduce) or a change to that shared contract. Worse, a raw session user carries nopositions[], soisPlatformAdminUserwould fall through torole === 'admin'— the retired D2 channel — refusing real permission-set platform admins and admitting only the channel this family is closing.So this needs the id-shaped judge, which #10348 records as not yet consolidated. Rather than mint a fourth, differently-named spelling, this PR uses the exact name and body #10352 is already landing (
isPlatformAdminUserId), so the two collide loudly on one identifier and the merge resolution is "keep one". #10348's other spellings are deliberately untouched here.The wider predicate is removed, not parked
With
/sso/registerno longer asking the org question,isOrgOrPlatformAdminlost its only production caller — and the compiler said so:error TS6133: 'isOrgOrPlatformAdmin' is declared but its value is never read(noUnusedLocals, tests excluded from the package tsconfig). It is removed rather than kept dead. ItsisOrgAdminGradeimport went with it.#5942's substance is not lost coverage: the one grade ladder keeps its own direct pins in
member-role-canonical.test.ts(case, comma and array spellings) and its other reader inlast-admin-guard.ts. What is deliberately gone is the claim that this seam asks the org question — it no longer does, and the new block pins that as a refusal rather than leaving it unstated.Pins — two-directional on purpose
The hook had no pins at all before this:
SSO_REGISTER_FORBIDDENappeared nowhere outside its ownthrow. A refusal-only suite would pass on a hook that refuses everyone, so both directions are asserted:403andSSO_REGISTER_FORBIDDEN(ADR-0112: code AND status, never one alone)422 /reserved/)401, the vendor's ownThe platform admin is granted the ADR-0068 way — an org-less
sys_user_permission_setlink toadmin_full_access— and the fixture assertssys_user.roleis NOT'admin', so the suite cannot pass by riding the retired scalar.Everything is real: a real ObjectQL engine over real better-sqlite3, the real
AuthManagerwith the realsso()plugin, real sign-up and real session cookies. Onlysys_permission_set/sys_user_permission_setare declared locally (they live in@objectstack/plugin-security) — thelast-admin-guard.test.tsprecedent, so a fixture adds no dependency edge.Ablation — predicted signature stated first, both directions
AuthManageris imported relatively (./auth-manager.js) and there is no vitest alias, so vitest executes the mutated source; thedist/staleness hazard does not apply to the mutated module, and no rebuild could change what these legs measured.git hash-objectda7f6561…7e29fb56…expected 422 to be 403×2,expected true to be false×11. Pins ② and ③ stayed green35112457…expected 403 to be 422,expected false to be true×2. Every refusal pin stayed greenda7f6561…— byte-identical to pre-ablationABLATIONmarker in the fileLeg B is the direct demonstration that a one-directional suite would have been blind here: under a hook that refuses everyone, every refusal assertion stays green and only the admission pins catch it.
Verification
Gate union run after the final commit on a clean worktree,
node scripts/pm/dispatch-gates.mjswith no paths passed, atbe985a623. Exit codes captured before any pipe; verdicts quoted from each gate's own line.Convention-triggered by the new/edited test files:
✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) … none newcheck-engine-double-contract: OK — 338 pinned, 133 in the DEBT ledger, 2 exempt.✓ where-matcher conformance holds: 265 matcher(s) discovered … 0 silently-wrongcheck-type-check-coverage: OK — 64/77 workspace packages type-checkedcheck-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured … none above its recorded number.check:type-check-debtfirst refused with a prerequisite, not a verdict (--re-measure cannot run: 30 workspace dependenc(ies) … have no built type entry point on disk). Satisfied it exactly aslint.ymldoes —turbo run build --filter='./packages/*' --filter='./packages/*/*',70 successful, 70 total— and re-ran to the green above.No ledger entry was raised.
@objectstack/plugin-authTEST_DEBT is left at 109 as instructed; the gate notes it now measures 108 (-1) and says lowering is optional (Not an error). Flagging rather than editing.Package verification, on the merged tree:
pnpm --filter @objectstack/plugin-auth typecheckclean,vitest run61 files / 1323 tests passed.In-repo consumer population — bounded
Every search validated against a known-present control term first.
/sso/registercallersobjectstack(this repo)admin/sso/register→ 9 files/api/v1/auth/admin/sso/register, including bothsys_sso_providermetadata actionsobjectui(console)api/v1/auth→ presentobjectos(docs)api/v1/auth→ presentexamples/, SDK,apps/signIn→ presentZero in-repo callers of the direct endpoint; deployments are not visible from here. This is the half of triage's recorded confidence gap that is measurable from the repo — it does not establish that no deployed org admin uses the endpoint.
Serial note
PR #10352 (#9968) is open and touches the same file.
auth-manager.tsis not oncheck-single-claim-paths.mjs's explicit path list, so the single-writer gate does not block either side; the residual risk is a merge conflict.origin/mainwas merged into this branch before the final commit (#10352 had not landed). The overlap is deliberate and named: both branches add a privateisPlatformAdminUserIdwith the same body — keep one. #10352 additionally references{@link isOrgOrPlatformAdmin}, which this PR removes, so that docblock line needs updating on whichever side lands second.Out of scope, filed separately
See the linked findings in the report; nothing outside this hook's region was changed.
packages/spec/**was not touched —SSO_REGISTER_FORBIDDENis already registered inerror-code-ledger.zod.ts, so the refusal reuses the existing catalog member and no error-code registration was needed.Generated by Claude Code