Two defects in the same step — changelog -> "Check for changelog fragments" in .github/workflows/release.yml (around lines 380-414 at the current main).
1. Script injection via github.base_ref
- name: Check for changelog fragments
run: |
...
CHANGED_FILES=$(git diff --name-only origin/${{ github.base_ref }}...HEAD)
${{ }} expressions are substituted into the script before bash sees it, so the base branch name is spliced in as shell source. On a pull_request event the base ref is repository data rather than fork data, but it is still attacker-influenced in any repository where contributors can create branches, and a branch name may legally contain `, $(, ; and &.
Reproduction
git checkout -b '$(touch /tmp/pwned)'
git push -u origin '$(touch /tmp/pwned)'
Then open a pull request targeting that branch. The runner executes:
git diff --name-only origin/$(touch /tmp/pwned)...HEAD
and the command substitution runs. Substitute any command for the payload.
Suggested fix
Pass it as an environment variable, which is data rather than source, and quote the expansion. This is what the rust template already does — scripts/check-changelog-fragment.rs is invoked with env: GITHUB_BASE_REF: ${{ github.base_ref }}.
- name: Check for changelog fragments
env:
GITHUB_BASE_REF: ${{ github.base_ref }}
run: |
set -euo pipefail
...
CHANGED_FILES=$(git diff --name-only "origin/${GITHUB_BASE_REF}...HEAD")
2. The gate can never fail
if [ "$SOURCE_CHANGED" -gt 0 ] && [ "$FRAGMENTS" -eq 0 ]; then
echo "::warning::No changelog fragment found. ..."
...
# Note: This is a warning, not a failure, to allow flexibility
# Change 'exit 0' to 'exit 1' to make it required
exit 0
fi
The comment says this is deliberate, so treat this half as a suggestion rather than a bug report. The practical effect, though, is that a job named "Changelog Fragment Check" is always green and the annotation is easy to miss in a long log, so source changes routinely land without a fragment and the generated changelog silently drifts from reality. The rust template's equivalent check exits 1 (scripts/check-changelog-fragment.rs:157), and the js template's changeset check fails too, so python is the odd one out among the three templates.
Workaround
Configure a required status check on a separate job, or grep the log for ::warning::No changelog fragment.
Suggested fix
if [ "$SOURCE_CHANGED" -gt 0 ] && [ "$FRAGMENTS" -eq 0 ]; then
echo "::error::No changelog fragment found. Please run 'scriv create' and document your changes."
echo " pip install 'scriv[toml]'"
echo " scriv create"
exit 1
fi
Also worth adding set -euo pipefail to the step: SOURCE_CHANGED=$(echo "$CHANGED_FILES" | grep -E "$SOURCE_PATTERN" | wc -l) currently relies on grep's exit status being swallowed by the pipe, which is fragile if the pipeline is ever reordered — grep -cE "$SOURCE_PATTERN" || true is the safer form.
Context
Found while auditing CI/CD false positives and false negatives in a repository seeded from these templates: link-foundation/browser-commander#75. Both fixes are applied there in link-foundation/browser-commander#76.
Two defects in the same step —
changelog-> "Check for changelog fragments" in.github/workflows/release.yml(around lines 380-414 at the currentmain).1. Script injection via
github.base_ref${{ }}expressions are substituted into the script before bash sees it, so the base branch name is spliced in as shell source. On apull_requestevent the base ref is repository data rather than fork data, but it is still attacker-influenced in any repository where contributors can create branches, and a branch name may legally contain`,$(,;and&.Reproduction
Then open a pull request targeting that branch. The runner executes:
and the command substitution runs. Substitute any command for the payload.
Suggested fix
Pass it as an environment variable, which is data rather than source, and quote the expansion. This is what the rust template already does —
scripts/check-changelog-fragment.rsis invoked withenv: GITHUB_BASE_REF: ${{ github.base_ref }}.2. The gate can never fail
The comment says this is deliberate, so treat this half as a suggestion rather than a bug report. The practical effect, though, is that a job named "Changelog Fragment Check" is always green and the annotation is easy to miss in a long log, so source changes routinely land without a fragment and the generated changelog silently drifts from reality. The rust template's equivalent check exits 1 (
scripts/check-changelog-fragment.rs:157), and the js template's changeset check fails too, so python is the odd one out among the three templates.Workaround
Configure a required status check on a separate job, or grep the log for
::warning::No changelog fragment.Suggested fix
Also worth adding
set -euo pipefailto the step:SOURCE_CHANGED=$(echo "$CHANGED_FILES" | grep -E "$SOURCE_PATTERN" | wc -l)currently relies ongrep's exit status being swallowed by the pipe, which is fragile if the pipeline is ever reordered —grep -cE "$SOURCE_PATTERN" || trueis the safer form.Context
Found while auditing CI/CD false positives and false negatives in a repository seeded from these templates: link-foundation/browser-commander#75. Both fixes are applied there in link-foundation/browser-commander#76.