fix: support pull_request_target case #12
Conversation
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![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/trunk-upgrade.yaml (1)
22-29: Pin the custom action to a commit SHA.Using a branch ref (
@fix/admin-permissions) risks supply-chain drift. Pin to a vetted commit SHA and optionally add a comment linking to the tag it corresponds to.- - name: Run Trunk Upgrade - uses: masterpointio/github-action-trunk-upgrade@fix/admin-permissions + - name: Run Trunk Upgrade + # Pin to an immutable commit SHA to prevent unexpected changes + uses: masterpointio/github-action-trunk-upgrade@<COMMIT_SHA> with: app-id: ${{ secrets.MP_BOT_APP_ID }} app-private-key: ${{ secrets.MP_BOT_APP_PRIVATE_KEY }} github-token: ${{ secrets.MASTERPOINT_TEAM_PAT }} reviewers: "@masterpointio/masterpoint-open-source"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/trunk-upgrade.yaml(1 hunks)action.yaml(1 hunks)
🔇 Additional comments (1)
.github/workflows/trunk-upgrade.yaml (1)
22-29: Validate inputs and token scopes.Confirm the action’s input names (
app-id,app-private-key) match its current interface and thatMASTERPOINT_TEAM_PAThas the minimal scopes required (e.g., contents:write and pull_request permissions for private repos; public_repo for public).
action.yaml
Outdated
| # For pull_request_target: checkout the actual PR code, not the base branch | ||
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | ||
| # Support forks by using the head repository when available | ||
| repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid running untrusted PR code with privileged credentials in pull_request_target.
Checking out fork code in pull_request_target can execute untrusted code with base-repo permissions/secrets. Prefer the merge ref from the base repo and drop persisted credentials to reduce blast radius.
Apply this safer checkout:
with:
token: ${{ inputs.github_token }}
- # For pull_request_target: checkout the actual PR code, not the base branch
- ref: ${{ github.event.pull_request.head.sha || github.sha }}
- # Support forks by using the head repository when available
- repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
+ # In pull_request_target, use the base repo's synthetic merge ref to avoid cross-repo clones
+ # and to prevent running fork code with elevated credentials. Otherwise, fall back to sha.
+ ref: ${{ (github.event_name == 'pull_request_target' && format('refs/pull/{0}/merge', github.event.number)) || github.event.pull_request.head.sha || github.sha }}
+ # Always clone from the base repository to avoid permission issues with forks
+ repository: ${{ github.repository }}
+ # Do not persist credentials into the repo's local config (reduces risk of token exfiltration)
+ persist-credentials: false
+ # Faster, smaller checkout
+ fetch-depth: 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # For pull_request_target: checkout the actual PR code, not the base branch | |
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | |
| # Support forks by using the head repository when available | |
| repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} | |
| with: | |
| token: ${{ inputs.github_token }} | |
| # In pull_request_target, use the base repo's synthetic merge ref to avoid cross-repo clones | |
| # and to prevent running fork code with elevated credentials. Otherwise, fall back to sha. | |
| ref: ${{ (github.event_name == 'pull_request_target' && format('refs/pull/{0}/merge', github.event.number)) || github.event.pull_request.head.sha || github.sha }} | |
| # Always clone from the base repository to avoid permission issues with forks | |
| repository: ${{ github.repository }} | |
| # Do not persist credentials into the repo's local config (reduces risk of token exfiltration) | |
| persist-credentials: false | |
| # Faster, smaller checkout | |
| fetch-depth: 1 |
🤖 Prompt for AI Agents
In action.yaml around lines 36 to 39, the checkout currently uses the PR head
ref and head repository which runs untrusted fork code in pull_request_target;
change the checkout to use the merge ref from the base repo (e.g.
github.event.pull_request.merge_commit_sha || github.sha) and set repository to
the base repo (github.repository) instead of the head repo, and disable
persisting credentials (persist-credentials: false) so actions do not expose
base-repo secrets to checked-out PR code.
💡 Verification agent
❓ Verification inconclusive
Cross-repo checkout will often fail with GITHUB_TOKEN.
The default GITHUB_TOKEN typically lacks access to fork repos; using head.repo.full_name will 404 on public forks and fail on private forks. The merge-ref approach above also resolves this reliability issue.
Restore merge-ref checkout for pull_request_target
The actions/checkout step using repository: ${{ github.event.pull_request.head.repo.full_name }} will 404 with GITHUB_TOKEN on forked repos; revert to the merge-ref approach (ref: ${{ github.event.pull_request.merge_commit_sha }}) for reliable cross-repo checkout (jvt.me).
🤖 Prompt for AI Agents
In action.yaml around lines 36 to 39, the checkout step currently tries to use
the PR head repo and head SHA which causes 404s with GITHUB_TOKEN on forked
repos; replace the ref with the merge commit ref and avoid using the forked repo
for pull_request_target. Set ref to ${{
github.event.pull_request.merge_commit_sha }} and use the target repository
(e.g. ${{ github.repository }}) so the action checks out the merge-ref in the
repository where the workflow runs, ensuring reliable cross-repo checkout with
GITHUB_TOKEN.
🤖 I have created a release *beep* *boop* --- ## [1.0.1](v1.0.0...v1.0.1) (2025-09-03) ### Bug Fixes * support pull_request_target case ([#12](#12)) ([ee89cc8](ee89cc8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
what
pull_request_targettrigger (needed for accessing secrets), the default checkout behavior checks out the base branch (main) instead of the PR branch, causing tests to run against the wrong code. Updated the checkout step to explicitly check out the PR code when running inpull_request_target context, with proper fallbacks for other trigger types.why
references
Summary by CodeRabbit
Bug Fixes
Chores