diff --git a/.changeset/automation-write-manage-metadata-gate.md b/.changeset/automation-write-manage-metadata-gate.md new file mode 100644 index 0000000000..055657e8aa --- /dev/null +++ b/.changeset/automation-write-manage-metadata-gate.md @@ -0,0 +1,20 @@ +--- +"@objectstack/runtime": patch +--- + +**Behaviour change (security tightening):** the `/api/v1/automation` **definition writes** now require the `manage_metadata` capability (#10145). + +`POST /api/v1/automation`, `PUT /api/v1/automation/:name` and `DELETE /api/v1/automation/:name` — `automation.create` / `automation.update` / `automation.delete` on the SDK — were reachable by **any authenticated caller**. They now answer **403 `PERMISSION_DENIED`** unless the caller holds `manage_metadata` (ADR-0066 D1's authoring capability), the same key the sibling `PUT /api/v1/meta/:type/:name` and every state-changing `/api/v1/packages/*` route already demand. Engine self-invocation (`isSystem`) bypasses, as on every other capability gate. + +**Existing credentialed callers that author flows over HTTP will start getting 403** and must be granted `manage_metadata`. A flow is authored metadata: this closes the last write door onto the metadata plane that did not ask the metadata plane's question. + +What was measured on a walled multi-organization deployment (`OS_TENANCY_POSTURE=isolated`): a plain tenant org owner holding `organization_admin` — the same session answered 403 by `PUT /meta/:type/:name`, `POST /ai/tools/:tool/execute` and `POST /packages/*` — created, modified and deleted flows through this door, all 200. Flow definitions are registered at **environment** scope, not organization scope, so the write crossed the tenant wall: a shipped flow deleted by one tenant read 404 for the actor, for an unrelated tenant **and** for the platform admin, and an injected flow read 200 for all three. + +**Deliberately unchanged — execution is not authoring:** + +- `POST /automation/:name/trigger` and the legacy `POST /automation/trigger/:name` **run** a flow. They keep their existing posture (authenticated, plus the flow's own `runAs` authorization envelope). +- `POST /automation/:name/runs/:runId/resume` is already fail-closed through the suspended node's `resumeAuthority`; a metadata capability in front of it would refuse the very user the flow paused for. +- `POST /automation/:name/toggle` mutates engine enablement rather than a definition, and is filed separately rather than folded into a security fix. +- The reads (`GET /automation`, `GET /automation/:name`, the run surfaces) are untouched; run-state reads keep their `sys_automation_run` grant. + +The gate sits ahead of the service probe and ahead of body validation, so a refused caller neither writes anything nor learns from a 501-vs-403 whether the deployment mounts automation at all. diff --git a/packages/runtime/src/domains/automation-body-validation.test.ts b/packages/runtime/src/domains/automation-body-validation.test.ts index 8dcd529732..eaf95c1479 100644 --- a/packages/runtime/src/domains/automation-body-validation.test.ts +++ b/packages/runtime/src/domains/automation-body-validation.test.ts @@ -42,7 +42,22 @@ function makeDispatcher() { return { dispatcher: new HttpDispatcher(kernel), spies }; } -const CTX = { request: {}, executionContext: { userId: 'user_1' } } as any; +/** + * [#10145] The caller carries `manage_metadata` — the ADR-0066 D1 authoring + * capability the `/automation` definition writes (`POST /`, `PUT /:name`, + * `DELETE /:name`) now demand, the same gate the sibling `/meta` and + * `/packages` writes already carry. + * + * The cases below are about BODY VALIDATION — which malformed shapes are + * refused, with which field codes. They were written when any + * authenticated session could register a flow, i.e. their `{ userId: 'user_1' }` + * stub encoded exactly the premise the gate destroys, so without a capability + * they would now stop at the 403 before reaching the behaviour each one is named + * after. Only the CALLER changes here; every mechanism, assertion and expected + * value is untouched. The gate itself is pinned in + * `automation-write-capability-gate.test.ts`. + */ +const CTX = { request: {}, executionContext: { userId: 'user_1', systemPermissions: ['manage_metadata'] } } as any; async function expectValidationFailure(run: Promise, label: string) { let thrown: unknown; diff --git a/packages/runtime/src/domains/automation-put-post-error-parity.test.ts b/packages/runtime/src/domains/automation-put-post-error-parity.test.ts index 26cdf4bd77..d1f5a3367f 100644 --- a/packages/runtime/src/domains/automation-put-post-error-parity.test.ts +++ b/packages/runtime/src/domains/automation-put-post-error-parity.test.ts @@ -107,7 +107,22 @@ function makeDispatcher() { return { dispatcher: new HttpDispatcher(kernel), registered, calls }; } -const CTX = { request: {}, executionContext: { userId: 'user_1' } } as any; +/** + * [#10145] The caller carries `manage_metadata` — the ADR-0066 D1 authoring + * capability the `/automation` definition writes (`POST /`, `PUT /:name`, + * `DELETE /:name`) now demand, the same gate the sibling `/meta` and + * `/packages` writes already carry. + * + * The cases below are about PARITY between the POST and PUT doors — that an + * identical refusal is classified identically whichever door it arrives at. They were written when any + * authenticated session could register a flow, i.e. their `{ userId: 'user_1' }` + * stub encoded exactly the premise the gate destroys, so without a capability + * they would now stop at the 403 before reaching the behaviour each one is named + * after. Only the CALLER changes here; every mechanism, assertion and expected + * value is untouched. The gate itself is pinned in + * `automation-write-capability-gate.test.ts`. + */ +const CTX = { request: {}, executionContext: { userId: 'user_1', systemPermissions: ['manage_metadata'] } } as any; /** A definition that is legal at every gate the fake runs. */ const WELL_FORMED = { diff --git a/packages/runtime/src/domains/automation-register-error-class.test.ts b/packages/runtime/src/domains/automation-register-error-class.test.ts index eeff2f60b0..ab2544c22c 100644 --- a/packages/runtime/src/domains/automation-register-error-class.test.ts +++ b/packages/runtime/src/domains/automation-register-error-class.test.ts @@ -139,7 +139,22 @@ function makeDispatcher(options?: { registerFlow?: (name: string, definition: un return { dispatcher: new HttpDispatcher(kernel), spies, registered }; } -const CTX = { request: {}, executionContext: { userId: 'user_1' } } as any; +/** + * [#10145] The caller carries `manage_metadata` — the ADR-0066 D1 authoring + * capability the `/automation` definition writes (`POST /`, `PUT /:name`, + * `DELETE /:name`) now demand, the same gate the sibling `/meta` and + * `/packages` writes already carry. + * + * The cases below are about the ERROR CLASS a rejected definition is served as + * (400 vs 500) and the unknown-flow 404. They were written when any + * authenticated session could register a flow, i.e. their `{ userId: 'user_1' }` + * stub encoded exactly the premise the gate destroys, so without a capability + * they would now stop at the 403 before reaching the behaviour each one is named + * after. Only the CALLER changes here; every mechanism, assertion and expected + * value is untouched. The gate itself is pinned in + * `automation-write-capability-gate.test.ts`. + */ +const CTX = { request: {}, executionContext: { userId: 'user_1', systemPermissions: ['manage_metadata'] } } as any; /** A definition that is legal at every gate the fake runs. */ const WELL_FORMED = { diff --git a/packages/runtime/src/domains/automation-write-capability-gate.test.ts b/packages/runtime/src/domains/automation-write-capability-gate.test.ts new file mode 100644 index 0000000000..428825cae4 --- /dev/null +++ b/packages/runtime/src/domains/automation-write-capability-gate.test.ts @@ -0,0 +1,341 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#10145] The `/automation` AUTHORING writes demand the `manage_metadata` + * capability (ADR-0066 D1) — the same gate the sibling `/meta` and `/packages` + * writes already carry. + * + * ## What was measured before this gate + * + * On a walled hosted-SaaS deployment (`OS_TENANCY_POSTURE=isolated`), a plain + * tenant org owner holding `organization_admin` and demonstrably NOT + * `manage_metadata` — the same session is answered **403** by + * `PUT /api/v1/meta/:type/:name`, `POST /api/v1/ai/tools/:tool/execute` and + * `POST /api/v1/packages/*` — could `POST` / `PUT` / `DELETE` + * `/api/v1/automation` and be answered 200. A flow is authored metadata + * registered at ENVIRONMENT scope, not organization scope, so the write landed + * on the layer every organization runs on: the filer deleted a shipped flow and + * read it back **404 as the actor, as an unrelated tenant, and as the platform + * admin**; an injected flow read 200 for all three. Privilege escalation and a + * cross-tenant one at once. + * + * The domain's own `GET /` audit note already said where this belongs: *"Flow + * definitions are metadata and are governed on the metadata plane"*. The write + * half is that plane's authoring capability, applied at this door. + * + * ## The scope line this file pins in BOTH directions + * + * AUTHORING is gated; EXECUTION is not. `POST /:name/trigger`, + * `POST /trigger/:name` and `POST /:name/runs/:runId/resume` run a flow rather + * than author one — `resume` is additionally fail-closed through the suspended + * node's `resumeAuthority` (#3801 / #5561) — and `POST /:name/toggle` mutates + * engine enablement. None of them writes a flow DEFINITION, so none of them is + * swept into a metadata gate here; the `stays ungated` block below is the audit + * that makes any future change to those four verdicts come through this file. + * + * ## What the refusal cases assert + * + * `status` AND `code` (the ADR-0112 envelope), plus — the point — that + * `registerFlow` / `unregisterFlow` were **never entered**. A gate that refused + * after the registry was already mutated would still be the defect and would + * still satisfy a status-only assertion, so the spy is the load-bearing + * assertion, not decoration. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import { HttpDispatcher } from '../http-dispatcher.js'; +import type { HttpProtocolContext } from '../http-dispatcher.js'; + +const FLOW = 'lead_auto_assignment'; + +/** A legal flow definition — so nothing below is refused for its shape. */ +const DEFINITION = { name: FLOW, label: 'Lead Auto Assignment', type: 'autolaunched', nodes: [], edges: [] }; + +interface Harness { + dispatcher: HttpDispatcher; + registerFlow: ReturnType; + unregisterFlow: ReturnType; + execute: ReturnType; + toggleFlow: ReturnType; + resume: ReturnType; + listFlows: ReturnType; + getFlow: ReturnType; + /** Flow names the registry currently holds — read from the STORE, not a response. */ + registered: () => string[]; +} + +/** + * A dispatcher over a stub automation service backed by a REAL in-memory + * registry, so a refused write can be checked against the registry rather than + * only against the answer. + * + * `automation: false` is a deployment that mounts no automation service at all + * — the 501 case the gate must not be distinguishable from. + */ +function boot({ automation = true }: { automation?: boolean } = {}): Harness { + const flows = new Map([[FLOW, { ...DEFINITION }]]); + + const registerFlow = vi.fn((name: string, definition: unknown) => { flows.set(name, definition); }); + const unregisterFlow = vi.fn((name: string) => { flows.delete(name); }); + const getFlow = vi.fn(async (name: string) => flows.get(name)); + const execute = vi.fn(async () => ({ success: true, runId: 'run_1', status: 'completed' })); + const toggleFlow = vi.fn(async () => undefined); + const resume = vi.fn(async () => ({ success: true, status: 'completed' })); + const listFlows = vi.fn(async () => [...flows.keys()]); + + const services: Record = {}; + if (automation) { + services.automation = { + handlerReady: true, + registerFlow, unregisterFlow, getFlow, execute, toggleFlow, resume, listFlows, + }; + } + + const resolve = (name: string): unknown => services[name]; + const kernel = { + getService: resolve, + getServiceAsync: async (name: string) => resolve(name), + context: { getService: resolve }, + }; + + return { + dispatcher: new HttpDispatcher(kernel as never), + registerFlow, unregisterFlow, execute, toggleFlow, resume, listFlows, getFlow, + registered: () => [...flows.keys()].sort(), + }; +} + +/** + * The filer's principal: a tenant org owner, authenticated, holding + * `organization_admin` and NO authoring capability. + */ +const UNENTITLED = (): HttpProtocolContext => ({ + request: {}, + executionContext: { + userId: 'u_northwind_owner', + positions: ['organization_admin'], + permissions: ['org_admin'], + systemPermissions: [], + }, +} as HttpProtocolContext); + +/** Authoring-adjacent capabilities that are NOT this gate's key. */ +const NEAR_MISS = (): HttpProtocolContext => ({ + request: {}, + executionContext: { userId: 'u_setup', systemPermissions: ['studio.access', 'setup.access'] }, +} as HttpProtocolContext); + +/** A metadata author. */ +const AUTHOR = (): HttpProtocolContext => ({ + request: {}, + executionContext: { userId: 'u_author', systemPermissions: ['manage_metadata'] }, +} as HttpProtocolContext); + +/** Engine self-invocation — never settable from the wire. */ +const SYSTEM = (): HttpProtocolContext => ({ + request: {}, + executionContext: { userId: 'usr_system', isSystem: true }, +} as HttpProtocolContext); + +/** No resolved identity at all. */ +const ANON = (): HttpProtocolContext => ({ request: {}, executionContext: {} } as HttpProtocolContext); + +const statusOf = (response: unknown): unknown => (response as any)?.status; +const codeOf = (response: unknown): unknown => { + const r = response as any; + return r?.body?.error?.code ?? r?.body?.error?.details?.code; +}; + +/** The three AUTHORING writes, each with the service method it must never reach. */ +const AUTHORING_WRITES = [ + { + name: 'POST /automation (createFlow)', + drive: (h: Harness, ctx: HttpProtocolContext) => + h.dispatcher.handleAutomation('', 'POST', { ...DEFINITION, name: 'probe_flow_x' }, ctx, undefined), + spy: (h: Harness) => h.registerFlow, + }, + { + name: 'PUT /automation/:name (updateFlow)', + drive: (h: Harness, ctx: HttpProtocolContext) => + h.dispatcher.handleAutomation(`/${FLOW}`, 'PUT', { ...DEFINITION, label: 'clobbered' }, ctx, undefined), + spy: (h: Harness) => h.registerFlow, + }, + { + name: 'DELETE /automation/:name (deleteFlow)', + drive: (h: Harness, ctx: HttpProtocolContext) => + h.dispatcher.handleAutomation(`/${FLOW}`, 'DELETE', undefined, ctx, undefined), + spy: (h: Harness) => h.unregisterFlow, + }, +] as const; + +describe('#10145 — /automation authoring writes require `manage_metadata`', () => { + describe('refusal — the unentitled tenant principal from the report', () => { + for (const route of AUTHORING_WRITES) { + it(`${route.name}: 403 PERMISSION_DENIED, and the registry is untouched`, async () => { + const h = boot(); + const { response } = await route.drive(h, UNENTITLED()); + + // ADR-0112 wants both halves: a 403 carrying no code, or the code + // on a 200, each satisfies exactly half of the contract. + expect(statusOf(response)).toBe(403); + expect(codeOf(response)).toBe('PERMISSION_DENIED'); + + // THE POINT. "Delete first, refuse second" would pass the two + // assertions above and still be the cross-tenant defect. + expect(route.spy(h)).not.toHaveBeenCalled(); + expect(h.registered()).toEqual([FLOW]); + }); + } + + it('refuses BEFORE the body is validated — a malformed definition is still not a 422', async () => { + // The gate sits ahead of #3899's body checks, so an unentitled caller + // learns nothing about the definition contract by probing it, and + // nothing reaches the registry on any body. + const h = boot(); + const { response } = await h.dispatcher.handleAutomation('', 'POST', { label: 'no name' }, UNENTITLED(), undefined); + + expect(statusOf(response)).toBe(403); + expect(codeOf(response)).toBe('PERMISSION_DENIED'); + expect(h.registerFlow).not.toHaveBeenCalled(); + }); + + it('an unrelated authoring-adjacent capability is not enough', async () => { + // `studio.access` / `setup.access` are the ADR-0106 D4 READ set; the + // write key is `manage_metadata`, exactly as `/packages` splits them. + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}`, 'DELETE', undefined, NEAR_MISS(), undefined); + + expect(statusOf(response)).toBe(403); + expect(h.unregisterFlow).not.toHaveBeenCalled(); + }); + + it('does not answer the caller\'s authorization topology in the refusal (#7450)', async () => { + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}`, 'DELETE', undefined, UNENTITLED(), undefined); + + const serialized = JSON.stringify((response as any).body); + expect(serialized).not.toContain('organization_admin'); + expect(serialized).not.toContain('u_northwind_owner'); + expect(serialized).not.toContain('org_admin'); + }); + + it('fires BEFORE the service probe, so 403-vs-501 leaks no deployment capability', async () => { + // A deployment with no automation service answers 501 to an entitled + // caller. An unentitled one must not be able to tell the two + // deployments apart — the posture the anonymous floor and the + // run-state gate in this domain already take. + const bare = boot({ automation: false }); + + const refused = await bare.dispatcher.handleAutomation('', 'POST', { ...DEFINITION }, UNENTITLED(), undefined); + expect(statusOf(refused.response)).toBe(403); + expect(codeOf(refused.response)).toBe('PERMISSION_DENIED'); + + const entitled = await bare.dispatcher.handleAutomation('', 'POST', { ...DEFINITION }, AUTHOR(), undefined); + expect(statusOf(entitled.response)).toBe(501); + }); + }); + + describe('positive control — an entitled principal writes exactly what it wrote before', () => { + it('POST /automation registers the definition verbatim', async () => { + const h = boot(); + const body = { ...DEFINITION, name: 'probe_flow_x' }; + const { response } = await h.dispatcher.handleAutomation('', 'POST', body, AUTHOR(), undefined); + + expect(statusOf(response)).toBe(200); + expect(h.registerFlow).toHaveBeenCalledWith('probe_flow_x', body); + expect(h.registered()).toEqual([FLOW, 'probe_flow_x'].sort()); + }); + + it('PUT /automation/:name updates through the same one argument shape', async () => { + const h = boot(); + const definition = { ...DEFINITION, label: 'Renamed' }; + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}`, 'PUT', definition, AUTHOR(), undefined); + + expect(statusOf(response)).toBe(200); + expect(h.registerFlow).toHaveBeenCalledWith(FLOW, definition); + }); + + it('DELETE /automation/:name deregisters', async () => { + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}`, 'DELETE', undefined, AUTHOR(), undefined); + + expect(statusOf(response)).toBe(200); + expect((response as any).body?.data?.deleted).toBe(true); + expect(h.unregisterFlow).toHaveBeenCalledWith(FLOW); + expect(h.registered()).toEqual([]); + }); + + it('engine self-invocation (`isSystem`) bypasses, matching every other capability gate', async () => { + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}`, 'DELETE', undefined, SYSTEM(), undefined); + + expect(statusOf(response)).toBe(200); + expect(h.unregisterFlow).toHaveBeenCalledWith(FLOW); + }); + }); + + describe('the anonymous floor still answers first — this gate is the second layer', () => { + for (const route of AUTHORING_WRITES) { + it(`${route.name}: anonymous is 401, not 403`, async () => { + // #5519's domain-wide floor. Preserved verbatim: an anonymous + // caller must not learn from a 401-vs-403 which capability the + // route wants. + const h = boot(); + const { response } = await route.drive(h, ANON()); + + expect(statusOf(response)).toBe(401); + expect(route.spy(h)).not.toHaveBeenCalled(); + }); + } + }); + + describe('the audit — EXECUTION and READ routes stay ungated by this capability', () => { + it('POST /:name/trigger runs for a caller with no authoring capability', async () => { + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}/trigger`, 'POST', {}, UNENTITLED(), undefined); + + expect(statusOf(response)).not.toBe(403); + expect(h.execute).toHaveBeenCalled(); + }); + + it('POST /trigger/:name (the legacy SDK shape) runs too', async () => { + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/trigger/${FLOW}`, 'POST', {}, UNENTITLED(), undefined); + + expect(statusOf(response)).not.toBe(403); + expect(h.execute).toHaveBeenCalled(); + }); + + it('POST /:name/toggle stays ungated — enablement is engine state, not a definition write', async () => { + // Deliberately OUT of this card's write set. Toggling is arguably an + // authoring write and is filed separately rather than folded in; if + // that verdict changes it changes here, in the open. + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}/toggle`, 'POST', { enabled: false }, UNENTITLED(), undefined); + + expect(statusOf(response)).not.toBe(403); + expect(h.toggleFlow).toHaveBeenCalledWith(FLOW, false); + }); + + it('POST /:name/runs/:runId/resume stays ungated — it is fail-closed on `resumeAuthority` (#3801/#5561)', async () => { + const h = boot(); + const { response } = await h.dispatcher.handleAutomation( + `/${FLOW}/runs/run_1/resume`, 'POST', { inputs: {} }, UNENTITLED(), undefined, + ); + + expect(statusOf(response)).not.toBe(403); + expect(h.resume).toHaveBeenCalled(); + }); + + it('the reads are untouched — GET / and GET /:name', async () => { + const h = boot(); + + const list = await h.dispatcher.handleAutomation('', 'GET', undefined, UNENTITLED(), undefined); + expect(statusOf(list.response)).toBe(200); + + const detail = await h.dispatcher.handleAutomation(`/${FLOW}`, 'GET', undefined, UNENTITLED(), undefined); + expect(statusOf(detail.response)).toBe(200); + }); + }); +}); diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 9ccf12279a..5f64e00f35 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -276,6 +276,119 @@ async function refuseUngrantedRunRead( }; } +/** + * [#10145] ADR-0066 D1's authoring capability, required by the `/automation` + * DEFINITION writes. + * + * The same key the sibling metadata doors already demand — `PUT /meta/:type/:name` + * on both of its transports (#6603 REST, #7019 dispatcher), `POST /meta/_migrate-stored`, + * and every state-changing `/packages` route (#7033). A flow IS authored metadata: + * the domain's own `GET /` audit note says so in as many words ("Flow definitions + * are metadata and are governed on the metadata plane (`/meta`, ADR-0106)"), and + * that note settled the READ posture while leaving the WRITE half of the same + * sentence unstated. This is that half. + */ +const FLOW_AUTHORING_CAPABILITY = 'manage_metadata'; + +/** [#10145] Refusal vocabulary for the authoring-write gate (ADR-0112: code AND status). */ +const FLOW_WRITE_DENY_STATUS = 403; +const FLOW_WRITE_DENY_CODE = 'PERMISSION_DENIED'; +const FLOW_WRITE_DENY_MESSAGE = + `Authoring automation flows requires the \`${FLOW_AUTHORING_CAPABILITY}\` capability.`; + +/** + * [#10145] Which `/automation` routes AUTHOR a flow definition. + * + * One predicate, for the reason {@link isRunStateRead} is one predicate: this + * domain gets one policy per data class, and a policy spelled at three call + * sites is three policies that happen to agree today. + * + * `POST /` → registerFlow (create) + * `PUT /:name` → registerFlow (update) + * `DELETE /:name` → unregisterFlow (deregister) + * + * ⛔ The EXECUTION routes are deliberately NOT here, and the omission is the + * ruling rather than an oversight — authoring and executing are different + * questions, and sweeping a run surface into a metadata gate would lock every + * ordinary user out of the flows built for them: + * + * - `POST /:name/trigger` and the legacy `POST /trigger/:name` RUN a flow. + * They are the door a member's own record action goes through. + * - `POST /:name/runs/:runId/resume` resumes a paused run and is already + * fail-closed on the suspended node's `resumeAuthority` (#3801 / #5561) — + * a second, unrelated gate in front of it would refuse the very user the + * flow paused for, which is the mistake #7968 records for the screen read. + * - `POST /:name/toggle` mutates ENGINE enablement rather than a definition. + * Arguably an authoring write; filed separately rather than folded in here, + * so the decision is made in the open instead of riding a security fix. + * + * The reads are untouched: `GET /` and `GET /:name` serve flow definitions and + * keep the posture the #7900 audit recorded for them. + */ +function isFlowAuthoringWrite(parts: string[], method: string): boolean { + // `POST /automation` — the create door. `parts` is empty only for the + // domain root, so `POST /trigger/:name` (parts `['trigger', name]`) and + // `POST /:name/trigger` cannot reach this arm. + if (method === 'POST') return parts.length === 0; + // `PUT /automation/:name` / `DELETE /automation/:name` — the update and + // deregister doors. Exactly one segment: a deeper path is a run surface. + if (method === 'PUT' || method === 'DELETE') return parts.length === 1; + return false; +} + +/** + * [#10145] The authoring-write gate: refuse a caller without + * {@link FLOW_AUTHORING_CAPABILITY}. + * + * ## What it closes, measured over HTTP + * + * On a walled multi-organization deployment (`OS_TENANCY_POSTURE=isolated`) a + * plain tenant org owner holding `organization_admin` and NOT this capability — + * the same session answered 403 by `PUT /meta/:type/:name`, + * `POST /ai/tools/:tool/execute` and `POST /packages/*` — created, modified and + * DELETED flows here, all 200. Flow metadata is registered at ENVIRONMENT + * scope, not organization scope, so the write crossed the tenant wall: a + * shipped flow one tenant deleted read 404 for the actor, for an unrelated + * tenant AND for the platform admin; an injected flow read 200 for all three. + * Privilege escalation and cross-tenant metadata mutation in one call. + * + * ## Structure — copied from this file's own neighbours, not reinvented + * + * Returns a refusal to short-circuit on, `undefined` to proceed, so the caller + * reads as a guard clause and no route can consume a denial as a value + * ({@link refuseUngrantedRunRead}'s shape). + * + * Engine self-invocation (`isSystem`, never settable from the wire) bypasses, + * matching `/meta`'s and `/packages`' gates and the run-state gate above: a + * door that refused what the engine's own metadata loader does would not be + * convergence. The capability channel is `systemPermissions` — CAPABILITIES, + * not permission-SET names, which ride `permissions` (#4705). + * + * The message names the CAPABILITY it wants and nothing about the caller (no + * positions, no permission-set names — #7450). + * + * ⚠️ Callers MUST run this BEFORE the automation service is resolved and before + * any body validation, so (a) an unentitled caller cannot use the 501-vs-403 + * answer to fingerprint whether this deployment mounts automation, (b) nothing + * is registered or unregistered before the refusal — "delete first, refuse + * second" is the worst shape here, and it is precisely what the report + * measured — and (c) the definition contract is not enumerable by probing + * 422s from outside the authoring cohort. + */ +function refuseUngrantedFlowWrite( + deps: DomainHandlerDeps, + context: HttpProtocolContext, +): HttpDispatcherResult | undefined { + const ec: any = context?.executionContext; + if (ec?.isSystem) return undefined; + if (new Set(ec?.systemPermissions ?? []).has(FLOW_AUTHORING_CAPABILITY)) return undefined; + + return { + handled: true, + response: deps.error(FLOW_WRITE_DENY_MESSAGE, FLOW_WRITE_DENY_STATUS, { code: FLOW_WRITE_DENY_CODE }), + }; +} + /** * [#7968] The screen route's gate: **the run's own trigger identity, OR the * `sys_automation_run` read grant as an operator override.** @@ -716,6 +829,21 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str if (refusal) return refusal; } + // [#10145] AUTHORING-WRITE GATE — `manage_metadata`, the capability the + // metadata plane these flow definitions live on already requires of every + // other door onto it. Placed with the two gates above and AHEAD of the + // service probe below for their reason, read one tier up: which capability + // a route requires must not vary with which automation service a deployment + // mounts, and a 501-vs-403 must not be what tells an unentitled caller + // whether automation is mounted here. Ahead of every body check too — a + // refused caller writes nothing and learns nothing about the definition + // contract. Which routes: `isFlowAuthoringWrite` above, one predicate, with + // the execution surfaces deliberately outside it. + if (isFlowAuthoringWrite(parts, m)) { + const refusal = refuseUngrantedFlowWrite(deps, context); + if (refusal) return refusal; + } + const automationService = await deps.getService(context, CoreServiceName.enum.automation); // [#4058] Empty slot — or a slot filled by a self-declared non-handler // (`handlerReady: false`, ADR-0076 D12), which is the same amount of diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index ba80466fdb..fb5ac075f6 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -69,6 +69,19 @@ const METADATA_AUTHOR = () => ({ request: {}, executionContext: { userId: 'u1', */ const PKG_ADMIN = () => ({ request: {}, executionContext: { userId: 'u_pkg_admin', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'] } }) as any; +/** + * [#10145] The same move for the `/automation` DEFINITION writes — `POST /`, + * `PUT /:name` and `DELETE /:name` now demand `manage_metadata`, the authoring + * capability the metadata plane these flows live on already required. The three + * cases using this caller are about ROUTING — which automation-service method a + * path reaches and with which arguments — and were written when an ordinary + * session could register a flow, which is precisely the premise the gate + * destroys. Only the caller changes; the gate itself is pinned in + * `domains/automation-write-capability-gate.test.ts`, and every EXECUTION route + * on the domain (trigger / toggle / resume) keeps `AUTHED_CALLER`, deliberately. + */ +const FLOW_AUTHOR = () => ({ request: {}, executionContext: { userId: 'u_flow_author', systemPermissions: ['manage_metadata'] } }) as any; + describe('HttpDispatcher', () => { let kernel: ObjectKernel; let dispatcher: HttpDispatcher; @@ -314,20 +327,20 @@ describe('HttpDispatcher', () => { it('should create a flow via POST /', async () => { const body = { name: 'new_flow', label: 'New Flow' }; - const result = await dispatcher.handleAutomation('', 'POST', body, AUTHED_CALLER()); + const result = await dispatcher.handleAutomation('', 'POST', body, FLOW_AUTHOR()); expect(result.handled).toBe(true); expect(mockAutomationService.registerFlow).toHaveBeenCalledWith('new_flow', body); }); it('should update a flow via PUT /:name', async () => { const body = { definition: { label: 'Updated' } }; - const result = await dispatcher.handleAutomation('flow_a', 'PUT', body, AUTHED_CALLER()); + const result = await dispatcher.handleAutomation('flow_a', 'PUT', body, FLOW_AUTHOR()); expect(result.handled).toBe(true); expect(mockAutomationService.registerFlow).toHaveBeenCalledWith('flow_a', { label: 'Updated' }); }); it('should delete a flow via DELETE /:name', async () => { - const result = await dispatcher.handleAutomation('flow_a', 'DELETE', {}, AUTHED_CALLER()); + const result = await dispatcher.handleAutomation('flow_a', 'DELETE', {}, FLOW_AUTHOR()); expect(result.handled).toBe(true); expect(mockAutomationService.unregisterFlow).toHaveBeenCalledWith('flow_a'); expect(result.response?.body?.data?.deleted).toBe(true); @@ -3260,8 +3273,21 @@ describe('HttpDispatcher', () => { // What this test pins is unchanged and is the part that matters: // the stub is NEVER CALLED, so nothing can read "flow executed" // off a flow that never ran. - for (const [path, method] of [['', 'GET'], ['', 'POST'], ['trigger/x', 'POST'], ['x/trigger', 'POST']] as const) { - const result = await dispatcher.handleAutomation(path, method, { name: 'x' }, AUTHED_CALLER()); + // [#10145] The caller is per row now. `POST /` is a DEFINITION + // write and demands `manage_metadata`, whose gate sits ahead of the + // service probe deliberately — so an unentitled caller stops at a + // 403 and never reaches the 501 this row exists to pin. Giving that + // one row the capability keeps the row measuring what it is named + // after; the execution rows keep the ordinary caller, which is + // exactly the scope line #10145 drew. + const rows = [ + ['', 'GET', AUTHED_CALLER], + ['', 'POST', FLOW_AUTHOR], + ['trigger/x', 'POST', AUTHED_CALLER], + ['x/trigger', 'POST', AUTHED_CALLER], + ] as const; + for (const [path, method, caller] of rows) { + const result = await dispatcher.handleAutomation(path, method, { name: 'x' }, caller()); expect(result.response?.status, `${method} /automation/${path}`).toBe(501); } expect(stub.execute).not.toHaveBeenCalled();