Harden release workflows against tag-derived script injection - #2119
Merged
Merged
Conversation
The tag-derived version was interpolated via ${{ }} expressions directly
into run blocks in release_macos.yml, release_docker.yml and the
create-release job of release.yml. A refname-legal hostile tag such as
1.2.3$(cmd) matches the '**[0-9]+.[0-9]+.[0-9]+*' push-tags filter and
reaches these steps unvalidated, so bash executes the command
substitution during expansion - before sign_and_notarize.sh runs and
with the Apple signing credentials in the step environment. The Linux
and Windows workflows already validate the version and pass it through
environment variables for exactly this reason; the macOS job (which runs
in parallel and has no needs gate), the docker job and the release body
step were missed.
Apply the same two controls everywhere:
- Validate the extracted version against the shared X.Y.Z[-suffix]
grammar up front and fail the job with a clear message on non-semver
tags (docker also accepts the leading 'v' it passes to build.py).
- Pass the version (and the Docker Hub username / hashes) through
environment variables and quote them, instead of splicing ${{ }}
expressions into run scripts, so a crafted tag/ref/input is treated as
inert data and cannot inject shell into the steps.
nullPointerEnjoyer
requested review from
ImplOfAnImpl,
anyxem and
erubboli
as code owners
September 19, 2026 14:09
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 3 selected item(s). |
erubboli
approved these changes
Sep 19, 2026
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
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
Closes a script-injection gap in the release pipeline: a refname-legal hostile tag (e.g.
1.2.3$(id>>proof.txt)) matches the'**[0-9]+.[0-9]+.[0-9]+*'push-tags filter and, before this change, reached shell execution in three places:release_macos.yml—Sign and Notarize GUIspliced${{ steps.get_version.outputs.VERSION }}directly into therun:block. GitHub's runner performs the expression substitution textually before bash parses the script, so$(...)from the tag name executed during word expansion — beforesign_and_notarize.sheven ran — in a step whose environment holdsMACOS_CERTIFICATE_BASE64,MACOS_CERTIFICATE_PASSWORD,APPLE_IDandAPPLE_ID_PASSWORD. This job runs in parallel with the Linux/Windows jobs (noneedsgate), so it fires even though those jobs reject non-semver tags.release_docker.yml— unvalidated version interpolated into the build/push step whose argument flows intodocker build/pushcommand strings executed withshell=Trueinbuild-tools/docker/build.py.release.yml(create-release) — version (and artifact hashes) interpolated into the release-bodyrun:block.The payload rides the tag name, not commit content, so commit review does not see it; it can point at any existing reviewed commit.
The Linux and Windows workflows already enforce the intended invariant —
release_linux.yml:63/release_windows.yml:51validate the version against^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$, andrelease_windows.ymlpasses it via environment variables with the comment "so that a crafted tag/ref/input cannot inject shell into the steps";packaging/common/lib.shvalidate_versionguards the same input for packages. This PR applies the same two controls to the missed paths:tag a semver releaseerror before anything privileged runs (docker accepts the optional leadingvit forwards tobuild.py, which strips it).env:and used as quoted shell variables ("$VERSION"), never spliced via${{ }}intorun:scripts, so a crafted tag/ref/input is inert data.GitHub's own guidance recommends exactly this pattern ("use an intermediate environment variable" for mitigating script injection attacks).
Verification
bash -nclean on all 9run:blocks across the three files.get_version/versionsteps and executed them withGITHUB_REFset to 7 tag cases × 3 workflows:1.2.3$(id>>proof.txt),1.2.3$(curl${IFS}…|sh),1.2.3\id`,1.2.3;id→ all **rejected** with exit 1;v0.4.1,0.4.1,v0.4.1-rc.1` → all accepted.${{ }}expression remaining inside anyrun:block is${{ matrix.arch }}(workflow-defined strategy value).$(id>>proof.txt)executed during expansion (file created before the signing script ran); with the new pattern the hostile string reaches the script as inert argument data and nothing executes.Notes for reviewers
derived version '…' is not X.Y.Z[-suffix]; tag a semver releaseinstead of proceeding.Mintlayer_Node_macos_${{ … }}_${{ matrix.arch }}inwith:blocks) are action inputs rather than shell, and now carry a validated version, so they were left unchanged.