Uh oh!
There was an error while loading. Please reload this page.
fix(runtime): gate the /automation definition writes on manage_metadata - #10242
Conversation
…ta (#10145) POST /automation, PUT /automation/:name and DELETE /automation/:name registered without any capability check, so any authenticated caller could author, modify and deregister flow definitions. Flow metadata is registered at environment scope, not organization scope, so on a walled multi-organization deployment the write crossed the tenant wall: a plain tenant org owner holding organization_admin (and answered 403 by /meta, /ai/tools and /packages) deleted a shipped flow that then read 404 for the actor, for an unrelated tenant and for the platform admin. The three definition writes now demand `manage_metadata` — ADR-0066 D1's authoring capability, the same key `PUT /meta/:type/:name` and every state-changing `/packages` route already carry. One predicate (`isFlowAuthoringWrite`), placed with the domain's anonymous floor and run-state read gate and ahead of both the service probe and body validation, so a refused caller writes nothing and cannot fingerprint whether automation is mounted. Execution routes are deliberately out of the write set: trigger (both shapes) and resume run flows rather than author them — resume is already fail-closed on resumeAuthority — and toggle mutates engine enablement, filed separately rather than folded into a security fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
… as an author (#10145) The `POST /` row of the #4058 stub-slot pin is a definition write, so the manage_metadata gate — which sits ahead of the service probe on purpose — answers it 403 before the 501 that row exists to pin. The caller is per row now: the write row authors, the execution rows keep the ordinary caller. Every assertion is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
📓 Docs Drift CheckThis PR changes 1 package(s): 43 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9a070afb7f9e81fa7c65e586d1958e5b61025389 && git checkout 9a070afb7f9e81fa7c65e586d1958e5b61025389
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 923c42470344b29a86f4bae37850e4305292c3ee c8e6be2532d0d408640c9e461f0b457dfd0ba57b && git checkout -B drift-repro 923c42470344b29a86f4bae37850e4305292c3ee && git merge --no-ff c8e6be2532d0d408640c9e461f0b457dfd0ba57b
node scripts/docs-audit/affected-docs.mjs --json 923c42470344b29a86f4bae37850e4305292c3ee
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#10145
⛔ This PR is parked OUTSIDE the merge queue, deliberately
This is an Urgent, measured, cross-tenant security fix, and it is not queued. The card carries
needs:contract-reviewbecause the change narrows the accept set on published REST/SDK surface (Clause-② — callers who write flows today will start getting 403). Under the enqueue gate that label may only be cleared by a seat at contract-review tier that is not the dispatching one, so: draft only — not flipped ready, no auto-merge armed, label not cleared, green CI notwithstanding. Saying so out loud is the point: a silent wait on an open cross-tenant hole is worse than a loud one.The defect, re-derived at head (not taken from the card)
The card is stamped at framework
8798cd2; this branch measured it at33024d905. It still reproduces, and the reproduction is in the diff: withpackages/runtime/src/domains/automation.tsreverted toorigin/mainand the new pin run, the unentitled principal is answered 200 on all three writes.The 13 that pass unfixed are the half the fix must not move: the anonymous 401 floor, the entitled-caller positive controls, and the whole execution-route audit block.
A flow definition is authored metadata registered at environment scope, not organization scope, so an ungated write is a privilege escalation and a cross-tenant one on any multi-organization deployment. The domain's own
GET /audit note already said where this belongs — "Flow definitions are metadata and are governed on the metadata plane (/meta, ADR-0106)" — and settled the READ posture while leaving the WRITE half of that same sentence unstated. This is that half.The fix
packages/runtime/src/domains/automation.ts— one predicate plus one guard clause, both modelled on the domain's existingisRunStateRead/refuseUngrantedRunReadpair rather than invented:isFlowAuthoringWrite(parts, method)—POST /,PUT /:name,DELETE /:name. One predicate, because a policy spelled at three call sites is three policies that happen to agree today.refuseUngrantedFlowWrite(deps, context)— 403PERMISSION_DENIED(ADR-0112: code and status) unless the caller holdsmanage_metadata(ADR-0066 D1) or isisSystem. The capability channel issystemPermissions, notpermissions(AI 路由的 req.user 丢失 capability 通道 —— ec.systemPermissions 从不透传,AI 域无法做任何 capability gate #4705). The message names the capability and nothing about the caller (The runtime dispatcher serialises a PermissionDeniedError'sdetailsto the client, sopositions/permissionSetsreach the browser on the/datatransport #7450).Placed with the anonymous floor and the run-state read gate, ahead of the automation-service probe and ahead of every body check, so (a) a 501-vs-403 cannot fingerprint whether a deployment mounts automation, (b) nothing is registered or unregistered before the refusal — "delete first, refuse second" is exactly what the report measured — and (c) the definition contract is not enumerable by probing 422s from outside the authoring cohort.
The write set was derived here, and execution was deliberately left out
Measured from the registrar in
domains/automation.tsand the ledger rows, not taken from the dispatch:POST /automation→registerFlowPUT /automation/:name→registerFlowDELETE /automation/:name→unregisterFlowPOST /automation/:name/trigger,POST /automation/trigger/:name→executePOST /automation/:name/runs/:runId/resume→resumeresumeAuthority(#3801 / #5561). A metadata capability in front of it would refuse the very user the flow paused for — the mistake #7968 records for the screen readPOST /automation/:name/toggle→toggleFlowGETsurfacessys_automation_rungrant (#7900)Both directions are pinned. The
stays ungatedblock in the new test is the audit that forces any future change to those four verdicts through this file.Pin sweep — whole repo, one pass
Every pin that drives an automation write, including the ones left alone and why:
Changed (caller only — every mechanism, assertion and expected value untouched):
domains/automation-body-validation.test.tsCTXgainsmanage_metadatadomains/automation-put-post-error-parity.test.tsCTXgainsmanage_metadatadomains/automation-register-error-class.test.tsCTXgainsmanage_metadatahttp-dispatcher.test.ts(3 routing cases)FLOW_AUTHORcaller;AUTHED_CALLERstays on every execution routehttp-dispatcher.test.ts(#4058 stub-slot row)POST /row authorsEach was written when any authenticated session could register a flow — i.e. the
{ userId: 'user_1' }stub encoded exactly the premise this gate destroys. Kept verbatim, deliberately: every existing refusal assertion for a genuinely illegal shape (malformed bodies, unknown keys, unknown flow names) and every anonymous-401 assertion —dispatcher-plugin.anonymous-gate.integration.test.tsandqa/dogfood/showcase-anonymous-deny-surfaces.dogfood.test.tsboth assert anonymousDELETE /automation/:nameis 401, and the new gate sits behind that floor so they are unaffected.Found and NOT changed, with the reason:
packages/client/src/client.test.ts—automation.create/update/deleteagainst a mocked fetch; asserts URL and method, no server semantics.packages/adapters/hono/src/hono.test.ts— automation coverage isGETonly.packages/verify/src/automation-*.test.ts,domains/automation-trigger-*,-resume-*,-toggle-unknown-flow,-screen-read-gate,-run-*— execution and read surfaces, outside the write set by design.packages/qa/dogfood/test/flow-*.dogfood.test.ts,showcase-declarative-mcp— register flows through the app stack, not over HTTP; theyGETand trigger only.packages/qa/dogfood/test/route-ledger-live-mount-parity.dogfood.test.ts— resolves mounted patterns; drives no automation write.packages/spec/src/api/automation-api.zod.test.ts— contract shape only; the spec declares no capability field for any route, so there is nothing to declare there.packages/qa/dogfood/test/authz-conformance.matrix.ts— itsdiscover()probe for this domain matchesshouldDenyAnonymous(only, so the new gate adds no key and the completeness ratchet is unmoved. Itsanonymous-deny-automationrow prose now under-describes the domain (the/packagessibling row names its per-route capability gates in the same field); filed rather than edited, being outside this card's declared file surface.route-ledger.tswas READ, never writtenThe card cites
route-ledger.ts:317,329,330as evidence. At head those line numbers have drifted off the write rows entirely (317 is a continuation of the legacy-trigger row; 329/330 areGET .../runsandGET /:name); the real rows are 319, 331, 332. No ledger change was needed and none was made — the row interface carries no capability field, so the gate is expressible without it. The one thing the ledger could gain is anotenaming the new gate, the way theresumerow documentsresumeAuthority; that file is under a hard serial with #10177, so it is filed for sequencing rather than taken.Verification
Gates re-derived with
node scripts/pm/dispatch-gates.mjsagainst the real diff (7 paths, three-dot vs merge base). It named five families the dispatch list could not —check:changeset-gate-self-tests,check:objectui-changeset,check-adr-0087-registration.mjs,check-changeset-no-major.mjs,check-empty-changeset.mjs— all five.changeset-triggered, and all five run. Results and the commit they were measured at are in the report comment on #10145.Generated by Claude Code