Skip to content

Commit d4c2cb1

Browse files
os-warrenclaude
andauthored
fix(plugin-auth): the auth catch-all yields only a 404 that disclaims ownership (#15918)
* fix(plugin-auth): the auth catch-all yields only a 404 that disclaims 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 * docs(changeset): carve the trailing-slash and doubled-slash spellings 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent abe980c commit d4c2cb1

7 files changed

Lines changed: 565 additions & 2 deletions
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
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
6+
7+
`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.
8+
9+
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
10+
11+
```
12+
POST /api/v1/auth/delete-user -> 200 {}
13+
```
14+
15+
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.
16+
17+
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.
18+
19+
**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.
20+
21+
**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.
22+
23+
**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.

packages/plugins/plugin-auth/src/auth-catchall-fallthrough.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,14 @@ async function mountCatchAll(owned: Record<string, () => Response>) {
6060
headers: { 'Content-Type': 'application/json' },
6161
});
6262
});
63-
(plugin as any).authManager = { handleRequest };
63+
// [#15417] The catch-all now asks the auth manager whether better-auth owns
64+
// the path before it yields, so this stand-in has to answer that too — the
65+
// fixture is a fake `AuthManager`, and this is part of the contract it
66+
// stands in for. Ownership is derived from the SAME `owned` table above, so
67+
// the file keeps meaning exactly what its title says: the paths better-auth
68+
// does not own are the ones that get yielded.
69+
const ownsRoute = async (req: Request) => Object.hasOwn(owned, new URL(req.url).pathname);
70+
(plugin as any).authManager = { handleRequest, ownsRoute };
6471

6572
const httpServer: any = { getRawApp: () => app, getPort: () => 0 };
6673
(plugin as any).registerAuthRoutes(httpServer, ctx);
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #15417 — WHICH 404 the auth catch-all is allowed to yield.
5+
*
6+
* `auth-catchall-fallthrough.test.ts` (#4088) pins that the catch-all yields at
7+
* all. This file pins the other edge: it may only yield a 404 that DISCLAIMS
8+
* ownership. A 404 from a path better-auth's own router serves is its ANSWER —
9+
* a switched-off capability — and handing that to the rest of the chain is how
10+
* it comes back as somebody else's `200 {}`.
11+
*
12+
* ── The measurement this file exists for ────────────────────────────────────
13+
*
14+
* #15417 reported `POST /api/v1/auth/admin/<nonexistent>` answering `200 {}` on
15+
* a cloud composition, with a nonexistent path as the control. Reproduced on a
16+
* framework-side boot (`@objectstack/verify` + the showcase stack, both with
17+
* better-auth's admin plugin off and on), that path answers **404** — bodyless,
18+
* no content-type — so the framework does not produce the reported status on
19+
* its own. What does produce it is the yield: register ONE broad downstream
20+
* mount after the catch-all — `app.all('/api/v1/*', c => c.json({}))`, the
21+
* shape a composition adds — and the same request comes back `200 {}`, because
22+
* the catch-all handed it on and the wildcard answered.
23+
*
24+
* That is a framework-side defect regardless of who mounts the wildcard,
25+
* because the request handed on need not be an unknown path at all:
26+
* `delete-user` answers 404 by the ledger's `disabled` disposition, and so does
27+
* every routed endpoint that 404s on a bad token or an unknown id. Those
28+
* answers were all up for grabs. Confirmed end-to-end on the framework-side
29+
* boot with the wildcard installed: `POST /api/v1/auth/delete-user` now
30+
* answers 404 where the wildcard's `200 {}` used to stand.
31+
*
32+
* ⚠️ Ownership is "does better-call ROUTE this", not "is it in `auth.api`".
33+
* Measured on the stock boot: the nine `/admin/oauth2/*` endpoints are in
34+
* `auth.api` and every one carries `SERVER_ONLY: true`, so `createRouter` skips
35+
* them and their 404 is an unrouted one — they stay yieldable, and the pin
36+
* below says so.
37+
*
38+
* ── Why the fixture stubs better-auth, and what it does NOT stub ────────────
39+
*
40+
* Same seam as the #4088 file: `handleRequest` is a path table so the test
41+
* controls exactly which paths the vendor claims, and the real
42+
* `registerAuthRoutes` runs on a real Hono app so the assertions are about the
43+
* shipped handler. The ownership decision is NOT stubbed — `ownsRoute` here
44+
* runs the real `buildBetterAuthRouteOwnership` over a fake `auth.api`, so the
45+
* matcher under test is the shipped one.
46+
*
47+
* The stub's 404 is `new Response(null, { status: 404 })` — bodyless, no
48+
* content-type — because that is what better-call 1.4.0 really returns for an
49+
* unrouted path (`dist/router.mjs`), and what the framework-side boot measured
50+
* on the wire. The #4088 fixture's JSON 404 is a convenience of that file.
51+
*/
52+
53+
import { describe, it, expect, vi } from 'vitest';
54+
import { Hono } from 'hono';
55+
import { AuthPlugin } from './auth-plugin';
56+
import { buildBetterAuthRouteOwnership } from './better-auth-route-ownership';
57+
import type { PluginContext } from '@objectstack/core';
58+
59+
const BASE = '/api/v1/auth';
60+
61+
/** What better-call returns for a path it does not route: bodyless, no content-type. */
62+
const unrouted404 = () => new Response(null, { status: 404, statusText: 'Not Found' });
63+
64+
/**
65+
* Mount the real route registration on a real Hono app.
66+
*
67+
* @param api the fake `auth.api` the REAL ownership matcher reads
68+
* @param answers path -> Response for the paths better-auth answers
69+
*/
70+
async function mountCatchAll(
71+
api: Record<string, { path: string; options: { method: string | string[] } }>,
72+
answers: Record<string, () => Response>,
73+
) {
74+
const app = new Hono();
75+
const ctx: PluginContext = {
76+
registerService: vi.fn(),
77+
getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)),
78+
getServices: vi.fn(() => new Map()),
79+
hook: vi.fn(),
80+
trigger: vi.fn(),
81+
logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() },
82+
getKernel: vi.fn(),
83+
} as any;
84+
85+
const plugin = new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long!!' });
86+
await plugin.init(ctx);
87+
88+
const ownership = buildBetterAuthRouteOwnership(api as any);
89+
const handleRequest = vi.fn(async (req: Request) => {
90+
const make = answers[new URL(req.url).pathname];
91+
return make ? make() : unrouted404();
92+
});
93+
(plugin as any).authManager = {
94+
handleRequest,
95+
// The shipped matcher, over the fake table — only the endpoint-path
96+
// derivation is inlined here (AuthManager's own is private).
97+
ownsRoute: async (req: Request) =>
98+
ownership.owns(req.method, new URL(req.url).pathname.slice(BASE.length)),
99+
};
100+
101+
(plugin as any).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx);
102+
return { app, handleRequest };
103+
}
104+
105+
/** The shape a composition adds: one wildcard over the whole API prefix. */
106+
const addDownstreamWildcard = (app: Hono) => app.all('/api/v1/*', (c) => c.json({}));
107+
108+
describe('#15417: the catch-all yields only a 404 that disclaims ownership', () => {
109+
it('does NOT yield a 404 from a path better-auth OWNS — even with a wildcard downstream', async () => {
110+
// `delete-user` is published and answers 404 because `user.deleteUser` is
111+
// deliberately unconfigured — `auth-route-ledger.ts`'s `disabled`
112+
// disposition. That 404 is an ANSWER and must reach the caller.
113+
const { app } = await mountCatchAll(
114+
{ deleteUser: { path: '/delete-user', options: { method: 'POST' } } },
115+
{ [`${BASE}/delete-user`]: () => new Response(null, { status: 404 }) },
116+
);
117+
addDownstreamWildcard(app);
118+
119+
const res = await app.request(`http://localhost${BASE}/delete-user`, { method: 'POST' });
120+
121+
expect(res.status).toBe(404);
122+
expect(await res.text()).toBe('');
123+
});
124+
125+
it('does not yield an owned 404 on a PARAMETERISED path either', async () => {
126+
// `/callback/:id` is routed and parameterised; a 404 from it is an answer.
127+
const { app } = await mountCatchAll(
128+
{ callback: { path: '/callback/:id', options: { method: 'GET' } } },
129+
{ [`${BASE}/callback/github`]: () => new Response(null, { status: 404 }) },
130+
);
131+
addDownstreamWildcard(app);
132+
133+
const res = await app.request(`http://localhost${BASE}/callback/github`);
134+
135+
expect(res.status).toBe(404);
136+
});
137+
138+
it('DOES yield a SERVER_ONLY endpoint\'s 404 — better-call never routed it', async () => {
139+
// Measured on the stock boot: all nine `/admin/oauth2/*` endpoints are in
140+
// `auth.api` carrying `SERVER_ONLY: true`. `createRouter` skips them, so the
141+
// 404 the wire sees is an unrouted one and yielding it is correct. Were the
142+
// table to trust `auth.api` wholesale instead of mirroring that skip, this
143+
// route would stop being yieldable and a composition serving it downstream
144+
// would break.
145+
const { app } = await mountCatchAll(
146+
{
147+
adminListOAuthResources: {
148+
path: '/admin/oauth2/resources',
149+
options: { method: 'GET', metadata: { SERVER_ONLY: true } },
150+
},
151+
} as any,
152+
{},
153+
);
154+
app.get(`${BASE}/admin/oauth2/resources`, (c) => c.json({ from: 'sibling' }));
155+
156+
const res = await app.request(`http://localhost${BASE}/admin/oauth2/resources`);
157+
158+
expect(res.status).toBe(200);
159+
expect(await res.json()).toEqual({ from: 'sibling' });
160+
});
161+
162+
it('STILL yields a 404 from a path better-auth does not own — #4088 intact', async () => {
163+
// The route plugin-hono-server mounts from its own kernel:ready hook, in
164+
// the registration order that used to 404. Nothing may make this red.
165+
const { app } = await mountCatchAll({ getSession: { path: '/get-session', options: { method: 'GET' } } }, {});
166+
app.get(`${BASE}/me/permissions`, (c) => c.json({ authenticated: true, from: 'hono-plugin' }));
167+
168+
const res = await app.request(`http://localhost${BASE}/me/permissions`);
169+
170+
expect(res.status).toBe(200);
171+
expect(await res.json()).toEqual({ authenticated: true, from: 'hono-plugin' });
172+
});
173+
174+
it('an unknown tail with nothing downstream still answers better-auth\'s 404', async () => {
175+
// The control the card could not run from outside. Unchanged by #15417:
176+
// the framework already answered 404 here, and still does.
177+
const { app } = await mountCatchAll({ getSession: { path: '/get-session', options: { method: 'GET' } } }, {});
178+
179+
const res = await app.request(`http://localhost${BASE}/admin/definitely-not-a-route-1989`, { method: 'POST' });
180+
181+
expect(res.status).toBe(404);
182+
});
183+
184+
it('a path better-auth owns is not yielded even when the DOWNSTREAM route is specific', async () => {
185+
// Precedence still favours the namespace owner (the #4088 file pins this
186+
// for 2xx; here it is pinned for the vendor's own 404 answer).
187+
const { app } = await mountCatchAll(
188+
{ deleteUser: { path: '/delete-user', options: { method: 'POST' } } },
189+
{ [`${BASE}/delete-user`]: () => new Response(null, { status: 404 }) },
190+
);
191+
app.post(`${BASE}/delete-user`, (c) => c.json({ hijacked: true }));
192+
193+
const res = await app.request(`http://localhost${BASE}/delete-user`, { method: 'POST' });
194+
195+
expect(res.status).toBe(404);
196+
expect(await res.text()).toBe('');
197+
});
198+
199+
it('ownership is per METHOD: the same path on a verb better-auth does not serve still yields', async () => {
200+
const { app } = await mountCatchAll(
201+
{ listUsers: { path: '/admin/list-users', options: { method: 'GET' } } },
202+
{},
203+
);
204+
app.post(`${BASE}/admin/list-users`, (c) => c.json({ from: 'sibling' }));
205+
206+
const res = await app.request(`http://localhost${BASE}/admin/list-users`, { method: 'POST' });
207+
208+
expect(res.status).toBe(200);
209+
expect(await res.json()).toEqual({ from: 'sibling' });
210+
});
211+
212+
it('still forwards to better-auth exactly once per request', async () => {
213+
const { app, handleRequest } = await mountCatchAll(
214+
{ deleteUser: { path: '/delete-user', options: { method: 'POST' } } },
215+
{ [`${BASE}/delete-user`]: () => new Response(null, { status: 404 }) },
216+
);
217+
addDownstreamWildcard(app);
218+
handleRequest.mockClear();
219+
220+
await app.request(`http://localhost${BASE}/delete-user`, { method: 'POST' });
221+
222+
expect(handleRequest).toHaveBeenCalledTimes(1);
223+
});
224+
});

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ import {
5959
} from './auth-session-audit.js';
6060
import { SESSION_ERASURE_PATHS } from './session-tombstone.js';
6161
import { envelopeVendorAdminRefusal } from './vendor-admin-refusal-envelope.js';
62+
import {
63+
buildBetterAuthRouteOwnership,
64+
type BetterAuthRouteOwnership,
65+
} from './better-auth-route-ownership.js';
6266
import {
6367
ADMIN_SESSION_COOKIE_KEY,
6468
STOP_IMPERSONATING_PATH,
@@ -5380,6 +5384,45 @@ export class AuthManager {
53805384
return response;
53815385
}
53825386

5387+
/**
5388+
* [#15417] Does better-auth ROUTE this request — i.e. is the path one its own
5389+
* router owns, whatever it then answers?
5390+
*
5391+
* The auth catch-all yields the request to the rest of the Hono chain when
5392+
* better-auth answers 404 (#4088), and it needs this to tell the two very
5393+
* different 404s apart: "I do not serve this path" (yieldable — that is how
5394+
* `plugin-hono-server`'s `/auth/me/*` routes stay reachable in either
5395+
* registration order) from "I serve it and the answer is 404" (NOT yieldable
5396+
* — a disabled capability's refusal is an answer, and handing it to a
5397+
* downstream wildcard is how it becomes `200 {}`). The mechanism, the
5398+
* measurement and the matching rules live in
5399+
* `better-auth-route-ownership.ts`.
5400+
*
5401+
* Keyed on the live instance and rebuilt whenever that instance is replaced,
5402+
* so a re-created auth (config change, test re-boot) never answers from a
5403+
* stale table. Returns `false` — the yielding, pre-#15417 answer — for any
5404+
* request it cannot decide, so a failure to enumerate can never take the
5405+
* #4088 surface down with it.
5406+
*/
5407+
async ownsRoute(request: Request): Promise<boolean> {
5408+
const endpointPath = this.betterAuthEndpointPath(request);
5409+
if (endpointPath === undefined) return false;
5410+
try {
5411+
const auth = await this.getOrCreateAuth();
5412+
if (this.routeOwnershipFor !== auth || !this.routeOwnership) {
5413+
this.routeOwnership = buildBetterAuthRouteOwnership((auth as any)?.api);
5414+
this.routeOwnershipFor = auth;
5415+
}
5416+
return this.routeOwnership.owns(request.method, endpointPath);
5417+
} catch {
5418+
return false;
5419+
}
5420+
}
5421+
5422+
/** Memoized `auth.api` ownership table, and the instance it was built from. */
5423+
private routeOwnership?: BetterAuthRouteOwnership;
5424+
private routeOwnershipFor?: unknown;
5425+
53835426
/**
53845427
* The better-auth endpoint path (`/admin/remove-user`) this request addresses,
53855428
* or `undefined` when it is not under the configured `basePath`.

0 commit comments

Comments
 (0)