From 12f418cc39d9fde6ad737ea1a1ba366b73e3d963 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 08:51:30 +0000 Subject: [PATCH 1/2] 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 Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V --- .../security-anonymous-deny-ordering.md | 27 +++ .../security-anonymous-deny-ordering.test.ts | 195 ++++++++++++++++++ packages/runtime/src/domains/security.ts | 34 ++- 3 files changed, 246 insertions(+), 10 deletions(-) create mode 100644 .changeset/security-anonymous-deny-ordering.md create mode 100644 packages/runtime/src/domains/security-anonymous-deny-ordering.test.ts diff --git a/.changeset/security-anonymous-deny-ordering.md b/.changeset/security-anonymous-deny-ordering.md new file mode 100644 index 0000000000..0f88231ec2 --- /dev/null +++ b/.changeset/security-anonymous-deny-ordering.md @@ -0,0 +1,27 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): consult the anonymous-deny gate before `/security`'s capability answer (#7911) + +On any deployment where the `security` slot is empty or its occupant does not +duck-type `ISecurityService`, an **unauthenticated** caller to +`/api/v1/security/suggested-bindings` got **503 "Security service not +available"** instead of **401 UNAUTHENTICATED** — a capability disclosure +served ahead of this admin surface's own "anonymous is denied +UNCONDITIONALLY" rule (#2567, #3963). + +`handleSecurityRequest` resolved the `security` service and returned the 503 +for an empty/non-duck-typing slot *before* reaching the +`!ec || shouldDenyAnonymous(...)` gate ~20 lines below. `/security` stands on +the same anonymous-deny floor as `/data`, `/meta`, `/actions` and +`/automation` (ADR-0056 D2 → #3963); this was the last of the six dispatcher +domains still ordered the wrong way, after `/ai/**` (#7653, fixed in #7910). + +The gate now runs first and decides once; the `!ec` arm is unchanged +(documented `#4127 batch 3` as behaviour-preserving) so the hoist changes +*when* the decision is made, not *what* it decides. The 503 answer is +unchanged for an authenticated caller against an empty/stubbed slot, and a +serveable slot still works for an authenticated caller and still denies +anonymous. No route-level `auth: false` opt-out exists on this domain, so +there is a single consult site. diff --git a/packages/runtime/src/domains/security-anonymous-deny-ordering.test.ts b/packages/runtime/src/domains/security-anonymous-deny-ordering.test.ts new file mode 100644 index 0000000000..a5ae390bfd --- /dev/null +++ b/packages/runtime/src/domains/security-anonymous-deny-ordering.test.ts @@ -0,0 +1,195 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7911 — the anonymous-deny gate is consulted BEFORE `/security`'s + * capability answer, not after it. + * + * ## The defect + * + * `handleSecurityRequest` resolved the `security` service and returned a + * capability answer — 503 "Security service not available" — for an empty or + * non-duck-typing slot BEFORE it reached the `!ec || shouldDenyAnonymous(...)` + * gate ~20 lines below. So on any deployment where the `security` slot is + * empty or stubbed, an unauthenticated caller to + * `/api/v1/security/suggested-bindings` got a 503 capability disclosure + * instead of the 401 refusal this admin surface's own comment calls + * UNCONDITIONAL (#2567, #3963). `/security` stands on the same anonymous-deny + * floor as `/data`, `/meta`, `/actions` and `/automation` (ADR-0056 D2 → + * #3963) — the sibling inversion on `/ai/**` was #7653, fixed in PR #7910; + * this was the last of the six dispatcher domains still ordered the wrong way. + * + * ## What must NOT change, and why the authenticated half is pinned just as + * hard + * + * A fix that 401s every caller unconditionally would satisfy Group A and + * still be a regression: an AUTHENTICATED caller against an empty/stubbed + * slot must still see the 503 "Security service not available" answer, + * unchanged. That negative pin is what proves this is a hoist (WHEN the gate + * decides) and not a deletion (WHAT it decides) — see `domains/security.ts`'s + * `[#4127 batch 3]` comment on the `!ec` arm, preserved verbatim across the + * move. + * + * No route-level `auth: false` opt-out exists on this domain (unlike `/ai`), + * so there is exactly one consult site and no per-route loop to re-enter. + */ + +import { describe, it, expect } from 'vitest'; +import { + ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE, +} from '@objectstack/core'; + +import { handleSecurityRequest } from './security.js'; +import { apiErrorResponse } from '../error-envelope.js'; +import type { DomainHandlerDeps } from '../domain-handler-registry.js'; +import type { HttpProtocolContext } from '../http-dispatcher.js'; + +// ── contexts ──────────────────────────────────────────────────────────────── +// Two anonymous shapes, both of which occur in the wild: `resolveExecutionContext` +// leaves `executionContext` UNDEFINED when identity resolution throws, and +// writes a `userId`-less record for a resolved-but-sessionless caller. +const anonUnresolved = () => ({ request: { headers: {} } }) as unknown as HttpProtocolContext; +const anonResolved = () => ({ + request: { headers: {} }, + executionContext: { isSystem: false, positions: [], permissions: [], systemPermissions: [] }, +}) as unknown as HttpProtocolContext; +const authed = () => ({ + request: { headers: {} }, + executionContext: { userId: 'usr_1', isSystem: false, positions: [], permissions: [], systemPermissions: [] }, +}) as unknown as HttpProtocolContext; +const system = () => ({ + request: { headers: {} }, + executionContext: { isSystem: true }, +}) as unknown as HttpProtocolContext; + +// ── deps ──────────────────────────────────────────────────────────────────── + +/** + * `error` is the REAL envelope builder the dispatcher wires in + * (`http-dispatcher.ts` → `apiErrorResponse`), not a stub that drops the third + * argument. Without it `details.code` would never be promoted into + * `error.code` and every `code` assertion below would be vacuous — the exact + * way an ADR-0112 envelope test can pass while asserting nothing. + */ +function makeDeps(opts: { + /** `undefined` → the slot is empty; a truthy value with no duck-typed + * methods reproduces the "stubbed occupant" arm of the same `if`. */ + securityService?: any; +} = {}): DomainHandlerDeps { + return { + resolveService: (async (_ctx: HttpProtocolContext, name: string) => + (name === 'security' ? opts.securityService : undefined)) as any, + success: (data: any) => ({ status: 200, body: { success: true, data } }), + error: (message: string, httpStatus = 500, details?: any) => + apiErrorResponse({ message, httpStatus, details }), + errorFromThrown: (e: any, fallbackStatus = 500) => + apiErrorResponse({ message: e?.message ?? 'Unexpected error', httpStatus: e?.status ?? e?.statusCode ?? fallbackStatus }), + } as unknown as DomainHandlerDeps; +} + +function dispatch(deps: DomainHandlerDeps, context: HttpProtocolContext, path: string, method = 'GET') { + return handleSecurityRequest(deps, path, method, {}, {}, context); +} + +/** Assert the ADR-0112 refusal envelope: status AND code, never one alone. */ +function expectAnonymousDenied(result: any) { + expect(result.handled).toBe(true); + expect(result.response.status).toBe(ANONYMOUS_DENY_STATUS); + expect(result.response.status).toBe(401); + expect(result.response.body.success).toBe(false); + expect(result.response.body.error.code).toBe(ANONYMOUS_DENY_CODE); + expect(result.response.body.error.code).toBe('UNAUTHENTICATED'); + expect(result.response.body.error.httpStatus).toBe(401); + expect(result.response.body.error.message).toBe(ANONYMOUS_DENY_MESSAGE); +} + +// ── Group A: the defect ───────────────────────────────────────────────────── + +describe('#7911 A — an empty/stubbed security slot still denies anonymous callers first', () => { + it('GET /security/suggested-bindings, empty slot → 401, not the 503 capability answer', async () => { + const result: any = await dispatch(makeDeps(), anonUnresolved(), '/suggested-bindings'); + expectAnonymousDenied(result); + expect(result.response.status).not.toBe(503); + expect(JSON.stringify(result.response.body)).not.toContain('Security service not available'); + }); + + it('GET /security/suggested-bindings, stubbed occupant (no duck-typed methods) → 401', async () => { + // A truthy occupant with none of the contract's methods takes the same + // `!service || typeof … !== 'function'` exit an empty slot takes. + const result: any = await dispatch(makeDeps({ securityService: {} }), anonUnresolved(), '/suggested-bindings'); + expectAnonymousDenied(result); + }); + + it('denies the resolved-but-sessionless anonymous shape identically', async () => { + expectAnonymousDenied(await dispatch(makeDeps(), anonResolved(), '/suggested-bindings')); + }); + + it('covers the write routes too, not just the list', async () => { + const cases: Array<[string, string]> = [ + ['/suggested-bindings/sug_1/confirm', 'POST'], + ['/suggested-bindings/sug_1/dismiss', 'POST'], + ]; + for (const [path, method] of cases) { + expectAnonymousDenied(await dispatch(makeDeps(), anonUnresolved(), path, method)); + } + }); +}); + +// ── Group B: the honest degradation — LOAD-BEARING, must stay green ───────── + +describe('#7911 B — the 503 capability answer is untouched for an authenticated caller', () => { + it('still 503s an empty slot for an authenticated caller', async () => { + const result: any = await dispatch(makeDeps(), authed(), '/suggested-bindings'); + expect(result.handled).toBe(true); + expect(result.response.status).toBe(503); + expect(result.response.body.error.message).toBe('Security service not available'); + }); + + it('still 503s a stubbed (non-duck-typing) occupant for an authenticated caller', async () => { + const result: any = await dispatch(makeDeps({ securityService: {} }), authed(), '/suggested-bindings'); + expect(result.response.status).toBe(503); + expect(result.response.body.error.message).toBe('Security service not available'); + }); + + it('lets an internal SYSTEM context through to the same 503 degradation', async () => { + // `isSystem` is never settable from the wire; a host dispatching + // internally must not be caught by a caller-facing gate. + const result: any = await dispatch(makeDeps(), system(), '/suggested-bindings'); + expect(result.response.status).toBe(503); + expect(result.response.body.error.message).toBe('Security service not available'); + }); +}); + +// ── Group C: the control — hoisted, not made blanket or left inert ────────── + +describe('#7911 C — a serveable service still works for an authenticated caller and still denies anonymous', () => { + const suggestions = [{ id: 'sug_1', status: 'pending' }]; + const served = { + listAudienceBindingSuggestions: async () => suggestions, + confirmAudienceBindingSuggestion: async (_ec: any, id: string) => ({ id, status: 'confirmed' }), + dismissAudienceBindingSuggestion: async (_ec: any, id: string) => ({ id, status: 'dismissed' }), + }; + + it('still denies anonymous even with a fully serveable service', async () => { + const deps = makeDeps({ securityService: served }); + expectAnonymousDenied(await dispatch(deps, anonUnresolved(), '/suggested-bindings')); + }); + + it('serves an authenticated caller the list', async () => { + const deps = makeDeps({ securityService: served }); + const result: any = await dispatch(deps, authed(), '/suggested-bindings'); + expect(result.handled).toBe(true); + expect(result.response.status).toBe(200); + expect(result.response.body).toEqual({ success: true, data: suggestions }); + }); + + it('serves an authenticated confirm/dismiss', async () => { + const deps = makeDeps({ securityService: served }); + const confirm: any = await dispatch(deps, authed(), '/suggested-bindings/sug_1/confirm', 'POST'); + expect(confirm.response.status).toBe(200); + expect(confirm.response.body.data).toEqual({ id: 'sug_1', status: 'confirmed' }); + + const dismiss: any = await dispatch(deps, authed(), '/suggested-bindings/sug_1/dismiss', 'POST'); + expect(dismiss.response.status).toBe(200); + expect(dismiss.response.body.data).toEqual({ id: 'sug_1', status: 'dismissed' }); + }); +}); diff --git a/packages/runtime/src/domains/security.ts b/packages/runtime/src/domains/security.ts index 8101d8e77e..e8c44d4d9a 100644 --- a/packages/runtime/src/domains/security.ts +++ b/packages/runtime/src/domains/security.ts @@ -64,16 +64,20 @@ export async function handleSecurityRequest( query: any, context: HttpProtocolContext, ): Promise { - // [#4127 batch 3] The `as any` was the only thing between this call and - // `ISecurityService`. The contract was written, `plugin-security` registers - // the slot, and all three methods used below were already declared — the - // slot name simply was not in the ledger, so nothing connected them. - const service = await deps.resolveService(context, 'security', context.environmentId); - if (!service || typeof service.listAudienceBindingSuggestions !== 'function') { - return { handled: true, response: deps.error('Security service not available', 503) }; - } - const ec = context.executionContext; + // [#7911] ANONYMOUS BASELINE — decided here, ahead of the capability probe + // below. This gate used to sit ~20 lines lower, AFTER `resolveService`'s + // "Security service not available" 503, which meant an empty or + // non-duck-typing `security` slot answered an unauthenticated caller with + // a capability disclosure (503) instead of the admin-surface refusal + // (401). `/security` stands on the same anonymous-deny floor as `/data`, + // `/meta`, `/actions` and `/automation` (ADR-0056 D2 → #3963) — see + // `domains/automation.ts`, which already gates ahead of its own + // `capabilityUnavailable` for the same reason, and `domains/ai.ts` + // (#7653/#7910), the sibling inversion this hoist mirrors. No route-level + // `auth: false` opt-out exists on this domain, so there is a single + // consult site and no loop to re-enter. + // // Admin surface — anonymous is denied UNCONDITIONALLY (#2567, #3963): // even before the opt-out was retired this seam never honoured it, so an // anonymous caller could never list or confirm audience bindings. Shares @@ -88,7 +92,8 @@ export async function handleSecurityRequest( // SecurityContext, …)`, non-optional precisely because a WRITE needs a // caller identity, unlike the optional one on the read — could not be seen // to hold even though it did. Checking `ec` directly makes the invariant - // legible to the compiler and to the next reader. + // legible to the compiler and to the next reader. The hoist changes WHEN + // this decides, not WHAT — the arm itself is unchanged. if (!ec || shouldDenyAnonymous({ userId: ec.userId, isSystem: ec.isSystem })) { return { handled: true, @@ -96,6 +101,15 @@ export async function handleSecurityRequest( }; } + // [#4127 batch 3] The `as any` was the only thing between this call and + // `ISecurityService`. The contract was written, `plugin-security` registers + // the slot, and all three methods used below were already declared — the + // slot name simply was not in the ledger, so nothing connected them. + const service = await deps.resolveService(context, 'security', context.environmentId); + if (!service || typeof service.listAudienceBindingSuggestions !== 'function') { + return { handled: true, response: deps.error('Security service not available', 503) }; + } + const m = method.toUpperCase(); // split+filter drops leading/trailing/duplicate slashes without a // regex over request-controlled input (CodeQL js/polynomial-redos). From 42629a7a6aa0c7d1a52cec6bdcea91ab40bb99f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 09:09:28 +0000 Subject: [PATCH 2/2] test(runtime): repair the pre-existing 503 pin for the #7911 hoist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V --- .../src/domain-handler-registry.test.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/runtime/src/domain-handler-registry.test.ts b/packages/runtime/src/domain-handler-registry.test.ts index e471f5a812..471fda5b6c 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -202,8 +202,22 @@ describe('HttpDispatcher extracted domains (PR-2)', () => { expect(notification.markRead).toHaveBeenCalledWith('u1', ['n1']); }); - it('/security responds 503 when no security service is wired (legacy in-handler semantics)', async () => { - const result = await makeDispatcher().dispatch('GET', '/security/suggested-bindings', undefined, {}, {} as any); + 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 () => { + // Direct delegate call for the same reason as the notifications case + // above: dispatch() re-resolves identity from the (mock, auth-less) + // kernel and would overwrite the seeded executionContext with an + // anonymous one, and an anonymous caller no longer reaches this probe + // at all post-#7911 (see the anonymous-denial test right below). This + // test's job is narrower than that: prove the in-handler 503 path + // (`!service || typeof service.listAudienceBindingSuggestions !== + // 'function'`) still exists once an authenticated caller has cleared + // the gate -- the negative pin proving #7911 was a hoist, not a + // deletion. Before #7911 the 503 sat AHEAD of the gate and this test + // reached it anonymously by accident of the harness ("legacy + // in-handler semantics"); now it sits after the gate, so the test + // authenticates deliberately instead of relying on that accident. + const context: any = { executionContext: { userId: 'admin-1' } }; + const result = await makeDispatcher().handleSecurity('/suggested-bindings', 'GET', undefined, {}, context); expect(result.handled).toBe(true); expect(result.response?.status).toBe(503); });