Skip to content

Commit 17202a7

Browse files
vaindclaude
vaind
and
claude
committed
fix: complete script injection hardening across all actions
PR #150 moved user inputs to env vars but left step outputs (`steps.*.outputs.*`) directly interpolated in `run:` blocks — an attacker controlling e.g. git tags in a dependency repo could still inject arbitrary commands. Additionally, switch all PowerShell run blocks from double-quote string interpolation (`"$env:VAR"`) to string concatenation (`'prefix' + $env:VAR`) to eliminate any possibility of subexpression evaluation. Changes: - updater/action.yml: move all remaining step outputs (tags, URLs, branch names) to env vars; replace double-quote interpolation with concatenation throughout - sentry-cli/integration-test/action.yml: same concatenation fix - danger/action.yml: move docker image version from direct interpolation to env var with semver validation Refs: VULN-1100 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 705635b commit 17202a7

3 files changed

Lines changed: 68 additions & 45 deletions

File tree

‎danger/action.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,13 @@ runs:
5959
env:
6060
GITHUB_TOKEN: ${{ inputs.api-token }}
6161
EXTRA_DANGERFILE_INPUT: ${{ inputs.extra-dangerfile }}
62+
DANGER_VERSION: ${{ steps.config.outputs.version }}
6263
run: |
64+
# Validate version looks like a semver tag (defense in depth)
65+
if ! [[ "$DANGER_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
66+
echo "::error::Invalid Danger version '$DANGER_VERSION'. Expected semver format (e.g., 13.0.4)."
67+
exit 1
68+
fi
6369
# Start a detached container with all necessary volumes and environment variables
6470
docker run -td --name danger \
6571
--entrypoint /bin/bash \
@@ -72,7 +78,7 @@ runs:
7278
-e "GITHUB_TOKEN" \
7379
-e DANGER_DISABLE_TRANSPILATION="true" \
7480
-e "EXTRA_DANGERFILE_INPUT" \
75-
ghcr.io/danger/danger-js:${{ steps.config.outputs.version }} \
81+
"ghcr.io/danger/danger-js:${DANGER_VERSION}" \
7682
-c "sleep infinity"
7783
7884
- name: Setup additional packages

‎sentry-cli/integration-test/action.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,5 @@ runs:
2020
ACTION_PATH: ${{ github.action_path }}
2121
TEST_PATH: ${{ inputs.path }}
2222
run: |
23-
Import-Module -Name "$env:ACTION_PATH/action.psm1" -Force
24-
Invoke-Pester -Output Detailed "$env:TEST_PATH"
23+
Import-Module -Name ($env:ACTION_PATH + '/action.psm1') -Force
24+
Invoke-Pester -Output Detailed $env:TEST_PATH

‎updater/action.yml‎

Lines changed: 59 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -77,47 +77,47 @@ runs:
7777
DEPENDENCY_NAME: ${{ inputs.name }}
7878
run: |
7979
# Validate that inputs.name contains only safe characters
80-
if ("$env:DEPENDENCY_NAME" -notmatch '^[a-zA-Z0-9_\./@\s-]+$') {
81-
Write-Output "::error::Invalid dependency name: '$env:DEPENDENCY_NAME'. Only alphanumeric characters, spaces, and _-./@ are allowed."
80+
if ($env:DEPENDENCY_NAME -notmatch '^[a-zA-Z0-9_\./@\s-]+$') {
81+
Write-Output ('::error::Invalid dependency name: "' + $env:DEPENDENCY_NAME + '". Only alphanumeric characters, spaces, and _-./@ are allowed.')
8282
exit 1
8383
}
84-
Write-Output "✓ Dependency name '$env:DEPENDENCY_NAME' is valid"
84+
Write-Output ('Dependency name "' + $env:DEPENDENCY_NAME + '" is valid')
8585
8686
- name: Validate dependency path
8787
shell: pwsh
8888
env:
8989
DEPENDENCY_PATH: ${{ inputs.path }}
9090
run: |
9191
# Validate that inputs.path contains only safe characters (including # for CMake dependencies)
92-
if ("$env:DEPENDENCY_PATH" -notmatch '^[a-zA-Z0-9_\./#-]+$') {
93-
Write-Output "::error::Invalid dependency path: '$env:DEPENDENCY_PATH'. Only alphanumeric characters and _-./# are allowed."
92+
if ($env:DEPENDENCY_PATH -notmatch '^[a-zA-Z0-9_\./#-]+$') {
93+
Write-Output ('::error::Invalid dependency path: "' + $env:DEPENDENCY_PATH + '". Only alphanumeric characters and _-./# are allowed.')
9494
exit 1
9595
}
96-
Write-Output "✓ Dependency path '$env:DEPENDENCY_PATH' is valid"
96+
Write-Output ('Dependency path "' + $env:DEPENDENCY_PATH + '" is valid')
9797
9898
- name: Validate changelog-entry
9999
shell: pwsh
100100
env:
101101
CHANGELOG_ENTRY: ${{ inputs.changelog-entry }}
102102
run: |
103103
# Validate that inputs.changelog-entry is either 'true' or 'false'
104-
if ("$env:CHANGELOG_ENTRY" -notin @('true', 'false')) {
105-
Write-Output "::error::Invalid changelog-entry value: '$env:CHANGELOG_ENTRY'. Only 'true' or 'false' are allowed."
104+
if ($env:CHANGELOG_ENTRY -notin @('true', 'false')) {
105+
Write-Output ('::error::Invalid changelog-entry value: "' + $env:CHANGELOG_ENTRY + '". Only "true" or "false" are allowed.')
106106
exit 1
107107
}
108-
Write-Output "✓ Changelog-entry value '$env:CHANGELOG_ENTRY' is valid"
108+
Write-Output ('Changelog-entry value "' + $env:CHANGELOG_ENTRY + '" is valid')
109109
110110
- name: Validate pr-strategy
111111
shell: pwsh
112112
env:
113113
PR_STRATEGY: ${{ inputs.pr-strategy }}
114114
run: |
115115
# Validate that inputs.pr-strategy is either 'create' or 'update'
116-
if ("$env:PR_STRATEGY" -notin @('create', 'update')) {
117-
Write-Output "::error::Invalid pr-strategy value: '$env:PR_STRATEGY'. Only 'create' or 'update' are allowed."
116+
if ($env:PR_STRATEGY -notin @('create', 'update')) {
117+
Write-Output ('::error::Invalid pr-strategy value: "' + $env:PR_STRATEGY + '". Only "create" or "update" are allowed.')
118118
exit 1
119119
}
120-
Write-Output "✓ PR strategy value '$env:PR_STRATEGY' is valid"
120+
Write-Output ('PR strategy value "' + $env:PR_STRATEGY + '" is valid')
121121
122122
- name: Validate post-update-script
123123
if: ${{ inputs.post-update-script != '' }}
@@ -126,11 +126,11 @@ runs:
126126
POST_UPDATE_SCRIPT: ${{ inputs.post-update-script }}
127127
run: |
128128
# Validate that inputs.post-update-script contains only safe characters
129-
if ("$env:POST_UPDATE_SCRIPT" -notmatch '^[a-zA-Z0-9_\./#\s-]+$') {
130-
Write-Output "::error::Invalid post-update-script path: '$env:POST_UPDATE_SCRIPT'. Only alphanumeric characters, spaces, and _-./# are allowed."
129+
if ($env:POST_UPDATE_SCRIPT -notmatch '^[a-zA-Z0-9_\./#\s-]+$') {
130+
Write-Output ('::error::Invalid post-update-script path: "' + $env:POST_UPDATE_SCRIPT + '". Only alphanumeric characters, spaces, and _-./# are allowed.')
131131
exit 1
132132
}
133-
Write-Output "✓ Post-update script path '$env:POST_UPDATE_SCRIPT' is valid"
133+
Write-Output ('Post-update script path "' + $env:POST_UPDATE_SCRIPT + '" is valid')
134134
135135
- name: Validate authentication inputs
136136
shell: pwsh
@@ -288,30 +288,31 @@ runs:
288288
PR_STRATEGY: ${{ inputs.pr-strategy }}
289289
DEPENDENCY_PATH: ${{ inputs.path }}
290290
TARGET_BRANCH: ${{ inputs.target-branch }}
291+
LATEST_TAG: ${{ steps.target.outputs.latestTag }}
291292
run: |
292293
if ([string]::IsNullOrEmpty($env:TARGET_BRANCH)) {
293294
$mainBranch = $(git remote show origin | Select-String "HEAD branch: (.*)").Matches[0].Groups[1].Value
294295
$prBranchPrefix = ''
295296
} else {
296297
$mainBranch = $env:TARGET_BRANCH
297-
$prBranchPrefix = "$mainBranch-"
298+
$prBranchPrefix = $mainBranch + '-'
298299
}
299300
$prBranch = switch ($env:PR_STRATEGY)
300301
{
301-
'create' { "deps/$env:DEPENDENCY_PATH/${{ steps.target.outputs.latestTag }}" }
302-
'update' { "deps/$env:DEPENDENCY_PATH" }
303-
default { throw "Unkown PR strategy '$env:PR_STRATEGY'." }
302+
'create' { 'deps/' + $env:DEPENDENCY_PATH + '/' + $env:LATEST_TAG }
303+
'update' { 'deps/' + $env:DEPENDENCY_PATH }
304+
default { throw ('Unkown PR strategy "' + $env:PR_STRATEGY + '".') }
304305
}
305306
$prBranch = $prBranchPrefix + $prBranch
306-
"baseBranch=$mainBranch" | Tee-Object $env:GITHUB_OUTPUT -Append
307-
"prBranch=$prBranch" | Tee-Object $env:GITHUB_OUTPUT -Append
307+
('baseBranch=' + $mainBranch) | Tee-Object $env:GITHUB_OUTPUT -Append
308+
('prBranch=' + $prBranch) | Tee-Object $env:GITHUB_OUTPUT -Append
308309
$nonBotCommits = ${{ github.action_path }}/scripts/nonbot-commits.ps1 `
309-
-RepoUrl "$(git config --get remote.origin.url)" -PrBranch $prBranch -MainBranch $mainBranch
310+
-RepoUrl $(git config --get remote.origin.url) -PrBranch $prBranch -MainBranch $mainBranch
310311
$changed = $nonBotCommits.Length -gt 0 ? 'true' : 'false'
311-
"changed=$changed" | Tee-Object $env:GITHUB_OUTPUT -Append
312-
if ("$changed" -eq "true")
312+
('changed=' + $changed) | Tee-Object $env:GITHUB_OUTPUT -Append
313+
if ($changed -eq 'true')
313314
{
314-
Write-Output "::warning::Target branch '$prBranch' has been changed manually - skipping updater to avoid overwriting these changes."
315+
Write-Output ('::warning::Target branch "' + $prBranch + '" has been changed manually - skipping updater to avoid overwriting these changes.')
315316
}
316317
317318
- name: Parse the existing PR URL
@@ -321,19 +322,22 @@ runs:
321322
working-directory: caller-repo
322323
env:
323324
GH_TOKEN: ${{ inputs.api-token || github.token }}
325+
BASE_BRANCH: ${{ steps.root.outputs.baseBranch }}
326+
PR_BRANCH: ${{ steps.root.outputs.prBranch }}
324327
run: |
325-
$urls = @(gh api 'repos/${{ github.repository }}/pulls?base=${{ steps.root.outputs.baseBranch }}&head=${{ github.repository_owner }}:${{ steps.root.outputs.prBranch }}' --jq '.[].html_url')
328+
$apiUrl = 'repos/${{ github.repository }}/pulls?base=' + $env:BASE_BRANCH + '&head=${{ github.repository_owner }}:' + $env:PR_BRANCH
329+
$urls = @(gh api $apiUrl --jq '.[].html_url')
326330
if ($urls.Length -eq 0)
327331
{
328-
"url=" | Tee-Object $env:GITHUB_OUTPUT -Append
332+
'url=' | Tee-Object $env:GITHUB_OUTPUT -Append
329333
}
330334
elseif ($urls.Length -eq 1)
331335
{
332-
"url=$($urls[0])" | Tee-Object $env:GITHUB_OUTPUT -Append
336+
('url=' + $urls[0]) | Tee-Object $env:GITHUB_OUTPUT -Append
333337
}
334338
else
335339
{
336-
throw "Unexpected number of PRs matched ($($urls.Length)): $urls"
340+
throw ('Unexpected number of PRs matched (' + $urls.Length + '): ' + $urls)
337341
}
338342
339343
- name: Show git diff
@@ -348,11 +352,14 @@ runs:
348352
working-directory: caller-repo
349353
env:
350354
GH_TOKEN: ${{ inputs.api-token || github.token }}
355+
TARGET_REPO_URL: ${{ steps.target.outputs.url }}
356+
ORIGINAL_TAG: ${{ steps.target.outputs.originalTag }}
357+
LATEST_TAG: ${{ steps.target.outputs.latestTag }}
351358
run: |
352359
$changelog = ${{ github.action_path }}/scripts/get-changelog.ps1 `
353-
-RepoUrl '${{ steps.target.outputs.url }}' `
354-
-OldTag '${{ steps.target.outputs.originalTag }}' `
355-
-NewTag '${{ steps.target.outputs.latestTag }}'
360+
-RepoUrl $env:TARGET_REPO_URL `
361+
-OldTag $env:ORIGINAL_TAG `
362+
-NewTag $env:LATEST_TAG
356363
${{ github.action_path }}/scripts/set-github-env.ps1 TARGET_CHANGELOG $changelog
357364
358365
# First we create a PR only if it doesn't exist. We will later overwrite the content with the same action.
@@ -382,14 +389,17 @@ runs:
382389
id: pr
383390
shell: pwsh
384391
working-directory: caller-repo
392+
env:
393+
CREATED_PR_URL: ${{ steps.create-pr.outputs.pull-request-url }}
394+
EXISTING_PR_URL: ${{ steps.existing-pr.outputs.url }}
385395
run: |
386-
if ('${{ steps.create-pr.outputs.pull-request-url }}' -ne '')
396+
if (-not [string]::IsNullOrEmpty($env:CREATED_PR_URL))
387397
{
388-
"url=${{ steps.create-pr.outputs.pull-request-url }}" | Tee-Object $env:GITHUB_OUTPUT -Append
398+
("url=" + $env:CREATED_PR_URL) | Tee-Object $env:GITHUB_OUTPUT -Append
389399
}
390-
elseif ('${{ steps.existing-pr.outputs.url }}' -ne '')
400+
elseif (-not [string]::IsNullOrEmpty($env:EXISTING_PR_URL))
391401
{
392-
"url=${{ steps.existing-pr.outputs.url }}" | Tee-Object $env:GITHUB_OUTPUT -Append
402+
("url=" + $env:EXISTING_PR_URL) | Tee-Object $env:GITHUB_OUTPUT -Append
393403
}
394404
else
395405
{
@@ -415,7 +425,9 @@ runs:
415425
DEPENDENCY_PATH: ${{ inputs.path }}
416426
POST_UPDATE_SCRIPT: ${{ inputs.post-update-script }}
417427
GH_TOKEN: ${{ inputs.api-token || github.token }}
418-
run: ${{ github.action_path }}/scripts/update-dependency.ps1 -Path $env:DEPENDENCY_PATH -Tag '${{ steps.target.outputs.latestTag }}' -OriginalTag '${{ steps.target.outputs.originalTag }}' -PostUpdateScript $env:POST_UPDATE_SCRIPT
428+
LATEST_TAG: ${{ steps.target.outputs.latestTag }}
429+
ORIGINAL_TAG: ${{ steps.target.outputs.originalTag }}
430+
run: ${{ github.action_path }}/scripts/update-dependency.ps1 -Path $env:DEPENDENCY_PATH -Tag $env:LATEST_TAG -OriginalTag $env:ORIGINAL_TAG -PostUpdateScript $env:POST_UPDATE_SCRIPT
419431

420432
- name: Update Changelog
421433
if: ${{ inputs.changelog-entry == 'true' && ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }}
@@ -425,14 +437,19 @@ runs:
425437
DEPENDENCY_NAME: ${{ inputs.name }}
426438
CHANGELOG_SECTION: ${{ inputs.changelog-section }}
427439
GH_TOKEN: ${{ inputs.api-token || github.token }}
440+
PR_URL: ${{ steps.pr.outputs.url }}
441+
TARGET_REPO_URL: ${{ steps.target.outputs.url }}
442+
TARGET_MAIN_BRANCH: ${{ steps.target.outputs.mainBranch }}
443+
ORIGINAL_TAG: ${{ steps.target.outputs.originalTag }}
444+
LATEST_TAG: ${{ steps.target.outputs.latestTag }}
428445
run: |
429446
${{ github.action_path }}/scripts/update-changelog.ps1 `
430447
-Name $env:DEPENDENCY_NAME `
431-
-PR '${{ steps.pr.outputs.url }}' `
432-
-RepoUrl '${{ steps.target.outputs.url }}' `
433-
-MainBranch '${{ steps.target.outputs.mainBranch }}' `
434-
-OldTag '${{ steps.target.outputs.originalTag }}' `
435-
-NewTag '${{ steps.target.outputs.latestTag }}' `
448+
-PR $env:PR_URL `
449+
-RepoUrl $env:TARGET_REPO_URL `
450+
-MainBranch $env:TARGET_MAIN_BRANCH `
451+
-OldTag $env:ORIGINAL_TAG `
452+
-NewTag $env:LATEST_TAG `
436453
-Section $env:CHANGELOG_SECTION
437454
438455
- name: Show final git diff

0 commit comments

Comments
 (0)