fix(actions): pass action_path through the environment like everything else - #7
Merged
Merged
Conversation
…g else
Enabling code scanning surfaced two `actions/code-injection` findings, one in
each composite action, both on the same line:
root="$(cd -- "${{ github.action_path }}/../../.." && pwd)"
Every other value these two files use — command, plan, state, target, output,
at, operation — is already passed through `env:` and read as a shell variable.
This one expression was interpolated straight into the script.
`github.action_path` is set by the runner, so this is not exploitable today.
The rule is about the shape rather than this particular value, and the shape is
what makes the habit reliable: a file with one exception teaches a reader that
interpolation is sometimes acceptable here, and the next value added may not be
runner-controlled. Both sites now go through `ACTION_PATH`.
Verified: `scripts/validate_module.sh` OK (10 contract tests, pin check),
actionlint 1.7.12 clean across the repository, and zizmor 1.26.1
`--persona=pedantic` reports no findings on either file.
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
1 participant
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.
Code scanning, enabled yesterday in #6, surfaced two
actions/code-injectionfindings — one per composite action, both the same line:root="$(cd -- "${{ github.action_path }}/../../.." && pwd)"Every other value these files use —
command,plan,state,target,output,at,operation— already goes throughenv:and is read as a shell variable. This single expression was interpolated straight into the script.Not exploitable, still worth fixing
github.action_pathis set by the runner, not by a caller, so there is no injection available through it today.The rule targets the shape, and the shape is what makes the habit hold. A file with one exception teaches its next reader that interpolating into
run:is sometimes fine here — and the next value someone adds may not be runner-controlled. The cost of consistency is one line; the cost of the exception is that the rule stops being a rule.Verification
scripts/validate_module.sh→ OK (10 contract tests, plus the pin check)--persona=pedantic→No findings to reporton both filesNot in scope here
The seven
actions/untrusted-checkoutfindings in the workflow files are a separate question about thecontract_shamodel — they are being written up on their own rather than folded in, because the answer is a design decision about deployment authority, not a mechanical edit.