ci: add Claude PR reviews #31372
ci: add Claude PR reviews #31372
Conversation
WalkthroughThis change introduces a new GitHub Actions workflow for automated PR reviews using Claude AI and adds Sequence DiagramsequenceDiagram
participant GitHub as GitHub Actions
participant Checkout as Checkout Repo
participant Node as Setup Node.js
participant Claude as Install Claude Code
participant Config as Configure MCP
participant Prompt as Create Prompt
participant Review as Run Claude Review
participant API as Anthropic API
GitHub->>Checkout: Fetch repository (depth 1)
Checkout-->>GitHub: Repository ready
GitHub->>Node: Setup Node.js v24
Node-->>GitHub: Node ready
GitHub->>Claude: Install Claude Code & MCP server
Claude-->>GitHub: Installation complete
GitHub->>Config: Configure settings.json with GITHUB_TOKEN
Config-->>GitHub: MCP configured
GitHub->>Prompt: Create /tmp/prompt.txt with PR details
Prompt-->>GitHub: Prompt file ready
GitHub->>Review: Run Claude Code Review
Review->>API: Call Anthropic API with review request
API-->>Review: Review analysis
Review-->>GitHub: Review complete
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Security and Code Quality ReviewI've reviewed this GitHub Actions workflow for Claude PR reviews. Here are my findings: 🔴 Critical Security Issues
|
|
💡 Best Practices
|
📋 SummaryThis workflow has critical security vulnerabilities that need to be addressed before merging: Main issue: Using Required changes:
Please reference your existing ai_pr_generation.yml workflow which implements these security patterns correctly. |
🔍 Detailed Issue: pull_request vs pull_request_targetLine 4: Using Attack scenario:
Your The difference:
Then you checkout the PR code explicitly in a controlled way. |
🔍 Detailed Issue: Missing PR CheckoutLine 16-18: Checkout configuration is incomplete Currently: - uses: actions/checkout@v5
with:
fetch-depth: 1This checks out the default branch (main), not the PR branch. The AI reviewer won't see any of the PR changes. What you need: - uses: actions/checkout@v5
with:
ref: [PR_HEAD_SHA_VARIABLE]
fetch-depth: 50 # or 0 for full historyWithout this, the workflow will review the main branch, not the PR changes. |
🔍 Detailed Issue: Node.js VersionLine 23: Node.js version 24 does not exist yet. The current versions are:
This will cause the workflow to fail. Recommendation: Use |
🔍 Detailed Issue: Input Validation and InjectionLine 32-47: Prompt construction with insufficient validation The current code directly interpolates GitHub context variables into a heredoc: cat > /tmp/prompt.txt << 'EOF'
REPO: [GITHUB_REPO_VAR]
PR NUMBER: [PR_NUMBER_VAR]While the single-quoted heredoc prevents shell expansion inside the heredoc, the outer variables are still expanded by GitHub Actions before the heredoc. Compare with ai_pr_generation.yml (lines 62-96):
Recommendation: Follow the same pattern with environment variables and validation as in your ai_pr_generation.yml workflow. |
✅ Positive AspectsWhile there are critical issues to address, some things are done well:
The workflow has good bones - it just needs security hardening before it can be safely deployed. |
🛠️ Recommended Action ItemsBefore merging, please address these in priority order: P0 - Critical (Must Fix):
P1 - High (Should Fix): P2 - Medium (Nice to Have): Reference implementations:
Would you like me to prepare a corrected version of the workflow? |
Overall AssessmentThis PR adds automated Claude-based PR reviews to the Deno repository. The implementation is generally well-structured, but I've identified several important issues that should be addressed. Critical Issues
Major Concerns
Recommendations
I'll add inline comments on specific issues. |
Specific Code IssuesLine 10: Timeout Duration
Line 18: Shallow Checkout
Line 23: Invalid Node Version
Line 44: Environment Variable Substitution
Lines 8-14: Missing Concurrency Control
Line 74: Token Usage
Missing: PR Size Check
|
Additional SuggestionsSecurity Considerations:
Performance Optimizations:
Comparison with ai_pr_generation.yml:
Testing:
Documentation:
|
.gitignore ChangeThe addition of .claude/settings.json to .gitignore (line 48) is appropriate since the workflow dynamically generates this file with secrets. However, note that line 47 already ignores .claude/settings.local.json. Consider documenting the difference between these two files:
This change is good and prevents accidentally committing generated configuration with sensitive tokens. |
Summary and Action ItemsMust Fix Before Merge:
Strongly Recommended:
Nice to Have:
The workflow is a good starting point, but issues 1-3 are critical and must be addressed for it to function correctly. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
.github/workflows/claude_pr_review.yml (1)
25-31: Add timeouts to npm install steps.Long npm installs can hang. Add timeouts to fail fast and avoid blocking other workflows.
Apply this diff to the Install Claude Code step:
- name: Install Claude Code + timeout-minutes: 10 shell: bash run: npm install -g @anthropic-ai/claude-code - name: Install GitHub MCP Server + timeout-minutes: 10 shell: bash run: npm install -g @modelcontextprotocol/server-github
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/claude_pr_review.yml(1 hunks).gitignore(1 hunks)
⏰ 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). (11)
- GitHub Check: test debug linux-aarch64
- GitHub Check: test debug linux-x86_64
- GitHub Check: test debug windows-x86_64
- GitHub Check: test release macos-x86_64
- GitHub Check: test debug macos-x86_64
- GitHub Check: build libs
- GitHub Check: test debug macos-aarch64
- GitHub Check: lint debug macos-x86_64
- GitHub Check: lint debug windows-x86_64
- GitHub Check: lint debug linux-x86_64
- GitHub Check: review
🔇 Additional comments (2)
.gitignore (1)
47-48: LGTM. Simple addition that prevents Claude configuration files from being committed, aligning with the new workflow's use of settings.json..github/workflows/claude_pr_review.yml (1)
20-23: Node.js version '24' does not exist; use '22' or 'lts/*'.Node.js v24 is not a valid release version. Use v22 LTS or the
lts/*specifier.Apply this diff:
- name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: '24' + node-version: '22'Likely an incorrect or invalid review comment.
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] |
There was a problem hiding this comment.
Switch to pull_request_target with controlled PR checkout.
The pull_request trigger runs the workflow code from the PR branch, allowing untrusted code execution and token exfiltration attacks. Use pull_request_target to run from the base branch, then explicitly checkout the PR head with a safe ref.
Apply this diff:
on:
- pull_request:
+ pull_request_target:
types: [opened, synchronize]Then update the checkout step (see the next comment) to explicitly fetch the PR head.
📝 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.
| on: | |
| pull_request: | |
| types: [opened, synchronize] | |
| on: | |
| pull_request_target: | |
| types: [opened, synchronize] |
🤖 Prompt for AI Agents
.github/workflows/claude_pr_review.yml lines 3-5: the workflow currently uses
the pull_request trigger which runs workflow code from the PR branch and can
execute untrusted code; change the trigger to pull_request_target so the
workflow runs from the base branch, and then update the checkout step to
explicitly fetch and checkout the PR head (use actions/checkout with ref:
github.event.pull_request.head.sha or github.event.pull_request.head.ref and
fetch-depth: 0 and token: ${{ secrets.GITHUB_TOKEN }}) to safely run the PR
changes while keeping workflow code executed from the base branch.
| - uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 1 |
There was a problem hiding this comment.
Add PR head ref and increase fetch-depth for better context.
The current checkout defaults to the base branch and uses a shallow clone. You need to explicitly checkout the PR head so Claude sees the PR changes. Also increase fetch-depth to provide more context.
Apply this diff:
- uses: actions/checkout@v5
with:
- fetch-depth: 1
+ fetch-depth: 0
+ ref: ${{ github.event.pull_request.head.sha }}📝 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.
| - uses: actions/checkout@v5 | |
| with: | |
| fetch-depth: 1 | |
| - uses: actions/checkout@v5 | |
| with: | |
| fetch-depth: 0 | |
| ref: ${{ github.event.pull_request.head.sha }} |
🤖 Prompt for AI Agents
.github/workflows/claude_pr_review.yml around lines 16-18: the checkout step
currently does a shallow checkout of the default branch and doesn't fetch the PR
head; update the checkout action to explicitly check out the PR head ref and
fetch full history by setting with: ref: ${{ github.event.pull_request.head.sha
}} (or head.ref) and fetch-depth: 0 so the workflow sees the PR changes and has
sufficient git history/context.
| - name: Configure Claude Code MCP | ||
| shell: bash | ||
| run: | | ||
| mkdir -p .claude | ||
| cat > .claude/settings.json << 'MCPEOF' | ||
| { | ||
| "mcpServers": { | ||
| "github": { | ||
| "command": "npx", | ||
| "args": ["-y", "@modelcontextprotocol/server-github"], | ||
| "env": { | ||
| "GITHUB_PERSONAL_ACCESS_TOKEN": "${GITHUB_TOKEN}" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| MCPEOF |
There was a problem hiding this comment.
Use environment variable for token; validate and escape inputs.
Embedding the token directly in shell script risks leaks. Use environment variables and validate/escape all inputs before passing to shell, especially for settings.json which will be loaded by Claude.
Apply this diff to use env var and remove direct token interpolation:
- name: Configure Claude Code MCP
shell: bash
+ env:
+ GITHUB_TOKEN_VAR: ${{ secrets.DENOBOT_PAT }}
run: |
mkdir -p .claude
cat > .claude/settings.json << 'MCPEOF'
{
"mcpServers": {
"github": {
"command": "npx",
"args": ["-y", "@modelcontextprotocol/server-github"],
"env": {
- "GITHUB_PERSONAL_ACCESS_TOKEN": "${GITHUB_TOKEN}"
+ "GITHUB_PERSONAL_ACCESS_TOKEN": "$GITHUB_TOKEN_VAR"
}
}
}
}
MCPEOFNote: Use heredoc 'MCPEOF' (which you already do) to prevent variable expansion, then reference the env var using $GITHUB_TOKEN_VAR.
📝 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.
| - name: Configure Claude Code MCP | |
| shell: bash | |
| run: | | |
| mkdir -p .claude | |
| cat > .claude/settings.json << 'MCPEOF' | |
| { | |
| "mcpServers": { | |
| "github": { | |
| "command": "npx", | |
| "args": ["-y", "@modelcontextprotocol/server-github"], | |
| "env": { | |
| "GITHUB_PERSONAL_ACCESS_TOKEN": "${GITHUB_TOKEN}" | |
| } | |
| } | |
| } | |
| } | |
| MCPEOF | |
| - name: Configure Claude Code MCP | |
| shell: bash | |
| env: | |
| GITHUB_TOKEN_VAR: ${{ secrets.DENOBOT_PAT }} | |
| run: | | |
| mkdir -p .claude | |
| cat > .claude/settings.json << "MCPEOF" | |
| { | |
| "mcpServers": { | |
| "github": { | |
| "command": "npx", | |
| "args": ["-y", "@modelcontextprotocol/server-github"], | |
| "env": { | |
| "GITHUB_PERSONAL_ACCESS_TOKEN": "$GITHUB_TOKEN_VAR" | |
| } | |
| } | |
| } | |
| } | |
| MCPEOF |
🤖 Prompt for AI Agents
.github/workflows/claude_pr_review.yml lines 33-49: the workflow currently
interpolates ${GITHUB_TOKEN} directly into the generated .claude/settings.json
which can leak secrets and allows unvalidated input; instead stop expanding the
token in the heredoc, write the token field to reference an environment variable
(e.g. GITHUB_TOKEN_VAR) so the runner injects it at runtime, ensure the heredoc
remains single-quoted to prevent expansion, and add a pre-check step that
validates the env var is set and escapes any characters before writing JSON
(fail the job with a clear error if the variable is missing or contains
disallowed content).
| - name: Create prompt file | ||
| shell: bash | ||
| run: | | ||
| cat > /tmp/prompt.txt << 'EOF' | ||
| REPO: ${{ github.repository }} | ||
| PR NUMBER: ${{ github.event.pull_request.number }} | ||
|
|
||
| Please review this pull request with a focus on: | ||
| - Code quality and best practices | ||
| - Potential bugs or issues | ||
| - Security implications | ||
| - Performance considerations | ||
|
|
||
| Note: The PR branch is already checked out in the current working directory. | ||
|
|
||
| Use `gh pr comment` for top-level feedback. | ||
| Use `mcp__github__add_comment_to_pending_review` to add inline comments on specific lines of code. | ||
| Only post GitHub comments - don't submit review text as messages. | ||
| EOF |
There was a problem hiding this comment.
🧩 Analysis chain
Use environment variables for GitHub context to improve input validation.
GitHub context variables like ${{ github.repository }} are interpolated by the Actions runner before shell execution, which bypasses normal shell escaping. For better control and consistency with input validation, pass these via environment variables.
Apply this diff:
- name: Create prompt file
shell: bash
+ env:
+ REPO: ${{ github.repository }}
+ PR_NUMBER: ${{ github.event.pull_request.number }}
run: |
cat > /tmp/prompt.txt << 'EOF'
- REPO: ${{ github.repository }}
- PR NUMBER: ${{ github.event.pull_request.number }}
+ REPO: $REPO
+ PR NUMBER: $PR_NUMBER
Please review this pull request with a focus on:
- Code quality and best practices
- Potential bugs or issues
- Security implications
- Performance considerations
Note: The PR branch is already checked out in the current working directory.
Use `gh pr comment` for top-level feedback.
Use `mcp__github__add_comment_to_pending_review` to add inline comments on specific lines of code.
Only post GitHub comments - don't submit review text as messages.
EOF
---
🌐 Web query:
```web
GitHub Actions when are expressions ${{ }} evaluated relative to bash script execution
💡 Result:
Short answer
- All GitHub Actions expressions ${ { ... } } are evaluated by the Actions engine before the shell executes your run script — they are expanded into literal text that the runner then executes. [1][2]
Key details
- Job-level expressions/ifs are evaluated before the job is dispatched (server-side), so they can decide whether a job is created at all. [2]
- Step-level expressions/ifs are evaluated by the runner before that step runs; the result is substituted into the step (including the run: body) and then the shell executes the substituted text. This is why using ${ { } } directly inside run: can lead to command/script injection if the expression contains uncontrolled input. [1][2][3]
- Best practice: put values into env: via ${ { ... } } and reference them from the shell (e.g., echo "$VAR") instead of directly embedding ${ { ... } } inside run:. [3]
References
[1] GitHub Actions — Evaluate expressions in workflows and actions.
[2] GitHub Actions — Workflow syntax (job/step if evaluation notes).
[3] GitHub Blog — recommendation: don’t use ${ { } } in run sections (use env variables instead).
Remove the "major issue" severity; the code is safe but the env variable approach is cleaner per GitHub best practices.
The ${{ }} expressions are evaluated by the Actions runner before the shell script executes, and the best practice is to pass values via env variables and reference them from the shell. Your original code using a single-quoted heredoc is safe (values are written literally to the file), but the suggested refactor aligns with GitHub's documented approach for better maintainability. Apply the diff as originally proposed—it's a valid improvement, not a critical fix.
🤖 Prompt for AI Agents
.github/workflows/claude_pr_review.yml lines 51-69: replace direct GitHub
expression interpolation inside the heredoc with environment variables and
reference those in the shell; add env: REPO: ${{ github.repository }} and
PR_NUMBER: ${{ github.event.pull_request.number }} to the step, then in the run
block write the heredoc using unquoted EOF and include REPO: $REPO and PR
NUMBER: $PR_NUMBER so the runner injects values at runtime via the environment
rather than pre-evaluating the expressions.
| - name: Run Claude Code Review | ||
| shell: bash | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.DENOBOT_PAT }} | ||
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| run: | | ||
| claude --print "$(cat /tmp/prompt.txt)" \ | ||
| --allowedTools "mcp__github__add_comment_to_pending_review,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)" |
There was a problem hiding this comment.
Add concurrency control, error handling, and tighten tool permissions.
Without concurrency control, rapid PR updates cause conflicting reviews. Missing error handling means failures go silent. Tool permissions are too permissive (Bash(gh pr comment:*) allows any gh command).
Apply these diffs:
- Add concurrency to the job block (after
permissions:):
permissions:
contents: read
pull-requests: write
id-token: write
+ concurrency:
+ group: claude-review-${{ github.event.pull_request.number }}
+ cancel-in-progress: true- Add per-step timeouts and error handling (add before the Claude step):
- name: Run Claude Code Review
+ timeout-minutes: 45
shell: bash
env:
GITHUB_TOKEN: ${{ secrets.DENOBOT_PAT }}
ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
run: |
claude --print "$(cat /tmp/prompt.txt)" \
- --allowedTools "mcp__github__add_comment_to_pending_review,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)"
+ --allowedTools "mcp__github__add_comment_to_pending_review,Bash(gh pr diff),Bash(gh pr view)"
+
+ - name: Notify on failure
+ if: failure()
+ shell: bash
+ env:
+ GITHUB_TOKEN: ${{ secrets.DENOBOT_PAT }}
+ run: |
+ gh pr comment ${{ github.event.pull_request.number }} \
+ --body "⚠️ Claude PR review failed. Please check workflow logs."Note: Removed wildcards from gh commands (e.g., gh pr comment:* → just review with diff/view, comment only via MCP tool). This prevents the Claude session from running arbitrary gh commands.
Committable suggestion skipped: line range outside the PR's diff.
No description provided.