CI: bump actions/checkout to v7 (v4 targets Node.js 20, now deprecated) - #42
CI: bump actions/checkout to v7 (v4 targets Node.js 20, now deprecated)#42jnasbyupgrade wants to merge 2 commits into
Conversation
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.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Claude finished @jnasbyupgrade's task in 1m 45s —— View job Code Review: CI: bump actions/checkout to v7
SummaryClean, well-scoped maintenance change. All 5 I verified the other 4 sites don't need the opt-in:
The added Notes (non-blocking)
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.
|
Claude finished @jnasbyupgrade's task in 2m 0s —— View job Code Review: CI: bump actions/checkout to v7
SummaryThe PR now has two commits, and the second (
One regression worth a look: the old step had The other two changes bundled into that same commit ( No bugs or security issues that would block merging; the |
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:
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.