Fix Claude Review error#1948
Conversation
Signed-off-by: Quanyi Ma <eli@patch.sh>
|
Claude finished @genedna's task in 2m 9s —— View job Code Review: Fix Claude Review error
This PR modifies Summary of Changes
Issues Found🔴 High: Overly Broad Tool PermissionsThe # Before (specific, targeted)
--allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)"
# After (very broad)
--allowedTools "mcp__github__,mcp__github_inline_comment__,Bash(gh)"
Previously, only 3 specific read-like 🔴 High: Removed Author Association Check for
|
There was a problem hiding this comment.
Pull request overview
This PR adjusts the claude-review.yml GitHub Actions workflow to address Claude review execution errors by changing when the job runs, improving PR SHA/repo resolution for comment-triggered runs, and loosening Claude action constraints.
Changes:
- Relaxed job trigger conditions for
pull_request_targetand@claudementions. - Updated PR ref resolution to also capture the PR source repository and pass it into checkout.
- Expanded Claude action configuration (
allowed_non_write_usersand tool allowlist).
| # Restrict tools that Claude can use during the review | ||
| claude_args: | | ||
| --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)" | ||
| --allowedTools "mcp__github__,mcp__github_inline_comment__,Bash(gh)" |
There was a problem hiding this comment.
The tool allowlist is substantially broadened from specific gh pr ... subcommands and a single inline-comment tool to mcp__github__, mcp__github_inline_comment__, and Bash(gh) (any gh subcommand). This weakens the intended sandbox and increases blast radius if the action is ever mis-prompted or compromised. Prefer keeping the narrowest possible tool list (only the exact MCP functions and gh pr view/diff/comment subcommands that are required).
| --allowedTools "mcp__github__,mcp__github_inline_comment__,Bash(gh)" | |
| --allowedTools "mcp__github_inline_comment__,Bash(gh pr view),Bash(gh pr diff),Bash(gh pr comment)" |
| # Run on PRs or when explicitly triggered via comments | ||
| if: | | ||
| ( | ||
| github.event_name == 'pull_request_target' && | ||
| ( | ||
| github.event.pull_request.author_association == 'OWNER' || | ||
| github.event.pull_request.author_association == 'MEMBER' || | ||
| github.event.pull_request.author_association == 'COLLABORATOR' | ||
| ) | ||
| ) || | ||
| github.event_name == 'pull_request_target' || | ||
| ( | ||
| (github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment') && | ||
| contains(github.event.comment.body, '@claude') && | ||
| ( | ||
| github.event.comment.author_association == 'OWNER' || | ||
| github.event.comment.author_association == 'MEMBER' || | ||
| github.event.comment.author_association == 'COLLABORATOR' | ||
| ) | ||
| contains(github.event.comment.body, '@claude') | ||
| ) || | ||
| ( | ||
| github.event_name == 'pull_request_review' && | ||
| contains(github.event.review.body, '@claude') && | ||
| ( | ||
| github.event.review.author_association == 'OWNER' || | ||
| github.event.review.author_association == 'MEMBER' || | ||
| github.event.review.author_association == 'COLLABORATOR' | ||
| ) | ||
| contains(github.event.review.body, '@claude') | ||
| ) |
There was a problem hiding this comment.
The job-level if now runs for all pull_request_target events (including PRs from forks) without restricting to trusted author_association or explicit @claude opt-in. With pull-requests: write / issues: write permissions and a checked-out fork repo, this is a significant security expansion. Consider re-adding the trusted-author guard (OWNER/MEMBER/COLLABORATOR) and/or requiring an explicit trigger (label/comment) before running on untrusted PRs.
| ( | ||
| (github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment') && | ||
| contains(github.event.comment.body, '@claude') && | ||
| ( | ||
| github.event.comment.author_association == 'OWNER' || | ||
| github.event.comment.author_association == 'MEMBER' || | ||
| github.event.comment.author_association == 'COLLABORATOR' | ||
| ) | ||
| contains(github.event.comment.body, '@claude') | ||
| ) || |
There was a problem hiding this comment.
For issue_comment events, this condition only checks for @claude in the comment body, so the workflow will also run for comments on issues (non-PRs). That can lead to confusing/no-op runs (no PR context to review) and unnecessary consumption of runner time. Gate the issue_comment branch on github.event.issue.pull_request at the job level (similar to the step-level guard), or explicitly exclude non-PR issues here.
| with: | ||
| anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| allowed_non_write_users: "*" |
There was a problem hiding this comment.
allowed_non_write_users: "*" allows any non-write user to invoke the action. Combined with pull_request_target and write-scoped GITHUB_TOKEN permissions, this can let untrusted users trigger automated commenting on PRs/issues using repository credentials. Consider restricting this to a vetted allowlist (or remove it) and rely on author_association / explicit maintainer trigger instead.
| allowed_non_write_users: "*" | |
| allowed_non_write_users: "" |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b71fa79b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| uses: actions/checkout@v6 | ||
| with: | ||
| # Checkout the PR source repository (fork) to ensure branches exist for the action | ||
| repository: ${{ steps.resolve-pr-ref.outputs.pr_repo || github.event.pull_request.head.repo.full_name || github.repository }} |
There was a problem hiding this comment.
Setting actions/checkout to github.event.pull_request.head.repo.full_name makes origin point at the contributor fork, but this workflow still relies on pull refs from origin (the next step configures refs/pull/*/head and the comment notes the action fetches origin pull/N/head). For forked PRs, refs/pull/<n>/head exists on the base repository, not the fork, so review runs can still fail with missing remote refs.
Useful? React with 👍 / 👎.
| github.event_name == 'pull_request_target' || | ||
| ( | ||
| (github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment') && | ||
| contains(github.event.comment.body, '@claude') && | ||
| ( | ||
| github.event.comment.author_association == 'OWNER' || | ||
| github.event.comment.author_association == 'MEMBER' || | ||
| github.event.comment.author_association == 'COLLABORATOR' | ||
| ) | ||
| contains(github.event.comment.body, '@claude') |
There was a problem hiding this comment.
This condition now allows pull_request_target and @claude comment triggers from any author, while the job later injects secrets.ANTHROPIC_API_KEY and a write-capable GITHUB_TOKEN into the action. That lets untrusted external users trigger privileged, billable runs simply by opening/syncing a PR or posting @claude, which was previously blocked by author_association checks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b71fa79b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| uses: actions/checkout@v6 | ||
| with: | ||
| # Checkout the PR source repository (fork) to ensure branches exist for the action | ||
| repository: ${{ steps.resolve-pr-ref.outputs.pr_repo || github.event.pull_request.head.repo.full_name || github.repository }} |
There was a problem hiding this comment.
Setting actions/checkout to github.event.pull_request.head.repo.full_name makes origin point at the contributor fork, but this workflow still relies on pull refs from origin (the next step configures refs/pull/*/head and the comment notes the action fetches origin pull/N/head). For forked PRs, refs/pull/<n>/head exists on the base repository, not the fork, so review runs can still fail with missing remote refs.
Useful? React with 👍 / 👎.
| github.event_name == 'pull_request_target' || | ||
| ( | ||
| (github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment') && | ||
| contains(github.event.comment.body, '@claude') && | ||
| ( | ||
| github.event.comment.author_association == 'OWNER' || | ||
| github.event.comment.author_association == 'MEMBER' || | ||
| github.event.comment.author_association == 'COLLABORATOR' | ||
| ) | ||
| contains(github.event.comment.body, '@claude') |
There was a problem hiding this comment.
This condition now allows pull_request_target and @claude comment triggers from any author, while the job later injects secrets.ANTHROPIC_API_KEY and a write-capable GITHUB_TOKEN into the action. That lets untrusted external users trigger privileged, billable runs simply by opening/syncing a PR or posting @claude, which was previously blocked by author_association checks.
Useful? React with 👍 / 👎.
1 participant
No description provided.