Skip to content

Commit d839afd

Browse files
Pass free-text workflow inputs through env, not shell interpolation
`inputs.message` (deprecate-npm.yml) and `inputs.version` (release-all.yml) are free-text workflow_dispatch inputs that were interpolated directly into run: scripts. Two problems: - Correctness: a message containing an apostrophe broke the script, since the value landed inside single quotes. - Safety: the value is substituted before bash parses the line, so a crafted input could run commands. workflow_dispatch needs write access, so this is not remotely exploitable, but it is the pattern GitHub explicitly warns against. Both now arrive as environment variables and are referenced as "$VERSION" / "$MESSAGE", which bash treats as data. Boolean and choice inputs are left inline: GitHub constrains those to their declared values, so they cannot carry arbitrary text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 566c28b commit d839afd

2 files changed

Lines changed: 27 additions & 12 deletions

File tree

.github/workflows/deprecate-npm.yml

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,13 @@ jobs:
4747
node-version: '20'
4848
registry-url: 'https://registry.npmjs.org'
4949

50+
# Inputs are passed through env rather than interpolated into the script.
51+
# `message` is free text: interpolating it directly would let an
52+
# apostrophe break the script, and a crafted value run commands.
5053
- name: Show current state
54+
env:
55+
PKG: ${{ inputs.package }}
5156
run: |
52-
PKG='${{ inputs.package }}'
5357
echo "Package: $PKG"
5458
echo "Versions: $(npm view "$PKG" versions --json 2>/dev/null | tr -d '\n[] "')"
5559
CURRENT="$(npm view "$PKG" deprecated 2>/dev/null || true)"
@@ -63,15 +67,17 @@ jobs:
6367
if: ${{ inputs.dry_run != true }}
6468
env:
6569
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
70+
PKG: ${{ inputs.package }}
71+
UNDO: ${{ inputs.undo }}
72+
MESSAGE: ${{ inputs.message }}
6673
run: |
6774
set -uo pipefail
68-
PKG='${{ inputs.package }}'
6975
7076
# An empty message is how npm clears a deprecation.
71-
if [ '${{ inputs.undo }}' = 'true' ]; then
77+
if [ "$UNDO" = 'true' ]; then
7278
MSG=''
7379
else
74-
MSG='${{ inputs.message }}'
80+
MSG="$MESSAGE"
7581
if [ -z "$MSG" ]; then
7682
echo "::error::A message is required unless undo=true"
7783
exit 1
@@ -80,15 +86,18 @@ jobs:
8086
8187
# @"*" covers every published version, present and future.
8288
if ! npm deprecate "$PKG@*" "$MSG"; then
83-
echo "::error::npm deprecate failed. The NPM_TOKEN must be an automation token \
89+
echo "::error::npm deprecate failed. NPM_TOKEN must be an automation token \
8490
belonging to an account with publish rights on $PKG (a 2FA-required publish token \
8591
will not work unattended)."
8692
exit 1
8793
fi
8894
8995
- name: Verify
96+
env:
97+
PKG: ${{ inputs.package }}
98+
DRY_RUN: ${{ inputs.dry_run }}
99+
UNDO: ${{ inputs.undo }}
90100
run: |
91-
PKG='${{ inputs.package }}'
92101
# npm's CDN can lag a few seconds behind a write.
93102
sleep 10
94103
RESULT="$(npm view "$PKG" deprecated 2>/dev/null || true)"
@@ -98,11 +107,11 @@ jobs:
98107
echo ""
99108
echo "| Package | Dry run | Deprecation notice |"
100109
echo "|---|---|---|"
101-
echo "| \`$PKG\` | ${{ inputs.dry_run }} | ${RESULT:-none} |"
110+
echo "| \`$PKG\` | $DRY_RUN | ${RESULT:-(none)} |"
102111
echo ""
103-
if [ "${{ inputs.dry_run }}" = "true" ]; then
112+
if [ "$DRY_RUN" = 'true' ]; then
104113
echo "Dry run — nothing was changed."
105-
elif [ '${{ inputs.undo }}' = 'true' ]; then
114+
elif [ "$UNDO" = 'true' ]; then
106115
echo "Deprecation cleared."
107116
else
108117
echo "Deprecated. \`npm install $PKG\` now prints the notice."

.github/workflows/release-all.yml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,14 @@ jobs:
4848
- name: Checkout
4949
uses: actions/checkout@v4
5050

51+
# `version` is free text, so it goes through env rather than being
52+
# interpolated into the script.
5153
- name: Validate umbrella tag format
54+
env:
55+
VERSION: ${{ inputs.version }}
5256
run: |
53-
if ! [[ "${{ inputs.version }}" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then
54-
echo "::error::Invalid version format: ${{ inputs.version }} (expected X.Y.Z or X.Y.Z-prerelease)"
57+
if ! [[ "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$ ]]; then
58+
echo "::error::Invalid version format: $VERSION (expected X.Y.Z or X.Y.Z-prerelease)"
5559
exit 1
5660
fi
5761
@@ -378,11 +382,13 @@ jobs:
378382
if: always()
379383
steps:
380384
- name: Summary
385+
env:
386+
VERSION: ${{ inputs.version }}
381387
run: |
382388
{
383389
echo "## Release Summary"
384390
echo ""
385-
echo "**Umbrella tag:** v${{ inputs.version }}"
391+
echo "**Umbrella tag:** v$VERSION"
386392
echo "**Dry run:** ${{ inputs.dry_run }}"
387393
echo ""
388394
echo "| Package | Preflight action |"

0 commit comments

Comments
 (0)