Skip to content

Commit a00fe2e

Browse files
pattonwebzclaude
andcommitted
Pass untrusted values into run: via env, not template splices
A ${{ }} expression is substituted into the script text before bash parses it, so any attacker-controlled value spliced into a run: block is executable. Three sources here qualify: - VERSION, grep'd from the plugin's `Version:` header, which is part of the PR diff. Assigned as VERSION="${{ ... }}" in both build steps, so a version of `1.0"; <cmd>; #` executes. - PRIMARY_ZIP_PATH / REF_ZIP_PATH in the summary, which embed that same VERSION. - github.head_ref, a branch name chosen by whoever opened the PR; git permits " and ; in refs. This is the actionlint template-injection warning the workflow has carried for a while - it now lints clean apart from the pre-existing labels.*.name one. Each was confirmed by rendering and executing both forms: spliced runs the injected command, env: prints it as text. Sync with equalizedigital/accessibility-new-window-warnings#39, where CodeRabbit flagged the summary occurrence. Artifact naming is unchanged for every event type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019LoLgW7oFjBPxGea9kiZJ2
1 parent dc053c9 commit a00fe2e

1 file changed

Lines changed: 33 additions & 15 deletions

File tree

.github/workflows/build-plugin-with-ref.yml

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,23 @@ jobs:
6666
fi
6767
6868
- name: Show selected mode/ref
69+
env:
70+
# head_ref is a branch name chosen by whoever opened the PR, and git
71+
# permits characters like " and ; in it - so spliced into the script
72+
# text it is executable. This is the actionlint/zizmor
73+
# template-injection warning this workflow has carried for a while.
74+
MODE: ${{ steps.setref.outputs.mode }}
75+
HEAD_REF: ${{ github.head_ref }}
76+
PR_NUMBER: ${{ github.event.pull_request.number }}
77+
RELEASE_TAG: ${{ github.event.release.tag_name }}
78+
REF_PARAM: ${{ steps.setref.outputs.ref_param }}
6979
run: |
7080
echo "Triggered on: $GITHUB_EVENT_NAME"
71-
echo "Mode: ${{ steps.setref.outputs.mode }}"
72-
echo "Head ref: ${{ github.head_ref }}"
73-
echo "PR number: ${{ github.event.pull_request.number }}"
74-
echo "Release tag: ${{ github.event.release.tag_name }}"
75-
echo "Ref param: ${{ steps.setref.outputs.ref_param }}"
81+
echo "Mode: ${MODE}"
82+
echo "Head ref: ${HEAD_REF}"
83+
echo "PR number: ${PR_NUMBER}"
84+
echo "Release tag: ${RELEASE_TAG}"
85+
echo "Ref param: ${REF_PARAM}"
7686
7787
- name: Set up Node.js
7888
uses: actions/setup-node@v4
@@ -144,6 +154,14 @@ jobs:
144154
145155
- name: Build primary dist zip (empty ref)
146156
if: steps.setref.outputs.skip_primary != 'true'
157+
env:
158+
# VERSION comes from the plugin's own `Version:` header, which is part
159+
# of the PR diff and so attacker-controlled on a fork PR. A ${{ }}
160+
# splice lands in the script text before bash parses it, making a
161+
# crafted version string executable; via env: it stays inert data.
162+
VERSION: ${{ steps.version.outputs.version }}
163+
SHORT_SHA: ${{ steps.version.outputs.short_sha }}
164+
PR_NUMBER: ${{ github.event.pull_request.number }}
147165
run: |
148166
echo "Building primary zip with empty ref..."
149167
npm run dist
@@ -158,12 +176,9 @@ jobs:
158176
mkdir -p builds
159177
160178
# Construct the new filename based on trigger mode
161-
VERSION="${{ steps.version.outputs.version }}"
162-
SHORT_SHA="${{ steps.version.outputs.short_sha }}"
163-
164179
if [ "${{ github.event_name }}" = "pull_request" ]; then
165180
# PR: accessibility-checker-{version}-{prnumber}-{hash}.zip
166-
PRIMARY_ZIP_NAME="accessibility-checker-${VERSION}-${{ github.event.pull_request.number }}-${SHORT_SHA}.zip"
181+
PRIMARY_ZIP_NAME="accessibility-checker-${VERSION}-${PR_NUMBER}-${SHORT_SHA}.zip"
167182
elif [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
168183
# Manual: accessibility-checker-{version}-{hash}.zip
169184
PRIMARY_ZIP_NAME="accessibility-checker-${VERSION}-${SHORT_SHA}.zip"
@@ -229,6 +244,10 @@ jobs:
229244
230245
- name: Build ref dist zip (custom ref)
231246
if: steps.check_ref.outputs.need_ref_build == 'true'
247+
env:
248+
VERSION: ${{ steps.version.outputs.version }}
249+
SHORT_SHA: ${{ steps.version.outputs.short_sha }}
250+
REF_VALUE: ${{ steps.setref.outputs.ref_param }}
232251
run: |
233252
echo "Building ref zip with custom ref value..."
234253
npm run dist
@@ -242,10 +261,6 @@ jobs:
242261
fi
243262
mkdir -p builds
244263
245-
VERSION="${{ steps.version.outputs.version }}"
246-
SHORT_SHA="${{ steps.version.outputs.short_sha }}"
247-
REF_VALUE="${{ steps.setref.outputs.ref_param }}"
248-
249264
# Ref build naming: accessibility-checker-{version}-ref-{refvalue}-{hash}.zip
250265
REF_ZIP_NAME="accessibility-checker-${VERSION}-ref-${REF_VALUE}-${SHORT_SHA}.zip"
251266
@@ -329,10 +344,13 @@ jobs:
329344
PRIMARY_PLAYGROUND_URL: ${{ steps.playground_primary.outputs.playground-url }}
330345
MODE: ${{ steps.setref.outputs.mode }}
331346
SKIP_PRIMARY: ${{ steps.setref.outputs.skip_primary }}
347+
PRIMARY_ZIP_PATH: ${{ env.PRIMARY_ZIP_PATH }}
348+
REF_ZIP_PATH: ${{ env.REF_ZIP_PATH }}
349+
REF_PARAM: ${{ steps.setref.outputs.ref_param }}
332350
run: |
333351
echo "=== Build Summary ==="
334352
echo "Mode: ${{ steps.setref.outputs.mode }}"
335-
echo "Primary zip (empty ref): ${{ env.PRIMARY_ZIP_PATH }}"
353+
echo "Primary zip (empty ref): ${PRIMARY_ZIP_PATH}"
336354
if [ -n "${PRIMARY_PLAYGROUND_URL:-}" ]; then
337355
echo "Playground (primary): ${PRIMARY_PLAYGROUND_URL}"
338356
elif [ "${MODE}" = "release" ]; then
@@ -343,7 +361,7 @@ jobs:
343361
echo "Playground (primary): not built for this run"
344362
fi
345363
if [ "${{ steps.check_ref.outputs.need_ref_build }}" = "true" ]; then
346-
echo "Ref zip (ref=${{ steps.setref.outputs.ref_param }}): ${{ env.REF_ZIP_PATH }}"
364+
echo "Ref zip (ref=${REF_PARAM}): ${REF_ZIP_PATH}"
347365
else
348366
echo "Ref zip: Not built"
349367
fi

0 commit comments

Comments
 (0)