Skip to content

Commit b81da66

Browse files
fix(runtime): the resume door checks WHO is resuming — the run's starter, or the sys_automation_run read grant (#19987) (#20005)
Fixes #19987 Clause-②: no ## What this changes `POST /automation/:name/runs/:runId/resume` now checks WHO is resuming. Before, the arm validated the body and called `automationService.resume(runId, signal)` without reading any identity. So an authenticated caller holding another user's run id (`u2` on a run whose `getRun().trigger.userId` is `u1`) got `200` and `resume` ran. The resumed run then continued under the context STORED on the run (`engine.ts` `const context = run.context`): its data nodes ran as `u1`, with values `u2` submitted. The screen read on the same pause already refused `u2`. The write door now asks the screen read's question (maintainer ruling Option B on #7968, applied to the write by triage's direction `5817076635`). Both doors call one predicate, `isRunStarterOrRunStateReader` (`packages/runtime/src/domains/automation.ts`): - the caller is the run's own starter (`getRun(runId).trigger.userId`), OR - the caller holds read access to `sys_automation_run` (the operator override, asked through `mayReadRunState`, the #7900 question), OR - the caller is a system context. Everyone else gets `403 PERMISSION_DENIED`, the read twin's code and status, and `resume()` is never called, so the pause stays parked. `refuseUnrelatedScreenRead` and the new `refuseUnrelatedResume` are now two thin guard clauses over that one predicate. Each owns only its refusal sentence and WHERE it asks. The read twin's behaviour and message are byte-identical to before. ## `resumeAuthority` today, as the ruling asks (read first, per triage's binding limit) `resumeAuthority` is the closed enum `'any' | 'service'` (`packages/spec/src/automation/node-executor.zod.ts`). It names no person. Measured on `origin/main` `6bf7f1c0`: | Node type | Declared `resumeAuthority` | Where | On this REST door | | :--- | :--- | :--- | :--- | | `screen` | `'any'` | `service-automation/src/builtin/screen-nodes.ts` | the route is the intended door; "any" names no specific caller ⇒ the new gate applies | | `wait` | `'any'` | `service-automation/src/builtin/wait-node.ts` | same | | `map` | `'any'` | `service-automation/src/builtin/map-node.ts` | same (the engine still walks to a service-gated item) | | `subflow` | `'any'` | `service-automation/src/builtin/subflow-node.ts` | same (the engine still judges the child the signal lands on) | | `approval` | `'service'` | `plugin-approvals/src/approval-node.ts` | refused by the ENGINE for every caller: the service marker is a symbol no JSON body can carry | | `approval_revise` | `'service'` | `plugin-approvals/src/approval-revise-node.ts` | same | | any undeclared type | treated as `'service'` (#5561) | `engine.ts` `resolveResumeAuthority` | same | An approver decides through `ApprovalService`, which resumes in process (`approval-service.ts`: `this.automation!.resume!(runId, { ...signal, [RESUME_AUTHORITY_SERVICE]: true })`) and never enters `handleAutomationRequest`. So no caller this door admitted before is a caller the node names, and triage's limit 「不得拒绝节点自身 `resumeAuthority` 已经明确授权的人」 holds by measurement: the set of node-named callers on this door is empty. The node gate still answers, unchanged and word for word, behind the new gate (pinned: an admitted starter on an `approval` pause still gets the engine's own 403 message). ## The two doors' rules, side by side | | REST `POST /automation/:name/runs/:runId/resume` (this PR) | MCP `resume_run` (`mcp.ts` `resumeActionRun`, unchanged) | | :--- | :--- | :--- | | Run's own starter | admitted | admitted | | `sys_automation_run` read grant, not the starter | admitted (operator override, as on the screen read) | refused (starter-only by design, no operator override) | | System context | admitted | n/a | | Stranger | `403 PERMISSION_DENIED` | `404` (one not-found for unknown, finished and foreign runs) | | Node gate (`resumeAuthority`) | still applies behind this gate | still applies | | Engine answer classification | `classifyResumeResult` | `classifyResumeResult` (same table) | ## Order of operations, decided by measurement The gate runs AFTER the body checks and immediately BEFORE `resume()`. - **After the body checks.** They read nothing about the run, so every `400 VALIDATION_FAILED` is byte-identical for every caller. The gate's lookup is spent only on a request the engine would otherwise receive. - **Before `resume()`.** A refused resume consumes nothing. - **Fail-closed on a run `getRun` cannot resolve. There is no 404-first step, and this is where the write departs from the read twin.** The read twin answers 404 first because `getSuspendedScreen` is an authoritative "is there anything to disclose" check. The write has no such probe short of `resume()`, and `resume()` consumes the pause. `getRun`'s `null` is not authoritative. The engine returns `null` for a run that is parked and resumable when the durable read degrades (`engine.ts` `getRun`: "a run that is parked and resumable reports as if it had never run"). It also returns `null` after the 1000-entry in-memory ring evicts the entry on a deployment with no suspended-run store (`DEFAULT_MAX_EXECUTION_LOG_SIZE`, the `splice` in `recordLog`), and a caller who can start runs can cause that eviction. Letting `null` through would open the gate in exactly those states. - **What moves, and for whom.** Only for a caller who is neither the run's starter, nor a grant holder, nor a system context: an unknown or finished run now answers them this `403` instead of the engine's `404`. As a side effect, a stranger can no longer tell a paused run id from an unknown one here. For every caller the gate admits, every engine answer is unchanged, including the `404` for unknown and finished runs (both pinned). One cost falls on honest callers: a caller WITHOUT the grant who mistypes a run id now gets `403` rather than `404`. ## Tests All readings are on HEAD `b4cb8a4d`, unless a line says otherwise. - **Failing probe first** (the card's named probe, committed as a test). `automation-resume-caller-gate.test.ts` was run against the unfixed arm (base `6bf7f1c0`) and gave `Tests 17 failed | 33 passed (50)`. The stranger `u2` on `u1`'s run got `200`, with `resume` called. - **New suite** `packages/runtime/src/domains/automation-resume-caller-gate.test.ts`: 50 cases. Every refusal row asserts the envelope `code` AND `status`, and asserts that `resume` was NOT called: - stranger refused, with the exact message, and nothing about the caller's authorization topology in the refusal (#7450); - a run with no `trigger.userId` refused; - a run `getRun` cannot resolve refused; - a `getRun` throw refused, even for the starter; - a service with no `getRun`: refused unless the grant admits; - permission subsystem down: stranger refused, starter admitted; - starter admitted with NO grant: the over-block guard; the signal is forwarded field by field and `explain` is never called; - grant holder admitted: `explain` is asked for `read` on `sys_automation_run` with the caller's own context; - system context admitted without asking anything; - the node gate's own 403 still arrives word for word for an admitted starter; - unchanged answers: starter on a finished run gets the engine's 404; grant holder on an unknown id gets the engine's 404; a malformed body is refused `VALIDATION_FAILED` before the gate for every caller; anonymous gets `401` first; no security service, or a partial one, admits; - a parity table (3 callers × 5 security postures × 2 runs = 30 cases) that drives BOTH doors with the same inputs and requires the same verdict, and on refusal the same `code`, the same `status` and the same requirement sentence. - **Rewritten pin.** `automation-screen-read-gate.test.ts` had a case pinning the defect itself ("leaves `resume`'s own authority checks alone": stranger not 403, `resume` called). It is replaced by two cases: the same stranger is refused at `resume` (same code and status, `resume` not called), and the triggering user is admitted with no grant. Comments in three sibling suites that said the resume door had no route-level gate are corrected. Their assertions are unchanged and still green. - **Mutation leg** (`node scripts/ablation-replace.mjs`, on committed `a99d03a4`, the gate's code identical at HEAD). It replaced `if (unrelated) return unrelated;` with an inert statement: anchor 1 → 0, blob `467c4019f123` → `e8f9a4915391`. With the gate removed, the two gate suites went `Tests 15 failed | 57 passed (72)`: every stranger and unresolved row turned red, and every starter, grant and system row stayed green. The direction is red, as expected. Restore: blob == HEAD (`467c4019f123`) and `git diff HEAD` empty. The suite imports the arm by relative source path, so no `dist/` is involved. - `pnpm --filter @objectstack/runtime exec vitest run --project local --maxWorkers=2` (on `a99d03a4`; the later commit changes only a ledger note string and a docs table row): `Test Files 276 passed (276)`, `Tests 3893 passed | 1 skipped`. - On HEAD `b4cb8a4d`, the nine suites that read the route ledger or drive the resume arm (`route-ledger.conformance`, `http-dispatcher`, `discovery-schema-conformance`, `auth-forward-fault-sanitization` and the five automation gate suites) were re-run: `Test Files 9 passed (9)`, `Tests 436 passed (436)`. - `--project repo`: `Test Files 2 passed`, `Tests 69 passed`. - `pnpm --filter @objectstack/runtime typecheck`: exit 0. Output: `check:test-typecheck: OK — @objectstack/runtime's test layer compiles`, and the new suite is not in the debt ledger. - `pnpm --filter @objectstack/dogfood exec vitest run test/flow-durable-suspend.dogfood.test.ts` against the rebuilt runtime `dist/` (it carries `isRunStarterOrRunStateReader`): `Tests 11 passed (11)`. That includes the real-engine resume after a real cold boot. - **Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on HEAD derives 92 commands. All 92 were run with exit codes captured before any pipe, and all exited 0. `--ran` reconciliation: `92 derived, 92 run, 0 NOT-MEASURED, 0 UNRUN` ("a DERIVED zero — all 92 recorded an exit code and none of them is 3"). Two runs needed a prerequisite first. `check:dual-build-cjs-loads` and `check:type-check-debt` refused with exit 3 (no `dist/`), and `check:skill-examples` refused the same way (no client-react `.d.ts`). A full `turbo run build` then ran (`73 successful, 73 total`), and all three were re-run green before the final union. - **Roster gates** whose rosters sit under a touched directory: `check:authz-resolver`, `check:error-code-casing`, `check:filter-alias-parity`, `check:route-ledger-census`, and also `check:error-status-conformance`. All exit 0. - `pnpm lint` (the full union, `eslint . --no-inline-config`): exit 0 in 112s. Two gates were red on the first pass, and both reds were caused by this diff: - `check:system-context-census`: the `isSystem` read moved from `refuseUnrelatedScreenRead` into the shared predicate. The row in `content/docs/permissions/system-context.mdx` is re-anchored and now names the resume door. - `check:doc-authoring`: tracker ids in the route-ledger note string. They are removed from the string. ## Docs `content/docs/automation/flows.mdx`: - "Who may resume" now has the caller question first and the node question second. It states the three admitted callers, the refusal, the ordering and the MCP door's stricter rule. - The pause table's `wait` (signal) row no longer says "any caller". ## Deviations, stated rather than chosen silently 1. **Refusal sentence.** The code and status are the read twin's. The requirement clause is the read twin's, word for word: both messages are built from one constant, `PAUSED_RUN_REQUIREMENT`. The verb phrase is this door's own: "Resuming a paused run requires being the identity that triggered the run, or read access to 'sys_automation_run'." A resume refused with "Reading a paused run's screen …" would name an operation the caller did not attempt. That is the misdirection #11666 took off the toggle door. If the literal read sentence is wanted instead, it is a one-line change: `RESUME_DENY_MESSAGE = SCREEN_READ_DENY_MESSAGE`. 2. **Unknown and finished runs** keep today's answers for every caller the gate admits, not for every caller. The reasons are in "Order of operations" above. 3. **Two files beyond the claim's listed file surface.** `packages/runtime/src/route-ledger.ts`: the resume row's `note` now describes the caller gate. `content/docs/permissions/system-context.mdx`: `check:system-context-census` refuses the tree until that row re-anchors. ## Acceptance notes (observations, not filed) - **Consequence of Option B, for the maintainer's eye.** A caller that resumes runs it did NOT start now needs `sys_automation_run` read. Examples: an integration feeding a `wait` node's signal, or support tooling. The changeset says so. No in-repo caller does this: the REST resume callers on `main` are the route mount, the client SDK method and the tests. - `WaitExecutorConfigSchema.webhookUrlPattern` defaults to `/api/v1/automation/resume/{executionId}/{nodeId}`, a path no route serves, and it has zero runtime readers. It is dormant, not filed. Carrier: none. - The client SDK's `automation.resume` docblock lists the resume refusals but not this 403. The list is incomplete, not wrong. Not filed. Carrier: none. ## NOT MEASURED locally (CI owns them) - Test Core shards, Temporal Conformance, the Dogfood Regression Gate beyond the one suite above, and Dogfood Verify CLI. - The workspace, examples and downstream type-check lanes: runtime's own `typecheck` ran. The public surface is unchanged (no export added or removed), so no importer owes a test. - The artifact-roster, wide-population and path-scheduled CI steps that `dispatch-gates` lists outside its derived total. --- _Generated by [Claude Code](https://claude.ai/code/session_01TnPAC1UsTGfHPXVUCL6iLn)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 61609ed commit b81da66

10 files changed

Lines changed: 737 additions & 77 deletions
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): `POST /automation/:name/runs/:runId/resume` checks WHO is resuming — the run's own starter, or the `sys_automation_run` read grant (#19987)
6+
7+
Clause-②: no
8+
9+
**The defect.** The resume route checked the body and then resumed. It never asked who the caller was. An authenticated user holding another user's run id could continue that user's paused run, and the run then went on under the identity STORED on the run: its data nodes ran as the user who started it, with the values the other user submitted. The screen read on the same pause, `GET /automation/:name/runs/:runId/screen`, already refused that caller.
10+
11+
**The fix.** The route now asks the screen read's own question, through the same function: the caller must be the user who started the run (`getRun(runId).trigger.userId`), OR hold read access to `sys_automation_run` (the operator override), OR be a system context. Anyone else gets `403 PERMISSION_DENIED`, the screen read's code and status, and the run is not touched: nothing reaches the engine, so the pause stays parked for the person it is waiting on. The message states the screen read's requirement word for word under this route's own verb: "Resuming a paused run requires being the identity that triggered the run, or read access to 'sys_automation_run'."
12+
13+
**What stays the same.**
14+
15+
- The user who started a run resumes it exactly as before, with no grant needed, and still does while the permission subsystem is down.
16+
- The suspended node's own gate (`resumeAuthority`) is unchanged and still answers behind this one. An `approval` pause is still refused on this route for every caller; approvers decide through the approvals API, which never uses this route.
17+
- Body checks come first, as before, so every `400` for a malformed body is unchanged for every caller.
18+
- For the starter, a `sys_automation_run` reader and a system context, every engine answer is unchanged, including the `404` for an unknown or finished run.
19+
20+
**Who needs to act.** A caller that resumes runs it did NOT start (for example an integration that feeds a `wait` node's signal, or a support tool) now needs read access to `sys_automation_run`. Without it, it gets the `403` above. Such a caller also gets that `403`, not the old `404`, for a run id that does not resolve. The check fails closed there because a run that is still paused can read as "not found" while the run store is degraded.

‎content/docs/automation/flows.mdx‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -764,7 +764,7 @@ POST /api/v1/automation/{flow}/runs/{runId}/resume
764764
| `approval` | a human decision | the approvals service (`POST /api/v1/approvals/requests/:id/approve\|reject`) — resumes down the matching `approve` / `reject` edge. **Decide through the approvals API**; the resume route above **refuses** an approval pause outright (see below). |
765765
| `screen` | a user submits the form | the UI runner posting the collected `inputs` — **validated server-side against the screen's declared `fields`** (see below); a `paused` response carrying the next `screen` chains multi-step wizards under one stable `runId` |
766766
| `wait` (timer) | an ISO-8601 duration elapses | **automatically** — a one-shot job resumes the run; after a cold boot the engine re-arms pending timers from the durable store (overdue timers resume immediately) |
767-
| `wait` (signal) | a named external event | any caller invoking `resume(runId)` |
767+
| `wait` (signal) | a named external event | the external producer calling the resume route — as the user who started the run, or with read access to `sys_automation_run` (see below); in process, `resume(runId)` on the automation service |
768768

769769
<Callout type="warn">
770770
**A pause ends the flow's retry-governed segment.** `errorHandling: { strategy:
@@ -807,11 +807,37 @@ change (there is no `FLOW_STRANDED`), and the trigger route's `400` carries
807807
referent there. The shape is `ResumeFailureDetailsSchema` in
808808
`@objectstack/spec/api`.
809809

810-
### Who may resume — the gate is the suspended node
811-
812-
The resume route is generic, so **the node the run is parked on** decides
813-
whether a raw resume is a legitimate continuation. Every action descriptor
814-
carries a `resumeAuthority`:
810+
### Who may resume — the caller, then the suspended node
811+
812+
The route asks two questions, in this order. Both must pass.
813+
814+
**1. Who is resuming.** A resumed run continues under the identity stored on
815+
the run, so its data nodes run as the user who **started** it, whoever sent the
816+
values. The route therefore admits only:
817+
818+
- the user who started the run (the run's `trigger.userId`) — no extra
819+
permission needed;
820+
- a caller with **read access to `sys_automation_run`** — the operator
821+
override, for support tooling or an integration that feeds a signal `wait` on
822+
runs it did not start;
823+
- a system context.
824+
825+
Anyone else gets **403** `PERMISSION_DENIED` — "Resuming a paused run requires
826+
being the identity that triggered the run, or read access to
827+
'sys_automation_run'." — and nothing reaches the engine, so the run stays
828+
parked for the person it is waiting on. This is the same rule, code and status
829+
as reading the run's screen (`GET /api/v1/automation/{flow}/runs/{runId}/screen`),
830+
checked by the same function. The check runs after the body checks, so a
831+
malformed body is still a `400` for every caller. If the run id does not
832+
resolve, only an admitted caller (a system context or a `sys_automation_run`
833+
reader) gets the engine's `404`. Everyone else gets the `403`, because a run
834+
that is still paused can read as "not found" while the run store is degraded.
835+
The MCP `resume_run` tool is stricter: only the user who started the run, with
836+
no operator override (see [actions as tools](/docs/ai/actions-as-tools)).
837+
838+
**2. What the run is parked on.** The resume route is generic, so **the node
839+
the run is parked on** decides whether a raw resume is a legitimate
840+
continuation. Every action descriptor carries a `resumeAuthority`:
815841

816842
- **`'any'`** (`screen`, `wait`): the caller supplies the continuation and the
817843
route is the intended door.

‎content/docs/permissions/system-context.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ The largest single consumer — **17 of the 112 sites**.
176176
| 55 | Package REST route capability gate bypassed | rest | Get: a marketplace publish over REST (`POST /packages/publish`, the one route the REST registrar mounts since #14503) without `manage_metadata`; the package read cohort (`studio.access` / `setup.access`) is enforced by the dispatcher `/packages` domain's own read gate, where the reads are served | `packages/rest/src/package-routes.ts#refusePackageRequest` |
177177
| 56 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `packages/runtime/src/domains/packages.ts#requireManageMetadata`, `#requireReadCapability` |
178178
| 57 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `packages/runtime/src/domains/activation-gate.ts#refuseUngrantedActivationWrite`, `#refuseUngrantedActivationAuthoring` |
179-
| 58 | Automation run-state read, flow-authoring write, unrelated-screen read and the two operator run-lifecycle writes all pass | runtime | Get: run state, flow writes, screen reads with no grant — and cancelling or restoring a suspension without the platform-operator rung. The lifecycle bypass is the in-process owner's door: plugin-approvals' revise-window recall (ADR-0044) cancels on behalf of a decision it already authorized and recorded | `packages/runtime/src/domains/automation.ts#mayReadRunState`, `#refuseUngrantedFlowWrite`, `#refuseUnrelatedScreenRead`, `#refuseUngrantedRunLifecycleWrite` |
179+
| 58 | Automation run-state read, flow-authoring write, the paused-run caller gate (screen read and resume) and the two operator run-lifecycle writes all pass | runtime | Get: run state, flow writes, a paused run's screen and its resume with no grant and without being the run's starter — and cancelling or restoring a suspension without the platform-operator rung. The screen read and the resume ask one predicate, so this is one bypass for both. The lifecycle bypass is the in-process owner's door: plugin-approvals' revise-window recall (ADR-0044) cancels on behalf of a decision it already authorized and recorded | `packages/runtime/src/domains/automation.ts#mayReadRunState`, `#refuseUngrantedFlowWrite`, `#isRunStarterOrRunStateReader`, `#refuseUngrantedRunLifecycleWrite` |
180180
| 59 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts#assertTenantAdmin` |
181181
| 60 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `packages/plugins/plugin-email/src/email-template-provenance.ts#bindEmailTemplateProvenanceStamp`, `packages/plugins/plugin-webhooks/src/webhook-provenance.ts#bindWebhookProvenanceStamp` |
182182
| 61 | **Automation flow data nodes re-add the `owner_id` stamp** (the one place row 2's gap is compensated inline) | service-automation | Get: a flow-authored INSERT under system elevation still lands owned, when the run resolved a user. Fill-only — flow-authored values win | `packages/services/service-automation/src/runtime-identity.ts#stampSystemInsertOwner`, called from `packages/services/service-automation/src/builtin/crud-nodes.ts#registerCrudNodes` |

0 commit comments

Comments
 (0)