diff --git a/.changeset/hono-auth-mount-follows-auth-base-path.md b/.changeset/hono-auth-mount-follows-auth-base-path.md new file mode 100644 index 0000000000..96fd6ab9da --- /dev/null +++ b/.changeset/hono-auth-mount-follows-auth-base-path.md @@ -0,0 +1,28 @@ +--- +"@objectstack/hono": minor +"@objectstack/plugin-auth": minor +--- + +`createHonoApp` mounts the auth surface where the auth service actually serves, and refuses a prefix it cannot serve it under. + +The documented embed did not reach better-auth at all. `createHonoApp` mounted `/auth/*` under its own `prefix` (default `/api`) while `AuthPlugin` configures better-auth with `basePath: '/api/v1/auth'`, so the two never intersected. The forwarded request could only 404, that 404 fell through to the terminal dispatcher catch-all, and the caller got a `200` with an empty body. Measured on a real kernel with `AuthPlugin`, driving `createHonoApp({ kernel })` with both defaults untouched: + +``` +POST /api/auth/sign-in/email (valid shape, wrong password) -> 200 {} +GET /api/auth/get-session -> 200 {} +POST /api/auth/sign-up/email -> 200 {} +``` + +A failed sign-in answering `200 {}` is the silent-success shape: a client that reads `res.ok` sends the user into an authenticated view with no session. The same boot now answers, through the same embed: + +``` +POST /api/v1/auth/sign-in/email (wrong password) -> 401 {"message":"Invalid email or password","code":"INVALID_EMAIL_OR_PASSWORD"} +GET /api/v1/auth/get-session -> 200 null +POST /api/v1/auth/delete-user -> 401 {"message":"Unauthorized","code":"UNAUTHORIZED"} +``` + +**Neither default moves.** `prefix` still defaults to `/api` and the auth `basePath` still defaults to `/api/v1/auth`. What changed is which of the two decides the mount: + +- **`@objectstack/hono`** — the `/auth/*` mount is derived from the auth service's configured `basePath`, read at app-construction time, rather than from `prefix`. An auth service that does not expose its base path keeps the previous `${prefix}/auth` mount, so a custom or older auth service is unaffected. +- **`@objectstack/hono`** — a `prefix` the auth base path is not inside now **refuses at construction**, naming both values and every one-line fix that actually constructs: move the app up to the base path's own parent namespace, or configure better-auth down under the prefix (carrying the leading slash the prefix may itself be missing). ⛔ A direction with no working answer is not offered rather than offered wrongly — a single-segment base has no usable parent prefix, because `''` falls back to `/api` and `'/'` mounts every other route of the app under `//`. Previously that composition served auth outside the namespace the host asked for while `${prefix}/auth/*` answered `200 {}`. This is the one behaviour that can stop an app booting: a deployment passing, say, `prefix: '/custom'` alongside the default auth base path was already not serving auth, and now says so instead of failing silently. +- **`@objectstack/plugin-auth`** — `AuthManager.getBasePath()` is new and public: the configured base path in its one normalised spelling (a leading slash added when absent, trailing slashes stripped), which is the spelling an HTTP adapter can mount on. ⛔ **Purely additive — no configured `basePath` changes anything this package does.** better-auth is still handed the configured string verbatim, and the route-ownership walk still normalises its own copy; that copy now reads this accessor instead of repeating the expression. ⛔ It is **not** the string better-auth receives, and it is **not** the single definition of the value. `getAuthIssuer()` and `getMcpResourceUrl()` still derive their own copies and are deliberately unchanged: they are the OAuth `iss` this AS advertises and the RFC 8707 resource identifier a token's `aud` is matched against, both compared by exact string by relying parties, so retiring their copies moves published identifiers and is not a tidy-up that belongs on this card (filed as #16399). Normalising the string handed to better-auth is that same move seen from the other side — it shifts the access-token `iss` off `getAuthIssuer()`, and this manager's own `verifyMcpAccessToken` then rejects every MCP token the deployment mints. Measured on a real `client_credentials` token, and not done. diff --git a/packages/adapters/hono/src/hono-auth-mount-basepath.test.ts b/packages/adapters/hono/src/hono-auth-mount-basepath.test.ts new file mode 100644 index 0000000000..e20eecd6fa --- /dev/null +++ b/packages/adapters/hono/src/hono-auth-mount-basepath.test.ts @@ -0,0 +1,385 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16025 — WHERE the adapter mounts the auth surface, and the boot refusal that + * guards it. + * + * `hono-auth-owned-404.test.ts` (#15928) pins WHICH 404 that mount may yield; + * its own "not covered" list names this file's subject as the gap it leaves: + * "the `basePath`/`prefix` alignment". This file closes it. + * + * ## The measurement this file exists for + * + * Re-driven on the CURRENT tree — a real `ObjectKernel` with `AuthPlugin` (a + * real `AuthManager` over better-auth) via `@objectstack/verify`'s `bootStack`, + * the DOCUMENTED embed `createHonoApp({ kernel })` with both defaults + * untouched, requests injected through the returned app. + * + * BEFORE: + * + * POST /api/auth/sign-in/email (valid shape, wrong password) -> 200 {} + * GET /api/auth/get-session -> 200 {} + * POST /api/auth/sign-up/email -> 200 {} + * POST /api/v1/auth/delete-user -> 404 ROUTE_NOT_FOUND + * + * AFTER: + * + * POST /api/v1/auth/sign-in/email (wrong password) -> 401 {"message":"Invalid email or password","code":"INVALID_EMAIL_OR_PASSWORD"} + * GET /api/v1/auth/get-session -> 200 null + * POST /api/v1/auth/sign-up/email -> 403 {"code":"SELF_REGISTRATION_CLOSED",…} + * POST /api/v1/auth/delete-user -> 401 {"message":"Unauthorized","code":"UNAUTHORIZED"} + * + * A failed sign-in answering `200 {}` is the silent-success shape: a client + * reading `res.ok` sends the user into an authenticated view with no session. + * The `401` row is better-auth answering for real — the arrival proof. + * + * Maintainer ruling of 2026-09-06 (director batch #54), options A + B: the + * mount FOLLOWS the auth service's `basePath` (B), and a `prefix` the base is + * not inside REFUSES AT BOOT naming both values (A). ⛔ Neither default moves. + * + * ## ⭐ Why these cases assert a RELATION, not the string `/api/v1/auth` + * + * A pin that asserted the mount equals `/api/v1/auth` would be mirroring a + * default that lives in another package (`@objectstack/plugin-auth`), and this + * package neither depends on it nor can. Every case below asserts the mount is + * WHATEVER THE SERVICE ANSWERED — so a repair that hard-coded today's default + * fails them, and moving that default in plugin-auth cannot silently invalidate + * them. `/api/v1/auth` appears in one case only, as the card's own composition. + * + * ## ⛔ What these cases do NOT cover + * + * - That the kernel's real `auth` service carries `getBasePath` at all, or + * that the wire paths under its answer are the ones better-auth really + * matches. Both are `@objectstack/plugin-auth`'s to keep + * (`auth-manager-base-path.test.ts` pins the accessor there; ⛔ note that + * `AuthManager` hands better-auth the CONFIGURED spelling verbatim, not + * this normalised one — deliberately, because the OAuth `iss` is derived + * from it), and both were measured on the real boot quoted above. This + * package does not depend on `@objectstack/plugin-auth` and gains no + * dependency here — the same boundary #15928's file records. + * - The `200 {}` the BEFORE rows carried. That is manufactured one layer out, + * by the terminal dispatcher catch-all rendering a `Response` result as + * `c.json(res, 200)`; the card names it as a sibling finding and places it + * outside its own scope. It still stands on `${prefix}/auth/*` after this + * change, and no case here asserts otherwise. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Hono } from 'hono'; + +const mockDispatcher = { + dispatch: vi.fn(), + handleAuth: vi.fn(), + getDiscoveryInfo: vi.fn(async () => ({})), +}; + +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 } }; + +/** better-auth's real refusal on a routed path — the arrival shape the card names. */ +const unauthorized = () => new Response( + JSON.stringify({ message: 'Unauthorized', code: 'UNAUTHORIZED' }), + { status: 401, headers: { 'Content-Type': 'application/json' } }, +); + +const kernelWith = (authService?: unknown) => ({ + name: 'test-kernel', + getService: (n: string) => (n === 'auth' && authService ? authService : undefined), +}) as any; + +/** A kernel whose `auth` is factory-registered: the sync accessor throws. */ +const kernelWithAsyncOnlyAuth = () => ({ + name: 'test-kernel', + getService: (n: string) => { + if (n === 'auth') throw new Error(`Service '${n}' is async - use await`); + return undefined; + }, +}) as any; + +/** An auth service that answers where it serves, in the kernel's real shape. */ +const authServiceAt = (basePath: unknown, answer: () => Response = unauthorized) => ({ + handleRequest: vi.fn(async () => answer()), + getBasePath: vi.fn(() => basePath as string), +}); + +/** The pre-#16025 shape: a service that does not say where it serves. */ +const authServiceWithoutAccessor = (answer: () => Response = unauthorized) => ({ + handleRequest: vi.fn(async () => answer()), +}); + +beforeEach(() => { + vi.clearAllMocks(); + mockDispatcher.dispatch.mockResolvedValue(DISPATCH_ANSWERED); + mockDispatcher.handleAuth.mockResolvedValue({ handled: false }); +}); + +describe('#16025 B: the /auth mount follows the auth service, not the adapter prefix', () => { + it("reaches the auth service on the card's own composition — default prefix, base /api/v1/auth", async () => { + // The documented embed: `createHonoApp({ kernel })`, both defaults untouched. + const svc = authServiceAt('/api/v1/auth'); + const app: Hono = createHonoApp({ kernel: kernelWith(svc) }); + + const res = await app.request('http://localhost/api/v1/auth/delete-user', { method: 'POST' }); + + // ADR-0112 envelope: the code and the status, not merely "it threw". + expect(res.status).toBe(401); + expect(await res.json()).toEqual({ message: 'Unauthorized', code: 'UNAUTHORIZED' }); + expect(svc.handleRequest).toHaveBeenCalledTimes(1); + // The load-bearing half: nothing downstream answered in its place. + expect(mockDispatcher.dispatch).not.toHaveBeenCalled(); + }); + + it('does NOT mount at `${prefix}/auth` any more — the wire path the card measured', async () => { + const svc = authServiceAt('/api/v1/auth'); + const app: Hono = createHonoApp({ kernel: kernelWith(svc) }); + + await app.request('http://localhost/api/auth/sign-in/email', { method: 'POST' }); + + // The defect was that THIS path claimed the mount and then forwarded a + // request better-auth does not route. It reaches the catch-all instead. + expect(svc.handleRequest).not.toHaveBeenCalled(); + expect(mockDispatcher.dispatch).toHaveBeenCalled(); + }); + + it('follows an ARBITRARY base the service answers — the rule, not the default', async () => { + const svc = authServiceAt('/api/v9/identity'); + const app: Hono = createHonoApp({ kernel: kernelWith(svc) }); + + const reached = await app.request('http://localhost/api/v9/identity/delete-user', { method: 'POST' }); + expect(reached.status).toBe(401); + expect(svc.handleRequest).toHaveBeenCalledTimes(1); + + // …and today's plugin-auth default is NOT special-cased into the mount. + await app.request('http://localhost/api/v1/auth/delete-user', { method: 'POST' }); + expect(svc.handleRequest).toHaveBeenCalledTimes(1); + }); + + it('resolves the adapter-owned /auth/config route relative to the mount', async () => { + const svc = { + handleRequest: vi.fn(async () => unauthorized()), + getBasePath: () => '/api/v1/auth', + getPublicConfig: () => ({ features: {} }), + }; + const app: Hono = createHonoApp({ kernel: kernelWith(svc) }); + + const res = await app.request('http://localhost/api/v1/auth/config'); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ success: true, data: { features: {} } }); + // `config` is answered by the adapter, never forwarded. + expect(svc.handleRequest).not.toHaveBeenCalled(); + }); + + it('normalises what the service answers — a missing leading or trailing slash is the same base', async () => { + for (const spelling of ['api/v1/auth', '/api/v1/auth/', '/api/v1/auth//']) { + const svc = authServiceAt(spelling); + const app: Hono = createHonoApp({ kernel: kernelWith(svc) }); + const res = await app.request('http://localhost/api/v1/auth/delete-user', { method: 'POST' }); + expect(res.status, `mount for ${JSON.stringify(spelling)}`).toBe(401); + } + }); +}); + +describe('#16025 A: a prefix the auth base is not inside refuses at boot', () => { + it('refuses, and the message names BOTH values and the fix', () => { + const svc = authServiceAt('/api/v1/auth'); + + let thrown: Error | undefined; + try { + createHonoApp({ kernel: kernelWith(svc), prefix: '/custom' }); + } catch (err) { + thrown = err as Error; + } + + expect(thrown, 'a misaligned composition must not build an app').toBeDefined(); + // Both values, because a refusal naming one of them cannot be acted on. + expect(thrown!.message).toContain('/api/v1/auth'); + expect(thrown!.message).toContain('/custom'); + // And the one-line fix, in both directions. + expect(thrown!.message).toContain('createHonoApp({ kernel, prefix:'); + expect(thrown!.message).toContain('new AuthPlugin({ basePath:'); + }); + + it('⭐ does NOT refuse the prefixes the base IS inside — the over-refusal control', () => { + // A repair that refused everything would pass the case above. These three + // are the compositions that must keep booting: the default embed, the + // prefix the card measured as already lining up, and the base itself. + const svc = authServiceAt('/api/v1/auth'); + for (const prefix of [undefined, '/api', '/api/v1', '/api/v1/auth'] as const) { + expect( + () => createHonoApp(prefix === undefined + ? { kernel: kernelWith(svc) } + : { kernel: kernelWith(svc), prefix }), + `prefix ${String(prefix)}`, + ).not.toThrow(); + } + }); + + /** + * ⭐ The refusal's own advice, DRIVEN — the domain the control above misses. + * + * The control above proves only that four LEADING-SLASH prefixes still build. + * The refusal's real domain is wider: a prefix written without a leading slash + * refuses here and reached better-auth on `main`, and a single-segment base + * has no usable parent namespace at all. Both were outside every pin, and both + * are where the first spelling of the message gave advice that does not work: + * `new AuthPlugin({ basePath: 'api/v1/auth' })` refuses again, and `prefix: '/'` + * mounts every other route of the app under `//`. + * + * ⭐ So this does not assert the message's WORDS. It parses the `Fix —` clauses + * back out and re-drives each one through `createHonoApp` at the top: whatever + * the refusal tells a caller to do has to produce an app. A `Fix:` line that + * does not fix is a false sentence in shipped code, and nothing but driving it + * can tell the two apart. + */ + const REFUSING_COMPOSITIONS = [ + { what: "the card's own shape — a prefix the default base is outside", basePath: '/api/v1/auth', prefix: '/custom' }, + { what: 'a prefix written WITHOUT a leading slash — served auth on main, refuses here', basePath: '/api/v1/auth', prefix: 'api/v1' }, + { what: "a nested mount's inner prefix, as createHonoApp sees it", basePath: '/api/v1/auth', prefix: '/v1' }, + { what: 'a SINGLE-SEGMENT base, whose parent namespace is not a usable prefix', basePath: '/auth', prefix: '/api' }, + { what: 'a base that only shares a prefix STRING with the namespace', basePath: '/apifoo/auth', prefix: '/api' }, + ] as const; + + /** Read the fixes back out of the refusal, as a caller would act on them. */ + const fixesIn = (message: string): Array<{ prefix?: string; basePath?: string }> => { + const tail = message.split('Fix — ')[1]; + expect(tail, 'the refusal must carry a Fix clause at all').toBeDefined(); + return tail.replace(/\.$/, '').split('; or ').map((clause) => ({ + prefix: /createHonoApp\(\{ kernel, prefix: '([^']*)' \}\)/.exec(clause)?.[1], + basePath: /new AuthPlugin\(\{ basePath: '([^']*)' \}\)/.exec(clause)?.[1], + })); + }; + + it.each(REFUSING_COMPOSITIONS)('⭐ $what — refuses, and every Fix it prints CONSTRUCTS', ({ basePath, prefix }) => { + let thrown: Error | undefined; + try { + createHonoApp({ kernel: kernelWith(authServiceAt(basePath)), prefix }); + } catch (err) { + thrown = err as Error; + } + expect(thrown, 'this composition is inside the refusal domain and must refuse').toBeDefined(); + + const fixes = fixesIn(thrown!.message); + expect(fixes.length, 'a refusal with no actionable fix is the defect this case exists for').toBeGreaterThan(0); + for (const fix of fixes) { + expect(fix.prefix ?? fix.basePath, 'every Fix clause must name something to change').toBeDefined(); + expect( + () => createHonoApp({ + kernel: kernelWith(authServiceAt(fix.basePath ?? basePath)), + prefix: fix.prefix ?? prefix, + }), + `the refusal's own advice must build an app — ${JSON.stringify(fix)}`, + ).not.toThrow(); + } + }); + + it('⛔ never suggests `prefix: \'/\'` — it mounts every other route under `//`', () => { + // `/auth`'s parent namespace IS the root, and `'/'` makes the dispatcher + // catch-all `'//*'`: 404 for everything. The first spelling suggested it. + let thrown: Error | undefined; + try { + createHonoApp({ kernel: kernelWith(authServiceAt('/auth')), prefix: '/api' }); + } catch (err) { + thrown = err as Error; + } + expect(thrown).toBeDefined(); + expect(thrown!.message).not.toContain("prefix: '/'"); + }); + + it('⛔ never suggests a basePath that refuses AGAIN — the no-leading-slash prefix', () => { + let thrown: Error | undefined; + try { + createHonoApp({ kernel: kernelWith(authServiceAt('/api/v1/auth')), prefix: 'api/v1' }); + } catch (err) { + thrown = err as Error; + } + expect(thrown).toBeDefined(); + // A base path is normalised to start with `/`, so it can never sit inside a + // prefix that does not — this is precisely what the first spelling advised. + expect(thrown!.message).not.toContain("new AuthPlugin({ basePath: 'api/v1/auth' })"); + expect(thrown!.message).toContain("prefix: '/api/v1'"); + }); + + it('⭐ the over-refusal control, widened: trailing slashes and the root still build', () => { + // The reviewer measured these constructing on both trees; the original + // control covered only `undefined`, `/api`, `/api/v1`, `/api/v1/auth`. + const svc = authServiceAt('/api/v1/auth'); + for (const prefix of ['', '/', '/api/', '/api/v1/'] as const) { + expect( + () => createHonoApp({ kernel: kernelWith(svc), prefix }), + `prefix ${JSON.stringify(prefix)}`, + ).not.toThrow(); + } + }); + + it('refuses a base that only SHARES A PREFIX STRING with the namespace', () => { + // `/apifoo/auth` starts with the five characters of `/api` and is not + // inside it — the same segment-boundary trap #16026 closed one layer down. + const svc = authServiceAt('/apifoo/auth'); + expect(() => createHonoApp({ kernel: kernelWith(svc), prefix: '/api' })).toThrow(/apifoo/); + }); +}); + +describe('#16025 residuals: what stays exactly as it was', () => { + it('an auth service that does not answer getBasePath keeps the ${prefix}/auth mount', async () => { + const svc = authServiceWithoutAccessor(); + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: '/api/v1' }); + + const res = await app.request('http://localhost/api/v1/auth/delete-user', { method: 'POST' }); + + expect(res.status).toBe(401); + expect(svc.handleRequest).toHaveBeenCalledTimes(1); + }); + + it('…and buys no refusal, because nothing here can tell aligned from misaligned', () => { + const svc = authServiceWithoutAccessor(); + expect(() => createHonoApp({ kernel: kernelWith(svc), prefix: '/custom' })).not.toThrow(); + }); + + it('a kernel with no auth service at all still mounts, and still reaches the dispatcher fallback', async () => { + mockDispatcher.handleAuth.mockResolvedValue({ handled: true, response: { body: { ok: true }, status: 200 } }); + const app: Hono = createHonoApp({ kernel: kernelWith(undefined) }); + + const res = await app.request('http://localhost/api/auth/anything', { method: 'POST' }); + + expect(res.status).toBe(200); + expect(mockDispatcher.handleAuth).toHaveBeenCalled(); + }); + + it('a factory-registered auth service (sync accessor throws) degrades to the legacy mount', async () => { + const app: Hono = createHonoApp({ kernel: kernelWithAsyncOnlyAuth(), prefix: '/api/v1' }); + + // No throw at construction, and the pre-#16025 mount is still in place. + await app.request('http://localhost/api/v1/auth/anything', { method: 'POST' }); + expect(mockDispatcher.handleAuth).toHaveBeenCalled(); + }); + + it('an unusable getBasePath answer degrades instead of moving the mount to nonsense', async () => { + const answers: unknown[] = [undefined, null, 42, '', ' ', '/', '//']; + for (const answer of answers) { + const svc = authServiceAt(answer); + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: '/api/v1' }); + const res = await app.request('http://localhost/api/v1/auth/delete-user', { method: 'POST' }); + expect(res.status, `answer ${JSON.stringify(answer)}`).toBe(401); + expect(svc.handleRequest, `answer ${JSON.stringify(answer)}`).toHaveBeenCalledTimes(1); + } + }); + + it('a getBasePath that THROWS degrades to the legacy mount rather than taking boot down', async () => { + const svc = { + handleRequest: vi.fn(async () => unauthorized()), + getBasePath: () => { throw new Error('service is still starting'); }, + }; + const app: Hono = createHonoApp({ kernel: kernelWith(svc), prefix: '/api/v1' }); + + const res = await app.request('http://localhost/api/v1/auth/delete-user', { method: 'POST' }); + expect(res.status).toBe(401); + expect(svc.handleRequest).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/adapters/hono/src/index.ts b/packages/adapters/hono/src/index.ts index ba7ea91924..84a253c64b 100644 --- a/packages/adapters/hono/src/index.ts +++ b/packages/adapters/hono/src/index.ts @@ -97,8 +97,176 @@ interface AuthService { * 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. + * + * [#16025] That disagreement is what the mount itself now avoids: it is + * derived from the same `basePath` (see `resolveAuthMount`), so on every + * service that answers `getBasePath` the request this predicate is asked + * about is already under the base it answers on. */ ownsRoute?(request: Request): Promise; + /** + * Where does this service's OWN router serve, i.e. what did it configure as + * its `basePath`? (#16025) + * + * Optional for the same reason `ownsRoute` is: this is a structural + * interface over whatever the kernel registered as `auth`, and an + * implementation predating the accessor must keep working. `AuthPlugin`'s + * `AuthManager` implements it, returning the very string it hands + * better-auth. A service that does not answer leaves the mount where it was + * before this card — see `resolveAuthMount`. + */ + getBasePath?(): string; +} + +/** + * The auth service's configured `basePath`, read at app-construction time, or + * `undefined` when there is nothing to read. (#16025) + * + * ## Why the SYNC accessor + * + * `createHonoApp` is synchronous and returns a mounted `Hono`, so the mount + * path has to be decided before any request exists. `kernel.getService` is the + * synchronous registry lookup; measured on a real boot it returns the very + * same `AuthManager` instance `getServiceAsync` resolves. It throws for a + * FACTORY-registered service that has not been instantiated ("is async - use + * await") exactly as it throws for a service nobody registered — both are + * "cannot read it here", and both land on the pre-#16025 mount rather than on + * a guess. + * + * ⛔ Every non-string, every throw and every empty answer is `undefined`. This + * function can only ever MOVE the mount onto an answer the auth service gave; + * it can never invent one. + */ +function readAuthBasePath(kernel: ObjectKernel): string | undefined { + let service: AuthService | null | undefined; + try { + const getService = (kernel as any)?.getService; + if (typeof getService !== 'function') return undefined; + service = getService.call(kernel, 'auth') as AuthService | null | undefined; + } catch { + return undefined; + } + if (!service || typeof service.getBasePath !== 'function') return undefined; + let raw: unknown; + try { + raw = service.getBasePath(); + } catch { + return undefined; + } + if (typeof raw !== 'string') return undefined; + const trimmed = raw.trim(); + if (trimmed === '' || trimmed === '/') return undefined; + const withSlash = trimmed.startsWith('/') ? trimmed : `/${trimmed}`; + const normalised = withSlash.replace(/\/+$/, ''); + return normalised === '' ? undefined : normalised; +} + +/** Is `path` the namespace `prefix` names, or something inside it? */ +function isUnderPrefix(path: string, prefix: string): boolean { + const base = prefix.replace(/\/+$/, ''); + if (base === '') return true; + return path === base || path.startsWith(`${base}/`); +} + +/** + * The fixes the refusal offers, each one CHECKED against the same predicate the + * refusal itself uses — because a `Fix:` line that does not fix is a false + * sentence in shipped code, and the first spelling of this message carried two. + * + * Two directions, and the caller picks: + * + * A — move the app UP to the namespace the base already sits in. Offered only + * when that parent is a USABLE prefix. The parent of a single-segment base + * such as `/auth` is `''`, which `createHonoApp` coerces straight back to + * `/api` (`options.prefix || '/api'`), and the `'/'` that reads as its + * equivalent mounts every OTHER route of the app under `//` — measured + * 404 for everything. Suggesting either is advice that does not work, and + * `'/'` is exactly what the first spelling suggested. + * B — move better-auth DOWN under the prefix the caller asked for. Always + * available, but only with a prefix carrying a LEADING SLASH: a base path + * is normalised to start with one and `isUnderPrefix` compares the two as + * written, so NO base path can sit inside a prefix spelled `api/v1`. The + * first spelling suggested `new AuthPlugin({ basePath: 'api/v1/auth' })` + * for exactly that prefix, and it refuses again. + * + * ⛔ This changes what the refusal SAYS, never which compositions it refuses. + */ +function authMountFixes(basePath: string, prefix: string): string[] { + const fixes: string[] = []; + + const parent = basePath.split('/').slice(0, -1).join('/'); + if (parent !== '' && isUnderPrefix(basePath, parent)) { + fixes.push(`pass a prefix the base path sits under (createHonoApp({ kernel, prefix: '${parent}' }))`); + } + + const rooted = (prefix.startsWith('/') ? prefix : `/${prefix}`).replace(/\/+$/, ''); + const candidate = `${rooted}/auth`; + if (isUnderPrefix(candidate, rooted)) { + fixes.push( + prefix.startsWith('/') + ? `configure the auth service to serve under this prefix (new AuthPlugin({ basePath: '${candidate}' }))` + : `spell the prefix with a leading slash and configure the auth service under it ` + + `(createHonoApp({ kernel, prefix: '${rooted}' }) with new AuthPlugin({ basePath: '${candidate}' })) — ` + + `a base path always starts with '/', so it can never sit inside a prefix that does not`, + ); + } + + return fixes; +} + +/** + * Where the `/auth/*` mount goes, and the boot refusal that guards it (#16025). + * + * ## B — the mount FOLLOWS THE AUTH SERVICE + * + * Maintainer ruling of 2026-09-06 (director batch #54), options A + B. The + * mount is derived from the auth service's own `basePath`, not from this + * adapter's `prefix`, because the two defaults do not compose and the failure + * was invisible. Measured on a real boot through this adapter, before the fix, + * with the documented embed `createHonoApp({ kernel })`: + * + * POST /api/auth/sign-in/email (valid shape, wrong password) -> 200 {} + * GET /api/auth/get-session -> 200 {} + * POST /api/auth/sign-up/email -> 200 {} + * + * — while the same boot answered the auth service directly at its own base: + * + * POST /api/v1/auth/delete-user -> 401 {"message":"Unauthorized","code":"UNAUTHORIZED"} + * + * A failed sign-in answering `200 {}` is the silent-success shape: a client + * reading `res.ok` sends the user into an authenticated view with no session. + * ⛔ Neither default moves — options C and D were rejected in the same ruling. + * + * ## A — and a MISALIGNED prefix refuses out loud + * + * Following the auth service makes the two line up by construction whenever + * the base sits inside the namespace the host asked for, which is true of both + * defaults (`/api/v1/auth` is under `/api`). It does NOT when a caller passes + * a `prefix` the base is outside of: the auth surface would then be served + * outside the namespace the host mounted, and `${prefix}/auth/*` would be + * answered by the terminal dispatcher catch-all — the `200 {}` above. That is + * the one combination this function refuses, naming both values, because the + * ruling's floor is that no combination may fail silently. + * + * ⚠️ Residual, recorded rather than implied: an auth service that does not + * answer `getBasePath` keeps the pre-#16025 mount and buys no refusal, because + * nothing here can tell an aligned custom service from a misaligned one. That + * is the behaviour before this change, not a new one. + */ +function resolveAuthMount(kernel: ObjectKernel, prefix: string): string { + const basePath = readAuthBasePath(kernel); + if (basePath === undefined) return `${prefix}/auth`; + if (!isUnderPrefix(basePath, prefix)) { + throw new Error( + `[@objectstack/hono] createHonoApp cannot mount the auth surface: the auth service serves ` + + `better-auth under basePath "${basePath}", which is not inside this app's prefix "${prefix}". ` + + `Mounting it anyway would put auth outside the namespace this app was given, and every request to ` + + `"${prefix}/auth/*" would be answered by the dispatcher catch-all instead — a 200 with an empty body, ` + + `which reads as success on a failed sign-in. Fix — ` + + `${authMountFixes(basePath, prefix).join('; or ')}.`, + ); + } + return basePath; } /** @@ -133,6 +301,11 @@ export function objectStackMiddleware(kernel: ObjectKernel) { export function createHonoApp(options: ObjectStackHonoOptions): Hono { const app = new Hono(); const prefix = options.prefix || '/api'; + // [#16025] Where `/auth/*` is mounted, and the boot refusal that guards it. + // Computed BEFORE any route is registered so a misaligned composition never + // gets a half-built app: see `resolveAuthMount` for the ruling and the + // measurement. + const authMount = resolveAuthMount(options.kernel, prefix); // ADR-0006 Phase 5: env resolution + multi-kernel routing belong to the // host's KernelResolver (the dispatcher resolves the `kernel-resolver` // service itself). The legacy envRegistry/kernelManager options are @@ -325,7 +498,7 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { /** * Hand a path THIS mount does not own to whatever else matched (#4117). * - * The `${prefix}/auth/*` mount below claims a whole namespace and used to be + * The `${authMount}/*` mount below claims a whole namespace and used to be * TERMINAL — it answered 404 for a path its auth service does not implement. * That is #4088's shape, which cost four fixes before #4116's scan started * enumerating it, and it is what #4087/#4112 had already concluded about the @@ -371,9 +544,9 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { }; // --- Auth (needs auth service integration) --- - app.all(`${prefix}/auth/*`, async (c, next) => { + app.all(`${authMount}/*`, async (c, next) => { try { - const path = c.req.path.substring(`${prefix}/auth/`.length); + const path = c.req.path.substring(authMount.length + 1); const method = c.req.method; // Try AuthPlugin service first (prefer async to support factory-based services) @@ -456,8 +629,10 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { // `@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 + // ⛔ #15928 left the mount untouched; what it narrowed is which 404 + // may be handed on. (#16025 later moved WHERE the mount sits — see + // `resolveAuthMount` — without touching this decision.) + // `/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. diff --git a/packages/plugins/plugin-auth/src/auth-manager-base-path.test.ts b/packages/plugins/plugin-auth/src/auth-manager-base-path.test.ts new file mode 100644 index 0000000000..790cca8edd --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-manager-base-path.test.ts @@ -0,0 +1,179 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #16025 — `AuthManager.getBasePath()`: the string an HTTP adapter reads to +// learn where better-auth serves. +// +// ⛔ NOT "the one definition" of that value, which an earlier spelling of this +// header claimed. Two more readers of `this.config.basePath` are live in +// `auth-manager.ts` — `getAuthIssuer()` and `getMcpResourceUrl()`, each with its +// own normaliser — and they are deliberately untouched: they are published OAuth +// identifiers, compared by exact string. The accessor's docblock carries the +// measurement and the reason. +// +// ## Why this member is public, and why a rename is a breaking change +// +// An HTTP adapter that mounts this service has to know where its routes live, +// and no member answered THAT question. (The value was not unreachable — +// `getAuthIssuer()` is public and its URL path is the configured base path — but +// parsing a path back out of an issuer identifier is reading a different +// contract that happens to contain the answer.) So `@objectstack/hono`'s +// `createHonoApp` mounted the auth surface under its OWN `prefix` option, whose +// default (`/api`) does not compose with this one (`/api/v1/auth`). Measured on +// a real boot with the documented embed `createHonoApp({ kernel })`, before the +// fix: +// +// POST /api/auth/sign-in/email (valid shape, wrong password) -> 200 {} +// +// `createHonoApp` now derives the mount from this method, by name, through a +// structural interface (the adapter does not depend on this package). ⇒ A +// rename or removal here silently returns that adapter to the mount above. +// +// ## ⛔ getBasePath() is NOT the string better-auth is handed +// +// `createAuthInstance` passes `configuredBasePath()` — the configured value +// VERBATIM — and the two differ exactly when a trailing slash is configured or +// a leading one is missing. That gap is deliberate: better-auth stamps the +// OAuth access-token `iss` from `baseURL + the string it was handed`, and +// `verifyMcpAccessToken` compares `iss` against `getAuthIssuer()`, which keeps +// the configured trailing slash. A draft of this card normalised the handed +// string and every MCP access token minted under a trailing-slash `basePath` +// was then rejected by this manager's own verifier. The cases below pin that +// pair on a REAL `betterAuth()` instance — `getAuthInstance().options.basePath` +// is what `createAuthInstance` actually passed, not a copy of the expression. +// +// ⛔ What is still NOT assertable from this package: that better-auth ROUTES +// under `getBasePath()`'s normalised answer on a real kernel boot. That runs in +// `@objectstack/verify` (`auth-base-path-contract.test.ts`), the nearest package +// that can hold a live better-auth and this manager at once. + +import { describe, it, expect } from 'vitest'; +import { AuthManager } from './auth-manager'; +import type { AuthManagerOptions } from './auth-manager'; + +const managerWith = (basePath?: unknown) => + new AuthManager({ ...(basePath === undefined ? {} : { basePath }) } as unknown as AuthManagerOptions); + +describe('#16025 AuthManager.getBasePath', () => { + it('is a public member — the surface @objectstack/hono reads by name', () => { + expect(typeof managerWith().getBasePath).toBe('function'); + }); + + it('defaults to the shipped base path when nothing is configured', () => { + expect(managerWith().getBasePath()).toBe('/api/v1/auth'); + }); + + it('answers the CONFIGURED base path, which is the point of asking', () => { + expect(managerWith('/api/v9/identity').getBasePath()).toBe('/api/v9/identity'); + }); + + it('normalises every spelling of the base path to the one an adapter can mount', () => { + // This is exactly the normalisation `betterAuthEndpointPath` has always + // applied; the method gives it a name and makes it public. ⛔ It does NOT + // change what better-auth is handed — see the real-instance cases at the + // bottom of this file. A configured `api/v1/auth` still reaches better-auth + // WITHOUT its leading slash while the ownership walk tests `/api/v1/auth`; + // they disagree as STRINGS and not as behaviour, because better-auth and + // better-call tolerate the missing slash, so `handleRequest` answers `200` + // and `ownsRoute` answers `true` on the same request. That divergence is + // latent and is NOT repaired here. + expect(managerWith('api/v1/auth').getBasePath()).toBe('/api/v1/auth'); + expect(managerWith('/api/v1/auth/').getBasePath()).toBe('/api/v1/auth'); + expect(managerWith('/api/v1/auth///').getBasePath()).toBe('/api/v1/auth'); + expect(managerWith('api/v1/auth/').getBasePath()).toBe('/api/v1/auth'); + }); + + it('treats an empty configured value as unset, exactly as the pre-#16025 readers did', () => { + expect(managerWith('').getBasePath()).toBe('/api/v1/auth'); + }); + + it('leaves a configured root as the empty base — unchanged behaviour, pinned so it is a decision', () => { + // `'/'` normalises to `''`, which is what `betterAuthEndpointPath` has + // always computed for it. The hono adapter rejects that answer as unusable + // and keeps its previous mount rather than mounting at the app root. + expect(managerWith('/').getBasePath()).toBe(''); + }); +}); + +/** + * The pair that broke, pinned where it broke. + * + * `getAuthInstance()` builds the real `betterAuth()` from `createAuthInstance`, + * so `options.basePath` is the string that site actually passed — an edit that + * normalises it again turns these red no matter which expression it uses. + */ +describe('#16025 what better-auth is actually configured with', () => { + const withSecret = (basePath: string) => + new AuthManager({ basePath, secret: 'x'.repeat(40) } as unknown as AuthManagerOptions); + + it('is the configured base path VERBATIM — a trailing slash survives', async () => { + const auth = await withSecret('/api/v1/auth/').getAuthInstance(); + expect(auth.options.basePath).toBe('/api/v1/auth/'); + }); + + it('⭐ agrees with getAuthIssuer() for every spelling — the iss verifyMcpAccessToken compares', async () => { + // better-auth's `ctx.baseURL` is `baseURL` + this string (adding a leading + // slash if absent), and @better-auth/oauth-provider stamps the access-token + // `iss` from it. `verifyMcpAccessToken` hands jose `issuer: + // getAuthIssuer()`, compared by EXACT string. So this is the pair whose + // disagreement rejects live tokens. + for (const configured of ['/api/v1/auth', '/api/v1/auth/', '/api/v9/identity/', 'api/v1/auth']) { + const manager = withSecret(configured); + const handed = (await manager.getAuthInstance()).options.basePath as string; + const rooted = handed.startsWith('/') ? handed : `/${handed}`; + expect(new URL(manager.getAuthIssuer()).pathname).toBe(rooted); + } + }); + + it('⛔ and is NOT getBasePath() when a trailing slash is configured — the gap is the point', async () => { + const manager = withSecret('/api/v1/auth/'); + expect((await manager.getAuthInstance()).options.basePath).toBe('/api/v1/auth/'); + expect(manager.getBasePath()).toBe('/api/v1/auth'); + }); +}); + +/** + * The MIRROR direction of the same split, pinned where it breaks. + * + * The three real-instance cases above guard ONE side: an edit that normalises + * the string handed to better-auth turns them red. Nothing guarded the other + * side. Point `betterAuthEndpointPath` at `configuredBasePath()` instead of + * `getBasePath()` — the mistake in the same shape, one method along — and + * every pin in this package stays green while `ownsRoute` stops recognising + * better-auth's own routes on every configured spelling that is not ALREADY + * normalised. + * + * ⚠️ That is #15928's class returning, not a cosmetic drift. `ownsRoute` + * answering `false` is what lets the auth catch-all YIELD better-auth's own + * 404s (`auth-catchall-yield.test.ts`), so a downstream wildcard answers + * `200 {}` where a real refusal stood — under a trailing-slash or + * no-leading-slash deployment only, which is exactly why no existing pin and no + * default composition could see it. + * + * ⭐ The two cases below discriminate BECAUSE the configured spelling is not + * the normalised one; the control that follows them does not, and is here to + * say so. `${getBasePath()}/get-session` is the URL an adapter that mounts on + * `getBasePath()` actually produces, so these ask the shipped question. + */ +describe('#16025 the ownership walk follows getBasePath(), not the configured spelling', () => { + const withSecret = (basePath: string) => + new AuthManager({ basePath, secret: 'x'.repeat(40) } as unknown as AuthManagerOptions); + + /** `ownsRoute` for a route better-auth really routes, addressed at the mount. */ + const ownsGetSession = (configured: string) => { + const manager = withSecret(configured); + const url = `http://localhost:3000${manager.getBasePath()}/get-session`; + return manager.ownsRoute(new Request(url, { method: 'GET' })); + }; + + it('⭐ owns …/get-session when a TRAILING SLASH is configured', async () => { + await expect(ownsGetSession('/api/v1/auth/')).resolves.toBe(true); + }); + + it('⭐ owns …/get-session when the LEADING SLASH is missing', async () => { + await expect(ownsGetSession('api/v1/auth')).resolves.toBe(true); + }); + + it('control — the already-normalised spelling, which the mirror mutation cannot move', async () => { + await expect(ownsGetSession('/api/v1/auth')).resolves.toBe(true); + }); +}); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 574c92703b..08634124f5 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -1249,7 +1249,7 @@ export class AuthManager { // bare host) so the reset-password / verify-email / magic-link URLs // better-auth derives from baseURL are always clickable links. baseURL: this.getCanonicalOrigin(), - basePath: this.config.basePath || '/api/v1/auth', + basePath: this.configuredBasePath(), // Database adapter configuration database: this.createDatabaseConfig(), @@ -5445,6 +5445,133 @@ export class AuthManager { return response; } + /** + * [#16025] The `basePath` string this manager hands better-auth: the + * configured value VERBATIM, or the shipped default when nothing is + * configured. Unchanged from before this card — only the reading of it moved + * here, so `getBasePath()` and this cannot drift apart by accident. + * + * ## ⛔ Never normalise here + * + * better-auth stamps the OAuth access-token `iss` from `ctx.context.baseURL`, + * which is `baseURL` + THIS string (`@better-auth/oauth-provider` 1.7.2: + * `iss: jwtPluginOptions?.jwt?.issuer ?? ctx.context.baseURL`, and this + * manager sets no `jwt.issuer`), while `verifyMcpAccessToken` hands + * `jose.jwtVerify` `issuer: getAuthIssuer()` — the configured value with a + * leading slash ADDED and a trailing one KEPT. `jose` compares `iss` by exact + * string, so the two agree only while this string is the configured one. + * Measured on bare better-auth 1.7.2 + `@better-auth/oauth-provider` 1.7.2 + * (memory adapter, this manager's own plugin wiring), a real + * `client_credentials` token, configured `basePath: '/api/v1/auth/'`: + * + * handed '/api/v1/auth/' ctx.baseURL …/auth/ iss …/auth/ verifier …/auth/ -> OK + * handed '/api/v1/auth' ctx.baseURL …/auth iss …/auth verifier …/auth/ -> REJECTED + * ERR_JWT_CLAIM_VALIDATION_FAILED: unexpected "iss" claim value + * + * ⇒ normalising this string rejects every MCP access token the deployment + * mints, for as long as a trailing slash is configured — fail-closed, and + * permanent. A draft of this card did exactly that. `getBasePath()` is the + * NORMALISED view an HTTP adapter mounts on and is deliberately NOT this; + * making the two one value moves a published OAuth identifier, which is + * #16399's decision, not this card's. + */ + private configuredBasePath(): string { + return this.config.basePath || '/api/v1/auth'; + } + + /** + * [#16025] The path prefix better-auth's routes are reachable under, in the + * single NORMALISED spelling an HTTP adapter can mount: a leading slash added + * when absent, trailing slashes stripped. + * + * ## Why this is public + * + * An HTTP adapter that mounts this service has to know where its routes live, + * and it had no member that answers THAT question. `config` is private. The + * value was not unreachable, though, and an earlier draft of this docblock + * said it was: `getAuthIssuer()` is public and its URL PATH is the configured + * base path (`http://localhost:3000/api/v1/auth`) — `auth-plugin.ts:3176` + * already reads a path that way, off `getMcpResourceUrl()`. What an adapter + * would have been doing is parsing a path back out of an OAuth issuer + * identifier, which is a different contract that happens to contain the + * answer. A dedicated accessor is the cleaner design; being the ONLY exposure + * was never the reason for it. + * + * `@objectstack/hono`'s `createHonoApp` mounted the auth surface under its OWN + * `prefix` option instead, whose default (`/api`) does not compose with this + * one (`/api/v1/auth`), so on the documented embed better-auth was never + * reached at all — measured, on a real boot: + * + * POST /api/auth/sign-in/email (valid shape, wrong password) -> 200 {} + * POST /api/v1/auth/delete-user -> 401 {"message":"Unauthorized","code":"UNAUTHORIZED"} + * + * ## ⛔ What this method is NOT — stated because the first spelling claimed it + * + * **It is NOT the string handed to better-auth.** `createAuthInstance` passes + * `configuredBasePath()`, the configured value verbatim, and the two differ + * exactly when the configured spelling carries a trailing slash or lacks a + * leading one. That difference is deliberate and load-bearing — see + * `configuredBasePath()` for the token rejection normalising there causes. + * What the two DO share is the wire paths they serve: better-call strips a + * trailing slash when routing and better-auth adds a missing leading one, so + * a mount at `/api/v1/auth/*` reaches a better-auth configured with + * `/api/v1/auth/` — measured on the same probe, which drove its whole OAuth + * exchange through that mount. + * + * **It is NOT the single definition of the base path.** FOUR readers of + * `this.config.basePath` existed in this file; this card leaves THREE, by + * collapsing the string handed to better-auth and `betterAuthEndpointPath`'s + * normalising copy onto `configuredBasePath()`. The two that remain keep + * their own normalisers: + * + * getAuthIssuer() adds a leading slash, KEEPS a trailing one + * getMcpResourceUrl() adds nothing, strips a trailing `/auth` + * + * They are deliberately untouched, and collapsing them is not a free move. + * `getAuthIssuer()` is the `iss` this AS advertises and `getMcpResourceUrl()` + * is the RFC 8707 resource identifier a token's `aud` is matched against — + * both compared by exact string by relying parties, so moving either + * re-selects tokens. Measured on this manager, at this commit: + * + * basePath '/api/v1/auth/' getAuthIssuer() -> …/api/v1/auth/ (trailing slash KEPT — + * and better-auth is handed + * the same spelling, which is + * why the pair still agrees) + * basePath 'api/v1/auth' getMcpResourceUrl() -> http://localhost:3000api/v1/mcp + * (malformed; pre-existing, + * unchanged by this card) + * + * ⇒ ⛔ Do not read this method as licence to assume one answer exists. Two + * more spellings of "the auth base path" are live in this file, and retiring + * them is a decision about published OAuth identifiers, not a tidy-up. Filed + * as #16399 rather than taken on a mount card. + * + * ## ⛔ No value moves — what this card actually changed here + * + * Every one of the three readers answers exactly what it answered on the + * merge base, for every configured spelling. `betterAuthEndpointPath` already + * applied this normalisation; better-auth already received the raw configured + * string. What is new is that the normalisation has a name and is PUBLIC, so + * an adapter can mount on it. A configured `'/'` still normalises to `''`, + * unchanged from before. + * + * ## What the collapsed readers actually disagreed about + * + * As STRINGS, and not as behaviour — measured, not inferred. A configured + * `'api/v1/auth'` reaches better-auth without its leading slash while the + * ownership walk tests `'/api/v1/auth'`; but better-auth/better-call tolerate + * the missing slash, so `handleRequest` answers `200` and `ownsRoute` answers + * `true` on the SAME request. The divergence is LATENT — and ⛔ it is NOT + * repaired here, only given one name per side: the mount and the ownership + * walk read `getBasePath()`, better-auth still receives the configured + * spelling. Repairing it means changing what better-auth is configured with, + * which is the very move measured above to reject live tokens. + */ + getBasePath(): string { + const configured = this.configuredBasePath(); + return (configured.startsWith('/') ? configured : `/${configured}`).replace(/\/+$/, ''); + } + /** * [#15417] Does better-auth ROUTE this request — i.e. is the path one its own * router owns, whatever it then answers? @@ -5499,8 +5626,7 @@ export class AuthManager { } catch { return undefined; } - const configured = this.config.basePath || '/api/v1/auth'; - const base = (configured.startsWith('/') ? configured : `/${configured}`).replace(/\/+$/, ''); + const base = this.getBasePath(); if (!pathname.startsWith(base)) return undefined; const endpoint = pathname.slice(base.length).replace(/\/+$/, ''); return endpoint.startsWith('/') ? endpoint : undefined; diff --git a/packages/verify/src/auth-base-path-contract.test.ts b/packages/verify/src/auth-base-path-contract.test.ts new file mode 100644 index 0000000000..db9962c732 --- /dev/null +++ b/packages/verify/src/auth-base-path-contract.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #16025 — the contract an HTTP adapter mounts the auth surface on, pinned on a +// REAL boot. +// +// `@objectstack/hono`'s `createHonoApp` derives its `/auth/*` mount from the +// auth service's own `basePath` (maintainer ruling 2026-09-06, director batch +// #54, options A + B). It reads that value by calling `getBasePath()` on +// whatever the kernel registered as `auth`, through a structural interface — +// the adapter neither depends on `@objectstack/plugin-auth` nor may. So two +// facts hold the mount up, and NEITHER is observable from the adapter's own +// package: +// +// ① the registered `auth` service really carries `getBasePath`, and it is +// reachable through the SYNCHRONOUS `kernel.getService`, which is the only +// accessor a synchronous `createHonoApp` can use; +// ② better-auth really ROUTES under the string it answers. +// +// ⛔ Fact ② does not hold because the two are one string, and an earlier +// spelling of this header said it did. `createAuthInstance` hands better-auth +// the CONFIGURED `basePath` verbatim while `getBasePath()` answers its +// normalised form; they differ exactly when a trailing slash is configured, and +// that gap is deliberate — normalising the handed string moves the OAuth +// access-token `iss` and this manager's own verifier then rejects the tokens it +// mints (`auth-manager.ts`, `configuredBasePath()`, carries the measurement). +// What holds the mount up is narrower and is what the rows below assert: the +// WIRE PATHS under `getBasePath()`'s answer are the ones better-auth routes. +// +// This file is where they are observable: `@objectstack/verify` boots the real +// kernel with the real `AuthPlugin`. ⛔ Neither fact may be inferred from the +// adapter's fixture-driven cases in `hono-auth-mount-basepath.test.ts`; that +// file pins the adapter's RULE against a stub and says so. +// +// ── The defect this exists to keep closed ────────────────────────────────── +// +// Measured on this harness before the fix, with the documented embed +// `createHonoApp({ kernel })` — adapter prefix defaulting to `/api`, auth +// basePath defaulting to `/api/v1/auth`: +// +// POST /api/auth/sign-in/email (valid shape, wrong password) -> 200 {} +// GET /api/auth/get-session -> 200 {} +// POST /api/auth/sign-up/email -> 200 {} +// +// A failed sign-in answering `200 {}` is the silent-success shape. It was +// produced by mounting `/auth/*` under the ADAPTER's prefix, forwarding a path +// better-auth does not route, and letting the resulting 404 fall to a terminal +// catch-all. The rows below are the same composition seen from the service +// side, which is where the two paths are told apart. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack } from './harness.js'; + +/** The auth service surface an HTTP adapter mounts against. */ +interface AuthServiceShape { + handleRequest(request: Request): Promise; + ownsRoute(request: Request): Promise; + getBasePath(): string; +} + +const app = { + manifest: { + id: 'com.example.auth-base-path', + namespace: 'authbasepath', + version: '0.0.1', + type: 'app', + name: 'Auth Base Path Fixture', + }, + objects: [], +}; + +const BOOT_TIMEOUT = 180_000; + +// One boot for the whole file: every case reads the same live AuthManager, and +// booting the stack per case is the expensive half of this suite. +let stack: Awaited>; +let auth: AuthServiceShape; + +beforeAll(async () => { + stack = await bootStack(app); + // ⭐ The SYNC accessor on purpose: `createHonoApp` is synchronous and decides + // the mount before any request exists, so an `auth` service reachable only + // through `getServiceAsync` would leave the mount where it was. + auth = stack.kernel.getService('auth') as unknown as AuthServiceShape; +}, BOOT_TIMEOUT); + +afterAll(async () => { + await stack?.stop(); +}, BOOT_TIMEOUT); + +const req = (method: string, path: string, body?: string) => + new Request(`http://localhost${path}`, { + method, + ...(body === undefined ? {} : { headers: { 'content-type': 'application/json' }, body }), + }); + +describe('#16025 fact ①: the registered auth service says where it serves', () => { + it('carries getBasePath, synchronously reachable, answering an absolute path', () => { + expect(typeof auth.getBasePath).toBe('function'); + const base = auth.getBasePath(); + expect(typeof base).toBe('string'); + expect(base.startsWith('/')).toBe(true); + expect(base.endsWith('/')).toBe(false); + expect(base.length).toBeGreaterThan(1); + }); + + it('is the same instance the async accessor resolves', async () => { + expect(auth).toBe(await stack.kernel.getServiceAsync('auth')); + }); +}); + +describe('#16025 fact ②: better-auth routes under exactly that answer', () => { + it('routes its own endpoints under the answered base', async () => { + const base = auth.getBasePath(); + expect(await auth.ownsRoute(req('POST', `${base}/sign-in/email`))).toBe(true); + expect(await auth.ownsRoute(req('GET', `${base}/get-session`))).toBe(true); + }); + + it('⭐ and NOT under a different base — the control that makes the row above mean something', async () => { + // Without this, an `ownsRoute` that answered `true` for everything would + // satisfy the case above while telling the adapter nothing. + expect(await auth.ownsRoute(req('POST', '/somewhere-else/sign-in/email'))).toBe(false); + expect(await auth.ownsRoute(req('GET', '/somewhere-else/get-session'))).toBe(false); + }); + + it('answers for real under the answered base — the arrival shape', async () => { + const base = auth.getBasePath(); + const res = await auth.handleRequest(req('POST', `${base}/delete-user`, '{}')); + // ADR-0112 envelope: the code and the status. A bare "it did not 404" would + // stay green on a transport that never reached better-auth at all. + expect(res.status).toBe(401); + expect(await res.json()).toEqual({ message: 'Unauthorized', code: 'UNAUTHORIZED' }); + }); +}); + +describe("#16025: the card's own composition, from the service side", () => { + it('disclaims the path the adapter used to mount — prefix `/api` plus `/auth`', async () => { + // The pre-fix wire path. better-auth does not route it, so the adapter's + // mount forwarded a request that could only 404, and the 404 then became + // somebody else's `200 {}`. + expect(await auth.ownsRoute(req('POST', '/api/auth/sign-in/email'))).toBe(false); + const res = await auth.handleRequest(req('POST', '/api/auth/delete-user', '{}')); + expect(res.status).toBe(404); + }); + + it('⭐ the two paths differ, which is the whole defect', () => { + // If a future change made the auth base `/api/auth`, the row above would + // stop being the defect's shape — and this assertion is what says so out + // loud instead of leaving two cases quietly asserting the same thing. + expect(auth.getBasePath()).not.toBe('/api/auth'); + }); +}); diff --git a/scripts/check-wildcard-fallthrough.mjs b/scripts/check-wildcard-fallthrough.mjs index 7dad490313..53f1da0760 100644 --- a/scripts/check-wildcard-fallthrough.mjs +++ b/scripts/check-wildcard-fallthrough.mjs @@ -124,7 +124,16 @@ const MOUNTS = { // conclusion from the other direction — "the wildcard was wider than the two // routes it served". Two independent reads landing on the same defect is the // argument for enumerating the shape rather than finding it by eye each time. - "packages/adapters/hono/src/index.ts:all `${prefix}/auth/*`": { yields: true }, + // + // #16025 renamed the PATTERN, not the handler: the mount is now derived from + // the auth service's own `basePath` (`authMount`) instead of the adapter's + // `prefix`, because the two defaults did not compose and auth was never + // reached on the documented embed. `yields` stays, and stays VERIFIED rather + // than asserted — the handler takes `next` and hands it to `yieldUnowned`, + // which awaits it, and `callsContinuation` counts that hand-off. Neither of + // the other two states would be true here: the mount does not own its + // namespace (`exempt`) and it is not terminal (`ratchet`). + "packages/adapters/hono/src/index.ts:all `${authMount}/*`": { yields: true }, 'packages/plugins/plugin-hono-server/src/adapter.ts:use *': { yields: true }, 'packages/plugins/plugin-hono-server/src/hono-plugin.ts:use *': { yields: true }, diff --git a/skills/objectstack-platform/SKILL.md b/skills/objectstack-platform/SKILL.md index 6b252a2e05..16b85c12fb 100644 --- a/skills/objectstack-platform/SKILL.md +++ b/skills/objectstack-platform/SKILL.md @@ -359,7 +359,7 @@ new DriverPlugin(new SqlDriver({ client: 'pg', connection: process.env.DATABASE_ ## HTTP Layer (Hono) -The HTTP layer is Hono-based. Two packages exist: +Two packages exist: | Package | Export | Use When | |:--------|:-------|:---------| @@ -376,25 +376,20 @@ dispatcher yourself. ```typescript import { createHonoApp } from '@objectstack/hono'; -const app = createHonoApp({ - kernel, // ObjectKernel instance - prefix: '/api', // API route prefix (default: '/api') -}); - -export default app; // Deploy to Cloudflare Workers, Deno, Bun, Node +// prefix defaults to '/api'. +export default createHonoApp({ kernel }); ``` -### Architecture +⚠️ **`prefix` does not move auth.** The `/auth/*` mount follows the auth +service's `basePath` (`AuthPlugin` default `/api/v1/auth`), not `prefix` — +`createHonoApp({ kernel })` reaches better-auth at `/api/v1/auth/*`. A `prefix` +that `basePath` is not inside **refuses at boot**, naming both values. -`createHonoApp` follows this architecture: - -1. Accept a `kernel` (ObjectKernel) instance -2. Create an `HttpDispatcher` internally -3. Mount explicit routes for auth and discovery -4. Delegate everything else to the dispatcher +### Architecture -This means **new routes added to HttpDispatcher work automatically** -without adapter code changes. +`createHonoApp` creates an `HttpDispatcher`, mounts explicit +routes for auth and discovery, and delegates everything else to it — so **new +routes added to HttpDispatcher work automatically**. ---