Skip to content

Feat: Automatic AI PR review#696

Open
dzikowski wants to merge 2 commits into
mainfrom
feat/automatic-pr-review
Open

Feat: Automatic AI PR review#696
dzikowski wants to merge 2 commits into
mainfrom
feat/automatic-pr-review

Conversation

@dzikowski
Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: Jakub Dzikowski <jakub.t.dzikowski@gmail.com>
Signed-off-by: Jakub Dzikowski <jakub.t.dzikowski@gmail.com>
Comment on lines +64 to +72
run: |
set -euo pipefail
pr="${{ inputs.pr_number }}"
review_file=".jaiph/tmp/${pr}-review.md"
test -f "${review_file}"
# temporary: print the review file to the console
# gh pr comment "${pr}" --body-file "${review_file}"
cat "${review_file}"
rm "${review_file}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the implementation of the pr variable is vulnerable to shell injection. (docs:https://securitylab.github.com/resources/github-actions-untrusted-input/#:~:text=Script%20injections,not%20vulnerable%20to%20the%20injection.)
Better to use the env var, not the ${{ }} syntax.

Suggested change
run: |
set -euo pipefail
pr="${{ inputs.pr_number }}"
review_file=".jaiph/tmp/${pr}-review.md"
test -f "${review_file}"
# temporary: print the review file to the console
# gh pr comment "${pr}" --body-file "${review_file}"
cat "${review_file}"
rm "${review_file}"
env:
# map input to an environment variable
PR_NUMBER: ${{ inputs.pr_number }}
run: |
set -euo pipefail
# use the env var, not the ${{ }} syntax
pr="$PR_NUMBER"
review_file=".jaiph/tmp/${pr}-review.md"
if [[ -f "$review_file" ]]; then
cat "$review_file"
rm "$review_file"
else
echo "Review file not found for PR $pr"
exit 1
fi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants