Skip to content

Commit 57a3bb3

Browse files
authored
fix(automation,approvals): gate the generic run-resume route on the suspended node (#3801) (#3822)
The resume route forwarded a caller-supplied signal straight into AutomationEngine.resume, which validated machine state only — nothing asked who. Approval nodes resume through that mechanism, so a raw resume walked the approve edge with no approver check, no sys_approval_action row and no status mirror. The gate keys on what the run is parked on: ActionDescriptor.resumeAuthority ('any' | 'service'), a suspension that records its own node type, and an unforgeable symbol marker the owning service stamps. It follows subflow pauses down to the child the signal would reach, and refuses with code 'forbidden' → 403 without consuming the suspension. Adjacent revise-window gap filed as #3823.
1 parent 0f8ad09 commit 57a3bb3

22 files changed

Lines changed: 814 additions & 31 deletions

File tree

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-automation": minor
4+
"@objectstack/plugin-approvals": minor
5+
"@objectstack/runtime": minor
6+
"@objectstack/client": patch
7+
---
8+
9+
fix(automation,approvals): the run-resume route is gated by the node the run is parked on (#3801)
10+
11+
`POST /api/v1/automation/:name/runs/:runId/resume` forwarded a caller-supplied
12+
`{ inputs, output, branchLabel }` straight into `AutomationEngine.resume`, and
13+
`resumeInternal` validated **machine state only** — the concurrent-resume latch,
14+
the run exists, the flow exists, the suspended node still exists. Nothing asked
15+
*who was calling*.
16+
17+
Approval nodes suspend and resume through exactly that mechanism. So a resume
18+
carrying `branchLabel: 'approve'` walked the approve edge with **no approver
19+
check, no `sys_approval_action` row and no status mirror** — the
20+
`sys_approval_request` row and the run then disagreed permanently. The only
21+
thing standing between the route and the approvals rules was convention; the
22+
showcase spelled it out in a comment ("decide via the approvals API, never a raw
23+
engine `resume`"), and a comment in an example is not an access control.
24+
25+
Removing the route was not the fix: it is load-bearing for **screen flows**
26+
the UI flow-runner posts `{ inputs }` there to advance a paused `screen` node.
27+
The gate therefore keys on **what the run is parked on**:
28+
29+
- `ActionDescriptor.resumeAuthority` (`'any'` | `'service'`, default `'any'`) —
30+
a pausing node declares who may continue it. `approval` declares `'service'`.
31+
- The engine refuses a `'service'` suspension unless the signal carries
32+
`RESUME_AUTHORITY_SERVICE` (`@objectstack/spec/contracts`), a **symbol** the
33+
owning service stamps in-process — a JSON body can never produce one, so the
34+
transport cannot forge it. `ApprovalService` stamps it on the tail of a
35+
decision it has already authorized and recorded.
36+
- The gate follows a **subflow** pause down to the child the signal would
37+
actually reach, so resuming the parent is not a way around it.
38+
- Refusal returns `{ success: false, code: 'forbidden' }` and the route answers
39+
**403**. Nothing is consumed — the request stays pending and the run stays
40+
parked, so the real decision still lands.
41+
42+
`screen` and `wait` pauses are unchanged, as is every path that already went
43+
through the approvals API. What changes for consumers:
44+
45+
- **FROM:** finishing an approval with
46+
`client.automation.resume(flow, runId, { branchLabel: 'approve' })`
47+
**TO:** `client.approvals.approve(requestId, …)` (or `.reject` / `.recall`).
48+
The old call now answers 403 and changes nothing.
49+
- Registering your own pausing node whose continuation belongs to a service
50+
rather than to whoever holds the run id? Declare `resumeAuthority: 'service'`
51+
on its descriptor and stamp `RESUME_AUTHORITY_SERVICE` on the signal from that
52+
service.
53+
54+
A suspension now records the node type that produced it
55+
(`SuspendedRun.nodeType` / `sys_automation_run.node_type`), captured at suspend
56+
time so a flow republished mid-pause cannot re-type the node out from under the
57+
gate; rows written before this fall back to the flow definition.

content/docs/automation/approvals.mdx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,13 @@ curl -b cookies.txt -X POST \
288288
`actorId` defaults to the caller. The actor **must** be in `pending_approvers`
289289
or the call returns 403 (`FORBIDDEN: actor '…' is not a pending approver`); a
290290
request that isn't pending returns 409 (`INVALID_STATE`). Always go through
291-
these endpoints — never resume the flow run directly.
291+
these endpoints — never resume the flow run directly, and since #3801 you
292+
**cannot**: `POST /api/v1/automation/{flow}/runs/{runId}/resume` answers 403 for
293+
a run parked on an `approval` node (including via a `subflow` pause) and changes
294+
nothing — the request stays pending and the run stays parked, so the real
295+
decision still lands. The approver slate, the `sys_approval_action` row and the
296+
status mirror all live on this path; it is the only one that produces a
297+
consistent outcome.
292298

293299
A decision may carry **file attachments**`attachments: string[]` of `sys_file`
294300
ids — recorded on its audit row (e.g. a signed contract on the approve):

content/docs/automation/flows.mdx

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,11 +389,34 @@ POST /api/v1/automation/{flow}/runs/{runId}/resume
389389

390390
| Pausing node | Suspends until… | Resumed by |
391391
| :--- | :--- | :--- |
392-
| `approval` | a human decision | the approvals service (`POST /api/v1/approvals/requests/:id/approve\|reject`) — resumes down the matching `approve` / `reject` edge. **Always decide through the approvals API**, never by calling `resume` directly: a direct resume strands the pending `sys_approval_request`, so the record stays locked and later approvals on the same record hit `DUPLICATE_REQUEST`. |
392+
| `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). |
393393
| `screen` | a user submits the form | the UI runner posting the collected `inputs`; a `paused` response carrying the next `screen` chains multi-step wizards under one stable `runId` |
394394
| `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) |
395395
| `wait` (signal) | a named external event | any caller invoking `resume(runId)` |
396396

397+
### Who may resume — the gate is the suspended node
398+
399+
The resume route is generic, so **the node the run is parked on** decides
400+
whether a raw resume is a legitimate continuation. Every action descriptor
401+
carries a `resumeAuthority`:
402+
403+
- **`'any'`** (the default — `screen`, `wait`, your own pausing nodes): the
404+
caller supplies the continuation and the route is the intended door.
405+
- **`'service'`** (`approval`): continuing is a *side effect* of a decision
406+
that some service must authorize and record first, so only that service may
407+
drive it. `ApprovalService.decide` enforces the approver slate, writes the
408+
`sys_approval_action` row, mirrors the status field — **then** resumes.
409+
410+
A resume of a `'service'` pause through the route answers **403** and changes
411+
nothing: the request stays pending and the run stays parked, so the real
412+
decision can still land. (Before this gate a raw resume walked the `approve`
413+
edge with no decision recorded, leaving the `sys_approval_request` row and the
414+
run permanently disagreeing.) The gate follows a **subflow** pause down to the
415+
child the signal would actually reach, so resuming the parent is no way around
416+
it. Registering a pausing node of your own? Declare
417+
`resumeAuthority: 'service'` on its descriptor when the decision to continue
418+
belongs to your service rather than to whoever holds the run id.
419+
397420
### Parallel approvals — one aggregating node, not two pauses
398421

399422
"Finance **and** legal must both sign off, concurrently" is **one `approval`
@@ -469,7 +492,8 @@ the chain resolves from either end:
469492
`${nodeId}.output` / `outputVariable` mapping as a synchronous subflow;
470493
- resuming the **parent** run id (what a UI holds from the original launch)
471494
**delegates** down to the suspended child — multi-screen child wizards keep
472-
the parent run id stable.
495+
the parent run id stable. The resume gate delegates with it: if the child is
496+
parked on an `approval`, resuming the parent is refused too.
473497

474498
A child that fails terminally after the pause fails every waiting ancestor, so
475499
no run is stranded as resumable-forever. See the worked example pair in the

content/docs/references/automation/node-executor.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ Canonical cross-paradigm action/node descriptor (ADR-0018)
7676
| **supportsRetry** | `boolean` || Supports retry on failure |
7777
| **needsOutbox** | `boolean` || Dispatch via service-messaging outbox (retry/idempotency/dead-letter) |
7878
| **isAsync** | `boolean` || Suspends the flow awaiting an external reply |
79+
| **resumeAuthority** | `Enum<'any' \| 'service'>` || Who may resume a run this node suspended: 'any' (the generic resume route) or 'service' (only the owning service, e.g. approvals) |
7980
| **maturity** | `Enum<'ga' \| 'beta' \| 'reserved'>` || Runtime maturity: ga (shipped), beta, or reserved (contract only — designers grey this out) |
8081
| **source** | `Enum<'builtin' \| 'plugin'>` || builtin = platform baseline; plugin = third-party contributed |
8182
| **deprecated** | `boolean` || Deprecated alias kept for back-compat |

docs/adr/0019-approval-as-flow-node.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,49 @@ not catch a *post-pause* child failure (the parent run fails terminally instead)
195195
does not count across a suspension; a crash exactly between child completion and the parent
196196
bubble leaves the parent paused — an operator can compensate with a manual
197197
`resume(parentRunId, { output })` (outbox-grade exactly-once chaining is future work).
198+
199+
## Addendum (2026-07-28, #3801) — the resume seam is a trust boundary: `resumeAuthority`
200+
201+
Collapsing approval onto the flow engine made `AutomationEngine.resume` the **one** way any pause
202+
continues — including an approval decision. That is the right execution model, but it left the
203+
authorization model behind: `POST /api/v1/automation/:name/runs/:runId/resume` forwards a
204+
caller-supplied `{ inputs, output, branchLabel }` straight through, and `resumeInternal` validated
205+
**machine state only** (the concurrent-resume latch, the run exists, the flow exists, the suspended
206+
node still exists). Nothing asked *who*. A raw resume with `branchLabel: 'approve'` therefore walked
207+
the approve edge with **no approver check, no `sys_approval_action` row and no status mirror**
208+
leaving the `sys_approval_request` row and the run permanently disagreeing. The only thing standing
209+
between the route and the approvals rules was convention, spelled out in a showcase comment. A
210+
comment in an example is not an access control.
211+
212+
Deleting the route was never an option: it is load-bearing for **screen flows** — the UI flow-runner
213+
posts `{ inputs }` to advance a paused `screen` node, which is its whole reason to exist. So the gate
214+
discriminates by **what the run is parked on**, not by the route:
215+
216+
- **`ActionDescriptor.resumeAuthority`** (ADR-0018 registry, `'any'` | `'service'`, default `'any'`).
217+
A pausing node declares who may continue it. `approval` declares `'service'`; `screen` / `wait`
218+
keep the default and behave exactly as before.
219+
- **The suspension carries its own node type.** `SuspendedRun.nodeType` (column
220+
`sys_automation_run.node_type`) is captured at suspend time rather than re-derived from the live
221+
flow, so a flow republished mid-pause cannot re-type the node out from under the gate. Rows
222+
written before this shipped fall back to the flow definition.
223+
- **The marker is a symbol.** `RESUME_AUTHORITY_SERVICE` (`@objectstack/spec/contracts`) is stamped
224+
on the `ResumeSignal` by the owning service — `ApprovalService` does it in one place
225+
(`serviceResume`), on the tail of a decision it already authorized and recorded. The transport
226+
builds its signal out of a JSON body, and no JSON body can produce a symbol-keyed property, so the
227+
route cannot forge it; the gate does not need to trust the caller's claims about itself.
228+
- **The gate follows the subflow chain.** A parent parked on a `subflow` node *delegates* the signal
229+
to its suspended child (see the 2026-06-10 addendum), so the gate resolves the effective suspension
230+
first and judges the node the signal actually lands on. Resuming the parent is not a way around it.
231+
- **Refusal is an authorization answer.** `resume` returns `{ success: false, code: 'forbidden' }`
232+
and the route answers **403** — not a 200 carrying `success: false`, which reads as "your resume
233+
ran and the flow failed". Nothing is consumed: the request stays pending and the run stays parked,
234+
so the real decision still lands.
235+
236+
Engine-internal continuations (subflow delegation and up-bubble, `map` re-entry, the wait-timer wake)
237+
go through the private `resumeInternal` and are **not** re-gated — they continue work an
238+
already-authorized call started.
239+
240+
**Known gap, tracked separately:** ADR-0044's revise window parks the run on an ordinary `wait` node
241+
(signal flavor) placed by the flow author, which the type-keyed gate cannot distinguish from any
242+
other wait. A raw resume there still forces a resubmit without a `sys_approval_action` row. Filed as
243+
a follow-up — closing it needs a per-suspension owner claim rather than a node-type one.

examples/app-showcase/src/automation/flows/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -987,8 +987,10 @@ export const ResilientSyncFlow = defineFlow({
987987
* rejection resumes down `reject`. One node, one suspend/resume, no token tree —
988988
* the multi-instance pattern Camunda and Step Functions use for exactly this.
989989
*
990-
* Decide via the approvals API (never a raw engine `resume`):
991-
* POST /api/v1/automation/showcase_invoice_signoff/runs/{runId}/... ← no
990+
* Decide via the approvals API — a raw engine `resume` is refused, not merely
991+
* discouraged: the `approval` node declares `resumeAuthority: 'service'`, so
992+
* the generic run-resume route answers 403 for a run parked on one (#3801).
993+
* POST /api/v1/automation/showcase_invoice_signoff/runs/{runId}/resume ← 403
992994
* POST /api/v1/approvals/requests/{id}/approve { actorId: 'position:finance' }
993995
* POST /api/v1/approvals/requests/{id}/approve { actorId: 'position:legal' } ← now it continues
994996
*/

packages/client/src/index.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2720,7 +2720,7 @@ export class ObjectStackClient {
27202720
return this.unwrapResponse(res) as Promise<T>;
27212721
},
27222722
/**
2723-
* Resume a run suspended at a `screen` (or `approval`) node — the
2723+
* Resume a run suspended at a `screen` (or `wait`) node — the
27242724
* screen-flow runtime's second half (ADR-0019 durable pause).
27252725
*
27262726
* `execute()` returns `{ status: 'paused', runId, screen }` when a flow
@@ -2729,6 +2729,13 @@ export class ObjectStackClient {
27292729
* NEXT `{ status: 'paused', screen }` of a multi-step wizard or the
27302730
* terminal `AutomationResult`. Without this method a paused run can only
27312731
* be finished by hand-rolling the HTTP call (#3528).
2732+
*
2733+
* **Not the door for an approval (#3801).** A run parked on an
2734+
* `approval` node — directly, or as the child of a `subflow` pause — is
2735+
* resumable only through the approvals API
2736+
* ({@link ObjectStackClient.approvals}: `approve` / `reject` / `recall`),
2737+
* which authorizes the decision and records it first. This call answers
2738+
* **403** for one and changes nothing.
27322739
*/
27332740
resume: async <T = any>(
27342741
flowName: string,
@@ -4423,9 +4430,10 @@ export class ScopedProjectClient {
44234430
return this.parent._unwrap<T>(res);
44244431
},
44254432
/**
4426-
* Resume a run suspended at a `screen` / `approval` node with the collected
4433+
* Resume a run suspended at a `screen` / `wait` node with the collected
44274434
* input (ADR-0019 durable pause). Mirrors the unscoped
4428-
* `client.automation.resume`.
4435+
* `client.automation.resume` — including its refusal (403) to resume an
4436+
* `approval` pause, which belongs to the approvals API (#3801).
44294437
*/
44304438
resume: async <T = any>(
44314439
flowName: string,

packages/plugins/plugin-approvals/src/approval-node.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,46 @@ describe('Approval node bridge (ADR-0019)', () => {
154154
expect(paused.runId).toBeDefined();
155155
});
156156

157+
// ── resume authorization gate (#3801) ───────────────────────────────
158+
//
159+
// `POST /automation/:name/runs/:runId/resume` reaches
160+
// `AutomationEngine.resume` with a caller-supplied signal. Before the gate,
161+
// the only thing between that route and the approvals rules was convention —
162+
// a comment in the showcase. These pin the enforcement.
163+
164+
it('declares the approval node resumable only by its owning service', () => {
165+
const approval = automation.getActionDescriptors().find(d => d.type === 'approval');
166+
expect(approval!.resumeAuthority).toBe('service');
167+
});
168+
169+
it('refuses a raw engine resume of an approval pause, leaving the request untouched', async () => {
170+
registerDecisionFlow(automation, [{ type: 'user', value: 'u1' }]);
171+
const paused = await automation.execute('deal_approval', {
172+
object: 'crm_deal', record: { id: 'd1' }, userId: 'submitter',
173+
});
174+
const request = (await fake.find('sys_approval_request', { where: { status: 'pending' } }))[0];
175+
176+
// Exactly the signal the resume route builds from `{ branchLabel }`.
177+
const refused = await automation.resume(paused.runId!, {
178+
branchLabel: 'approve', output: { decision: 'approve' },
179+
});
180+
181+
expect(refused).toMatchObject({ success: false, code: 'forbidden' });
182+
// The approve branch did NOT run…
183+
expect(marks).toHaveLength(0);
184+
// …the request is still pending, with no decision recorded…
185+
const stillPending = (await fake.find('sys_approval_request', { where: { id: request.id } }))[0];
186+
expect(stillPending.status).toBe('pending');
187+
const actions = await fake.find('sys_approval_action', { where: { request_id: request.id } });
188+
expect(actions.map((a: any) => a.action)).toEqual(['submit']);
189+
// …and the run is still parked, so the real decision can still land.
190+
expect(automation.listSuspendedRuns()).toHaveLength(1);
191+
192+
const out = await service.decide(request.id, { decision: 'approve', actorId: 'u1' }, SYSTEM_CTX);
193+
expect(out).toMatchObject({ finalized: true, resumed: true });
194+
expect(marks).toEqual(['on_approved']);
195+
});
196+
157197
it('resumes down the reject branch on rejection', async () => {
158198
registerDecisionFlow(automation, [{ type: 'user', value: 'u1' }]);
159199
await automation.execute('deal_approval', { object: 'crm_deal', record: { id: 'd1' } });

packages/plugins/plugin-approvals/src/approval-node.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,15 @@ export function registerApprovalNode(
104104
// Human decision: the run suspends here awaiting an external reply.
105105
supportsPause: true,
106106
isAsync: true,
107+
// #3801: this pause is NOT resumable through the generic run-resume
108+
// route. Continuing an approval is a side effect of a DECISION, and the
109+
// decision is the thing that must be authorized (the approver slate),
110+
// recorded (`sys_approval_action`) and mirrored (the status field) —
111+
// all of which lives in `ApprovalService.decide`. The engine now refuses
112+
// any resume of an approval suspension that does not carry the service's
113+
// in-process marker, so "decide via the approvals API, never a raw engine
114+
// resume" is enforced rather than merely documented.
115+
resumeAuthority: 'service',
107116
// Publish the node's config contract (ADR-0018 §configSchema) so the
108117
// Studio flow designer renders the Approval property form from the engine
109118
// rather than a hardcoded client form — the engine owns the shape.

0 commit comments

Comments
 (0)