From 6f75ea3100b4c17299f5a78e3f437d95b08f54f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 20:55:21 +0000 Subject: [PATCH] test(dogfood): cover /actions and /automation in the anonymous-deny proof artifact (#5570) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `authz-conformance.matrix.ts` names `showcase-anonymous-deny-surfaces.dogfood.test.ts` as the proof artifact for #2567's "anonymous posture is uniform across HTTP surfaces" claim, but the suite drove only `/data` and `/meta`. #5519 found the claim false on exactly the two surfaces it did not drive — the dispatcher-mounted `/actions` and `/automation` — and the artifact was silent throughout. PR #5569 built the gate in `packages/runtime`; this is the evidence half. - six new anonymous cases on the shared showcase boot: POST a `script` action, POST `/automation/:name/trigger`, GET `/automation`, DELETE `/automation/:name` (all 401), plus the two authenticated contrasts. - one case pins that all four surfaces answer the same code and message, reading each family in its own declared envelope rather than through a tolerant `??` chain. - two matrix rows (`anonymous-deny-actions`, `anonymous-deny-automation`) with their `covers` keys, ratchet probes for both gates, and a `(h)` bites case, so deleting either gate fails CI as STALE covers. No `packages/runtime` change: this adds proof, not defence. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016FNvXhtSdnEGEfLEsMmvxh --- .../dogfood/test/authz-conformance.matrix.ts | 17 ++ .../qa/dogfood/test/authz-conformance.test.ts | 44 ++++++ ...se-anonymous-deny-surfaces.dogfood.test.ts | 149 ++++++++++++++++++ 3 files changed, 210 insertions(+) diff --git a/packages/qa/dogfood/test/authz-conformance.matrix.ts b/packages/qa/dogfood/test/authz-conformance.matrix.ts index d3a42b9883..82d34f142a 100644 --- a/packages/qa/dogfood/test/authz-conformance.matrix.ts +++ b/packages/qa/dogfood/test/authz-conformance.matrix.ts @@ -89,6 +89,23 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ enforcement: 'rest/rest-server.ts registerMetadataEndpoints guarded registrar (enforceAuth → shouldDenyAnonymous) — every /meta route inherits the gate; runtime/http-dispatcher.ts handleMetadata mirrors it for the dispatcher metadata catch-all', proof: 'showcase-anonymous-deny-surfaces.dogfood.test.ts', covers: ['meta:rest-server.ts:registerMetadataEndpoints', 'meta:http-dispatcher.ts:handleMetadata'] }, + // #5519 — the two DISPATCHER-mounted execution surfaces. `@objectstack/rest` + // gated `/data` and `/meta`; these routes are mounted by a SECOND + // registration path (dispatcher-plugin.ts, straight onto the host + // IHttpServer) and inherited none of it, so the "#2567 uniform posture" + // claim above was false on them until PR #5569. The proof artifact was + // silent too — #5570 is the evidence half, and these two rows are what make + // the gate's removal fail CI instead of review. + { id: 'anonymous-deny-actions', summary: 'anonymous-deny on the business-action dispatch surface (#2567 surface 2 / #5519)', state: 'enforced', + enforcement: 'runtime/domains/actions.ts handleActionsRequest — shouldDenyAnonymous as the handler\'s FIRST statement, ahead of the ADR-0066 D4 requiredPermissions gate and the ADR-0104 param contract; those keep their semantics and simply run after the auth baseline, so an anonymous caller never reaches action dispatch and never learns the route\'s shape', + proof: 'showcase-anonymous-deny-surfaces.dogfood.test.ts', + covers: ['actions:domains/actions.ts:anonymous-gate'], + note: 'A `type: \'script\'` action body runs `isSystem: true` (elevated), so an ungated POST was an anonymous privilege-escalating WRITE, not merely an information leak — #5519 measured `POST /actions/showcase_task/showcase_mark_done/:id` answering 200 with the update applied. Internal dispatch is unaffected: this handler is a pure HTTP seam (the MCP `run_action` bridge enters through action-execution.invokeBusinessAction, declarative endpoints through the transport fallback seam with their own `authRequired` gate), so `authRequired: false` public endpoints stay public.' }, + { id: 'anonymous-deny-automation', summary: 'anonymous-deny on the automation/flow surface (#2567 surface 3 / #5519)', state: 'enforced', + enforcement: 'runtime/domains/automation.ts handleAutomationRequest — shouldDenyAnonymous DOMAIN-WIDE at the top, and deliberately BEFORE the isServiceServeable probe so the 401/501 difference cannot be used to fingerprint whether a deployment mounts automation', + proof: 'showcase-anonymous-deny-surfaces.dogfood.test.ts', + covers: ['automation:domains/automation.ts:anonymous-gate'], + note: 'Ungated, an anonymous caller could start real flow runs (`POST /:name/trigger`), read the full flow inventory (`GET /automation`), and DEREGISTER a registered flow (`DELETE /:name` → `{deleted:true}`) — the destructive one, which #5519 did not originally record. Gating the DOMAIN rather than each route is what keeps a newly added automation route from arriving ungated. Engine-internal triggers (record-change, schedule) never speak HTTP and are untouched.' }, // ── #2992 / ADR-0096 D4 — latent execution surfaces (pre-wiring identity // admission). Neither surface is reachable by a client today; these rows diff --git a/packages/qa/dogfood/test/authz-conformance.test.ts b/packages/qa/dogfood/test/authz-conformance.test.ts index 6854101281..a29db19a86 100644 --- a/packages/qa/dogfood/test/authz-conformance.test.ts +++ b/packages/qa/dogfood/test/authz-conformance.test.ts @@ -46,6 +46,25 @@ const PROBES: ReadonlyArray<{ file: string; re: RegExp; key: (m: RegExpExecArray re: /async\s+(handleMetadata)\s*\(/g, key: (m) => `meta:http-dispatcher.ts:${m[1]}`, }, + // ── #5519 — the two dispatcher-mounted execution surfaces ────────────── + // These are GATE pins, in the shape the MCP rows below already use: the key + // exists only while the domain handler still consults `shouldDenyAnonymous`. + // Delete the gate (the #5519 regression, in either domain) and the key + // vanishes → the covering row goes STALE → red CI. `/actions` and + // `/automation` are mounted by dispatcher-plugin.ts, a separate registration + // path from the `@objectstack/rest` one that gates `/data` and `/meta`, which + // is exactly why they diverged unnoticed. + { + file: 'packages/runtime/src/domains/actions.ts', + re: /shouldDenyAnonymous\s*\(/g, + key: () => 'actions:domains/actions.ts:anonymous-gate', + }, + { + file: 'packages/runtime/src/domains/automation.ts', + re: /shouldDenyAnonymous\s*\(/g, + key: () => 'automation:domains/automation.ts:anonymous-gate', + }, + // Raw-hono standard /data routes — genuinely pattern-based: ANY new // `rawApp.(`${prefix}/data...`)` → a new key → CI fails until a row covers it. { @@ -149,6 +168,13 @@ const HIGH_RISK = [ // entry point rather than gating it, so there is nothing left to mark // high-risk there) 'anonymous-deny-meta', + // #5519 — the dispatcher-mounted execution surfaces. `/actions` reaches a + // `script` body that runs `isSystem: true` elevated and `/automation` starts, + // lists and deregisters flows, so both guard the same object data as REST + // `/data` through sibling entry points. Proven end-to-end by the same + // surfaces proof (#5570). + 'anonymous-deny-actions', + 'anonymous-deny-automation', // #2948/#3003 — write-integrity face: without the strip, `readonly: true` // is false compliance (declared ≠ enforced) and approval/status columns are // one direct PATCH away from self-approval. @@ -245,4 +271,22 @@ describe('#2567 — anonymous-deny surface ratchet bites', () => { ); expect(problems.some((p) => /STALE covers/.test(p) && p.includes(stdio))).toBe(true); }); + + // ── #5519 — the dispatcher execution-surface gates bite too ──────────── + it('(h) deleting either /actions or /automation anonymous gate → STALE covers failure (#5519)', () => { + for (const gate of [ + 'actions:domains/actions.ts:anonymous-gate', + 'automation:domains/automation.ts:anonymous-gate', + ]) { + // Baseline sanity: the gate is in source TODAY. If this ever goes false + // the surface has regressed to its pre-#5569 state, which is the whole + // point of the pin. + expect(discoverAnonymousDenySurfaces().has(gate), `${gate} must be in source`).toBe(true); + const problems = checkLedger( + AUTHZ_CONFORMANCE, + opts(() => new Set([...discoverAnonymousDenySurfaces()].filter((k) => k !== gate))), + ); + expect(problems.some((p) => /STALE covers/.test(p) && p.includes(gate))).toBe(true); + } + }); }); diff --git a/packages/qa/dogfood/test/showcase-anonymous-deny-surfaces.dogfood.test.ts b/packages/qa/dogfood/test/showcase-anonymous-deny-surfaces.dogfood.test.ts index 1150d4b42d..95f8415175 100644 --- a/packages/qa/dogfood/test/showcase-anonymous-deny-surfaces.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-anonymous-deny-surfaces.dogfood.test.ts @@ -8,11 +8,34 @@ // - the raw-hono standard `/data` routes (order-dependent shadowing) — that // surface has since been deleted outright (#4073), which removes the entry // point rather than gating it +// - the DISPATCHER-mounted execution surfaces `/actions/*` and `/automation/*` +// (#5519, gated by PR #5569 — see the block below) // // This proof boots the real showcase HTTP stack ON THE PLATFORM DEFAULT (the // verify harness passes no `requireAuth` override, so the flipped secure default // is what a fresh production deployment gets) and asserts every surface denies // an anonymous caller with 401 while an authenticated member is unaffected. +// +// ── Why `/actions` and `/automation` are here (#5570) ──────────────────────── +// +// `authz-conformance.matrix.ts` names THIS FILE as the proof artifact for the +// "#2567 anonymous posture is uniform across surfaces" claim. #5519 then found +// the claim false on exactly two surfaces this file did not drive: anonymous +// callers could POST a `script` action (whose body runs `isSystem: true` +// elevated) and could trigger, list, or DEREGISTER automation flows. The +// artifact was silent throughout — a declared ≠ proven gap living in the test +// layer. PR #5569 built the gate in `packages/runtime`; #5570 is the evidence +// half, so the proof file once again covers everything the matrix row claims +// it covers. +// +// The value this boot adds OVER #5569's own runtime integration test +// (`dispatcher-plugin.anonymous-gate.integration.test.ts`) is precisely the +// comparison that test documented it could NOT make: it boots a LiteKernel that +// mounts no `/data` and no `/meta`, so it had no second surface to contrast +// against. Here all four surfaces are served by ONE process — and by TWO +// different registration paths (`@objectstack/rest` owns `/data` + `/meta`; +// `dispatcher-plugin.ts` mounts `/actions` + `/automation` straight onto the +// host server) — which is the divergence #5519 was about in the first place. import { describe, it, expect, beforeAll } from 'vitest'; import { type VerifyStack } from '@objectstack/verify'; @@ -20,6 +43,18 @@ import { getSharedShowcase } from './shared-showcase.js'; const OBJ = '/data/showcase_private_note'; +// `showcase_mark_done` is a `type: 'script'` action declared on `showcase_task` +// whose body performs an `api.write` update — the exact action #5519 measured +// answering 200 to an anonymous caller. The record id is deliberately a +// non-existent one: the gate is the FIRST statement of `handleActionsRequest`, +// so an anonymous request is refused before any object, action or record is +// resolved. Needing a real record to get a 401 would mean the gate had moved +// behind the lookups. +const ACTION = '/actions/showcase_task/showcase_mark_done/anon-probe-id'; +// A real showcase flow declaration, so the deregister case names something that +// genuinely exists in the app's metadata. +const FLOW = 'showcase_reassign_wizard'; + describe('showcase: anonymous posture is uniform across surfaces (#2567)', () => { let stack: VerifyStack; let memberToken: string; @@ -30,6 +65,15 @@ describe('showcase: anonymous posture is uniform across surfaces (#2567)', () => memberToken = await stack.signUp('surfaces-member@verify.test'); }, 60_000); + /** An HTTP call carrying no credential of any kind. */ + const anon = (method: string, path: string, body?: unknown) => + stack.api(path, { + method, + ...(body === undefined + ? {} + : { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) }), + }); + // ── /meta ────────────────────────────────────────────────────────────── it('anonymous GET /meta is denied (401)', async () => { const r = await stack.api('/meta', { method: 'GET' }); @@ -51,4 +95,109 @@ describe('showcase: anonymous posture is uniform across surfaces (#2567)', () => const r = await stack.apiAs(memberToken, 'GET', OBJ); expect(r.status).toBe(200); }); + + // ── /actions (dispatcher-mounted; runtime domains/actions.ts) — #5519 ─── + it('anonymous POST of a script action is denied (401)', async () => { + const r = await anon('POST', ACTION, { params: {} }); + expect(r.status, 'anonymous action dispatch must be 401').toBe(401); + }); + + it('an authenticated member is NOT denied on the action surface', async () => { + // Whatever the action surface answers a MEMBER — 200, a 400 from the param + // contract, a 403 from `requiredPermissions` — it is an answer about + // authorization, not about anonymity. The assertion is deliberately the + // same `.not.toBe(401)` the /meta contrast uses: this file's subject is the + // anonymous posture, and pinning the member's exact status here would make + // it fail for reasons that belong to other proofs. + const r = await stack.apiAs(memberToken, 'POST', ACTION, { params: {} }); + expect(r.status, 'an authenticated caller must clear the auth gate').not.toBe(401); + }); + + // ── /automation (dispatcher-mounted; runtime domains/automation.ts) ───── + // + // The gate is DOMAIN-WIDE and sits ahead of the `isServiceServeable` probe on + // purpose: this stack installs no `@objectstack/service-automation`, so the + // domain's own answer here is 501. If the gate ran after the probe, anonymous + // and authenticated callers would both get 501 and the 401/501 difference + // would fingerprint whether a deployment mounts automation at all. The + // authenticated 501 case below is what gives these three cases their teeth: + // in this one process, the same route answers 401 to anonymous and 501 to a + // member, so the 401 can only be the gate's answer. + it('anonymous POST /automation/:name/trigger is denied (401)', async () => { + const r = await anon('POST', `/automation/${FLOW}/trigger`, { recordId: 'anon-probe-id' }); + expect(r.status, 'anonymous flow trigger must be 401').toBe(401); + }); + + it('anonymous GET /automation is denied (401) — the flow inventory stays private', async () => { + const r = await anon('GET', '/automation'); + expect(r.status, 'anonymous flow listing must be 401').toBe(401); + }); + + it('anonymous DELETE /automation/:name is denied (401) — the destructive one', async () => { + const r = await anon('DELETE', `/automation/${FLOW}`); + expect(r.status, 'anonymous flow deregistration must be 401').toBe(401); + }); + + it('an authenticated caller reaches the domain, which answers 501 — not 401', async () => { + const r = await stack.apiAs(memberToken, 'GET', '/automation'); + expect(r.status, 'authenticated flow listing must clear the auth gate').not.toBe(401); + // The domain's OWN answer on a stack with no automation service. Asserting + // it (rather than only `.not.toBe(401)`) is what proves the anonymous 401 + // above is produced by the gate and not by the domain: drop the gate and + // the anonymous cases collapse onto THIS status. + expect(r.status, 'no @objectstack/service-automation is installed on this boot').toBe(501); + }); + + // ── one code, one message — two wrappers ─────────────────────────────── + it('every denied surface answers the SAME code and message (the wrappers differ)', async () => { + const rest = await Promise.all([ + anon('GET', '/meta').then((r) => r.json()), + anon('GET', OBJ).then((r) => r.json()), + ]); + const dispatcher = await Promise.all([ + anon('POST', ACTION, { params: {} }).then((r) => r.json()), + anon('POST', `/automation/${FLOW}/trigger`, {}).then((r) => r.json()), + anon('GET', '/automation').then((r) => r.json()), + anon('DELETE', `/automation/${FLOW}`).then((r) => r.json()), + ]); + + // Each family is read in ITS OWN declared shape — no `??` chain across the + // two, because a tolerant reader here would hide the day one of them + // changes. `@objectstack/rest` returns the flat `ANONYMOUS_DENY_BODY` + // (`{ error: , message }`); the dispatcher returns its standard + // wrapper (`{ success: false, error: { code, message, httpStatus } }`). + for (const body of rest) { + expect(body).toEqual({ + error: 'UNAUTHENTICATED', + message: 'Authentication is required to access this endpoint.', + }); + } + for (const body of dispatcher) { + expect(body).toMatchObject({ + success: false, + error: { + code: 'UNAUTHENTICATED', + message: 'Authentication is required to access this endpoint.', + httpStatus: 401, + }, + }); + } + + // What is genuinely uniform — and what #2567 claims — is the SEMANTICS: one + // status, one code, one message, whichever surface you knock on. The two + // wrappers are a known, pre-existing platform-wide split (ADR-0112's + // amendment records `@objectstack/rest`'s flat envelope and the dispatcher's + // wrapped one as the two live shapes). Pinned here so that split cannot + // quietly widen into two different DENIALS. + const codes = new Set([ + ...rest.map((b: any) => b.error), + ...dispatcher.map((b: any) => b.error.code), + ]); + const messages = new Set([ + ...rest.map((b: any) => b.message), + ...dispatcher.map((b: any) => b.error.message), + ]); + expect([...codes]).toEqual(['UNAUTHENTICATED']); + expect([...messages]).toEqual(['Authentication is required to access this endpoint.']); + }); });