Skip to content

Commit 86d2e5e

Browse files
hotlongclaude
andauthored
fix(runtime): consult anonymous-deny gate before /security's 503 (#7958)
* fix(runtime): consult anonymous-deny gate before /security's 503 (#7911) handleSecurityRequest resolved the security service and returned 503 "Security service not available" for an empty/non-duck-typing slot BEFORE reaching the !ec || shouldDenyAnonymous(...) gate ~20 lines below, so an unauthenticated caller to /api/v1/security/suggested-bindings got a capability disclosure (503) instead of the admin-surface refusal (401 UNAUTHENTICATED) this handler's own comment calls unconditional (#2567, #3963). Straight hoist, mirroring the #7653/#7910 fix on domains/ai.ts: the gate now runs first and decides once. The !ec arm is unchanged (documented #4127 batch 3 as behaviour-preserving) so this changes WHEN the decision is made, not WHAT it decides. The 503 answer stays unchanged for an authenticated caller against an empty/stubbed slot, and a serveable slot still works authenticated and still denies anonymous. No route-level auth: false opt-out exists on this domain, so there is a single consult site. Adds packages/runtime/src/domains/security-anonymous-deny-ordering.test.ts (10 cases, mirroring ai-anonymous-deny-ordering.test.ts's group shape) and a patch changeset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V * test(runtime): repair the pre-existing 503 pin for the #7911 hoist CI caught this correctly: domain-handler-registry.test.ts's "/security responds 503 when no security service is wired (legacy in-handler semantics)" reached its 503 via dispatch(), which re-resolves identity from the mock kernel and answers anonymously — so after #7911's hoist the anonymous-deny gate now intercepts it first and returns 401. The test's NAME says its job is to prove the 503 comes from INSIDE the handler, not to assert anonymous-vs-authenticated ordering (the next test down already covers anonymous denial with a wired service). Flipping the assertion to 401 would have destroyed that purpose and left nothing pinning the in-handler 503 path #7911's report calls out as the required negative control. Repaired using the pattern already established in this file at :188-190 for /notifications: call the public handleSecurity() delegate directly with a seeded AUTHENTICATED executionContext, bypassing dispatch()'s identity re-resolution, so the test proves what its (renamed) name claims — no service wired => 503 from inside the handler, once the gate has been cleared — without also asserting the ordering #7911 just fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a5d3aa1 commit 86d2e5e

4 files changed

Lines changed: 262 additions & 12 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): consult the anonymous-deny gate before `/security`'s capability answer (#7911)
6+
7+
On any deployment where the `security` slot is empty or its occupant does not
8+
duck-type `ISecurityService`, an **unauthenticated** caller to
9+
`/api/v1/security/suggested-bindings` got **503 "Security service not
10+
available"** instead of **401 UNAUTHENTICATED** — a capability disclosure
11+
served ahead of this admin surface's own "anonymous is denied
12+
UNCONDITIONALLY" rule (#2567, #3963).
13+
14+
`handleSecurityRequest` resolved the `security` service and returned the 503
15+
for an empty/non-duck-typing slot *before* reaching the
16+
`!ec || shouldDenyAnonymous(...)` gate ~20 lines below. `/security` stands on
17+
the same anonymous-deny floor as `/data`, `/meta`, `/actions` and
18+
`/automation` (ADR-0056 D2 → #3963); this was the last of the six dispatcher
19+
domains still ordered the wrong way, after `/ai/**` (#7653, fixed in #7910).
20+
21+
The gate now runs first and decides once; the `!ec` arm is unchanged
22+
(documented `#4127 batch 3` as behaviour-preserving) so the hoist changes
23+
*when* the decision is made, not *what* it decides. The 503 answer is
24+
unchanged for an authenticated caller against an empty/stubbed slot, and a
25+
serveable slot still works for an authenticated caller and still denies
26+
anonymous. No route-level `auth: false` opt-out exists on this domain, so
27+
there is a single consult site.

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,22 @@ describe('HttpDispatcher extracted domains (PR-2)', () => {
202202
expect(notification.markRead).toHaveBeenCalledWith('u1', ['n1']);
203203
});
204204

205-
it('/security responds 503 when no security service is wired (legacy in-handler semantics)', async () => {
206-
const result = await makeDispatcher().dispatch('GET', '/security/suggested-bindings', undefined, {}, {} as any);
205+
it('/security still answers 503 from inside the handler when no service is wired, for an authenticated caller (#7911: the anonymous-deny gate now runs BEFORE this, not after)', async () => {
206+
// Direct delegate call for the same reason as the notifications case
207+
// above: dispatch() re-resolves identity from the (mock, auth-less)
208+
// kernel and would overwrite the seeded executionContext with an
209+
// anonymous one, and an anonymous caller no longer reaches this probe
210+
// at all post-#7911 (see the anonymous-denial test right below). This
211+
// test's job is narrower than that: prove the in-handler 503 path
212+
// (`!service || typeof service.listAudienceBindingSuggestions !==
213+
// 'function'`) still exists once an authenticated caller has cleared
214+
// the gate -- the negative pin proving #7911 was a hoist, not a
215+
// deletion. Before #7911 the 503 sat AHEAD of the gate and this test
216+
// reached it anonymously by accident of the harness ("legacy
217+
// in-handler semantics"); now it sits after the gate, so the test
218+
// authenticates deliberately instead of relying on that accident.
219+
const context: any = { executionContext: { userId: 'admin-1' } };
220+
const result = await makeDispatcher().handleSecurity('/suggested-bindings', 'GET', undefined, {}, context);
207221
expect(result.handled).toBe(true);
208222
expect(result.response?.status).toBe(503);
209223
});
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #7911 — the anonymous-deny gate is consulted BEFORE `/security`'s
5+
* capability answer, not after it.
6+
*
7+
* ## The defect
8+
*
9+
* `handleSecurityRequest` resolved the `security` service and returned a
10+
* capability answer — 503 "Security service not available" — for an empty or
11+
* non-duck-typing slot BEFORE it reached the `!ec || shouldDenyAnonymous(...)`
12+
* gate ~20 lines below. So on any deployment where the `security` slot is
13+
* empty or stubbed, an unauthenticated caller to
14+
* `/api/v1/security/suggested-bindings` got a 503 capability disclosure
15+
* instead of the 401 refusal this admin surface's own comment calls
16+
* UNCONDITIONAL (#2567, #3963). `/security` stands on the same anonymous-deny
17+
* floor as `/data`, `/meta`, `/actions` and `/automation` (ADR-0056 D2 →
18+
* #3963) — the sibling inversion on `/ai/**` was #7653, fixed in PR #7910;
19+
* this was the last of the six dispatcher domains still ordered the wrong way.
20+
*
21+
* ## What must NOT change, and why the authenticated half is pinned just as
22+
* hard
23+
*
24+
* A fix that 401s every caller unconditionally would satisfy Group A and
25+
* still be a regression: an AUTHENTICATED caller against an empty/stubbed
26+
* slot must still see the 503 "Security service not available" answer,
27+
* unchanged. That negative pin is what proves this is a hoist (WHEN the gate
28+
* decides) and not a deletion (WHAT it decides) — see `domains/security.ts`'s
29+
* `[#4127 batch 3]` comment on the `!ec` arm, preserved verbatim across the
30+
* move.
31+
*
32+
* No route-level `auth: false` opt-out exists on this domain (unlike `/ai`),
33+
* so there is exactly one consult site and no per-route loop to re-enter.
34+
*/
35+
36+
import { describe, it, expect } from 'vitest';
37+
import {
38+
ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE,
39+
} from '@objectstack/core';
40+
41+
import { handleSecurityRequest } from './security.js';
42+
import { apiErrorResponse } from '../error-envelope.js';
43+
import type { DomainHandlerDeps } from '../domain-handler-registry.js';
44+
import type { HttpProtocolContext } from '../http-dispatcher.js';
45+
46+
// ── contexts ────────────────────────────────────────────────────────────────
47+
// Two anonymous shapes, both of which occur in the wild: `resolveExecutionContext`
48+
// leaves `executionContext` UNDEFINED when identity resolution throws, and
49+
// writes a `userId`-less record for a resolved-but-sessionless caller.
50+
const anonUnresolved = () => ({ request: { headers: {} } }) as unknown as HttpProtocolContext;
51+
const anonResolved = () => ({
52+
request: { headers: {} },
53+
executionContext: { isSystem: false, positions: [], permissions: [], systemPermissions: [] },
54+
}) as unknown as HttpProtocolContext;
55+
const authed = () => ({
56+
request: { headers: {} },
57+
executionContext: { userId: 'usr_1', isSystem: false, positions: [], permissions: [], systemPermissions: [] },
58+
}) as unknown as HttpProtocolContext;
59+
const system = () => ({
60+
request: { headers: {} },
61+
executionContext: { isSystem: true },
62+
}) as unknown as HttpProtocolContext;
63+
64+
// ── deps ────────────────────────────────────────────────────────────────────
65+
66+
/**
67+
* `error` is the REAL envelope builder the dispatcher wires in
68+
* (`http-dispatcher.ts` → `apiErrorResponse`), not a stub that drops the third
69+
* argument. Without it `details.code` would never be promoted into
70+
* `error.code` and every `code` assertion below would be vacuous — the exact
71+
* way an ADR-0112 envelope test can pass while asserting nothing.
72+
*/
73+
function makeDeps(opts: {
74+
/** `undefined` → the slot is empty; a truthy value with no duck-typed
75+
* methods reproduces the "stubbed occupant" arm of the same `if`. */
76+
securityService?: any;
77+
} = {}): DomainHandlerDeps {
78+
return {
79+
resolveService: (async (_ctx: HttpProtocolContext, name: string) =>
80+
(name === 'security' ? opts.securityService : undefined)) as any,
81+
success: (data: any) => ({ status: 200, body: { success: true, data } }),
82+
error: (message: string, httpStatus = 500, details?: any) =>
83+
apiErrorResponse({ message, httpStatus, details }),
84+
errorFromThrown: (e: any, fallbackStatus = 500) =>
85+
apiErrorResponse({ message: e?.message ?? 'Unexpected error', httpStatus: e?.status ?? e?.statusCode ?? fallbackStatus }),
86+
} as unknown as DomainHandlerDeps;
87+
}
88+
89+
function dispatch(deps: DomainHandlerDeps, context: HttpProtocolContext, path: string, method = 'GET') {
90+
return handleSecurityRequest(deps, path, method, {}, {}, context);
91+
}
92+
93+
/** Assert the ADR-0112 refusal envelope: status AND code, never one alone. */
94+
function expectAnonymousDenied(result: any) {
95+
expect(result.handled).toBe(true);
96+
expect(result.response.status).toBe(ANONYMOUS_DENY_STATUS);
97+
expect(result.response.status).toBe(401);
98+
expect(result.response.body.success).toBe(false);
99+
expect(result.response.body.error.code).toBe(ANONYMOUS_DENY_CODE);
100+
expect(result.response.body.error.code).toBe('UNAUTHENTICATED');
101+
expect(result.response.body.error.httpStatus).toBe(401);
102+
expect(result.response.body.error.message).toBe(ANONYMOUS_DENY_MESSAGE);
103+
}
104+
105+
// ── Group A: the defect ─────────────────────────────────────────────────────
106+
107+
describe('#7911 A — an empty/stubbed security slot still denies anonymous callers first', () => {
108+
it('GET /security/suggested-bindings, empty slot → 401, not the 503 capability answer', async () => {
109+
const result: any = await dispatch(makeDeps(), anonUnresolved(), '/suggested-bindings');
110+
expectAnonymousDenied(result);
111+
expect(result.response.status).not.toBe(503);
112+
expect(JSON.stringify(result.response.body)).not.toContain('Security service not available');
113+
});
114+
115+
it('GET /security/suggested-bindings, stubbed occupant (no duck-typed methods) → 401', async () => {
116+
// A truthy occupant with none of the contract's methods takes the same
117+
// `!service || typeof … !== 'function'` exit an empty slot takes.
118+
const result: any = await dispatch(makeDeps({ securityService: {} }), anonUnresolved(), '/suggested-bindings');
119+
expectAnonymousDenied(result);
120+
});
121+
122+
it('denies the resolved-but-sessionless anonymous shape identically', async () => {
123+
expectAnonymousDenied(await dispatch(makeDeps(), anonResolved(), '/suggested-bindings'));
124+
});
125+
126+
it('covers the write routes too, not just the list', async () => {
127+
const cases: Array<[string, string]> = [
128+
['/suggested-bindings/sug_1/confirm', 'POST'],
129+
['/suggested-bindings/sug_1/dismiss', 'POST'],
130+
];
131+
for (const [path, method] of cases) {
132+
expectAnonymousDenied(await dispatch(makeDeps(), anonUnresolved(), path, method));
133+
}
134+
});
135+
});
136+
137+
// ── Group B: the honest degradation — LOAD-BEARING, must stay green ─────────
138+
139+
describe('#7911 B — the 503 capability answer is untouched for an authenticated caller', () => {
140+
it('still 503s an empty slot for an authenticated caller', async () => {
141+
const result: any = await dispatch(makeDeps(), authed(), '/suggested-bindings');
142+
expect(result.handled).toBe(true);
143+
expect(result.response.status).toBe(503);
144+
expect(result.response.body.error.message).toBe('Security service not available');
145+
});
146+
147+
it('still 503s a stubbed (non-duck-typing) occupant for an authenticated caller', async () => {
148+
const result: any = await dispatch(makeDeps({ securityService: {} }), authed(), '/suggested-bindings');
149+
expect(result.response.status).toBe(503);
150+
expect(result.response.body.error.message).toBe('Security service not available');
151+
});
152+
153+
it('lets an internal SYSTEM context through to the same 503 degradation', async () => {
154+
// `isSystem` is never settable from the wire; a host dispatching
155+
// internally must not be caught by a caller-facing gate.
156+
const result: any = await dispatch(makeDeps(), system(), '/suggested-bindings');
157+
expect(result.response.status).toBe(503);
158+
expect(result.response.body.error.message).toBe('Security service not available');
159+
});
160+
});
161+
162+
// ── Group C: the control — hoisted, not made blanket or left inert ──────────
163+
164+
describe('#7911 C — a serveable service still works for an authenticated caller and still denies anonymous', () => {
165+
const suggestions = [{ id: 'sug_1', status: 'pending' }];
166+
const served = {
167+
listAudienceBindingSuggestions: async () => suggestions,
168+
confirmAudienceBindingSuggestion: async (_ec: any, id: string) => ({ id, status: 'confirmed' }),
169+
dismissAudienceBindingSuggestion: async (_ec: any, id: string) => ({ id, status: 'dismissed' }),
170+
};
171+
172+
it('still denies anonymous even with a fully serveable service', async () => {
173+
const deps = makeDeps({ securityService: served });
174+
expectAnonymousDenied(await dispatch(deps, anonUnresolved(), '/suggested-bindings'));
175+
});
176+
177+
it('serves an authenticated caller the list', async () => {
178+
const deps = makeDeps({ securityService: served });
179+
const result: any = await dispatch(deps, authed(), '/suggested-bindings');
180+
expect(result.handled).toBe(true);
181+
expect(result.response.status).toBe(200);
182+
expect(result.response.body).toEqual({ success: true, data: suggestions });
183+
});
184+
185+
it('serves an authenticated confirm/dismiss', async () => {
186+
const deps = makeDeps({ securityService: served });
187+
const confirm: any = await dispatch(deps, authed(), '/suggested-bindings/sug_1/confirm', 'POST');
188+
expect(confirm.response.status).toBe(200);
189+
expect(confirm.response.body.data).toEqual({ id: 'sug_1', status: 'confirmed' });
190+
191+
const dismiss: any = await dispatch(deps, authed(), '/suggested-bindings/sug_1/dismiss', 'POST');
192+
expect(dismiss.response.status).toBe(200);
193+
expect(dismiss.response.body.data).toEqual({ id: 'sug_1', status: 'dismissed' });
194+
});
195+
});

packages/runtime/src/domains/security.ts

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,16 +64,20 @@ export async function handleSecurityRequest(
6464
query: any,
6565
context: HttpProtocolContext,
6666
): Promise<HttpDispatcherResult> {
67-
// [#4127 batch 3] The `as any` was the only thing between this call and
68-
// `ISecurityService`. The contract was written, `plugin-security` registers
69-
// the slot, and all three methods used below were already declared — the
70-
// slot name simply was not in the ledger, so nothing connected them.
71-
const service = await deps.resolveService(context, 'security', context.environmentId);
72-
if (!service || typeof service.listAudienceBindingSuggestions !== 'function') {
73-
return { handled: true, response: deps.error('Security service not available', 503) };
74-
}
75-
7667
const ec = context.executionContext;
68+
// [#7911] ANONYMOUS BASELINE — decided here, ahead of the capability probe
69+
// below. This gate used to sit ~20 lines lower, AFTER `resolveService`'s
70+
// "Security service not available" 503, which meant an empty or
71+
// non-duck-typing `security` slot answered an unauthenticated caller with
72+
// a capability disclosure (503) instead of the admin-surface refusal
73+
// (401). `/security` stands on the same anonymous-deny floor as `/data`,
74+
// `/meta`, `/actions` and `/automation` (ADR-0056 D2 → #3963) — see
75+
// `domains/automation.ts`, which already gates ahead of its own
76+
// `capabilityUnavailable` for the same reason, and `domains/ai.ts`
77+
// (#7653/#7910), the sibling inversion this hoist mirrors. No route-level
78+
// `auth: false` opt-out exists on this domain, so there is a single
79+
// consult site and no loop to re-enter.
80+
//
7781
// Admin surface — anonymous is denied UNCONDITIONALLY (#2567, #3963):
7882
// even before the opt-out was retired this seam never honoured it, so an
7983
// anonymous caller could never list or confirm audience bindings. Shares
@@ -88,14 +92,24 @@ export async function handleSecurityRequest(
8892
// SecurityContext, …)`, non-optional precisely because a WRITE needs a
8993
// caller identity, unlike the optional one on the read — could not be seen
9094
// to hold even though it did. Checking `ec` directly makes the invariant
91-
// legible to the compiler and to the next reader.
95+
// legible to the compiler and to the next reader. The hoist changes WHEN
96+
// this decides, not WHAT — the arm itself is unchanged.
9297
if (!ec || shouldDenyAnonymous({ userId: ec.userId, isSystem: ec.isSystem })) {
9398
return {
9499
handled: true,
95100
response: deps.error(ANONYMOUS_DENY_MESSAGE, ANONYMOUS_DENY_STATUS, { code: ANONYMOUS_DENY_CODE }),
96101
};
97102
}
98103

104+
// [#4127 batch 3] The `as any` was the only thing between this call and
105+
// `ISecurityService`. The contract was written, `plugin-security` registers
106+
// the slot, and all three methods used below were already declared — the
107+
// slot name simply was not in the ledger, so nothing connected them.
108+
const service = await deps.resolveService(context, 'security', context.environmentId);
109+
if (!service || typeof service.listAudienceBindingSuggestions !== 'function') {
110+
return { handled: true, response: deps.error('Security service not available', 503) };
111+
}
112+
99113
const m = method.toUpperCase();
100114
// split+filter drops leading/trailing/duplicate slashes without a
101115
// regex over request-controlled input (CodeQL js/polynomial-redos).

0 commit comments

Comments
 (0)