Repository navigation
fix(ci): skip Claude review on fork PRs - #1009
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughWalkthroughUpdated the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
6c567c1 to
52db6a2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/claude-code-review.yml (1)
103-146: 🧹 Nitpick | 🔵 Trivial
respondjob lacks equivalent fork handling.The
reviewjob now handles fork PRs with aGITHUB_TOKENfallback, but therespondjob (for@claudementions) does not include similar handling. If a user mentions@claudeon a fork PR, it may still fail with OIDC errors.If fork PR interactions via
@claudeare expected, consider adding the same conditionalgithub_tokeninput here. If not, consider adding a fork-exclusion condition to therespondjob'sifclause to fail fast with a clear skip rather than an authentication error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/claude-code-review.yml around lines 103 - 146, The respond job (named respond) currently calls anthropics/claude-code-action with only anthropic_api_key and will fail on fork PRs like the review job; either add the same conditional github_token fallback input used in the review job (pass github_token: ${{ secrets.GITHUB_TOKEN }} when github.event.pull_request.head.repo.fork == true) to the claude-code-action step or tighten the respond job's if condition to exclude forks (e.g., require github.event.pull_request.head.repo.fork == false) so it fails fast; update the claude_args/plugin inputs only as needed and reference the respond job and the claude-code-action step when applying the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/claude-code-review.yml:
- Line 61: The current github_token input is being set to an empty string for
non-fork PRs which prevents OIDC fallback; instead split the invocation into two
conditional steps (or duplicate the step) so one step runs when
github.event.pull_request.head.repo.fork is true and includes github_token: ${{
github.token }}, and the other runs when that condition is false and omits the
github_token input entirely; update the workflow to use these two conditional
steps so the github_token key is never present for non-fork PRs (refer to the
github_token input line and the github.event.pull_request.head.repo.fork
condition in your change).
- Around line 59-61: The workflow currently sets github_token via the
github_token expression and the prompt always instructs Claude to call
mcp__github_inline_comment__create_inline_comment, but fork PRs run as
github-actions[bot] and cannot post inline review comments; update the
workflow/prompt logic so fork PRs do not attempt inline comments: detect fork
PRs using the existing github_token expression
(github.event.pull_request.head.repo.fork) and for forks either (a) switch the
prompt to request summary-only comments instead of calling
mcp__github_inline_comment__create_inline_comment, (b) add a clear documented
limitation in the prompt that inline comments will be skipped for forks, or (c)
revert to skipping fork PRs entirely—ensure the change is applied where the
prompt or action invokes mcp__github_inline_comment__create_inline_comment and
that the prompt path for fork PRs is altered accordingly.
---
Outside diff comments:
In @.github/workflows/claude-code-review.yml:
- Around line 103-146: The respond job (named respond) currently calls
anthropics/claude-code-action with only anthropic_api_key and will fail on fork
PRs like the review job; either add the same conditional github_token fallback
input used in the review job (pass github_token: ${{ secrets.GITHUB_TOKEN }}
when github.event.pull_request.head.repo.fork == true) to the claude-code-action
step or tighten the respond job's if condition to exclude forks (e.g., require
github.event.pull_request.head.repo.fork == false) so it fails fast; update the
claude_args/plugin inputs only as needed and reference the respond job and the
claude-code-action step when applying the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4c90578e-17ad-4493-aea9-9ed895143b18
📒 Files selected for processing (1)
.github/workflows/claude-code-review.yml
| anthropic_api_key: ${{ env.ANTHROPIC_API_KEY }} | ||
| # Fork PRs: OIDC fails, fall back to GITHUB_TOKEN (posts as github-actions[bot]) | ||
| # Non-fork PRs: OIDC succeeds, uses Claude App token (posts as claude[bot]) | ||
| github_token: ${{ github.event.pull_request.head.repo.fork && github.token || '' }} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does anthropics/claude-code-action support passing empty string for github_token to fall back to OIDC authentication?
💡 Result:
No, anthropics/claude-code-action does not support passing an empty string for github_token to fall back to OIDC authentication. The github_token input is optional and defaults to empty. When not provided (empty), the action attempts OIDC authentication by default, exchanging an OIDC token for a Claude GitHub App token via Anthropic's /github-app-token-exchange endpoint. This requires proper setup: Claude GitHub App installed on the repo, id-token: write permission, and workflow validation. Providing a github_token (even the default GITHUB_TOKEN) bypasses OIDC and uses the provided token directly. There is no explicit fallback mechanism for empty strings beyond the default behavior. Issues occur with OIDC due to validation failures (e.g., actor permissions, workflow mismatches), resolved by providing a custom github_token. Examples from action.yml: github_token: required: false (defaults to ""), passed as OVERRIDE_GITHUB_TOKEN. Docs and issues confirm: omit for OIDC/GitHub App, provide for custom token.
Citations:
- 1: https://github.com/anthropics/claude-code-action/blob/main/docs/faq.md
- 2: https://github.com/anthropics/claude-code-action/blob/main/docs/setup.md
- 3: https://github.com/anthropics/claude-code-action
- 4: https://docs.claude.com/en/docs/claude-code/github-actions
- 5: https://github.com/anthropics/claude-code-action/blob/eb99fb38/action.yml
- 6: https://github.com/anthropics/claude-code-action/blob/eb99fb38/base-action/action.yml
Use conditional input omission instead of empty string for OIDC fallback.
The action requires the github_token input to be omitted entirely for OIDC authentication, not set to an empty string. Passing '' may be interpreted as an empty token value rather than triggering OIDC fallback. For non-fork PRs, omit the github_token input entirely; for fork PRs, provide ${{ github.token }}.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/claude-code-review.yml at line 61, The current
github_token input is being set to an empty string for non-fork PRs which
prevents OIDC fallback; instead split the invocation into two conditional steps
(or duplicate the step) so one step runs when
github.event.pull_request.head.repo.fork is true and includes github_token: ${{
github.token }}, and the other runs when that condition is false and omits the
github_token input entirely; update the workflow to use these two conditional
steps so the github_token key is never present for non-fork PRs (refer to the
github_token input line and the github.event.pull_request.head.repo.fork
condition in your change).
52db6a2 to
ca1c7bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/claude-code-review.yml (1)
116-120:⚠️ Potential issue | 🟠 MajorUse the correct ref context variable for
issue_commentandpull_request_review_commentevents.On
issue_commentevents,github.event.pull_requestis undefined, so the fallbackgithub.sharesolves to the default branch tip instead of the PR head. This causes Claude to analyze the wrong code tree when responding to@claudecomments on pull requests. Use${{ github.event.pull_request.head.sha || github.event.issue.pull_request.head.sha || github.sha }}to ensure the correct ref is checked out for all event types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/claude-code-review.yml around lines 116 - 120, Update the Checkout step to use the correct ref expression so the PR head is checked out for issue_comment and pull_request_review_comment events: change the ref input to evaluate the PR head SHA from either github.event.pull_request.head.sha or github.event.issue.pull_request.head.sha before falling back to github.sha; keep uses: actions/checkout@v6 and fetch-depth: 0 unchanged and only replace the ref value to `${{ github.event.pull_request.head.sha || github.event.issue.pull_request.head.sha || github.sha }}` so Claude analyzes the correct code tree for those comment events.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/claude-code-review.yml:
- Around line 4-6: Update both jobs named "review" and "respond" to include a
same-repo guard in their if conditions by adding &&
github.event.pull_request.head.repo.full_name == github.repository so they only
run for PRs from the base repo (this prevents checkout of untrusted fork code
via github.event.pull_request.head.sha while secrets like ANTHROPIC_API_KEY or
id-token: write are available); also correct the top comment that incorrectly
states fork PR CI requires maintainer approval for pull_request_target events to
reflect that pull_request_target can run for forks and is not safe without the
same-repo check.
---
Outside diff comments:
In @.github/workflows/claude-code-review.yml:
- Around line 116-120: Update the Checkout step to use the correct ref
expression so the PR head is checked out for issue_comment and
pull_request_review_comment events: change the ref input to evaluate the PR head
SHA from either github.event.pull_request.head.sha or
github.event.issue.pull_request.head.sha before falling back to github.sha; keep
uses: actions/checkout@v6 and fetch-depth: 0 unchanged and only replace the ref
value to `${{ github.event.pull_request.head.sha ||
github.event.issue.pull_request.head.sha || github.sha }}` so Claude analyzes
the correct code tree for those comment events.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 86b5fcd8-4a40-4494-84f7-b21017fe4c90
📒 Files selected for processing (1)
.github/workflows/claude-code-review.yml
| # pull_request_target runs in base repo context — has OIDC + secrets even for forks. | ||
| # Safe because fork CI requires maintainer approval before running. | ||
| pull_request_target: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant sections
fd -t f 'claude-code-review.yml' --exec wc -l {}Repository: lightseekorg/smg
Length of output: 42
🏁 Script executed:
cat -n .github/workflows/claude-code-review.yml | head -150Repository: lightseekorg/smg
Length of output: 6997
🏁 Script executed:
cat -n .github/workflows/labeler.yml | head -30Repository: lightseekorg/smg
Length of output: 690
Add same-repo guard to both jobs to prevent fork PR code from reaching privileged contexts.
The review job (lines 28–30) and respond job (lines 106–109) both lack a check ensuring the PR originates from the base repository. This allows fork PRs to trigger these jobs, checkout fork code via github.event.pull_request.head.sha (lines 42, 120), and feed untrusted content—including REVIEW.md—into Claude while the jobs hold id-token: write and ANTHROPIC_API_KEY secrets on a self-hosted runner.
The pull_request_target trigger does not require fork approval; GitHub's documentation explicitly warns against combining it with checkout of untrusted PR code. Add && github.event.pull_request.head.repo.full_name == github.repository to both job conditions to restrict execution to same-repo PRs only.
🔒 Fixes needed
Review job (lines 28–30):
if: >-
github.event_name == 'pull_request_target'
&& github.repository == 'lightseekorg/smg'
+ && github.event.pull_request.head.repo.full_name == github.repository
&& github.actor != 'dependabot[bot]'Respond job (lines 106–109):
if: >-
(github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment')
&& contains(github.event.comment.body, '@claude')
&& github.repository == 'lightseekorg/smg'
+ && (github.event.pull_request == null || github.event.pull_request.head.repo.full_name == github.repository)Also correct the misleading comment at lines 4–5; fork CI does not require maintainer approval for pull_request_target events.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/claude-code-review.yml around lines 4 - 6, Update both
jobs named "review" and "respond" to include a same-repo guard in their if
conditions by adding && github.event.pull_request.head.repo.full_name ==
github.repository so they only run for PRs from the base repo (this prevents
checkout of untrusted fork code via github.event.pull_request.head.sha while
secrets like ANTHROPIC_API_KEY or id-token: write are available); also correct
the top comment that incorrectly states fork PR CI requires maintainer approval
for pull_request_target events to reflect that pull_request_target can run for
forks and is not safe without the same-repo check.
Fork PRs can't safely use pull_request_target (some workflows run without maintainer approval). Revert to pull_request trigger and skip fork PRs. Fork PRs are still reviewed by CodeRabbit and Gemini. What changed: - .github/workflows/claude-code-review.yml: - Trigger: pull_request (not pull_request_target) - Add !github.event.pull_request.head.repo.fork condition - Remove ref override (not needed for pull_request) Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
ca1c7bd to
95ff5cc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/claude-code-review.yml (1)
102-108:⚠️ Potential issue | 🔴 CriticalCritical: Respond job lacks fork PR protection.
The
respondjob runs on a self-hosted runner with access to secrets but doesn't prevent execution on fork PRs. Whenissue_commentorpull_request_review_commentevents occur on a fork PR, the job will execute with the same security risks as thereviewjob.Add fork protection to the
respondjob condition. For these event types, you'll need to check if there's an associated PR and whether it's from a fork:🔒 Proposed fix to add fork protection
respond: name: Claude Respond if: >- (github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment') && contains(github.event.comment.body, '@claude') && github.repository == 'lightseekorg/smg' + && (github.event.issue.pull_request == null || !github.event.pull_request.head.repo.fork)Note: For
issue_commentevents,github.event.issue.pull_requestis non-null when the comment is on a PR. Forpull_request_review_comment,github.event.pull_requestis always present. This condition allows non-PR comments while blocking fork PR comments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/claude-code-review.yml around lines 102 - 108, The respond job's "if" expression lacks fork-PR protection; update the respond job's if condition (the "respond" job and its if expression) to block execution when the comment is on a PR from a fork by adding checks using github.event.issue.pull_request and github.event.pull_request.head.repo.full_name vs github.repository: for issue_comment allow only when github.event.issue.pull_request is null or the associated PR's head repo equals github.repository, and for pull_request_review_comment require github.event.pull_request.head.repo.full_name == github.repository; modify the existing if to combine the current event checks with these fork-protection checks (referencing github.event_name, github.event.issue.pull_request, github.event.pull_request.head.repo.full_name, and github.repository).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/claude-code-review.yml:
- Line 30: The PR description and workflow implementation diverge: the workflow
uses the boolean check github.event.pull_request.head.repo.fork to block forked
PRs while the description references
github.event.pull_request.head.repo.full_name == 'lightseekorg/smg'; update the
PR description to state that the workflow checks the repo.fork boolean
(github.event.pull_request.head.repo.fork) instead of matching full_name so the
documentation matches the actual implementation.
---
Outside diff comments:
In @.github/workflows/claude-code-review.yml:
- Around line 102-108: The respond job's "if" expression lacks fork-PR
protection; update the respond job's if condition (the "respond" job and its if
expression) to block execution when the comment is on a PR from a fork by adding
checks using github.event.issue.pull_request and
github.event.pull_request.head.repo.full_name vs github.repository: for
issue_comment allow only when github.event.issue.pull_request is null or the
associated PR's head repo equals github.repository, and for
pull_request_review_comment require
github.event.pull_request.head.repo.full_name == github.repository; modify the
existing if to combine the current event checks with these fork-protection
checks (referencing github.event_name, github.event.issue.pull_request,
github.event.pull_request.head.repo.full_name, and github.repository).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88de0c50-072a-43da-9767-29a647aaf735
📒 Files selected for processing (1)
.github/workflows/claude-code-review.yml
| github.event_name == 'pull_request' | ||
| && github.repository == 'lightseekorg/smg' | ||
| && github.actor != 'dependabot[bot]' | ||
| && !github.event.pull_request.head.repo.fork |
There was a problem hiding this comment.
Minor: Implementation differs from PR description.
The PR description mentions using github.event.pull_request.head.repo.full_name == 'lightseekorg/smg', but the implementation uses !github.event.pull_request.head.repo.fork. Both approaches correctly block fork PRs, though the boolean check is more idiomatic. Consider updating the PR description to match the actual implementation for documentation accuracy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/claude-code-review.yml at line 30, The PR description and
workflow implementation diverge: the workflow uses the boolean check
github.event.pull_request.head.repo.fork to block forked PRs while the
description references github.event.pull_request.head.repo.full_name ==
'lightseekorg/smg'; update the PR description to state that the workflow checks
the repo.fork boolean (github.event.pull_request.head.repo.fork) instead of
matching full_name so the documentation matches the actual implementation.
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Reverts the pull_request_target-based CI approval bypass in favor of a simpler label gate on top of regular pull_request triggers. Motivation: - pull_request_target runs in the base-branch context and is widely regarded as dangerous (untrusted fork code + base-branch token). - The previous setup produced "skipped" noise on regular internal PRs because the ci-approved check was evaluated uniformly. - Stability of pull_request_target events has been inconsistent. Changes: - pr-test-rust.yml: drop pull_request_target, add 'labeled' to pull_request types so adding ci-approved re-triggers CI. Rewrite ci-gate as an exclusion gate that skips the pipeline only when the workflow was triggered by adding a non-ci-approved label. First-tier expensive jobs (build-wheel) now `needs` the cheap tier (pre-commit, python-lint, grpc-proto-build-check, unit-tests), so a failing cheap job auto-skips everything downstream for ALL PRs (internal and fork). build-wheel also gates on fork+ci-approved; benchmarks and e2e-vendor inherit the gate through their build-wheel dependency. Cleaned up stale github.event_name != pull_request_target branches in existing if: conditions. - claude-code-review.yml: drop pull_request_target entirely. Claude review does not run on fork PRs (pre-#1009 behavior restored). - labeler.yml: unchanged - labeler legitimately needs write perms on fork PRs (via pull_request_target) to add labels; it only runs actions/labeler with no fork code execution, so the risk is minimal. Required follow-up (repo admin): Settings -> Actions -> General -> Fork pull request workflows from outside collaborators: change from "Require approval for all outside collaborators" to "Require approval for first-time contributors". Without this, returning contributors still need a manual "Approve and run" click on every push - reintroducing the original annoyance. Signed-off-by: Chang Su <chang.s.su@oracle.com>
1 participant
Summary
Fork PRs fail Claude Code review because GitHub blocks OIDC tokens for fork PRs (security restriction). The Claude GitHub App can't authenticate, resulting in
Bad credentialserrors.Fixes: https://github.com/lightseekorg/smg/actions/runs/23842171147/job/69525530493
What changed
Added
github.event.pull_request.head.repo.full_name == 'lightseekorg/smg'condition to the review job. Fork PRs skip the review silently.Test Plan
Summary by CodeRabbit