Skip to content

Commit ce8bfc9

Browse files
claude[bot]claude
andauthored
fix(runtime): domain claims and the membership skip list stop at a segment boundary (#16842)
* wip: #16263 segment-boundary domain claims * fix(runtime): domain claims and the membership skip list stop at a segment boundary `DomainRoute.match` defaulted to `'prefix'` — a bare `path.startsWith(prefix)` with no segment boundary — and ten shipped routes carried that implicit default, so `/datax`, `/metaxyz`, `/uifoo`, `/aixx` and `/packagesomething` were each claimed by a domain that does not own them. `/auth` was the eleventh member of the family and was repaired on its own; this closes the rest at the seam so the next domain added is boundary-correct by default. The default is `'segment'` now. `match: 'prefix'` still buys the legacy bare `startsWith` claim for a route that asks for it in writing, and the three `?`-suffixed routes (`/keys?`, `/mcp?`, `/mcp/skill?`) declare it — a prefix ending in `'?'` has no `/` after it, so a segment match cannot express the query-string form at all. `enforceProjectMembership`'s control-plane skip list carried the same predicate with a worse consequence: a claim that is too wide sends traffic somewhere wrong, a skip list that is too wide sends traffic past a check. `/authentication/foo` satisfied `startsWith('/auth')` and was waved past the membership check. Its boundary is `'/'`, `'?'` or end-of-string, so `/auth?redirect=...` keeps the exemption it has today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a814bdb commit ce8bfc9

9 files changed

Lines changed: 593 additions & 20 deletions
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/runtime": minor
3+
---
4+
5+
Dispatcher domain routes stop claiming their lexical neighbours: `DomainRoute.match` now defaults to `'segment'`, and the project-membership skip list gained the same boundary.
6+
7+
Ten shipped routes — `/actions`, `/ai`, `/analytics`, `/automation`, `/data`, `/i18n`, `/meta`, `/notifications`, `/packages`, `/ui` — carried the implicit `'prefix'` default, a bare `path.startsWith(prefix)` with no segment boundary. So `/datax`, `/metaxyz`, `/uifoo`, `/aixx` and `/packagesomething` were each claimed by a domain that does not own them, and a package mounting one of those namespaces later would have been shadowed by a domain that never wanted it. `/auth` was the eleventh member of the family and was repaired on its own; this closes the rest at the seam rather than one route at a time, so the eleventh domain someone adds is boundary-correct without having to remember anything.
8+
9+
- **The default moved, the mode did not go away.** `match: 'segment'` (the prefix exactly, plus everything under `prefix + '/'`) is the default; `match: 'prefix'` still buys the bare `startsWith` claim for a route that asks for it in writing. One shape genuinely needs it and now declares it: a prefix ending in `'?'` (`/keys?`, `/mcp?`, `/mcp/skill?`), which reproduces the legacy branch's query-string form for adapters that pass the query through in the path. There is no `/` after that `'?'`, so a segment match cannot express it — those three routes match exactly what they always did.
10+
- **What each narrowed claim used to answer, measured per domain rather than assumed.** They were not uniform: `/actionsx`, `/aixx`, `/automationx`, `/metaxyz` and `/packagesomething` answered `401`; `/i18nxx` and `/notificationsx` answered `501`; `/analyticsx` and `/uifoo` fell through unhandled. `/data` was the worst and the reason per-domain measurement was owed — its handler reads the sub-path as an OBJECT NAME, so `GET /datax` answered a **success envelope for a fabricated object** and `GET /datax/foo` **threw** `Record foo not found in x`. Each of the ten now answers the dispatcher's `ROUTE_NOT_FOUND` envelope, which is what they should always have answered. No caller depended on any of these: nothing in the repo builds a dispatch path by concatenating a domain prefix without a separator, and no route-ledger row or SDK method addresses a shape of this kind.
11+
- **The membership skip list, which was the same predicate with a worse consequence.** `enforceProjectMembership` skipped the control plane with `skipPaths.some(p => path.startsWith(p))` and `'/auth'` in the list, so `/authentication/foo` was waved **past the membership check** rather than merely routed somewhere wrong. It was latent — nothing claims `/authentication/*`, so such a request 404s first — and it would have gone live the day any domain claimed a path of that shape. The skip list stops at `'/'`, `'?'` or end-of-string now; the `'?'` form is part of the boundary on purpose, so `/auth?redirect=…` keeps the exemption it has today and the control plane is not newly gated.
12+
13+
Every domain still claims itself and every path under it, `/auth/me/permissions` included; the registry header comment that described the old rough edges as deliberate no longer describes code that has them.

packages/runtime/src/domain-handler-registry.test.ts

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
*
66
* Two layers under test:
77
* 1. `DomainHandlerRegistry` matching semantics (first-match, exact vs
8-
* prefix, method restriction) — deliberately faithful to the legacy
9-
* if-chain, rough edges included.
8+
* segment vs prefix, method restriction). These were deliberately
9+
* faithful to the legacy if-chain, rough edges included, until #16263
10+
* made `'segment'` the default; the legacy bare-`startsWith` claim is
11+
* still available to a route that declares `match: 'prefix'`.
1012
* 2. `HttpDispatcher` integration: the four seeded builtin domains
1113
* (/health /ready /analytics /i18n) behave exactly as their legacy
1214
* if-chain branches did, and `registerDomainHandler` is the public
@@ -65,17 +67,43 @@ describe('DomainHandlerRegistry', () => {
6567
expect(registry.resolve('/a/x', 'GET')?.handler).toBe(first);
6668
});
6769

68-
it("match: 'exact' does not claim sub-paths; default prefix match does (legacy startsWith, rough edges included)", () => {
70+
it("match: 'exact' does not claim sub-paths; the DEFAULT claims the prefix and everything under it, and stops there", () => {
6971
const registry = new DomainHandlerRegistry();
7072
registry.register({ prefix: '/health', match: 'exact', handler: okHandler('h') });
7173
registry.register({ prefix: '/i18n', handler: okHandler('i') });
7274
expect(registry.resolve('/health', 'GET')).toBeDefined();
7375
expect(registry.resolve('/health/deep', 'GET')).toBeUndefined();
76+
expect(registry.resolve('/i18n', 'GET')).toBeDefined();
77+
expect(registry.resolve('/i18n/locales', 'GET')).toBeDefined();
78+
// [#16263] The default is `'segment'`. This assertion USED TO READ
79+
// `toBeDefined()` and pinned the legacy rough edge on purpose
80+
// ("bare startsWith also matches '/i18nxx'"); the edge is the defect
81+
// #16263 removed, so the pin is inverted rather than deleted — the
82+
// sibling namespace must be provably released, not merely unasserted.
83+
expect(registry.resolve('/i18nxx', 'GET')).toBeUndefined();
84+
});
85+
86+
it("match: 'prefix' still buys the legacy bare-startsWith claim — it is declared now, not inherited", () => {
87+
const registry = new DomainHandlerRegistry();
88+
registry.register({ prefix: '/i18n', match: 'prefix', handler: okHandler('i') });
89+
expect(registry.resolve('/i18n', 'GET')).toBeDefined();
7490
expect(registry.resolve('/i18n/locales', 'GET')).toBeDefined();
75-
// Faithful legacy semantics: bare startsWith also matches '/i18nxx'.
7691
expect(registry.resolve('/i18nxx', 'GET')).toBeDefined();
7792
});
7893

94+
it("a `?`-suffixed prefix is why 'prefix' survives: no '/' follows the '?', so 'segment' cannot express it", () => {
95+
const registry = new DomainHandlerRegistry();
96+
registry.register({ prefix: '/keys', match: 'segment', handler: okHandler('k') });
97+
registry.register({ prefix: '/keys?', match: 'prefix', handler: okHandler('kq') });
98+
expect(registry.resolve('/keys', 'GET')?.prefix).toBe('/keys');
99+
expect(registry.resolve('/keys/rotate', 'GET')?.prefix).toBe('/keys');
100+
expect(registry.resolve('/keys?scope=x', 'GET')?.prefix).toBe('/keys?');
101+
// …and the segment route alone would NOT have claimed the query form.
102+
const segmentOnly = new DomainHandlerRegistry();
103+
segmentOnly.register({ prefix: '/keys', match: 'segment', handler: okHandler('k') });
104+
expect(segmentOnly.resolve('/keys?scope=x', 'GET')).toBeUndefined();
105+
});
106+
79107
it('restricts by method when `methods` is set (case-insensitive on input)', () => {
80108
const registry = new DomainHandlerRegistry();
81109
registry.register({ prefix: '/health', match: 'exact', methods: ['GET'], handler: okHandler('h') });

packages/runtime/src/domain-handler-registry.ts

Lines changed: 48 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,13 @@
2727
* a slot exclusively can still self-register via
2828
* {@link HttpDispatcher.registerDomainHandler}.
2929
*
30-
* Matching semantics are deliberately faithful to the legacy if-chain,
31-
* INCLUDING its rough edges (`match: 'prefix'` on `/i18n` also matches
32-
* `/i18nxx`, exactly as `startsWith` did) — fixing those edges is explicitly
33-
* not this seam's job; behavior preservation is.
30+
* Matching semantics were deliberately faithful to the legacy if-chain,
31+
* INCLUDING its rough edges, for as long as the migration needed behaviour
32+
* preservation to be the only promise this seam made. That period is over and
33+
* the edges are fixed (#16263): a domain claim now stops at a SEGMENT
34+
* BOUNDARY by default, so `/i18n` no longer claims `/i18nxx`. The legacy
35+
* `startsWith` shape is still reachable, but only where a route ASKS for it in
36+
* writing (`match: 'prefix'`) — see {@link DomainRoute.match}.
3437
*/
3538

3639
import type { HttpProtocolContext, HttpDispatcherResult } from './http-dispatcher.js';
@@ -60,10 +63,43 @@ export interface DomainRoute {
6063
/** Path prefix the domain claims, e.g. `'/i18n'`. */
6164
prefix: string;
6265
/**
63-
* `'prefix'` — legacy `startsWith(prefix)` semantics (default).
66+
* How much of the path space this route claims.
67+
*
68+
* `'segment'` — **the default**: the path equals the prefix, or is
69+
* followed by `'/'`. Claims `/i18n` and everything under `/i18n/`, and
70+
* does NOT claim `/i18nxx`.
6471
* `'exact'` — the path must equal the prefix exactly.
65-
* `'segment'` — exact, or followed by `'/'` (the legacy
66-
* `=== p || startsWith(p + '/')` branch shape; does NOT claim `/i18nxx`).
72+
* `'prefix'` — bare `startsWith(prefix)`, NO segment boundary: the legacy
73+
* if-chain's shape, which also claims `/i18nxx`.
74+
*
75+
* ## Why `'segment'` is the default and `'prefix'` must be asked for
76+
*
77+
* The reasoning is #16026's, applied to the whole table rather than to one
78+
* prefix. A bare `startsWith` claim reaches SIBLING NAMESPACES: `/authx`,
79+
* `/authentication/foo`, `/datax`, `/metaxyz`, `/uifoo` are not paths of
80+
* the domain that was claiming them by any reading, and each is a
81+
* plausible namespace someone mounts later — a route registered there is
82+
* SHADOWED by a domain that never wanted it. `'segment'` claims the prefix
83+
* exactly and everything under `prefix + '/'`, which is the whole of what
84+
* a domain owns, so narrowing to it removes only claims a domain does not
85+
* own and keeps every sub-path fallthrough intact (#4088's
86+
* `/auth/me/permissions` is the case that pins that half).
87+
*
88+
* `'segment'` was already the codebase's own spelling for a
89+
* boundary-correct claim — `/auth`, `/keys`, `/mcp`, `/mcp/skill`,
90+
* `/security` and `/share-links` each declared it — so this makes the
91+
* table's majority spelling its default rather than introducing a
92+
* convention.
93+
*
94+
* ⚠️ `'prefix'` is NOT deprecated, and one shape genuinely needs it: a
95+
* prefix ending in `'?'` (`'/keys?'`, `'/mcp?'`), which reproduces the
96+
* legacy branch's query-string form for adapters that pass the query
97+
* through in `path`. There is no `/` after that `'?'`, so a segment match
98+
* cannot express it. Those routes declare `match: 'prefix'` in writing.
99+
*
100+
* ⛔ Do not reach for `'prefix'` to widen a domain's claim over its
101+
* lexical neighbours. The default changed because that claim was never
102+
* anything but a migration artefact.
67103
*/
68104
match?: 'prefix' | 'exact' | 'segment';
69105
/** Restrict to these UPPERCASE HTTP methods. Omit = all methods. */
@@ -362,10 +398,12 @@ export class DomainHandlerRegistry {
362398
switch (route.match) {
363399
case 'exact':
364400
return path === route.prefix;
365-
case 'segment':
366-
return path === route.prefix || path.startsWith(route.prefix + '/');
367-
default:
401+
case 'prefix':
402+
// Bare `startsWith`, no segment boundary — the legacy
403+
// if-chain's shape, now reachable only by asking for it.
368404
return path.startsWith(route.prefix);
405+
default:
406+
return path === route.prefix || path.startsWith(route.prefix + '/');
369407
}
370408
}
371409

packages/runtime/src/domains/auth.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.
1717
* The route this domain claims — `/auth` and its slash-separated sub-paths,
1818
* and NOTHING ELSE (#16026).
1919
*
20-
* ## Why `match: 'segment'` is spelled out rather than left to the default
20+
* ## Why this claim stops at a segment boundary
2121
*
22-
* `DomainRoute.match` defaults to `'prefix'`, i.e. a bare
22+
* `DomainRoute.match` USED TO default to `'prefix'`, i.e. a bare
2323
* `path.startsWith('/auth')` with no segment boundary — the legacy if-chain's
2424
* shape, which `DomainHandlerRegistry` preserved deliberately when the domains
2525
* were lifted out of it. On this prefix that rough edge claims SIBLING
@@ -51,6 +51,14 @@ import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.
5151
* already declare (`/keys`, `/mcp`, `/mcp/skill`), so this is the codebase's
5252
* own established spelling for the fix, not a new convention.
5353
*
54+
* ⭐ [#16263] It is now also the registry's DEFAULT, so this line no longer
55+
* changes what `/auth` matches — the reasoning above was applied to the whole
56+
* route table rather than to this one prefix. The declaration is kept, not
57+
* deleted: it states at the route what the route claims, which is the fact
58+
* every case in `auth-claim-segment-boundary.test.ts` is about, and it keeps
59+
* this claim pinned to `'segment'` explicitly rather than to whatever the
60+
* default happens to be later.
61+
*
5462
* ⚠️ What this does NOT fix, deliberately: the `200 {}` those rows carried.
5563
* That answer is manufactured one layer OUT, where the adapter renders a
5664
* dispatcher result — the auth service itself answers an honest 404 for every

0 commit comments

Comments
 (0)