From 62fce1ac500c2609589ebb3cd488904666f72795 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 13:36:34 +0000 Subject: [PATCH 1/2] fix(plugin-auth): the auth catch-all yields only a 404 that disclaims ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../auth-catchall-owned-404-not-yielded.md | 21 ++ .../src/auth-catchall-fallthrough.test.ts | 9 +- .../src/auth-catchall-yield.test.ts | 224 ++++++++++++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 43 ++++ .../plugins/plugin-auth/src/auth-plugin.ts | 15 +- .../src/better-auth-route-ownership.test.ts | 100 ++++++++ .../src/better-auth-route-ownership.ts | 153 ++++++++++++ 7 files changed, 563 insertions(+), 2 deletions(-) create mode 100644 .changeset/auth-catchall-owned-404-not-yielded.md create mode 100644 packages/plugins/plugin-auth/src/auth-catchall-yield.test.ts create mode 100644 packages/plugins/plugin-auth/src/better-auth-route-ownership.test.ts create mode 100644 packages/plugins/plugin-auth/src/better-auth-route-ownership.ts diff --git a/.changeset/auth-catchall-owned-404-not-yielded.md b/.changeset/auth-catchall-owned-404-not-yielded.md new file mode 100644 index 0000000000..6ceafdcc58 --- /dev/null +++ b/.changeset/auth-catchall-owned-404-not-yielded.md @@ -0,0 +1,21 @@ +--- +"@objectstack/plugin-auth": patch +--- + +The auth catch-all yields only a 404 that disclaims ownership — better-auth's own 404 answers can no longer be replaced by another route's + +`registerAuthRoutes` mounts one catch-all over the whole auth namespace (`rawApp.all(`${basePath}/*`)`), and since #4088 that catch-all 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 those two are reachable only when HonoServerPlugin happens to register first. + +What the yield could not express is **which** 404 may be handed on, because it had only the status to go on. So every 404 was yielded, including the ones that are better-auth's own answer on a path its router serves. Measured with the shipped handler on a real Hono app: add one broad downstream mount — `app.all('/api/v1/*', c => c.json({}))`, the shape a composition adds — and + +``` +POST /api/v1/auth/delete-user -> 200 {} +``` + +where better-auth answered 404 because `user.deleteUser` is deliberately unconfigured. That route is not hypothetical: `auth-route-ledger.ts` carries it under the `disabled` disposition precisely because it is published and refused — and the same holds for every 404 a routed endpoint produces for a bad token, an unknown id, or an admin family the deployment does mount. Those answers were all 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 `auth-route-ledger.conformance.test.ts` reads and the same one the `/admin/` dogfood sweep derives from, because there is no route table to enumerate by hand; matching mirrors better-call's own `createRouter` walk, including its `SERVER_ONLY` skip and its `:param` syntax. That skip is load-bearing rather than cosmetic: measured on the stock boot, the nine `/admin/oauth2/*` endpoints are in `auth.api` and every one carries `SERVER_ONLY: true`, so better-call never routes them — their 404 is an unrouted one and stays yieldable, because ownership is "does better-call route this", not "is it in `auth.api`". An ownership table that cannot be built answers "not owned", so an enumeration failure degrades to the previous behaviour rather than taking the #4088 surface down with it. + +**The mount is untouched.** It still claims exactly `${basePath}/*` and still forwards every request under it to better-auth. What narrowed is only which 404 may be handed on. + +**Upgrade note — a composition that mounts a route matching paths under the auth base path may see a 404 where it previously saw its own answer.** Affected: deployments that register a route which also matches `/api/v1/auth/...` — most often a broad wildcard over the API prefix — mounted *after* AuthPlugin. Before this release, any request to a path better-auth serves but answers 404 on (a switched-off capability, not an unknown path) was passed to that route and the caller received *its* response, commonly `200` with an empty object. From this release the caller receives better-auth's 404. Callers that treated such a response as success — `res.ok`, `status === 200`, "no error thrown" — will start seeing the refusal that was always the real answer; that is the point of the change, and the wire shape they now get is the one a deployment without the extra mount has always returned. Nothing to do if you mount no such route: paths better-auth does **not** own are yielded exactly as before, so `/auth/me/permissions`, `/auth/me/localization` and any other sibling route under the auth prefix are unaffected in either registration order. diff --git a/packages/plugins/plugin-auth/src/auth-catchall-fallthrough.test.ts b/packages/plugins/plugin-auth/src/auth-catchall-fallthrough.test.ts index 503f64b230..51ecf87807 100644 --- a/packages/plugins/plugin-auth/src/auth-catchall-fallthrough.test.ts +++ b/packages/plugins/plugin-auth/src/auth-catchall-fallthrough.test.ts @@ -60,7 +60,14 @@ async function mountCatchAll(owned: Record Response>) { headers: { 'Content-Type': 'application/json' }, }); }); - (plugin as any).authManager = { handleRequest }; + // [#15417] The catch-all now asks the auth manager whether better-auth owns + // the path before it yields, so this stand-in has to answer that too — the + // fixture is a fake `AuthManager`, and this is part of the contract it + // stands in for. Ownership is derived from the SAME `owned` table above, so + // the file keeps meaning exactly what its title says: the paths better-auth + // does not own are the ones that get yielded. + const ownsRoute = async (req: Request) => Object.hasOwn(owned, new URL(req.url).pathname); + (plugin as any).authManager = { handleRequest, ownsRoute }; const httpServer: any = { getRawApp: () => app, getPort: () => 0 }; (plugin as any).registerAuthRoutes(httpServer, ctx); diff --git a/packages/plugins/plugin-auth/src/auth-catchall-yield.test.ts b/packages/plugins/plugin-auth/src/auth-catchall-yield.test.ts new file mode 100644 index 0000000000..8d502c431b --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-catchall-yield.test.ts @@ -0,0 +1,224 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15417 — WHICH 404 the auth catch-all is allowed to yield. + * + * `auth-catchall-fallthrough.test.ts` (#4088) pins that the catch-all yields at + * all. This file pins the other edge: it may only yield a 404 that DISCLAIMS + * ownership. A 404 from a path better-auth's own router serves is its ANSWER — + * a switched-off capability — and handing that to the rest of the chain is how + * it comes back as somebody else's `200 {}`. + * + * ── The measurement this file exists for ──────────────────────────────────── + * + * #15417 reported `POST /api/v1/auth/admin/` answering `200 {}` on + * a cloud composition, with a nonexistent path as the control. Reproduced on a + * framework-side boot (`@objectstack/verify` + the showcase stack, both with + * better-auth's admin plugin off and on), that path answers **404** — bodyless, + * no content-type — so the framework does not produce the reported status on + * its own. What does produce it is the yield: register ONE broad downstream + * mount after the catch-all — `app.all('/api/v1/*', c => c.json({}))`, the + * shape a composition adds — and the same request comes back `200 {}`, because + * the catch-all handed it on and the wildcard answered. + * + * That is a framework-side defect regardless of who mounts the wildcard, + * because the request handed on need not be an unknown path at all: + * `delete-user` answers 404 by the ledger's `disabled` disposition, and so does + * every routed endpoint that 404s on a bad token or an unknown id. Those + * answers were all up for grabs. Confirmed end-to-end on the framework-side + * boot with the wildcard installed: `POST /api/v1/auth/delete-user` now + * answers 404 where the wildcard's `200 {}` used to stand. + * + * ⚠️ Ownership is "does better-call ROUTE this", not "is it in `auth.api`". + * Measured on the stock boot: the nine `/admin/oauth2/*` endpoints are in + * `auth.api` and every one carries `SERVER_ONLY: true`, so `createRouter` skips + * them and their 404 is an unrouted one — they stay yieldable, and the pin + * below says so. + * + * ── Why the fixture stubs better-auth, and what it does NOT stub ──────────── + * + * Same seam as the #4088 file: `handleRequest` is a path table so the test + * controls exactly which paths the vendor claims, and the real + * `registerAuthRoutes` runs on a real Hono app so the assertions are about the + * shipped handler. The ownership decision is NOT stubbed — `ownsRoute` here + * runs the real `buildBetterAuthRouteOwnership` over a fake `auth.api`, so the + * matcher under test is the shipped one. + * + * The stub's 404 is `new Response(null, { status: 404 })` — bodyless, no + * content-type — because that is what better-call 1.4.0 really returns for an + * unrouted path (`dist/router.mjs`), and what the framework-side boot measured + * on the wire. The #4088 fixture's JSON 404 is a convenience of that file. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { Hono } from 'hono'; +import { AuthPlugin } from './auth-plugin'; +import { buildBetterAuthRouteOwnership } from './better-auth-route-ownership'; +import type { PluginContext } from '@objectstack/core'; + +const BASE = '/api/v1/auth'; + +/** What better-call returns for a path it does not route: bodyless, no content-type. */ +const unrouted404 = () => new Response(null, { status: 404, statusText: 'Not Found' }); + +/** + * Mount the real route registration on a real Hono app. + * + * @param api the fake `auth.api` the REAL ownership matcher reads + * @param answers path -> Response for the paths better-auth answers + */ +async function mountCatchAll( + api: Record, + answers: Record Response>, +) { + const app = new Hono(); + const ctx: PluginContext = { + registerService: vi.fn(), + getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)), + getServices: vi.fn(() => new Map()), + hook: vi.fn(), + trigger: vi.fn(), + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + getKernel: vi.fn(), + } as any; + + const plugin = new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long!!' }); + await plugin.init(ctx); + + const ownership = buildBetterAuthRouteOwnership(api as any); + const handleRequest = vi.fn(async (req: Request) => { + const make = answers[new URL(req.url).pathname]; + return make ? make() : unrouted404(); + }); + (plugin as any).authManager = { + handleRequest, + // The shipped matcher, over the fake table — only the endpoint-path + // derivation is inlined here (AuthManager's own is private). + ownsRoute: async (req: Request) => + ownership.owns(req.method, new URL(req.url).pathname.slice(BASE.length)), + }; + + (plugin as any).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx); + return { app, handleRequest }; +} + +/** The shape a composition adds: one wildcard over the whole API prefix. */ +const addDownstreamWildcard = (app: Hono) => app.all('/api/v1/*', (c) => c.json({})); + +describe('#15417: the catch-all yields only a 404 that disclaims ownership', () => { + it('does NOT yield a 404 from a path better-auth OWNS — even with a wildcard downstream', async () => { + // `delete-user` is published and answers 404 because `user.deleteUser` is + // deliberately unconfigured — `auth-route-ledger.ts`'s `disabled` + // disposition. That 404 is an ANSWER and must reach the caller. + const { app } = await mountCatchAll( + { deleteUser: { path: '/delete-user', options: { method: 'POST' } } }, + { [`${BASE}/delete-user`]: () => new Response(null, { status: 404 }) }, + ); + addDownstreamWildcard(app); + + const res = await app.request(`http://localhost${BASE}/delete-user`, { method: 'POST' }); + + expect(res.status).toBe(404); + expect(await res.text()).toBe(''); + }); + + it('does not yield an owned 404 on a PARAMETERISED path either', async () => { + // `/callback/:id` is routed and parameterised; a 404 from it is an answer. + const { app } = await mountCatchAll( + { callback: { path: '/callback/:id', options: { method: 'GET' } } }, + { [`${BASE}/callback/github`]: () => new Response(null, { status: 404 }) }, + ); + addDownstreamWildcard(app); + + const res = await app.request(`http://localhost${BASE}/callback/github`); + + expect(res.status).toBe(404); + }); + + it('DOES yield a SERVER_ONLY endpoint\'s 404 — better-call never routed it', async () => { + // Measured on the stock boot: all nine `/admin/oauth2/*` endpoints are in + // `auth.api` carrying `SERVER_ONLY: true`. `createRouter` skips them, so the + // 404 the wire sees is an unrouted one and yielding it is correct. Were the + // table to trust `auth.api` wholesale instead of mirroring that skip, this + // route would stop being yieldable and a composition serving it downstream + // would break. + const { app } = await mountCatchAll( + { + adminListOAuthResources: { + path: '/admin/oauth2/resources', + options: { method: 'GET', metadata: { SERVER_ONLY: true } }, + }, + } as any, + {}, + ); + app.get(`${BASE}/admin/oauth2/resources`, (c) => c.json({ from: 'sibling' })); + + const res = await app.request(`http://localhost${BASE}/admin/oauth2/resources`); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ from: 'sibling' }); + }); + + it('STILL yields a 404 from a path better-auth does not own — #4088 intact', async () => { + // The route plugin-hono-server mounts from its own kernel:ready hook, in + // the registration order that used to 404. Nothing may make this red. + const { app } = await mountCatchAll({ getSession: { path: '/get-session', options: { method: 'GET' } } }, {}); + app.get(`${BASE}/me/permissions`, (c) => c.json({ authenticated: true, from: 'hono-plugin' })); + + const res = await app.request(`http://localhost${BASE}/me/permissions`); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ authenticated: true, from: 'hono-plugin' }); + }); + + it('an unknown tail with nothing downstream still answers better-auth\'s 404', async () => { + // The control the card could not run from outside. Unchanged by #15417: + // the framework already answered 404 here, and still does. + const { app } = await mountCatchAll({ getSession: { path: '/get-session', options: { method: 'GET' } } }, {}); + + const res = await app.request(`http://localhost${BASE}/admin/definitely-not-a-route-1989`, { method: 'POST' }); + + expect(res.status).toBe(404); + }); + + it('a path better-auth owns is not yielded even when the DOWNSTREAM route is specific', async () => { + // Precedence still favours the namespace owner (the #4088 file pins this + // for 2xx; here it is pinned for the vendor's own 404 answer). + const { app } = await mountCatchAll( + { deleteUser: { path: '/delete-user', options: { method: 'POST' } } }, + { [`${BASE}/delete-user`]: () => new Response(null, { status: 404 }) }, + ); + app.post(`${BASE}/delete-user`, (c) => c.json({ hijacked: true })); + + const res = await app.request(`http://localhost${BASE}/delete-user`, { method: 'POST' }); + + expect(res.status).toBe(404); + expect(await res.text()).toBe(''); + }); + + it('ownership is per METHOD: the same path on a verb better-auth does not serve still yields', async () => { + const { app } = await mountCatchAll( + { listUsers: { path: '/admin/list-users', options: { method: 'GET' } } }, + {}, + ); + app.post(`${BASE}/admin/list-users`, (c) => c.json({ from: 'sibling' })); + + const res = await app.request(`http://localhost${BASE}/admin/list-users`, { method: 'POST' }); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ from: 'sibling' }); + }); + + it('still forwards to better-auth exactly once per request', async () => { + const { app, handleRequest } = await mountCatchAll( + { deleteUser: { path: '/delete-user', options: { method: 'POST' } } }, + { [`${BASE}/delete-user`]: () => new Response(null, { status: 404 }) }, + ); + addDownstreamWildcard(app); + handleRequest.mockClear(); + + await app.request(`http://localhost${BASE}/delete-user`, { method: 'POST' }); + + expect(handleRequest).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 30ee873658..e0e47f9e18 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -59,6 +59,10 @@ import { } from './auth-session-audit.js'; import { SESSION_ERASURE_PATHS } from './session-tombstone.js'; import { envelopeVendorAdminRefusal } from './vendor-admin-refusal-envelope.js'; +import { + buildBetterAuthRouteOwnership, + type BetterAuthRouteOwnership, +} from './better-auth-route-ownership.js'; import { ADMIN_SESSION_COOKIE_KEY, STOP_IMPERSONATING_PATH, @@ -5380,6 +5384,45 @@ export class AuthManager { return response; } + /** + * [#15417] Does better-auth ROUTE this request — i.e. is the path one its own + * router owns, whatever it then answers? + * + * The auth catch-all yields the request to the rest of the Hono chain when + * better-auth answers 404 (#4088), and it needs this to tell the two very + * different 404s apart: "I do not serve this path" (yieldable — that is how + * `plugin-hono-server`'s `/auth/me/*` routes stay reachable in either + * registration order) from "I serve it and the answer is 404" (NOT yieldable + * — a disabled capability's refusal is an answer, and handing it to a + * downstream wildcard is how it becomes `200 {}`). The mechanism, the + * measurement and the matching rules live in + * `better-auth-route-ownership.ts`. + * + * Keyed on the live instance and rebuilt whenever that instance is replaced, + * so a re-created auth (config change, test re-boot) never answers from a + * stale table. Returns `false` — the yielding, pre-#15417 answer — for any + * request it cannot decide, so a failure to enumerate can never take the + * #4088 surface down with it. + */ + async ownsRoute(request: Request): Promise { + const endpointPath = this.betterAuthEndpointPath(request); + if (endpointPath === undefined) return false; + try { + const auth = await this.getOrCreateAuth(); + if (this.routeOwnershipFor !== auth || !this.routeOwnership) { + this.routeOwnership = buildBetterAuthRouteOwnership((auth as any)?.api); + this.routeOwnershipFor = auth; + } + return this.routeOwnership.owns(request.method, endpointPath); + } catch { + return false; + } + } + + /** Memoized `auth.api` ownership table, and the instance it was built from. */ + private routeOwnership?: BetterAuthRouteOwnership; + private routeOwnershipFor?: unknown; + /** * The better-auth endpoint path (`/admin/remove-user`) this request addresses, * or `undefined` when it is not under the configured `basePath`. diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index e22b9f8631..e7e45c8dc9 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -2968,7 +2968,20 @@ export class AuthPlugin implements Plugin { // Forward the original request to better-auth handler const response = await this.authManager!.handleRequest(c.req.raw); - if (response.status === 404) { + // [#15417] Yield only a 404 that DISCLAIMS OWNERSHIP. `ownsRoute` asks + // better-auth's live `auth.api` — the same seam the route ledger and the + // `/admin/` dogfood sweep read — whether its own router serves this + // path; see `better-auth-route-ownership.ts` for the mechanism and the + // measurement. A 404 from a path it DOES serve is its answer (a + // switched-off capability such as `delete-user`, a bad token, an + // unknown id) and must reach the caller, exactly like the + // 401/403 the block below already refuses to yield. Measured with the + // shipped handler on a real Hono app: with one broad downstream mount + // in the chain — the shape a composition adds — such an answer used to + // come back `200 {}`, which is the silent-success shape #15417 is + // about. ⛔ The mount is untouched and still claims `${basePath}/*`; + // what narrowed is which 404 may be handed on. + if (response.status === 404 && !(await this.authManager!.ownsRoute(c.req.raw))) { await next(); // A non-404 downstream means something else answered — hand that back. // NOT `c.finalized`: reaching the end of the chain with nothing diff --git a/packages/plugins/plugin-auth/src/better-auth-route-ownership.test.ts b/packages/plugins/plugin-auth/src/better-auth-route-ownership.test.ts new file mode 100644 index 0000000000..4db030ef39 --- /dev/null +++ b/packages/plugins/plugin-auth/src/better-auth-route-ownership.test.ts @@ -0,0 +1,100 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #15417 — the ownership table the auth catch-all's yield is conditioned on. +// +// The table is only as good as its agreement with better-call's own router +// construction, so the cases below are the ones that construction makes +// meaningful: the three skips it performs, the `:param` syntax it registers, +// and the method set it keys on. The paths are real better-auth paths taken +// from `auth-route-ledger.ts`'s `BETTER_AUTH_MOUNTED_SURFACE`, so a rename +// upstream shows up here as well as there. + +import { describe, it, expect } from 'vitest'; +import { buildBetterAuthRouteOwnership } from './better-auth-route-ownership'; + +/** A slice of a real `auth.api`, in the shape better-auth exposes. */ +const API = { + getSession: { path: '/get-session', options: { method: ['GET', 'POST'] } }, + listUsers: { path: '/admin/list-users', options: { method: 'GET' } }, + setRole: { path: '/admin/set-role', options: { method: 'POST' } }, + deleteUser: { path: '/delete-user', options: { method: 'POST' } }, + resource: { path: '/admin/oauth2/resources/:identifier', options: { method: 'GET' } }, + resourceClient: { + path: '/admin/oauth2/resources/:identifier/clients/:client_id', + options: { method: 'POST' }, + }, + callback: { path: '/callback/:id', options: { method: ['GET', 'POST'] } }, +}; + +describe('#15417 better-auth route ownership', () => { + const table = buildBetterAuthRouteOwnership(API as any); + + it('reads a non-empty table (a zero table would make every caller yield)', () => { + expect(table.size).toBeGreaterThan(0); + }); + + it('owns a literal path on a method it declares', () => { + expect(table.owns('GET', '/admin/list-users')).toBe(true); + expect(table.owns('POST', '/admin/set-role')).toBe(true); + }); + + it('owns every method in a multi-method declaration, and only those', () => { + expect(table.owns('GET', '/get-session')).toBe(true); + expect(table.owns('POST', '/get-session')).toBe(true); + // The method is part of ownership: better-call routes per (method, path). + expect(table.owns('DELETE', '/get-session')).toBe(false); + }); + + it('is case-insensitive about the verb, since the wire is not', () => { + expect(table.owns('get', '/admin/list-users')).toBe(true); + }); + + it('does NOT own a path nothing declares — the #4088 yield still applies', () => { + // The two routes plugin-hono-server mounts. If these ever read `true` the + // console's whole permission layer becomes order-dependent again. + expect(table.owns('GET', '/me/permissions')).toBe(false); + expect(table.owns('GET', '/me/localization')).toBe(false); + expect(table.owns('POST', '/admin/definitely-not-a-route-1989')).toBe(false); + }); + + it('fills a `:param` with exactly one non-empty segment', () => { + expect(table.owns('GET', '/admin/oauth2/resources/abc123')).toBe(true); + expect(table.owns('POST', '/admin/oauth2/resources/abc123/clients/cli_9')).toBe(true); + expect(table.owns('GET', '/callback/github')).toBe(true); + }); + + it('does not let a `:param` swallow a longer or shorter path', () => { + // A param is ONE segment. Were it a prefix match, every unknown tail under + // a parameterised route would read as owned and stop being yielded. + expect(table.owns('GET', '/admin/oauth2/resources/abc123/extra')).toBe(false); + expect(table.owns('GET', '/admin/oauth2/resources')).toBe(false); + expect(table.owns('GET', '/callback')).toBe(false); + }); + + it('skips SERVER_ONLY endpoints, exactly as better-call\'s createRouter does', () => { + const t = buildBetterAuthRouteOwnership({ + hidden: { path: '/internal-only', options: { method: 'POST', metadata: { SERVER_ONLY: true } } }, + } as any); + // Not routed by better-call ⇒ not owned here ⇒ still yieldable. + expect(t.size).toBe(0); + expect(t.owns('POST', '/internal-only')).toBe(false); + }); + + it('skips entries with no options or no path, as createRouter does', () => { + const t = buildBetterAuthRouteOwnership({ + noOptions: { path: '/x' }, + noPath: { options: { method: 'POST' } }, + } as any); + expect(t.size).toBe(0); + }); + + it('an unresolvable api yields an empty table that owns nothing', () => { + // The safe direction: an enumeration failure must degrade to the + // pre-#15417 behaviour (yield), never to "owns everything". + for (const bad of [undefined, null, {}]) { + const t = buildBetterAuthRouteOwnership(bad as any); + expect(t.size).toBe(0); + expect(t.owns('POST', '/admin/set-role')).toBe(false); + } + }); +}); diff --git a/packages/plugins/plugin-auth/src/better-auth-route-ownership.ts b/packages/plugins/plugin-auth/src/better-auth-route-ownership.ts new file mode 100644 index 0000000000..c3b37cdc83 --- /dev/null +++ b/packages/plugins/plugin-auth/src/better-auth-route-ownership.ts @@ -0,0 +1,153 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Does better-auth OWN this path? (#15417) + * + * ## Why the question exists + * + * `registerAuthRoutes` mounts one catch-all over the whole auth namespace — + * `rawApp.all(`${basePath}/*`)` — and since #4088 that catch-all is NOT + * terminal: when better-auth answers 404 the handler calls `next()` and lets + * whatever else matched answer instead. #4088 needed that, and still does: + * `plugin-hono-server` mounts `/auth/me/permissions` and + * `/auth/me/localization` from its own `kernel:ready` hook, and before the + * yield those two were reachable only if HonoServerPlugin happened to register + * first. The yield made a load-bearing surface independent of `kernel.use()` + * order. + * + * What #4088 could not express is WHICH 404 may be yielded, because it had only + * the status to go on. So today every 404 is yielded, including the ones that + * are better-auth's own ANSWER on a path it owns. Measured on this tree + * (`plugin-auth/src/auth-catchall-yield.test.ts`, and the probe that produced + * this module) with the shipped handler on a real Hono app: register one broad + * downstream mount — `app.all('/api/v1/*', c => c.json({}))`, the shape a + * composition adds — and + * + * POST /api/v1/auth/delete-user -> 200 {} + * + * where better-auth answered 404 because `user.deleteUser` is deliberately + * unconfigured. That route is not hypothetical: `auth-route-ledger.ts` carries + * it under the `disabled` disposition precisely because it is published and + * refused. A 404 that says "this capability is switched off" is a real answer, + * and it was up for grabs — as is every 404 a routed endpoint produces for a + * bad token, an unknown id, or an admin family the deployment does mount. + * + * ⚠️ MEASURED, and it is the reason this module skips `SERVER_ONLY` rather + * than trusting `auth.api` wholesale: the nine `/admin/oauth2/*` endpoints are + * IN `auth.api` on the stock boot and all nine carry `SERVER_ONLY: true`, so + * better-call never routes them and their 404 is an unrouted one. Those stay + * yieldable, correctly — "in `auth.api`" is not the same question as "routed", + * and only the second one is ownership. + * + * ## The seam, and why it is this one + * + * There is no route table to enumerate by hand — `auth-route-ledger.ts` says so + * in its own header, and it is the reason that ledger exists: better-auth is a + * third-party dependency on its own release cadence, its endpoint set varies + * with `AuthPluginConfig`, and **the live `auth.api` instance IS the route + * table**. Every endpoint object carries `.path` and `.options.method`. That is + * the seam `auth-route-ledger.conformance.test.ts` reads, the seam + * `admin-route-nonadmin-refusal.dogfood.test.ts` derives its sweep from, and so + * it is the seam this module asks. + * + * ⛔ This does NOT narrow the mount. The catch-all still claims exactly + * `${basePath}/*` and still forwards every request under it to better-auth, + * unchanged. What narrows is only the YIELD: a 404 from a path better-auth owns + * is its answer and is returned; a 404 from a path it does not own is a + * disclaimer of ownership and is yielded, exactly as #4088 wrote it. Neither + * `/auth/me/permissions` nor `/auth/me/localization` is a better-auth endpoint, + * so both keep winning in either registration order — the property #4088 exists + * to hold. + * + * ## Matching mirrors better-call's own router construction + * + * `createRouter` (better-call 1.4.0 `dist/router.mjs`) walks `Object.values( + * endpoints)`, skips any endpoint without `options` or `path`, skips + * `options.metadata.SERVER_ONLY`, and registers one route per declared method. + * This module does the same walk over the same objects, so the set it calls + * "owned" is the set better-call routed. Path params are `rou3` syntax: + * `:name` matches exactly one non-empty segment (`/reset-password/:token`), + * and a trailing `**` matches the rest. + * + * ⚠️ Ownership is about the PATH TABLE, never about the answer. This module + * reads no bodies, makes no authorization decision, and cannot turn a refusal + * into an admission or the reverse — it only decides whether the catch-all is + * still allowed to hand the request to somebody else. + */ + +/** The shape of a better-auth endpoint object, as `auth.api` exposes it. */ +export interface BetterAuthEndpointLike { + path?: string; + options?: { + method?: string | readonly string[]; + metadata?: { SERVER_ONLY?: boolean } | undefined; + }; +} + +/** One routed (method, path-pattern) pair, pre-split for matching. */ +interface OwnedRoute { + /** Upper-case verbs this pattern answers, or `'*'` for any. */ + methods: ReadonlySet; + /** `/admin/oauth2/resources/:identifier` -> ['admin','oauth2','resources',':identifier'] */ + segments: readonly string[]; + /** True when the pattern ends in `**` and therefore matches a longer path. */ + matchesRest: boolean; +} + +/** Answers "does better-auth route this?" for one better-auth instance. */ +export interface BetterAuthRouteOwnership { + /** Number of routed (method, path) pairs — 0 means the table did not resolve. */ + readonly size: number; + /** + * `endpointPath` is better-auth's own `ctx.path` spelling (`/admin/set-role`), + * i.e. what `AuthManager.betterAuthEndpointPath` returns — never the wire path. + */ + owns(method: string, endpointPath: string): boolean; +} + +const splitPath = (p: string): string[] => p.split('/').filter((s) => s.length > 0); + +/** + * Build the ownership table from a live `auth.api`. + * + * Returns a table of size 0 for an unusable input rather than throwing: the + * caller's safe direction is to keep today's behaviour (yield), never to break + * the #4088 surface because an enumeration failed. + */ +export function buildBetterAuthRouteOwnership( + api: Record | undefined | null, +): BetterAuthRouteOwnership { + const routes: OwnedRoute[] = []; + for (const endpoint of Object.values(api ?? {})) { + // Same three skips better-call's `createRouter` performs, in the same order. + if (!endpoint?.options || typeof endpoint.path !== 'string') continue; + if (endpoint.options.metadata?.SERVER_ONLY) continue; + const declared = endpoint.options.method; + const methods = new Set( + (Array.isArray(declared) ? declared : [declared ?? 'POST']).map((m) => String(m).toUpperCase()), + ); + const segments = splitPath(endpoint.path); + const matchesRest = segments[segments.length - 1] === '**'; + routes.push({ + methods, + segments: matchesRest ? segments.slice(0, -1) : segments, + matchesRest, + }); + } + + return { + size: routes.length, + owns(method: string, endpointPath: string): boolean { + if (routes.length === 0) return false; + const verb = String(method).toUpperCase(); + const parts = splitPath(endpointPath); + return routes.some((route) => { + if (!route.methods.has(verb) && !route.methods.has('*')) return false; + if (route.matchesRest ? parts.length < route.segments.length : parts.length !== route.segments.length) { + return false; + } + return route.segments.every((seg, i) => (seg.startsWith(':') ? parts[i].length > 0 : seg === parts[i])); + }); + }, + }; +} From 81226d5ba5ac186a8adaae34176597fb1e7dbb02 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 14:56:18 +0000 Subject: [PATCH 2/2] docs(changeset): carve the trailing-slash and doubled-slash spellings out of the "yielded as before" claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .changeset/auth-catchall-owned-404-not-yielded.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/auth-catchall-owned-404-not-yielded.md b/.changeset/auth-catchall-owned-404-not-yielded.md index 6ceafdcc58..4be96afaec 100644 --- a/.changeset/auth-catchall-owned-404-not-yielded.md +++ b/.changeset/auth-catchall-owned-404-not-yielded.md @@ -18,4 +18,6 @@ The catch-all now asks better-auth's live instance whether it owns the path befo **The mount is untouched.** It still claims exactly `${basePath}/*` and still forwards every request under it to better-auth. What narrowed is only which 404 may be handed on. -**Upgrade note — a composition that mounts a route matching paths under the auth base path may see a 404 where it previously saw its own answer.** Affected: deployments that register a route which also matches `/api/v1/auth/...` — most often a broad wildcard over the API prefix — mounted *after* AuthPlugin. Before this release, any request to a path better-auth serves but answers 404 on (a switched-off capability, not an unknown path) was passed to that route and the caller received *its* response, commonly `200` with an empty object. From this release the caller receives better-auth's 404. Callers that treated such a response as success — `res.ok`, `status === 200`, "no error thrown" — will start seeing the refusal that was always the real answer; that is the point of the change, and the wire shape they now get is the one a deployment without the extra mount has always returned. Nothing to do if you mount no such route: paths better-auth does **not** own are yielded exactly as before, so `/auth/me/permissions`, `/auth/me/localization` and any other sibling route under the auth prefix are unaffected in either registration order. +**Upgrade note — a composition that mounts a route matching paths under the auth base path may see a 404 where it previously saw its own answer.** Affected: deployments that register a route which also matches `/api/v1/auth/...` — most often a broad wildcard over the API prefix — mounted *after* AuthPlugin. Before this release, any request to a path better-auth serves but answers 404 on (a switched-off capability, not an unknown path) was passed to that route and the caller received *its* response, commonly `200` with an empty object. From this release the caller receives better-auth's 404. Callers that treated such a response as success — `res.ok`, `status === 200`, "no error thrown" — will start seeing the refusal that was always the real answer; that is the point of the change, and the wire shape they now get is the one a deployment without the extra mount has always returned. Nothing to do if you mount no such route: paths better-auth does **not** own are yielded as before, so `/auth/me/permissions`, `/auth/me/localization` and any other sibling route under the auth prefix are unaffected in either registration order. + +**One carve-out to that sentence, measured and bounded.** 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. Only those two spellings, only of a path better-auth already owns, and only where such a mount exists: no route in this repo registers a spelling of that shape, and every genuinely unowned path — every `/auth/me/*` route included — is yielded exactly as it was. Aligning the table with better-call's own pre-checks is tracked as a follow-up rather than carried here.