From 12ec7b76fba9e70321a43ae0258eb2306d48fad6 Mon Sep 17 00:00:00 2001 From: Meekail Zain Date: Tue, 12 May 2026 14:51:34 +0000 Subject: [PATCH 1/2] Harden claude-pr-action.yml against pull_request_target abuse Triaged from a third-party security review of the workflow. This commit addresses the must-fix bundle (M1+M2+M3 from that triage); deferred items (show_full_output, install-script pinning, base-ref validation, job timeouts) will land in a follow-up PR. - Block fork PRs from non-collaborators before any PR code is staged. pull_request_target gives this job write-scoped GITHUB_TOKEN and the CLAUDE_CODE_OAUTH_TOKEN secret. Without this gate, a fork PR can hijack both via e.g. a .gitattributes textconv driver invoked by Claude's `git diff` calls. The new step runs first and exits 1 unless the head repo is the same as the base repo or the PR author has write/maintain/admin permission on the base. - Run the Claude warmup in mktemp -d instead of $GITHUB_WORKSPACE. Claude Code auto-discovers CLAUDE.md and .claude/ from CWD; with the PR tree checked out and CLAUDE_CODE_OAUTH_TOKEN in env, an attacker workspace CLAUDE.md could prompt-inject the warmup into echoing the token into workflow logs. - Pin third-party actions to commit SHAs: * actions/checkout@v4 -> @34e114876b0b... (v4.3.1) * actions/upload-artifact@v4 -> @ea165f8d65b6... (v4.6.2) * anthropics/claude-code-action@v1 -> @dde2242db6af... (v1.0.120) - Replace blocklist allowedTools with a strict allowlist for both the review and summary steps. The previous `Bash(gh *)` + denylist allowed e.g. `gh api --method DELETE repos/...` because the denylist only covered specific subcommands. New allowlist only matches read-shaped invocations (`gh api repos/*`, `git diff *`, etc.); summary additionally permits `gh api --method PATCH repos/*` so it can update prior walkthrough comments. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/claude-pr-action.yml | 62 +++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/.github/workflows/claude-pr-action.yml b/.github/workflows/claude-pr-action.yml index 68e3cd4ac..9343f23ad 100644 --- a/.github/workflows/claude-pr-action.yml +++ b/.github/workflows/claude-pr-action.yml @@ -200,13 +200,45 @@ jobs: PR_NUMBER: ${{ needs.resolve.outputs.pr }} BASE_REF: ${{ needs.resolve.outputs.base }} steps: + # SECURITY GATE — must run before any step that materializes PR code. + # pull_request_target grants this job write-scoped GITHUB_TOKEN and the + # CLAUDE_CODE_OAUTH_TOKEN secret. If we then check out the fork's tree + # and let Claude run `git diff` etc. on it, a malicious PR can hijack + # those credentials (e.g. via a `.gitattributes` textconv driver). + # Restrict fork PRs to authors with write/maintain/admin on the base + # repo; same-repo PRs and workflow_dispatch already require write perms + # to reach this point. + - name: Block fork PRs from non-collaborators + if: github.event_name == 'pull_request_target' + env: + GH_TOKEN: ${{ github.token }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + run: | + set -euo pipefail + if [[ "$HEAD_REPO" == "${{ github.repository }}" ]]; then + echo "Same-repo PR; collaborator check not required." + exit 0 + fi + perm=$(gh api "repos/${{ github.repository }}/collaborators/$PR_AUTHOR/permission" \ + --jq .permission 2>/dev/null || echo none) + case "$perm" in + admin|maintain|write) + echo "Fork PR author $PR_AUTHOR has $perm; allowing." + ;; + *) + echo "::error::Fork PR from $PR_AUTHOR ($HEAD_REPO) lacks write access (perm=$perm); refusing to run on untrusted code." + exit 1 + ;; + esac + # refs/pull//merge is GitHub's synthetic merge commit (base tip # merged with PR head). Checking it out gives us both parents in one # shot: HEAD^1 = base tip, HEAD^2 = PR head — no separate base fetch # needed. Caveat: the merge ref only exists when the PR is mergeable; # if it's missing/stale, the workflow will fail fast on checkout, which # is the right behavior for an unreviewable PR. - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 with: ref: refs/pull/${{ env.PR_NUMBER }}/merge @@ -242,7 +274,14 @@ jobs: curl -fsSL https://claude.ai/install.sh | bash export PATH="$HOME/.local/bin:$PATH" claude --version - timeout 60 claude --print -p "Say OK" || echo "Warmup complete" + # Run the warmup in a clean directory so Claude Code does not + # auto-discover CLAUDE.md / .claude/ from the (untrusted) PR tree + # at $GITHUB_WORKSPACE. The OAUTH token is in env at this step, + # and a malicious workspace CLAUDE.md could otherwise prompt-inject + # the warmup invocation into echoing it. + warmup_dir=$(mktemp -d) + ( cd "$warmup_dir" && timeout 60 claude --print -p "Say OK" ) || echo "Warmup complete" + rm -rf "$warmup_dir" # claude-code-action only auto-configures the inline-comment MCP server # for pull_request* events. Wire it up manually so it works regardless @@ -276,7 +315,7 @@ jobs: id: review if: env.ACTION == 'review' timeout-minutes: 30 - uses: anthropics/claude-code-action@v1 + uses: anthropics/claude-code-action@dde2242db6af13460b916652159b6ba19a598f30 # v1.0.120 env: # Same token is exposed to the model's `gh` subprocess so it can # comment on the PR. Mirrors the `github_token:` input below. @@ -293,8 +332,14 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} allowed_bots: "github-actions[bot]" show_full_output: true + # Strict allowlist (no disallowedTools). Patterns match against the + # literal command string, so `Bash(gh api repos/*)` permits read + # paths like `gh api repos/foo/bar/pulls/1/comments` but rejects + # `gh api --method DELETE repos/...` because the next token after + # `gh api` isn't `repos/*`. Add new entries here when a skill needs + # a new subcommand — never widen to `Bash(gh *)` or `Bash(git *)`. claude_args: | - --model claude-opus-4-7 --effort xhigh --allowedTools "Skill,mcp__github_inline_comment__create_inline_comment,Bash(gh *),Bash(git *),Bash(jq *),Bash(head *),Bash(grep *),Read,Grep,Glob" --disallowedTools "Bash(gh pr close *),Bash(gh pr merge *),Bash(gh pr edit *),Bash(gh pr lock *),Bash(gh pr ready *),Bash(gh pr reopen *),Bash(gh pr review *),Bash(gh repo *),Bash(gh release *),Bash(gh workflow *),Bash(gh run *),Bash(gh secret *),Bash(gh ssh-key *),Bash(gh auth *),Bash(git push *),Bash(git reset *),Bash(git rebase *),Bash(git checkout *),Bash(git clean *)" --mcp-config ${{ steps.mcp.outputs.file }} + --model claude-opus-4-7 --effort xhigh --allowedTools "Skill,mcp__github_inline_comment__create_inline_comment,Bash(gh pr view *),Bash(gh pr comment *),Bash(gh pr diff *),Bash(gh api repos/*),Bash(gh api /repos/*),Bash(gh api --paginate repos/*),Bash(gh api --paginate /repos/*),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(git cat-file *),Bash(git ls-files *),Bash(git rev-parse *),Bash(jq *),Bash(head *),Bash(grep *),Read,Grep,Glob" --mcp-config ${{ steps.mcp.outputs.file }} settings: | { "hasCompletedOnboarding": true } prompt: | @@ -376,7 +421,7 @@ jobs: id: summary if: env.ACTION == 'summary' timeout-minutes: 20 - uses: anthropics/claude-code-action@v1 + uses: anthropics/claude-code-action@dde2242db6af13460b916652159b6ba19a598f30 # v1.0.120 env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: @@ -385,8 +430,11 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} allowed_bots: "github-actions[bot]" show_full_output: true + # Strict allowlist (see review step for rationale). Summary may + # PATCH a prior walkthrough comment, so `gh api --method PATCH + # repos/*` is included; review intentionally does not allow PATCH. claude_args: | - --model claude-opus-4-7 --effort xhigh --allowedTools "Skill,Bash(gh *),Bash(git *),Bash(jq *),Read,Grep,Glob" --disallowedTools "Bash(gh pr close *),Bash(gh pr merge *),Bash(gh pr edit *),Bash(gh pr lock *),Bash(gh pr ready *),Bash(gh pr reopen *),Bash(gh pr review *),Bash(gh repo *),Bash(gh release *),Bash(gh workflow *),Bash(gh run *),Bash(gh secret *),Bash(gh ssh-key *),Bash(gh auth *),Bash(git push *),Bash(git reset *),Bash(git rebase *),Bash(git checkout *),Bash(git clean *)" + --model claude-opus-4-7 --effort xhigh --allowedTools "Skill,Bash(gh pr view *),Bash(gh pr comment *),Bash(gh pr diff *),Bash(gh api repos/*),Bash(gh api /repos/*),Bash(gh api --paginate repos/*),Bash(gh api --paginate /repos/*),Bash(gh api --method PATCH repos/*),Bash(gh api --method PATCH /repos/*),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(git cat-file *),Bash(git ls-files *),Bash(git rev-parse *),Bash(jq *),Read,Grep,Glob" settings: | { "hasCompletedOnboarding": true } prompt: | @@ -454,7 +502,7 @@ jobs: - name: Upload Claude execution log if: always() && (steps.review.outputs.execution_file != '' || steps.summary.outputs.execution_file != '') - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: claude-${{ env.ACTION }}-pr${{ env.PR_NUMBER }}-log path: ${{ steps.review.outputs.execution_file || steps.summary.outputs.execution_file }} From 0d98511e14b3e69f318a77f83ecc4ed7e55b8381 Mon Sep 17 00:00:00 2001 From: Meekail Zain Date: Tue, 12 May 2026 15:06:10 +0000 Subject: [PATCH 2/2] Backfill allowlist patterns observed in past Claude PR Action runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After landing the strict allowlist in the previous commit, I downloaded the claude-execution-output.json artifacts from the 10 most recent successful runs of this workflow and extracted every tool_use event. Several Bash patterns that the model actually invokes were missing from the new allowlist and would have caused regressions. Verified-missing patterns added to the review job: Bash(git fetch *) (shallow-clone deepening) Bash(git status), Bash(git status *) Bash(git ls-remote *) (used to verify remote refs) Bash(git rev-list *) Bash(git merge-base *) Bash(echo *) (chained as `git log; echo ---; git diff`) Bash(ls), Bash(ls *) (workspace inspection / skill discovery) Verified-missing patterns added to the summary job (superset of above): Bash(cat *) (cat > /tmp/walkthrough.md < --- .github/workflows/claude-pr-action.yml | 34 +++++++++++++++++--------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/.github/workflows/claude-pr-action.yml b/.github/workflows/claude-pr-action.yml index 9343f23ad..5bbadf7cb 100644 --- a/.github/workflows/claude-pr-action.yml +++ b/.github/workflows/claude-pr-action.yml @@ -332,14 +332,18 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} allowed_bots: "github-actions[bot]" show_full_output: true - # Strict allowlist (no disallowedTools). Patterns match against the - # literal command string, so `Bash(gh api repos/*)` permits read - # paths like `gh api repos/foo/bar/pulls/1/comments` but rejects - # `gh api --method DELETE repos/...` because the next token after - # `gh api` isn't `repos/*`. Add new entries here when a skill needs - # a new subcommand — never widen to `Bash(gh *)` or `Bash(git *)`. + # Strict allowlist for Bash(...) and MCP tools (no disallowedTools). + # The matcher splits chained commands on `;`/`&&`/`|` and checks + # each segment, so `Bash(git diff *)` covers `BASE=…; git diff …` + # but `Bash(echo *)` is still needed for the `echo "---"` segment. + # Built-in tools (Read, Grep, Glob, Skill, TodoWrite, ToolSearch, + # Write, Edit) bypass --allowedTools enforcement, but Read/Grep/ + # Glob/Skill are listed for documentation. Patterns are verified + # against past run transcripts; widen only when a transcript shows + # a needed command being denied — never to `Bash(gh *)`/`Bash(git + # *)`/`Bash(*)`. claude_args: | - --model claude-opus-4-7 --effort xhigh --allowedTools "Skill,mcp__github_inline_comment__create_inline_comment,Bash(gh pr view *),Bash(gh pr comment *),Bash(gh pr diff *),Bash(gh api repos/*),Bash(gh api /repos/*),Bash(gh api --paginate repos/*),Bash(gh api --paginate /repos/*),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(git cat-file *),Bash(git ls-files *),Bash(git rev-parse *),Bash(jq *),Bash(head *),Bash(grep *),Read,Grep,Glob" --mcp-config ${{ steps.mcp.outputs.file }} + --model claude-opus-4-7 --effort xhigh --allowedTools "Skill,mcp__github_inline_comment__create_inline_comment,Bash(gh pr view *),Bash(gh pr comment *),Bash(gh pr diff *),Bash(gh api repos/*),Bash(gh api /repos/*),Bash(gh api --paginate repos/*),Bash(gh api --paginate /repos/*),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(git cat-file *),Bash(git ls-files *),Bash(git rev-parse *),Bash(git rev-list *),Bash(git ls-remote *),Bash(git merge-base *),Bash(git fetch *),Bash(git status),Bash(git status *),Bash(jq *),Bash(head *),Bash(grep *),Bash(echo *),Bash(ls),Bash(ls *),Read,Grep,Glob" --mcp-config ${{ steps.mcp.outputs.file }} settings: | { "hasCompletedOnboarding": true } prompt: | @@ -430,11 +434,19 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} allowed_bots: "github-actions[bot]" show_full_output: true - # Strict allowlist (see review step for rationale). Summary may - # PATCH a prior walkthrough comment, so `gh api --method PATCH - # repos/*` is included; review intentionally does not allow PATCH. + # Strict allowlist for Bash(...) and MCP (see review step for the + # full rationale). Summary additionally needs: + # - `gh api -X{POST,PATCH} repos/*/issues/*/comments` to create + # or update the walkthrough top-level comment (past runs use + # either `gh api -F body=@-` or `gh pr comment --body-file`); + # - `cat *` for `cat > /tmp/walkthrough.md <