Skip to content

Commit 130a873

Browse files
committed
fix(rest): exempt a request from the ADR-0069 gate only when it carries a real path (#7432)
`isAuthGateAllowlisted(undefined)` returns `true` — it treats "no path" as allow-listed. REST's `enforceAuth` passed `req.path` straight through, so a request whose `path` was absent or an empty string read as allow-listed on every route and the ADR-0069 gate (expired password / enforced MFA) did not fire for a session policy says must be blocked. `enforceAuth` now applies the guard the sibling seam already carries (`shouldDenyAnonymous`, core/src/security/anonymous-deny.ts:122): a path exempts a gated session only when it is a non-empty string the allow-list actually accepts. Nothing shipped was bypassable — the hono adapter populates `path` at all three request-construction sites. What is fixed is the direction of the default: the guard was carried by the caller's discipline on a fail-OPEN seam, so a new transport adapter or any synthetic request disabled a security gate by omission with no test going red. Tests pin the consequence, not the guard expression: absent `path` and empty-string `path` against a gated session are blocked (both go red with the guard removed), while every allow-listed path shape still passes, an ordinary gated request still blocks, an ungated pathless session is untouched, and OPTIONS preflight stays exempt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012J7U4wwUHYyZ7EgsUHuJA7
1 parent 31fb03d commit 130a873

3 files changed

Lines changed: 104 additions & 1 deletion

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
fix(rest): exempt a request from the ADR-0069 auth gate only when it carries a REAL path (#7432)
6+
7+
`isAuthGateAllowlisted(undefined)` returns `true` — it treats "no path" as
8+
allow-listed. REST's `enforceAuth` passed `req.path` straight through, so a
9+
request whose `path` was absent or an empty string read as allow-listed on
10+
**every** route and the ADR-0069 gate (expired password / enforced MFA) did not
11+
fire for a session policy says must be blocked.
12+
13+
`enforceAuth` now applies the guard its sibling seam already carries
14+
(`shouldDenyAnonymous`, `@objectstack/core/security`): a path exempts a gated
15+
session only when it is a non-empty string that the allow-list actually accepts.
16+
17+
**Nothing shipped was bypassable.** The hono adapter populates `path` at all
18+
three request-construction sites, so no live transport reached this seam without
19+
one. What is fixed is the direction of the default: the guard was carried by the
20+
**caller's** discipline on a fail-OPEN seam, so a new transport adapter — or any
21+
synthetic request — disabled a security gate by omission with no test going red.
22+
23+
No behaviour change for any request that carries a path: allow-listed paths
24+
(auth, remediation, health, UI-bootstrap reads) still pass, protected paths still
25+
block, and `OPTIONS` preflight is still exempt. A gated session with no path is
26+
now blocked rather than waved through.

packages/rest/src/rest-auth-gate.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,68 @@ describe('RestServer.enforceAuth — ADR-0069 auth-policy gate', () => {
6060
expect(r.blocked).toBe(false);
6161
});
6262
});
63+
64+
// #7432 — a request carrying NO path must not be exempt from the gate.
65+
//
66+
// `isAuthGateAllowlisted(undefined)` returns `true` (it treats "no path" as
67+
// allow-listed), so passing `req.path` through raw made a pathless request read
68+
// as allow-listed on every route and the gate silently did not fire. These pin
69+
// the CONSEQUENCE — blocked / not blocked — rather than the guard expression, so
70+
// they stay meaningful if the guard is rewritten. Removing the guard turns the
71+
// first two red; they are the reverse-verification target.
72+
describe('#7432 — enforceAuth exempts only a REAL path from the ADR-0069 gate', () => {
73+
const gate = (req: any, context: any) => {
74+
const { res, state } = makeRes();
75+
const blocked = (rest as any).enforceAuth(req, res, context);
76+
return { blocked, state };
77+
};
78+
const GATED = { userId: 'u1', authGate: { code: 'PASSWORD_EXPIRED', message: 'change it' } };
79+
80+
it('blocks a gated session when `path` is ABSENT', () => {
81+
const r = gate({ method: 'GET' }, GATED);
82+
expect(r.blocked).toBe(true);
83+
expect(r.state.status).toBe(403);
84+
expect(r.state.body.error.code).toBe('PASSWORD_EXPIRED');
85+
});
86+
87+
it('blocks a gated session when `path` is the EMPTY STRING', () => {
88+
const r = gate({ method: 'GET', path: '' }, GATED);
89+
expect(r.blocked).toBe(true);
90+
expect(r.state.status).toBe(403);
91+
expect(r.state.body.error.code).toBe('PASSWORD_EXPIRED');
92+
});
93+
94+
// The load-bearing half: a guard that fixes the bypass by blocking everything
95+
// is not a fix. The allow-list must still allow, and an ungated session must
96+
// still be ungated — including on the pathless requests above, where the gate
97+
// branch is now reached but has no gate to apply.
98+
it('still lets a gated session reach every allow-listed path shape', () => {
99+
for (const path of [
100+
'/api/v1/auth/change-password', // ALLOW_PREFIXES
101+
'/auth/two-factor/enable', // dispatcher path shape
102+
'/api/v1/environments/env1/auth/sign-out', // embedded `/auth/` segment
103+
'/api/v1/health', // ALLOW_SUFFIXES
104+
'/api/v1/me/apps', // UI-bootstrap read
105+
]) {
106+
expect(gate({ method: 'GET', path }, GATED).blocked).toBe(false);
107+
}
108+
});
109+
110+
it('still blocks an ordinary gated request on a protected path', () => {
111+
const r = gate({ method: 'GET', path: '/api/v1/data/sys_user' }, GATED);
112+
expect(r.blocked).toBe(true);
113+
expect(r.state.status).toBe(403);
114+
});
115+
116+
it('does not block an UNGATED session that happens to carry no path', () => {
117+
// The guard tightens the exemption, not the gate: with no `authGate` there
118+
// is nothing to enforce, so a pathless authenticated request passes exactly
119+
// as it did before. (`shouldDenyAnonymous` sees `userId`, so no 401 either.)
120+
expect(gate({ method: 'GET' }, { userId: 'u1' }).blocked).toBe(false);
121+
expect(gate({ method: 'GET', path: '' }, { userId: 'u1' }).blocked).toBe(false);
122+
});
123+
124+
it('still ignores OPTIONS preflight when gated and pathless', () => {
125+
expect(gate({ method: 'OPTIONS' }, GATED).blocked).toBe(false);
126+
});
127+
});

packages/rest/src/rest-server.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2432,7 +2432,19 @@ export class RestServer {
24322432
// blocked from protected resources, while the core allow-list keeps auth
24332433
// + remediation reachable. Runs before the anonymous check.
24342434
const gate = context?.authGate;
2435-
if (gate && req?.method !== 'OPTIONS' && !isAuthGateAllowlisted(req?.path)) {
2435+
// Exemption requires a REAL, non-empty path — mirrors the sibling seam
2436+
// (`shouldDenyAnonymous`, core/src/security/anonymous-deny.ts:122).
2437+
//
2438+
// ⚠️ `isAuthGateAllowlisted(undefined)` returns `true` (it treats "no
2439+
// path" as allow-listed). Passed the raw value, a request whose `path`
2440+
// is absent or empty read as allow-listed on EVERY route, so the gate
2441+
// did not fire for a session policy says must be blocked — fail-OPEN by
2442+
// omission. No shipped transport reaches here without a `path` (the
2443+
// hono adapter sets it at all three request-construction sites), so this
2444+
// is the default being made safe, not a live bypass being closed (#7432).
2445+
const pathExempt =
2446+
typeof req?.path === 'string' && req.path.length > 0 && isAuthGateAllowlisted(req.path);
2447+
if (gate && req?.method !== 'OPTIONS' && !pathExempt) {
24362448
res.status(403).json({ error: { code: gate.code, message: gate.message } });
24372449
return true;
24382450
}

0 commit comments

Comments
 (0)