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..4be96afaec --- /dev/null +++ b/.changeset/auth-catchall-owned-404-not-yielded.md @@ -0,0 +1,23 @@ +--- +"@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 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. 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 2a247ce41b..b18d185127 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -2983,7 +2983,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])); + }); + }, + }; +}