fix(workflows): Prevent shell injection in fast-revert workflow - #4309
Conversation
Use environment variables to store GitHub context data instead of directly interpolating them in the run script. This prevents potential script injection attacks. Changes: - Move GitHub context variables to env block - Reference variables using proper shell variable syntax with quotes - Apply to: github.repository, github.run_id, github.event.repository.id, and github.event.number/github.event.inputs.pr Fixes: https://linear.app/getsentry/issue/VULN-1575 Fixes: https://linear.app/getsentry/issue/DI-1872 Co-Authored-By: fix-it-felix-sentry[bot] <260785270+fix-it-felix-sentry[bot]@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7c6ae0e. Configure here.
| env: | ||
| GITHUB_TOKEN: ${{ secrets.BUMP_SENTRY_TOKEN }} | ||
| GITHUB_REPOSITORY: ${{ github.repository }} | ||
| GITHUB_RUN_ID: ${{ github.run_id }} |
There was a problem hiding this comment.
Reserved env var names override silently ignored
Low Severity
GITHUB_REPOSITORY and GITHUB_RUN_ID are reserved GitHub Actions default environment variables that the documentation states cannot be overwritten. Setting them in the step-level env: block is silently ignored at runtime. This works today only because the default values happen to match what's being assigned, but using reserved names is unnecessary and could break if GitHub starts enforcing this restriction with a hard error. Using non-reserved names (e.g., GH_REPO, RUN_ID) would be more robust.
Reviewed by Cursor Bugbot for commit 7c6ae0e. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| token: ${{ secrets.BUMP_SENTRY_TOKEN }} | ||
| - name: comment on failure | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.BUMP_SENTRY_TOKEN }} |
There was a problem hiding this comment.
Bug: The workflow attempts to overwrite the reserved GITHUB_TOKEN environment variable. This is ignored, and the subsequent API call will use a token with no permissions, causing it to fail.
Severity: MEDIUM
Suggested Fix
Instead of trying to overwrite GITHUB_TOKEN, use a different environment variable name for your PAT, for example, GH_TOKEN. Then, update the curl command to use this new variable: Authorization: Bearer ${{ env.GH_TOKEN }}.
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: .github/workflows/fast-revert.yml#L34
Potential issue: The workflow at `.github/workflows/fast-revert.yml` attempts to
overwrite the reserved `GITHUB_TOKEN` environment variable with a secret PAT at line 34.
GitHub Actions silently ignores this assignment. Because the workflow also specifies
`permissions: {}` at line 11, the default `GITHUB_TOKEN` that is ultimately used has no
permissions. As a result, the `curl` command that relies on this token to post a comment
to the pull request will fail with a 403 error, preventing the failure notification from
being sent.
Did we get this right? 👍 / 👎 to inform future reviews.
1 participant
Summary
This PR fixes a security finding related to potential shell injection in the
fast-revert.ymlGitHub Actions workflow.Changes
github.repository,github.run_id,github.event.repository.id,github.event.number,github.event.inputs.pr) to anenv:blockrun:script to reference these values as environment variables with proper quotingReferences
Testing
Security Impact
While the Semgrep Assistant indicated this might be a false positive (since the variables used are mostly GitHub-controlled identifiers), following security best practices by using environment variables provides defense-in-depth and prevents any potential injection vectors.