fix: Prevent script injection in GitHub Actions workflows#150
fix: Prevent script injection in GitHub Actions workflows#150fix-it-felix-sentry[bot] wants to merge 1 commit intomainfrom
Conversation
Fix GitHub Actions script injection vulnerability by using intermediate environment variables instead of direct interpolation of github context data in run steps. Changes: - updater/action.yml: Use env vars for inputs.name, inputs.path, inputs.changelog-entry, inputs.pr-strategy, and inputs.post-update-script - sentry-cli/integration-test/action.yml: Use env vars for github.action_path and inputs.path This prevents potential code injection attacks where untrusted input could be executed as shell commands. Fixes: https://linear.app/getsentry/issue/VULN-1100 Fixes: https://linear.app/getsentry/issue/DI-1657
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Fixes
- Prevent script injection in GitHub Actions workflows ([#150](https://github.com/getsentry/github-workflows/pull/150))If none of the above apply, you can opt out of this check by adding |
| if ("$env:DEPENDENCY_NAME" -notmatch '^[a-zA-Z0-9_\./@\s-]+$') { | ||
| Write-Output "::error::Invalid dependency name: '$env:DEPENDENCY_NAME'. Only alphanumeric characters, spaces, and _-./@ are allowed." |
There was a problem hiding this comment.
Bug: Using double-quoted environment variables in PowerShell allows for subexpression evaluation and command injection before the validation logic is executed.
Severity: CRITICAL
Suggested Fix
Replace double quotes with single quotes when referencing environment variables in PowerShell validation checks. For example, change "$env:DEPENDENCY_NAME" to '$env:DEPENDENCY_NAME'. This ensures PowerShell treats the variable's content as a literal string, preventing subexpression evaluation and command injection.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: updater/action.yml#L80-L81
Potential issue: The use of double-quoted environment variables in PowerShell, such as
`"$env:DEPENDENCY_NAME"`, allows for subexpression evaluation. If an input contains a
payload like `test$(Write-Host 'injected')`, the malicious command within `$()` is
executed before the regex validation runs. This happens because PowerShell expands the
variable and evaluates any subexpressions within it before passing the result to the
`-notmatch` operator. This vulnerability is present in the input validation steps and
subsequent script invocations, undermining the intended security fix.
Did we get this right? 👍 / 👎 to inform future reviews.
Summary
This PR fixes a high-severity security vulnerability where untrusted input from GitHub context data could be directly interpolated into shell commands, allowing potential code injection attacks.
Changes
updater/action.yml
inputs.name,inputs.path,inputs.changelog-entry,inputs.pr-strategy, andinputs.post-update-scriptin validation steps${{ inputs.* }}to usingenv:with quoted environment variable references"$env:VAR_NAME"sentry-cli/integration-test/action.yml
github.action_pathandinputs.pathenv:with quoted environment variable referencesSecurity Impact
The previous implementation was vulnerable to script injection because GitHub context data (like
inputs.*andgithub.*) can contain arbitrary user input. By directly interpolating these values into shell commands, an attacker could potentially:The fix uses intermediate environment variables which prevents the injection by treating the input as data rather than executable code.
References
Testing
The existing test suite should validate that the functionality remains intact. The validation logic is unchanged, only the method of passing the values to the shell has been made secure.