Skip to content

Commit b379f7d

Browse files
vaindclaudejpnurmi
authored
fix(updater): Prevent script injection vulnerabilities (#98)
* fix(updater): Prevent script injection vulnerabilities Add input validation and use environment variables instead of direct interpolation to prevent potential script injection attacks through user-controlled workflow inputs. - Add validate-inputs job to check for safe characters in inputs.name and inputs.path - Move all environment variable declarations to job level for better organization - Replace direct interpolation in PR titles and PowerShell scripts with env variables - Ensure all user inputs are properly sanitized before use 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * refactor: Split input validation into separate steps Split the single validation step into two distinct steps for better clarity and more granular error reporting: - Validate dependency name - Validate dependency path Each step now also logs a success message when validation passes. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * fix: Use [:space:] character class for spaces in regex Fix the regex pattern to properly match spaces in dependency names by using the [:space:] POSIX character class instead of a literal space in the regex pattern. This fixes CI failures for test cases that include spaces in the dependency name like "Workflow args test script". 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * fix: Correct regex character class syntax for hyphens Move hyphens to the end of character classes in regex patterns to ensure they are treated as literal characters rather than ranges. This fixes validation failures for inputs containing hyphens like "WORKFLOW-TEST-DEPENDENCY-DO-NOT-MERGE". 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * refactor: Use PowerShell for input validation steps Convert the validation steps from Bash to PowerShell for consistency with the rest of the workflow which uses PowerShell as its default shell. - Use PowerShell's -notmatch operator instead of Bash regex - Use Write-Output instead of echo - Maintain the same validation logic and error messages 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * docs: Add changelog entry for script injection security fix Add entry to CHANGELOG.md documenting the security improvements to prevent script injection vulnerabilities in the updater workflow. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Apply suggestion from @jpnurmi Co-authored-by: J-P Nurmi <[email protected]> * Apply suggestion from @vaind * Apply suggestion from @vaind * Apply suggestion from @vaind * Apply suggestion from @vaind --------- Co-authored-by: Claude <[email protected]> Co-authored-by: J-P Nurmi <[email protected]>
1 parent 1949ea0 commit b379f7d

File tree

2 files changed

+49
-14
lines changed

2 files changed

+49
-14
lines changed

‎.github/workflows/updater.yml‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,28 @@ jobs:
7272
with:
7373
access_token: ${{ github.token }}
7474

75+
validate-inputs:
76+
runs-on: ubuntu-latest
77+
steps:
78+
- name: Validate dependency name
79+
shell: pwsh
80+
run: |
81+
# Validate that inputs.name contains only safe characters
82+
if ('${{ inputs.name }}' -notmatch '^[a-zA-Z0-9_\./@\s-]+$') {
83+
Write-Output "::error::Invalid dependency name: '${{ inputs.name }}'. Only alphanumeric characters, spaces, and _-./@ are allowed."
84+
exit 1
85+
}
86+
Write-Output "✓ Dependency name '${{ inputs.name }}' is valid"
87+
- name: Validate dependency path
88+
shell: pwsh
89+
run: |
90+
# Validate that inputs.path contains only safe characters
91+
if ('${{ inputs.path }}' -notmatch '^[a-zA-Z0-9_\./-]+$') {
92+
Write-Output "::error::Invalid dependency path: '${{ inputs.path }}'. Only alphanumeric characters and _-./ are allowed."
93+
exit 1
94+
}
95+
Write-Output "✓ Dependency path '${{ inputs.path }}' is valid"
96+
7597
# What we need to accomplish:
7698
# * update to the latest tag
7799
# * create a PR
@@ -90,6 +112,7 @@ jobs:
90112
# We do different approach on subsequent runs because otherwise we would spam users' mailboxes
91113
# with notifications about pushes to existing PRs. This way there is actually no push if not needed.
92114
update:
115+
needs: validate-inputs
93116
runs-on: ${{ inputs.runs-on }}
94117
# Map the job outputs to step outputs
95118
outputs:
@@ -102,6 +125,12 @@ jobs:
102125
defaults:
103126
run:
104127
shell: pwsh
128+
env:
129+
DEPENDENCY_NAME: ${{ inputs.name }}
130+
DEPENDENCY_PATH: ${{ inputs.path }}
131+
DEPENDENCY_PATTERN: ${{ inputs.pattern }}
132+
CHANGELOG_SECTION: ${{ inputs.changelog-section }}
133+
PR_STRATEGY: ${{ inputs.pr-strategy }}
105134
steps:
106135
- uses: actions/checkout@v4
107136
with:
@@ -121,18 +150,18 @@ jobs:
121150
122151
- name: Update to the latest version
123152
id: target
124-
run: ${{ runner.temp }}/ghwf/updater/scripts/update-dependency.ps1 -Path '${{ inputs.path }}' -Pattern '${{ inputs.pattern }}'
153+
run: ${{ runner.temp }}/ghwf/updater/scripts/update-dependency.ps1 -Path $env:DEPENDENCY_PATH -Pattern $env:DEPENDENCY_PATTERN
125154

126155
- name: Get the base repo info
127156
if: steps.target.outputs.latestTag != steps.target.outputs.originalTag
128157
id: root
129158
run: |
130159
$mainBranch = $(git remote show origin | Select-String "HEAD branch: (.*)").Matches[0].Groups[1].Value
131-
$prBranch = switch ('${{ inputs.pr-strategy }}')
160+
$prBranch = switch ($env:PR_STRATEGY)
132161
{
133-
'create' { 'deps/${{ inputs.path }}/${{ steps.target.outputs.latestTag }}' }
134-
'update' { 'deps/${{ inputs.path }}' }
135-
default { throw "Unkown PR strategy '${{ inputs.pr-strategy }}'." }
162+
'create' { "deps/$env:DEPENDENCY_PATH/${{ steps.target.outputs.latestTag }}" }
163+
'update' { "deps/$env:DEPENDENCY_PATH" }
164+
default { throw "Unkown PR strategy '$env:PR_STRATEGY'." }
136165
}
137166
"baseBranch=$mainBranch" | Tee-Object $env:GITHUB_OUTPUT -Append
138167
"prBranch=$prBranch" | Tee-Object $env:GITHUB_OUTPUT -Append
@@ -185,11 +214,11 @@ jobs:
185214
with:
186215
base: ${{ steps.root.outputs.baseBranch }}
187216
branch: ${{ steps.root.outputs.prBranch }}
188-
commit-message: 'chore: update ${{ inputs.path }} to ${{ steps.target.outputs.latestTag }}'
217+
commit-message: 'chore: update ${{ env.DEPENDENCY_PATH }} to ${{ steps.target.outputs.latestTag }}'
189218
author: 'GitHub <[email protected]>'
190-
title: 'chore(deps): update ${{ inputs.name }} to ${{ steps.target.outputs.latestTagNice }}'
219+
title: 'chore(deps): update ${{ env.DEPENDENCY_NAME }} to ${{ steps.target.outputs.latestTagNice }}'
191220
body: |
192-
Bumps ${{ inputs.path }} from ${{ steps.target.outputs.originalTag }} to ${{ steps.target.outputs.latestTag }}.
221+
Bumps ${{ env.DEPENDENCY_PATH }} from ${{ steps.target.outputs.originalTag }} to ${{ steps.target.outputs.latestTag }}.
193222
194223
Auto-generated by a [dependency updater](https://github.com/getsentry/github-workflows/blob/main/.github/workflows/updater.yml).
195224
${{ env.TARGET_CHANGELOG }}
@@ -223,19 +252,19 @@ jobs:
223252

224253
- name: 'After new PR: redo the update'
225254
if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.existing-pr.outputs.url == '') && ( steps.root.outputs.changed == 'false') }}
226-
run: ${{ runner.temp }}/ghwf/updater/scripts/update-dependency.ps1 -Path '${{ inputs.path }}' -Tag '${{ steps.target.outputs.latestTag }}'
255+
run: ${{ runner.temp }}/ghwf/updater/scripts/update-dependency.ps1 -Path $env:DEPENDENCY_PATH -Tag '${{ steps.target.outputs.latestTag }}'
227256

228257
- name: Update Changelog
229258
if: ${{ inputs.changelog-entry && ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }}
230259
run: |
231260
${{ runner.temp }}/ghwf/updater/scripts/update-changelog.ps1 `
232-
-Name '${{ inputs.name }}' `
261+
-Name $env:DEPENDENCY_NAME `
233262
-PR '${{ steps.pr.outputs.url }}' `
234263
-RepoUrl '${{ steps.target.outputs.url }}' `
235264
-MainBranch '${{ steps.target.outputs.mainBranch }}' `
236265
-OldTag '${{ steps.target.outputs.originalTag }}' `
237266
-NewTag '${{ steps.target.outputs.latestTag }}' `
238-
-Section '${{ inputs.changelog-section }}'
267+
-Section $env:CHANGELOG_SECTION
239268
240269
- run: git --no-pager diff
241270
if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }}
@@ -247,11 +276,11 @@ jobs:
247276
with:
248277
base: ${{ steps.root.outputs.baseBranch }}
249278
branch: ${{ steps.root.outputs.prBranch }}
250-
commit-message: 'chore: update ${{ inputs.path }} to ${{ steps.target.outputs.latestTag }}'
279+
commit-message: 'chore: update ${{ env.DEPENDENCY_PATH }} to ${{ steps.target.outputs.latestTag }}'
251280
author: 'GitHub <[email protected]>'
252-
title: 'chore(deps): update ${{ inputs.name }} to ${{ steps.target.outputs.latestTagNice }}'
281+
title: 'chore(deps): update ${{ env.DEPENDENCY_NAME }} to ${{ steps.target.outputs.latestTagNice }}'
253282
body: |
254-
Bumps ${{ inputs.path }} from ${{ steps.target.outputs.originalTag }} to ${{ steps.target.outputs.latestTag }}.
283+
Bumps ${{ env.DEPENDENCY_PATH }} from ${{ steps.target.outputs.originalTag }} to ${{ steps.target.outputs.latestTag }}.
255284
256285
Auto-generated by a [dependency updater](https://github.com/getsentry/github-workflows/blob/main/.github/workflows/updater.yml).
257286
${{ env.TARGET_CHANGELOG }}

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
### Security
6+
7+
- Updater - Prevent script injection vulnerabilities through workflow inputs ([#98](https://github.com/getsentry/github-workflows/pull/98))
8+
39
## 2.13.1
410

511
### Fixes

0 commit comments

Comments
 (0)