Harden peer-review GitHub Action by removing github.actor trust checks and validating reviewers via API #5389
Conversation
Removed conditions for excluded GitHub actors in the peer review check.
|
👋 Hi @S3DFX-CYBER! This pull request needs a peer review before it can be merged. Please request a review from a team member who is not:
Once a valid peer review is submitted, this check will pass automatically. Thank you! |
WalkthroughRefactors the peer-review GitHub Actions workflow: consolidates triggers, moves actor/exclusion checks into the script (including Dependabot), derives PR author, filters APPROVED reviews to form VALID_REVIEWERS (excludes author, bots, and logins containing "copilot"), simplifies comment detection/posting, and directly manages the final peer-review label. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Monthly LeaderboardHi @S3DFX-CYBER! Here's how you rank for January 2026:
Leaderboard based on contributions in January 2026. Keep up the great work! 🚀 |
❌ Pre-commit checks failedThe pre-commit hooks found issues that need to be fixed. Please run the following commands locally to fix them: # Install pre-commit if you haven't already
pip install pre-commit
# Run pre-commit on all files
pre-commit run --all-files
# Or run pre-commit on staged files only
pre-commit runAfter running these commands, the pre-commit hooks will automatically fix most issues. 💡 Tip: You can set up pre-commit to run automatically on every commit by running: pre-commit installPre-commit outputFor more information, see the pre-commit documentation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/check-peer-review.yml (2)
64-67: Consider more robust error handling.The current error detection uses string matching on the response body, which could miss other error conditions. Consider checking HTTP status codes for more reliable error detection.
🔎 Proposed improvement
# Get all reviews for the PR -REVIEWS_RESPONSE=$(curl -s -X GET \ +REVIEWS_RESPONSE=$(curl -s -w "\n%{http_code}" -X GET \ -H "Authorization: token $GITHUB_TOKEN" \ -H "Accept: application/vnd.github.v3+json" \ "https://api.github.com/repos/$REPO_OWNER/$REPO_NAME/pulls/$PR_NUMBER/reviews") +HTTP_CODE=$(echo "$REVIEWS_RESPONSE" | tail -n 1) +REVIEWS_BODY=$(echo "$REVIEWS_RESPONSE" | sed '$d') + # Check if the API request was successful -if [[ "$REVIEWS_RESPONSE" == *"message"*"Not Found"* ]] || [[ "$REVIEWS_RESPONSE" == *"Resource not accessible by integration"* ]]; then - echo "Error: Could not fetch PR reviews. Response: $REVIEWS_RESPONSE" +if [ "$HTTP_CODE" -ne 200 ]; then + echo "Error: Could not fetch PR reviews. HTTP Code: $HTTP_CODE, Response: $REVIEWS_BODY" exit 1 fiThen use
$REVIEWS_BODYinstead of$REVIEWS_RESPONSEin subsequent jq commands.
69-76: Consider adding case-insensitive matching for more robust bot exclusion.Line 75 uses
contains("copilot"), which is case-sensitive. While GitHub's Copilot bot appears to use lowercase, applying case-insensitive matching makes this logic more resilient:Suggested improvement
-select(.user.login | contains("copilot") | not) | +select(.user.login | ascii_downcase | contains("copilot") | not) |The "coderabbitai[bot]" exclusion on line 74 follows GitHub's standard bot naming convention.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/check-peer-review.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/check-peer-review.yml
32-32: could not parse as YAML: did not find expected key
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (2)
.github/workflows/check-peer-review.yml (2)
86-128: Excellent idempotent commenting implementation.The use of a unique marker (
<!-- peer-review-check -->) to prevent duplicate comments is a best practice. The implementation correctly:
- Checks for existing comments before posting
- Uses jq to construct valid JSON payloads
- Captures and validates HTTP status codes
- Provides clear feedback to users
38-76: Strong security improvement by using API-based validation.The refactored approach addresses the security concerns outlined in the PR objectives:
- Eliminates actor-based trust: No longer relies on
github.actorwhich can be inaccurate inpull_request_targetcontexts- API as source of truth: Fetches PR metadata and reviews directly from the GitHub API
- Explicit validation: Checks actual review state (APPROVED) and reviewer identity
- Clear exclusion rules: Filters out the PR author, bots, and specific users
This implementation aligns with GitHub Actions security best practices and significantly reduces the risk of unauthorized bypasses.
github-project-automation
bot
moved this from Backlog
to Ready
in 📌 OWASP BLT Project Board
github-actions
bot
added
the
changes-requested
❌ Pre-commit checks failedThe pre-commit hooks found issues that need to be fixed. Please run the following commands locally to fix them: # Install pre-commit if you haven't already
pip install pre-commit
# Run pre-commit on all files
pre-commit run --all-files
# Or run pre-commit on staged files only
pre-commit runAfter running these commands, the pre-commit hooks will automatically fix most issues. 💡 Tip: You can set up pre-commit to run automatically on every commit by running: pre-commit installPre-commit outputFor more information, see the pre-commit documentation. |
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/check-peer-review.yml (1)
30-33: Fix YAML syntax error on lines 32-33.Lines 32-33 contain trailing whitespace and/or incorrect indentation that prevents YAML parsing. The workflow will fail to execute until this is resolved.
🔎 Proposed fix
Remove all content from lines 32-33 (or ensure proper indentation):
- name: Install jq run: sudo apt-get update && sudo apt-get install -y jq - - + - name: Check for Peer Review and Add Label
🧹 Nitpick comments (1)
.github/workflows/check-peer-review.yml (1)
70-77: Consider refining the "copilot" exclusion filter.Line 76 uses
contains("copilot")which will exclude any username containing the substring "copilot" (e.g., "my-copilot-test", "copilot-user", etc.). If the intent is only to exclude Copilot bot accounts, consider using a more specific pattern.🔎 Alternative: More specific Copilot bot filter
If you want to exclude only GitHub Copilot bots specifically:
-select(.user.login | contains("copilot") | not) | +select(.user.login | test("copilot.*\\[bot\\]$") | not) |Or to exclude both bots and the copilot service account:
-select(.user.login | contains("copilot") | not) | +select(.user.login | test("^(.*copilot.*\\[bot\\]|copilot)$"; "i") | not) |However, if the broad exclusion is intentional to catch all copilot-related accounts, the current implementation is fine.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/check-peer-review.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/check-peer-review.yml
33-33: could not parse as YAML: did not find expected key
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: remove_last_active_label
- GitHub Check: auto-assign
- GitHub Check: remove_last_active_label
- GitHub Check: add_changes_requested_label
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (1)
.github/workflows/check-peer-review.yml (1)
38-135: Security improvements are well-implemented.The shift from actor-based trust to API-driven validation significantly improves security:
- Prevents bypasses via event metadata manipulation
- Validates reviews against actual review state
- Implements proper exclusion logic with self-approval prevention
- Uses idempotent commenting to avoid spam
|
Hi, the peer-review is meant for contributors and the actors in the file is either maintainer or AI bot whose review shouldn't be considered. Hence the exclusion. |
Add condition to skip peer review for Dependabot PRs.
github-actions
bot
removed
last-active: 0d
❌ Pre-commit checks failedThe pre-commit hooks found issues that need to be fixed. Please run the following commands locally to fix them: # Install pre-commit if you haven't already
pip install pre-commit
# Run pre-commit on all files
pre-commit run --all-files
# Or run pre-commit on staged files only
pre-commit runAfter running these commands, the pre-commit hooks will automatically fix most issues. 💡 Tip: You can set up pre-commit to run automatically on every commit by running: pre-commit installPre-commit outputFor more information, see the pre-commit documentation. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/check-peer-review.yml (1)
30-33: CRITICAL: Fix YAML syntax error preventing workflow execution.The static analysis tool reports a YAML parsing error on line 33 caused by trailing whitespace and incorrect indentation on lines 32-33. This prevents the workflow from running at all and is a blocker for merging.
🔎 Proposed fix
Remove trailing whitespace and fix indentation:
- name: Install jq run: sudo apt-get update && sudo apt-get install -y jq - - + - name: Check for Peer Review and Add LabelNote: This issue was previously flagged and must be resolved before the PR can be merged.
🧹 Nitpick comments (1)
.github/workflows/check-peer-review.yml (1)
77-84: Consider case-insensitive matching for copilot exclusion.The current filter uses case-sensitive matching for "copilot". While GitHub usernames are case-sensitive and copilot bots likely use lowercase, consider using
ascii_downcasefor defense in depth.🔎 Optional enhancement
VALID_REVIEWERS=$(echo "$REVIEWS_RESPONSE" | jq -r --arg author "$PR_AUTHOR" \ '.[] | select(.state == "APPROVED") | select(.user.login != $author) | select(.user.login != "DonnieBLT") | select(.user.login != "coderabbitai[bot]") | - select(.user.login | contains("copilot") | not) | + select(.user.login | ascii_downcase | contains("copilot") | not) | .user.login' | sort -u)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/check-peer-review.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/check-peer-review.yml
33-33: could not parse as YAML: did not find expected key
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (2)
.github/workflows/check-peer-review.yml (2)
58-63: Good fix for dependabot PRs.This early-exit logic correctly addresses the concern raised in previous reviews about dependabot PRs being incorrectly flagged with
needs-peer-review. By settinghas_peer_review=trueand exiting early, dependabot PRs can proceed with auto-merge as intended.
38-142: Excellent security hardening aligns with GitHub Actions best practices.The refactored implementation successfully eliminates trust decisions based on
github.actorand instead validates reviewers using the GitHub API as the source of truth. Key security improvements include:
- Explicit PR author identification via API (line 55)
- Review validation based on actual API state rather than event metadata (lines 66-69)
- Idempotent commenting prevents duplicate notifications (lines 101-136)
- No PR code is checked out or executed (maintaining security boundary)
This approach prevents potential bypass scenarios where workflows triggered by trusted actors or automation could circumvent review requirements.
Based on PR objectives: This implementation achieves the stated goal of hardening peer-review enforcement by removing reliance on mutable event metadata.
❌ Pre-commit checks failedThe pre-commit hooks found issues that need to be fixed. Please run the following commands locally to fix them: # Install pre-commit if you haven't already
pip install pre-commit
# Run pre-commit on all files
pre-commit run --all-files
# Or run pre-commit on staged files only
pre-commit runAfter running these commands, the pre-commit hooks will automatically fix most issues. 💡 Tip: You can set up pre-commit to run automatically on every commit by running: pre-commit installPre-commit outputFor more information, see the pre-commit documentation. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/check-peer-review.yml (1)
30-32: YAML syntax error still present on line 32.The trailing whitespace on line 32 that was flagged in previous reviews still needs to be removed to fix the YAML parsing error.
🧹 Nitpick comments (1)
.github/workflows/check-peer-review.yml (1)
87-87: Minor inconsistency in echo statement.The echo statement mentions "coderabbitai" without the
[bot]suffix, while line 82 filters for "coderabbitai[bot]". Consider updating the echo for consistency:-echo "Valid approved reviewers (excluding author, DonnieBLT, coderabbitai, and copilot): $VALID_REVIEWERS" +echo "Valid approved reviewers (excluding author, DonnieBLT, coderabbitai[bot], and copilot): $VALID_REVIEWERS"
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/check-peer-review.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (2)
.github/workflows/check-peer-review.yml (2)
58-63: Excellent fix for the dependabot auto-merge issue.This early-exit logic correctly addresses the bug flagged in previous reviews where dependabot PRs were incorrectly labeled as
needs-peer-review. By marking them as peer-reviewed before the review check, dependabot PRs will now receive thehas-peer-reviewlabel and proceed with auto-merge as intended.
76-84: Robust reviewer filtering implementation.The inline jq filter correctly implements all exclusions mentioned in the PR objectives:
- Self-approvals (PR author)
- Specific trusted users (DonnieBLT)
- Bot accounts (coderabbitai[bot], copilot variants)
The use of
--argfor parameter passing and chainedselect()statements is idiomatic and secure.
github-actions
bot
removed
the
changes-requested
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/check-peer-review.yml (2)
98-141: Critical: Undefined variable$COMMENTS_RESPONSEcauses script failure.Line 104 references
$COMMENTS_RESPONSE, but this variable is never defined. The script does not fetch PR comments from the API before attempting to search for existing comments. This will cause the command to fail and break the idempotent commenting logic.🔎 Proposed fix
Add a curl call to fetch PR comments before line 104:
echo "Reviews found: $(echo "$REVIEWS_RESPONSE" | jq length)" echo "Valid approved reviewers (excluding author, DonnieBLT, coderabbitai, and copilot): $VALID_REVIEWERS" - # Check if there are any valid reviewers - if [ -z "$VALID_REVIEWERS" ]; then - echo "No peer review found from a valid reviewer." - echo "has_peer_review=false" >> "$GITHUB_OUTPUT" - - # 1. Search for the specific fingerprint marker (Don't leave this empty!) - EXISTING_COMMENT=$(echo "$COMMENTS_RESPONSE" | jq -r '.[] | select(.body | contains("<!-- peer-review-notification -->")) | .id' | head -n 1) + # Check if there are any valid reviewers + if [ -z "$VALID_REVIEWERS" ]; then + echo "No peer review found from a valid reviewer." + echo "has_peer_review=false" >> "$GITHUB_OUTPUT" + + # Fetch existing comments to check if we already posted + COMMENTS_RESPONSE=$(curl -s -X GET \ + -H "Authorization: token $GITHUB_TOKEN" \ + -H "Accept: application/vnd.github.v3+json" \ + "https://api.github.com/repos/$REPO_OWNER/$REPO_NAME/issues/$PR_NUMBER/comments") + + # Search for the specific fingerprint marker + EXISTING_COMMENT=$(echo "$COMMENTS_RESPONSE" | jq -r '.[] | select(.body | contains("<!-- peer-review-notification -->")) | .id' | head -n 1)Also fix the inconsistent indentation on lines 99-101 and 137-141 (these lines have extra leading spaces).
143-227: Critical: YAML indentation error breaks workflow syntax.The "Add peer review status label" step (lines 143-227) has incorrect indentation. This entire step block is nested too deeply, making it appear to be part of the previous step's
runblock rather than a separate step in the workflow. This will cause a YAML parsing error and prevent the workflow from executing.Additionally, lines 144-147 have inconsistent indentation with extra leading spaces before
if,uses, andwith.🔎 Proposed fix
The entire block from lines 143-227 needs to be dedented to align with the previous step at line 41. The step should look like this:
- - name: Add peer review status label - if: always() - uses: actions/github-script@v7 - with: + - name: Add peer review status label + if: always() + uses: actions/github-script@v7 + with: github-token: ${{ secrets.GITHUB_TOKEN }} script: |All subsequent lines (148-227) should maintain the corrected indentation hierarchy.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/check-peer-review.yml
🔇 Additional comments (3)
.github/workflows/check-peer-review.yml (3)
38-40: LGTM: Explicit jq installation.Explicitly installing
jqensures the required tool is available regardless of runner image variations.
65-70: LGTM: Dependabot exclusion correctly implemented.The early skip for Dependabot PRs is correctly placed after fetching the PR author from the API and properly sets
has_peer_review=trueto prevent blocking automated dependency updates.
84-96: LGTM: Comprehensive reviewer filtering.The filtering logic correctly excludes self-approvals, bot accounts, and maintainers from the peer-review validation. The case-insensitive substring check for "copilot" is properly implemented using
ascii_downcase.
github-actions
bot
added
tests: failed
github-actions
bot
added
changes-requested
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/check-peer-review.yml (1)
62-62: Critical:contains("")matches any comment, breaking notification logic.Line 62 uses
contains("")which matches any non-null string. When any comment exists on the PR,EXISTING_COMMENTwill be populated with the first comment ID, causing line 64's check to fail and preventing the workflow from posting the required peer-review notification.This is the same critical bug flagged in multiple past reviews (Reference IDs: 8136523, 8139310).
🔎 Proposed fix with unique marker
Add a unique HTML marker to the comment body:
if [ -z "$EXISTING_COMMENT" ]; then - jq -n --arg body "👋 Hi @${PR_AUTHOR}! This PR needs a peer review from a maintainer to proceed." '{body: $body}' > /tmp/comment.json + jq -n --arg body "👋 Hi @${PR_AUTHOR}! This PR needs a peer review from a maintainer to proceed. + +<!-- peer-review-notification -->" '{body: $body}' > /tmp/comment.jsonThen update the check to look for this marker:
- EXISTING_COMMENT=$(echo "$COMMENTS_RESPONSE" | jq -r '.[] | select(.body | contains("")) | .id' | head -n 1) + EXISTING_COMMENT=$(echo "$COMMENTS_RESPONSE" | jq -r '.[] | select(.body | contains("<!-- peer-review-notification -->")) | .id' | head -n 1)
🧹 Nitpick comments (1)
.github/workflows/check-peer-review.yml (1)
47-53: Consider excluding dependabot variants from valid reviewers.The filter excludes the PR author, DonnieBLT, coderabbitai[bot], and copilot variants, but does not exclude dependabot variants. While dependabot PRs are handled at the author level (lines 38-41), it's prudent to also filter dependabot from reviewers for consistency and defense-in-depth.
🔎 Proposed addition to reviewer filter
VALID_REVIEWERS=$(echo "$REVIEWS_RESPONSE" | jq -r --arg author "$PR_AUTHOR" ' .[] | select(.state == "APPROVED") | select(.user.login != $author) | select(.user.login != "DonnieBLT") | select(.user.login != "coderabbitai[bot]") | + select(.user.login != "dependabot[bot]") | + select(.user.login != "dependabot-preview[bot]") | + select(.user.login != "dependabot") | select(.user.login | ascii_downcase | contains("copilot") | not) | .user.login' | sort -u)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/check-peer-review.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (3)
.github/workflows/check-peer-review.yml (3)
5-7: LGTM! Trigger type formatting is concise.The inline array format for trigger types is valid and more concise than multi-line format. No functional change.
32-36: LGTM! API-based PR author detection is correct.Fetching the PR author via API rather than relying on
github.actoraligns with the PR's security objectives.
74-91: LGTM! Label management logic is clean and correct.The label synchronization correctly determines the target label based on peer-review status, removes the old label (with error handling), and adds the new label. The implementation is straightforward and handles edge cases properly.
Added additional bot exclusions for peer review checks.
| jq -n \ | ||
| --arg body "<!-- peer-review-check --> | ||
| 👋 Hi @${PR_AUTHOR}! | ||
| EXISTING_COMMENT=$(echo "$COMMENTS_RESPONSE" | jq -r '.[] | select(.body | contains("")) | .id' | head -n 1) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/check-peer-review.yml (1)
66-66: Critical:contains("")matches any comment, breaking idempotent notification.Line 66 uses
contains("")which matches any non-null string in jq. When any comment exists on the PR,EXISTING_COMMENTwill be populated with the first comment ID, causing the check on line 68 to fail and preventing the workflow from posting the required peer-review notification.This is the same critical bug flagged in past reviews (Reference ID: 8136523, 8139310).
🔎 Proposed fix
Add a unique HTML comment marker to the posted comment and check for it:
- EXISTING_COMMENT=$(echo "$COMMENTS_RESPONSE" | jq -r '.[] | select(.body | contains("")) | .id' | head -n 1) + EXISTING_COMMENT=$(echo "$COMMENTS_RESPONSE" | jq -r '.[] | select(.body | contains("<!-- peer-review-check -->")) | .id' | head -n 1)And update the comment body to include the marker:
if [ -z "$EXISTING_COMMENT" ]; then - jq -n --arg body "👋 Hi @${PR_AUTHOR}! This PR needs a peer review from a maintainer to proceed." '{body: $body}' > /tmp/comment.json + jq -n --arg body "<!-- peer-review-check --> +👋 Hi @${PR_AUTHOR}! This PR needs a peer review from a maintainer to proceed." '{body: $body}' > /tmp/comment.jsonThis ensures the workflow only detects its own comment and won't spam duplicate notifications.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/check-peer-review.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run Tests
- GitHub Check: Run pre-commit
- GitHub Check: Analyze (python)
- GitHub Check: docker-test
🔇 Additional comments (3)
.github/workflows/check-peer-review.yml (3)
5-7: LGTM: Trigger definitions are correctly formatted.The inline array format for event types is clear and covers the appropriate events for peer-review validation.
47-57: LGTM: Review filtering logic is correct.The API-based reviewer validation properly excludes self-approvals, known bots, and copilot variants using case-insensitive matching. The jq filter correctly identifies APPROVED reviews from distinct, non-excluded reviewers.
78-95: LGTM: Label management is correctly simplified.The streamlined approach correctly determines the target label based on review status, removes the opposite label in a fault-tolerant way, and applies the final label. The
if: always()ensures labels are updated consistently.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
github-actions
bot
removed
the
changes-requested
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/check-peer-review.yml (1)
45-49: Expand PR author exclusion list to match reviewer filtering.The exclusion only checks for
dependabot[bot]anddependabot, but lines 58-60 filter additional actors (DonnieBLT, copilot variants) from reviewers. For consistency and to match the PR objectives, PRs from these actors should also skip peer-review enforcement.🔎 Proposed fix
- # 2. Skip for Dependabot - if [[ "$PR_AUTHOR" == "dependabot[bot]" || "$PR_AUTHOR" == "dependabot" ]]; then + # 2. Skip for bots and excluded maintainers + if [[ "$PR_AUTHOR" == "dependabot[bot]" || + "$PR_AUTHOR" == "dependabot-preview[bot]" || + "$PR_AUTHOR" == "dependabot" || + "$PR_AUTHOR" == "DonnieBLT" ]] || + [[ "$PR_AUTHOR" =~ copilot ]]; then echo "has_peer_review=true" >> "$GITHUB_OUTPUT" exit 0 fiBased on past review comments and PR objectives.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/check-peer-review.yml
🧰 Additional context used
🪛 GitHub Actions: Check Peer Review
.github/workflows/check-peer-review.yml
[error] 1-1: Could not fetch PR reviews. API response indicates Not Found or access issue (curl GET to fetch reviews for PR #5389).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run Tests
- GitHub Check: Run pre-commit
- GitHub Check: docker-test
🔇 Additional comments (2)
.github/workflows/check-peer-review.yml (2)
32-41: Good error handling for PR details fetch.The HTTP status check and error exit ensure the script fails fast if the API call fails, preventing downstream issues with invalid data.
88-127: Label management logic is well-structured.The implementation properly:
- Determines the appropriate label based on review status
- Ensures labels exist before use via
ensureLabelExists(addressing past Sentry bug 8141493)- Removes conflicting labels with error suppression
- Applies the correct final label
github-actions
bot
added
changes-requested
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @.github/workflows/check-peer-review.yml:
- Line 96: The string literal around the GitHub Actions expression is broken for
the variable hasReview; fix the quote placement so the entire expression
steps.check_peer_review.outputs.has_peer_review is wrapped as a single string
before comparing to 'true' (i.e., move the stray single quote that currently
appears before the template expression to after it so the comparison uses the
full interpolated value).
- Line 137: There is a duplicated closing bracket/token sequence "});" at the
end of the workflow which causes a JavaScript/YAML syntax error; remove the
extra "});" so only one closing "});" remains to properly close the preceding
block and restore valid workflow syntax.
♻️ Duplicate comments (1)
.github/workflows/check-peer-review.yml (1)
45-49: Expand PR author exclusion list to include all bot and maintainer variants.The exclusion logic only checks for
dependabot[bot]anddependabot, but based on the PR objectives and past review comments, PRs from other bots and maintainers should also skip peer review:
- Missing
dependabot-preview[bot](mentioned in other workflows)- Missing
DonnieBLT(maintainer mentioned in PR comments)- Missing copilot variants (filtered from reviewers on line 66 but not from PR authors)
🔎 Proposed fix
- # 2. Skip for Dependabot - if [[ "$PR_AUTHOR" == "dependabot[bot]" || "$PR_AUTHOR" == "dependabot" ]]; then + # 2. Skip for bots and maintainers + if [[ "$PR_AUTHOR" == "dependabot[bot]" || + "$PR_AUTHOR" == "dependabot-preview[bot]" || + "$PR_AUTHOR" == "dependabot" || + "$PR_AUTHOR" == "DonnieBLT" || + "$PR_AUTHOR" =~ copilot ]]; then echo "has_peer_review=true" >> "$GITHUB_OUTPUT" exit 0 fiNote: Using
=~for copilot enables regex matching, or use== *copilot*for glob matching (without quotes around the pattern).Based on learnings from past review comments requesting expansion of exclusions.
🧹 Nitpick comments (1)
.github/workflows/check-peer-review.yml (1)
76-76: Minor: Remove leading whitespace for consistency.Line 76 has a leading space before the variable assignment. While this doesn't affect functionality, it's inconsistent with the rest of the script formatting.
🔎 Proposed fix
- EXISTING_COMMENT=$(echo "$COMMENTS_RESPONSE" | jq -r '.[] | select(.body | contains("<!-- peer-review-check -->")) | .id' | head -n 1) + EXISTING_COMMENT=$(echo "$COMMENTS_RESPONSE" | jq -r '.[] | select(.body | contains("<!-- peer-review-check -->")) | .id' | head -n 1)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
.github/workflows/check-peer-review.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (2)
.github/workflows/check-peer-review.yml (2)
32-43: LGTM! Robust error handling for PR details fetch.The error handling pattern correctly captures the HTTP status code, validates the response, and exits early on failures. This addresses the critical concern raised in past reviews about handling API errors gracefully.
51-59: LGTM! Error handling added for reviews API call.The addition of HTTP status checking for the reviews API call addresses the critical pipeline failure mentioned in past reviews. The error handling pattern matches the one used for PR details fetch, ensuring consistency.
| async function ensureLabelExists(labelName, color, desc) { | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| script: | | ||
| const hasReview = '${{ steps.check_peer_review.outputs.has_peer_review }} === 'true'; |
There was a problem hiding this comment.
Critical: Fix JavaScript string syntax error.
Line 96 has a malformed string due to incorrect quote placement. The single quote is placed before the template variable instead of wrapping the entire value, causing a syntax error that will prevent the workflow from running.
🔎 Proposed fix
- const hasReview = '${{ steps.check_peer_review.outputs.has_peer_review }} === 'true';
+ const hasReview = '${{ steps.check_peer_review.outputs.has_peer_review }}' === 'true';The closing single quote must come after the GitHub Actions expression, not in the middle of it.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hasReview = '${{ steps.check_peer_review.outputs.has_peer_review }} === 'true'; | |
| const hasReview = '${{ steps.check_peer_review.outputs.has_peer_review }}' === 'true'; |
🤖 Prompt for AI Agents
In @.github/workflows/check-peer-review.yml around line 96, The string literal
around the GitHub Actions expression is broken for the variable hasReview; fix
the quote placement so the entire expression
steps.check_peer_review.outputs.has_peer_review is wrapped as a single string
before comparing to 'true' (i.e., move the stray single quote that currently
appears before the template expression to after it so the comparison uses the
full interpolated value).
2 participants
Summary
This PR hardens the peer-review enforcement workflow by removing reliance on github.actor–based trust checks and instead validating peer reviews directly via the GitHub Pull Request Reviews API.
This prevents potential trust bypass scenarios and ensures that peer-review enforcement is based on verifiable review data rather than event-trigger metadata.
Problem
The previous implementation relied on github.actor to determine whether a workflow should run or skip certain checks.
However:
github.actor represents the entity that triggered the workflow, not necessarily the PR author or reviewer
In pull_request / pull_request_review contexts, this can lead to incorrect trust assumptions
A malicious contributor could potentially trigger workflows through trusted actors (bots or automation), bypassing intended checks
This pattern is documented as a security concern in GitHub Actions when used for authorization or trust decisions.
Solution
This PR replaces actor-based checks with:
Direct retrieval of PR metadata via GitHub API
Explicit identification of the PR author
Validation of approved peer reviews from distinct, non-excluded reviewers
Filtering of bot accounts and self-approvals
Idempotent commenting using a unique marker to avoid duplicate messages
The workflow now enforces peer review strictly based on:
Actual review state (APPROVED)
Actual reviewer identity
Explicit exclusion rules
Security Impact
Eliminates trust decisions based on mutable event metadata
Prevents reviewer impersonation or automation-based bypass
Aligns with GitHub Actions security best practices
Reduces risk of unauthorized merges or label application
Additional Notes
No behavior change for valid contributors
No changes to repository permissions or secrets usage
Workflow logic remains readable and auditable
jq installation added explicitly to ensure runner consistency
Checklist
[x] Removes github.actor–based authorization logic
[x] Uses GitHub API as source of truth
[x] Prevents duplicate comments
[x] Handles bot and self-review exclusions
[x] No breaking changes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.