CI: fix and improve claude-code-review.yml - #27
Conversation
Combines three related CI-workflow improvements into one PR (touching only claude-code-review.yml and claude.yml): - Bump actions/checkout@v4 -> @v7 (current latest major) to clear the Node.js-20-deprecation warning it triggers on every run. Supersedes/ incorporates upstream PR Postgres-Extensions#13 and fork branch ci/bump-actions-versions, which made the same v4->v7 bump to the same two lines. anthropics/claude-code-action@v1 is still current (a floating v1 tag exists at v1.0.185) so it is left as-is. - Add track_progress: true to the claude-code-action step in claude-code-review.yml so it posts a live, updating checklist comment as it works instead of staying silent until the whole run finishes -- which, combined with this workflow's cost gate, could leave a PR dark for the better part of an hour. Disabled specifically for labeled-triggered runs (see below): 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. - Add a claude-debug PR-label toggle so a maintainer can skip the cost gate and turn on full transcript output (show_full_output) by just labeling the PR, without editing/pushing the workflow file. The label is queried live via `gh pr view` inside the step rather than read from the event payload, since GitHub's "Re-run jobs" replays the original stored payload and would miss a label added afterward. `labeled` is added to the trigger types so applying the label alone starts a fresh run, scoped tightly in the job's `if:` so an unrelated label can't re-trigger this paid workflow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
actions/checkout now refuses, by default, to check out a fork PR's head under pull_request_target (a "pwn request" guard) -- this job has always been safe to opt out of that guard (trusted-fork gate + read-only use), it just started failing when the guard shipped.
allow-unsafe-pr-checkout: true plus a repository:/ref: override checking out the fork directly is the wrong fix -- it silences the checkout refusal but breaks claude-code-action's own internal fetch of refs/pull/<n>/head (which only exists on this repo, not the fork), per Postgres-Extensions/extension_tools#28 hitting and fixing the identical mistake. The action already fetches and reads the PR's actual content itself; this step only needs to check out the base branch.
The review step drives claude-code-action with a bare prompt: (no @claude mention), which runs it in "agent mode". That mode decides which MCP servers to start from an --allowedTools flag inside claude_args, not from the invoked plugin's own allowed-tools frontmatter. Without mcp__github_inline_comment__create_inline_comment listed there, that MCP server never starts, so the code-review plugin silently falls back to one consolidated PR comment instead of real per-line inline comments.
…d repo owner head.repo.owner.login only identifies the fork for fork-headed PRs; for an upstream-branch-headed PR (base and head both in this repo) it's always this repo's own org, never the actual author, so the gate silently skipped review on every such PR regardless of who opened it. user.login is GitHub's own authenticated record of who opened the PR and isn't attacker-spoofable, so this isn't a weaker check -- it's the more correct one, and covers both fork-headed and upstream-headed PRs.
…onsolidate-claude-code-review
…onsolidate-claude-code-review # Conflicts: # .github/workflows/claude-code-review.yml
…e-perms' into consolidate-claude-code-review # Conflicts: # .github/workflows/claude-code-review.yml
📝 WalkthroughWalkthroughThe Claude review workflow now runs on selected pull request and label events. It restricts execution to non-draft pull requests from Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub
participant ReviewWorkflow
participant CI
participant Claude
GitHub->>ReviewWorkflow: Send pull request or claude-debug label event
ReviewWorkflow->>GitHub: Check author, draft status, and label state
ReviewWorkflow->>CI: Wait for sibling checks when not debugging
CI-->>ReviewWorkflow: Report check status
ReviewWorkflow->>Claude: Start review with configured output and inline-comment tools
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dropped along with the fork-checkout override, but it's independently worth keeping: this job never pushes anything, so there's no reason to leave a push-capable credential in .git/config for the rest of the job. Matches Postgres-Extensions/pg_count_nulls#53's version of this same fix.
…ckout step Lost when the fork-checkout override (repository:/ref:) was removed from this step's with: block earlier -- deleting the whole block wholesale threw away persist-credentials: false along with it, silently reverting to actions/checkout's default of true. This job's permissions grant pull-requests: write, a real write-capable credential; nothing here legitimately runs git push, so there's no reason to leave it persisted in .git/config for the rest of the job to misuse if anything later goes wrong. Found via the same pattern in Postgres-Extensions/object_reference#27 (see ~/more-fixes.md), where the same wholesale with:-block deletion had the same effect; pg_count_nulls#53's version of this fix got it right by removing only repository:/ref: and keeping fetch-depth/persist-credentials.
…ckout step Lost when the fork-checkout override (repository:/ref:) was removed from this step's with: block earlier -- deleting the whole block wholesale threw away persist-credentials: false along with it, silently reverting to actions/checkout's default of true. This job's permissions grant pull-requests: write, a real write-capable credential; nothing here legitimately runs git push, so there's no reason to leave it persisted in .git/config for the rest of the job to misuse if anything later goes wrong. Found via the same pattern in Postgres-Extensions/object_reference#27 (see ~/more-fixes.md), where the same wholesale with:-block deletion had the same effect; pg_count_nulls#53's version of this fix got it right by removing only repository:/ref: and keeping fetch-depth/persist-credentials.
jnasbyupgrade
changed the title
|
@claude review |
|
Claude finished @jnasbyupgrade's task in 4m 28s —— View job Review of PR #27 — fix and improve
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/claude-code-review.yml:
- 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.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a6e153c9-232e-4db3-8deb-9ec532e1ec37
📒 Files selected for processing (2)
.github/workflows/claude-code-review.yml.github/workflows/claude.yml
| # 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 |
There was a problem hiding this comment.
🔒 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/**' . || trueRepository: 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:
- 1: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target
- 2: https://www.wiz.io/blog/github-actions-security-threat-model-and-defenses
- 3: https://securitylab.github.com/resources/github-actions-new-patterns-and-mitigations/
- 4: https://adaptive-enforcement-lab.com/secure/github-actions-security/workflows/reusable/
- 5: https://adaptive-enforcement-lab.com/secure/github-actions-security/workflows/triggers/fork-patterns/
- 6: https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows
- 7: [BUG] Environment-scoped secrets unreachable from reusable workflow without secrets: inherit, despite called job declaring environment actions/runner#4453
- 8: https://stackoverflow.com/questions/76952023/how-to-make-github-actions-safely-access-secrets-for-prs-created-from-forks
- 9: https://docs.github.com/en/enterprise-cloud@latest/actions/reference/security/securely-using-pull_request_target
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
9c47926
into
Postgres-Extensions:master
1 participant
Summary
user.login(PR author), nothead.repo.owner.login-- correct for both fork-headed and upstream-branch-headed PRs.ref:/repository:override,persist-credentials: false) --claude-code-actionreads the PR's content itself.track_progress: true-- posts a live-updating comment instead of staying silent until the run finishes.claude-debugPR label -- skips the cost-gate wait and enables full transcript output when applied.claude_argsallows the inline-comment MCP tool, so findings post as real per-line comments instead of one consolidated comment.🤖 Generated with Claude Code