fix(ci): run code review on pull requests from forks - #6722
Conversation
Claude Code Review has been failing for every external contributor, e.g. PR #6677: error: Error message: Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable Attempt 3 failed: Could not fetch an OIDC token. For `pull_request` events raised from a fork, GitHub ignores the workflow's `permissions:` block: the run log shows Contents/PullRequests/Statuses all downgraded to read, no OIDC token is minted despite `id-token: write`, and repository secrets are withheld, so `claude_code_oauth_token` is empty too. The job could not authenticate, and could not have posted a review if it had. Switch to `pull_request_target`, which runs in base-repository context and therefore has secrets, OIDC and real permissions. That makes the PR diff untrusted input to a job holding write scope, so: - Permissions cut to `contents: read` + `pull-requests: write`. The previous `contents: write`, `checks: write`, `statuses: write` and `issues: write` grants were inert under `pull_request` from forks but would have been real here. - The base ref stays at the workspace root; the PR head is checked out into `pr-head/` with `persist-credentials: false` and exposed via `--add-dir`, so untrusted code is never the working directory. - `github_token` + `allowed_non_write_users` let the fork author trigger the run, since the actor never has write access. - The only write path is .github/scripts/pr-review-comment.sh, which takes the PR number from the environment rather than an argument so it cannot be retargeted, and takes the body as an argument rather than a path so no file on the runner can be turned into a public comment. Capped at 2 calls via CLAUDE_CODE_SCRIPT_CAPS. - `--allowedTools` narrowed from bare `Bash,Read,Glob,Grep,WebFetch,WebSearch` to Read/Glob/Grep, read-only `gh pr` and `git` commands, and that helper. WebFetch/WebSearch are dropped - with untrusted content in context they are an exfiltration channel. - The prompt states the diff and pr-head/ are data, not instructions. Claude-Session: https://claude.ai/code/session_01UGaA2Fjvb2F3iYAz2rifM8
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe PR adds a validated PR comment helper. The review workflow now uses ChangesSecure PR review workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This change enables fork PR reviews while limiting repository access and constraining comments to the triggering pull request. No current merge-blocking risk is identified. Sequence Diagram(s)sequenceDiagram
participant GitHub_Actions
participant Claude_Code_Review
participant pr_head
participant pr_review_comment_sh
participant gh
GitHub_Actions->>Claude_Code_Review: provide PR_NUMBER, GH_REPO, and scoped token
Claude_Code_Review->>gh: read PR view and diff
Claude_Code_Review->>pr_head: read PR head
Claude_Code_Review->>pr_review_comment_sh: submit review markdown
pr_review_comment_sh->>gh: post comment to triggering pull request
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR changes Claude review execution to
Confidence Score: 3/5The PR is not safe to merge until untrusted added-directory configuration is disabled and the base checkout stops persisting its write-capable credential. Fork-controlled Claude skills can be loaded from Files Needing Attention: .github/workflows/claude-code-review.yml
|
| Filename | Overview |
|---|---|
| .github/workflows/claude-code-review.yml | Moves reviews to a hardened pull_request_target workflow, but added-directory configuration discovery can combine with the persisted base credential to disclose the write-capable token. |
| .github/scripts/pr-review-comment.sh | Adds a narrowly targeted, quoted helper that rejects empty bodies and posts only to the repository and pull request supplied by trusted workflow environment variables. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Fork[Untrusted fork PR] --> Skill[pr-head/.claude skill or command]
Skill --> AddDir[--add-dir pr-head]
AddDir --> Agent[Privileged review agent]
Checkout[Base checkout] --> Token[Persisted GITHUB_TOKEN]
Token --> Read[Unrestricted Read]
Agent --> Read
Read --> Helper[PR comment helper]
Helper --> Public[Public PR comment]
Reviews (1): Last reviewed commit: "fix(ci): run code review on pull request..." | Re-trigger Greptile
| # or https://code.claude.com/docs/en/cli-reference for available options | ||
|
|
||
| claude_args: | | ||
| --add-dir pr-head --allowedTools "Read,Glob,Grep,Bash(.github/scripts/pr-review-comment.sh:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(git diff:*),Bash(git log:*),Bash(git show:*)" |
There was a problem hiding this comment.
Untrusted skills expose credentials
Passing the untrusted pr-head directory through --add-dir causes Claude Code to load PR-controlled .claude/skills/ and .claude/commands/. The base checkout also persists the write-capable GITHUB_TOKEN, and unrestricted Read can access its runner-owned credential file. A fork can therefore supply instructions that make the agent read the token and publish it through the permitted comment helper. Disable project configuration discovery for the added directory and set persist-credentials: false on the base checkout.
How this was verified: The configured Claude version loads skills from added directories, while the base checkout stores its pull-request-write token in an OS-readable runner file and the comment helper publishes model-provided text verbatim.
#6722 got the trigger right, but the run now fails one step later: Refusing to check out fork pull request code from a 'pull_request_target' workflow. ... set 'allow-unsafe-pr-checkout: true' on the actions/checkout step. actions/checkout blocks fork checkouts under pull_request_target because fetching and then EXECUTING fork code in the trusted context is the classic pwn request. This workflow only reads it: the head lands in pr-head/ rather than the workspace root, no build, restore or test step runs against it, and Claude's tools are limited to Read/Glob/Grep plus read-only git and gh. Take the documented opt-in, with a comment recording the conditions it depends on so a later step that builds or runs anything from pr-head/ has to revisit it. Claude-Session: https://claude.ai/code/session_014wbSbRPmzSRgvRV5EwxHZE
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.65.68 to 1.66.16. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.66.16 <!-- Release notes generated using configuration in .github/release.yml at v1.66.16 --> ## What's Changed ### Other Changes * fix: isolated name is lowercase (#6727) by @koryphaee in thomhurst/TUnit#6728 * fix: preserve concurrent Assert.Multiple failures by @thomhurst in thomhurst/TUnit#6730 * fix: preserve original HTTP mock request content by @thomhurst in thomhurst/TUnit#6731 ### Dependencies * chore(deps): update tunit to 1.66.10 by @thomhurst in thomhurst/TUnit#6726 * chore(deps): update dependency dompurify to v3.4.15 by @thomhurst in thomhurst/TUnit#6732 **Full Changelog**: thomhurst/TUnit@v1.66.10...v1.66.16 ## 1.66.10 <!-- Release notes generated using configuration in .github/release.yml at v1.66.10 --> ## What's Changed ### Other Changes * fix: restore null suppression for built-in assertion methods by @thomhurst in thomhurst/TUnit#6725 ### Dependencies * chore(deps): update tunit to 1.66.8 by @thomhurst in thomhurst/TUnit#6724 **Full Changelog**: thomhurst/TUnit@v1.66.8...v1.66.10 ## 1.66.8 <!-- Release notes generated using configuration in .github/release.yml at v1.66.8 --> ## What's Changed ### Other Changes * fix(ci): make issue triage work for external reporters by @thomhurst in thomhurst/TUnit#6720 * fix(ci): run code review on pull requests from forks by @thomhurst in thomhurst/TUnit#6722 * fix: suppress nullability warnings after Should NotBeNull assertions by @mvanhorn in thomhurst/TUnit#6700 * fix: Avoid HTML report CLI option clashes by @mvanhorn in thomhurst/TUnit#6677 ### Dependencies * chore(deps): update tunit to 1.66.0 by @thomhurst in thomhurst/TUnit#6719 * chore(deps): update dependency microsoft.kiota.abstractions to 2.1.1 by @thomhurst in thomhurst/TUnit#6721 * chore(deps): update dependency awssdk.sqs to 4.0.100.12 by @thomhurst in thomhurst/TUnit#6723 **Full Changelog**: thomhurst/TUnit@v1.66.0...v1.66.8 ## 1.66.0 <!-- Release notes generated using configuration in .github/release.yml at v1.66.0 --> ## What's Changed ### Other Changes * Compile all C# documentation snippets by @thomhurst in thomhurst/TUnit#6695 * Fix `Type` assignability assertions to evaluate represented type (not `RuntimeType`) by @thomhurst with @Copilot in thomhurst/TUnit#6711 * Clarify ClassDataSource constructor requirements by @thomhurst in thomhurst/TUnit#6716 * Add programmatic HTML reporting settings by @thomhurst in thomhurst/TUnit#6699 * Fix timeout cancellation diagnostics by @thomhurst in thomhurst/TUnit#6715 ### Dependencies * chore(deps): update tunit to 1.65.68 by @thomhurst in thomhurst/TUnit#6682 * chore(deps): update dependency verify.tool to v0.9.1 by @thomhurst in thomhurst/TUnit#6683 * chore(deps): update dependency mockolate to 3.4.1 by @thomhurst in thomhurst/TUnit#6685 * chore(deps): update dependency serialize-javascript to v7.1.1 by @thomhurst in thomhurst/TUnit#6687 * chore(deps): update dependency qs to v6.16.0 by @thomhurst in thomhurst/TUnit#6691 * chore(deps): update dependency system.reactive to v7 by @thomhurst in thomhurst/TUnit#6696 * chore(deps): update dependency imposter to 0.1.10 by @thomhurst in thomhurst/TUnit#6701 * chore(deps): update dependency microsoft.kiota.abstractions to 2.1.0 by @thomhurst in thomhurst/TUnit#6704 * chore(deps): update mstest to 4.4.0 by @thomhurst in thomhurst/TUnit#6705 * chore(deps): update dependency dotnet-trace to v10 by @thomhurst in thomhurst/TUnit#6706 * chore(deps): update microsoft.testing by @thomhurst in thomhurst/TUnit#6703 * chore(deps): update microsoft.testing by @thomhurst in thomhurst/TUnit#6713 * chore(deps): bump fast-uri from 3.1.5 to 3.1.7 in /docs by @dependabot[bot] in thomhurst/TUnit#6707 **Full Changelog**: thomhurst/TUnit@v1.65.68...v1.66.0 Commits viewable in [compare view](thomhurst/TUnit@v1.65.68...v1.66.16). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.66.0 to 1.66.16. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.66.16 <!-- Release notes generated using configuration in .github/release.yml at v1.66.16 --> ## What's Changed ### Other Changes * fix: isolated name is lowercase (#6727) by @koryphaee in thomhurst/TUnit#6728 * fix: preserve concurrent Assert.Multiple failures by @thomhurst in thomhurst/TUnit#6730 * fix: preserve original HTTP mock request content by @thomhurst in thomhurst/TUnit#6731 ### Dependencies * chore(deps): update tunit to 1.66.10 by @thomhurst in thomhurst/TUnit#6726 * chore(deps): update dependency dompurify to v3.4.15 by @thomhurst in thomhurst/TUnit#6732 **Full Changelog**: thomhurst/TUnit@v1.66.10...v1.66.16 ## 1.66.10 <!-- Release notes generated using configuration in .github/release.yml at v1.66.10 --> ## What's Changed ### Other Changes * fix: restore null suppression for built-in assertion methods by @thomhurst in thomhurst/TUnit#6725 ### Dependencies * chore(deps): update tunit to 1.66.8 by @thomhurst in thomhurst/TUnit#6724 **Full Changelog**: thomhurst/TUnit@v1.66.8...v1.66.10 ## 1.66.8 <!-- Release notes generated using configuration in .github/release.yml at v1.66.8 --> ## What's Changed ### Other Changes * fix(ci): make issue triage work for external reporters by @thomhurst in thomhurst/TUnit#6720 * fix(ci): run code review on pull requests from forks by @thomhurst in thomhurst/TUnit#6722 * fix: suppress nullability warnings after Should NotBeNull assertions by @mvanhorn in thomhurst/TUnit#6700 * fix: Avoid HTML report CLI option clashes by @mvanhorn in thomhurst/TUnit#6677 ### Dependencies * chore(deps): update tunit to 1.66.0 by @thomhurst in thomhurst/TUnit#6719 * chore(deps): update dependency microsoft.kiota.abstractions to 2.1.1 by @thomhurst in thomhurst/TUnit#6721 * chore(deps): update dependency awssdk.sqs to 4.0.100.12 by @thomhurst in thomhurst/TUnit#6723 **Full Changelog**: thomhurst/TUnit@v1.66.0...v1.66.8 Commits viewable in [compare view](thomhurst/TUnit@v1.66.0...v1.66.16). </details> Updated [TUnit.AspNetCore](https://github.com/thomhurst/TUnit) from 1.66.0 to 1.66.16. <details> <summary>Release notes</summary> _Sourced from [TUnit.AspNetCore's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.66.16 <!-- Release notes generated using configuration in .github/release.yml at v1.66.16 --> ## What's Changed ### Other Changes * fix: isolated name is lowercase (#6727) by @koryphaee in thomhurst/TUnit#6728 * fix: preserve concurrent Assert.Multiple failures by @thomhurst in thomhurst/TUnit#6730 * fix: preserve original HTTP mock request content by @thomhurst in thomhurst/TUnit#6731 ### Dependencies * chore(deps): update tunit to 1.66.10 by @thomhurst in thomhurst/TUnit#6726 * chore(deps): update dependency dompurify to v3.4.15 by @thomhurst in thomhurst/TUnit#6732 **Full Changelog**: thomhurst/TUnit@v1.66.10...v1.66.16 ## 1.66.10 <!-- Release notes generated using configuration in .github/release.yml at v1.66.10 --> ## What's Changed ### Other Changes * fix: restore null suppression for built-in assertion methods by @thomhurst in thomhurst/TUnit#6725 ### Dependencies * chore(deps): update tunit to 1.66.8 by @thomhurst in thomhurst/TUnit#6724 **Full Changelog**: thomhurst/TUnit@v1.66.8...v1.66.10 ## 1.66.8 <!-- Release notes generated using configuration in .github/release.yml at v1.66.8 --> ## What's Changed ### Other Changes * fix(ci): make issue triage work for external reporters by @thomhurst in thomhurst/TUnit#6720 * fix(ci): run code review on pull requests from forks by @thomhurst in thomhurst/TUnit#6722 * fix: suppress nullability warnings after Should NotBeNull assertions by @mvanhorn in thomhurst/TUnit#6700 * fix: Avoid HTML report CLI option clashes by @mvanhorn in thomhurst/TUnit#6677 ### Dependencies * chore(deps): update tunit to 1.66.0 by @thomhurst in thomhurst/TUnit#6719 * chore(deps): update dependency microsoft.kiota.abstractions to 2.1.1 by @thomhurst in thomhurst/TUnit#6721 * chore(deps): update dependency awssdk.sqs to 4.0.100.12 by @thomhurst in thomhurst/TUnit#6723 **Full Changelog**: thomhurst/TUnit@v1.66.0...v1.66.8 Commits viewable in [compare view](thomhurst/TUnit@v1.66.0...v1.66.16). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Updated [TUnit.Core](https://github.com/thomhurst/TUnit) from 1.65.68 to 1.66.27. <details> <summary>Release notes</summary> _Sourced from [TUnit.Core's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.66.27 <!-- Release notes generated using configuration in .github/release.yml at v1.66.27 --> ## What's Changed ### Other Changes * fix(mocks): initialize mock state before base constructor callbacks by @thomhurst in thomhurst/TUnit#6741 ### Dependencies * chore(deps): update tunit to 1.66.16 by @thomhurst in thomhurst/TUnit#6733 * chore(deps): update dependency testcontainers.postgresql to 4.15.0 by @thomhurst in thomhurst/TUnit#6736 * chore(deps): update dependency testcontainers.redis to 4.15.0 by @thomhurst in thomhurst/TUnit#6737 * chore(deps): update dependency testcontainers.kafka to 4.15.0 by @thomhurst in thomhurst/TUnit#6735 * chore(deps): update dependency mockolate to 3.5.0 by @thomhurst in thomhurst/TUnit#6739 **Full Changelog**: thomhurst/TUnit@v1.66.16...v1.66.27 ## 1.66.16 <!-- Release notes generated using configuration in .github/release.yml at v1.66.16 --> ## What's Changed ### Other Changes * fix: isolated name is lowercase (#6727) by @koryphaee in thomhurst/TUnit#6728 * fix: preserve concurrent Assert.Multiple failures by @thomhurst in thomhurst/TUnit#6730 * fix: preserve original HTTP mock request content by @thomhurst in thomhurst/TUnit#6731 ### Dependencies * chore(deps): update tunit to 1.66.10 by @thomhurst in thomhurst/TUnit#6726 * chore(deps): update dependency dompurify to v3.4.15 by @thomhurst in thomhurst/TUnit#6732 **Full Changelog**: thomhurst/TUnit@v1.66.10...v1.66.16 ## 1.66.10 <!-- Release notes generated using configuration in .github/release.yml at v1.66.10 --> ## What's Changed ### Other Changes * fix: restore null suppression for built-in assertion methods by @thomhurst in thomhurst/TUnit#6725 ### Dependencies * chore(deps): update tunit to 1.66.8 by @thomhurst in thomhurst/TUnit#6724 **Full Changelog**: thomhurst/TUnit@v1.66.8...v1.66.10 ## 1.66.8 <!-- Release notes generated using configuration in .github/release.yml at v1.66.8 --> ## What's Changed ### Other Changes * fix(ci): make issue triage work for external reporters by @thomhurst in thomhurst/TUnit#6720 * fix(ci): run code review on pull requests from forks by @thomhurst in thomhurst/TUnit#6722 * fix: suppress nullability warnings after Should NotBeNull assertions by @mvanhorn in thomhurst/TUnit#6700 * fix: Avoid HTML report CLI option clashes by @mvanhorn in thomhurst/TUnit#6677 ### Dependencies * chore(deps): update tunit to 1.66.0 by @thomhurst in thomhurst/TUnit#6719 * chore(deps): update dependency microsoft.kiota.abstractions to 2.1.1 by @thomhurst in thomhurst/TUnit#6721 * chore(deps): update dependency awssdk.sqs to 4.0.100.12 by @thomhurst in thomhurst/TUnit#6723 **Full Changelog**: thomhurst/TUnit@v1.66.0...v1.66.8 ## 1.66.0 <!-- Release notes generated using configuration in .github/release.yml at v1.66.0 --> ## What's Changed ### Other Changes * Compile all C# documentation snippets by @thomhurst in thomhurst/TUnit#6695 * Fix `Type` assignability assertions to evaluate represented type (not `RuntimeType`) by @thomhurst with @Copilot in thomhurst/TUnit#6711 * Clarify ClassDataSource constructor requirements by @thomhurst in thomhurst/TUnit#6716 * Add programmatic HTML reporting settings by @thomhurst in thomhurst/TUnit#6699 * Fix timeout cancellation diagnostics by @thomhurst in thomhurst/TUnit#6715 ### Dependencies * chore(deps): update tunit to 1.65.68 by @thomhurst in thomhurst/TUnit#6682 * chore(deps): update dependency verify.tool to v0.9.1 by @thomhurst in thomhurst/TUnit#6683 * chore(deps): update dependency mockolate to 3.4.1 by @thomhurst in thomhurst/TUnit#6685 * chore(deps): update dependency serialize-javascript to v7.1.1 by @thomhurst in thomhurst/TUnit#6687 * chore(deps): update dependency qs to v6.16.0 by @thomhurst in thomhurst/TUnit#6691 * chore(deps): update dependency system.reactive to v7 by @thomhurst in thomhurst/TUnit#6696 * chore(deps): update dependency imposter to 0.1.10 by @thomhurst in thomhurst/TUnit#6701 * chore(deps): update dependency microsoft.kiota.abstractions to 2.1.0 by @thomhurst in thomhurst/TUnit#6704 * chore(deps): update mstest to 4.4.0 by @thomhurst in thomhurst/TUnit#6705 * chore(deps): update dependency dotnet-trace to v10 by @thomhurst in thomhurst/TUnit#6706 * chore(deps): update microsoft.testing by @thomhurst in thomhurst/TUnit#6703 * chore(deps): update microsoft.testing by @thomhurst in thomhurst/TUnit#6713 * chore(deps): bump fast-uri from 3.1.5 to 3.1.7 in /docs by @dependabot[bot] in thomhurst/TUnit#6707 **Full Changelog**: thomhurst/TUnit@v1.65.68...v1.66.0 Commits viewable in [compare view](thomhurst/TUnit@v1.65.68...v1.66.27). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Follow-up to #6720 (which fixed the same class of breakage for Issue Triage). Different root cause, though.
Root cause
claude-reviewfails on every fork PR — e.g. #6677:id-token: writeis declared. The problem is that forpull_requestevents raised from a fork, GitHub ignores thepermissions:block. From that run's own log:contents: writeContents: readpull-requests: writePullRequests: readstatuses: writeStatuses: readid-token: writeSecrets are withheld too, so
secrets.CLAUDE_CODE_OAUTH_TOKENis empty. The job cannot authenticate, and could not have posted a review even if it had. This is a platform constraint, not a misconfiguration — it can only be fixed by changing the trigger.Fix
Switch to
pull_request_target, which runs in base-repository context and so has secrets, OIDC and real permissions.That inverts the trust model: the PR diff becomes untrusted input to a job that holds write scope. Hardening accordingly:
contents: read+pull-requests: write. The oldcontents: write/checks: write/statuses: write/issues: writewere inert under the old trigger but would be real under this one.pr-head/withpersist-credentials: false, exposed via--add-dir— the pattern from the action's security docs.github_token+allowed_non_write_users: "*", since the actor is the fork author and never has write access..github/scripts/pr-review-comment.shtakes the PR number from the environment rather than an argument (cannot be retargeted) and the body as an argument rather than a path (no file on the runner can be turned into a public comment). Capped at 2 calls viaCLAUDE_CODE_SCRIPT_CAPS.--allowedToolsnarrowed from bareBash,Read,Glob,Grep,WebFetch,WebSearchtoRead/Glob/Grep, read-onlygh prandgitcommands, and that helper.WebFetch/WebSearchare dropped — with untrusted content in context they're an exfiltration channel.pr-head/are data, not instructions.Residual risk: an injected instruction in a fork diff could cause an unwanted comment on that same PR. Bounded to
pull-requests: write, with no code access.Verification
pull_request_targetworkflows run from the base branch, so this PR's ownclaude-reviewcheck still executes the old file and will still fail. The change can only be verified on the next fork PR after merge.https://claude.ai/code/session_01UGaA2Fjvb2F3iYAz2rifM8
Summary by CodeRabbit