-
Notifications
You must be signed in to change notification settings - Fork 124
Potential fix for code scanning alert no. 1035: Environment variable built from user-controlled sources #1480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… user-controlled sources The concern is that the file in the artifact might be created by an attacker to inject values into the env var being set. To address this vulnerability, we should strictly control the value of PR_NUMBER before it is written to $GITHUB_ENV. This means ensuring that: - Only valid, single-line numeric data is used (since PR numbers are always positive integers). - All potential for newlines, extra characters, or injection attacks is removed. - explicitly validate that the value is non-empty and matches expectations = Also, wrap the assignment in double quotes to further protect against expansion. Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses a security vulnerability (code scanning alert #1035) related to environment variable injection in the GitHub Actions workflow. The fix strengthens the validation of PR numbers before they're written to $GITHUB_ENV to prevent potential injection attacks.
Key Changes:
- Replaced
tr -cd '0-9'sanitization with explicitgrep -E '^[0-9]+$'validation that CodeQL recognizes - Added explicit error handling with conditional logic when PR number validation fails
- Simplified the code by removing intermediate variables and using direct file redirection
Co-authored-by: synthead <synthead@github.com>
synthead
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Unit Test Results 1 files 1 suites 10m 25s ⏱️ Results for commit 093b7a1. ♻️ This comment has been updated with latest results. |
Potential fix for https://github.com/github/gh-gei/security/code-scanning/1035
To address this vulnerability, we should strictly control the value of
PR_NUMBERbefore it is written to$GITHUB_ENV. This means ensuring that:This was already being done -
tr -cd '0-9'removes (-d) the complement (-c) of the specified characters. But CodeQL doesn't recognize that, so we'll use its suggested approach (grep -E '^[0-9]+$') instead.Suggested fixes powered by Copilot Autofix. Review carefully before merging.