block checking out fork pr for pull_request_target and workflow_run#2454
block checking out fork pr for pull_request_target and workflow_run#2454aiqiaoy wants to merge 3 commits into
Conversation
| // Determine the GitHub URL that the repository is being hosted from | ||
| result.githubServerUrl = core.getInput('github-server-url'); | ||
| core.debug(`GitHub Host URL = ${result.githubServerUrl}`); | ||
| // Allow unsafe PR checkout (opt-in for pull_request_target / workflow_run fork PRs) |
There was a problem hiding this comment.
Can this apply to merge/head ref + issue_comment constructions by default as well?
There was a problem hiding this comment.
Thanks for the review, The risk of issue_comment is on our radar, however it will not be included in this change. This is because issue_comment event payload only has the PR number (no ref or sha), so we can't detect fork PR without an extra API call and issue_comment has a slightly different threat model (gated by who can comment rather than who can open a PR) that deserves its own design.
There was a problem hiding this comment.
Pull request overview
Adds a guardrail to prevent accidentally checking out fork pull request code in privileged GitHub Actions contexts (pull_request_target and PR-triggered workflow_run), with an explicit opt-in escape hatch for advanced workflows.
Changes:
- Introduces
assertSafePrCheckoutto detect and block unsafe fork-PR checkouts in privileged events unlessallow-unsafe-pr-checkoutis enabled. - Adds a new action input (
allow-unsafe-pr-checkout) and threads it through input parsing and settings. - Updates documentation and adds test coverage for the new safety checks.
Show a summary per file
| File | Description |
|---|---|
| src/unsafe-pr-checkout-helper.ts | New helper that blocks unsafe fork PR checkout patterns in privileged events unless opted in. |
| src/ref-helper.ts | Exports fromPayload for reuse by the new safety helper. |
| src/input-helper.ts | Parses allow-unsafe-pr-checkout and invokes the new safety assertion during input handling. |
| src/git-source-settings.ts | Extends settings interface with allowUnsafePrCheckout. |
| README.md | Documents the new input and its intent. |
| action.yml | Adds the new input to the action metadata. |
| test/unsafe-pr-checkout-helper.test.ts | New unit tests validating allow/refuse behavior across events and patterns. |
| test/input-helper.test.ts | Asserts the new setting defaults to false. |
| test/git-auth-helper.test.ts | Updates test settings object to include the new required field. |
| dist/index.js | Bundled output updated to include the new logic. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 9/10 changed files
- Comments generated: 4
| # Required to check out fork pull request code from a workflow triggered by | ||
| # `pull_request_target` or `workflow_run`. See [Pwn Requests](todo:need-link) for | ||
| # the risks. Set to `true` only after reviewing the risks. |
| allow-unsafe-pr-checkout: | ||
| description: > | ||
| Required to check out fork pull request code from a workflow triggered by | ||
| `pull_request_target` or `workflow_run`. See [Pwn Requests](todo:need-link) | ||
| for the risks. Set to `true` only after reviewing the risks. | ||
| default: false |
Implements https://github.com/github/c2c-actions/pull/10159
TL;DR; This PR adds a check that refuses to check out fork pull request code when the workflow trigger is either
pull_request_targetorworkflow_run, unless the workflow author explicitly opts in via a new inputallow-unsafe-pr-checkout: true.