Skip to content

automation: signal.output reopens the reserved-variable forgery #3853 closed on inputs #3879

Description

@os-zhuang

Found reviewing my own #3860. Demonstrated with a repro, not reasoned.

The hole

#3853 guarded signal.variables at the route. signal.output is passed through unchecked:

if (b.output && typeof b.output === 'object') signal.output = b.output;   // no check

and output keys are merged under the suspended node's id (engine.ts:2214-2218):

variables.set(`${run.nodeId}.${key}`, value);

For a run parked on a map node, run.nodeId is the map node id. So the same forgery lands through the other field:

POST /api/v1/automation/{flow}/runs/{parentRunId}/resume
{ "output": { "$mapItemDone": true, "$mapItemOutput": { "result": "FORGED via output" } } }
→ { started: 2, results: [ { "result": "FORGED via output" } ] }

Scope

The #3853 map gate still holds: a batch whose pending item sits on an approval is refused before any of this, so the approval bypass remains closed. The residual is forging the recorded result of an item parked on an ungated screen / wait — map-state corruption, not a decision bypass. output also cannot reach bare $runId / $record (it is always node-prefixed); that path stays covered by the inputs guard.

Root cause — worth more than the patch

This is the second escape with the same shape (the first: the gate followed subflow: but not map:). The seam has three open-coded writers into one variable map — output prefixed, variables bare, and the engine's own map handoff — so "guard the field that was exploited" invites the next field.

Fix should be structural: one chokepoint where a resume signal reaches the variable map, taking both fields, checking once, applying all-or-nothing before the suspension is consumed; the rule in the engine (so the SDK and every future transport inherit it) with the transport only mapping the verdict onto a status; and engine-built signals exempted by a marker the transport cannot mint.

Note the placement argument in #3853 was the error: "strict at the untrusted boundary" is right about where a rule BINDS, not where it LIVES.

Refs #3801, #3853, #3860.

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