Skip to content

Commit 96537da

Browse files
committed
wip: anchor the auth-gate allow-list to a mount boundary
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTBcV7zZHmokdyQgXjbyEU
1 parent cf6e0a1 commit 96537da

3 files changed

Lines changed: 232 additions & 18 deletions

File tree

packages/core/src/security/auth-gate.test.ts

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,128 @@ describe('auth-gate (ADR-0069 session gate)', () => {
2727
expect(isAuthGateAllowlisted('/api/v1/auth/sign-out/?x=1')).toBe(true);
2828
expect(isAuthGateAllowlisted('/api/v1/data/x/')).toBe(false);
2929
});
30+
31+
// ── [#16839] The allow-list is ANCHORED ────────────────────────────────
32+
//
33+
// It used to match unanchored: `path.includes('/auth/')` at ANY position
34+
// and an `endsWith` suffix test at ANY depth. So a segment whose VALUE
35+
// spelled an allow-listed token carried the exemption, and object names
36+
// and record ids are TENANT-CONTROLLED. Both seams hand this predicate a
37+
// data-plane path directly — `HttpDispatcher.enforceAuthGate(context,
38+
// cleanPath)` and `RestServer.enforceAuth` (`req.path`) — so a tenant that
39+
// declared an object named `auth`, or held a record whose id is `health`,
40+
// handed a password-expired / MFA-required session a bypass on that
41+
// object's data routes.
42+
//
43+
// ⛔ These pin the DECISION for the exact paths the card measured, not the
44+
// spelling of the predicate, so they survive a rewrite of it.
45+
describe('[#16839] a tenant-controlled segment cannot buy the exemption', () => {
46+
it('gates the four paths that were falsely exempt', () => {
47+
for (const p of [
48+
'/data/auth/123', // an object named `auth`
49+
'/meta/auth/objects', // an object named `auth`
50+
'/data/x/health', // a record whose id is `health`
51+
'/data/xyz/me/apps',
52+
]) {
53+
expect(isAuthGateAllowlisted(p), p).toBe(false);
54+
}
55+
});
56+
57+
it('gates the same shapes under the REST mount, where the seam sees the base', () => {
58+
for (const p of [
59+
'/api/v1/data/auth/123',
60+
'/api/v1/meta/auth/objects',
61+
'/api/v1/data/health', // `/data/:object` with object = `health`
62+
'/api/v1/data/x/health', // `/data/:object/:id` with id = `health`
63+
'/api/v1/data/contacts/me/apps',
64+
'/api/v1/data/environments/x/health', // a scope-shaped OBJECT name, mid-path
65+
]) {
66+
expect(isAuthGateAllowlisted(p), p).toBe(false);
67+
}
68+
});
69+
70+
// ⭐ The card's own two control rows. Without them the block above would
71+
// read the same for a predicate that had simply started refusing
72+
// everything.
73+
it('CONTROL — the genuinely-exempt path stays exempt and the protected one stays gated', () => {
74+
expect(isAuthGateAllowlisted('/auth/me')).toBe(true);
75+
expect(isAuthGateAllowlisted('/data/contacts/1')).toBe(false);
76+
});
77+
78+
// The other direction, at full width: every mount shape a real
79+
// remediation / bootstrap route arrives in must still be exempt. An
80+
// anchoring that is too strict fails HERE rather than in production.
81+
it('keeps every genuinely-exempt route shape exempt', () => {
82+
for (const p of [
83+
// dispatcher shape — the hono adapter strips the app prefix
84+
'/auth/sign-out', '/auth/two-factor/enable', '/auth/me/localization',
85+
'/auth', '/health', '/ready', '/discovery',
86+
// REST + better-auth mounts
87+
'/api/auth', '/api/auth/sign-in',
88+
'/api/v1/auth', '/api/v1/auth/change-password', '/api/v1/auth/me/permissions',
89+
'/api/v1/health', '/api/v1/ready', '/api/v1/discovery',
90+
'/api/v1/me/apps', '/api/v1/me/localization',
91+
// environment-scoped mount — the dispatcher evaluates the gate
92+
// BEFORE its scoped-URL strip, so this spelling reaches the predicate
93+
'/api/v1/environments/env_1/auth/sign-out',
94+
'/environments/env_1/auth/sign-out',
95+
'/api/v1/environments/env_1/discovery',
96+
// the legacy `projects` spelling of the same scope (ADR-0006)
97+
'/api/v1/projects/env_1/auth/sign-out',
98+
]) {
99+
expect(isAuthGateAllowlisted(p), p).toBe(true);
100+
}
101+
});
102+
103+
// ⭐ CLAUSE ② DISCHARGE — the repair only ever REMOVES exemptions.
104+
//
105+
// The dispatch declared "nothing is newly accepted"; this measures it
106+
// instead of asserting it. `preAnchoringAllowlisted` is the predicate
107+
// this file's subject replaced, transcribed verbatim from `origin/main`
108+
// cf6e0a193b, and the corpus is every path of up to four segments drawn
109+
// from the vocabulary the two spellings can disagree on. A single
110+
// `new && !old` row means a path became NEWLY exempt, which is a
111+
// widening and is not this card's to make.
112+
it('is a strict SUBSET of the pre-anchoring allow-list — nothing becomes newly exempt', () => {
113+
const OLD_PREFIXES = ['/api/v1/auth/', '/api/auth/', '/auth/'];
114+
const OLD_SUFFIXES = ['/health', '/ready', '/discovery', '/me/apps', '/me/localization'];
115+
const preAnchoringAllowlisted = (rawPath: string | undefined | null): boolean => {
116+
if (!rawPath) return true;
117+
let path = rawPath.split('?')[0] || '/';
118+
let end = path.length;
119+
while (end > 1 && path.charCodeAt(end - 1) === 47) end--;
120+
path = path.slice(0, end) || '/';
121+
if (path.includes('/auth/')) return true;
122+
for (const p of OLD_PREFIXES) if (path.startsWith(p) || path === p.replace(/\/$/, '')) return true;
123+
for (const s of OLD_SUFFIXES) if (path.endsWith(s)) return true;
124+
return false;
125+
};
126+
127+
const SEG = ['api', 'v1', 'auth', 'health', 'ready', 'discovery', 'me', 'apps',
128+
'localization', 'data', 'meta', 'ui', 'environments', 'projects', 'env1', 'x'];
129+
const corpus: string[] = ['/', ''];
130+
for (const a of SEG) {
131+
corpus.push(`/${a}`);
132+
for (const b of SEG) {
133+
corpus.push(`/${a}/${b}`);
134+
for (const c of SEG) {
135+
corpus.push(`/${a}/${b}/${c}`);
136+
for (const d of SEG) corpus.push(`/${a}/${b}/${c}/${d}`);
137+
}
138+
}
139+
}
140+
141+
const widened = corpus.filter((p) => isAuthGateAllowlisted(p) && !preAnchoringAllowlisted(p));
142+
expect(widened).toEqual([]);
143+
// Anti-vacuity: the corpus really does exercise both predicates, and
144+
// the repair really did remove exemptions — a corpus that narrowed
145+
// nothing would satisfy the line above without measuring anything.
146+
const narrowed = corpus.filter((p) => !isAuthGateAllowlisted(p) && preAnchoringAllowlisted(p));
147+
expect(corpus.length).toBeGreaterThan(10_000);
148+
expect(narrowed.length).toBeGreaterThan(0);
149+
expect(narrowed).toContain('/data/x/health');
150+
});
151+
});
30152
});
31153

32154
describe('evaluateAuthGate', () => {

packages/core/src/security/auth-gate.ts

Lines changed: 89 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,67 @@ export function normalizeAuthGate(sessionUser: any): AuthGate | null {
5757
}
5858

5959
// Endpoints a gated user MUST still reach to remediate or bootstrap the
60-
// remediation UI. Matched against the request path (query stripped). Covers
61-
// both REST (`/api/v1/auth/…`) and dispatcher (`/auth/…`) path shapes.
62-
const ALLOW_PREFIXES = ['/api/v1/auth/', '/api/auth/', '/auth/'];
63-
const ALLOW_SUFFIXES = ['/health', '/ready', '/discovery', '/me/apps', '/me/localization'];
60+
// remediation UI. Matched against the request path (query stripped).
61+
//
62+
// [#16839] Every test below is ANCHORED to a mount boundary, because the
63+
// allow-list is a set of ROUTES and a route is identified by where it sits,
64+
// not by text appearing somewhere in a path. The two predicates this replaced
65+
// were unanchored — `path.includes('/auth/')` matched at ANY position and a
66+
// suffix test matched at ANY depth — so a segment whose VALUE happened to
67+
// spell an allow-listed token carried the exemption: `/data/auth/123` (an
68+
// object named `auth`), `/data/x/health` (a record whose id is `health`),
69+
// `/data/xyz/me/apps`. Both seams pass a data-plane path straight in
70+
// (`HttpDispatcher.enforceAuthGate`, `RestServer.enforceAuth`), so those were
71+
// reachable requests, and object + record names are tenant-controlled.
72+
73+
/**
74+
* The mount bases an allow-listed route can sit at, as SEGMENT lists, longest
75+
* first. `[]` is the dispatcher shape: the hono adapter hands
76+
* `HttpDispatcher.dispatch` the app prefix already stripped, so a dispatcher
77+
* path arrives as `/auth/…`, `/health`, `/environments/<id>/auth/…`. The other
78+
* two are the REST/better-auth mounts (`${basePath}/${version}` and
79+
* better-auth's own `${basePath}/auth`) at their shipped defaults — the same
80+
* three bases the pre-anchoring `ALLOW_PREFIXES` enumerated.
81+
*
82+
* ⚠️ A host that moves `api.basePath`/`api.version` off those defaults is no
83+
* longer named here. That is the deliberate price of anchoring and it cannot
84+
* be avoided: `/rest/v2/health` and `/data/xyz/health` are the SAME SHAPE, so
85+
* a rule that accepts an arbitrary base is the defect. It costs nothing at
86+
* either live seam — the dispatcher's path is base-stripped (matched by `[]`),
87+
* and REST registers its control-plane routes without `enforceAuth` at all.
88+
*/
89+
const MOUNT_BASES: readonly (readonly string[])[] = [['api', 'v1'], ['api'], []];
90+
91+
/**
92+
* Segments that open an environment scope between the base and the route
93+
* (`/api/v1/environments/<id>/auth/…`). The dispatcher evaluates the gate
94+
* BEFORE its scoped-URL strip, so the scoped spelling reaches this predicate;
95+
* `projects` is ADR-0006's superseded spelling, which the REST scope strip
96+
* still accepts.
97+
*/
98+
const SCOPE_SEGMENTS: readonly string[] = ['environments', 'projects'];
99+
100+
/**
101+
* The bootstrap reads, as EXACT routes at a mount (replaces the old
102+
* `ALLOW_SUFFIXES` endsWith test). `/health`, `/ready` and `/discovery` are
103+
* the dispatcher's probes and discovery document; `/me/apps` and
104+
* `/me/localization` are the current-user reads the remediation UI needs
105+
* (`plugin-hono-server/src/current-user-endpoints.ts`).
106+
*/
107+
const ALLOW_ROUTES: readonly (readonly string[])[] = [
108+
['health'],
109+
['ready'],
110+
['discovery'],
111+
['me', 'apps'],
112+
['me', 'localization'],
113+
];
114+
115+
/** Do `segments` start with every segment of `prefix`? */
116+
function startsWithSegments(segments: readonly string[], prefix: readonly string[]): boolean {
117+
if (segments.length < prefix.length) return false;
118+
for (let k = 0; k < prefix.length; k++) if (segments[k] !== prefix[k]) return false;
119+
return true;
120+
}
64121

65122
/** True when `path` is exempt from the auth gate (auth + remediation + health). */
66123
export function isAuthGateAllowlisted(rawPath: string | undefined | null): boolean {
@@ -71,14 +128,34 @@ export function isAuthGateAllowlisted(rawPath: string | undefined | null): boole
71128
let end = path.length;
72129
while (end > 1 && path.charCodeAt(end - 1) === 47) end--;
73130
path = path.slice(0, end) || '/';
74-
// Any path with an `/auth/` segment is an auth endpoint (covers project-
75-
// scoped mounts like `/api/v1/environments/:env/auth/...`).
76-
if (path.includes('/auth/')) return true;
77-
for (const p of ALLOW_PREFIXES) {
78-
if (path.startsWith(p) || path === p.replace(/\/$/, '')) return true;
79-
}
80-
for (const s of ALLOW_SUFFIXES) {
81-
if (path.endsWith(s)) return true;
131+
// Segment view — `''` entries dropped so `//auth//me` cannot smuggle an
132+
// empty segment past the position tests below.
133+
const segments = path.split('/').filter((s) => s !== '');
134+
for (const base of MOUNT_BASES) {
135+
if (!startsWithSegments(segments, base)) continue;
136+
let i = base.length;
137+
let scoped = false;
138+
// One optional environment scope, and only immediately after the base —
139+
// which is why `/data/environments/x/health` is NOT a scoped `/health`.
140+
if (i + 1 < segments.length && SCOPE_SEGMENTS.includes(segments[i] as string)) {
141+
i += 2;
142+
scoped = true;
143+
}
144+
if (segments[i] === 'auth') {
145+
// `<base>[/<scope>/<id>]/auth/…` — the remediation surface. This is the
146+
// anchored replacement for `path.includes('/auth/')`.
147+
if (i + 1 < segments.length) return true;
148+
// Bare `<base>/auth`, exempt only UNSCOPED: that is exactly what the old
149+
// `ALLOW_PREFIXES` equality branch admitted (`/auth`, `/api/auth`,
150+
// `/api/v1/auth`). The scoped spelling was never exempt and must not
151+
// become so here — this repair only ever removes exemptions.
152+
if (!scoped) return true;
153+
}
154+
for (const route of ALLOW_ROUTES) {
155+
if (segments.length - i === route.length && startsWithSegments(segments.slice(i), route)) {
156+
return true;
157+
}
158+
}
82159
}
83160
return false;
84161
}

packages/rest/src/auth-gate-allowlist-fault-window.measurement.test.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -379,13 +379,28 @@ describe('[#15021] §3 reachability — which MOUNTED REST routes carry an allow
379379
it('READING (not a drive): the gate block sits on the shared identity path, so EVERY route that resolves a context inherits the refusal', () => {
380380
// The census above is about PATTERNS this server mounts. It is not a claim
381381
// about concrete requests: `/api/v1/data/:object` with `object` = `health`
382-
// materializes `/api/v1/data/health`, which `isAuthGateAllowlisted` answers
383-
// `true` for (the `ALLOW_SUFFIXES` rule is a suffix test, not a route test).
384-
// That over-broad direction is #7898's subject, ⛔ not this card's, and it
385-
// is recorded here only so the census is not read as "the allow-list never
386-
// fires on this door".
387-
expect(isAuthGateAllowlisted('/api/v1/data/health')).toBe(true);
382+
// materializes `/api/v1/data/health`, and that concrete path used to be
383+
// ALLOW-LISTED — which is what made the over-broad direction reachable at
384+
// this very door.
385+
//
386+
// ⚠️ RE-AIMED IN PLACE (#16839), per this file's own header instruction.
387+
// The superseded assertion and its reason, verbatim:
388+
//
389+
// // ... which `isAuthGateAllowlisted` answers `true` for (the
390+
// // `ALLOW_SUFFIXES` rule is a suffix test, not a route test).
391+
// // That over-broad direction is #7898's subject, ⛔ not this card's
392+
// expect(isAuthGateAllowlisted('/api/v1/data/health')).toBe(true);
393+
//
394+
// The allow-list is anchored to a mount boundary now, so a record id or an
395+
// object name can no longer spell its way into the exemption. The reading
396+
// this leg exists to record is UNCHANGED — the census must still not be
397+
// read as "the allow-list never fires on this door", and the discovery
398+
// document below is what makes it fire.
399+
expect(isAuthGateAllowlisted('/api/v1/data/health')).toBe(false);
388400
expect(isAuthGateAllowlisted('/api/v1/data/sys_user')).toBe(false);
401+
// ⭐ POSITIVE CONTROL — the predicate did not simply start refusing
402+
// everything: the allow-listed mounted route the census found still is.
403+
expect(isAuthGateAllowlisted('/api/v1/discovery')).toBe(true);
389404
});
390405
});
391406

0 commit comments

Comments
 (0)