Harden loop-action command execution against unquoted shell expansion - #404
Closed
cobusgreyling wants to merge 2 commits into
Closed
Harden loop-action command execution against unquoted shell expansion#404cobusgreyling wants to merge 2 commits into
cobusgreyling wants to merge 2 commits into
Conversation
inputs.command was spliced as raw text into the run: script before bash parsed it. For multi-line command: | blocks this silently broke sandbox isolation: only the first line reached loop-sandbox run -- ...; every later line executed as a bare top-level statement on the runner, bypassing the sandbox and circuit-breaker entirely. Write command to a temp script (passed via env:, never re-templated by the workflow YAML) and invoke it as a single quoted bash "$SCRIPT" call from both the sandbox and direct paths. Document safe usage and warn against embedding untrusted input in command in the README.
Run the temporary command script with bash -eo pipefail on the sandbox and direct paths so an intermediate failure in a multi-line command stops execution immediately and the step returns non-zero, instead of continuing on to later lines. Drop the chmod +x on the script since it's always invoked as an explicit argument to bash and never executed directly. Document sandbox-shell as a loop-sandbox compatibility option rather than something required for shell syntax in command.
Contributor
Loop Readiness AuditScore: 100/100 (L3) Strong loop readiness — good candidate for L3 with explicit gates. Top suggestions
Posted by |
12 tasks
Owner
Author
|
Superseded by merge of #395 (same commits; original author @amarjaleelbanbhan). This same-repo branch was only opened so required checks could run while the fork PR’s workflows sat in |
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.
Summary
Same change as #395 (from @amarjaleelbanbhan), re-hosted on a same-repo branch so required
audit/validatechecks can run (fork PR workflows were stuck inaction_required).inputs.commandwas spliced as raw text into therun:script before bash parsed it. For multi-linecommand: |blocks this silently broke sandbox isolation: only the first line reachedloop-sandbox run -- ...; every later line executed as a bare top-level statement on the runner, bypassing the sandbox and circuit-breaker entirely.Write
commandto a temp script (passed viaenv:, never re-templated by the workflow YAML) and invoke it as a single quotedbash -eo pipefail "$SCRIPT"call from both the sandbox and direct paths. Document safe usage and warn against embedding untrusted input incommandin the README.Closes #395 (supersedes / merges the same commits).
Attribution
Checklist
bash -eo pipefail) on all execution paths