Repository navigation
The framework guard reads raw YAML, so an output-only command can satisfy it #3060
Description
🤖 Generated by the Agentic Engineer
Evidence
scripts/guard-kubescape-gate-frameworks.sh (added in #3057) asserts the Kubescape gate still names
every required framework. It matches against the raw workflow text and excludes comment lines, so a
command that merely prints the invocation still counts as one:
run: |
echo "ksail workload scan --framework nsa,mitre" # decoy — satisfies the guard
FRAMEWORKS=nsa
ksail workload scan --framework "$FRAMEWORKS" # the real command, unreadable to the guardThe regex never sees the variable form, finds only the echo, and exits 0 on a repository scanning one
framework. Reproduced by Codex during review of #3057 and accepted there rather than patched.
Why it was not fixed inline, and why a blacklist is the wrong shape
The obvious fix — also reject echo and printf — is the third iteration of the same move. #3057
already excluded comments after one decoy, and this is the next spelling. Enumerating forbidden
prefixes only ever catches the ones somebody thought of: cat, a heredoc, :, true &&, a string
in a with: block. Each round closes one spelling and leaves the class open.
The convergent fix inverts the direction: parse the workflow structurally and read only what
actually executes, rather than grepping text and subtracting known decoys.
Current exposure is bounded
- The realistic regression — someone edits
nsa,mitretonsa— is caught today. - Anything the guard cannot read triggers its fail-closed path, so an unreadable form is a failure,
never a silent pass. - Reaching the hole requires deliberately adding an output-only line that names the frameworks. That
is an adversarial construction, not a plausible edit.
So this is a real gap in a defence-in-depth check, not an open door.
Smallest useful change
Rewrite the guard as a Go validator beside the repo's existing ones
(scripts/validate-concurrency-queue, scripts/validate-merge-group-heal,
scripts/validate-flux-verify), which is also what AGENTS.md's "bash first, Go when it grows"
rule points at now that this script parses, normalises and compares sets. It would parse the workflow
YAML, extract run: scalars, and identify genuine invocations — dropping the text-matching entirely.
Acceptance criteria
- The guard identifies scan invocations from parsed workflow structure, not raw text
- The
echodecoy above is rejected, with that exact fixture in the test suite - Every existing assertion in
scripts/tests/test-kubescape-gate-frameworks-guard.shstill holds
— required members, identical sets across both workflows, fail-closed on no invocation, and the
wiring checks - Ablation-proved: removing the structural parse makes the decoy case fail
- The residual note in the guard header is removed rather than reworded
Metadata
Metadata
Assignees
Labels
Type
Fields
Projects
- Status
Activity
coderabbitai commentedon Aug 10, 2026
🔗 Related PRs
#2830 - feat(ci): publish Kubescape posture findings to Code Scanning [merged]
#2917 - feat(security): reconcile live Kubescape findings into deduped backlog issues [merged]
#2930 - feat(ci): verify published evidence before promoting the prod digest [merged]
#2947 - fix(ci): refuse to promote a prod digest whose evidence cannot be verified [merged]
#3043 - security(supply-chain): trust only commit-pinned shared publish workflows [merged]
📝 Issue Planner
Check the box below or use the
@coderabbitai plancommand to generate an implementation plan and prompts that you can use with your favorite coding assistant.🧪 Issue enrichment is currently in open beta.
You can configure auto-planning by selecting labels in the issue_enrichment configuration.
To disable automatic issue enrichment, add the following to your
.coderabbit.yaml:💬 Have feedback or questions? Drop into our discord!
security(ci): invert the invocation match to an allow-list
devantler commentedon Aug 10, 2026
Closed as fixed in #3057 at
1da2132a, without the Go rewrite this issue proposed.The framing here was that closing the class needed structural YAML parsing, because the alternative on the table was another exclusion. CodeRabbit reviewing the same head supplied the third option: invert to an allow-list — require
ksailas the line's first non-blank token. That rejects every output-only form at once (echo,printf,cat, heredoc bodies,:,true &&, a quoted string in awith:block) with no parser.Four decoy shapes are pinned in
scripts/tests/test-kubescape-gate-frameworks-guard.sh, and reverting to the comment-only exclusion makes theechocase fail, so the inversion is ablation-proved. The residual note is removed from the guard header rather than reworded.A structural parse may still be worth it one day, but it is no longer needed for this hole.
devantler commentedon Aug 10, 2026
Reopening — I closed this on an overclaim
I closed this saying the allow-list inversion (require
ksailas the first non-blank token) closedthe class. It did not. Codex reproduced a surviving decoy on the very next review round:
The heredoc body line genuinely begins with
ksail, so the allow-list accepts it, while the realcommand's variable form stays unreadable. Guard reports identical valid sets; Kubescape scans NSA only.
What I got wrong, precisely
The inversion closed the command-shape class —
echo,printf,cat <<EOFas a command, aquoted string in a
with:block. Those all fail because their first token is notksail.It does not touch the shell-context class: a line whose first token is
ksailbut which isdata rather than an executed command — a heredoc body, and by extension anything inside a
non-executing block. Those are different axes, and I treated closing one as closing both.
Why I am not adding heredoc detection
Same reason the original issue gave, and it survives the correction: tracking heredoc state in a
line-oriented bash matcher is the start of reimplementing a shell parser one construct at a time, and
the next construct is already visible (a
casebranch,if false; then, a commented-out block thatbash -nwould still parse). The convergent fix remains what this issue asked for — parse theworkflow YAML, take the
run:scalars, and identify commands with shell awareness.Current exposure, restated honestly
Unchanged from the original filing, and the allow-list did narrow it:
nsa,mitretonsa— is caught.So: a real remaining gap in a defence-in-depth check, narrower than when this was filed, and not an
open door. The guard header's residual note is restored and points here.
Acceptance criteria from the original filing stand, plus:
docs(ci): restore the guard's residual note, correctly scoped
chore: agent-claim #3060 nonce=51f9b67d293ee7f671006e75dc1504f3
security(ci): refuse an always-false job condition, not only a false …
security(ci): strip a shell comment before joining its continuation
refactor(ci): drop the stripComment wrapper its last caller no longer…