fix shell/code injection in pr-review.yaml#296
fix shell/code injection in pr-review.yaml#296pradeeban merged 1 commit intoControlCore-Project:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes a code injection vulnerability in the GitHub Actions workflow file .github/workflows/PR-review.yaml by properly handling untrusted user inputs. The workflow is triggered by issue comments and previously used direct template expansion of user-controlled values (like github.event.comment.body and github.event.issue.number) within shell run: blocks, which could allow malicious users to inject arbitrary shell commands.
Changes:
- Modified the "PR Info" step to assign untrusted GitHub event inputs to environment variables before using them in shell commands
- Modified the "Get PR Details" step to use environment variables for API calls instead of direct template expansion
- Removed trailing whitespace in the workflow file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #Assign untrusted inputs to environment variables first | ||
| COMMENT_BODY: ${{ github.event.comment.body }} | ||
| ISSUE_NUM: ${{ github.event.issue.number }} | ||
| REPO: ${{ github.repository }} | ||
| #Use shell variables ("$VAR") instead of template tags |
There was a problem hiding this comment.
The comment placement here is confusing. Line 21's comment appears between the env: key and its values, making the structure unclear. Consider either: (1) indenting it to align with the environment variables (e.g., " #Assign..."), (2) placing it on the same line as env: (e.g., "env: #Assign..."), or (3) moving it above the env: line. Similarly, line 25's comment should be moved inside the run block or placed differently to improve readability.
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| REPO: ${{ github.repository }} | ||
| ISSUE_NUM: ${{ github.event.issue.number }} | ||
| #Use env vars for the API call to prevent injection | ||
| #Use quotes around variables to prevent word splitting | ||
| run: | |
There was a problem hiding this comment.
The comment placement here is confusing. Lines 43-44 appear at the wrong indentation level, between the environment variable definitions and the run: key. Consider indenting these comments to align with the environment variables (e.g., " #Use env vars...") to improve readability and make the YAML structure clearer.
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| REPO: ${{ github.repository }} | |
| ISSUE_NUM: ${{ github.event.issue.number }} | |
| #Use env vars for the API call to prevent injection | |
| #Use quotes around variables to prevent word splitting | |
| run: | | |
| # Use env vars for the API call to prevent injection | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| REPO: ${{ github.repository }} | |
| ISSUE_NUM: ${{ github.event.issue.number }} | |
| run: | | |
| # Use quotes around variables to prevent word splitting |
2 participants
@pradeeban, done with the changes, please have a look. This fixes #295