Skip to content

fix(plugin-auth): the auth catch-all yields only a 404 that disclaims ownership - #15918

Merged
os-warren merged 4 commits into
mainfrom
claude/issue-15417-auth-catchall-404
Sep 5, 2026
Merged

fix(plugin-auth): the auth catch-all yields only a 404 that disclaims ownership#15918
os-warren merged 4 commits into
mainfrom
claude/issue-15417-auth-catchall-404

Conversation

@os-warren

@os-warren os-warren commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Part of #15417

⚠️ Part of, not a closing keyword, and deliberately. This lands the framework-side defect the card's mechanism turned out to be. The card's headline claim does not reproduce framework-side, and its second observation is a composition question that is being split out — both measured below. #15417 remains open for the maintainer to re-triage on this evidence.

Step 1 first: the card's own control, on a framework-side boot

The card asked for this before anything else, and it is the row that gives every other row meaning. Booted the real stack (@objectstack/verify + the showcase app, packages/qa/dogfood), member and platform-admin sessions:

POST /api/v1/auth/admin/definitely-not-a-route-1989  member -> 404  len=0  ct=(none)   <-- CONTROL
POST /api/v1/auth/admin/definitely-not-a-route-1989  ADMIN  -> 404  len=0  ct=(none)
POST /api/v1/auth/admin/definitely-not-a-route-1989  anon   -> 404  len=0
POST /api/v1/auth/admin/update-user      member/ADMIN      -> 404  len=0
GET  /api/v1/auth/admin/list-users       member/ADMIN      -> 404  len=0
POST /api/v1/auth/admin/set-role         member/ADMIN      -> 404  len=0
POST /api/v1/auth/admin/ban-user         member            -> 403  PERMISSION_DENIED
POST /api/v1/auth/admin/set-user-password member           -> 403  PERMISSION_DENIED
POST /api/v1/auth/update-user            member            -> 200  {"status":true}   (positive control; readback confirmed the write)

The reported 200 {} does not reproduce on a framework-side boot — the nonexistent path answers 404. The two 403 PERMISSION_DENIED rows and the positive control reproduce exactly as the card recorded them, so the boot is comparable; it is specifically the vacuous 200 that is absent.

What does produce it — the mechanism, reproduced framework-side

The catch-all is involved, but not through its width. Since #4088 it is deliberately not terminal: when better-auth answers 404 it calls next() and lets whatever else matched answer instead. That yield is load-bearing — plugin-hono-server mounts /auth/me/permissions and /auth/me/localization from its own kernel:ready hook, and without it they are reachable only when HonoServerPlugin happens to register first.

The yield had only the status to go on, so it yielded every 404. Add one broad downstream mount — app.all('/api/v1/*', c => c.json({})), the shape a composition adds — and on the same real stack:

POST /api/v1/auth/admin/definitely-not-a-route-1989  -> 200 {}     <-- the card's reading, reproduced
POST /api/v1/auth/delete-user                        -> 200 {}     <-- and this is the real defect

delete-user is a route better-auth serves; it answers 404 because user.deleteUser is deliberately unconfigured, and auth-route-ledger.ts carries it under the disabled disposition for exactly that reason. Its answer was being replaced by a downstream route's. The same held for every 404 a routed endpoint produces — a bad token, an unknown id, an admin family the deployment does mount.

The fix: narrow the yield, never the mount

⛔ The mount is untouched — still exactly rawApp.all(\${basePath}/*`)`, still forwarding every request under it. What narrowed is which 404 may be handed on: the catch-all now asks better-auth's live instance whether it owns the path, and yields only when it does not.

The seam is auth.api — the same one auth-route-ledger.conformance.test.ts reads and the /admin/ dogfood sweep derives from, because there is no route table to enumerate by hand. Matching mirrors better-call's own createRouter walk.

⚠️ That mirroring is load-bearing, and it is where my first draft was wrong. I claimed the nine /admin/oauth2/* endpoints were protected examples. Measured on the stock boot, all nine are in auth.api and all nine carry SERVER_ONLY: true — better-call never routes them, so their 404 genuinely disclaims ownership and they stay yieldable. Ownership is "does better-call route this", not "is it in auth.api". The prose and a dedicated pin now say so; the e2e caught the overclaim, not review.

A table that cannot be built answers "not owned", so an enumeration failure degrades to the pre-#4088-era behaviour rather than taking that surface down with it.

One carve-out, measured and bounded

The changeset used to say a path better-auth does not own is "yielded exactly as before". The clause-② review built a live differential — better-call 1.4.0's createRouter plus processRequest's pre-checks, re-run over rou3 0.9.2 against the real auth.api at the stock and the maximal configuration, 4004 + 5278 = 9282 (method, path) pairs over 572 + 754 paths — and found 0 divergences in the yield direction. It also found the sentence is not unconditional in the other direction, and the changeset now carries the carve-out in the same words:

A trailing-slash or doubled-slash spelling of a path better-auth DOES own/api/v1/auth/delete-user/, /api/v1/auth//sign-in/social — is now claimed rather than yielded. better-call treats those spellings as unrouted (it refuses on a // and on trailing-slash parity before it looks the route up), while this ownership table strips the trailing slash and drops empty segments and so counts them as owned. On a composition with a broad downstream mount, such a spelling therefore answers better-auth's 404 instead of that mount's response. Bounded at 91 + 153 pairs (stock) and 121 + 212 (maximal), and confirmed on the wire: POST /auth/delete-user/ answers 404 at this head where the pre-fix yield gave the wildcard's 200 {}.

Left as it is, deliberately: no route in this repo registers a spelling of that shape, nothing under /auth/me/* or any genuinely unowned path is touched, and where it does show the effect is that a near-miss spelling stops answering a foreign mount's vacuous 200 — the direction this change argues for. Aligning ownsRoute with better-call's own pre-checks, with a pin, is a follow-up card rather than a source change on this head.

Both directions, after the fix, on the real stack

no downstream mount with the wildcard
POST /auth/delete-user (owned, answers 404) 404 404 — protected
GET /auth/admin/oauth2/resources (SERVER_ONLY, unrouted) 404 200 {} — still yielded, correctly
POST /auth/admin/definitely-not-a-route-1989 (unowned) 404 200 {} — still yielded (see split, below)
GET /auth/me/permissions (#4088 surface) 200 200 — intact

The full post-fix table is byte-identical to the pre-fix table above: ban-user still answers the member 403 PERMISSION_DENIED, POST /auth/update-user still answers 200 {"status":true} and the write still lands (read back from the store).

SERVER_ONLY was independently confirmed by review at exactly 9 /admin/oauth2/* endpoints at both configurations, with the unrouted-404 signature on the wire (len=0 ct=(none), the same signature as the nonexistent path) — and admin-route-nonadmin-refusal.dogfood.test.ts already classifies those same nine as not-mounted.

Pins, and each one mutated

Every pin was mutated, the mutation proven on disk by a git hash-object delta plus marker counts, restored with git checkout HEAD -- ABSOLUTE_PATH under a trap ... EXIT INT TERM, and the restore proven by an empty git diff HEAD plus the blob back at its HEAD value.

mutation suite result
yield condition reverted to the pre-fix status === 404 auth-catchall-yield.test.ts 3 failed / 5 passed — red
owns() forced to always return true auth-catchall-yield.test.ts 3 failed / 5 passed — red
:param matching relaxed to a prefix match better-auth-route-ownership.test.ts 1 failed / 9 passed — red

The ablation ran at 62fce1ac5; its subject files are byte-identical at ee26bc613. Review checked that carry-over argument independently and found all seven files identical at both commits. Review also re-ran the legs itself, including a boot-level leg 1 with a rebuilt dist/ and an ablation-dist-preflight marker check in both directions.

⚠️ Recorded rather than fixed: leg 3's in-tree pin is thin — under the same mutation review's live differential reds 102 of 140 rows while the in-tree suite reds 1 of 10. The leg discriminates, so it is not a false pin; a live-table pin for it is a noted follow-up.

auth-catchall-fallthrough.test.ts (#4088) is green throughout. Its fake AuthManager gained the one method the catch-all now calls, derived from that file's own owned table so the file keeps meaning what its title says.

Half 2 — measured, and it is a composition question

admin/list-users · admin/set-role · admin/update-user, the three the card measured absent:

  • better-auth admin plugin off (the stock composition): absent from auth.api — 9 /admin/ endpoints, all oauth2 — and they answer 404.
  • admin plugin on (OS_SCIM_ENABLED=true, which forces it): all three present — 24 /admin/ endpoints — answering real vendor refusals (403 YOU_ARE_NOT_ALLOWED_TO_LIST_USERS and siblings).

So the three routes are configuration-dependent, and the ledger is not wrong. BETTER_AUTH_MOUNTED_SURFACE is pinned at the maximal LEDGERED_PLUGIN_CONFIG and its own header already states that a deployment running fewer plugins serves a subset and that this is correct — it is publication, not liveness, in as many words. ⛔ Deleting those rows would both break the exact-equality conformance test and misreport the mounted surface. Triage recorded the split criterion in advance; this is that case, so it is filed separately (#15920) rather than blocking this PR.

Verification

At head 81226d5ba. Every exit code captured immediately after a single redirected command, never off a multi-command line and never through a pipe.

  • Gate family re-derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack from the actual changed files — no stale-tree warning, and the family is identical to the previous round's 50. 50 run, 47 exit 0.
  • node scripts/check-adr-0087-registration.mjs --self-testexit 0 (325 assertions over real temp git repos), as the control for the run below.
  • node scripts/check-adr-0087-registration.mjs --base origin/main --head 81226d5ba5ac186a8adaae34176597fb1e7dbb02exit 0.
  • ⚠️ 3 gates NOT MEASURED, all exit 3 = PREREQUISITE NOT MET, none of them passes: check:dual-build-cjs-loads, check:published-readme-exports, check:dts-closure. All three read built output and refuse until pnpm build has populated dist/; this round ran in a fresh worktree with nothing built, which is why check:dts-closure joins the two from last round. Review confirmed the first two CI-green on the previous headcheck:dual-build-cjs-loads in Build Core, check:published-readme-exports in Type Check · consumer gates (not Lint) — and this round moved only the changeset plus an unrelated auth-plugin.ts change that arrived with the origin/main merge.
  • Source is unchanged from the reviewed head: this round's only authored edit is the changeset carve-out.

Earlier rounds, retained: pnpm --filter @objectstack/plugin-auth test exit 0 (99 files / 2085 tests; review measured 100 / 2087 with its own scratch file), pnpm --filter @objectstack/plugin-auth typecheck exit 0 including check:test-typecheck, which compiles the test layer.

… ownership

`registerAuthRoutes` mounts one catch-all over the auth namespace and, since
#4088, deliberately yields to the rest of the Hono chain when better-auth
answers 404 — that is what keeps `plugin-hono-server`'s `/auth/me/permissions`
and `/auth/me/localization` reachable in either registration order.

The yield had only the status to go on, so it could not tell "I do not serve
this path" from "I serve it and the answer is 404". Measured with the shipped
handler on a real Hono app: with one broad downstream mount in the chain —
`app.all('/api/v1/*', c => c.json({}))`, the shape a composition adds —
`POST /api/v1/auth/delete-user` came back `200 {}` where better-auth had
answered 404 because `user.deleteUser` is unconfigured. `auth-route-ledger.ts`
carries that route under the `disabled` disposition precisely because it is
published and refused, and the same held for every 404 a routed endpoint
produces for a bad token, an unknown id, or an admin family the deployment does
mount. All of those answers were up for grabs.

The catch-all now asks better-auth's live instance whether it owns the path
before it yields. The seam is `auth.api` — the same one the route ledger's
conformance test reads and the `/admin/` dogfood sweep derives from — and the
matching mirrors better-call's own `createRouter` walk: its `SERVER_ONLY` skip,
its `:param` syntax, its per-method registration. The skip is load-bearing, not
cosmetic: measured on the stock boot, all nine `/admin/oauth2/*` endpoints are
in `auth.api` carrying `SERVER_ONLY: true`, so better-call never routes them and
their 404 stays yieldable. Ownership is "does better-call route this", not "is
it in `auth.api`". A table that cannot be built answers "not owned", so an
enumeration failure degrades to the previous behaviour instead of taking the
#4088 surface down with it.

The mount is untouched: it still claims exactly `${basePath}/*` and still
forwards every request under it. What narrowed is which 404 may be handed on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
@github-actions github-actions Bot added the size/l label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 17 documentable anchor(s).

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via /api/v1/* (route, a path literal on a changed line))
  • content/docs/kernel/contracts/auth-service.mdx (via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx (via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class))
What this run could not see
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

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 cee3961759160ed72aacb407f15288cbc018d2ebpackageMentionDocs.

Which tree this was computed on

This run read content/docs from af784d80873ffad2bb2459eaa2ca726c3cd85634 — the merge of head 81226d5ba5ac186a8adaae34176597fb1e7dbb02 into base cee3961759160ed72aacb407f15288cbc018d2eb, 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 af784d80873ffad2bb2459eaa2ca726c3cd85634 && git checkout af784d80873ffad2bb2459eaa2ca726c3cd85634
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cee3961759160ed72aacb407f15288cbc018d2eb 81226d5ba5ac186a8adaae34176597fb1e7dbb02 && git checkout -B drift-repro cee3961759160ed72aacb407f15288cbc018d2eb && git merge --no-ff 81226d5ba5ac186a8adaae34176597fb1e7dbb02

node scripts/docs-audit/affected-docs.mjs --json cee3961759160ed72aacb407f15288cbc018d2eb

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs cee3961759160ed72aacb407f15288cbc018d2eb → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

Clause-② contract review — PR #15918 (card #15417) — verdict: PASS

Tier: CONTRACT_REVIEW_TIER = claude-fable-5-1 (on this head scripts/pm/dispatch-gates.mjs:9852). Evidence: the PM seat's attestation that this Agent call carried an explicit model: fable override, plus my own system-prompt identity (claude-fable-5-1) — override + self-report. Not read from get_session (it describes the parent session and cannot see a per-subagent override), and not claimed as an "exact match" reading; the transcript-stamp verification is the parent's.

Independence pair (C4, machine spelling):

  • Implemented-by: claude/issue-15417-auth-catchall-404
  • Reviewed-by: session_01XpTx2tbq3pZRYAdoGt6E6Y

The dev was a mode:subagent of this same PM session, so its identity is its branch (2026-09-02 ruling, reading a); the two grammars are disjoint, so this is not the SELF-REVIEW case by the rule's own comparison. Stated for the record: I am a subagent of the same seat, and my brief carried the seat's summary of the dev's claims. I treated every sentence of it as a claim to falsify and re-measured each below; nothing in this verdict rests on the brief's or the dev's word.

Subject: head ee26bc613, merge-base abdceef8c, detached worktree /home/user/objectstack-review-15918 (own pnpm install, own pnpm --filter '@objectstack/dogfood...' build, exit 0). 7 files, +563/−2 — six under plugin-auth/ plus the changeset. Head, merge-base, file set, the fences and check-adr-0087-registration were verified by the PM before dispatch and are not redone here.

1 · The falsification (blocking bar — met)

Booted the real stack myself — @objectstack/verify + @objectstack/example-showcase through the dogfood harness, stock config (OS_SCIM_ENABLED unset → admin: pluginConfig.admin ?? scimEffective at auth-manager.ts:2716 resolves the admin plugin off), member and platform-admin sessions:

POST /auth/admin/definitely-not-a-route-1989  member/ADMIN/anon -> 404  len=0  ct=(none)   <-- CONTROL
POST /auth/admin/update-user · GET /auth/admin/list-users · POST /auth/admin/set-role   member -> 404 len=0 ct=(none)
POST /auth/admin/ban-user · POST /auth/admin/set-user-password   member -> 403 {"success":false,"error":{"code":"PERMISSION_DENIED",…}}
POST /auth/update-user   member -> 200 {"status":true}   (readback: sys_user.name changed in the store)
GET  /auth/me/permissions · /auth/me/localization   member -> 200 (real bodies)
POST /auth/delete-user   member -> 404  len=0  ct=application/json   (the ledger's `disabled` route)

Every row of the dev's Step-1 table reproduces byte-for-byte, including the store readback. The boot is the comparable one in the sense that matters: the auth surface is live (positive control lands), the ObjectStack raw mounts refuse (403 rows), and the nonexistent path answers 404, not 200 {}.

Is the comparison sound against the card's stack? Two checks beyond the rows. (i) The #4088 yield (ea24593bd) is an ancestor of the card's framework pin 5b2ad1b41, and git show 5b2ad1b41:…/auth-plugin.ts carries the if (response.status === 404) line — so the mechanism the dev names was present on the stack the card measured. (ii) What in-repo could independently produce the 200 {}? Nothing: the only other auth catch-all in the tree, the @objectstack/hono adapter's app.all('${prefix}/auth/*') (packages/adapters/hono/src/index.ts:343, #4117), yields 404s the same way but into a terminal dispatcher whose unknown-path answer is its own semantic 404 (http-dispatcher.ts "3. Fallback — return semantic 404"), never a 200. ⚠️ NOT MEASURED: the cloud composition itself — objectstack-ai/cloud is not reachable from this session, so "the shape a composition adds" stays the dev's hypothesis about cloud; what is measured is that the framework alone answers 404 and that one downstream wildcard is sufficient to reproduce the card's row (§2). The falsification is sound.

2 · The mechanism and the fix, end-to-end on the real stack (blocking bar — met)

Second boot, same stack, http-server.getRawApp().all('/api/v1/*', c => c.json({})) installed after boot and before the first request (Hono's matcher freezes on first match — the dev's probe must have done the same):

request (member) head ee26bc613 pre-fix yield (leg 1, rebuilt dist/)
POST /auth/admin/definitely-not-a-route-1989 (unowned) 200 {} — yielded (the split half) 200 {}
POST /auth/delete-user (owned, disabled) 404 — protected 200 {} — the defect
GET /auth/admin/oauth2/resources (SERVER_ONLY) 200 {} — yielded, correctly 200 {}
GET /auth/me/permissions · /me/localization 200 real bodies (hono-server, not the wildcard) 200
POST /auth/admin/ban-user 403 PERMISSION_DENIED same
POST /auth/update-user 200 {"status":true} same

The leg-1 column is a boot-level ablation, not the unit one: auth-plugin.ts mutated (blob c8d7c214… vs HEAD e7e45c8d…, removed-marker 0, injected-marker 1), pnpm --filter @objectstack/plugin-auth build exit 0, scripts/ablation-dist-preflight.mjs @objectstack/plugin-auth ABLATION_LEG1_never_setmarker present in 2 built files (the dogfood harness resolves plugin-auth from dist/); then restore under trap … EXIT INT TERM → blob back at e7e45c8d…, git diff HEAD empty, rebuild exit 0, preflight --absentmarker absent from all 12 built files, control re-run at head → delete-user 404. The card's row and the sharper defect both reproduce through the shipped handler, and the fix closes exactly the second one.

3 · #4088 intact, and owns() vs better-call (blocking bars — met, with one bounded divergence named)

Oracle. better-call 1.4.0 dist/router.mjs createRouter (lines 20–25: skip !options || !path, skip metadata.SERVER_ONLY, one rou3.addRoute per declared method) plus processRequest's pre-checks (lines 33–51: basePath, /\/{2,}/, trailing-slash parity, !route?.data), re-run verbatim over rou3 0.9.2 on the live auth.api of a real AuthManager — same boot as auth-route-ledger.conformance.test.ts — at both the stock and the maximal (LEDGERED_PLUGIN_CONFIG) configuration. better-auth 1.7.2 constructs its router with openapi: {disabled:true}, basePath = new URL(ctx.baseURL).pathname (dist/api/index.mjs:152–162) and skipTrailingSlashes: false (ObjectStack passes no advanced.skipTrailingSlashes).

Subject. The shipped AuthManager.ownsRoute(request) — endpoint-path derivation included.

Inputs. Every endpoint path with params filled (abc123, ., a.b, x%20y, -, __, %2F, ä), plus /extra, minus its last segment, a capitalised segment, a trailing slash, a // in two positions, the fixed unowned set (/me/permissions, /me/localization, /me/apps, /admin/definitely-not-a-route-1989, /config, /oidc/x, the base itself) — × 7 verbs: 4004 pairs over 572 paths (stock), 5278 over 754 (maximal).

Result: 0 divergences in the yield direction. Every (method, path) better-call routes is owned and every unowned path yields — /me/permissions, /me/localization, the control path, every SERVER_ONLY endpoint, every wrong verb, every extra or missing segment. Verified dead in practice: the module's '*'/default-POST branches (better-call registers an undefined method under rou3's any-method "", and rou3 has no * semantics) and its **-only wildcard handling — the live table has 0 endpoints with an undefined or * method and 0 paths with any syntax beyond plain :param, at both configs.

⚠️ One divergence class, in the swallow direction, bounded and named: a trailing-slash or double-slash spelling of an OWNED path (POST /api/v1/auth/delete-user/, /api/v1/auth//sign-in/social): better-call answers its unrouted 404 (router.mjs:39/44) while owns() says owned, because betterAuthEndpointPath strips trailing slashes and splitPath drops empty segments — 91+153 pairs at stock, 121+212 at maximal, and confirmed on the wire (POST /auth/delete-user/ → 404 at head vs 200 {} under the pre-fix yield behind the wildcard). This is something that used to fall through and now does not, so I state it against the bar plainly. I grade it non-blocking because: no route in the tree registers such a spelling (hono-server registers exact paths; Hono's strict mode keeps /x and /x/ distinct), nothing under /auth/me/* or any unowned path is touched, the change is observable only on a composition with a broad downstream mount, and there its effect is that a near-miss spelling stops answering the wildcard's vacuous 200 {} — the card's own complaint. But the changeset's sentence "paths better-auth does not own are yielded exactly as before" overstates on exactly these spellings. Requested before undraft — PM's call, text-only: carve the two spellings out of that sentence (e.g. "…except a trailing-slash or // spelling of a path it does own, which better-call refuses as unrouted and this table counts as owned"), and file the one-line alignment (mirror router.mjs:39/44 in ownsRoute, with a pin) as a follow-up card rather than moving code on this head.

#4088 fixture: the diff is +7 comment lines and one line replaced; no assertion loosened, ownsRoute derived from the file's own owned table. 6/6 green at head and under every ablation leg.

4 · SERVER_ONLY — verified, and it decides the nine correctly

At both configs auth.api carries exactly 9 /admin/oauth2/* endpoints and all nine have options.metadata.SERVER_ONLY: true (the other two SERVER_ONLY entries are the two /.well-known/* documents, which auth-plugin.ts mounts at the root itself). createRouter skips them (router.mjs:22), so their 404 is the unrouted one — measured through the shipped handleRequest: GET /admin/oauth2/resources404 len=0 ct=(none), the same signature as the nonexistent path — and the catch-all yields it (§2 row 3). Independent corroboration already in the tree: admin-route-nonadmin-refusal.dogfood.test.ts classifies precisely these nine as not-mounted. The e2e's correction of the first draft is right; the nine are out of scope, not a hole.

⭐ Side finding (not this PR's defect, relevant to #15920's premise): BETTER_AUTH_MOUNTED_SURFACE lists those nine rows as "what the catch-all exposes", but the conformance enumeration applies no SERVER_ONLY filter and better-call never routes them — so the publication inventory over-states by nine rows. Worth a card; the dogfood sweep and this PR now both know the distinction the ledger does not.

5 · Ablation — blob argument checked, legs re-run

The blob argument is airtight: at 62fce1ac5 and at HEAD, auth-plugin.ts = e7e45c8dc915bdc7a30adec07654e073689fad58 and better-auth-route-ownership.ts = c3b37cdc83aecc76e460226d858c8fac24ccbf7f — and in fact every one of the seven files is byte-identical across the two commits; the merge commit touches none of them.

Re-run in my worktree after the full suite had finished (no reader of src/ concurrent with the mutations), each mutation proved by blob delta + marker counts, restored under trap … EXIT INT TERM, restore proved by git diff HEAD empty and the blob back at HEAD's:

leg mutation blob suite result
1 yield condition → status === 404 5cab6b85… auth-catchall-yield.test.ts 3 failed / 5 passed — the three "does NOT yield an owned 404" tests; #4088 file stays 6/6
2 owns() → true (my shape keeps the empty-table guard) 88bbc62e… same 2 failed / 6 passed — the two yield-must-still-happen tests (the dev's unconditional true also reds the SERVER_ONLY pin, hence its 3)
3 :param → prefix match 56836810… better-auth-route-ownership.test.ts 1 failed / 9 passed — "does not let a :param swallow a longer or shorter path"; the e2e suite stays 8/8

Leg 3 is as thin as flagged — one assertion in one unit file. So I ran the live-table differential of §3 under the same mutation: 102 / 140 "other" divergences (every …/extra spelling under a parameterised or literal route reads as owned). The leg discriminates strongly; the in-tree pin for it is just narrow. Non-blocking suggestion: a conformance-style pin over the live auth.api ("every ledgered route with /extra appended is not owned") would make that leg red on measurement rather than on a hand-written slice.

6 · The #4088 fixture's JSON 404 — the dev's unfiled observation is correct, and not card-worthy

Measured: better-call's unrouted 404 is new Response(null, {status: 404, statusText: 'Not Found'}) (router.mjs:33–51) — on the wire len=0 ct=(none), in both my direct handleRequest probe and the boot. So the fixture's {message:'Not Found', code:'NOT_FOUND'} with application/json is indeed a convenience of that file. It does not weaken the pin: "returned verbatim" is body-agnostic pass-through, and a distinguishable body makes the verbatim assertion stronger. The new suite pins the measured shape. A one-line comment correction can ride on the next touch of that file; no card. (Bonus fact the wire gave up: the routed 404 — delete-user with a session — is len=0 ct=application/json, so the two 404s are distinguishable by content-type alone, which corroborates the ownership split from the outside.)

7 · Half 2 — split, not fixed; ledger untouched; reasoning holds

auth-route-ledger.ts blob e48823bab… at merge-base, 62fce1ac5 and HEAD — untouched. Measured, not re-read: stock auth.api has 9 /admin/ endpoints (all oauth2, all SERVER_ONLY) and the three routes answer the unrouted 404; at the maximal config all three are routed (owns() true, GET /admin/list-users401 UNAUTHENTICATED enveloped — a real answer). The ledger's own header pins publication at LEDGERED_PLUGIN_CONFIG and says a subset deployment is correct. Triage's "be prepared for fix-the-ledger" was answered by measurement in the other direction, and the split criterion triage pre-recorded is the one applied. Correct — with the §4 caveat about the nine SERVER_ONLY rows, which #15920's triage should see before it leans on "the ledger is not wrong".

8 · The two exit-3 gates — CI measured both on this head

Both jobs ran (not path-skipped) and both gate steps printed their own pass line on ee26bc613:

  • Build Core (job 101320683364, success) at 14:27:34Z: ✓ check:dual-build-cjs-loads — 103 published require entry point(s) across 66 package(s) load; 619 emitted CommonJS file(s) parse; …
  • Type Check · consumer gates (job 101320642184, success) at 14:27:31Z: ✓ check:published-readme-exports — 60 published document(s) across 79 workspace package(s); 215 import statement(s), … 200/200 @objectstack/ specifier(s) naming a workspace member. (the step lives in typecheck-consumers, not in Lint & Repo Gates — worth knowing for the next NOT MEASURED placement).

The dev's argument (no package.json/exports/build config/README moved; new module package-internal — confirmed, index.ts does not re-export it) was right, and it was correctly labelled an argument. All 33 check runs on this head are green or path-skipped; none failed.

9 · Other measurements

  • pnpm --filter @objectstack/plugin-auth test100 files / 2087 tests green, exit 0 (the dev's 99/2085 plus my scratch differential file, since deleted).
  • pnpm --filter @objectstack/plugin-auth typecheckexit 0, including check:test-typecheck ("test layer compiles").
  • The three subject suites at head: 3 files / 24 tests green.

10 · The open question — stated fairly; costs to add (⛔ not ruling)

  • A fair. B under-costed in one respect: Hono's app.routes lists registrations but cannot say which one will answer without re-running the matcher, and the @objectstack/hono adapter has no such seam at all — so B is a plugin-auth-only answer. C is correctly marked a reversal of vendor-admin-refusal-envelope.ts's narrowing 2; one cost to add: it also changes what plugin-auth 的终结式 catch-all 吞掉 /api/v1/auth/* 下别人的路由 —— console 权限层目前靠 kernel.use() 顺序才活着 #4088's own text guarantees ("the wire shape for a genuinely unclaimed auth path is unchanged") and would move that fixture's verbatim-404 pin.
  • ⭐ Orthogonal to A/B/C and not on the dev's list: the @objectstack/hono adapter's /auth/* mount yields every 404 too (adapters/hono 的 ${prefix}/auth/* 与 ${prefix}/storage/* 是终结式通配 —— 与 #4088 同一缺陷,只是这个包在仓内没有消费者 #4117, packages/adapters/hono/src/index.ts:395–405), untouched here. Its downstream is the terminal dispatcher, so an owned 404 handed on there comes back as the dispatcher's 404 envelope — status kept, better-auth's answer replaced. Same defect class, narrower effect, different package; the adapter's own header says the cloud control plane annotates the app it returns. Deserves a follow-up card with the same ownership check, whatever is ruled on A/B/C. Non-blocking for this PR: different package, the plugin-auth changeset names registerAuthRoutes explicitly, and nothing there produces a 200.

Checklist (derived judgments · semver · flags)

Public surface: AuthManager.ownsRoute(request) is a new public method on an exported class (index.ts:12 export * from './auth-manager.js') — additive; AuthService (spec) is untouched (still handleRequest only) and the catch-all calls the concrete manager, so no contract widening; buildBetterAuthRouteOwnership is package-internal. Accept-set change: the catch-all no longer yields a 404 on a (method, path) better-call routes — exactly as the changeset states, with the one spelling carve-out of §3. ② Semver: patch is consistent with precedent for a plugin-auth wire-behaviour fix carrying an upgrade note (plugin-auth-find-envelope-limbs.md), and ADR-0087 sees no breaking marker. ③ Flags: one open_questions entry, correctly escalated not decided; two out_of_scope_findings, one filed (#15920) and one correctly unfiled (§6). Findings I add: the adapter's identical yield (§10), the nine SERVER_ONLY rows in the mounted-surface ledger (§4), the spelling divergence (§3), the thin leg-3 pin (§5).

Verdict — PASS

The card's headline is falsified on the real framework boot with the comparison rows reproducing exactly; the yield mechanism is demonstrated through the shipped handler in both directions, including the boot-level ablation that makes POST /auth/delete-user answer a wildcard's 200 {} under the pre-fix condition and 404 at head; owns() agrees with better-call's own router construction on the live table at both configurations with zero divergences in the yield direction; the nine SERVER_ONLY endpoints are correctly out of scope; #4088's surface is intact on every real route; the ledger is untouched and its reading holds; the two exit-3 gates are CI-measured green on this head. Requested before undraft, PM's call and text-only: the changeset's "yielded exactly as before" sentence per §3. Non-blocking follow-ups: the ownsRoute trailing-slash/// alignment (§3), the adapter's yield (§10), the nine ledger rows (§4), a live-table pin for leg 3 (§5). ⛔ Nothing pushed, undrafted or merged; no git stash. Worktree left provably clean at ee26bc613: git status --short 0 lines, git diff HEAD 0 lines, all seven files' blobs equal HEAD's. Scratch scripts and logs suffixed -15918-review (24 files); both scratch test files deleted before the typecheck and the final proof.


Generated by Claude Code — reviewer for the domain:services PM seat (PM session 03324ae2-0f5b-5ad2-8a2e-cf4aaff5a909)

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y


Generated by Claude Code

… out of the "yielded as before" claim

Text-only; no source, pins or behaviour move on this commit.

The changeset said a path better-auth does not own is "yielded exactly as
before". A live differential built for the clause-② review — better-call
1.4.0's `createRouter` plus `processRequest`'s pre-checks, re-run over rou3
0.9.2 against the real `auth.api` at the stock and the maximal configuration,
4004 + 5278 (method, path) pairs — found 0 divergences in the yield direction,
so that half of the claim is now measured rather than asserted. It also found
the sentence is not unconditional in the other direction.

A TRAILING-SLASH OR DOUBLED-SLASH SPELLING OF A PATH BETTER-AUTH DOES OWN —
`/api/v1/auth/delete-user/`, `/api/v1/auth//sign-in/social` — is claimed by
this ownership table rather than yielded. better-call refuses those spellings
as unrouted: it returns its 404 on a `//` and on trailing-slash parity before
it ever looks the route up, while `betterAuthEndpointPath` strips the trailing
slash and `splitPath` drops empty segments, so the table counts them as owned.
On a composition with a broad downstream mount, such a spelling now answers
better-auth's 404 instead of that mount's response — measured on the wire:
`POST /auth/delete-user/` answers 404 at this head where the pre-fix yield gave
the wildcard's `200 {}`. Bounded at 91 + 153 pairs (stock) and 121 + 212
(maximal).

Left as it is, deliberately: no route in this repo registers a spelling of that
shape, nothing under `/auth/me/*` or any genuinely unowned path is touched, and
the effect where it does show is that a near-miss spelling stops answering a
foreign mount's vacuous 200 — the direction this change argues for. Aligning
`ownsRoute` with better-call's own pre-checks, with a pin, is a follow-up card
rather than a source change on this head.

The changeset feeds release notes, which is why the sentence had to stop being
unqualified even though the divergence is graded non-blocking.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
@os-warren
os-warren marked this pull request as ready for review September 5, 2026 15:06

Copy link
Copy Markdown
Collaborator Author

PM verification of the carve-out — undrafted and armed

Verified at head 81226d5ba5ac186a8adaae34176597fb1e7dbb02.

⚠️ The "source untouched" check needed a different method this round, and the author said so

The author disclosed that its origin/main merge pulled in an unrelated hunk, so "only the changeset moved" is true of its authored edits but not of the tree. ⇒ A plain byte-identity check against the reviewed head ee26bc613 would have failed, and failing it would have been the right answer for the wrong reason. So I checked what the difference actually is:

⇒ The review's PASS still stands on the code it reviewed: the merge added a sibling registration and touched nothing the review examined. ⭐ Worth noting the author volunteered this rather than letting "text-only round" stand unqualified — the check I would otherwise have run would have produced a confusing red.

The carve-out

Unconditional claim gone ("yielded exactly as before"0 occurrences); both spellings named (1 each for the trailing-slash and doubled-slash forms); an impossible control string → 0. check-adr-0087-registration exit 0, with --self-test exit 0 as the author's control. The PR's own file set against its merge-base is back to 7 files — the merged hunk is part of the base now, so it correctly drops out.

The wording does more than qualify: it names the mechanism on both sides (better-call refuses on a // and on trailing-slash parity before it looks the route up, while the ownership table strips the trailing slash and drops empty segments), gives concrete example paths, states the wire consequence, and bounds it three ways — only those two spellings, only of a path better-auth already owns, and only where such a mount exists. The alignment is tracked as a follow-up rather than smuggled in.

⭐ The author's own diagnosis of the pattern, which is the most useful thing in this round

Unprompted, on the five instances this session:

all five were prose claiming invariance across a set I had only sampled. The specific habit that produced this one is that I wrote "yielded exactly as before" from the design intent of the change rather than from any enumeration — my pins covered a handful of hand-picked paths, and nothing in my process asked "over which set is this quantified, and did I measure that set?" before the sentence went into a file that feeds release notes.

And the guard it proposes:

treat every unqualified "unchanged / as before / in both directions" in a changeset as requiring either a named measured set or an explicit carve-out.

⇒ That is a sharper rule than "read the prose", because it is mechanically checkable: those phrases are greppable, and each occurrence either cites a set or does not. All five of this session's instances (#15720's rider, #15787's cost sentence, #15838's "unchanged in both directions", #15903's changeset reason, and this one) would have been caught by it at authoring time rather than at review. Recorded here for the maintainer; ⛔ a changeset-gate change is not this seat's to make.

Still NOT MEASURED, and correctly not claimed

Three gates answered exit 3 = PREREQUISITE NOT MET on a fresh unbuilt worktree — check:dual-build-cjs-loads, check:published-readme-exports, check:dts-closure. ⛔ None reported as a pass. The review independently confirmed the first two CI-green on this head, with check:published-readme-exports living in Type Check · consumer gates rather than Lint. The gate family was re-derived clean with no stale-tree warning and is identical to last round's 50; 47 exit 0.

⚠️ The author ran the full derived family rather than only the changeset-matching gates, precisely because the merge moved source it did not author. That is the right call and not the one a "text-only round" would suggest.

This PR does not close #15417. The card's headline was measured false on a framework boot, the wildcard-yield question (options A/B/C, where C is an explicit reversal of vendor-admin-refusal-envelope.ts's recorded narrowing) is the maintainer's, and #15928 records the same unconditioned yield in the @objectstack/hono adapter.

Undrafted and auto-merge armed.


Generated by Claude Code

@os-warren
os-warren enabled auto-merge September 5, 2026 15:06
@os-warren
os-warren added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit d4c2cb1 Sep 5, 2026
38 checks passed
@os-warren
os-warren deleted the claude/issue-15417-auth-catchall-404 branch September 5, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants