From 5ddff5274c5846a18b05eb181d794161311e8e29 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 19:57:56 +0000 Subject: [PATCH] fix(hono): the /auth/* mount yields only a 404 that disclaims ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `createHonoApp`'s `${prefix}/auth/*` mount handed the request on whenever the auth service answered 404, with only the status to go on, so a 404 a routed endpoint produced — a real answer — was replaceable by whatever matched next. Measured on a real boot through this adapter (real kernel + AuthPlugin, `prefix: '/api/v1'`): GET /api/v1/auth/delete-user/callback?token=abc&callbackURL=/x better-auth direct : 404 {"message":"Not found","code":"NOT_FOUND"} through the mount : 200 {} No composition had to install the overwriting layer: the `${prefix}/*` dispatcher catch-all registered by the same function is terminal and answers `200 {}` for paths under `/auth/`. `auth-route-ledger.ts` carries that route under its `disabled` disposition because it is published and answers 404, so the ledger's answer was true of the auth service and false on the wire. The mount now asks the auth service whether its own router serves the path, through an optional `ownsRoute(request)` — the seam `AuthManager` grew for the plugin-side half of the same defect — and yields only when it does not. The adapter does not import the ownership walk: it does not depend on `@objectstack/plugin-auth` and gains no dependency here. Every answer that is not a literal `true` means yield, so a service predating the method is unaffected and a failure to decide can never cost #4088's ordering-independent surface. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../hono-auth-mount-owned-404-not-yielded.md | 32 +++ .../hono/src/hono-auth-owned-404.test.ts | 230 ++++++++++++++++++ packages/adapters/hono/src/index.ts | 64 ++++- 3 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 .changeset/hono-auth-mount-owned-404-not-yielded.md create mode 100644 packages/adapters/hono/src/hono-auth-owned-404.test.ts 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(); }