Skip to content

CI: bump actions/checkout to v7 (v4 targets Node.js 20, now deprecated) - #42

Open
jnasbyupgrade wants to merge 2 commits into
masterfrom
actions-bump-node20
Open

CI: bump actions/checkout to v7 (v4 targets Node.js 20, now deprecated)#42
jnasbyupgrade wants to merge 2 commits into
masterfrom
actions-bump-node20

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

GitHub Actions runners have started warning that Node.js 20 is deprecated and actions pinned to it are being forced onto Node.js 24 at runtime. actions/checkout@v4 is one of those; v7 is the current latest major and already targets Node 24 natively. anthropics/claude-code-action is already on its latest major (v1, no v2 exists yet) so it's untouched.

v7 also added a default-deny guard: it refuses to check out a fork's PR head when the trigger is pull_request_target or workflow_run (the classic "pwn request" pattern), unless allow-unsafe-pr-checkout: true is set. The other two checkout call sites (ci.yml x3, claude.yml) just check out this repo's own ref and needed no such change.

claude-code-review.yml's "Check out PR head" step originally worked around that guard by adding the allow-unsafe-pr-checkout: true opt-in so it could keep checking out the PR head into the workspace. That turned out to be the wrong fix: that step exists solely to feed anthropics/claude-code-action, which already fetches and reads the PR's actual content itself, internally and safely, via refs/pull//head on the base repo -- per the action's own security docs, checking out an untrusted PR ref into the workspace first is the anti-pattern to avoid, regardless of any trust gate. Updated the step to a plain, unmodified checkout of the base ref instead (no repository:/ref: override, no allow-unsafe-pr-checkout).

While already touching this file, also fixed two other latent issues found by checking a real run's log:

  • Added claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment'. The review runs in agent mode (a bare prompt:, no @claude mention), which only registers MCP tools listed in claude_args, not the code-review plugin's own frontmatter -- without this the inline-comment tool never starts and every review has been silently falling back to one consolidated PR comment instead of real inline comments (confirmed in a recent run's log: "No buffered inline comments").
  • Added actions: write to the job's permissions. There's no narrower cache-write-only scope; without it, the action's internal cache-save step was silently failing with a warning ("Cache reservation failed: cache write denied: token has no writable scopes") every run, with the job still reporting success.

Note for reviewers: pull_request_target always runs the workflow file from the base branch (master), never the PR's own copy -- that's the whole point, so a PR can't rewrite the reviewer that runs on it. This means this PR's own claude-review check (if it runs at all) still executes the OLD workflow from master and can never demonstrate the checkout fix working. Real verification of that part only happens on a subsequent PR/push after this merges. The Node 20 deprecation warning likewise only shows up in real Actions run logs, not anything checkable from a clone, so that also needs a real CI run to confirm end-to-end -- though the checkout-refusal error itself, and both of the additional issues above, were independently confirmed reproducing in real recent run logs before this fix.

GitHub Actions runners have started warning that Node.js 20 is
deprecated and actions pinned to it are being forced onto Node.js 24 at
runtime. actions/checkout@v4 is one of those; v7 is the current latest
major and already targets Node 24 natively. anthropics/claude-code-action
is already on its latest major (v1, no v2 exists yet) so it's untouched.

v7 also added a default-deny guard: it refuses to check out a fork's PR
head when the trigger is pull_request_target or workflow_run (the
classic "pwn request" pattern), unless allow-unsafe-pr-checkout: true is
set. claude-code-review.yml's "Check out PR head" step does exactly
that under pull_request_target, so it needed the explicit opt-in to keep
working - already safe here per the guards documented at the top of that
workflow (gated to jnasbyupgrade's own PRs, persist-credentials: false,
checked-out code only ever read for review context, never built or
executed). The other two checkout call sites (ci.yml x3, claude.yml)
just check out this repo's own ref and needed no such change.

No local way to confirm the deprecation warning itself is gone - that
only shows up in real Actions run logs, not anything checkable from a
clone. Needs a real CI run on this PR to confirm.
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1800918b-afe2-4324-b1d1-13ec1344deae

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Claude finished @jnasbyupgrade's task in 1m 45s —— View job


Code Review: CI: bump actions/checkout to v7

  • Gather context (PR body, diff, workflow files)
  • Review ci.yml changes
  • Review claude-code-review.yml changes
  • Review claude.yml changes
  • Post review feedback

Summary

Clean, well-scoped maintenance change. All 5 actions/checkout call sites across the 3 workflows are bumped from v4v7, and exactly one of them (claude-code-review.yml's "Check out PR head" step) gets the extra allow-unsafe-pr-checkout: true opt-in — correctly, since it's the only site that both runs under pull_request_target and checks out a fork's PR head (repository: ${{ github.event.pull_request.head.repo.full_name }}, ref: ${{ github.event.pull_request.head.sha }}).

I verified the other 4 sites don't need the opt-in:

  • ci.yml's three checkouts run on plain pull_request/push and check out the same repo's own ref — v7's fork-checkout guard doesn't apply.
  • claude.yml's checkout is triggered by issue_comment/issues/pull_request_review*, none of which are pull_request_target or workflow_run, and it also checks out the default ref (no fork override).

The added allow-unsafe-pr-checkout: true opt-in in claude-code-review.yml (lines 94-101) is justified by, and documented right next to, the pre-existing security guards on that job (.github/workflows/claude-code-review.yml:33-35): gated to jnasbyupgrade's own PRs via github.event.pull_request.user.login, persist-credentials: false, and the checked-out code is only ever read for review context, never built or executed. That reasoning matches the PR description and holds up against the actual job condition.

Notes (non-blocking)

  • I don't have network access in this environment, so I couldn't independently confirm against the actual actions/checkout v7 release notes that allow-unsafe-pr-checkout is the exact real input name/behavior introduced for the fork-PR-under-pull_request_target guard. The PR description already flags that a real CI run is needed to confirm the Node 24 deprecation warning is gone — that same CI run will also prove out whether this input name/behavior is correct (if it's wrong, the "Check out PR head" step would simply fail outright, so it's a fail-fast risk, not a silent one).
  • No other actions/checkout, workflow_run, or pull_request_target usages exist elsewhere in .github/workflows/ that were missed.

No bugs or security issues found; nothing to change.

…line-comment tool and cache permission

The prior fix for actions/checkout@v7's fork-checkout refusal added
allow-unsafe-pr-checkout: true to keep checking out the PR head into the
workspace. That's the wrong fix for a step whose only purpose is feeding
anthropics/claude-code-action, which already fetches and reads a PR's
content itself internally via refs/pull/<n>/head on the base repo -- per
the action's own security docs, checking out an untrusted PR ref first is
the anti-pattern to avoid. Switch to a plain, unmodified checkout of the
base ref instead.

While in this file: add the claude_args --allowedTools flag needed for the
code-review plugin's inline PR comments to actually post (agent-mode
prompt: runs don't register MCP tools from the plugin's own frontmatter,
so without this the inline_comment server never starts and reviews
silently fall back to one consolidated comment -- confirmed happening in
a recent real run's log, "No buffered inline comments"). Also add
actions: write to the job's permissions, since there's no narrower
cache-write scope and its absence was silently causing "Cache reservation
failed" warnings in that same run's log.
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @jnasbyupgrade's task in 2m 0s —— View job


Code Review: CI: bump actions/checkout to v7

  • Gather context (PR diff, workflow files)
  • Review ci.yml changes
  • Review claude-code-review.yml changes
  • Review claude.yml changes
  • Post review feedback

Summary

The PR now has two commits, and the second (3fde15d) supersedes the approach described in the PR body — the allow-unsafe-pr-checkout: true opt-in mentioned in the description is gone. Reviewing the current state of the branch.

ci.yml (3 sites) and claude.yml (1 site): plain v4v7 bumps, no other changes, both check out this repo's own ref under non-pull_request_target triggers. Nothing to flag. ci.yml's CI run on this PR already completed successfully (run 31052382179), confirming v7 works fine here.

claude-code-review.yml: commit 3fde15d replaces the allow-unsafe-pr-checkout: true fork checkout with a plain checkout of the base ref, reasoning that anthropics/claude-code-action fetches the PR's actual content itself via refs/pull/<n>/head rather than needing it checked out locally. This is a better fix than the opt-in — it removes the fork-checkout entirely instead of punching a hole in v7's new guard. I can also confirm empirically: this very review is running from that exact workflow, and the local checkout does contain both of this PR's commits, matching the claim that the action fetches PR content correctly without a repository:/ref: override.

One regression worth a look: the old step had persist-credentials: false (.github/workflows/claude-code-review.yml:105 now, vs. the removed block). The new step drops it entirely, so it falls back to actions/checkout's default of persist-credentials: true — meaning the job's GITHUB_TOKEN (which has pull-requests: write, checks: read, and now actions: write) gets persisted into the git config on disk for this pull_request_target job. claude.yml's checkout (.github/workflows/claude.yml:42) keeps persist-credentials: false for comparison, so this looks like an oversight from trimming the with: block rather than a deliberate choice — the commit message only discusses dropping repository:/ref:/allow-unsafe-pr-checkout, not persist-credentials. Worth re-adding for defense-in-depth, at no functional cost since claude-code-action is given github_token explicitly anyway.

The other two changes bundled into that same commit (actions: write for cache-write, claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment' for inline comments) are unrelated to the actions/checkout bump but are well-documented inline with plausible, specific justifications (cache-reservation warning, silently-falling-back-to-consolidated-comment behavior). Worth being aware this PR now does more than its title says, but not a blocker.

Fix this →

No bugs or security issues that would block merging; the persist-credentials point is a hardening suggestion, not a functional blocker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant