Skip to content

Commit da1cffb

Browse files
os-litantclaude
andauthored
fix(runtime): the dispatcher's scope strip matches /environments/, the prefix its own hint parser reads (#15859)
* fix(runtime): the dispatcher's scope strip matches `/environments/`, the prefix its own hint parser reads `HttpDispatcher.dispatch()` reads one scoped-URL convention in three places: `extractEnvironmentIdFromPath` (the environment-id hint), the `acceptOAuthAccessToken` test, and the scope strip that lets `DomainHandlerRegistry` match the remainder. Only the first had been moved to the ADR-0006 `/environments/` spelling. The strip's comment already claimed `/environments/:environmentId`; its regex matched `/projects/`. Driven through the real `@objectstack/hono` catch-all — the entry cloud hosts mount, and the only one that hands `dispatch()` a still-scoped path — the strip never fired, and the registry matches from the head of the path: GET /api/v1/environments/env_alpha/health 404 ROUTE_NOT_FOUND -> 200 GET /api/v1/environments/env_alpha/data/task 404 ROUTE_NOT_FOUND -> reaches /data GET /api/v1/health (control) 200 -> 200 GET /api/v1/data/task (control) reaches /data, unchanged GET /api/v1/no-such-domain (negative) 404 -> 404 The legacy spelling is not kept as an alias. Nothing parses `/projects/<id>`, so stripping it discarded the only place the request named an environment and served it from the host default; ADR-0006 D2 retired `project` on the API surface with no aliases, and `content/docs/api/environment-routing.mdx` tells callers to replace it. It now answers 404, which is the honest response. The OAuth-on-MCP gate moves in the same change because repairing the strip is what makes it reachable: left behind, a scoped `/mcp` caller would reach the domain with its OAuth 2.1 access token refused. The orphaned pre-rename docblock stacked above `extractEnvironmentIdFromPath` is deleted — it named a "project UUID" and the retired URL form, and it is the shape of prose this card exists to remove. Part of #15488 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * docs(runtime): the denial envelope's docblock names the prefix the strip actually removes `routeObjectFromPath` documents what it expects of the dispatcher's cleaned path, and said `/projects/:environmentId` prefix stripped. Same sentence, same strip, same retired spelling as the regex this branch repaired — a comment that teaches the next reader the wrong invariant is the whole subject of this card, so leaving one of them standing one file over would only defer it. Prose only; `routeObjectFromPath` itself matches `/^\/data\/([^/?#]+)/` and is unchanged. Part of #15488 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 4ca358d commit da1cffb

4 files changed

Lines changed: 233 additions & 10 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
An environment-scoped URL now reaches a dispatcher domain instead of answering 404.
6+
7+
`HttpDispatcher.dispatch()` reads the scoped-URL prefix in three places — the environment-id hint parser, the OAuth-on-MCP gate, and the scope strip that lets `DomainHandlerRegistry` match the remainder. Only the first had been moved to the ADR-0006 `/environments/` spelling; the other two still matched the retired `/projects/` one. The strip therefore never fired on a real scoped URL, and since the registry matches from the head of the path, every environment-scoped request arriving through the `@objectstack/hono` catch-all — the entry cloud hosts mount, and the only one that hands `dispatch()` a still-scoped path — matched no domain at all:
8+
9+
```
10+
GET /api/v1/environments/<id>/data/task -> 404 ROUTE_NOT_FOUND (now: reaches /data)
11+
GET /api/v1/environments/<id>/health -> 404 ROUTE_NOT_FOUND (now: 200)
12+
GET /api/v1/data/task (control) -> reaches /data, unchanged
13+
```
14+
15+
The dispatcher-plugin's own scoped mounts were never affected: they pass a pre-stripped subpath (`${prefix}/environments/:environmentId/automation` dispatches the literal `/automation`), which is why the standalone server showed nothing.
16+
17+
The OAuth 2.1 gate moved with it. An access token is honoured only on the MCP surface, and that test runs against the still-scoped path — so `/api/v1/environments/<id>/mcp` would have reached the MCP domain with its token refused had the strip been repaired alone.
18+
19+
**If you still emit the old spelling**: replace `/api/v1/projects/:projectId/...` with `/api/v1/environments/:environmentId/...`, as `content/docs/api/environment-routing.mdx` has instructed since ADR-0006 D2. That prefix is no longer stripped, and it was never a working alias in the first place: nothing parses `/projects/<id>`, so stripping it discarded the only place the request named an environment and served it from the host default instead. ADR-0006 D2 retired `project` on the API surface with no aliases, so the repair is one spelling in all three readings rather than a two-prefix alternation.
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { HttpDispatcher, type HttpDispatcherResult } from './http-dispatcher.js';
5+
6+
/**
7+
* The scoped-URL prefix is ONE convention, read three times in one request —
8+
* and the three readings had drifted apart.
9+
*
10+
* `dispatch()` handles an environment-scoped URL by reading the same prefix in
11+
* three places:
12+
*
13+
* 1. `extractEnvironmentIdFromPath` (via `prepareResolverHints`) parses the
14+
* environment id out of it and hands it to the host's `KernelResolver`;
15+
* 2. the `acceptOAuthAccessToken` test decides whether an OAuth 2.1 access
16+
* token is honoured, and runs against the STILL-SCOPED path;
17+
* 3. the scope strip removes the prefix so `DomainHandlerRegistry` — which
18+
* matches from the head of the path — can claim the remainder.
19+
*
20+
* Reading 1 said `/environments/`; readings 2 and 3 still said the
21+
* pre-ADR-0006 `/projects/`. What that cost, measured through the real
22+
* `@objectstack/hono` catch-all before the repair:
23+
*
24+
* ```
25+
* GET /api/v1/environments/env_alpha/health -> 404 ROUTE_NOT_FOUND
26+
* GET /api/v1/environments/env_alpha/data/task -> 404 ROUTE_NOT_FOUND
27+
* GET /api/v1/health (control) -> 200
28+
* GET /api/v1/data/task (control) -> 503 SERVICE_UNAVAILABLE (i.e. it REACHED /data)
29+
* GET /api/v1/projects/env_alpha/health -> 200 <- stripped, but nothing ever parsed the id
30+
* ```
31+
*
32+
* The legacy spelling was not a working alias in exchange: nothing parses
33+
* `/projects/<id>`, so stripping it discarded the only place the environment
34+
* was named and served the request from the host default. ADR-0006 D2 retired
35+
* `project` on the API surface with NO aliases, and
36+
* `content/docs/api/environment-routing.mdx` tells callers to replace
37+
* `/api/v1/projects/:projectId/...` with `/api/v1/environments/:environmentId/...`
38+
* — so the repair is one spelling everywhere, not a two-prefix alternation.
39+
*
40+
* ## Why this suite drives `dispatch()` with a hand-derived subpath
41+
*
42+
* `packages/runtime` cannot depend on `@objectstack/hono` (that adapter depends
43+
* on THIS package). What the adapter contributes is one line —
44+
* `const subPath = c.req.path.substring(prefix.length)` in the
45+
* `app.all(`${prefix}/*`)` catch-all of `packages/adapters/hono/src/index.ts` —
46+
* so {@link subPathAsTheCatchAllDerivesIt} reproduces exactly that, and nothing
47+
* else. It matters that it is the catch-all: it is the ONLY entry that hands
48+
* `dispatch()` a still-scoped path. Every scoped mount in `dispatcher-plugin.ts`
49+
* passes a pre-stripped subpath (`registerAutomationRoutes` mounts
50+
* `${prefix}/environments/:environmentId/automation` but dispatches the literal
51+
* `'/automation'`), which is why the standalone server never saw this.
52+
*/
53+
54+
const PREFIX = '/api/v1';
55+
const ENV_ID = 'env_alpha';
56+
57+
/** The `@objectstack/hono` catch-all's one contribution, reproduced exactly. */
58+
function subPathAsTheCatchAllDerivesIt(url: string): string {
59+
return url.substring(PREFIX.length);
60+
}
61+
62+
/**
63+
* A kernel that provides NO services at all.
64+
*
65+
* Deliberate: it makes "which domain claimed the path" the only variable. A
66+
* claimed `/data` answers 503 SERVICE_UNAVAILABLE (the domain ran and found no
67+
* data service); an unclaimed path answers 404 ROUTE_NOT_FOUND. Those two are
68+
* the whole signal, and a kernel with services wired would blur them.
69+
*/
70+
function bareKernel(): any {
71+
return {
72+
getState: () => 'running',
73+
getService: () => undefined,
74+
getServiceAsync: async () => undefined,
75+
};
76+
}
77+
78+
function dispatcher(): HttpDispatcher {
79+
return new HttpDispatcher(bareKernel(), undefined, { enforceProjectMembership: false });
80+
}
81+
82+
/**
83+
* Narrow the optional `response`. Lifted from `http-dispatcher.ready.test.ts`
84+
* for the reason stated there: this package's test layer IS type-checked, and
85+
* `expect(res.response).toBeDefined()` narrows nothing. "Answered no response
86+
* at all" and "answered the wrong status" must stay distinguishable.
87+
*/
88+
function responseOf(res: HttpDispatcherResult, what: string): NonNullable<HttpDispatcherResult['response']> {
89+
const { response } = res;
90+
if (!response) throw new Error(`${what} answered no response at all`);
91+
return response;
92+
}
93+
94+
/** `GET <url>` through the catch-all derivation; returns status + error code + the context it mutated. */
95+
async function get(url: string): Promise<{ status: number; code: unknown; ctx: any }> {
96+
const ctx: any = { request: new Request(`http://pin.local${url}`) };
97+
const res = await dispatcher().dispatch('GET', subPathAsTheCatchAllDerivesIt(url), undefined, {}, ctx, PREFIX);
98+
const response = responseOf(res, `GET ${url}`);
99+
return { status: response.status, code: (response.body as any)?.error?.code, ctx };
100+
}
101+
102+
describe('scoped-URL prefix — the environment-scoped URL reaches a dispatcher domain', () => {
103+
it('CONTROL: the unscoped forms reach their domains (a suite that measured nothing would fail here first)', async () => {
104+
expect((await get(`${PREFIX}/health`)).status).toBe(200);
105+
106+
// `/data` has no exact/short-circuit answer, so it proves the DOMAIN ran
107+
// rather than merely that some route matched: the domain body is what
108+
// raises 503 on a missing data service.
109+
await expect(get(`${PREFIX}/data/task`)).rejects.toMatchObject({ statusCode: 503 });
110+
});
111+
112+
it('NEGATIVE CONTROL: an unclaimed path answers 404 ROUTE_NOT_FOUND — the shape "matched no domain" takes', async () => {
113+
const res = await get(`${PREFIX}/no-such-domain`);
114+
expect(res.status).toBe(404);
115+
expect(res.code).toBe('ROUTE_NOT_FOUND');
116+
});
117+
118+
it('the environment-scoped URL is stripped and reaches the same domain as the unscoped one', async () => {
119+
expect((await get(`${PREFIX}/environments/${ENV_ID}/health`)).status).toBe(200);
120+
await expect(get(`${PREFIX}/environments/${ENV_ID}/data/task`)).rejects.toMatchObject({ statusCode: 503 });
121+
});
122+
123+
it('parses the environment id off the SAME prefix it strips — the two readings must not drift again', async () => {
124+
const { ctx } = await get(`${PREFIX}/environments/${ENV_ID}/health`);
125+
expect(ctx.urlEnvironmentId).toBe(ENV_ID);
126+
});
127+
128+
it('the retired `/projects/` spelling resolves nothing — ADR-0006 D2 grants no alias', async () => {
129+
const res = await get(`${PREFIX}/projects/${ENV_ID}/health`);
130+
expect(res.status).toBe(404);
131+
expect(res.code).toBe('ROUTE_NOT_FOUND');
132+
133+
// The point of removing it, stated as an assertion: this spelling was
134+
// never a working alias. Nothing parses it, so the pre-repair strip
135+
// deleted the only mention of the environment and served the request
136+
// from the host default. A 404 is the honest answer to a URL naming an
137+
// environment the dispatcher cannot resolve.
138+
expect(res.ctx.urlEnvironmentId).toBeUndefined();
139+
});
140+
});
141+
142+
describe('scoped-URL prefix — the OAuth-on-MCP gate reads the same prefix', () => {
143+
/**
144+
* Capture the `acceptOAuthAccessToken` decision `resolveRequestScope`
145+
* computes. It is handed to a private method, so the instance-level
146+
* override is the observation seam; throwing afterwards is deliberate —
147+
* `resolveRequestScope` catches it and leaves `executionContext` undefined,
148+
* which is precisely the anonymous-request path and keeps this pin free of
149+
* any identity stack.
150+
*/
151+
async function acceptsOAuthFor(url: string): Promise<boolean | undefined> {
152+
const d = dispatcher();
153+
let seen: boolean | undefined;
154+
(d as any).timedResolveExecutionContext = async (opts: { acceptOAuthAccessToken?: boolean }) => {
155+
seen = opts.acceptOAuthAccessToken;
156+
throw new Error('captured — anonymous from here');
157+
};
158+
const ctx: any = { request: new Request(`http://pin.local${url}`) };
159+
await d.resolveRequestScope(ctx, subPathAsTheCatchAllDerivesIt(url));
160+
return seen;
161+
}
162+
163+
it('honours OAuth access tokens on the plain AND the environment-scoped MCP route', async () => {
164+
expect(await acceptsOAuthFor(`${PREFIX}/mcp`)).toBe(true);
165+
expect(await acceptsOAuthFor(`${PREFIX}/environments/${ENV_ID}/mcp`)).toBe(true);
166+
});
167+
168+
it('CONTROL: refuses them off the MCP surface, and on the retired `/projects/` spelling', async () => {
169+
expect(await acceptsOAuthFor(`${PREFIX}/data/task`)).toBe(false);
170+
expect(await acceptsOAuthFor(`${PREFIX}/projects/${ENV_ID}/mcp`)).toBe(false);
171+
});
172+
});

packages/runtime/src/http-dispatcher.ts

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -560,9 +560,18 @@ export class HttpDispatcher {
560560
// OAuth 2.1 access tokens are honoured ONLY on the MCP
561561
// surface (#2698): their coarse tool-family scopes are
562562
// enforced at MCP tool dispatch, which other routes don't do.
563-
// Matches the plain and `/projects/:id`-scoped route forms
564-
// (the scoped prefix is stripped only by the caller, later).
565-
acceptOAuthAccessToken: /^(?:\/projects\/[^/]+)?\/mcp(?:[/?]|$)/.test(cleanPath),
563+
// Matches the plain and `/environments/:environmentId`-scoped
564+
// route forms (the scoped prefix is stripped only by the caller,
565+
// later, which is why this tests the still-scoped path).
566+
//
567+
// Third reading of the one scoped-URL convention, and it carried
568+
// the same pre-ADR-0006 `/projects/` residue as the strip in
569+
// `dispatch()`. On its own that was unobservable — a scoped
570+
// `/mcp` URL never reached the MCP domain at all — so repairing
571+
// the strip is exactly what makes this line reachable, and the two
572+
// have to move together: left behind, scoped MCP callers would
573+
// reach the domain with their OAuth 2.1 access tokens refused.
574+
acceptOAuthAccessToken: /^(?:\/environments\/[^/]+)?\/mcp(?:[/?]|$)/.test(cleanPath),
566575
});
567576
} catch {
568577
// anonymous request — leave executionContext undefined
@@ -1130,11 +1139,6 @@ export class HttpDispatcher {
11301139
return handleKeysRequest(this.domainDeps, method, body, context);
11311140
}
11321141

1133-
/**
1134-
* Parse a project UUID out of a scoped URL path such as
1135-
* `/api/v1/environments/abc-123/data/task` or `/projects/abc-123/meta`.
1136-
* Returns `undefined` when the path does not match the scoped pattern.
1137-
*/
11381142
/**
11391143
* Parse an environment UUID out of a scoped URL path such as
11401144
* `/api/v1/environments/abc-123/data/task` or `/environments/abc-123/meta`.
@@ -2195,7 +2199,35 @@ export class HttpDispatcher {
21952199
// Strip the `/environments/:environmentId` prefix so the protocol dispatchers
21962200
// below (meta, data, ui, automation, …) see the same shape whether
21972201
// the caller used host-based routing, `X-Environment-Id`, or a scoped URL.
2198-
const scopedMatch = cleanPath.match(/^\/projects\/[^/]+(\/.*)?$/);
2202+
//
2203+
// The prefix spelled here MUST stay equal to the one
2204+
// `extractEnvironmentIdFromPath` parses — they are one convention read
2205+
// twice per request, and they had drifted apart. This line matched the
2206+
// pre-ADR-0006 `/projects/` spelling while the hint parser already read
2207+
// `/environments/`, and the consequence was a live routing defect rather
2208+
// than stale prose:
2209+
//
2210+
// • An environment-scoped URL reached this line UNSTRIPPED, so
2211+
// `DomainHandlerRegistry` (prefix/segment matching from the head of
2212+
// the path) matched no domain at all — measured through the real
2213+
// `@objectstack/hono` catch-all: `GET /api/v1/environments/<id>/data/task`
2214+
// → 404 ROUTE_NOT_FOUND, against the unscoped `GET /api/v1/data/task`
2215+
// → the `/data` domain. That catch-all is the ONLY entry that hands
2216+
// `dispatch()` a still-scoped path; every scoped mount in
2217+
// `dispatcher-plugin.ts` passes a pre-stripped subpath, which is why
2218+
// the defect was invisible to the standalone server.
2219+
// • The legacy spelling was not "still supported" in exchange. Nothing
2220+
// parses `/projects/<id>`, so stripping it discarded the only place
2221+
// the environment was named: `/projects/<id>/data/task` was served
2222+
// with `urlEnvironmentId` undefined — from the host default, not the
2223+
// environment its own URL named.
2224+
//
2225+
// One spelling, deliberately, and not a two-prefix alternation: ADR-0006
2226+
// D2 retired `project` on the API surface with no aliases, and the
2227+
// published migration checklist (`content/docs/api/environment-routing.mdx`)
2228+
// tells callers to replace `/api/v1/projects/:projectId/...` with
2229+
// `/api/v1/environments/:environmentId/...`.
2230+
const scopedMatch = cleanPath.match(/^\/environments\/[^/]+(\/.*)?$/);
21992231
if (scopedMatch) {
22002232
cleanPath = scopedMatch[1] ?? '';
22012233
}

packages/runtime/src/security/permission-denied-envelope.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
* Mirrors `@objectstack/rest`'s `req.params?.object` on the `/data/:object`
6262
* family — the only dispatcher routes whose path carries an object name.
6363
* Expects the dispatcher's already-cleaned path (trailing slash removed,
64-
* `/projects/:environmentId` prefix stripped).
64+
* `/environments/:environmentId` prefix stripped).
6565
*/
6666
export function routeObjectFromPath(cleanPath: string): string | undefined {
6767
const m = /^\/data\/([^/?#]+)/.exec(cleanPath);

0 commit comments

Comments
 (0)