Skip to content

Commit a656ea1

Browse files
committed
fix(runtime): canonicalise the API root to the discovery route (#17625)
`dispatch()` strips one trailing slash, so both root spellings the dispatcher accepts — `${prefix}/` (arriving as `/`) and `${prefix}` (arriving as ``, the MSW/base-URL-stripped form) — collapsed onto the empty string. Only the discovery branch at the foot of the method knew that meant the API root; the ADR-0069 gate, which runs far above it, did not. That disagreement was invisible while `isAuthGateAllowlisted` exempted a falsy path, and became a 403 on the bare-root discovery request once the predicate went fail-closed. Normalising the root to `/` would relocate the 403 rather than remove it: a segment-less path matches no `ALLOW_ROUTES` entry, and the discovery branch tests `/discovery` or the empty string, neither of which `/` satisfies. The root is canonicalised to `/discovery` instead — the route it has always served — read from one constant by both sites so the two cannot drift again. `packages/core` is untouched and `ALLOW_ROUTES` is unchanged: the only input whose gate answer moves is the API root, which gains exactly the exemption `/discovery` already carried, and gains it by being that route. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
1 parent 49cd715 commit a656ea1

2 files changed

Lines changed: 273 additions & 3 deletions

File tree

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#17625, the runtime half of #7898's ruling A] The API root reaches the
5+
* discovery payload for a GATED session.
6+
*
7+
* ## What moved, and why this file exists at all
8+
*
9+
* `dispatch()` strips one trailing slash, so BOTH root spellings the
10+
* dispatcher accepts — `${prefix}/` (arriving as `'/'`) and `${prefix}`
11+
* (arriving as `''`, the MSW/base-URL-stripped form) — used to travel on as
12+
* `''`. Only the discovery branch at the foot of the method knew that meant
13+
* the API root; the ADR-0069 gate, which runs far above it, did not. While
14+
* `isAuthGateAllowlisted` answered `true` for a falsy path that disagreement
15+
* was invisible. #7898 made the predicate fail-closed, and the bare-root
16+
* discovery request started answering 403.
17+
*
18+
* ⚠️ The obvious repair is measured WRONG upstream and must not be re-tried
19+
* here: normalising `'' → '/'` relocates the 403 instead of removing it.
20+
* `isAuthGateAllowlisted('/')` is `false` (a segment-less path matches no
21+
* `ALLOW_ROUTES` entry) and the discovery branch tests `'/discovery'` or `''`,
22+
* which `'/'` satisfies neither. Both legs are pinned in
23+
* `packages/core/src/security/auth-gate.test.ts` → "does not exempt the
24+
* dispatcher bare-root `cleanPath` — step 2 is #17625".
25+
*
26+
* The delivered repair canonicalises the root to `'/discovery'` — the route it
27+
* has always served — so the gate and the branch read one spelling. ⛔ Core is
28+
* untouched and `ALLOW_ROUTES` is unchanged: the root gains exactly the
29+
* exemption `/discovery` already carried, and gains it by BEING that route.
30+
*
31+
* ## The genuinely-absent-path leg is NOT restated here
32+
*
33+
* "A caller that reaches the gate with no path at all is still refused" is
34+
* `packages/core`'s pin, delivered by #7898's own round
35+
* (`auth-gate.test.ts` → "[#7898] a falsy path is not exempt (fail-closed)",
36+
* which drives `isAuthGateAllowlisted` over `undefined`, `null` and `''`).
37+
* ⛔ Restating it against `HttpDispatcher` would measure nothing new: this
38+
* transport has no pathless call shape — `dispatch()` takes `path: string` and
39+
* the root canonicalisation below is reached only from the two ROOT spellings.
40+
* Referenced, not duplicated.
41+
*
42+
* ## Why every case runs on a fixture whose gate is provably ON
43+
*
44+
* `enforceAuthGate` fails open in a great many ways — no `auth` service, no
45+
* `isAuthGateActive`, no `getSession`, an unreadable header bag, any thrown
46+
* error — and under every one of them a 200 on the root is indistinguishable
47+
* from the repair working. So the gated fixture below is paired with a
48+
* POSITIVE CONTROL on a protected path in the same `describe`: if the control
49+
* stops answering 403 with the gate's own code, every other case in this file
50+
* is measuring a gate that is simply off, and the file says so by going red.
51+
*/
52+
53+
import { describe, it, expect, vi } from 'vitest';
54+
import { HttpDispatcher } from './http-dispatcher.js';
55+
56+
/** A session user carrying an ADR-0069 gate posture (`normalizeAuthGate`'s shape). */
57+
const GATED_USER = {
58+
id: 'u_gated',
59+
authGate: { code: 'PASSWORD_EXPIRED', message: 'Your password has expired.' },
60+
};
61+
62+
/** The same user with no gate — the negative-direction control. */
63+
const UNGATED_USER = { id: 'u_clear' };
64+
65+
/** A path nothing allow-lists, used as the gate's positive control. */
66+
const PROTECTED_PATH = '/data/task';
67+
68+
function makeDispatcher(sessionUser: unknown, gateActive = true) {
69+
const services: Record<string, any> = {
70+
objectql: {
71+
find: vi.fn().mockResolvedValue([]),
72+
getObjects: vi.fn().mockReturnValue({}),
73+
registry: {
74+
getObject: vi.fn().mockReturnValue(null),
75+
getRegisteredTypes: vi.fn().mockReturnValue([]),
76+
},
77+
},
78+
auth: {
79+
isAuthGateActive: () => gateActive,
80+
getApi: async () => ({ getSession: async () => ({ user: sessionUser }) }),
81+
},
82+
};
83+
const kernel: any = {
84+
getState: () => 'running',
85+
getService: (n: string) => services[n] ?? null,
86+
getServiceAsync: async (n: string) => services[n] ?? null,
87+
context: { getService: (n: string) => services[n] ?? null },
88+
};
89+
return new HttpDispatcher(kernel, undefined, { enforceProjectMembership: false });
90+
}
91+
92+
/**
93+
* Drive one request and hand back the result plus the context the dispatcher
94+
* wrote through. `routePath` is the value `prepareResolverHints` recorded, and
95+
* therefore the spelling every stage below it — the gate included — was handed.
96+
*/
97+
async function dispatch(sessionUser: unknown, method: string, path: string, gateActive = true) {
98+
const dispatcher = makeDispatcher(sessionUser, gateActive);
99+
const context: any = { request: new Request(`http://localhost/api/v1${path}`) };
100+
const result = await dispatcher.dispatch(method, path, undefined, {}, context, '/api/v1');
101+
return { result, context };
102+
}
103+
104+
/** The gate's 403 carries its `code` in the envelope's `details` (`error(msg, 403, { code })`). */
105+
const gateCodeOf = (result: any) =>
106+
result.response?.body?.error?.details?.code ?? result.response?.body?.error?.code;
107+
108+
describe('[#17625] the API root resolves to the discovery route for a gated session', () => {
109+
it('⭐ POSITIVE CONTROL — this fixture really does gate: a protected path answers 403 with the gate code', async () => {
110+
// ⛔ Do not delete or weaken this. `enforceAuthGate` fails open on any
111+
// hiccup, so without a request that the SAME fixture refuses, every
112+
// 200 below is compatible with "the gate never ran".
113+
const { result } = await dispatch(GATED_USER, 'GET', PROTECTED_PATH);
114+
expect(result.handled).toBe(true);
115+
expect(result.response?.status).toBe(403);
116+
expect(gateCodeOf(result)).toBe('PASSWORD_EXPIRED');
117+
});
118+
119+
it('PIN 1 — `GET ${prefix}/` returns the discovery payload (was 403 after #7898)', async () => {
120+
const { result } = await dispatch(GATED_USER, 'GET', '/');
121+
expect(result.handled).toBe(true);
122+
expect(result.response?.status).toBe(200);
123+
// The discovery document itself, not merely "not a 403".
124+
expect(result.response?.body?.data?.name).toBe('ObjectOS');
125+
expect(result.response?.body?.data?.routes).toBeDefined();
126+
});
127+
128+
it('PIN 2 — `GET ${prefix}` (no trailing slash) is unchanged: still the discovery payload', async () => {
129+
const { result } = await dispatch(GATED_USER, 'GET', '');
130+
expect(result.handled).toBe(true);
131+
expect(result.response?.status).toBe(200);
132+
expect(result.response?.body?.data?.name).toBe('ObjectOS');
133+
expect(result.response?.body?.data?.routes).toBeDefined();
134+
});
135+
136+
it('serves the SAME document for both root spellings and for the named route', async () => {
137+
// One route, three spellings — the property the canonicalisation buys.
138+
const [slash, bare, named] = await Promise.all([
139+
dispatch(GATED_USER, 'GET', '/'),
140+
dispatch(GATED_USER, 'GET', ''),
141+
dispatch(GATED_USER, 'GET', '/discovery'),
142+
]);
143+
for (const r of [slash, bare, named]) expect(r.result.response?.status).toBe(200);
144+
expect(slash.result.response?.body?.data).toEqual(named.result.response?.body?.data);
145+
expect(bare.result.response?.body?.data).toEqual(named.result.response?.body?.data);
146+
});
147+
148+
it('⭐ THE MECHANISM — the gate is handed the allow-listed route NAME, never `""` or `"/"`', async () => {
149+
// This is the assertion that makes the repair the RULED one rather than
150+
// a coincidence: both root spellings are canonicalised BEFORE the gate,
151+
// so what the gate evaluates is `/discovery` — a name `ALLOW_ROUTES`
152+
// already carries. ⛔ If this ever reads `'/'`, the fix has regressed to
153+
// the shape upstream measured insufficient, and PIN 1 would only still
154+
// pass because something else started exempting the root.
155+
for (const path of ['/', '']) {
156+
const { context } = await dispatch(GATED_USER, 'GET', path);
157+
expect(context.routePath, path).toBe('/discovery');
158+
}
159+
// …and a path that is NOT the root is not rewritten.
160+
const { context } = await dispatch(GATED_USER, 'GET', PROTECTED_PATH);
161+
expect(context.routePath).toBe(PROTECTED_PATH);
162+
});
163+
164+
it('⭐ NEGATIVE-DIRECTION CONTROL — the repair narrows nothing: an UNGATED session still reads the root', async () => {
165+
for (const path of ['/', '', '/discovery']) {
166+
const { result } = await dispatch(UNGATED_USER, 'GET', path);
167+
expect(result.response?.status, path).toBe(200);
168+
expect(result.response?.body?.data?.name, path).toBe('ObjectOS');
169+
}
170+
});
171+
172+
it('the named `/discovery` route keeps answering for a gated session', async () => {
173+
const { result } = await dispatch(GATED_USER, 'GET', '/discovery');
174+
expect(result.response?.status).toBe(200);
175+
expect(result.response?.body?.data?.name).toBe('ObjectOS');
176+
});
177+
});
178+
179+
describe('[#17625] the boundary — what the canonicalisation deliberately does NOT move', () => {
180+
it('the ENVIRONMENT-SCOPED root keeps its own answer: `${prefix}/environments/<id>` is still gated', async () => {
181+
// ⚠️ A different input class, and deliberately untouched. The gate runs
182+
// BEFORE the scoped-URL strip, so this request is judged on
183+
// `/environments/<id>` — which matched no `ALLOW_ROUTES` entry before
184+
// #7898 either, so its answer did not move in that card and must not
185+
// move in this one. The `''` the strip produces afterwards is why the
186+
// discovery branch keeps its `''` arm.
187+
const { result } = await dispatch(GATED_USER, 'GET', '/environments/env-1');
188+
expect(result.response?.status).toBe(403);
189+
expect(gateCodeOf(result)).toBe('PASSWORD_EXPIRED');
190+
});
191+
192+
it('a non-root path that merely LOOKS empty after the strip is not the root', async () => {
193+
// `//` strips to `'/'`, not to `''`, so it is not canonicalised and is
194+
// not exempt — recorded so a later reader does not widen the rule into
195+
// "any number of trailing slashes is the root".
196+
const { result, context } = await dispatch(GATED_USER, 'GET', '//');
197+
expect(context.routePath).toBe('/');
198+
expect(result.response?.status).toBe(403);
199+
});
200+
});

‎packages/runtime/src/http-dispatcher.ts‎

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,24 @@ function isPathWithinPrefix(path: string, prefix: string): boolean {
246246
return Number.isNaN(next) || next === 47 /* '/' */ || next === 63 /* '?' */;
247247
}
248248

249+
/**
250+
* The protocol-standard discovery route, and the canonical spelling of the API
251+
* root (#17625).
252+
*
253+
* Read by exactly two sites — the root canonicalisation at the top of
254+
* `dispatch()` and the discovery branch that serves it — so "which route does
255+
* the bare root resolve to" is answered once. ⛔ Never re-spell either site as
256+
* a literal: the whole defect #17625 repairs was two places disagreeing about
257+
* what the empty path meant, and a third disagreement is one edit away if the
258+
* value is typed twice.
259+
*
260+
* It is also the string the ADR-0069 gate sees for a root request, which is
261+
* why the value has to be the ALLOW-LISTED route name rather than `'/'`:
262+
* `ALLOW_ROUTES` in `packages/core/src/security/auth-gate.ts` carries
263+
* `['discovery']` and nothing that matches a segment-less path.
264+
*/
265+
const DISCOVERY_ROUTE = '/discovery';
266+
249267
/**
250268
* `services.search`'s in-process remedy string (#7939), kept out of the
251269
* shared `inProcessServiceMessage('search')` path on purpose: that helper's
@@ -2498,6 +2516,48 @@ export class HttpDispatcher {
24982516
async dispatch(method: string, path: string, body: any, query: any, context: HttpProtocolContext, prefix?: string): Promise<HttpDispatcherResult> {
24992517
let cleanPath = path.replace(/\/$/, ''); // Remove trailing slash if present, but strict on clean paths
25002518

2519+
// ── The API root IS the discovery route, under a second spelling ──
2520+
// [#17625, the runtime half of #7898's ruling A] The trailing-slash
2521+
// strip above collapses BOTH root spellings the dispatcher accepts —
2522+
// `${prefix}/` (arriving as `'/'`) and `${prefix}` (arriving as `''`,
2523+
// the MSW/base-URL-stripped form) — onto `''`. That empty string then
2524+
// travelled through every cross-cutting stage below as a path that
2525+
// names no route, and only the discovery branch at the foot of this
2526+
// method knew it meant the API root. The gate does not read that
2527+
// branch, so the two disagreed the moment `isAuthGateAllowlisted`
2528+
// stopped exempting a falsy path (#7898): a gated session's
2529+
// `GET ${prefix}/` answered 403 instead of the discovery payload.
2530+
//
2531+
// WHY NORMALISING TO `'/'` IS NOT THE FIX, measured rather than
2532+
// assumed. `isAuthGateAllowlisted('/')` is `false` — `'/'` has no
2533+
// segments, so no `ALLOW_ROUTES` entry can match it — and the
2534+
// discovery branch tests `'/discovery'` or `''`, which `'/'` satisfies
2535+
// neither. `'' → '/'` therefore RELOCATES the 403 rather than removing
2536+
// it; both legs are pinned upstream in
2537+
// `packages/core/src/security/auth-gate.test.ts` ("does not exempt the
2538+
// dispatcher bare-root `cleanPath` — step 2 is #17625").
2539+
//
2540+
// So the root is canonicalised to the route it has always served
2541+
// instead, and the alias stops being a path that nothing recognises.
2542+
// ⛔ This is NOT a tolerance re-added to the allow-list: `packages/core`
2543+
// is untouched, `ALLOW_ROUTES` is unchanged, and the only input whose
2544+
// gate answer moves is the API root — which gains exactly the exemption
2545+
// `/discovery` already had, and gains it by BEING that route. Every
2546+
// other path, empty-but-not-root callers included, is unaffected: a
2547+
// caller that reaches the gate with no path at all is refused at the
2548+
// predicate, and that seam stays core's (`isAuthGateAllowlisted`
2549+
// fail-closed, `shouldDenyAnonymous` declaring the pathless case
2550+
// itself) — ⛔ not re-derived here.
2551+
//
2552+
// ⚠️ One spelling, read from one constant, deliberately: the branch
2553+
// below matches on `DISCOVERY_ROUTE` too, so the canonical form cannot
2554+
// drift from the route it canonicalises to. The branch keeps its `''`
2555+
// arm regardless — the scoped-URL strip further down re-creates `''`
2556+
// for `${prefix}/environments/<id>`, which is a different input class,
2557+
// is gated on its own scoped spelling BEFORE the strip, and is not
2558+
// touched by this card.
2559+
if (cleanPath === '') cleanPath = DISCOVERY_ROUTE;
2560+
25012561
// ── Liveness carve-out — the ONE route family that runs no preamble ──
25022562
// [#15910, maintainer ruling 2026-09-06 (decision batch #57), option C,
25032563
// verbatim 「同意」] "Carve liveness out of the identity step. `/health`
@@ -2635,9 +2695,19 @@ export class HttpDispatcher {
26352695
}
26362696

26372697
// 0. Discovery Endpoint (GET /discovery or GET /)
2638-
// Standard route: /discovery (protocol-compliant)
2639-
// Legacy route: / (empty path, for backward compatibility — MSW strips base URL)
2640-
if ((cleanPath === '/discovery' || cleanPath === '') && method === 'GET') {
2698+
// Standard route: /discovery (protocol-compliant) — and, since #17625,
2699+
// the spelling the API root arrives here as: `${prefix}` / `${prefix}/`
2700+
// are canonicalised to `DISCOVERY_ROUTE` at the top of `dispatch()`, so
2701+
// the root reaches this branch under the same name the ADR-0069 gate
2702+
// allow-lists instead of as an empty path only this branch understood.
2703+
//
2704+
// The `''` arm is still LIVE and ⛔ must not be deleted as dead: the
2705+
// scoped-URL strip above re-creates `''` from
2706+
// `${prefix}/environments/<id>`, whose gate decision was already taken
2707+
// on its own scoped spelling before the strip ran. That is a different
2708+
// input class from the unscoped root and #17625 deliberately left it
2709+
// exactly as it was.
2710+
if ((cleanPath === DISCOVERY_ROUTE || cleanPath === '') && method === 'GET') {
26412711
const info = await this.getDiscoveryInfo(prefix ?? '', context);
26422712
return {
26432713
handled: true,

0 commit comments

Comments
 (0)