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/auth-gate-allowlist-anchored.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"@objectstack/core": patch
---

`isAuthGateAllowlisted` matches allow-listed routes at a mount boundary, so an object named `auth` or a record whose id is `health` no longer bypasses the ADR-0069 authentication-policy gate.

The predicate that decides which paths are exempt from the password-expiry / enforced-MFA gate matched with two UNANCHORED tests: `path.includes('/auth/')` matched at any position, and an `endsWith` test over `['/health', '/ready', '/discovery', '/me/apps', '/me/localization']` matched at any depth. A path segment whose VALUE merely spelled one of those tokens therefore carried the exemption — and object names and record ids are tenant-controlled. Both transport seams hand the predicate a data-plane path directly (`HttpDispatcher.enforceAuthGate` passes `cleanPath`, `RestServer.enforceAuth` passes `req.path`), so these were reachable requests. Measured on the built package before the repair: `/data/auth/123`, `/meta/auth/objects`, `/data/x/health` and `/data/xyz/me/apps` were all exempt, while `/auth/me` (exempt) and `/data/contacts/1` (gated) held as controls.

- **What replaced them.** The path is read as segments and each test is anchored to a mount base — `/api/v1`, `/api`, or the empty base the dispatcher sees (the hono adapter hands `dispatch()` the app prefix already stripped) — plus at most one environment scope immediately after that base (`/environments/<id>`, or ADR-0006's superseded `/projects/<id>`), because the dispatcher evaluates the gate before its scoped-URL strip. `/auth/…` at that position stays exempt; the five bootstrap reads are EXACT routes there instead of suffixes. The scope is only recognised immediately after a base, which is why `/data/environments/x/health` is not a scoped `/health`.
- **This only ever removes exemptions.** Measured, not asserted: over a generated corpus of 111,152 paths, the number that are newly exempt is **0** and 25,979 stopped being exempt. The check is kept as a test, with the pre-anchoring predicate transcribed beside it, so a later widening cannot arrive quietly.
- **Every genuinely-exempt shape still is**, pinned in both directions: `/auth/sign-out`, `/health`, `/ready`, `/discovery` (dispatcher shapes); `/api/auth/sign-in`, `/api/v1/auth/change-password`, `/api/v1/auth/me/permissions`, `/api/v1/health`, `/api/v1/me/apps`, `/api/v1/me/localization`; and the scoped `/api/v1/environments/<id>/auth/sign-out`.

**If you serve the API from a non-default mount,** an allow-listed route reached as `${basePath}/${version}/…` with `basePath`/`version` moved off `/api` and `v1` is no longer named by the allow-list. That price cannot be avoided: `/rest/v2/health` and `/data/xyz/health` are the same shape, so a rule that accepts an arbitrary base is the defect itself. It costs nothing at either live seam — the dispatcher's path arrives base-stripped, and REST registers its control-plane routes without `enforceAuth` at all — but if you gate a custom mount through this predicate, mount the remediation routes under one of the named bases.
2 changes: 1 addition & 1 deletion docs/qa/platform-checklist/areas/access-security.json
Original file line number Diff line number Diff line change
Expand Up @@ -2599,7 +2599,7 @@
},
"source": [
"packages/plugins/plugin-hono-server/src/current-user-endpoints.ts#tabPermissions (/auth/me/permissions aggregation + most-permissive merge), (/auth/me/localization), (/me/apps requiredPermissions/tabPermissions filter), (the /api/v1 prefix)",
"packages/core/src/security/auth-gate.ts#ALLOW_SUFFIXES (ALLOW_SUFFIXES — /me/apps + /me/localization reachable to gated users)",
"packages/core/src/security/auth-gate.ts#ALLOW_ROUTES (ALLOW_ROUTES — /me/apps + /me/localization reachable to gated users. Was ALLOW_SUFFIXES, an endsWith test that also exempted any path merely ENDING in those two — /data/xyz/me/apps among them; the allow-list is anchored to a mount base now, so these are EXACT routes at a mount and a record id can no longer spell its way into the exemption)",
"#7616 (delegated permission-set resolution — the enforcement path's own answer), #2752 (/me/apps registry sourcing), #3391 (effective apiOperations annotation), #4093 (guarded degraded branch), ADR-0090 D5 (additive baseline)",
"cross-ref access-security.anonymous-deny-surfaces — the 401 floor this trio is the declared exception to",
"cross-ref access-security.fls-mask-and-strip — owns the FLS enforcement this item's clause 2 cross-checks"
Expand Down
122 changes: 122 additions & 0 deletions packages/core/src/security/auth-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,128 @@ describe('auth-gate (ADR-0069 session gate)', () => {
expect(isAuthGateAllowlisted('/api/v1/auth/sign-out/?x=1')).toBe(true);
expect(isAuthGateAllowlisted('/api/v1/data/x/')).toBe(false);
});

// ── [#16839] The allow-list is ANCHORED ────────────────────────────────
//
// It used to match unanchored: `path.includes('/auth/')` at ANY position
// and an `endsWith` suffix test at ANY depth. So a segment whose VALUE
// spelled an allow-listed token carried the exemption, and object names
// and record ids are TENANT-CONTROLLED. Both seams hand this predicate a
// data-plane path directly — `HttpDispatcher.enforceAuthGate(context,
// cleanPath)` and `RestServer.enforceAuth` (`req.path`) — so a tenant that
// declared an object named `auth`, or held a record whose id is `health`,
// handed a password-expired / MFA-required session a bypass on that
// object's data routes.
//
// ⛔ These pin the DECISION for the exact paths the card measured, not the
// spelling of the predicate, so they survive a rewrite of it.
describe('[#16839] a tenant-controlled segment cannot buy the exemption', () => {
it('gates the four paths that were falsely exempt', () => {
for (const p of [
'/data/auth/123', // an object named `auth`
'/meta/auth/objects', // an object named `auth`
'/data/x/health', // a record whose id is `health`
'/data/xyz/me/apps',
]) {
expect(isAuthGateAllowlisted(p), p).toBe(false);
}
});

it('gates the same shapes under the REST mount, where the seam sees the base', () => {
for (const p of [
'/api/v1/data/auth/123',
'/api/v1/meta/auth/objects',
'/api/v1/data/health', // `/data/:object` with object = `health`
'/api/v1/data/x/health', // `/data/:object/:id` with id = `health`
'/api/v1/data/contacts/me/apps',
'/api/v1/data/environments/x/health', // a scope-shaped OBJECT name, mid-path
]) {
expect(isAuthGateAllowlisted(p), p).toBe(false);
}
});

// ⭐ The card's own two control rows. Without them the block above would
// read the same for a predicate that had simply started refusing
// everything.
it('CONTROL — the genuinely-exempt path stays exempt and the protected one stays gated', () => {
expect(isAuthGateAllowlisted('/auth/me')).toBe(true);
expect(isAuthGateAllowlisted('/data/contacts/1')).toBe(false);
});

// The other direction, at full width: every mount shape a real
// remediation / bootstrap route arrives in must still be exempt. An
// anchoring that is too strict fails HERE rather than in production.
it('keeps every genuinely-exempt route shape exempt', () => {
for (const p of [
// dispatcher shape — the hono adapter strips the app prefix
'/auth/sign-out', '/auth/two-factor/enable', '/auth/me/localization',
'/auth', '/health', '/ready', '/discovery',
// REST + better-auth mounts
'/api/auth', '/api/auth/sign-in',
'/api/v1/auth', '/api/v1/auth/change-password', '/api/v1/auth/me/permissions',
'/api/v1/health', '/api/v1/ready', '/api/v1/discovery',
'/api/v1/me/apps', '/api/v1/me/localization',
// environment-scoped mount — the dispatcher evaluates the gate
// BEFORE its scoped-URL strip, so this spelling reaches the predicate
'/api/v1/environments/env_1/auth/sign-out',
'/environments/env_1/auth/sign-out',
'/api/v1/environments/env_1/discovery',
// the legacy `projects` spelling of the same scope (ADR-0006)
'/api/v1/projects/env_1/auth/sign-out',
]) {
expect(isAuthGateAllowlisted(p), p).toBe(true);
}
});

// ⭐ CLAUSE ② DISCHARGE — the repair only ever REMOVES exemptions.
//
// The dispatch declared "nothing is newly accepted"; this measures it
// instead of asserting it. `preAnchoringAllowlisted` is the predicate
// this file's subject replaced, transcribed verbatim from `origin/main`
// cf6e0a193b, and the corpus is every path of up to four segments drawn
// from the vocabulary the two spellings can disagree on. A single
// `new && !old` row means a path became NEWLY exempt, which is a
// widening and is not this card's to make.
it('is a strict SUBSET of the pre-anchoring allow-list — nothing becomes newly exempt', () => {
const OLD_PREFIXES = ['/api/v1/auth/', '/api/auth/', '/auth/'];
const OLD_SUFFIXES = ['/health', '/ready', '/discovery', '/me/apps', '/me/localization'];
const preAnchoringAllowlisted = (rawPath: string | undefined | null): boolean => {
if (!rawPath) return true;
let path = rawPath.split('?')[0] || '/';
let end = path.length;
while (end > 1 && path.charCodeAt(end - 1) === 47) end--;
path = path.slice(0, end) || '/';
if (path.includes('/auth/')) return true;
for (const p of OLD_PREFIXES) if (path.startsWith(p) || path === p.replace(/\/$/, '')) return true;
for (const s of OLD_SUFFIXES) if (path.endsWith(s)) return true;
return false;
};

const SEG = ['api', 'v1', 'auth', 'health', 'ready', 'discovery', 'me', 'apps',
'localization', 'data', 'meta', 'ui', 'environments', 'projects', 'env1', 'x'];
const corpus: string[] = ['/', ''];
for (const a of SEG) {
corpus.push(`/${a}`);
for (const b of SEG) {
corpus.push(`/${a}/${b}`);
for (const c of SEG) {
corpus.push(`/${a}/${b}/${c}`);
for (const d of SEG) corpus.push(`/${a}/${b}/${c}/${d}`);
}
}
}

const widened = corpus.filter((p) => isAuthGateAllowlisted(p) && !preAnchoringAllowlisted(p));
expect(widened).toEqual([]);
// Anti-vacuity: the corpus really does exercise both predicates, and
// the repair really did remove exemptions — a corpus that narrowed
// nothing would satisfy the line above without measuring anything.
const narrowed = corpus.filter((p) => !isAuthGateAllowlisted(p) && preAnchoringAllowlisted(p));
expect(corpus.length).toBeGreaterThan(10_000);
expect(narrowed.length).toBeGreaterThan(0);
expect(narrowed).toContain('/data/x/health');
});
});
});

describe('evaluateAuthGate', () => {
Expand Down
101 changes: 89 additions & 12 deletions packages/core/src/security/auth-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,67 @@ export function normalizeAuthGate(sessionUser: any): AuthGate | null {
}

// Endpoints a gated user MUST still reach to remediate or bootstrap the
// remediation UI. Matched against the request path (query stripped). Covers
// both REST (`/api/v1/auth/…`) and dispatcher (`/auth/…`) path shapes.
const ALLOW_PREFIXES = ['/api/v1/auth/', '/api/auth/', '/auth/'];
const ALLOW_SUFFIXES = ['/health', '/ready', '/discovery', '/me/apps', '/me/localization'];
// remediation UI. Matched against the request path (query stripped).
//
// [#16839] Every test below is ANCHORED to a mount boundary, because the
// allow-list is a set of ROUTES and a route is identified by where it sits,
// not by text appearing somewhere in a path. The two predicates this replaced
// were unanchored — `path.includes('/auth/')` matched at ANY position and a
// suffix test matched at ANY depth — so a segment whose VALUE happened to
// spell an allow-listed token carried the exemption: `/data/auth/123` (an
// object named `auth`), `/data/x/health` (a record whose id is `health`),
// `/data/xyz/me/apps`. Both seams pass a data-plane path straight in
// (`HttpDispatcher.enforceAuthGate`, `RestServer.enforceAuth`), so those were
// reachable requests, and object + record names are tenant-controlled.

/**
* The mount bases an allow-listed route can sit at, as SEGMENT lists, longest
* first. `[]` is the dispatcher shape: the hono adapter hands
* `HttpDispatcher.dispatch` the app prefix already stripped, so a dispatcher
* path arrives as `/auth/…`, `/health`, `/environments/<id>/auth/…`. The other
* two are the REST/better-auth mounts (`${basePath}/${version}` and
* better-auth's own `${basePath}/auth`) at their shipped defaults — the same
* three bases the pre-anchoring `ALLOW_PREFIXES` enumerated.
*
* ⚠️ A host that moves `api.basePath`/`api.version` off those defaults is no
* longer named here. That is the deliberate price of anchoring and it cannot
* be avoided: `/rest/v2/health` and `/data/xyz/health` are the SAME SHAPE, so
* a rule that accepts an arbitrary base is the defect. It costs nothing at
* either live seam — the dispatcher's path is base-stripped (matched by `[]`),
* and REST registers its control-plane routes without `enforceAuth` at all.
*/
const MOUNT_BASES: readonly (readonly string[])[] = [['api', 'v1'], ['api'], []];

/**
* Segments that open an environment scope between the base and the route
* (`/api/v1/environments/<id>/auth/…`). The dispatcher evaluates the gate
* BEFORE its scoped-URL strip, so the scoped spelling reaches this predicate;
* `projects` is ADR-0006's superseded spelling, which the REST scope strip
* still accepts.
*/
const SCOPE_SEGMENTS: readonly string[] = ['environments', 'projects'];

/**
* The bootstrap reads, as EXACT routes at a mount (replaces the old
* `ALLOW_SUFFIXES` endsWith test). `/health`, `/ready` and `/discovery` are
* the dispatcher's probes and discovery document; `/me/apps` and
* `/me/localization` are the current-user reads the remediation UI needs
* (`plugin-hono-server/src/current-user-endpoints.ts`).
*/
const ALLOW_ROUTES: readonly (readonly string[])[] = [
['health'],
['ready'],
['discovery'],
['me', 'apps'],
['me', 'localization'],
];

/** Do `segments` start with every segment of `prefix`? */
function startsWithSegments(segments: readonly string[], prefix: readonly string[]): boolean {
if (segments.length < prefix.length) return false;
for (let k = 0; k < prefix.length; k++) if (segments[k] !== prefix[k]) return false;
return true;
}

/** True when `path` is exempt from the auth gate (auth + remediation + health). */
export function isAuthGateAllowlisted(rawPath: string | undefined | null): boolean {
Expand All @@ -71,14 +128,34 @@ export function isAuthGateAllowlisted(rawPath: string | undefined | null): boole
let end = path.length;
while (end > 1 && path.charCodeAt(end - 1) === 47) end--;
path = path.slice(0, end) || '/';
// Any path with an `/auth/` segment is an auth endpoint (covers project-
// scoped mounts like `/api/v1/environments/:env/auth/...`).
if (path.includes('/auth/')) return true;
for (const p of ALLOW_PREFIXES) {
if (path.startsWith(p) || path === p.replace(/\/$/, '')) return true;
}
for (const s of ALLOW_SUFFIXES) {
if (path.endsWith(s)) return true;
// Segment view — `''` entries dropped so `//auth//me` cannot smuggle an
// empty segment past the position tests below.
const segments = path.split('/').filter((s) => s !== '');
for (const base of MOUNT_BASES) {
if (!startsWithSegments(segments, base)) continue;
let i = base.length;
let scoped = false;
// One optional environment scope, and only immediately after the base —
// which is why `/data/environments/x/health` is NOT a scoped `/health`.
if (i + 1 < segments.length && SCOPE_SEGMENTS.includes(segments[i] as string)) {
i += 2;
scoped = true;
}
if (segments[i] === 'auth') {
// `<base>[/<scope>/<id>]/auth/…` — the remediation surface. This is the
// anchored replacement for `path.includes('/auth/')`.
if (i + 1 < segments.length) return true;
// Bare `<base>/auth`, exempt only UNSCOPED: that is exactly what the old
// `ALLOW_PREFIXES` equality branch admitted (`/auth`, `/api/auth`,
// `/api/v1/auth`). The scoped spelling was never exempt and must not
// become so here — this repair only ever removes exemptions.
if (!scoped) return true;
}
for (const route of ALLOW_ROUTES) {
if (segments.length - i === route.length && startsWithSegments(segments.slice(i), route)) {
return true;
}
}
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,11 @@ async function drive(rest: RestServer, path: string, method = 'GET'): Promise<Dr
}

// The allow-list's four reachable path SHAPES, read off `auth-gate.ts` rather
// than invented here: an `ALLOW_PREFIXES` entry, the dispatcher path shape, an
// embedded `/auth/` segment, and an `ALLOW_SUFFIXES` entry.
// than invented here: a REST-mounted `/auth/` route, the dispatcher path shape,
// an environment-scoped `/auth/` route, and an exact bootstrap route.
// (#16839 renamed the constants these were read off — `ALLOW_PREFIXES` and
// `ALLOW_SUFFIXES` became `MOUNT_BASES` + `ALLOW_ROUTES` when the allow-list
// was anchored — but every SHAPE below is still allow-listed, which §0 drives.)
const ALLOWLISTED_PATHS = [
'/api/v1/auth/change-password',
'/auth/two-factor/enable',
Expand Down Expand Up @@ -379,13 +382,28 @@ describe('[#15021] §3 reachability — which MOUNTED REST routes carry an allow
it('READING (not a drive): the gate block sits on the shared identity path, so EVERY route that resolves a context inherits the refusal', () => {
// The census above is about PATTERNS this server mounts. It is not a claim
// about concrete requests: `/api/v1/data/:object` with `object` = `health`
// materializes `/api/v1/data/health`, which `isAuthGateAllowlisted` answers
// `true` for (the `ALLOW_SUFFIXES` rule is a suffix test, not a route test).
// That over-broad direction is #7898's subject, ⛔ not this card's, and it
// is recorded here only so the census is not read as "the allow-list never
// fires on this door".
expect(isAuthGateAllowlisted('/api/v1/data/health')).toBe(true);
// materializes `/api/v1/data/health`, and that concrete path used to be
// ALLOW-LISTED — which is what made the over-broad direction reachable at
// this very door.
//
// ⚠️ RE-AIMED IN PLACE (#16839), per this file's own header instruction.
// The superseded assertion and its reason, verbatim:
//
// // ... which `isAuthGateAllowlisted` answers `true` for (the
// // `ALLOW_SUFFIXES` rule is a suffix test, not a route test).
// // That over-broad direction is #7898's subject, ⛔ not this card's
// expect(isAuthGateAllowlisted('/api/v1/data/health')).toBe(true);
//
// The allow-list is anchored to a mount boundary now, so a record id or an
// object name can no longer spell its way into the exemption. The reading
// this leg exists to record is UNCHANGED — the census must still not be
// read as "the allow-list never fires on this door", and the discovery
// document below is what makes it fire.
expect(isAuthGateAllowlisted('/api/v1/data/health')).toBe(false);
expect(isAuthGateAllowlisted('/api/v1/data/sys_user')).toBe(false);
// ⭐ POSITIVE CONTROL — the predicate did not simply start refusing
// everything: the allow-listed mounted route the census found still is.
expect(isAuthGateAllowlisted('/api/v1/discovery')).toBe(true);
});
});

Expand Down
Loading