diff --git a/.changeset/hono-auth-mount-owned-404-not-yielded.md b/.changeset/hono-auth-mount-owned-404-not-yielded.md new file mode 100644 index 0000000000..cfafa0f524 --- /dev/null +++ b/.changeset/hono-auth-mount-owned-404-not-yielded.md @@ -0,0 +1,32 @@ +--- +'@objectstack/hono': patch +--- + +The Hono adapter's `/auth/*` mount yields only a 404 that disclaims ownership + +`createHonoApp`'s `${prefix}/auth/*` mount forwards every request under it to the +kernel's `auth` service and, since #4117, hands the request on to the rest of the +chain when that service answers 404 — which is what keeps `/auth/me/permissions` +and `/auth/me/localization` reachable through the gated `dispatch()`. 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 on a real boot through this adapter (a real kernel with `AuthPlugin`, +`prefix: '/api/v1'`), `GET /api/v1/auth/delete-user/callback?token=…&callbackURL=…` +answered `404 {"message":"Not found","code":"NOT_FOUND"}` from better-auth and +`200 {}` on the wire. `plugin-auth`'s route ledger carries that route under its +`disabled` disposition precisely because it is published and answers 404, so the +ledger's recorded answer was true of the auth service and false on this adapter's +wire. Nothing had to be composed in for that: the `${prefix}/*` dispatcher +catch-all this same function registers is terminal and answers `200 {}` for paths +under `/auth/`. + +The mount now asks the auth service whether its own router serves the path, via +an optional `ownsRoute(request)` — the seam `AuthManager` grew in the plugin-side +fix for the same defect — and yields only when it does not. Every answer that is +not a literal `true` (no such method, a throw, anything else) means yield, so a +service predating the method behaves exactly as before and a failure to decide +can never cost the ordering-independent surface. + +⛔ The mount is unchanged and still claims `${prefix}/auth/*`; 401/403 were never +yielded and still are not. What narrowed is only which 404 may be handed on. diff --git a/packages/adapters/hono/src/hono-auth-owned-404.test.ts b/packages/adapters/hono/src/hono-auth-owned-404.test.ts new file mode 100644 index 0000000000..b2b015caad --- /dev/null +++ b/packages/adapters/hono/src/hono-auth-owned-404.test.ts @@ -0,0 +1,230 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15928 — WHICH 404 the adapter's `/auth/*` mount is allowed to yield. + * + * `hono-wildcard-fallthrough.test.ts` (#4117) pins that the mount yields at + * all. This file pins the other edge, the one PR #15918 (card #15417) closed on + * the `plugin-auth` side of the identical defect: the mount may yield only a + * 404 that DISCLAIMS OWNERSHIP. A 404 from a path the auth service's own router + * SERVES is its answer, and handing that on is how it becomes somebody else's + * `200 {}`. + * + * ## The measurement this file exists for + * + * Card #15928 recorded the location and the "identical unconditioned yield" + * reading from a REVIEWER of PR #15918, explicitly unmeasured on this seat. + * Measured here, on a real boot through this adapter — a real `ObjectKernel` + * with `AuthPlugin` (so a real `AuthManager`, a real better-auth with 100 + * `auth.api` entries), `createHonoApp({ kernel, prefix: '/api/v1' })`, requests + * injected through the returned app — the reading is CONFIRMED, and the blast + * radius at this layer is wider than the plugin's: + * + * GET /api/v1/auth/delete-user/callback?token=abc&callbackURL=/x + * better-auth direct : 404 application/json {"message":"Not found","code":"NOT_FOUND"} + * through this mount : 200 application/json {} + * + * ⚠️ Wider because the plugin-side defect needed a composition to mount a + * downstream wildcard, and this one does not: the `${prefix}/*` dispatcher + * catch-all that overwrites the answer is registered by `createHonoApp` itself, + * is terminal, and answers `200 {}` for paths under `/auth/`. Measured on the + * same boot: `POST /api/v1/auth/definitely-not-a-route-1989` and + * `GET /api/v1/auth/me/permissions` both come back `200 {}` from it. + * + * That route is not hypothetical. `plugin-auth`'s `auth-route-ledger.ts` carries + * `POST /api/v1/auth/delete-user` and `GET /api/v1/auth/delete-user/callback` + * under the `disabled` disposition precisely because they are published and + * answer 404 (`user.deleteUser` is deliberately unconfigured, maintainer ruling + * 2026-08-12). So the ledger's recorded answer was true of the auth service and + * false on this adapter's wire. + * + * ## ⭐ What these cases COVER, and what they do NOT + * + * COVERED: the adapter's DECISION LOGIC — given an auth service that answers + * `ownsRoute`, which 404s this mount yields and which it returns, what it does + * with a service that has no `ownsRoute` at all, and what it passes to it. + * + * ⛔ NOT COVERED by any case in this file: + * - better-auth's real route table. The fixture's `ownsRoute` is a path set, + * not `buildBetterAuthRouteOwnership` over a real `auth.api` — that matcher + * is `plugin-auth`'s and is pinned there + * (`better-auth-route-ownership.test.ts`). `@objectstack/hono` does not + * depend on `@objectstack/plugin-auth` and gains no dependency here. + * - that the kernel's `auth` service really carries `ownsRoute`. Measured on + * the real boot above (`kernel.getServiceAsync('auth')` → `_AuthManager`, + * `typeof ownsRoute === 'function'`), NOT pinned by a case here. + * - the `basePath`/`prefix` alignment. `AuthManager.ownsRoute` answers on the + * AUTH SERVICE's configured `basePath`; measured on the real boot, + * `ownsRoute('POST', '/api/v1/auth/delete-user')` is `true` while + * `ownsRoute('POST', '/api/auth/delete-user')` is `false`. A deployment + * whose adapter `prefix` and auth `basePath` disagree therefore gets `false` + * for everything and keeps the pre-#15928 yield — the safe direction, and + * the reason every undecidable answer is `false`. + * - trailing-slash and doubled-slash spellings, the one known divergence of + * the plugin-side walk (it claims them; better-call refuses them as + * unrouted). Inherited here, unpinned here. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Hono } from 'hono'; + +const mockDispatcher = { + getDiscoveryInfo: vi.fn().mockReturnValue({ version: '1.0', routes: {} }), + handleAuth: vi.fn(), + dispatch: vi.fn(), +}; + +vi.mock('@objectstack/runtime', () => ({ + HttpDispatcher: function HttpDispatcher() { return mockDispatcher; }, +})); + +import { createHonoApp } from './index'; + +/** The shape of the `200 {}` the real dispatcher catch-all answers with. */ +const DISPATCH_ANSWERED = { handled: true, response: { body: {}, status: 200 } }; + +/** 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' }); + +/** better-auth's OWN 404 on a path it serves — enveloped, with a content-type. */ +const owned404 = () => new Response(JSON.stringify({ message: 'Not found', code: 'NOT_FOUND' }), { + status: 404, + headers: { 'Content-Type': 'application/json' }, +}); + +const kernelWith = (authService?: unknown) => ({ + name: 'test-kernel', + getService: (n: string) => (n === 'auth' && authService ? authService : undefined), +}) as any; + +/** + * An auth service in the shape the kernel really registers: `handleRequest` + * plus the optional `ownsRoute`. `owned` is the set of wire paths it claims — + * a path SET, deliberately, so these cases are about the adapter's decision and + * not about a route matcher this package does not own. + */ +const authServiceOwning = (owned: string[], answer: () => Response) => ({ + handleRequest: vi.fn(async () => answer()), + ownsRoute: vi.fn(async (req: Request) => owned.includes(new URL(req.url).pathname)), +}); + +const PREFIX = '/api/v1'; + +describe('#15928: the adapter yields only a 404 that disclaims ownership', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockDispatcher.dispatch.mockResolvedValue(DISPATCH_ANSWERED); + mockDispatcher.handleAuth.mockResolvedValue({ handled: false }); + }); + + it('does NOT yield a 404 from a path the auth service OWNS — the answer reaches the caller', async () => { + // The measured case: published, routed, and 404 BY DESIGN. Before this fix + // the mount handed it to the dispatcher catch-all, which answered 200 {}. + const svc = authServiceOwning([`${PREFIX}/auth/delete-user/callback`], owned404); + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: PREFIX }); + + const res = await app.request(`http://localhost${PREFIX}/auth/delete-user/callback?token=abc&callbackURL=/x`); + + expect(res.status).toBe(404); + expect(res.headers.get('content-type')).toContain('application/json'); + expect(await res.json()).toEqual({ message: 'Not found', code: 'NOT_FOUND' }); + // The load-bearing half: nothing downstream was ever given the chance. + expect(mockDispatcher.dispatch).not.toHaveBeenCalled(); + }); + + it('DOES yield a 404 the auth service disclaims — #4088 ordering-independence intact', async () => { + // `/auth/me/permissions` is the canonical disclaimed path: nothing in + // better-auth serves it, and objectui's permission layer reads it. #4088 + // made this mount non-terminal for it, and that must survive this fix. + const svc = authServiceOwning([], unrouted404); + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: PREFIX }); + + const res = await app.request(`http://localhost${PREFIX}/auth/me/permissions`); + + expect(mockDispatcher.dispatch).toHaveBeenCalled(); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({}); + }); + + it('an auth service with NO `ownsRoute` keeps the pre-#15928 behaviour exactly', async () => { + // The back-compat population: this is a STRUCTURAL interface over whatever + // the kernel registered, so a service predating the method must still yield. + const svc = { handleRequest: vi.fn(async () => unrouted404()) }; + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: PREFIX }); + + const res = await app.request(`http://localhost${PREFIX}/auth/anything-at-all`); + + expect(mockDispatcher.dispatch).toHaveBeenCalled(); + expect(res.status).toBe(200); + }); + + it('an `ownsRoute` that THROWS yields — a failure to decide never costs the #4088 surface', async () => { + const svc = { + handleRequest: vi.fn(async () => unrouted404()), + ownsRoute: vi.fn(async () => { throw new Error('auth.api unreachable'); }), + }; + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: PREFIX }); + + const res = await app.request(`http://localhost${PREFIX}/auth/me/permissions`); + + expect(svc.ownsRoute).toHaveBeenCalled(); + expect(mockDispatcher.dispatch).toHaveBeenCalled(); + expect(res.status).toBe(200); + }); + + it('only a literal `true` stops the yield — any other answer is "not owned"', async () => { + // `undefined` is what an implementation returning nothing gives back. It + // must read as "could not decide" (yield), never as "owned" (swallow). + const svc = { + handleRequest: vi.fn(async () => unrouted404()), + ownsRoute: vi.fn(async () => undefined as unknown as boolean), + }; + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: PREFIX }); + + const res = await app.request(`http://localhost${PREFIX}/auth/me/permissions`); + + expect(mockDispatcher.dispatch).toHaveBeenCalled(); + expect(res.status).toBe(200); + }); + + it('is not consulted at all on a non-404 — the predicate can only STOP a yield', async () => { + const svc = { + handleRequest: vi.fn(async () => new Response(JSON.stringify({ error: 'nope' }), { status: 401 })), + ownsRoute: vi.fn(async () => true), + }; + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: PREFIX }); + + const res = await app.request(`http://localhost${PREFIX}/auth/protected`); + + expect(res.status).toBe(401); + expect(svc.ownsRoute).not.toHaveBeenCalled(); + expect(mockDispatcher.dispatch).not.toHaveBeenCalled(); + }); + + it('is asked with the RAW request — full wire URL and method, not the stripped subpath', async () => { + // `AuthManager.ownsRoute` derives better-auth's endpoint path from the + // request URL against its own configured `basePath`, and matches per + // method. Hand it the stripped `delete-user/callback` and it decides + // nothing. This pins the argument, not the answer. + const svc = authServiceOwning([`${PREFIX}/auth/delete-user`], owned404); + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: PREFIX }); + + await app.request(`http://localhost${PREFIX}/auth/delete-user`, { method: 'POST' }); + + const seen = svc.ownsRoute.mock.calls[0][0] as Request; + expect(seen).toBeInstanceOf(Request); + expect(new URL(seen.url).pathname).toBe(`${PREFIX}/auth/delete-user`); + expect(seen.method).toBe('POST'); + }); + + it('respects a non-default `prefix` — ownership is asked on that mount too', async () => { + const svc = authServiceOwning(['/custom/auth/delete-user/callback'], owned404); + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: '/custom' }); + + const res = await app.request('http://localhost/custom/auth/delete-user/callback'); + + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ message: 'Not found', code: 'NOT_FOUND' }); + expect(mockDispatcher.dispatch).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/adapters/hono/src/index.ts b/packages/adapters/hono/src/index.ts index 5d79789583..ba7ea91924 100644 --- a/packages/adapters/hono/src/index.ts +++ b/packages/adapters/hono/src/index.ts @@ -85,6 +85,20 @@ export interface ObjectStackHonoOptions { */ interface AuthService { handleRequest(request: Request): Promise; + /** + * Does the auth service's OWN router serve this path? (#15928) + * + * Optional on purpose: this is a structural interface over whatever the + * kernel registered as the `auth` service, and an implementation predating + * the method must keep working. `AuthPlugin`'s `AuthManager` implements it + * (#15417 / PR #15918) by asking better-auth's live `auth.api` — the same + * seam the route ledger's conformance test and the `/admin/` dogfood sweep + * read. It answers on better-auth's endpoint-path spelling derived from the + * AUTH SERVICE's configured `basePath`, not from this adapter's `prefix`, + * so a deployment whose two disagree gets `false` for everything — the + * yielding, pre-#15928 answer, which is the safe direction. + */ + ownsRoute?(request: Request): Promise; } /** @@ -339,6 +353,23 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { if (!c.res) c.res = fallback(); }; + /** + * Does the auth service claim this path? (#15928) + * + * Every non-`true` answer — no such method, a throw, anything but `true` — + * is `false`, i.e. "yield", i.e. exactly what this mount did before #15928. + * A failure to decide can therefore never take the #4088 surface down with + * it; the only thing this predicate can do is STOP a yield. + */ + const authOwnsRoute = async (authService: AuthService, request: Request): Promise => { + if (typeof authService.ownsRoute !== 'function') return false; + try { + return (await authService.ownsRoute(request)) === true; + } catch { + return false; + } + }; + // --- Auth (needs auth service integration) --- app.all(`${prefix}/auth/*`, async (c, next) => { try { @@ -401,7 +432,38 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { // 404 from better-auth means "not one of my endpoints" — the #4092 // signal. `/auth/me/permissions` is the canonical example: nothing in // better-auth serves it, `plugin-hono-server` does. - if (response.status === 404) return yieldUnowned(c, next, forwarded); + // + // [#15928] …but ONLY when better-auth disclaims the path. A 404 from a + // path its own router SERVES is its answer, not a disclaimer, and + // yielding it hands a real answer to whatever matched next. Here that + // "whatever" is not hypothetical and needs no composition to install + // it: the `${prefix}/*` dispatcher catch-all below is registered by + // THIS function, is terminal, and answers `200 {}` for paths under + // `/auth/`. Measured on a real boot through this adapter (a real + // kernel with AuthPlugin, `prefix: '/api/v1'`): + // + // GET /api/v1/auth/delete-user/callback?token=…&callbackURL=… + // better-auth direct : 404 {"message":"Not found","code":"NOT_FOUND"} + // through this mount : 200 {} + // + // That route is published and answers 404 because `user.deleteUser` is + // deliberately unconfigured — `auth-route-ledger.ts` carries the pair + // under the `disabled` disposition for exactly that reason. So the + // ledger's recorded answer was true of the auth service and false on + // this adapter's wire. Same defect, same fix shape as PR #15918 took in + // the plugin, adapted to the seam available here: the adapter cannot + // import `buildBetterAuthRouteOwnership` (it does not depend on + // `@objectstack/plugin-auth`, and should not), so it asks the auth + // SERVICE, which is the very `AuthManager` instance that owns the walk. + // + // ⛔ The mount is untouched and still claims `${prefix}/auth/*`; what + // narrowed is which 404 may be handed on. `/auth/me/permissions` and + // `/auth/me/localization` are not better-auth endpoints, so they are + // disclaimed and still yield — #4088's ordering-independent surface, + // which objectui's permission layer reads, is unchanged. + if (response.status === 404 && !(await authOwnsRoute(authService, c.req.raw))) { + return yieldUnowned(c, next, forwarded); + } return forwarded(); }