Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .changeset/adapter-hono-auth-wildcard-yields.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
"@objectstack/adapter-hono": patch
---

fix(adapters/hono): the auth wildcard yields paths the auth service does not own (#4117)

`app.all('${prefix}/auth/*')` claimed a whole namespace and was **terminal**: it
returned the auth service's response unconditionally, including better-auth's 404
for a path it does not implement, and the legacy `handleAuth` bridge's own
`handled: false` 404. That is the #4088 shape, found by #4116's enumeration after
manual greps had missed it.

A 404 from better-auth, or `handled: false` from the dispatcher, now means "not
this mount's path" and the handler yields. The predicate is the dispatcher's own
`handled` flag wherever one exists — an explicit ownership answer beats inferring
one from a status; only the better-auth hand-off lacks such a flag, and there the
404 is the signal, as in #4092.

**What changes on the wire.** An unowned path under `${prefix}/auth/*` used to get
a 404 built by this mount. It now continues to the `${prefix}/*` dispatcher
catch-all and gets a real, gate-carrying `dispatch()` attempt, so a domain handler
registered for such a path becomes reachable — this adapter's actual extension
mechanism. When nothing anywhere claims the path the reply is still the same
enveloped `{ success: false, error: { message: 'Not Found', code: 404 } }`. Paths
the auth service does own are untouched, and a 401/403 from it is never treated as
a disclaimer of ownership.

No configuration changes and no new routes.
145 changes: 145 additions & 0 deletions packages/adapters/hono/src/hono-wildcard-fallthrough.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #4117 — the `/auth/*` mount must yield paths it does not own.
*
* It claims a whole namespace and used to be TERMINAL: it answered 404 for a path
* its auth service does not implement. Found by #4116's scan, which no manual grep
* had managed. Its `/storage/*` twin was ratcheted alongside it and is now gone
* entirely — #4087/#4112 deleted that bridge after reaching the same conclusion
* from the other direction ("the wildcard was wider than the two routes it
* served").
*
* ## What yielding buys HERE, precisely
*
* Not what it bought in #4092. There, yielding made another plugin's route
* reachable. In this adapter it cannot: the `${prefix}/*` dispatcher catch-all
* is registered right after these two and is DELIBERATELY terminal (ADR-0076
* OQ#9, #3576/#3608 — the gate stages live inside `dispatch()`, so splitting it
* into per-prefix Hono mounts would bypass them). So a route registered later
* under `/api/auth/*` is swallowed by the catch-all whether or not these two
* yield, and Hono-route mounting is not this adapter's extension path at all —
* registering a domain handler is.
*
* What yielding buys is that an unowned `/auth/*` or `/storage/*` path now
* reaches that gated `dispatch()` instead of dead-ending in a 404 built two
* mounts earlier. A domain handler registered for such a path becomes
* reachable, which is exactly the adapter's intended mechanism. These tests pin
* that, and pin that the owners still win everything they own.
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import type { Hono } from 'hono';

const mockDispatcher = {
getDiscoveryInfo: vi.fn().mockReturnValue({ version: '1.0', endpoints: [] }),
handleAuth: vi.fn(),
handleGraphQL: vi.fn(),
dispatch: vi.fn(),
};

vi.mock('@objectstack/runtime', () => ({
HttpDispatcher: function HttpDispatcher() { return mockDispatcher; },
}));

import { createHonoApp } from './index';

/** No domain claimed the path — the dispatcher's own ownership signal. */
const UNHANDLED = { handled: false };
const HANDLED = (body: unknown, status = 200) => ({ handled: true, response: { body, status } });

/** A kernel whose `auth` service is (or is not) present. */
const kernelWith = (authService?: unknown) => ({
name: 'test-kernel',
getService: (n: string) => (n === 'auth' && authService ? authService : undefined),
}) as any;

const authServiceReturning = (status: number, body: unknown = { from: 'better-auth' }) => ({
handleRequest: vi.fn(async () => new Response(JSON.stringify(body), {
status,
headers: { 'Content-Type': 'application/json' },
})),
});

describe('auth mount yields paths better-auth does not own (#4117)', () => {
beforeEach(() => {
vi.clearAllMocks();
mockDispatcher.dispatch.mockResolvedValue(UNHANDLED);
mockDispatcher.handleAuth.mockResolvedValue(UNHANDLED);
});

it('reaches the gated dispatch when better-auth 404s', async () => {
// `/auth/me/permissions` is the canonical unowned path: nothing in
// better-auth serves it. Before #4117 this 404'd here; now the request gets
// a real dispatch attempt, so a domain handler for it is reachable.
mockDispatcher.dispatch.mockResolvedValue(HANDLED({ authenticated: true, from: 'dispatch' }));
const app: Hono = createHonoApp({ kernel: kernelWith(authServiceReturning(404)) });

const res = await app.request('/api/auth/me/permissions');

expect(mockDispatcher.dispatch).toHaveBeenCalled();
expect(res.status).toBe(200);
expect(await res.json()).toEqual({ authenticated: true, from: 'dispatch' });
});

it('keeps better-auth winning the paths it DOES own', async () => {
const app: Hono = createHonoApp({ kernel: kernelWith(authServiceReturning(200, { user: null })) });

const res = await app.request('/api/auth/get-session');

expect(res.status).toBe(200);
expect(await res.json()).toEqual({ user: null });
expect(mockDispatcher.dispatch).not.toHaveBeenCalled();
});

it('does NOT yield on 401 — a real answer, not a disclaimer of ownership', async () => {
const app: Hono = createHonoApp({ kernel: kernelWith(authServiceReturning(401, { error: 'nope' })) });

const res = await app.request('/api/auth/protected');

expect(res.status).toBe(401);
expect(await res.json()).toEqual({ error: 'nope' });
expect(mockDispatcher.dispatch).not.toHaveBeenCalled();
});

it('yields on the dispatcher path too, keyed on `handled: false`', async () => {
// No auth service at all, so the mount falls back to `dispatcher.handleAuth`
// — whose explicit `handled` flag beats inferring ownership from a status.
mockDispatcher.dispatch.mockResolvedValue(HANDLED({ currency: 'USD' }));
const app: Hono = createHonoApp({ kernel: kernelWith() });

const res = await app.request('/api/auth/me/localization');

expect(mockDispatcher.handleAuth).toHaveBeenCalled();
expect(mockDispatcher.dispatch).toHaveBeenCalled();
expect(res.status).toBe(200);
expect(await res.json()).toEqual({ currency: 'USD' });
});

it('still lets handleAuth answer what it DOES handle', async () => {
mockDispatcher.handleAuth.mockResolvedValue(HANDLED({ ok: true }));
const app: Hono = createHonoApp({ kernel: kernelWith() });

const res = await app.request('/api/auth/login', { method: 'POST' });

expect(res.status).toBe(200);
expect(await res.json()).toEqual({ ok: true });
expect(mockDispatcher.dispatch).not.toHaveBeenCalled();
});
});

describe('the enveloped 404 survives when nothing anywhere claims the path', () => {
beforeEach(() => vi.clearAllMocks());

it('still ends in the enveloped 404 when nothing anywhere claims it', async () => {
mockDispatcher.handleAuth.mockResolvedValue(UNHANDLED);
mockDispatcher.dispatch.mockResolvedValue(UNHANDLED);
const app: Hono = createHonoApp({ kernel: kernelWith() });

const res = await app.request('/api/auth/nope');

expect(res.status).toBe(404);
// Not Hono's bare "404 Not Found" text — the platform envelope is preserved.
expect(await res.json()).toEqual({ success: false, error: { message: 'Not Found', code: 404 } });
});
});
43 changes: 41 additions & 2 deletions packages/adapters/hono/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,39 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
return c.redirect(prefix);
});

/**
* 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
* 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
* `/storage` bridge it deleted: "the wildcard was wider than the two routes it
* served".
*
* What yielding buys HERE is narrower than in #4092, and worth stating so
* nobody over-reads it. It does NOT make a later-registered Hono route
* reachable: the `${prefix}/*` dispatcher catch-all below is DELIBERATELY
* terminal (ADR-0076 OQ#9, #3576/#3608 — the gate stages live inside
* `dispatch()`), so it would swallow such a route either way, and mounting Hono
* routes is not this adapter's extension path anyway. It means an unowned
* `/auth/*` path now reaches that gated `dispatch()` instead of dead-ending in
* a 404 built one mount earlier, so a domain handler registered for it becomes
* reachable — which IS the adapter's mechanism.
*
* `c.res = …` rather than `return …`: `app.use('*', cors(…))` above puts a
* middleware in this chain, and Hono's compose only assigns a handler's
* RETURNED Response while `c.finalized` is false. Reaching the end of the chain
* runs notFound, which sets a response and flips that flag, so a `return` here
* is silently dropped (learned the hard way in #4092).
*/
const yieldUnowned = async (c: any, next: any, fallback: () => Response) => {
await next();
if (!c.res) c.res = fallback();
};

// --- Auth (needs auth service integration) ---
app.all(`${prefix}/auth/*`, async (c) => {
app.all(`${prefix}/auth/*`, async (c, next) => {
try {
const path = c.req.path.substring(`${prefix}/auth/`.length);
const method = c.req.method;
Expand Down Expand Up @@ -330,17 +361,25 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {

if (authService && typeof authService.handleRequest === 'function') {
const response = await authService.handleRequest(c.req.raw);
return new Response(response.body, {
const forwarded = () => new Response(response.body, {
status: response.status,
headers: response.headers,
});
// 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);
return forwarded();
}

// Fallback to legacy dispatcher
const body = method === 'GET' || method === 'HEAD'
? {}
: await c.req.json().catch(() => ({}));
const result = await dispatcher.handleAuth(path, method, body, { request: c.req.raw });
// `handled: false` is the dispatcher saying no auth domain claimed it —
// an explicit ownership signal, better than inferring one from a status.
if (!result.handled) return yieldUnowned(c, next, () => toResponse(c, result));
return toResponse(c, result);
} catch (err: any) {
return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500);
Expand Down
59 changes: 44 additions & 15 deletions scripts/check-wildcard-fallthrough.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,21 @@ const MOUNTS = {
// adapter's and plugin's own `*` middlewares). Terminal here would take down
// the entire surface, not one namespace.
'packages/cli/src/commands/serve.ts:use *': { yields: true },
// Fixed by #4117, found BY this scan (manual greps had missed it). A
// pre-processing shim that hands off to the env's auth service, not an owner:
// a path that service does not claim now continues to the `${prefix}/*`
// dispatcher below. Narrower than #4092 — it does not make a later Hono route
// reachable (the dispatcher catch-all is deliberately terminal and would
// swallow it anyway); it means an unowned path gets a real gated `dispatch()`
// attempt. Keyed on the dispatcher's own `handled` flag where one exists.
//
// Its `${prefix}/storage/*` twin was ratcheted here alongside it and is now
// gone entirely: #4087/#4112 deleted that bridge, having reached the same
// 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 },

'packages/plugins/plugin-hono-server/src/adapter.ts:use *': { yields: true },
'packages/plugins/plugin-hono-server/src/hono-plugin.ts:use *': { yields: true },

Expand Down Expand Up @@ -147,17 +162,9 @@ const MOUNTS = {

// ── Ratchet: real, tracked, NOT blessed ─────────────────────────────────
//
// Both are the #4088 shape in an adapter that no in-repo package depends on,
// so nothing is broken today — but it ships, and ADR-0076's own trigger is an
// out-of-tree embedder (`../objectbase`'s gateway). An embedder mounting a
// route under either prefix hits exactly #4088. Found BY this scan; manual
// greps for the pattern had missed both. Tracked by #4117.
'packages/adapters/hono/src/index.ts:all `${prefix}/auth/*`': {
ratchet: '#4117 — terminal, same shape as #4088; adapter has no in-repo consumer',
},
'packages/adapters/hono/src/index.ts:all `${prefix}/storage/*`': {
ratchet: '#4117 — terminal, same shape as #4088; adapter has no in-repo consumer',
},
// Empty as of #4117. The mechanism stays — it is how the next terminal wildcard
// gets recorded honestly instead of being either fixed on the spot or quietly
// skipped. Declare the current state plus a `ratchet` naming the issue.
};

/** HTTP-verb registrars plus `use`; anything that can claim a path pattern. */
Expand Down Expand Up @@ -229,9 +236,23 @@ function callsContinuation(fn, src) {
// a call inside it is not this handler yielding.
if (rebinds(node)) return;
// `next()` — or `return next()` / `await next()`, same shape.
if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === name) {
found = true;
return;
if (ts.isCallExpression(node)) {
// `next()` — or `return next()` / `await next()`, same shape.
if (ts.isIdentifier(node.expression) && node.expression.text === name) {
found = true;
return;
}
// …or HANDED to a helper that awaits it: `yieldUnowned(c, next, …)`.
// Counting this is deliberate (#4117). `adapters/hono` factors the yield
// into one helper precisely because the compose subtlety it encodes should
// be written once, and a checker that only recognised a direct call would
// push code toward duplicating it. The trade-off is a handler that passes
// the continuation somewhere that never awaits it — narrower than the false
// NEGATIVE it replaces, and the ledger still requires a human to classify.
if (node.arguments.some((a) => ts.isIdentifier(a) && a.text === name)) {
found = true;
return;
}
}
ts.forEachChild(node, visit);
};
Expand Down Expand Up @@ -463,6 +484,14 @@ function selfTest() {
!yieldsIn('app.all("/a/*", async (c, next) => { items.forEach((next) => next()); return r; });'),
'an INNER binding shadowing the name must not count as yielding',
);
assert(
yieldsIn('app.all("/a/*", async (c, next) => yieldUnowned(c, next, fb));'),
'handing the continuation to a helper that awaits it DOES count (#4117)',
);
assert(
!yieldsIn('app.all("/a/*", async (c, next) => { log("no next here"); return r; });'),
'a call that neither invokes nor receives the continuation must not count',
);

// `resolveHandler` — a handler passed by name must still be analysed, or the
// scan reports a yielding mount as terminal (cloud#923 mounts it that way).
Expand All @@ -486,7 +515,7 @@ function selfTest() {
'method is part of the key',
);

console.log('✓ self-test: 15 cases');
console.log('✓ self-test: 17 cases');
}

if (process.argv.includes('--self-test')) selfTest();
Expand Down
Loading