Skip to content

Commit c1eafe6

Browse files
os-litantclaude
andauthored
fix(runtime): stop the /auth domain claiming every path that merely starts with auth (#16265)
* fix(runtime): stop the /auth domain claiming every path that merely starts with `auth` `createAuthDomain` registered `{ prefix: '/auth' }` with no `match`, and `DomainRoute.match` defaults to `'prefix'` — `path.startsWith('/auth')`, no segment boundary. `DomainHandlerRegistry` preserved that rough edge on purpose when the domains were lifted out of the legacy if-chain, and on this prefix it claims SIBLING NAMESPACES. Measured on a real boot before the fix (a real `ObjectKernel` with `AuthPlugin`, `createHonoApp({ kernel, prefix: '/api/v1' })`, authenticated as the dev admin): GET /api/v1/authx -> 200 {} claimed GET /api/v1/authx/foo -> 200 {} claimed GET /api/v1/authentication/foo -> 200 {} claimed GET /api/v1/aut/foo -> 404 ROUTE_NOT_FOUND control GET /api/v1/zzz/foo -> 404 ROUTE_NOT_FOUND control The route now declares `match: 'segment'`, the spelling the registry's other boundary-correct domains (`/keys`, `/mcp`, `/mcp/skill`) already use. The fallthrough is NOT removed and must not be: `/auth/me/permissions` and `/auth/me/localization` are not better-auth endpoints, so the adapter's `/auth/*` mount disclaims them and they reach `dispatch()` here (#4088 — objectui's permission layer reads the former). `'segment'` keeps claiming `/auth` exactly and everything under `/auth/`, and the new suite pins those rows as the overshoot control with the same weight as the narrowed ones. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * test(runtime): pin that a domain registered at `/authx` is REACHED, not merely that auth stops answering The nine cases in this pin all read the auth service spy: "not called" is how they conclude the `/auth` domain did not claim a path. The contract review measured what that cannot see — a repair which KEEPS the wide `startsWith('/auth')` claim and moves the refusal INSIDE `handleAuthRequest` passes all nine green, because the service is still never called and the `ROUTE_NOT_FOUND` envelope is still what comes back. Under that shape `/authx` is still SHADOWED: a domain mounted there never runs, which is the harm the card names and the changeset says is gone. The new case observes the REGISTRY instead. `registerDomainHandler` appends to a first-match-wins table, so a probe domain registered at `/authx` AFTER construction sits BEHIND the auth route — exactly where a package mounting that namespace later would sit — and is reachable only if the auth route declines the path. The evidence asserted is the probe's OWN response coming back out of `dispatch()` for `/authx` and `/authx/foo`, not the absence of a call. Test-only: no production file changes, and the existing `patch` changeset on `@objectstack/runtime` is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f0d5b64 commit c1eafe6

3 files changed

Lines changed: 286 additions & 0 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
The `/auth` dispatcher domain no longer claims sibling namespaces such as `/authx` and `/authentication/foo`.
6+
7+
`createAuthDomain` registered `{ prefix: '/auth' }` without a `match`, and `DomainRoute.match` defaults to `'prefix'` — a bare `path.startsWith('/auth')` with no segment boundary. Every path whose first segment merely *began* with the five characters `auth` was therefore claimed by the auth domain and forwarded to the auth service, instead of falling through to the dispatcher's `ROUTE_NOT_FOUND`. Measured on a real boot (a real kernel with `AuthPlugin`, served through `createHonoApp({ kernel, prefix: '/api/v1' })`), `GET /api/v1/authx`, `/api/v1/authx/foo` and `/api/v1/authentication/foo` were all claimed; `/api/v1/aut/foo` and `/api/v1/zzz/foo` were not, which is what located the boundary at the `auth` prefix.
8+
9+
The route now declares `match: 'segment'` — the spelling the registry's other boundary-correct domains (`/keys`, `/mcp`, `/mcp/skill`) already use. It claims `/auth` exactly and everything under `/auth/`, and nothing else.
10+
11+
**What does not change.** `/auth/me/permissions` and `/auth/me/localization` still reach `dispatch()`. Neither is a better-auth endpoint, so the adapter's `/auth/*` mount disclaims them and they arrive at this domain; `'segment'` keeps claiming them, which the accompanying test pins as an overshoot control alongside the three narrowed rows.
12+
13+
**If you mounted a namespace under `/authx`, `/authentication`, or any other first segment starting with `auth`,** it was previously shadowed by the auth domain and answered by the auth service. It is now reachable — register a domain handler for it, or expect `ROUTE_NOT_FOUND`.
Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#16026] The `/auth` domain claims `/auth` and its slash-separated
5+
* sub-paths — and NOT every path that merely STARTS WITH the five characters
6+
* `/auth`.
7+
*
8+
* ## The defect this pins
9+
*
10+
* `createAuthDomain` returned `{ prefix: '/auth', handler }` with no `match`,
11+
* and `DomainRoute.match` defaults to `'prefix'` — a bare
12+
* `path.startsWith('/auth')`. `DomainHandlerRegistry` preserved that rough
13+
* edge on purpose when the domains were lifted out of the legacy if-chain
14+
* ("`match: 'prefix'` on `/i18n` also matches `/i18nxx`, exactly as
15+
* `startsWith` did"), so on this prefix the claim reached SIBLING NAMESPACES.
16+
*
17+
* Measured on a real boot before the fix — a real `ObjectKernel` with
18+
* `AuthPlugin` (a real `AuthManager` over better-auth), `createHonoApp({
19+
* kernel, prefix: '/api/v1' })`, authenticated as the dev admin, requests
20+
* injected through the returned app:
21+
*
22+
* GET /api/v1/auth -> 200 {} claimed
23+
* GET /api/v1/auth/ -> 200 {} claimed
24+
* GET /api/v1/authx -> 200 {} claimed <- defect
25+
* GET /api/v1/authx/foo -> 200 {} claimed <- defect
26+
* GET /api/v1/authentication/foo -> 200 {} claimed <- defect
27+
* GET /api/v1/aut/foo -> 404 ROUTE_NOT_FOUND control
28+
* GET /api/v1/zzz/foo -> 404 ROUTE_NOT_FOUND control
29+
*
30+
* `/authentication/foo` is not an auth path by any reading, and `/authx` is a
31+
* plausible namespace someone mounts later.
32+
*
33+
* ## ⭐ Why the CLAIMED rows are cases here and not background
34+
*
35+
* A pin that only asserted the three defect rows cannot fail in the direction
36+
* that matters most: a repair which stopped claiming `/auth` ALTOGETHER would
37+
* pass every one of them, and would break the surface this card is forbidden
38+
* to touch. `/auth/me/permissions` and `/auth/me/localization` are not
39+
* better-auth endpoints, so the adapter's `/auth/*` mount disclaims them and
40+
* they reach `dispatch()` here (#4088 — objectui's permission layer reads the
41+
* former). So the claimed rows are the OVERSHOOT control and carry the same
42+
* weight as the defect rows.
43+
*
44+
* The observation that separates the two classes is the auth service's
45+
* `handleRequest` spy: this domain forwards to it and does nothing else, so
46+
* "called" means the domain claimed the path and "not called" means the path
47+
* fell through the registry to the dispatcher's terminal `ROUTE_NOT_FOUND`.
48+
* `/aut/foo` and `/zzz/foo` are the card's own clean rows, carried so a run
49+
* where EVERYTHING 404s is distinguishable from the fix.
50+
*
51+
* ⚠️ That spy is not sufficient on its own, and the LAST case in this file is
52+
* why: "not called" cannot separate a RELEASED namespace from a still-wide
53+
* claim that refuses inside `handleAuthRequest`. That case observes the
54+
* registry resolving `/authx` to a domain registered there after construction
55+
* — its own docblock carries the argument.
56+
*
57+
* ## ⛔ Not covered here
58+
*
59+
* The `200 {}` those rows carried. It is manufactured one layer out, in the
60+
* `@objectstack/hono` adapter's dispatcher-result rendering — the auth service
61+
* answers an honest 404 for every row above, and this domain hands that
62+
* `Response` back untouched (`HttpDispatcherResult.result`, whose declared
63+
* contract is "direct response objects (Response/NextResponse)"). Nothing in
64+
* this file asserts a wire status for a claimed path, because at this layer
65+
* there is not one.
66+
*/
67+
68+
import { describe, it, expect, vi } from 'vitest';
69+
import { HttpDispatcher } from '../http-dispatcher.js';
70+
71+
/** Paths the `/auth` domain must NOT claim — the defect rows. */
72+
const SIBLING_NAMESPACES = ['/authx', '/authx/foo', '/authentication/foo'];
73+
74+
/** The card's own clean rows: never claimed, before or after. */
75+
const CLEAN_CONTROLS = ['/aut/foo', '/zzz/foo'];
76+
77+
/**
78+
* Paths the `/auth` domain MUST keep claiming. The last two are the #4088
79+
* boundary the card and its triage both underline.
80+
*/
81+
const STILL_CLAIMED = ['/auth', '/auth/me/permissions', '/auth/me/localization'];
82+
83+
/** better-auth's answer for a path its router does not route: bodyless 404. */
84+
const unrouted404 = () => new Response(null, { status: 404 });
85+
86+
function makeFixture() {
87+
const handleRequest = vi.fn(async () => unrouted404());
88+
const services: Record<string, any> = {
89+
objectql: {
90+
find: vi.fn().mockResolvedValue([]),
91+
getObjects: vi.fn().mockReturnValue({}),
92+
registry: {
93+
getObject: vi.fn().mockReturnValue(null),
94+
getRegisteredTypes: vi.fn().mockReturnValue([]),
95+
},
96+
},
97+
// Deliberately WITHOUT `isAuthGateActive`: the ADR-0069 gate is not
98+
// what these cases are about, and an auth service that does not
99+
// implement it is the shape the gate itself skips on.
100+
auth: { handleRequest },
101+
};
102+
const kernel: any = {
103+
getState: () => 'running',
104+
getService: (name: string) => services[name] ?? null,
105+
getServiceAsync: async (name: string) => services[name] ?? null,
106+
context: { getService: (name: string) => services[name] ?? null },
107+
};
108+
const dispatcher = new HttpDispatcher(kernel, undefined, { enforceProjectMembership: false });
109+
return { dispatcher, handleRequest };
110+
}
111+
112+
const dispatchGet = async (path: string) => {
113+
const { dispatcher, handleRequest } = makeFixture();
114+
const result = await dispatcher.dispatch('GET', path, undefined, {}, { request: new Request(`http://localhost${path}`) } as any);
115+
return { result, handleRequest };
116+
};
117+
118+
describe('#16026: the /auth claim stops at a segment boundary', () => {
119+
describe('sibling namespaces are NOT claimed — they fall through to ROUTE_NOT_FOUND', () => {
120+
for (const path of SIBLING_NAMESPACES) {
121+
it(`${path} is refused with the ROUTE_NOT_FOUND envelope and never reaches the auth service`, async () => {
122+
const { result, handleRequest } = await dispatchGet(path);
123+
124+
// The domain did not claim it — the observation that would
125+
// have gone the other way before the fix.
126+
expect(handleRequest).not.toHaveBeenCalled();
127+
128+
// Refusal asserts the ENVELOPE (code + status), never a bare
129+
// "it did not succeed": an unrelated 404 from any other layer
130+
// would otherwise read as this fix working.
131+
expect(result.handled).toBe(true);
132+
expect(result.response?.status).toBe(404);
133+
expect(result.response?.body?.error?.code).toBe('ROUTE_NOT_FOUND');
134+
expect(result.response?.body?.error?.httpStatus).toBe(404);
135+
expect(result.response?.body?.error?.route).toBe(path);
136+
expect(result.response?.body?.success).toBe(false);
137+
});
138+
}
139+
});
140+
141+
describe("the card's clean rows are unchanged — the harness can tell the classes apart", () => {
142+
for (const path of CLEAN_CONTROLS) {
143+
it(`${path} still answers ROUTE_NOT_FOUND`, async () => {
144+
const { result, handleRequest } = await dispatchGet(path);
145+
expect(handleRequest).not.toHaveBeenCalled();
146+
expect(result.response?.status).toBe(404);
147+
expect(result.response?.body?.error?.code).toBe('ROUTE_NOT_FOUND');
148+
});
149+
}
150+
});
151+
152+
describe('⭐ the fallthrough is NOT removed — /auth and its sub-paths still reach dispatch()', () => {
153+
for (const path of STILL_CLAIMED) {
154+
it(`${path} is still claimed and still forwarded to the auth service`, async () => {
155+
const { result, handleRequest } = await dispatchGet(path);
156+
157+
expect(handleRequest).toHaveBeenCalledTimes(1);
158+
expect(result.handled).toBe(true);
159+
// Forwarded whole: the domain hands back the service's own
160+
// Response rather than an envelope of its own.
161+
expect(result.result).toBeInstanceOf(Response);
162+
expect((result.result as Response).status).toBe(404);
163+
// …and specifically NOT the dispatcher's terminal refusal.
164+
expect(result.response?.body?.error?.code).toBeUndefined();
165+
});
166+
}
167+
});
168+
169+
it('a trailing slash is the same claim — dispatch() strips it before the registry sees it', async () => {
170+
const { result, handleRequest } = await dispatchGet('/auth/');
171+
expect(handleRequest).toHaveBeenCalledTimes(1);
172+
expect(result.result).toBeInstanceOf(Response);
173+
expect(result.response?.body?.error?.code).toBeUndefined();
174+
});
175+
176+
/**
177+
* ⭐ The registry-resolution case — what every case above is blind to.
178+
*
179+
* The nine cases above read the auth service spy: "not called" is how they
180+
* conclude "the domain did not claim this path". That observation cannot
181+
* tell the delivered fix apart from a repair which KEEPS the wide
182+
* `startsWith('/auth')` claim and moves the refusal INSIDE
183+
* `handleAuthRequest`. Under that shape the service is still never called
184+
* and the `ROUTE_NOT_FOUND` envelope is still what comes back, so all nine
185+
* stay green — while `/authx` is still SHADOWED and a domain someone mounts
186+
* there never runs. That shadowing is the harm the card names and the
187+
* changeset says is gone, so it needs an observation of its own.
188+
*
189+
* This case observes the REGISTRY instead of the spy.
190+
* `registerDomainHandler` appends to a first-match-wins table
191+
* (`DomainHandlerRegistry.register` → `resolve` walks in registration
192+
* order), so a probe registered AFTER construction sits BEHIND the auth
193+
* route — exactly where a package that mounts `/authx` later would sit. It
194+
* is reachable only if the auth route declines the path, and the evidence
195+
* is the probe's OWN response coming back out of `dispatch()`, not the
196+
* absence of a call.
197+
*
198+
* ⛔ What falsifies it: the registry resolving `/authx` or `/authx/foo` to
199+
* anything other than the probe. In this fixture the auth route is the only
200+
* other claimant, so red here means the claim did not stop at the segment
201+
* boundary — stated about the registry, which is where the shadowing lives.
202+
*/
203+
it('⭐ a domain registered at /authx AFTER construction is REACHED — the claim released the namespace, it did not merely stop answering', async () => {
204+
const { dispatcher, handleRequest } = makeFixture();
205+
const probe = vi.fn(async (req: any) => ({
206+
handled: true as const,
207+
response: { status: 200, body: { success: true, data: { probe: '/authx', path: req.path } } },
208+
}));
209+
dispatcher.registerDomainHandler({ prefix: '/authx', match: 'segment', handler: probe });
210+
211+
for (const path of ['/authx', '/authx/foo']) {
212+
const result = await dispatcher.dispatch('GET', path, undefined, {}, { request: new Request(`http://localhost${path}`) } as any);
213+
214+
// The registry resolved to the PROBE: its own response is what the
215+
// dispatcher handed back, for this exact path.
216+
expect(result.handled).toBe(true);
217+
expect(result.response?.status).toBe(200);
218+
expect(result.response?.body?.data?.probe).toBe('/authx');
219+
expect(result.response?.body?.data?.path).toBe(path);
220+
// …and specifically NOT the terminal refusal, which is what a claim
221+
// that is still wide but refuses in the handler would produce.
222+
expect(result.response?.body?.error?.code).toBeUndefined();
223+
}
224+
225+
expect(probe).toHaveBeenCalledTimes(2);
226+
expect(handleRequest).not.toHaveBeenCalled();
227+
});
228+
});

packages/runtime/src/domains/auth.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,54 @@ import { CoreServiceName } from '@objectstack/spec/system';
1313
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
1414
import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';
1515

16+
/**
17+
* The route this domain claims — `/auth` and its slash-separated sub-paths,
18+
* and NOTHING ELSE (#16026).
19+
*
20+
* ## Why `match: 'segment'` is spelled out rather than left to the default
21+
*
22+
* `DomainRoute.match` defaults to `'prefix'`, i.e. a bare
23+
* `path.startsWith('/auth')` with no segment boundary — the legacy if-chain's
24+
* shape, which `DomainHandlerRegistry` preserved deliberately when the domains
25+
* were lifted out of it. On this prefix that rough edge claims SIBLING
26+
* NAMESPACES: `/authx`, `/authx/foo` and `/authentication/foo` are not auth
27+
* paths by any reading, and `/authx` is a plausible namespace someone mounts
28+
* later. Measured on a real boot — a real `ObjectKernel` with `AuthPlugin`,
29+
* `createHonoApp({ kernel, prefix: '/api/v1' })`, authenticated as the dev
30+
* admin, requests injected through the returned app — all three were claimed
31+
* here, forwarded to better-auth, and answered `200 {}`:
32+
*
33+
* GET /api/v1/authx -> 200 {} (claimed here)
34+
* GET /api/v1/authx/foo -> 200 {} (claimed here)
35+
* GET /api/v1/authentication/foo -> 200 {} (claimed here)
36+
* GET /api/v1/aut/foo -> 404 ROUTE_NOT_FOUND (control)
37+
* GET /api/v1/zzz/foo -> 404 ROUTE_NOT_FOUND (control)
38+
*
39+
* The two control rows are what prove the boundary is the `auth` prefix and
40+
* not the whole catch-all; they are unchanged by this route's `match`.
41+
*
42+
* ⛔ This narrows the CLAIM only — it does not remove the fallthrough, and it
43+
* must not. `/auth/me/permissions` and `/auth/me/localization` are not
44+
* better-auth endpoints, so the adapter's `/auth/*` mount disclaims them and
45+
* they arrive HERE (#4088; objectui's permission layer reads the former).
46+
* `'segment'` claims `'/auth'` exactly and everything under `'/auth/'`, so
47+
* both keep reaching `dispatch()` — that is the whole point of choosing this
48+
* mode over anything narrower.
49+
*
50+
* `'segment'` is also what the registry's other boundary-correct domains
51+
* already declare (`/keys`, `/mcp`, `/mcp/skill`), so this is the codebase's
52+
* own established spelling for the fix, not a new convention.
53+
*
54+
* ⚠️ What this does NOT fix, deliberately: the `200 {}` those rows carried.
55+
* That answer is manufactured one layer OUT, where the adapter renders a
56+
* dispatcher result — the auth service itself answers an honest 404 for every
57+
* path above. Tracked separately on #16026; ⛔ do not "fix" it by narrowing
58+
* this claim further.
59+
*/
1660
export function createAuthDomain(deps: DomainHandlerDeps): DomainRoute {
1761
return {
1862
prefix: '/auth',
63+
match: 'segment',
1964
handler: (req, context) =>
2065
handleAuthRequest(deps, req.path.substring(5), req.method, req.body, context),
2166
};

0 commit comments

Comments
 (0)