Skip to content

Commit 554ff92

Browse files
os-zhuangclaude
andauthored
fix(adapters/hono): the auth wildcard yields paths the auth service does not own, and unbreak the #4116 ledger (#4117) (#4133)
Two things, and the second is urgent. ## The fix (#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 #4088's shape. #4116's scan found it; the manual greps that preceded that scan had not. 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, exactly as in #4092. What this buys here is narrower than #4092 and the code says so: it does NOT make a later-registered Hono route reachable, because the `${prefix}/*` dispatcher catch-all below is deliberately terminal (ADR-0076 OQ#9, #3576/#3608) and would swallow one either way. 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 this adapter's actual extension mechanism. The first draft of the tests asserted the #4092 outcome and failed, which is how the distinction got established rather than assumed. ## Unbreaking main #4122 shipped the ledger declaring TWO ratcheted mounts here. Between its base (974c6d4) and its merge, #4087/#4112 landed and DELETED the `/storage/*` bridge — so the squash produced a main where the ledger declares a mount that no longer exists, and `check:wildcard-fallthrough` fails on main with "DECLARED but not found". That is my merge race, and this commit removes the entry. Worth recording why the guard behaved well here: the stale-entry arm is what caught it, immediately, on the first run against the new main. And #4112 reached the same verdict about that wildcard independently, from the other direction — "the wildcard was wider than the two routes it served". Two independent reads landing on one defect is the argument for enumerating the shape rather than finding it by eye each time. Also extends `callsContinuation`: handing the continuation to a helper (`yieldUnowned(c, next, …)`) now counts as yielding. Without that the checker called this fix terminal — a false negative that would have pushed the compose subtlety toward being duplicated at each call site instead of written once. Guard: 7 yielding / 0 ratcheted / 5 exempt over 12 mounts. Self-test 17 cases. adapters/hono 73 passed. Verified both defences bite: reverting the fix fails the guard with the TERMINAL diagnostic and fails 2 of the 6 new tests. The 3 `tsc` errors and 2 `eslint` rule-definition errors in this package are pre-existing, measured identical on a clean tree. Claude-Session: https://claude.ai/code/session_013VZLsKypjrGhiLpio2uWKu Co-authored-by: Claude <noreply@anthropic.com>
1 parent 857a6cf commit 554ff92

4 files changed

Lines changed: 258 additions & 17 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
"@objectstack/adapter-hono": patch
3+
---
4+
5+
fix(adapters/hono): the auth wildcard yields paths the auth service does not own (#4117)
6+
7+
`app.all('${prefix}/auth/*')` claimed a whole namespace and was **terminal**: it
8+
returned the auth service's response unconditionally, including better-auth's 404
9+
for a path it does not implement, and the legacy `handleAuth` bridge's own
10+
`handled: false` 404. That is the #4088 shape, found by #4116's enumeration after
11+
manual greps had missed it.
12+
13+
A 404 from better-auth, or `handled: false` from the dispatcher, now means "not
14+
this mount's path" and the handler yields. The predicate is the dispatcher's own
15+
`handled` flag wherever one exists — an explicit ownership answer beats inferring
16+
one from a status; only the better-auth hand-off lacks such a flag, and there the
17+
404 is the signal, as in #4092.
18+
19+
**What changes on the wire.** An unowned path under `${prefix}/auth/*` used to get
20+
a 404 built by this mount. It now continues to the `${prefix}/*` dispatcher
21+
catch-all and gets a real, gate-carrying `dispatch()` attempt, so a domain handler
22+
registered for such a path becomes reachable — this adapter's actual extension
23+
mechanism. When nothing anywhere claims the path the reply is still the same
24+
enveloped `{ success: false, error: { message: 'Not Found', code: 404 } }`. Paths
25+
the auth service does own are untouched, and a 401/403 from it is never treated as
26+
a disclaimer of ownership.
27+
28+
No configuration changes and no new routes.
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #4117 — the `/auth/*` mount must yield paths it does not own.
5+
*
6+
* It claims a whole namespace and used to be TERMINAL: it answered 404 for a path
7+
* its auth service does not implement. Found by #4116's scan, which no manual grep
8+
* had managed. Its `/storage/*` twin was ratcheted alongside it and is now gone
9+
* entirely — #4087/#4112 deleted that bridge after reaching the same conclusion
10+
* from the other direction ("the wildcard was wider than the two routes it
11+
* served").
12+
*
13+
* ## What yielding buys HERE, precisely
14+
*
15+
* Not what it bought in #4092. There, yielding made another plugin's route
16+
* reachable. In this adapter it cannot: the `${prefix}/*` dispatcher catch-all
17+
* is registered right after these two and is DELIBERATELY terminal (ADR-0076
18+
* OQ#9, #3576/#3608 — the gate stages live inside `dispatch()`, so splitting it
19+
* into per-prefix Hono mounts would bypass them). So a route registered later
20+
* under `/api/auth/*` is swallowed by the catch-all whether or not these two
21+
* yield, and Hono-route mounting is not this adapter's extension path at all —
22+
* registering a domain handler is.
23+
*
24+
* What yielding buys is that an unowned `/auth/*` or `/storage/*` path now
25+
* reaches that gated `dispatch()` instead of dead-ending in a 404 built two
26+
* mounts earlier. A domain handler registered for such a path becomes
27+
* reachable, which is exactly the adapter's intended mechanism. These tests pin
28+
* that, and pin that the owners still win everything they own.
29+
*/
30+
31+
import { describe, it, expect, vi, beforeEach } from 'vitest';
32+
import type { Hono } from 'hono';
33+
34+
const mockDispatcher = {
35+
getDiscoveryInfo: vi.fn().mockReturnValue({ version: '1.0', endpoints: [] }),
36+
handleAuth: vi.fn(),
37+
handleGraphQL: vi.fn(),
38+
dispatch: vi.fn(),
39+
};
40+
41+
vi.mock('@objectstack/runtime', () => ({
42+
HttpDispatcher: function HttpDispatcher() { return mockDispatcher; },
43+
}));
44+
45+
import { createHonoApp } from './index';
46+
47+
/** No domain claimed the path — the dispatcher's own ownership signal. */
48+
const UNHANDLED = { handled: false };
49+
const HANDLED = (body: unknown, status = 200) => ({ handled: true, response: { body, status } });
50+
51+
/** A kernel whose `auth` service is (or is not) present. */
52+
const kernelWith = (authService?: unknown) => ({
53+
name: 'test-kernel',
54+
getService: (n: string) => (n === 'auth' && authService ? authService : undefined),
55+
}) as any;
56+
57+
const authServiceReturning = (status: number, body: unknown = { from: 'better-auth' }) => ({
58+
handleRequest: vi.fn(async () => new Response(JSON.stringify(body), {
59+
status,
60+
headers: { 'Content-Type': 'application/json' },
61+
})),
62+
});
63+
64+
describe('auth mount yields paths better-auth does not own (#4117)', () => {
65+
beforeEach(() => {
66+
vi.clearAllMocks();
67+
mockDispatcher.dispatch.mockResolvedValue(UNHANDLED);
68+
mockDispatcher.handleAuth.mockResolvedValue(UNHANDLED);
69+
});
70+
71+
it('reaches the gated dispatch when better-auth 404s', async () => {
72+
// `/auth/me/permissions` is the canonical unowned path: nothing in
73+
// better-auth serves it. Before #4117 this 404'd here; now the request gets
74+
// a real dispatch attempt, so a domain handler for it is reachable.
75+
mockDispatcher.dispatch.mockResolvedValue(HANDLED({ authenticated: true, from: 'dispatch' }));
76+
const app: Hono = createHonoApp({ kernel: kernelWith(authServiceReturning(404)) });
77+
78+
const res = await app.request('/api/auth/me/permissions');
79+
80+
expect(mockDispatcher.dispatch).toHaveBeenCalled();
81+
expect(res.status).toBe(200);
82+
expect(await res.json()).toEqual({ authenticated: true, from: 'dispatch' });
83+
});
84+
85+
it('keeps better-auth winning the paths it DOES own', async () => {
86+
const app: Hono = createHonoApp({ kernel: kernelWith(authServiceReturning(200, { user: null })) });
87+
88+
const res = await app.request('/api/auth/get-session');
89+
90+
expect(res.status).toBe(200);
91+
expect(await res.json()).toEqual({ user: null });
92+
expect(mockDispatcher.dispatch).not.toHaveBeenCalled();
93+
});
94+
95+
it('does NOT yield on 401 — a real answer, not a disclaimer of ownership', async () => {
96+
const app: Hono = createHonoApp({ kernel: kernelWith(authServiceReturning(401, { error: 'nope' })) });
97+
98+
const res = await app.request('/api/auth/protected');
99+
100+
expect(res.status).toBe(401);
101+
expect(await res.json()).toEqual({ error: 'nope' });
102+
expect(mockDispatcher.dispatch).not.toHaveBeenCalled();
103+
});
104+
105+
it('yields on the dispatcher path too, keyed on `handled: false`', async () => {
106+
// No auth service at all, so the mount falls back to `dispatcher.handleAuth`
107+
// — whose explicit `handled` flag beats inferring ownership from a status.
108+
mockDispatcher.dispatch.mockResolvedValue(HANDLED({ currency: 'USD' }));
109+
const app: Hono = createHonoApp({ kernel: kernelWith() });
110+
111+
const res = await app.request('/api/auth/me/localization');
112+
113+
expect(mockDispatcher.handleAuth).toHaveBeenCalled();
114+
expect(mockDispatcher.dispatch).toHaveBeenCalled();
115+
expect(res.status).toBe(200);
116+
expect(await res.json()).toEqual({ currency: 'USD' });
117+
});
118+
119+
it('still lets handleAuth answer what it DOES handle', async () => {
120+
mockDispatcher.handleAuth.mockResolvedValue(HANDLED({ ok: true }));
121+
const app: Hono = createHonoApp({ kernel: kernelWith() });
122+
123+
const res = await app.request('/api/auth/login', { method: 'POST' });
124+
125+
expect(res.status).toBe(200);
126+
expect(await res.json()).toEqual({ ok: true });
127+
expect(mockDispatcher.dispatch).not.toHaveBeenCalled();
128+
});
129+
});
130+
131+
describe('the enveloped 404 survives when nothing anywhere claims the path', () => {
132+
beforeEach(() => vi.clearAllMocks());
133+
134+
it('still ends in the enveloped 404 when nothing anywhere claims it', async () => {
135+
mockDispatcher.handleAuth.mockResolvedValue(UNHANDLED);
136+
mockDispatcher.dispatch.mockResolvedValue(UNHANDLED);
137+
const app: Hono = createHonoApp({ kernel: kernelWith() });
138+
139+
const res = await app.request('/api/auth/nope');
140+
141+
expect(res.status).toBe(404);
142+
// Not Hono's bare "404 Not Found" text — the platform envelope is preserved.
143+
expect(await res.json()).toEqual({ success: false, error: { message: 'Not Found', code: 404 } });
144+
});
145+
});

packages/adapters/hono/src/index.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,39 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
275275
return c.redirect(prefix);
276276
});
277277

278+
/**
279+
* Hand a path THIS mount does not own to whatever else matched (#4117).
280+
*
281+
* The `${prefix}/auth/*` mount below claims a whole namespace and used to be
282+
* TERMINAL — it answered 404 for a path its auth service does not implement.
283+
* That is #4088's shape, which cost four fixes before #4116's scan started
284+
* enumerating it, and it is what #4087/#4112 had already concluded about the
285+
* `/storage` bridge it deleted: "the wildcard was wider than the two routes it
286+
* served".
287+
*
288+
* What yielding buys HERE is narrower than in #4092, and worth stating so
289+
* nobody over-reads it. It does NOT make a later-registered Hono route
290+
* reachable: the `${prefix}/*` dispatcher catch-all below is DELIBERATELY
291+
* terminal (ADR-0076 OQ#9, #3576/#3608 — the gate stages live inside
292+
* `dispatch()`), so it would swallow such a route either way, and mounting Hono
293+
* routes is not this adapter's extension path anyway. It means an unowned
294+
* `/auth/*` path now reaches that gated `dispatch()` instead of dead-ending in
295+
* a 404 built one mount earlier, so a domain handler registered for it becomes
296+
* reachable — which IS the adapter's mechanism.
297+
*
298+
* `c.res = …` rather than `return …`: `app.use('*', cors(…))` above puts a
299+
* middleware in this chain, and Hono's compose only assigns a handler's
300+
* RETURNED Response while `c.finalized` is false. Reaching the end of the chain
301+
* runs notFound, which sets a response and flips that flag, so a `return` here
302+
* is silently dropped (learned the hard way in #4092).
303+
*/
304+
const yieldUnowned = async (c: any, next: any, fallback: () => Response) => {
305+
await next();
306+
if (!c.res) c.res = fallback();
307+
};
308+
278309
// --- Auth (needs auth service integration) ---
279-
app.all(`${prefix}/auth/*`, async (c) => {
310+
app.all(`${prefix}/auth/*`, async (c, next) => {
280311
try {
281312
const path = c.req.path.substring(`${prefix}/auth/`.length);
282313
const method = c.req.method;
@@ -330,17 +361,25 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
330361

331362
if (authService && typeof authService.handleRequest === 'function') {
332363
const response = await authService.handleRequest(c.req.raw);
333-
return new Response(response.body, {
364+
const forwarded = () => new Response(response.body, {
334365
status: response.status,
335366
headers: response.headers,
336367
});
368+
// 404 from better-auth means "not one of my endpoints" — the #4092
369+
// signal. `/auth/me/permissions` is the canonical example: nothing in
370+
// better-auth serves it, `plugin-hono-server` does.
371+
if (response.status === 404) return yieldUnowned(c, next, forwarded);
372+
return forwarded();
337373
}
338374

339375
// Fallback to legacy dispatcher
340376
const body = method === 'GET' || method === 'HEAD'
341377
? {}
342378
: await c.req.json().catch(() => ({}));
343379
const result = await dispatcher.handleAuth(path, method, body, { request: c.req.raw });
380+
// `handled: false` is the dispatcher saying no auth domain claimed it —
381+
// an explicit ownership signal, better than inferring one from a status.
382+
if (!result.handled) return yieldUnowned(c, next, () => toResponse(c, result));
344383
return toResponse(c, result);
345384
} catch (err: any) {
346385
return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500);

scripts/check-wildcard-fallthrough.mjs

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,21 @@ const MOUNTS = {
107107
// adapter's and plugin's own `*` middlewares). Terminal here would take down
108108
// the entire surface, not one namespace.
109109
'packages/cli/src/commands/serve.ts:use *': { yields: true },
110+
// Fixed by #4117, found BY this scan (manual greps had missed it). A
111+
// pre-processing shim that hands off to the env's auth service, not an owner:
112+
// a path that service does not claim now continues to the `${prefix}/*`
113+
// dispatcher below. Narrower than #4092 — it does not make a later Hono route
114+
// reachable (the dispatcher catch-all is deliberately terminal and would
115+
// swallow it anyway); it means an unowned path gets a real gated `dispatch()`
116+
// attempt. Keyed on the dispatcher's own `handled` flag where one exists.
117+
//
118+
// Its `${prefix}/storage/*` twin was ratcheted here alongside it and is now
119+
// gone entirely: #4087/#4112 deleted that bridge, having reached the same
120+
// conclusion from the other direction — "the wildcard was wider than the two
121+
// routes it served". Two independent reads landing on the same defect is the
122+
// argument for enumerating the shape rather than finding it by eye each time.
123+
"packages/adapters/hono/src/index.ts:all `${prefix}/auth/*`": { yields: true },
124+
110125
'packages/plugins/plugin-hono-server/src/adapter.ts:use *': { yields: true },
111126
'packages/plugins/plugin-hono-server/src/hono-plugin.ts:use *': { yields: true },
112127

@@ -147,17 +162,9 @@ const MOUNTS = {
147162

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

163170
/** HTTP-verb registrars plus `use`; anything that can claim a path pattern. */
@@ -229,9 +236,23 @@ function callsContinuation(fn, src) {
229236
// a call inside it is not this handler yielding.
230237
if (rebinds(node)) return;
231238
// `next()` — or `return next()` / `await next()`, same shape.
232-
if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === name) {
233-
found = true;
234-
return;
239+
if (ts.isCallExpression(node)) {
240+
// `next()` — or `return next()` / `await next()`, same shape.
241+
if (ts.isIdentifier(node.expression) && node.expression.text === name) {
242+
found = true;
243+
return;
244+
}
245+
// …or HANDED to a helper that awaits it: `yieldUnowned(c, next, …)`.
246+
// Counting this is deliberate (#4117). `adapters/hono` factors the yield
247+
// into one helper precisely because the compose subtlety it encodes should
248+
// be written once, and a checker that only recognised a direct call would
249+
// push code toward duplicating it. The trade-off is a handler that passes
250+
// the continuation somewhere that never awaits it — narrower than the false
251+
// NEGATIVE it replaces, and the ledger still requires a human to classify.
252+
if (node.arguments.some((a) => ts.isIdentifier(a) && a.text === name)) {
253+
found = true;
254+
return;
255+
}
235256
}
236257
ts.forEachChild(node, visit);
237258
};
@@ -463,6 +484,14 @@ function selfTest() {
463484
!yieldsIn('app.all("/a/*", async (c, next) => { items.forEach((next) => next()); return r; });'),
464485
'an INNER binding shadowing the name must not count as yielding',
465486
);
487+
assert(
488+
yieldsIn('app.all("/a/*", async (c, next) => yieldUnowned(c, next, fb));'),
489+
'handing the continuation to a helper that awaits it DOES count (#4117)',
490+
);
491+
assert(
492+
!yieldsIn('app.all("/a/*", async (c, next) => { log("no next here"); return r; });'),
493+
'a call that neither invokes nor receives the continuation must not count',
494+
);
466495

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

489-
console.log('✓ self-test: 15 cases');
518+
console.log('✓ self-test: 17 cases');
490519
}
491520

492521
if (process.argv.includes('--self-test')) selfTest();

0 commit comments

Comments
 (0)