CI: use pull_request_target so review-checklist runs on fork PRs#6435
Open
brtnfld wants to merge 15 commits into
Open
CI: use pull_request_target so review-checklist runs on fork PRs#6435brtnfld wants to merge 15 commits into
brtnfld wants to merge 15 commits into
Conversation
pull_request events from forks receive a read-only GITHUB_TOKEN and are skipped by the job condition, so no checklist comment is posted and no reviewers are assigned. Switching to pull_request_target fixes this: GitHub always executes the workflow from the base repo (HDFGroup/hdf5) with a full write token, regardless of whether the PR comes from a fork. The fork's code is never checked out or executed. The cross-repo guard (head.repo.full_name == github.repository) is removed since it is no longer needed — pull_request_target only fires for PRs targeting this repo's branches.
brtnfld
requested review from
glennsong09,
hyoklee,
jhendersonHDF and
lrknox
as code owners
…rk PRs pull_request_target carries a GitHub security warning because it grants write access to the privileged workflow at trigger time, which is risky if the checkout is ever changed to use the fork's head ref. Replace it with the recommended two-workflow pattern: - review-checklist-gather.yml fires on pull_request (read-only, safe for forks) and uploads the PR number as a short-lived artifact. - review-checklist.yml fires on workflow_run when the gather job completes, downloads the artifact for the PR number, then posts the checklist comment and assigns reviewers with a full write token. It never executes any code from the fork. The script gains a prNumber parameter so the main workflow can supply the PR number it read from the artifact; falls back to the event payload for pull_request_review triggers.
…un context In the workflow_run path context.payload.pull_request is undefined, so reading .user.login from it throws a TypeError. Use prData.user.login instead — prData is already fetched from the API at step 5. Also remove the stale comment saying fork PRs require pull_request_target; they are now handled via the two-workflow pattern.
The workflow_run approach adds a second hop before the checklist posts and introduced a context.payload.pull_request crash in the gather path. pull_request_target is simpler and equally safe here: GitHub always executes the workflow from HDFGroup/hdf5:develop with a full write token — the fork's code is never checked out or executed.
zizmor is a static analyser that catches security issues specific to GitHub Actions: pull_request_target misuse, expression injection, unpinned action SHAs, and overly broad permissions. Runs on push to develop and on PRs that touch .github/, uploads SARIF to the Security tab (free for public repos via Advanced Security), and annotates PR diffs inline with any findings.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Replace zizmorcore/zizmor-action (third-party) with a direct pip install of zizmor==1.25.2. SARIF output is still uploaded to the Security tab via github/codeql-action/upload-sarif (first-party). The only actions used are from actions/ and github/, both trusted.
jhendersonHDF
previously approved these changes
Jun 8, 2026
…t finding - zizmor.yml: pin zizmor wheel to its SHA256 hash via --require-hashes so the install is fully reproducible and passes zizmor's unpinned-tools audit - review-checklist.yml: add zizmor: ignore[pull-request-target] comment so zizmor does not flag its own host workflow; document why the usage is safe
- actions/checkout: v4.2.2 → v5.0.1 (node20 → node24) - actions/github-script: v7.0.1 → v8.0.0 (node20 → node24) - github/codeql-action: v3.36.2 → v4.36.2 (node20 → node24; v3 deprecated Dec 2026)
jhendersonHDF
previously approved these changes
Jun 8, 2026
GitHub only grants a read-only GITHUB_TOKEN for pull_request_review events when the PR originates from a fork, so the comment post fails with 403. Restrict the review trigger to same-repo PRs; pull_request_target already handles fork PRs for the open/sync/reopen case with a full write token.
pull_request_review grants only a read-only token for fork PRs, causing a 403 when posting the checklist comment. Fix with a two-path design: - pull_request_target handles open/sync/reopen for all PRs (full token, immediate) - A new gather workflow (review-checklist-gather.yml) fires on pull_request_review, saves the PR number as an artifact, and exits. The main workflow then fires via workflow_run with a full write token. The script gains prNumber and isReview parameters. isReview suppresses reviewer assignment (which only applies on open/sync/reopen). prAuthor is sourced from prData.user.login since context.payload.pull_request is not populated in the workflow_run context.
…orkflow pull_request_review triggers a fork workflow approval gate for first-time contributors, which blocks every PR. There is no pull_request_review_target equivalent in GitHub Actions. Instead, rely on pull_request_target (opened/synchronize/reopened) only. Approval boxes are still evaluated correctly: computeApprovals() re-reads all existing reviews on every push, so boxes auto-check the next time the author pushes after a reviewer approves. Also restores the simpler script signature (no prNumber/isReview params) and sources prAuthor from prData.user.login which works in all contexts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
review-checklistworkflow trigger frompull_requesttopull_request_targetso it runs on PRs that originate from forks (e.g. contributor forks targetingHDFGroup/hdf5:develop)head.repo.full_name == github.repository) —pull_request_targetonly fires for PRs targeting this repo's branchesWhy
pull_requestevents from forks receive a read-onlyGITHUB_TOKEN. The job condition evaluated tofalsefor all fork PRs, so no checklist comment was posted and no reviewers were assigned. Reported against #6434.