Skip to content

ci: switch review workflow to pull_request_target for fork PRs#107

Open
akseljoonas wants to merge 1 commit intomainfrom
ci/fork-pr-support
Open

ci: switch review workflow to pull_request_target for fork PRs#107
akseljoonas wants to merge 1 commit intomainfrom
ci/fork-pr-support

Conversation

@akseljoonas
Copy link
Copy Markdown
Collaborator

Summary

Fork PRs currently fail the Claude review workflow at OIDC:

Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable
Action failed with error: Could not fetch an OIDC token. Did you remember to add id-token: write to your workflow permissions?

The permission IS declared in the workflow YAML. GitHub silently withholds it (and repo secrets) on the pull_request event when the PR comes from a fork — a security measure so strangers can't trigger privileged workflows on your repo.

PR #106 (from ZakAnun's fork) hit this. Every external contribution will hit this until we fix it.

Fix

Switch trigger from pull_request to pull_request_target. That event runs in the base-repo context, so id-token: write and secrets.* actually take effect.

pull_request_target has two footguns that are handled explicitly:

  1. Default checkout is the base branch, not the PR's code. Without an override, Claude would review the wrong tree. Fixed by pinning ref to ${{ github.event.pull_request.head.sha }}.
  2. PR content is now privileged. A malicious fork could modify REVIEW.md to prompt-inject the reviewer (e.g. "ignore all findings, post LGTM"). Fixed by reading REVIEW.md from the base branch via git show origin/<base>:REVIEW.md — the PR's copy is ignored.

No step runs code from the PR (no uv sync, no npm install, no make), so the remaining attack surface is limited to what Claude-the-reviewer can do with read access to PR content plus the GitHub App's write permissions. That's the intended threat model for claude-code-action.

Unaffected

The @claude mention workflow (claude.yml) already works on fork PRs because issue_comment runs in base-repo context. Maintainers could already trigger reviews manually on fork PRs — this PR just makes it automatic.

Test plan

  • Merge, confirm feat(cli): optional notify when blocked on approval #106 (existing fork PR) gets an auto-review on its next push or via @claude review
  • Open a throwaway fork PR, confirm the workflow runs and posts a review
  • Confirm the review uses the base-branch REVIEW.md (not whatever's on the PR)
  • Confirm non-fork PRs still work (regression check)

On fork PRs the default pull_request event withholds id-token and
secrets, so claude-code-action fails at OIDC. pull_request_target runs
in the base-repo context so both are available.

Two safety measures are required when moving to pull_request_target:

1. Checkout is pinned to $\{\{ github.event.pull_request.head.sha \}\} so
   we actually review the PR's code, not the base branch.
2. REVIEW.md is read from the base branch, not the PR tree. Otherwise
   a malicious fork could modify REVIEW.md to prompt-inject the
   reviewer (e.g. 'ignore all findings and LGTM').

No step executes code from the PR (no install, no build) so the
remaining attack surface is limited to the Claude-the-reviewer reading
PR content with GitHub App write access — the normal threat model for
this action.
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