-
Notifications
You must be signed in to change notification settings - Fork 33
fix shell/code injection in pr-review.yaml #296
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,10 +17,16 @@ jobs: | |||||||||||||||||||||||||
| contains(github.event.comment.body, '/gemini-review') | ||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||
| - name: PR Info | ||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||
| #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 | ||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||
| echo "Comment: ${{ github.event.comment.body }}" | ||||||||||||||||||||||||||
| echo "Issue Number: ${{ github.event.issue.number }}" | ||||||||||||||||||||||||||
| echo "Repository: ${{ github.repository }}" | ||||||||||||||||||||||||||
| echo "Comment: $COMMENT_BODY" | ||||||||||||||||||||||||||
| echo "Issue Number: $ISSUE_NUM" | ||||||||||||||||||||||||||
| echo "Repository: $REPO" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - name: Checkout Repo | ||||||||||||||||||||||||||
| uses: actions/checkout@v3 | ||||||||||||||||||||||||||
|
|
@@ -30,17 +36,20 @@ jobs: | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - name: Get PR Details | ||||||||||||||||||||||||||
| id: pr | ||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||
| PR_JSON=$(gh api repos/${{ github.repository }}/pulls/${{ github.event.issue.number }}) | ||||||||||||||||||||||||||
| echo "head_sha=$(echo $PR_JSON | jq -r .head.sha)" >> $GITHUB_OUTPUT | ||||||||||||||||||||||||||
| echo "base_sha=$(echo $PR_JSON | jq -r .base.sha)" >> $GITHUB_OUTPUT | ||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||
| 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: | | ||||||||||||||||||||||||||
|
Comment on lines
40
to
+45
|
||||||||||||||||||||||||||
| 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 |
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.
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.