Skip to content

fix(campaign): reject a when clause with no depends_on (inert-gate fail-open)#177

Merged
xunholy merged 1 commit into
mainfrom
fix/campaign-when-fail-open
Jul 1, 2026
Merged

fix(campaign): reject a when clause with no depends_on (inert-gate fail-open)#177
xunholy merged 1 commit into
mainfrom
fix/campaign-when-fail-open

Conversation

@xunholy

@xunholy xunholy commented Jun 30, 2026

Copy link
Copy Markdown
Owner

A campaign step's when is evaluated against its predecessor's output, and that check lives entirely inside the Runner's if step.DependsOn != "" block. A step with when: but no depends_on: never reaches the check — its tool runs UNCONDITIONALLY. when is how an operator gates a destructive tool conditional (e.g. when: contains "AUTHORIZED"), so a missing depends_on turns an intended gate into a silently-inert one: a fail-open. ParseYAML now rejects When != "" && DependsOn == "" at load (matching the package's validate-at-parse philosophy). ParseYAML is the sole production construction path, so this fully closes it. Found via the internal automation-engine sweep (rest verified sound). Test proven to fail pre-fix. task ci green; eval 17/17.

…il-open)

A campaign step's `when` clause is evaluated against its predecessor's output,
and that evaluation lives entirely inside the Runner's `if step.DependsOn != ""`
block (campaign.go:284). A step authored with a `when:` but no `depends_on:`
therefore never reaches the when check — the Runner runs its tool
UNCONDITIONALLY. `when` is exactly the mechanism an operator uses to make a
destructive tool conditional (e.g. `when: contains "AUTHORIZED"`), so a missing
depends_on turns an intended gate into a silently-inert one: a fail-open.

ParseYAML now rejects `When != "" && DependsOn == ""` at load, matching the
package's existing validate-at-parse philosophy (duplicate ids, unresolved /
self / forward depends_on, cycles, and malformed/negative timeouts already fail
at load, not mid-run). A `when` with no predecessor has no output to evaluate
against, so it is meaningless as well as unsafe — failing loudly at load forces
the operator to make the dependency explicit instead of getting an inert guard.
ParseYAML is the sole production construction path (the web campaign API,
ParseFile, and eval all route through it; there is no struct-literal bypass), so
the parse-time rejection fully closes the fail-open.

Found via the internal audit sweep of the automation engines (rules / campaign /
workflows); the rest of those engines verified sound (tool dispatch routes
through RunTool's gates, template-into-params is operator-scoped and non-
recursive, cooldown is race-free, inFlight is leak-safe).

Test: TestParseYAML_RejectsWhenWithoutDependsOn — a step with `when` and no
`depends_on` is rejected; proven to fail pre-fix (the campaign parsed clean and
the step would have run unconditionally).

Verified: task ci green (lint 0 / vet / build / test:full -race 0 fail /
govulncheck clean); task eval 17/17.
@xunholy xunholy merged commit 278dd04 into main Jul 1, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant