diff --git a/.changeset/automation-toggle-manage-metadata.md b/.changeset/automation-toggle-manage-metadata.md new file mode 100644 index 0000000000..f20b153a89 --- /dev/null +++ b/.changeset/automation-toggle-manage-metadata.md @@ -0,0 +1,30 @@ +--- +"@objectstack/runtime": minor +--- + +**BREAKING (authorization):** `POST /api/v1/automation/:name/toggle` now requires the `manage_metadata` capability. A caller that holds a session but not that capability is answered **403 `PERMISSION_DENIED`** where it previously received **200** with the flow's enablement changed. + +This narrows what the API accepts, so it ships as `minor` with the breaking surface named rather than as a `patch`. + + + +**The exact surface that moves** + +| | before | after | +|---|---|---| +| authenticated caller **with** `manage_metadata` | 200, flow toggled | 200, flow toggled — unchanged | +| authenticated caller **without** it | 200, flow toggled | **403 `PERMISSION_DENIED`**, `toggleFlow` never entered | +| anonymous caller | 401 | 401 — unchanged, the #5519 floor still answers first | +| engine self-invocation (`isSystem`) | 200 | 200 — unchanged | + +Nothing else on the domain moves. The execution doors keep their posture: `POST /:name/trigger`, the legacy `POST /trigger/:name` and `POST /:name/runs/:runId/resume` are untouched, so ordinary members can still run the flows built for them. The reads are untouched. `GET /automation/_status` still serves enablement to any authenticated caller — this change is about mutating the bit, not observing it. + +**Why enablement joined the metadata write set** + +#10145 gated the automation definition writes (`POST /`, `PUT /:name`, `DELETE /:name`) and deliberately left `toggle` out in the open, because whether disabling a flow is authoring or operating is a product call. It was filed, measured over HTTP, and ruled on 2026-08-23. + +The measurement is why "it is engine state, so leave it" did not survive: **the enabled bit is not a row, so no organization wall scopes it.** `toggleFlow(name, enabled)` writes an in-process map keyed by flow name only, `getFlowRuntimeStates()` reads that same map with no caller and no organization, and the automation service is one instance per environment. On a real, non-degraded `isolated` posture, a tenant org owner without the capability — refused 403 by `PUT /meta/:type/:name`, `POST /automation` and `DELETE /automation/:name` at the same session — switched a shipped flow off, and an unrelated tenant in a **different organization** plus the platform admin both read it off, symmetrically in both directions. Disabling a shipped flow is functionally equivalent to deleting it for as long as it stays off, and `DELETE /:name` was already gated. Mitigating but not exculpating: the override is process-local, so a cold boot reads `enabled: true` again. + +**No new capability name was minted.** The change is one arm on the existing `isFlowAuthoringWrite` predicate in `packages/runtime/src/domains/automation.ts` — the #10145 author wrote that as a single function precisely so this ruling would be one edit rather than a fourth copy of the policy. Fail-closed by construction, exactly like its three siblings: an absent `executionContext`, an absent `systemPermissions` or an empty one all refuse, and the gate runs ahead of the body checks so a refused caller learns nothing about the toggle contract. + +**Migration.** A caller that toggles flows programmatically — `client.automation.toggle(name, enabled)` — must present a principal holding `manage_metadata`; the same capability its `create` / `update` / `delete` neighbours have required since #10145. No caller of this route was found in this repo, in the Console UI (`objectstack-ai/objectui`, which posts only `/trigger` and `/resume` and merely *displays* enablement), or in the example apps, so the expected migration surface is programmatic SDK callers rather than end-user UI. diff --git a/packages/qa/dogfood/test/authz-conformance.matrix.ts b/packages/qa/dogfood/test/authz-conformance.matrix.ts index 760647ac64..7afd28f437 100644 --- a/packages/qa/dogfood/test/authz-conformance.matrix.ts +++ b/packages/qa/dogfood/test/authz-conformance.matrix.ts @@ -149,7 +149,7 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ 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; per-route capability predicates run after this floor — `manage_metadata` for the three flow-AUTHORING writes (create `POST /` / update `PUT /:name` / deregister `DELETE /:name`, selected by the one `isFlowAuthoringWrite` predicate, #10145), fail-closed by construction (an absent executionContext, an absent `systemPermissions` or an empty one all refuse) and answering 403 `PERMISSION_DENIED`, with only engine `isSystem` bypassing; the run-state reads (#7900) and `resume` (#3801 / #5561) carry their own separate per-route predicates, and the execution doors (trigger / execute / toggle) sit outside all of them', + 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; per-route capability predicates run after this floor — `manage_metadata` for the four gated flow writes (create `POST /` / update `PUT /:name` / deregister `DELETE /:name`, #10145, plus enablement `POST /:name/toggle` since the #10243 ruling of 2026-08-23, which measured that the enabled bit is not a ROW and so reaches every organization on the deployment), all selected by the ONE `isFlowAuthoringWrite` predicate, fail-closed by construction (an absent executionContext, an absent `systemPermissions` or an empty one all refuse) and answering 403 `PERMISSION_DENIED`, with only engine `isSystem` bypassing; the run-state reads (#7900) and `resume` (#3801 / #5561) carry their own separate per-route predicates, and the execution doors (trigger / execute) sit outside all of them — including `POST /trigger/:name` for a flow literally NAMED `toggle`, which the toggle arm deliberately excludes so a name cannot cost a member its run door', 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.' }, diff --git a/packages/qa/dogfood/test/automation-toggle-tenant-scope.dogfood.test.ts b/packages/qa/dogfood/test/automation-toggle-tenant-scope.dogfood.test.ts index 0973cd31cf..04025af464 100644 --- a/packages/qa/dogfood/test/automation-toggle-tenant-scope.dogfood.test.ts +++ b/packages/qa/dogfood/test/automation-toggle-tenant-scope.dogfood.test.ts @@ -1,59 +1,63 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * #10243 — the BLAST-RADIUS half of the toggle card, measured over HTTP. + * #10243 — the toggle card, now RULED: `POST /automation/:name/toggle` is + * gated on `manage_metadata`, and this file pins the closed door over HTTP. * - * ## ⛔ What this file is, and what it deliberately is not + * ## ⭐ This file was re-pointed, and the re-pointing is the record * - * It **records a measurement**. It does **not** rule. Whether - * `POST /automation/:name/toggle` belongs in the `manage_metadata` write set is - * a product and security decision for the maintainer, and nothing here argues - * either way — no severity, no recommendation. When the ruling lands, this file - * is one of the two places it lands (the other is the ungated-execution audit - * block in `packages/runtime/src/domains/automation-write-capability-gate.test.ts`): - * a ruling that toggle IS an authoring write flips these expectations to a 403, - * and the flip is the point — an unrecorded verdict cannot be revisited. + * It first landed (PR #10996) as a pure MEASUREMENT of the open half of the + * card: whether one tenant's ungated toggle reached every organization. It + * did — and it said, in this docblock, that a ruling *"flips these expectations + * to a 403, and the flip is the point — an unrecorded verdict cannot be + * revisited."* The ruling landed on 2026-08-23 (option A: toggle joins the + * `manage_metadata` write set, one arm on the existing `isFlowAuthoringWrite`, + * ⛔ no new capability name), so the flip is now taken, deliberately and in the + * same PR as the predicate change rather than left to go silently red. * - * ## The question, and why only half of it was open + * ## What was measured BEFORE the gate — the reason the ruling went this way * - * #10145 gated the automation DEFINITION writes (`POST /automation`, - * `PUT /automation/:name`, `DELETE /automation/:name`) on `manage_metadata` and - * deliberately left `toggle` ungated, on the rule that authoring and executing - * are different questions. Two separable facts follow from that, and only the - * second was ever open: + * On this same harness, with a real non-degraded `isolated` posture and two org + * owners in DIFFERENT organizations: tenant A — holding `organization_admin`, + * demonstrably NOT `manage_metadata`, and answered 403 by `PUT /meta/...`, + * `POST /automation` and `DELETE /automation/:name` at the same session — + * switched the CRM app's shipped flow off with a 200, and tenant B and the + * platform admin both read it off. Symmetrically, in both directions. * - * 1. `toggle` is reachable by any authenticated caller with no authoring - * capability — MEASURED and pinned by #10145's audit block. - * 2. flow ENABLEMENT is environment-scoped, so one tenant's toggle reaches - * every organization — asserted from the scoping #10145 measured for flow - * DEFINITIONS, and never reproduced for enablement itself. + * Mitigating but not exculpating: the override is process-local, so a cold boot + * on the same database reads `enabled: true` again. * - * This file is (2): toggle as tenant A, read the enabled state back as tenant B - * and as the platform admin — the same three principals and the same read-back - * table #10145's report used for definitions. + * ## Which legs changed, and which deliberately did NOT * - * ## ⚠️ The vacuity trap this harness has to stay clear of, stated up front + * The cross-tenant read-back was a CONSEQUENCE of the route being reachable, so + * it is not what this file pins any more — the unprivileged tenant is now + * refused at the door and never reaches the toggle at all. What survives + * untouched is the leg that measures WHERE THE STATE LIVES, because that fact + * is unchanged by the gate and is the one that made the result independent of + * this harness's admitted limitation: * * `multiTenant: 'posture-only'` activates the tenancy POSTURE and no row wall * (see `BootOptions.multiTenant`) — the enterprise `@objectstack/organizations` - * runtime is cloud-private and genuinely absent from this workspace. A fixture - * that booted this way and asserted *isolation* would assert nothing and pass. - * The mirror image is just as real and is the trap for THIS file: in a stack - * with no wall, "tenant B saw tenant A's write" is true of everything, and - * would prove nothing about a walled deployment. + * runtime is cloud-private and genuinely absent from this workspace. An + * organization wall scopes ROWS, and the enabled bit is not a row: + * `toggleFlow(name, enabled)` writes the automation engine's in-process + * `flowEnabled` map, keyed by flow name and nothing else, `getFlowRuntimeStates()` + * reads that same map with no caller, no organization and no argument at all, + * and the automation service is ONE instance per environment. `it('mutates + * ENGINE state, not the persisted definition')` below still measures exactly + * that discriminator over HTTP — and it is still the leg that would fail, + * loudly, if enablement ever became org-stamped state a wall could scope. It is + * simply driven by an ENTITLED caller now, since an unentitled one no longer + * gets that far. * - * What keeps the measurement honest is that the bit under test never reaches - * the plane a wall operates on. An organization wall scopes ROWS. The enabled - * bit is not a row: `toggleFlow(name, enabled)` writes the automation engine's - * in-process `flowEnabled` map, keyed by flow name and nothing else, and - * `getFlowRuntimeStates()` reads that same map with no caller, no organization - * and no argument at all. `it('mutates ENGINE state, not the persisted - * definition')` below measures exactly that discriminator over HTTP — after the - * toggle the flow's persisted `status` is still `active` while its runtime - * `enabled` is `false` — and it is the leg that would fail, loudly, if - * enablement ever became org-stamped state that a wall could scope. Until it - * does, no wall has anything to scope, which is why the result does not depend - * on the stand-in. + * ## ⚠️ The vacuity trap, restated for the new shape + * + * A gate test that only ever asserts refusals passes just as well when the + * route is broken, missing, or refusing everyone — which is not what was ruled. + * So the positive control is load-bearing here, not decoration: a caller WITH + * `manage_metadata` still toggles, 200, in both directions, and the engine + * state actually moves. Refusal and permission are both asserted, or neither + * means anything. * * ## The harness * @@ -108,9 +112,13 @@ async function readEnabled(stack: VerifyStack, token: string): Promise { +describe('#10243 — POST /automation/:name/toggle demands `manage_metadata`', () => { let stack: VerifyStack; - /** Platform admin — the seeded first user. #10145's `founder`. */ + /** + * Platform admin — the seeded first user. #10145's `founder`, and now also + * this file's ENTITLED principal: the positive control and the + * where-the-state-lives leg are driven through it. + */ let adminToken: string; /** Tenant A org owner — the actor. #10145's `northwind`. */ let tenantAToken: string; @@ -193,6 +201,11 @@ describe('#10243 — cross-organization reach of POST /automation/:name/toggle', // The same control #10145's report used to prove the account is not // secretly entitled. Asserts `code` AND `status` — the repo's minimum for a // refusal case, since a bare "it threw" passes for the wrong reasons. + // + // ⛔ `POST /:name/toggle` is deliberately NOT in this list even though it + // now belongs to the same write set: this control has to be independent of + // the route under test, or "tenant A is unprivileged" would be established + // by the very gate the next test measures. const meta = await stack.apiAs(tenantAToken, 'PUT', '/meta/object/crm_lead', { name: 'crm_lead' }); expect(meta.status).toBe(403); expect(((await meta.json()) as { error?: { code?: string } }).error?.code).toBe('FORBIDDEN'); @@ -214,25 +227,55 @@ describe('#10243 — cross-organization reach of POST /automation/:name/toggle', } }); - it('MEASURED: tenant A toggles the flow off, and tenant B and the platform admin both read it off', async () => { - const toggle = await stack.apiAs(tenantAToken, 'POST', `/automation/${FLOW}/toggle`, { enabled: false }); - expect(toggle.status, `toggle as tenant A: ${await toggle.clone().text()}`).toBe(200); + it('[#10243 FLIPPED] tenant A is REFUSED at the door — 403 PERMISSION_DENIED, in both directions', async () => { + // ⭐ This is the assertion the ruling inverted. It read `.toBe(200)` with a + // cross-organization read-back table under it; that table measured a + // CONSEQUENCE of the route being reachable, and the route is not reachable + // for this principal any more. Both directions are driven because the + // measurement that produced the ruling was symmetric — a gate that only + // refused "off" would leave the same environment-wide reach one boolean away. + for (const enabled of [false, true]) { + const res = await stack.apiAs(tenantAToken, 'POST', `/automation/${FLOW}/toggle`, { enabled }); + expect(res.status, `toggle {enabled:${enabled}} as tenant A: ${await res.clone().text()}`).toBe(403); + expect( + ((await res.json()) as { error?: { code?: string } }).error?.code, + 'ADR-0112 wants both halves — a 403 carrying no code satisfies exactly half the contract', + ).toBe('PERMISSION_DENIED'); + } + + // THE POINT, and the reason this is not a status-only assertion: the + // refusal has to land BEFORE the engine is touched. "Toggle first, refuse + // second" would satisfy the two assertions above and still be the + // cross-tenant defect. All three principals still read the flow ENABLED. + for (const [who, token] of [['tenantA', tenantAToken], ['tenantB', tenantBToken], ['admin', adminToken]] as const) { + expect((await readEnabled(stack, token)).enabled, `${who} after A was refused`).toBe(true); + } + }); + + it('[#10243] positive control: a `manage_metadata` holder still toggles, 200 — the gate does not refuse everyone', async () => { + // ⚠️ Load-bearing, not decoration. Every assertion in the test above passes + // just as well against a route that is broken, unmounted, or refusing + // every caller — none of which is what was ruled. The capability holder's + // 200 is the half that says a gate was installed rather than a door welded + // shut, and it is what leaves the following leg something to measure. + const toggle = await stack.apiAs(adminToken, 'POST', `/automation/${FLOW}/toggle`, { enabled: false }); + expect(toggle.status, `toggle as the entitled admin: ${await toggle.clone().text()}`).toBe(200); expect((await toggle.json()) as unknown).toMatchObject({ data: { name: FLOW, enabled: false } }); - // The read-back table. Tenant B holds no membership of tenant A's - // organization and the platform admin is org-less; both nevertheless - // observe the actor's mutation. - expect((await readEnabled(stack, tenantBToken)).enabled, 'tenant B after A toggled off').toBe(false); - expect((await readEnabled(stack, adminToken)).enabled, 'platform admin after A toggled off').toBe(false); - expect((await readEnabled(stack, tenantAToken)).enabled, 'tenant A after A toggled off').toBe(false); + expect((await readEnabled(stack, adminToken)).enabled, 'admin after the entitled toggle').toBe(false); }); it('mutates ENGINE state, not the persisted definition — the bit an organization wall has nothing to scope', async () => { - // Runs after the toggle above (file order is the sequence). The persisted - // `status` — the flow's authored metadata, the thing an org overlay could - // carry — is untouched, while the runtime `enabled` bit is off. That - // divergence is where the state lives, and it is what makes the read-back - // above independent of whether a row wall is installed. + // ⭐ KEPT, deliberately: the gate changed WHO may toggle, not WHERE the + // mutated bit lives, and this leg measures the latter. It is what made the + // original cross-tenant result independent of this harness's missing row + // wall, and it is what would fail — loudly — if enablement ever became + // org-stamped state a wall could scope. Only its driver changed: it runs + // after the ENTITLED toggle above (file order is the sequence), because an + // unentitled caller no longer gets far enough to move anything. + // + // The persisted `status` — the flow's authored metadata, the thing an org + // overlay could carry — is untouched, while the runtime `enabled` bit is off. const state = await readEnabled(stack, adminToken); expect(state.enabled).toBe(false); expect(state.status).toBe('active'); @@ -252,11 +295,26 @@ describe('#10243 — cross-organization reach of POST /automation/:name/toggle', expect(fromEngine?.enabled, 'engine state after the HTTP toggle').toBe(false); }); - it('symmetric: tenant A switches it back on, and the other two read it on again', async () => { - const toggle = await stack.apiAs(tenantAToken, 'POST', `/automation/${FLOW}/toggle`, { enabled: true }); + it('symmetric: the entitled caller switches it back on, and all three read it on again', async () => { + const toggle = await stack.apiAs(adminToken, 'POST', `/automation/${FLOW}/toggle`, { enabled: true }); expect(toggle.status).toBe(200); - expect((await readEnabled(stack, tenantBToken)).enabled, 'tenant B after A re-enabled').toBe(true); - expect((await readEnabled(stack, adminToken)).enabled, 'platform admin after A re-enabled').toBe(true); + // The environment-wide reach of the bit is unchanged and still visible — + // that was never the defect. WHO may reach it is what the ruling narrowed, + // so the same three-principal read-back is now evidence that an ENTITLED + // toggle still behaves exactly as it did. + for (const [who, token] of [['tenantA', tenantAToken], ['tenantB', tenantBToken], ['admin', adminToken]] as const) { + expect((await readEnabled(stack, token)).enabled, `${who} after the entitled re-enable`).toBe(true); + } + }); + + it('[#10243] the EXECUTION door beside it did not move — tenant A can still trigger', async () => { + // ⛔ The over-block this ruling deliberately did not make. If the gate had + // been spelled one segment too wide, an ordinary member would lose the + // ability to RUN the flows built for them — the mistake #7968 records for + // the paused-run screen read. Asserted as "not 403": what this pins is the + // authorization verdict, not the flow's business outcome. + const res = await stack.apiAs(tenantAToken, 'POST', `/automation/${FLOW}/trigger`, {}); + expect(res.status, `trigger as tenant A: ${await res.clone().text()}`).not.toBe(403); }); }); diff --git a/packages/runtime/src/domains/automation-toggle-unknown-flow.test.ts b/packages/runtime/src/domains/automation-toggle-unknown-flow.test.ts index 3f1f1fdaf4..c52761defc 100644 --- a/packages/runtime/src/domains/automation-toggle-unknown-flow.test.ts +++ b/packages/runtime/src/domains/automation-toggle-unknown-flow.test.ts @@ -53,7 +53,20 @@ function makeDispatcher(flowNames: string[] = ['welcome_flow']) { return { dispatcher: new HttpDispatcher(kernel), spies, enabled }; } -const CTX = { request: {}, executionContext: { userId: 'user_1' } } as any; +/** + * [#10243] The caller now holds `manage_metadata`. + * + * This file is about ERROR MAPPING on `POST /:name/toggle` — that an unknown + * flow is a 404 rather than a 500, and that a malformed body is a 400. Its + * `{ userId: 'user_1' }` stub encoded the premise the 2026-08-23 ruling + * destroys: toggle joined the `manage_metadata` write set, so without a + * capability every case here would stop at the 403 in front of the behaviour it + * is named after — and #7535's 404 would read as "still fixed" while nothing + * measured it. Only the caller changes; every mechanism, assertion and expected + * value below 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; describe('#7535 — toggling a flow that does not exist is 404, not 500', () => { it('answers 404 in the house error envelope, naming the unknown flow', async () => { diff --git a/packages/runtime/src/domains/automation-write-capability-gate.test.ts b/packages/runtime/src/domains/automation-write-capability-gate.test.ts index 428825cae4..776b71f823 100644 --- a/packages/runtime/src/domains/automation-write-capability-gate.test.ts +++ b/packages/runtime/src/domains/automation-write-capability-gate.test.ts @@ -28,10 +28,35 @@ * 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. + * node's `resumeAuthority` (#3801 / #5561). 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 verdicts come through this file. + * + * ## [#10243] `POST /:name/toggle` CROSSED that line — deliberately, by ruling + * + * ⭐ This is the flip, recorded here rather than left to be discovered. #10145 + * pinned toggle as ungated **in the open**, saying the verdict was a product + * call and not a code call, so that a change to it would land in this file and + * be visible. It was filed as #10243, measured over HTTP, and ruled on + * 2026-08-23: toggle joins the `manage_metadata` write set. One arm on the + * existing `isFlowAuthoringWrite`; ⛔ no new capability name (option C was + * declined). + * + * The measurement is why "it is engine state" did not carry the day. The bit is + * not a ROW, so no organization wall scopes it: `toggleFlow` writes an + * in-process map keyed by flow name only, `getFlowRuntimeStates()` reads it + * with no caller and no organization, and the automation service is one + * instance per environment. On a real non-degraded `isolated` posture, an + * unentitled tenant org owner switched a shipped flow off and an unrelated + * tenant in a DIFFERENT organization — plus the platform admin — read it off, + * symmetrically in both directions. Disabling a shipped flow is functionally + * equivalent to deleting it for as long as it stays off, and `DELETE /:name` + * was already gated. The dogfood measurement now pins the closed door: + * `packages/qa/dogfood/test/automation-toggle-tenant-scope.dogfood.test.ts`. + * + * ⚠️ This narrows the accept set — 200 → 403 for callers without the + * capability. It is a breaking change and ships as one. * * ## What the refusal cases assert * @@ -147,7 +172,16 @@ const codeOf = (response: unknown): unknown => { return r?.body?.error?.code ?? r?.body?.error?.details?.code; }; -/** The three AUTHORING writes, each with the service method it must never reach. */ +/** + * The gated writes, each with the service method it must never reach. + * + * [#10243] Four, not three: `POST /:name/toggle` joined by ruling. It is listed + * HERE rather than given a parallel block of its own so it inherits every + * direction the other three are held to — the 403 + `PERMISSION_DENIED` + * envelope, the "the service method was never entered" spy assertion, and the + * anonymous-floor-answers-first loop — instead of being pinned by whichever + * subset someone remembered to copy. + */ const AUTHORING_WRITES = [ { name: 'POST /automation (createFlow)', @@ -167,6 +201,15 @@ const AUTHORING_WRITES = [ h.dispatcher.handleAutomation(`/${FLOW}`, 'DELETE', undefined, ctx, undefined), spy: (h: Harness) => h.unregisterFlow, }, + { + // [#10243] The enablement door. `enabled: false` deliberately — the + // caller trying to switch a shipped flow OFF is the one the measurement + // caught reaching every organization on the deployment. + name: 'POST /automation/:name/toggle (toggleFlow)', + drive: (h: Harness, ctx: HttpProtocolContext) => + h.dispatcher.handleAutomation(`/${FLOW}/toggle`, 'POST', { enabled: false }, ctx, undefined), + spy: (h: Harness) => h.toggleFlow, + }, ] as const; describe('#10145 — /automation authoring writes require `manage_metadata`', () => { @@ -266,6 +309,36 @@ describe('#10145 — /automation authoring writes require `manage_metadata`', () expect(h.registered()).toEqual([]); }); + it('[#10243] POST /automation/:name/toggle still toggles, in BOTH directions', async () => { + // ⭐ The control that keeps the ruling from being read as "refuse + // everyone". A gate nobody can pass is not what was ruled — the + // capability holder's 200 is half of the change, and both + // directions are asserted because the measurement that produced + // the ruling was symmetric. + const h = boot(); + + const off = await h.dispatcher.handleAutomation(`/${FLOW}/toggle`, 'POST', { enabled: false }, AUTHOR(), undefined); + expect(statusOf(off.response)).toBe(200); + expect(h.toggleFlow).toHaveBeenLastCalledWith(FLOW, false); + + const on = await h.dispatcher.handleAutomation(`/${FLOW}/toggle`, 'POST', { enabled: true }, AUTHOR(), undefined); + expect(statusOf(on.response)).toBe(200); + expect(h.toggleFlow).toHaveBeenLastCalledWith(FLOW, true); + }); + + it('[#10243] the gate is ahead of the body checks on toggle too', async () => { + // #3899 refuses `{ enable: false }` with a 400 naming the key. An + // unentitled caller must not get that 400 — it would teach the + // toggle body contract to a caller who may not use the route, the + // same posture `POST /` already takes for the definition contract. + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}/toggle`, 'POST', { enable: false }, UNENTITLED(), undefined); + + expect(statusOf(response)).toBe(403); + expect(codeOf(response)).toBe('PERMISSION_DENIED'); + expect(h.toggleFlow).not.toHaveBeenCalled(); + }); + 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); @@ -273,6 +346,14 @@ describe('#10145 — /automation authoring writes require `manage_metadata`', () expect(statusOf(response)).toBe(200); expect(h.unregisterFlow).toHaveBeenCalledWith(FLOW); }); + + it('[#10243] `isSystem` bypasses on toggle too — the engine disables its own flows', async () => { + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}/toggle`, 'POST', { enabled: false }, SYSTEM(), undefined); + + expect(statusOf(response)).toBe(200); + expect(h.toggleFlow).toHaveBeenCalledWith(FLOW, false); + }); }); describe('the anonymous floor still answers first — this gate is the second layer', () => { @@ -307,15 +388,57 @@ describe('#10145 — /automation authoring writes require `manage_metadata`', () 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. + it('[#10243 FLIPPED] POST /:name/toggle is NO LONGER in this block — it is gated now', async () => { + // ⭐ This assertion used to read `.not.toBe(403)` and + // `toHaveBeenCalledWith(FLOW, false)`. It is inverted deliberately, + // by the 2026-08-23 ruling on #10243, and the inversion is kept in + // this block — rather than only added to the refusal loop above — + // so that the audit reads as a CHANGED verdict instead of a pin + // that quietly vanished. The full battery for this route (envelope, + // spy, anonymous floor, isSystem bypass) runs off AUTHORING_WRITES. const h = boot(); const { response } = await h.dispatcher.handleAutomation(`/${FLOW}/toggle`, 'POST', { enabled: false }, UNENTITLED(), undefined); + expect(statusOf(response)).toBe(403); + expect(codeOf(response)).toBe('PERMISSION_DENIED'); + expect(h.toggleFlow).not.toHaveBeenCalled(); + }); + + it('[#10243] the legacy EXECUTION door is not caught by the toggle arm — even for a flow named `toggle`', async () => { + // ⛔ The one over-block the ruling did not authorize. `POST + // /automation/trigger/:name` is the legacy run door, and for a flow + // literally named `toggle` its path is `/trigger/toggle` — the same + // shape the new arm matches (`parts[1] === 'toggle'`). The router + // answers that path with `execute` ABOVE the toggle arm, so the + // predicate excludes `parts[0] === 'trigger'` to match the router + // exactly. Without that exclusion this runs 403 and an ordinary + // member loses the ability to run a flow because of its NAME. + const h = boot(); + // A flow actually NAMED `toggle` has to exist, or the shared + // existence probe (#9378) answers 404 before `execute` and the + // assertion below would pass for the wrong reason — `not.toBe(403)` + // is satisfied by a 404 just as well. Seeded through the harness's + // own registry rather than over HTTP, so this fixture does not + // depend on the gate it is measuring. + h.registerFlow('toggle', { ...DEFINITION, name: 'toggle' }); + + const { response } = await h.dispatcher.handleAutomation('/trigger/toggle', 'POST', {}, UNENTITLED(), undefined); + expect(statusOf(response)).not.toBe(403); - expect(h.toggleFlow).toHaveBeenCalledWith(FLOW, false); + expect(h.execute).toHaveBeenCalled(); + expect(h.toggleFlow).not.toHaveBeenCalled(); + }); + + it('[#10243] a deeper spelling cannot slip past the arm — `/:name/toggle/anything`', async () => { + // The router's toggle arm tests `parts[1] === 'toggle'` with NO + // length check, so this path still reaches `toggleFlow`. A gate + // written as `parts.length === 2` would be narrower than its own + // route, which is a bypass rather than a style difference. + const h = boot(); + const { response } = await h.dispatcher.handleAutomation(`/${FLOW}/toggle/x`, 'POST', { enabled: false }, UNENTITLED(), undefined); + + expect(statusOf(response)).toBe(403); + expect(h.toggleFlow).not.toHaveBeenCalled(); }); it('POST /:name/runs/:runId/resume stays ungated — it is fail-closed on `resumeAuthority` (#3801/#5561)', async () => { diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 5f64e00f35..a037e92c0c 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -297,18 +297,42 @@ const FLOW_WRITE_DENY_MESSAGE = `Authoring automation flows requires the \`${FLOW_AUTHORING_CAPABILITY}\` capability.`; /** - * [#10145] Which `/automation` routes AUTHOR a flow definition. + * [#10145] Which `/automation` routes the `manage_metadata` write set covers. * * 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 + * sites is three policies that happen to agree today. [#10243] That is why the + * toggle ruling below was one arm here rather than a fourth copy of the policy. + * + * `POST /` → registerFlow (create) + * `PUT /:name` → registerFlow (update) + * `DELETE /:name` → unregisterFlow (deregister) + * `POST /:name/toggle` → toggleFlow (enablement — #10243, see below) + * + * ## [#10243] Why `toggle` joins them — ruled, not inferred + * + * #10145 left it out and said so in the open, because whether disabling a flow + * is authoring or operating is a product call rather than a code call. It was + * filed, MEASURED over HTTP, and ruled (2026-08-23). What the measurement found + * is the reason the answer is not "it is engine state, so leave it": + * + * - The bit is NOT a row, so no organization wall scopes it. `toggleFlow` + * writes an in-process map keyed by flow NAME only; `getFlowRuntimeStates` + * reads that same map with no caller, no organization and no argument; and + * the automation service is ONE instance per environment. + * - So on a real, non-degraded `isolated` posture, a tenant org owner without + * this capability switched a shipped flow off and an unrelated tenant in a + * DIFFERENT organization — and the platform admin — read it off, in both + * directions. Environment-wide reach from an unentitled caller. + * - Mitigating but not exculpating: the override is process-local, so a cold + * boot reads `enabled: true` again. + * + * Disabling a shipped flow is functionally equivalent to deleting it for as + * long as it stays off, and `DELETE /:name` is already here. No new capability + * name was minted for it (option C was declined): one predicate, one policy. + * + * ⛔ The EXECUTION routes are still 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: * @@ -318,9 +342,6 @@ const FLOW_WRITE_DENY_MESSAGE = * 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. @@ -329,7 +350,23 @@ 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; + if (method === 'POST' && parts.length === 0) return true; + // [#10243] `POST /automation/:name/toggle` — the enablement door. + // + // Matched exactly as the ROUTER matches it, not approximately, because a + // gate narrower than its route is a bypass and a gate wider than its route + // is an over-block: + // + // - No upper bound on depth. The toggle arm below tests `parts[1] === + // 'toggle'` with no length check, so `/:name/toggle/anything` still + // reaches `toggleFlow`; `parts.length === 2` here would leave exactly + // that spelling ungated. + // - `parts[0] === 'trigger'` is excluded. `POST /automation/trigger/:name` + // is the LEGACY EXECUTION door and it is answered ABOVE this domain's + // toggle arm, so for a flow literally named `toggle` the path + // `/automation/trigger/toggle` RUNS that flow. Gating it would over-block + // an execution door, which is the one thing the ruling did not do. + if (method === 'POST' && parts[1] === 'toggle') return parts[0] !== 'trigger'; // `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; @@ -751,8 +788,11 @@ async function respondToFlowTrigger( * filter — validated, #7360) * GET /:name → getFlow * POST / → createFlow (registerFlow) + * ⚑ authoring write — `manage_metadata` (#10145) * PUT /:name → updateFlow + * ⚑ authoring write — `manage_metadata` (#10145) * DELETE /:name → deleteFlow (unregisterFlow) + * ⚑ authoring write — `manage_metadata` (#10145) * POST /:name/trigger → execute (legacy: trigger/:name also supported; * unknown name → 404, disabled → 409 `FLOW_DISABLED`, * no start node → 422 `FLOW_NO_START_NODE`, a run that @@ -760,6 +800,9 @@ async function respondToFlowTrigger( * a run that PAUSED → 200 with `runId` / `screen`, * on whichever attempt it paused — #9510) * POST /:name/toggle → toggleFlow (unknown name → 404, #7535) + * ⚑ authoring write — `manage_metadata` (#10243): + * enablement is environment-wide, so an + * unentitled toggle reached every organization * GET /:name/runs → listRuns (query: limit, cursor — validated, #7300; * status — validated AND honoured, #7359) * ⚑ run-state read — `sys_automation_run` grant (#7900) @@ -838,7 +881,9 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // 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. + // the execution surfaces deliberately outside it — [#10243] `POST + // /:name/toggle` moved INSIDE it by ruling, and moved by editing that one + // predicate rather than by adding a check here. if (isFlowAuthoringWrite(parts, m)) { const refusal = refuseUngrantedFlowWrite(deps, context); if (refusal) return refusal; diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 476a2a6bbe..786dd7181e 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -72,13 +72,17 @@ const PKG_ADMIN = () => ({ request: {}, executionContext: { userId: 'u_pkg_admin /** * [#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 + * capability the metadata plane these flows live on already required. The 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. + * `domains/automation-write-capability-gate.test.ts`. + * + * [#10243] `POST /:name/toggle` uses this caller too, since the 2026-08-23 + * ruling put enablement in the same write set. The EXECUTION routes on the + * domain (trigger / resume) keep `AUTHED_CALLER`, deliberately — that half of + * the line did not move. */ const FLOW_AUTHOR = () => ({ request: {}, executionContext: { userId: 'u_flow_author', systemPermissions: ['manage_metadata'] } }) as any; @@ -392,7 +396,11 @@ describe('HttpDispatcher', () => { }); it('should toggle a flow via POST /:name/toggle', async () => { - const result = await dispatcher.handleAutomation('flow_a/toggle', 'POST', { enabled: false }, AUTHED_CALLER()); + // [#10243] `FLOW_AUTHOR`, not `AUTHED_CALLER`: toggle joined the + // `manage_metadata` write set by ruling. This case is about ROUTING + // — which service method the path reaches, with which arguments — + // so only the caller changes. + const result = await dispatcher.handleAutomation('flow_a/toggle', 'POST', { enabled: false }, FLOW_AUTHOR()); expect(result.handled).toBe(true); expect(mockAutomationService.toggleFlow).toHaveBeenCalledWith('flow_a', false); }); diff --git a/packages/runtime/src/route-ledger.ts b/packages/runtime/src/route-ledger.ts index dc4502dc26..f06d61046f 100644 --- a/packages/runtime/src/route-ledger.ts +++ b/packages/runtime/src/route-ledger.ts @@ -317,12 +317,13 @@ export const ROUTE_LEDGER: readonly RouteLedgerEntry[] = [ note: 'legacy verb-first shape; duplicates execute() against a different URL — candidates for consolidation' }, { route: 'GET /automation', domain: '/automation', disposition: 'sdk', client: 'automation.list' }, { route: 'POST /automation', domain: '/automation', disposition: 'sdk', client: 'automation.create', - note: "authored metadata, so `manage_metadata` gates it (#10145): a flow definition lives on the metadata plane (ADR-0106), and this door now asks the capability every other door onto that plane already asks. Fail-closed by construction — an absent executionContext, an absent `systemPermissions` or an empty one all fall through to the refusal, 403 with code `PERMISSION_DENIED` (ADR-0112); only engine self-invocation (`isSystem`, never settable from the wire) bypasses. WHICH routes is one predicate, `isFlowAuthoringWrite` in `domains/automation.ts` — this row plus PUT/DELETE `/:name` below, with the execution doors (trigger / execute / toggle / resume) deliberately outside it. Second layer, not the first: the #5519 anonymous floor answers an unidentified caller 401 here, not 403. Pinned in `domains/automation-write-capability-gate.test.ts`" }, + note: "authored metadata, so `manage_metadata` gates it (#10145): a flow definition lives on the metadata plane (ADR-0106), and this door now asks the capability every other door onto that plane already asks. Fail-closed by construction — an absent executionContext, an absent `systemPermissions` or an empty one all fall through to the refusal, 403 with code `PERMISSION_DENIED` (ADR-0112); only engine self-invocation (`isSystem`, never settable from the wire) bypasses. WHICH routes is one predicate, `isFlowAuthoringWrite` in `domains/automation.ts` — this row, PUT/DELETE `/:name` below, and (since the #10243 ruling) `POST /:name/toggle`, with the execution doors (trigger / execute / resume) deliberately outside it. Second layer, not the first: the #5519 anonymous floor answers an unidentified caller 401 here, not 403. Pinned in `domains/automation-write-capability-gate.test.ts`" }, { route: 'GET /automation/actions', domain: '/automation', disposition: 'sdk', client: 'automation.listActions' }, { route: 'GET /automation/connectors', domain: '/automation', disposition: 'sdk', client: 'automation.listConnectors' }, { route: 'GET /automation/_status', domain: '/automation', disposition: 'sdk', client: 'automation.getRuntimeStatus' }, { route: 'POST /automation/:name/trigger', domain: '/automation', disposition: 'sdk', client: 'automation.execute' }, - { route: 'POST /automation/:name/toggle', domain: '/automation', disposition: 'sdk', client: 'automation.toggle' }, + { route: 'POST /automation/:name/toggle', domain: '/automation', disposition: 'sdk', client: 'automation.toggle', + note: "enablement, and since the #10243 ruling (2026-08-23) `manage_metadata` gates it — the same `isFlowAuthoringWrite` door as `POST /automation` above, NOT a fourth copy of the policy. #10145 deliberately left this one out as engine state and filed the question; the measurement is what settled it. The enabled bit is not a ROW, so no organization wall scopes it: `toggleFlow` writes an in-process map keyed by flow name only, `getFlowRuntimeStates()` reads it with no caller and no organization, and the automation service is ONE instance per environment — so an unentitled tenant org owner switched a shipped flow off and an unrelated tenant in a different organization, plus the platform admin, read it off, in both directions. Disabling a shipped flow is equivalent to deleting it for as long as it stays off, and DELETE was already gated. ⚠️ BREAKING: 200 → 403 for callers without the capability. Fail-closed on an absent executionContext, an absent `systemPermissions` or an empty one, refusing 403 `PERMISSION_DENIED`, with only `isSystem` bypassing; the #5519 anonymous floor still answers 401 first. The predicate excludes `POST /automation/trigger/:name` so a flow literally NAMED `toggle` keeps its execution door. Pinned in `domains/automation-write-capability-gate.test.ts` and `qa/dogfood/test/automation-toggle-tenant-scope.dogfood.test.ts`" }, { route: 'POST /automation/:name/runs/:runId/resume', domain: '/automation', disposition: 'sdk', client: 'automation.resume', note: "generic, so the SUSPENDED NODE gates it (#3801): a pause whose descriptor declares resumeAuthority:'service' — today `approval` / `approval_revise` — answers 403 here and continues only through its owning service (ApprovalService.decide), which authorizes and records the decision first. A node type that declares NO resumeAuthority answers 403 too, fail-closed since #5561: this door is an opt-in a descriptor states with 'any'. Screen/wait pauses are unaffected because they declare it; this route is the screen-flow runner's door" }, { route: 'GET /automation/:name/runs/:runId/screen', domain: '/automation', disposition: 'sdk', client: 'automation.getScreen' },