Skip to content

tests(identity): pin the admin credential clauses and a derived non-admin refusal sweep over the whole /admin/ surface - #9664

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-9482-admin-lifecycle-clause-pins
Aug 18, 2026
Merged

tests(identity): pin the admin credential clauses and a derived non-admin refusal sweep over the whole /admin/ surface#9664
os-zhuang merged 2 commits into
mainfrom
claude/issue-9482-admin-lifecycle-clause-pins

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Part of #9482.

Deliberately Part of, not Fixes: this pins the clauses that are automatable on this stack and re-scopes the item honestly, but five of the item's clauses are blocked on a product decision (#9652) and one is a screenshot oracle. Closing #9482 on merge would lose that remaining half.

QA run #9401 scored identity-auth.admin-lifecycle-operations green on a pin covering exactly one of its ten clauses (C8, the audit-log shapes).

What lands

admin-route-nonadmin-refusal.dogfood.test.ts — C9's refusal half, over a DERIVED population.

The clause is "the gate holds both ways for EVERY admin operation", and a test that hardcodes today's routes cannot pin that — it passes forever while route N+1 ships unguarded. So the population is read off the running stack:

halfsourceroutes
AhonoApp.routes — ObjectStack raw mounts, registered ahead of better-auth's catch-all so they never appear in auth.api9
Bauth.api — the better-auth endpoint table, the seam auth-route-ledger.conformance.test.ts already uses24
union (2 overlap: create-user, set-user-password)31

The item's clause-9 prose names six routes. The real surface is 31.

The derived set and the suite's classification table are checked for exact agreement in both directions: a route the stack serves with no entry fails by name, and an entry the stack no longer serves fails as stale. That is what puts route N+1 in scope automatically — it cannot be added without someone recording what a non-admin must get from it.

The payloads are load-bearing. Measured: better-auth, and the ObjectStack sso / unlock-user / oauth2 mounts, validate the request body before the authorization check. An empty-body /admin/ban-user answers a plain member:

400 {"message":"[body.userId] Invalid input: ...","code":"VALIDATION_ERROR"}

byte-identical to what the platform admin gets. A route-walk built on empty bodies asserts nothing about authorization while looking exactly like a passing security sweep. Every route here is fired with a payload valid enough to reach the gate, and refusals assert code as well as status.

admin-credential-lifecycle.dogfood.test.ts — C1 and C2, both sides.

  • create-user with an explicit password alongsidegeneratePassword: true applies the explicit one and returns no generated password — with a generatePassword-only create as the contrast, so the clause cannot be satisfied by generatePassword being inert. The account carries its credential sys_account row.
  • set-user-password rotates: the original password is proven to work before the rotation, the new one after, and the old one is refused 401 INVALID_EMAIL_OR_PASSWORD.
  • Both refusals are asserted with the state read back: a forged create leaves no row behind; a forged rotation leaves the victim's real password working and the hijack password dead.

What is NOT pinned, and why

clausestatus
C1, C2, C9 (refusal half)pinned here
C8already pinned
C7 — console impersonation indicatormanual: its oracle is a screenshot
C0, C3, C4, C5, C6, and C9's allowed half on the better-auth bucketblocked on a product decision — #9652

#9652 in one line: better-auth's admin plugin authorizes on the legacy user.role === 'admin' scalar, which ADR-0068 D2 deliberately stopped synthesizing — so ban-user, unban-user, set-role, remove-user, impersonate-user, revoke-user-session(s), list-users, get-user, list-user-sessions and update-user refuse the platform admin himself with 403 YOU_ARE_NOT_ALLOWED_TO_*. Measured on the seeded dev admin: sys_user.role is 'user', positions is ['user','platform_admin']. Confirmed by construction — writing role = 'admin' onto that same row flips ban / unban / list-users / impersonate to 200 in the same boot, and the ban then persists and refuses the banned user's sign-in.

Pinning a clause there would require changing an admin route's behaviour, which is not a tests-only call — so it is reported, not done. Neither side of that bucket's allowed half is asserted, so fixing #9652 will not have to edit these tests.

The four /admin/sso/* routes are classified capability-disabled: with SSO off, anonymous, member and admin receive identical capability errors, so authorization is not observable on them here. They carry a tripwire — member and admin must get the same answer — so the day SSO is enabled they go red and get reclassified rather than coasting on a meaningless green (#9653).

Findings filed (unassigned, none fixed here)

Verification

Suite:vitest run over both new files — Test Files 2 passed (2), Tests 12 passed (12), exit 0.

Measured on the tree of a4362e217. Head is 2a3fb5835, which differs from it by seven // comment lines and nothing else — verified mechanically (every +/- line in git diff a4362e217 2a3fb5835 begins with //), so no executable code changed after the green run.

Ablations. Dogfood resolves @objectstack/plugin-auth through dist/, so each leg rebuilt the package and proved the mutation reached the artifact with scripts/ablation-dist-preflight.mjs before its colour counted.

ablationpreflightpredictedactual
Plant an ungated new /admin/ route✓ marker in 2 built files (2 sourcemap hits excluded)classification-coverage test RED, naming the new routeREDPOST /api/v1/auth/admin/ablation-new-route: expected [ Array(1) ] to deeply equal []
Disable the shared ADR-0068 gateAdmin✓ marker in 2 built filesobjectstack-gate RED (member not 403) and credential-lifecycle's non-admin test REDRED, both — member create-user returned 200 with a real user created, expected 200 to be 403
restore, both legs--absent: marker gone from all 12 built filessource restored byte-identical

The first ablation is the one that matters for this card's premise: it demonstrates that a newly mounted /admin/ route is in scope automatically and fails until it is classified.

Local gates, derived from the changed paths with node scripts/pm/dispatch-gates.mjs (not recalled):

check:platform-checklist OK — 15 areas, 190 items
check:nul-bytes OK — 6162 files, no raw control bytes
check:cross-package-test-inputs --verify OK — 12 packages, all declared
check:test-source-alias OK check:type-source-resolution OK
check:query-options-erasure OK — ratchet holds, no files added
check:engine-double-contract OK — 319 pinned check:where-matcher OK — 255 matchers
check:type-check-coverage OK @objectstack/dogfood typecheck OK
spec: check:empty-state / check:liveness / check:strictness-ledger / check:variant-docs OK

The two new tests add no filesystem reads, so they are not cross-package escaping tests and need no new declaration — verified by query, with a control proving the search works (0 hits in the new files, 4 in the sibling escaping test). Neither file contains .skip / .only / .todo, also verified with a control.

Two legs deferred, stated rather than implied.check:type-check-debt is NOT MEASURED locally: it refuses without the full workspace closure built, naming @objectstack/service-knowledge as unbuilt, and that refusal means not-measured, never not-applicable. CI runs it with the closure built. And the universal-invariant assertion's own ablation was not run — the shared verify lock stayed contended (queue depth 8-10 for ~25 minutes). Its discrimination is inferred from the gateAdmin ablation, which produced exactly the condition it tests (a plain member receiving 2xx from an /admin/ route) and was caught by the sibling assertion in the same file; it is not directly observed.

CI convergence is the PM's, per the 2026-08-10 ruling.

Changeset

None: the diff is packages/qa/dogfood/test/** (@objectstack/dogfood is private: true) plus the checklist JSON. Nothing publishable changes, and scripts/check-empty-changeset.mjs rejects an empty one — so this carries the skip-changeset label instead.


Generated by Claude Code

…dmin refusal sweep (#9482)
QA run #9401 scored `identity-auth.admin-lifecycle-operations` green on a pin
covering exactly one of its ten clauses (C8, the audit-log shapes). Two new
dogfood pins close the clauses that are automatable on this stack, and the
item's `automated.ref` is re-scoped to say precisely what stays manual, what is
now pinned, and what is blocked on a product decision rather than on a test.
Derived non-admin refusal sweep. The clause is "the gate holds both ways for
EVERY admin operation", so the route population is read off the RUNNING stack
rather than listed: `honoApp.routes` for the 9 ObjectStack raw mounts that sit
ahead of better-auth's catch-all, unioned with `auth.api` for the 24 endpoints
the catch-all publishes. 31 routes, against the 6 the clause prose names. The
derived set and the suite's classification table are checked for exact agreement
in both directions, so a newly mounted /admin/ route fails the suite by name
until someone records what a non-admin must get from it -- which is what makes
this catch a silently unguarded new route rather than only today's.
Every route is fired with a payload valid enough to REACH the authorization
check. That is load-bearing: better-auth and the ObjectStack sso/unlock-user/
oauth2 mounts validate the body first, so an empty-body route-walk draws an
identical 400 VALIDATION_ERROR for member and admin alike and asserts nothing
about the gate while looking exactly like a passing security sweep.
Credential clauses. create-user applies an EXPLICIT password supplied alongside
generatePassword:true and returns no generated one, with a generatePassword-only
create as the contrast so the clause cannot be satisfied by generatePassword
being inert; the account carries its credential sys_account row. set-user-password
rotates -- the original password is proven to work before the rotation, the new
one after, the old one refused 401 INVALID_EMAIL_OR_PASSWORD. Both routes'
refusals are asserted with the state read back: a forged create leaves no row,
and a forged rotation leaves the victim's real password working.
Measured and NOT pinned. C0/C3/C4/C5/C6 ride better-auth's own admin endpoints,
which authorize on the legacy `user.role === 'admin'` scalar that ADR-0068 D2
deliberately stopped synthesizing -- so they refuse the PLATFORM ADMIN himself
(403 YOU_ARE_NOT_ALLOWED_TO_*). Filed as #9652 with the three options; not fixed
here, because changing an admin route's behaviour is not a tests-only call.
Neither side of that bucket's allowed half is asserted, so the fix will not have
to edit these tests. C7's oracle is a screenshot and stays manual.
Findings filed unassigned: #9652 (the gate mismatch), #9653 (the /admin/sso/*
bridges carry no ObjectStack-side gate and their delegated gate is unprovable
while SSO is off), #9654 (validation runs before authentication on the
ObjectStack /admin/ mounts).
Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

@os-zhuangos-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 18, 2026 — with Claude
…9482)
The `objectstack-gate` bucket no longer contains the /admin/sso/* routes, so
the example of a post-gate semantic error no longer applies to them. Names the
two measured cases instead: 200 for unlock-user, 404 RESOURCE_NOT_FOUND for
toggle-disabled's unknown client. Comment only, no assertion changes.
Co-Authored-By: Claude <noreply@anthropic.com>
@os-zhuang
os-zhuang marked this pull request as ready for review August 18, 2026 15:01
@os-zhuang
os-zhuang added this pull request to the merge queueAug 18, 2026
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — accepted, ready + enqueued. Two questions returned: one answered here, one ⛔ not mine.

domain:cli seat, session session_012WKSnqAaoqtW3QX7SSf1Vk. Dev returned status: needs_decision — correctly, because the remaining clauses need a decision, not because this diff does.

Verified myself, not taken on report

The head is not the tree that was tested — and the dev said so. Green run was measured at a4362e217; head is 2a3fb5835. I re-derived the claim mechanically rather than accepting the prose:

git diff a4362e217 2a3fb5835 → 0 non-comment changed lines

Every +/- line is a // comment. The green suite applies to the head. ⭐ Declaring a post-verification commit and proving it inert is exactly the right handling; the failure mode it avoids is a green reported against a tree nobody merges.

Six required checks all completed: success — including TypeScript Type Check, which covers the one gate the dev declared NOT MEASURED: check:type-check-debt refused without the full workspace closure (@objectstack/service-knowledge unbuilt). ⭐ "that refusal means not-measured, never not-applicable" — the correct reading, and the distinction that stops a refused gate being filed as a passed one.

Both ablations are real, each with scripts/ablation-dist-preflight.mjs proving the mutation reached dist/ before its colour counted — necessary here because dogfood resolves plugin-auth through the built artifact. The load-bearing leg is the planted ungated route: predicted red, observed red, naming the route. That is what makes "route N+1 is in scope automatically" a measured property rather than an intention.

✅ Q2 — answered, and this one is a PM call: keep #9482 open, Blocked-by: #9652

The dev used Part of #9482 where my dispatch prompt said Fixes #9482, followed AGENTS.md over my brief, and flagged the conflict instead of choosing silently. It was right on all three counts:

#9482 stays open. Once this PR merges I will set pm:blocked + a Blocked-by: #9652 body line so the unlock scan re-queues it automatically when that decision lands. Its body is short and intact, so the edit is safe (⛔ not the API-truncation trap).

⛔ Q1 (#9652) — NOT this seat's to decide, and I am not adjudicating it

The A/B/C question is who authorizes an admin route. That sits on the human floor twice over: it is a security / authorization boundary, and option C would reopen ADR-0068 D2 rather than edit anything. ⛔ The PM's代裁 lane covers bug repair and code tidying — never this. It goes to the triage seat for routing and to the maintainer for the call.

What I will say, as information rather than adjudication: the dev's analysis is unusually well-grounded — it did not merely observe the 403s, it flipped them by construction (writing role='admin' onto the seeded admin's row turns ban / unban / list-users / impersonate to 200 in the same boot), which converts "I think this is the cause" into "this is the cause." And it correctly classified the blast radius: fails closed, so a broken admin capability rather than an escalation — QA #9401's "no access-control hole" verdict stands. The user-visible cost is real today: sys_user's Ban / Unban / Impersonate console actions are type:'api' proxies onto those routes, and ADR-0071's SCIM path forces the plugin on.

Also worth keeping

Two zero-match traps caught by reading counts instead of exit codes — a vitest -t filter that matched nothing and reported 7 skipped (7) at exit 0, and a pnpm install --workspace-concurrency=2 that errored on an unknown option. Both are the same failure this lane has paid for before: a run that did nothing is indistinguishable from a run that passed, unless you read what it actually executed.

⚠️ One ablation leg was not observed — the universal-invariant assertion's own — because the shared verify lock held queue depth 8–10 for ~25 minutes. Its discrimination is inferred from the gateAdmin leg, which produced exactly the condition it tests. Declared, not implied. That lock is now #9661; ⛔ not this PR's problem.

Findings #9652 / #9653 / #9654 filed unassigned for triage. ⛔ Not graded by this seat.


Generated by Claude Code

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

Labels

size/lskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-zhuang@claude