Skip to content

CI: fix and improve claude-code-review.yml #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jnasbyupgrade merged 9 commits into from
Aug 9, 2026
Merged

CI: fix and improve claude-code-review.yml #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ad6e722
ci: bump Actions pins, add track_progress, add claude-debug label toggle
jnasbyupgrade Aug 4, 2026
325b37c
CI: fix claude-review failing at checkout on every fork PR
jnasbyupgrade Aug 5, 2026
f65714d
Fix checkout pattern: don't redirect origin to the fork
jnasbyupgrade Aug 5, 2026
9247047
CI: register inline-comment MCP tool for claude-code-review
jnasbyupgrade Aug 5, 2026
e4a126d
Fold in #26: check PR author (user.login), not head repo owner
jnasbyupgrade Aug 7, 2026
0bf7e55
Merge remote-tracking branch 'origin/fix-pwn-request-checkout' into c…
jnasbyupgrade Aug 7, 2026
402098a
Merge remote-tracking branch 'origin/ci-workflow-improvements' into c…
jnasbyupgrade Aug 7, 2026
0b49622
Merge remote-tracking branch 'upstream/fix-claude-review-mcp-and-cach…
jnasbyupgrade Aug 7, 2026
92cc528
Restore persist-credentials: false on the base-ref checkout
jnasbyupgrade Aug 7, 2026
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
115 changes: 103 additions & 12 deletions .github/workflows/claude-code-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,72 @@ name: Claude Code Review
# write-capable token. The job is gated to PRs from the trusted `jnasbyupgrade`
# fork only — an arbitrary external fork can never trigger this secret-bearing
# job. The workflow file always comes from the base branch (master), so a PR
# cannot modify the reviewer that runs on it. We check out the PR head only for
# read context (persist-credentials: false) and never build or execute PR code.
# cannot modify the reviewer that runs on it. This workflow never checks out
# the PR's own ref into the workspace (see the checkout step below) --
# claude-code-action fetches and reads the PR's content itself, safely, and
# never builds or executes it.
on:
pull_request_target:
types: [opened, synchronize, reopened, ready_for_review]
types: [opened, synchronize, reopened, ready_for_review, labeled]

concurrency:
group: claude-review-${{ github.event.pull_request.number }}
cancel-in-progress: true

jobs:
claude-review:
# Trusted fork only, and skip drafts (don't spend API/CI on unfinished PRs).
# To add more trusted owners, extend the head-owner check.
# !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE user.login CHECK
# BELOW !!! It is the ONLY thing standing between an arbitrary external
# PR and this job's write-capable GITHUB_TOKEN and CLAUDE_CODE_OAUTH_TOKEN.
# Drop or loosen this check and any PR can trigger a job that runs with
# this repo's secrets. To trust an additional person, EXTEND this
# condition explicitly (e.g. `|| ... == 'other-trusted-account'`) --
# never replace it with something broader (a wildcard, a check on a
# label or a comment -- those ARE attacker-controlled on their own PR,
# unlike `user.login`, which is GitHub's own authenticated record of who
# actually opened the PR and can't be spoofed by anyone else).
#
# Deliberately checks the PR AUTHOR (user.login), not
# head.repo.owner.login: the latter only identifies "who owns the fork"
# for fork-headed PRs -- for an upstream-branch-headed PR (base and head
# both in this repo, e.g. from `gh stack`, or `gh pr create` without a
# fork), it's always this repo's own org, never the actual author, so
# that check silently skipped review on every such PR regardless of who
# opened it. `user.login` works correctly for both fork-headed and
# upstream-branch-headed PRs.
#
# Skip drafts too (don't spend API/CI on unfinished PRs). `labeled` is
# only in the trigger list so the claude-debug toggle below can kick off
# a fresh run without a push; scope it tightly here so an unrelated
# label doesn't re-run this (paid) workflow.
if: >-
github.event.pull_request.draft == false &&
github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade'
github.event.pull_request.user.login == 'jnasbyupgrade' &&
(github.event.action != 'labeled' || github.event.label.name == 'claude-debug')
runs-on: ubuntu-latest
timeout-minutes: 60
permissions:
contents: read
pull-requests: write # post the review comments
checks: read # read sibling check-runs for the cost gate
steps:
# Live-query the claude-debug label rather than trusting
# github.event.pull_request.labels (the event payload captured at
# trigger time): GitHub's "Re-run jobs" replays that ORIGINAL stored
# payload, so a payload-based check would miss a label added after a
# run already started. A live `gh pr view` call always reflects the
# PR's current labels, whether this is a fresh trigger or a re-run.
- name: Check for claude-debug label (live)
id: debug_label
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
PR: ${{ github.event.pull_request.number }}
run: |
debug=$(gh pr view "$PR" --repo "$REPO" --json labels --jq 'any(.labels[]; .name == "claude-debug")')
echo "debug=$debug" >> "$GITHUB_OUTPUT"
echo "claude-debug label present: $debug"

# COST GATE: the paid Claude review is the last thing to run. Wait for the
# PR head's OTHER check-runs to finish and only proceed if they are clean.
# If any sibling check failed we skip the review to avoid spending money
Expand All @@ -41,7 +83,9 @@ jobs:
# - decision=run : all sibling checks completed with a good conclusion,
# OR no sibling checks exist after a short grace window
# (nothing to gate on), OR the poll timed out is treated
# as skip (see below).
# as skip (see below), OR the claude-debug label is
# present (skip the wait entirely for fast debug
# iteration; see the live-query step above).
# - decision=skip : at least one sibling check failed/cancelled/etc, or
# we timed out waiting for still-pending checks.
# We exclude this workflow's own check-run (job name `claude-review`) so the
Expand All @@ -52,7 +96,13 @@ jobs:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
SHA: ${{ github.event.pull_request.head.sha }}
DEBUG_LABEL: ${{ steps.debug_label.outputs.debug }}
run: |
if [ "$DEBUG_LABEL" = "true" ]; then
echo "claude-debug label present — skipping cost gate wait"
echo "decision=run" >> "$GITHUB_OUTPUT"
exit 0
fi
decision=skip
for i in $(seq 1 72); do # ~24 min max
json=$(gh api "repos/$REPO/commits/$SHA/check-runs" --paginate \
Expand All @@ -74,14 +124,31 @@ jobs:
echo "decision=$decision" >> "$GITHUB_OUTPUT"
echo "gate decision: $decision"

- name: Check out PR head (read-only context)
- name: Check out base branch
if: steps.gate.outputs.decision == 'run'
# Deliberately NO ref:/repository: override -- this checks out this
# repo's own base branch (master), not the PR's fork/ref. Checking
# out an untrusted PR ref into the workspace root before this action
# is exactly the anti-pattern anthropics/claude-code-action's own
# docs/security.md warns against; its "preferred" pattern is a plain
# checkout of the base ref, nothing more. claude-code-action fetches
# and reviews the PR's actual content itself, from ITS OWN internal
# logic (see its src/github/operations/branch.ts): for a fork PR it
# fetches origin's refs/pull/<n>/head -- a ref GitHub maintains on
# THIS repo for any PR, fork or not, so it never needs direct access
# to the fork's own remote at all. That's why this step must leave
# `origin` pointing at this repo (the default) rather than being
# redirected to the fork: an earlier version of this step did that,
# which broke the action's own internal fetch ("couldn't find remote
# ref pull/<n>/head") since that ref doesn't exist on the fork.
# Intentionally tracks the major-version tag (not a pinned SHA) so
# upstream fixes are picked up automatically.
uses: actions/checkout@v4
# upstream fixes are picked up automatically. No ref:/repository:
# override (see the comment above), but persist-credentials: false
# is still worth keeping explicitly: this job never needs to push
# anything, so there's no reason to leave a push-capable credential
# sitting in .git/config for the rest of the job to (mis)use.
uses: actions/checkout@v7

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== workflow files =="
git ls-files '.github/workflows/*' || true

echo
echo "== relevant checkout usages =="
rg -n "actions/checkout@|pull-request|github-token|zizmor|permissions:" .github/workflows || true

echo
echo "== workflow snippets =="
for f in .github/workflows/claude-code-review.yml .github/workflows/claude.yml; do
  if [ -f "$f" ]; then
    echo "--- $f lines 1-220 ---"
    sed -n '1,220p' "$f" | nl -ba | sed -n '1,220p'
  else
    echo "MISSING $f"
  fi
done

echo
echo "== repo files mentioning zizmor/actionlint/security =="
rg -n "zizmor|actionlint|secrets|GITHUB_TOKEN|permissions:|pull-request|pull_request_target" -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' . || true

Repository: Postgres-Extensions/object_reference

Length of output: 1018

🌐 Web query:

GitHub Actions reusable workflows trusted payload pull_request_target permissions secrets

💡 Result:

The pull_request_target event is a powerful but sensitive trigger in GitHub Actions that grants workflows access to base repository secrets and read/write tokens [1][2][3]. When used in conjunction with reusable workflows, it creates a specific security model that must be managed carefully to avoid privilege escalation or unauthorized secret access [4][5]. Key Security Implications and Mechanisms: 1. Trusted Execution Context: Workflows triggered by pull_request_target run the workflow file from the base repository's default branch, not the potentially untrusted pull request branch [1][2]. This design choice is intended to ensure that the logic executing with elevated privileges cannot be directly tampered with by an external contributor [2]. 2. Reusable Workflow Inheritance: Reusable workflows inherit the security context (secrets and permissions) of the caller [4][5]. If a caller workflow is triggered by pull_request_target, the called reusable workflow gains the same elevated access to repository and organization secrets [4][3]. 3. Secret Exposure Risks: - If a caller uses secrets: inherit, the reusable workflow gains access to all secrets available to the caller, including environment-scoped secrets [6][4][7]. - If the reusable workflow contains steps that checkout, build, or execute code from an untrusted pull request, an attacker may be able to extract these secrets or tokens [1][2][8]. - To mitigate this, avoid checking out untrusted code unless you use an approval gate or ensure the code is never executed [1][9]. Best Practices for Security: - Use Explicit Secret Passing: Avoid secrets: inherit whenever possible. Explicitly pass only the specific secrets required by the reusable workflow to minimize exposure [4]. - Pin Reusable Workflows: Always pin reusable workflows to a specific commit SHA rather than a branch or tag to prevent supply chain attacks where the underlying workflow is compromised [4][5]. - Workflow Splitting: Use a two-stage approach. An unprivileged workflow triggered by pull_request (which does not have secret access) handles initial PR processing. A second, privileged workflow (triggered by workflow_run) is invoked only after validation and runs in a secure context [3]. - Restrict Permissions: Follow the principle of least privilege by explicitly setting permissions for the GITHUB_TOKEN in your workflow YAML, limiting it to only what is strictly necessary [1]. - Use Environment Protection: For critical deployments triggered by pull_request_target, utilize GitHub Environments with mandatory manual approval gates [5]. This ensures that even if a workflow is triggered, the sensitive actions (like deployments or secret usage) are gated by a human reviewer [5].

Citations:

Pin actions/checkout to a full commit SHA.

Both workflows use the mutable v7 tag; the write-capable claude-code-review.yml workflow should not run code from a changeable GitHub action tag. Pin both .github/workflows/claude-code-review.yml#L150 and .github/workflows/claude.yml#L39 to the same full commit SHA.

🧰 Tools
🪛 zizmor (1.29.0)

[error] 150-150: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

📍 Affects 2 files
  • .github/workflows/claude-code-review.yml#L150-L150 (this comment)
  • .github/workflows/claude.yml#L39-L39
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/claude-code-review.yml at line 150, Replace the mutable
actions/checkout@v7 reference with the same full commit SHA in both
.github/workflows/claude-code-review.yml (lines 150-150) and
.github/workflows/claude.yml (lines 39-39), ensuring both workflow checkout
steps use the identical pinned revision.

Source: Linters/SAST tools

with:
repository: ${{ github.event.pull_request.head.repo.full_name }}
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 1
persist-credentials: false

Expand All @@ -95,10 +162,34 @@ jobs:
# pull_request_target. GITHUB_TOKEN is repo/workflow-scoped (independent
# of the actor's role) and has pull-requests: write here.
github_token: ${{ secrets.GITHUB_TOKEN }}
# Post (and keep updating) a live tracking-comment checklist as
# Claude works, instead of staying silent until the whole run
# finishes — without this, the cost gate above plus the review
# itself can leave a PR with zero visible progress for the better
# part of an hour. Disabled specifically for `labeled`-triggered
# runs: the action's own track_progress validation only accepts
# opened/synchronize/reopened/ready_for_review for pull_request(_target)
# events and throws for any other action, and `labeled` is exactly
# how the claude-debug toggle re-triggers this workflow.
track_progress: ${{ github.event.action != 'labeled' }}
# NOTE: plugin_marketplaces can't be pinned — it tracks the
# marketplace repo's default branch (upstream anthropics/claude-code).
plugin_marketplaces: 'https://github.com/anthropics/claude-code.git'
plugins: 'code-review@claude-code-plugins'
# claude-debug label (live-queried above) turns on the raw JSON
# transcript for debugging. Normally off: it can include tool
# execution results, which shouldn't be publicly visible in Actions
# logs.
show_full_output: ${{ steps.debug_label.outputs.debug == 'true' }}
# A bare `prompt:` with no @claude mention runs the action in "agent
# mode", which starts MCP servers based on an --allowedTools flag
# inside claude_args -- it does not look at the invoked plugin's own
# allowed-tools frontmatter. Without this, the github_inline_comment
# MCP server never starts, so mcp__github_inline_comment__create_inline_comment
# doesn't exist in the session at all, and the code-review plugin
# silently falls back to a single consolidated PR comment instead of
# real per-line inline comments (no error, no warning either way).
claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment'
# --comment is required: without it, the code-review plugin only
# prints its findings to the job log and never posts anything to
# the PR (confirmed by capturing the hidden SDK transcript on a
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/claude.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
- name: Checkout repository
# Intentionally tracks the major-version tag (not a pinned SHA) so
# upstream fixes are picked up automatically.
uses: actions/checkout@v4
uses: actions/checkout@v7
with:
fetch-depth: 1
persist-credentials: false
Expand Down
Loading