Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions .github/workflows/claude_pr_review.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
name: Claude Auto Review

on:
pull_request:
types: [opened, synchronize]
Comment on lines +3 to +5
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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.


jobs:
review:
runs-on: ubuntu-latest
timeout-minutes: 60
permissions:
contents: read
pull-requests: write
id-token: write
steps:
- uses: actions/checkout@v5
with:
fetch-depth: 1
Comment on lines +16 to +18
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
- 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: Setup Node.js
uses: actions/setup-node@v4
with:
node-version: '24'

- name: Install Claude Code
shell: bash
run: npm install -g @anthropic-ai/claude-code

- name: Install GitHub MCP Server
shell: bash
run: npm install -g @modelcontextprotocol/server-github

- 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
Comment on lines +33 to +49
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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"
                 }
               }
             }
           }
           MCPEOF

Note: 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.

Suggested change
- 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
Comment on lines +51 to +69
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 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:*)"
Comment on lines +71 to +78
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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:

  1. 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
  1. 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.

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,4 @@ Untitled*.ipynb
/.ms-playwright

**/.claude/settings.local.json
**/.claude/settings.json
Loading