← Community
bugfixed

Effect-node args are omitted from the plan hash when they contain a binding, letting an approved send be redirected without breaking the pin

ShwetaShweta#118d ago · 82 views
affected: station-v0.82fixed in: station-v0.86

primitives/plan_pin.py binds a live-run approval to the signed plan root so
that editing a workflow after approval invalidates the approval. Its docstring
states the case it exists to prevent: "Approve payroll_run on Monday, edit it
on Wednesday to add a Stripe charge, and Wednesday's run inherits Monday's
blessing."

For an effect node -- the kind that moves money and fires irreversible
external sends -- the node's argument values are omitted from that hash
whenever its args contain at least one {{binding}}. plan_workflow()'s effect
branch reads:

args_have_binding = _has_binding(n["args"])
args = _resolve(n["args"], outputs, ctx)
p = integ.plan(**args) if not args_have_binding else {"deferred": True,
"action_class": integ.action_class}
...
h = _sha({"id": ..., "provider": ..., "verb": ..., "plan": p,
"for_each_iters": ..., "for_each_unbounded": ...})

p is the only carrier of the node's arguments into the leaf hash. When a
binding is present, p is a two-key stub and the arguments never reach it, so
the leaf -- and therefore the workflow root the approval is pinned to --
describes only which action will run, never with what. run_workflow()'s effect
branch has no equivalent deferral: it always resolves args and calls
integ.plan(**args) for real.

The consequence is that an approved, pinned workflow whose send node names one
recipient can be edited to name a different recipient, and plan_pin.check()
still reports it as the approved plan. Proven with an A/B where the ONLY
difference between arms is a single binding in the node's args:

args fully literal -> integ.plan() runs, the recipient is folded into the
hash, the root changes, plan_pin.check() REFUSES with
"this workflow has CHANGED since it was approved"
one binding in args -> plan() skipped, recipient absent from the hash, the
root is byte-identical, plan_pin.check() returns
ok=True

The action used is resend_email_send, action_class "external_send" --
irreversible, no compensator, and listed as such in the pinned plan's own blast
radius. A binding in an effect node's args is the ordinary shape of a real
workflow: any node consuming an upstream node's output has one. The covered
case is the exception, not the rule.

Scope, stated precisely: arguments that feed the spend estimate are still
backstopped by plan_pin's separate approved-spend ceiling. Editing amount_cents
from 500 to 999999 is refused with "this run would spend 999999 cents; you
approved 500" even though the plan root is unchanged. The gap is every argument
that does not affect estimated spend -- recipient, subject, message body,
description, target record id, destination.

Reproduction steps:

  1. Extract a clean station-v0.82 tarball.
  2. Build a workflow with one effect node (action_id "resend_email_send", args

{"to": "alice@example.com", "subject": ..., "message": ...}) where message
is bound to an upstream transform's output, e.g. "{{nodes.prep.output}}".
Call plan_workflow() and pin the result with plan_pin.pin().

  1. Change ONLY the node's "to" argument to a different address, re-plan, and

call plan_pin.check() against the pin from step 2.

  1. For contrast, repeat steps 2-3 with the message argument written as a

literal string instead of a binding.

Expected: step 3 reports that the workflow changed since approval and refuses
the live run, exactly as step 4 does.

Actual: step 3's re-planned workflow_root is byte-identical to the pinned root
and plan_pin.check() returns ok=True, so the redirected send runs live under
the original approval. Step 4 correctly returns ok=False.

Root cause: plan_workflow()'s effect branch substitutes a {"deferred": true}
stub for the provider's resolved plan whenever _has_binding(n["args"]) is true,
and that stub is what gets hashed into the leaf. The deferral exists because a
binding may be unresolvable at plan time, but the consequence is that the
approved plan identity stops describing the node's arguments entirely rather
than describing whatever part of them is already known.

Suggested fix: hash the resolved argument structure alongside the deferral
marker rather than instead of it. Even when integ.plan() cannot run, the
resolved args are already computed on the line above, and unresolved bindings
can be hashed as their literal binding text -- so a node whose bound fields are
unchanged but whose literal fields were edited produces a different leaf. That
preserves the reason for the deferral while restoring the property plan-pinning
depends on.

5 pts

1 reply

Fixed in station-v0.86. Excellent catch — when an effect node's args contained a {{binding}}, the args never entered the plan-leaf hash, so a bound effect's arguments could be swapped without changing the plan_root and the plan-pin waved it through. The raw args template is now folded into the leaf hash. (This surfaced a follow-on interaction with incremental since injection, fixed in station-v0.87 — the pin check now plans the static spec.) Credited.

Sign in to reply.