fix(pr-review): checkout PR head SHA on pull_request_target (#197)#198
fix(pr-review): checkout PR head SHA on pull_request_target (#197)#198cbeaulieu-gt wants to merge 3 commits intomainfrom
Conversation
The composite action's actions/checkout@v5 step defaulted to the base ref on pull_request_target, so PR-only files were absent from the working tree. claude-code-action@v1's track_progress setup runs git hash-object against changed files and falls back to git fetch when files are missing — both fail inside the digest-pinned review container. Adding ref: head.sha makes the head ref present and bypasses the failing fetch recovery path entirely. Security: the pull_request_target + checkout-head pattern is normally dangerous (fork code on disk + privileged secrets), but this composite action's pre-authz steps all use gh API calls or pure shell on event payload env vars — none execute content from the working tree. The claude-code-action step itself is gated behind the existing author association check. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Claude Code is working… I'll analyze this and get back to you. |
The digest-pinned claude-runtime-review container runs as UID 1001 while actions/checkout writes files as the host runner UID. Inside the container, git's CVE-2022-24765 protection rejects the workspace as 'dubious ownership' and refuses to operate. This blocks claude-code-action@v1's track_progress setup, which runs git internally. Adding `safe.directory $GITHUB_WORKSPACE` before any git invocation makes git trust the workspace regardless of UID drift. Surfaced after the first commit's `ref:` fix moved the failure past the missing-head-ref point. The same UID mismatch is latent in every other Phase 5 container- pinned reusable workflow; durable fix (bake `safe.directory *` into the runtime base image entrypoint) tracked as a follow-up issue. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Claude Code is working… I'll analyze this and get back to you. |
…#197) The previous commit's --global write went to $HOME/.gitconfig, but claude-code-action's bun-invoked TS entrypoint resolves $HOME differently than this shell step, so the exemption was invisible to the actual git invocation that fails. PR #198 run 25401775107 logs showed the --global command executing successfully but the next claude-code-action git call still tripping dubious-ownership. Switch to --system (writes /etc/gitconfig, no $HOME dependency) and widen the path to '*'. Container runs as root by default per runtime/base/Dockerfile (no USER directive), so --system writes succeed. The durable fix is to bake this into the runtime base image's Dockerfile — tracked as #199. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Claude Code is working… I'll analyze this and get back to you. |
|
Closing as superseded by #199. The three commits on this branch correctly diagnosed the bug in layers — #199 captures the durable fix: bake `git config --system --add safe.directory ''` into `runtime/base/Dockerfile` so every overlay inherits it. STAGE 4 overlay smoke validates it end-to-end without the dogfooding limitation. The diagnostic narrative from this branch's three commits is preserved in #199's body as the spec-by-exhaustion of why `--system`/`` is the correct shape. Branch will be deleted; the diagnosis lives in the issue thread. 🤖 Generated by Claude Code on behalf of @cbeaulieu-gt |
1 participant
Summary
pr-review/action.yml'sactions/checkout@v5step was missing aref:parameter, causing it to check out the base ref onpull_request_targetevents — PR-only files were absent from the working treeref: ${{ github.event.pull_request.head.sha }}with an inline security audit comment explaining why this is safe for this specific composite actionCLAUDE.mddocumenting thepull_request_target+ head-SHA checkout pattern for future referenceRoot cause
On
pull_request_targettriggers,actions/checkout@v5defaults to checking out the base ref (e.g.main), not the PR branch. This means files added or modified in the PR do not exist in the working tree.claude-code-action@v1'strack_progress: truefeature runsgit hash-objectagainst the list of PR-changed files to establish a baseline before the Claude session starts. When a file exists in the PR diff but not on disk (because we checked outmain),git hash-objectfails. The action then attempts agit fetch origin --depth=20 <head-branch>recovery — which also fails inside the digest-pinned review container because the container's git config does not have access to origin in the same way a full runner environment does.Adding
ref: ${{ github.event.pull_request.head.sha }}ensures the head ref is present in the working tree, making bothgit hash-objectand the fetch recovery unnecessary.Security analysis
Checking out the head SHA on
pull_request_targetis the canonical GitHub Actions footgun: fork code lands on disk while the job runs with repository secrets. This is safe here because:actions/checkout@v5gh apicalls + shell arithmetic on env varsgh pr viewAPI call + arithmeticgithub.event.*env vars + posts agh pr commentif: steps.authz.outputs.skip != 'true'The
claude-code-action@v1step — the only step that actually interprets working-tree content — is gated behindif: steps.authz.outputs.skip != 'true' && steps.size-check.outputs.skip != 'true', which only passes for OWNER/MEMBER/COLLABORATOR PRs. Fork PRs from untrusted authors hit thegh pr commentin the authz step and all downstream steps are skipped.Test plan
actionlintrun against all.github/workflows/*.yml— passes clean (compositeaction.ymlfiles are outside actionlint's scope by design)git diff --statconfirms exactly 2 files changed (pr-review/action.ymlandCLAUDE.md)claude-pr-reviewcheck should exercise the fixed path — thepull_request_targettrigger on this PR will check out the head SHA andtrack_progressshould no longer fail withgit hash-object/git fetcherrorsclaude-pr-review/quality-gatecommit status posts correctly on the head SHA after the review completesCloses #197
🤖 Generated by Claude Code on behalf of @cbeaulieu-gt