Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/domain-claim-segment-boundary.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"@objectstack/runtime": minor
---

Dispatcher domain routes stop claiming their lexical neighbours: `DomainRoute.match` now defaults to `'segment'`, and the project-membership skip list gained the same boundary.

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.

- **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.
- **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.
- **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.

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.
36 changes: 32 additions & 4 deletions packages/runtime/src/domain-handler-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
*
* Two layers under test:
* 1. `DomainHandlerRegistry` matching semantics (first-match, exact vs
* prefix, method restriction) — deliberately faithful to the legacy
* if-chain, rough edges included.
* segment vs prefix, method restriction). These were deliberately
* faithful to the legacy if-chain, rough edges included, until #16263
* made `'segment'` the default; the legacy bare-`startsWith` claim is
* still available to a route that declares `match: 'prefix'`.
* 2. `HttpDispatcher` integration: the four seeded builtin domains
* (/health /ready /analytics /i18n) behave exactly as their legacy
* if-chain branches did, and `registerDomainHandler` is the public
Expand Down Expand Up @@ -65,17 +67,43 @@ describe('DomainHandlerRegistry', () => {
expect(registry.resolve('/a/x', 'GET')?.handler).toBe(first);
});

it("match: 'exact' does not claim sub-paths; default prefix match does (legacy startsWith, rough edges included)", () => {
it("match: 'exact' does not claim sub-paths; the DEFAULT claims the prefix and everything under it, and stops there", () => {
const registry = new DomainHandlerRegistry();
registry.register({ prefix: '/health', match: 'exact', handler: okHandler('h') });
registry.register({ prefix: '/i18n', handler: okHandler('i') });
expect(registry.resolve('/health', 'GET')).toBeDefined();
expect(registry.resolve('/health/deep', 'GET')).toBeUndefined();
expect(registry.resolve('/i18n', 'GET')).toBeDefined();
expect(registry.resolve('/i18n/locales', 'GET')).toBeDefined();
// [#16263] The default is `'segment'`. This assertion USED TO READ
// `toBeDefined()` and pinned the legacy rough edge on purpose
// ("bare startsWith also matches '/i18nxx'"); the edge is the defect
// #16263 removed, so the pin is inverted rather than deleted — the
// sibling namespace must be provably released, not merely unasserted.
expect(registry.resolve('/i18nxx', 'GET')).toBeUndefined();
});

it("match: 'prefix' still buys the legacy bare-startsWith claim — it is declared now, not inherited", () => {
const registry = new DomainHandlerRegistry();
registry.register({ prefix: '/i18n', match: 'prefix', handler: okHandler('i') });
expect(registry.resolve('/i18n', 'GET')).toBeDefined();
expect(registry.resolve('/i18n/locales', 'GET')).toBeDefined();
// Faithful legacy semantics: bare startsWith also matches '/i18nxx'.
expect(registry.resolve('/i18nxx', 'GET')).toBeDefined();
});

it("a `?`-suffixed prefix is why 'prefix' survives: no '/' follows the '?', so 'segment' cannot express it", () => {
const registry = new DomainHandlerRegistry();
registry.register({ prefix: '/keys', match: 'segment', handler: okHandler('k') });
registry.register({ prefix: '/keys?', match: 'prefix', handler: okHandler('kq') });
expect(registry.resolve('/keys', 'GET')?.prefix).toBe('/keys');
expect(registry.resolve('/keys/rotate', 'GET')?.prefix).toBe('/keys');
expect(registry.resolve('/keys?scope=x', 'GET')?.prefix).toBe('/keys?');
// …and the segment route alone would NOT have claimed the query form.
const segmentOnly = new DomainHandlerRegistry();
segmentOnly.register({ prefix: '/keys', match: 'segment', handler: okHandler('k') });
expect(segmentOnly.resolve('/keys?scope=x', 'GET')).toBeUndefined();
});

it('restricts by method when `methods` is set (case-insensitive on input)', () => {
const registry = new DomainHandlerRegistry();
registry.register({ prefix: '/health', match: 'exact', methods: ['GET'], handler: okHandler('h') });
Expand Down
58 changes: 48 additions & 10 deletions packages/runtime/src/domain-handler-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,13 @@
* a slot exclusively can still self-register via
* {@link HttpDispatcher.registerDomainHandler}.
*
* Matching semantics are deliberately faithful to the legacy if-chain,
* INCLUDING its rough edges (`match: 'prefix'` on `/i18n` also matches
* `/i18nxx`, exactly as `startsWith` did) — fixing those edges is explicitly
* not this seam's job; behavior preservation is.
* Matching semantics were deliberately faithful to the legacy if-chain,
* INCLUDING its rough edges, for as long as the migration needed behaviour
* preservation to be the only promise this seam made. That period is over and
* the edges are fixed (#16263): a domain claim now stops at a SEGMENT
* BOUNDARY by default, so `/i18n` no longer claims `/i18nxx`. The legacy
* `startsWith` shape is still reachable, but only where a route ASKS for it in
* writing (`match: 'prefix'`) — see {@link DomainRoute.match}.
*/

import type { HttpProtocolContext, HttpDispatcherResult } from './http-dispatcher.js';
Expand Down Expand Up @@ -60,10 +63,43 @@ export interface DomainRoute {
/** Path prefix the domain claims, e.g. `'/i18n'`. */
prefix: string;
/**
* `'prefix'` — legacy `startsWith(prefix)` semantics (default).
* How much of the path space this route claims.
*
* `'segment'` — **the default**: the path equals the prefix, or is
* followed by `'/'`. Claims `/i18n` and everything under `/i18n/`, and
* does NOT claim `/i18nxx`.
* `'exact'` — the path must equal the prefix exactly.
* `'segment'` — exact, or followed by `'/'` (the legacy
* `=== p || startsWith(p + '/')` branch shape; does NOT claim `/i18nxx`).
* `'prefix'` — bare `startsWith(prefix)`, NO segment boundary: the legacy
* if-chain's shape, which also claims `/i18nxx`.
*
* ## Why `'segment'` is the default and `'prefix'` must be asked for
*
* The reasoning is #16026's, applied to the whole table rather than to one
* prefix. A bare `startsWith` claim reaches SIBLING NAMESPACES: `/authx`,
* `/authentication/foo`, `/datax`, `/metaxyz`, `/uifoo` are not paths of
* the domain that was claiming them by any reading, and each is a
* plausible namespace someone mounts later — a route registered there is
* SHADOWED by a domain that never wanted it. `'segment'` claims the prefix
* exactly and everything under `prefix + '/'`, which is the whole of what
* a domain owns, so narrowing to it removes only claims a domain does not
* own and keeps every sub-path fallthrough intact (#4088's
* `/auth/me/permissions` is the case that pins that half).
*
* `'segment'` was already the codebase's own spelling for a
* boundary-correct claim — `/auth`, `/keys`, `/mcp`, `/mcp/skill`,
* `/security` and `/share-links` each declared it — so this makes the
* table's majority spelling its default rather than introducing a
* convention.
*
* ⚠️ `'prefix'` is NOT deprecated, and one shape genuinely needs it: a
* prefix ending in `'?'` (`'/keys?'`, `'/mcp?'`), which reproduces the
* legacy branch's query-string form for adapters that pass the query
* through in `path`. There is no `/` after that `'?'`, so a segment match
* cannot express it. Those routes declare `match: 'prefix'` in writing.
*
* ⛔ Do not reach for `'prefix'` to widen a domain's claim over its
* lexical neighbours. The default changed because that claim was never
* anything but a migration artefact.
*/
match?: 'prefix' | 'exact' | 'segment';
/** Restrict to these UPPERCASE HTTP methods. Omit = all methods. */
Expand Down Expand Up @@ -362,10 +398,12 @@ export class DomainHandlerRegistry {
switch (route.match) {
case 'exact':
return path === route.prefix;
case 'segment':
return path === route.prefix || path.startsWith(route.prefix + '/');
default:
case 'prefix':
// Bare `startsWith`, no segment boundary — the legacy
// if-chain's shape, now reachable only by asking for it.
return path.startsWith(route.prefix);
default:
return path === route.prefix || path.startsWith(route.prefix + '/');
}
}

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