Skip to content

automation: the #3801 resume gate follows subflow: but not map:, and the resume body can write engine-internal variables #3853

Description

@os-zhuang

Found while reviewing the seam #3822 just closed. Demonstrated, not merely reasoned
a repro runs green against main @ 57a3bb3; see below.

Two defects in the same trust boundary, one of them a hole in the gate #3822 shipped.

1. The gate does not follow the map: correlation chain

resumeInternal treats the two linked-run correlations oppositely
(packages/services/service-automation/src/engine.ts:2080):

if (run.correlation?.startsWith('map:')) {
    await this.executeNode(node, flow, variables, context, steps);   // RE-RUN the map node
} else {
    await this.traverseNext(node, flow, variables, context, steps, signal?.branchLabel);
}

resolveEffectiveSuspension (the gate's chain walk) only follows subflow:. A run parked
on a map node is therefore judged on map itself — resumeAuthority: 'any' — and let
through, even when the item the map is waiting on is parked on an approval.

map is the batch-approval shape (ADR-0037 A2, showcase_release_signoff), so the
run id a launcher holds is the map parent's. state.started is already incremented past
the in-flight item before the suspend (builtin/map-node.ts:155-156), and the re-entry
only records a result when $mapItemDone is set (map-node.ts:113-118). So an empty-body
resume of the parent skips the pending item's approval entirely:

before:  { started: 1, results: [] }
POST /api/v1/automation/release_signoff/runs/{parentRunId}/resume   {}
after:   { started: 2, results: [] }      ← item 1 skipped, no result recorded
suspended child runs: 2                   ← item 1 orphaned (its request stays pending) + item 2

Knock-on: when item 1's approval is eventually decided, bubbleToParent resumes the parent —
which is now waiting on item 2 — so that output lands under the wrong index and the
misalignment cascades.

2. signal.variables can write the engine's reserved $ namespace

signal.variables (the route's inputs) are applied as bare flow variables, so a caller
can write the exact handoff keys bubbleToParent uses. The node id is readable from
GET /api/v1/automation/:name (an SDK route). That upgrades the skip above into forging
the recorded outcome
:

POST /api/v1/automation/release_signoff/runs/{parentRunId}/resume
{ "inputs": { "signoffs.$mapItemDone": true,
              "signoffs.$mapItemOutput": { "result": "FORGED — never approved" } } }
after:  { started: 2, results: [ { "result": "FORGED — never approved" } ] }

Downstream reads config.outputVariable and believes that item was signed off with the
attacker's value. $ is the engine's namespace generally — $runId, $flowName,
$flowLabel, $record, $error, $parentRunId, $parentMapNode,
$parentOutputVariable, <nodeId>.$mapState — none of which a transport caller should be
able to set. $runId in particular is what approval / wait nodes read to correlate
external state back to the run.

Note (2) is not fixed by (1): a map whose pending item is parked on an ungated screen
/ wait node is still resumable, and the handoff keys are still writable there.

Control

Resuming the child run directly is correctly refused (code: 'forbidden') — #3822's gate
works on that side. The gap is exactly the parent-side path, which is the id a launcher holds.

Fix

  1. resolveEffectiveSuspension follows map: as well as subflow:, judging the child the
    map is waiting on. The semantics land right: "this item's decision is still open, so you
    may not advance the map past it."
  2. The route rejects caller-supplied variable names in the engine namespace ($… or a
    .$ segment). Filtering at the transport, not in the engine, keeps bubbleToParent's
    in-process handoff working — it is the same "strict at the untrusted boundary" split the
    resume gate already uses.

Refs #3801, #3822. Distinct from #3823 (revise-window wait), which needs a per-suspension
owner claim rather than a chain walk.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions