Merge rust-bitcoin/rust-bitcoin#6757: ci: gen PR labels from master instead of fork
What the developer wrote
Authored by Andrew Poelstra
Merge rust-bitcoin/rust-bitcoin#6757: ci: gen PR labels from master instead of fork
000113045370d6dac13f8a91101bc31134278315 ci: gen PR labels from master instead of fork (satsfy (Renato Britto))
Pull request description:
This fixes a CI issue and doubles as a security fix.
The labeler job now fails on every fork PR. This is because, [yesterday](https://github.com/rust-bitcoin/rust-bitcoin/commit/2d138d732b791f7d1a7ef715794f5c2f4fad3b62), CI's `actions/checkout` was updated to v7 and refuses to fetch fork PR code from a `pull_request_target` workflow unless the step opts in with `allow-unsafe-pr-checkout`. IIUC, that was a safety issue because the old job checked out the fork and ran `cargo metadata` on its manifests while holding the base repo's token (known as [pwn-request vulnerability](https://www.endorlabs.com/learn/pwn-request-threat-a-hidden-danger-in-github-actions)).
But from my understanding of this workflow, the fork checkout is not necessary. So I dropped it and, making the problem go away.
One consequence is: if a PR adds a new crate, the PR will not get label `C-<crate>`. Seems acceptable to me.
Btw I see 0.32.xx and 0.32.xxx have this same issue.
ACKs for top commit:
tcharding:
utACK 000113045370d6dac13f8a91101bc31134278315
jamillambert:
ACK 000113045370d6dac13f8a91101bc31134278315
apoelstra:
ACK 000113045370d6dac13f8a91101bc31134278315; successfully ran local tests
Tree-SHA512: 80e918fbf2a595f4724459b64e1d2facf4fc88917dd4681e3e7b9dc93987156cabe3d5ec51b62bad1dd3e638e5f41ccb83b79a1a3d9cd52157c1f8ec3e06200e
What changed, and why it matters
This change fixes a GitHub Actions workflow that was checking out code from pull request forks and running a Rust metadata tool on it while holding a write-capable repository token. That combination is a known attack path (nicknamed 'pwn-request') where a malicious pull request could potentially steal or abuse the repository token. The patch removes the fork checkout entirely and instead generates labels from the repository's own master branch, closing the exposure. It is a defensive hardening fix rather than a fix for an already-exploited bug.
Verify that the labeler still functions for in-repo PRs and that no other workflows in this repository perform privileged fork checkouts. Consider applying the same pattern to the 0.32.x maintenance branches mentioned by the contributor. No immediate code-level mitigation is needed by downstream users.
Security signals we found
Removal of pull-request merge commit checkout in a pull_request_target workflow
Elimination of executing cargo metadata on fork-contributed manifests with base-repo token
Workflow now operates only on trusted master branch code
PR description explicitly frames change as a security fix for pwn-request exposure
Evidence from the diff
The manage-pr.yml workflow previously performed two checkouts: one of master into ./master and one of the PR merge commit into ./merge, then ran cargo metadata via gen_label_config.sh against the merge directory. Because the workflow triggers on pull_request_target, it runs in the base repository context with access to the base repo’s GITHUB_TOKEN. Checking out and executing code from a fork PR under those conditions is the classic pwn-request pattern. The patch removes the merge checkout and the SCAN_DIR indirection, running the label generator directly against the checked-out master tree. This eliminates execution of fork-supplied code in a privileged workflow, at the cost of not auto-labeling newly added crates in a PR.
Changed components
.github/workflows/manage-pr.ymlGitHub Actions labeler jobInspect captured patch +2 / −9
### .github/workflows/manage-pr.yml
@@ -13,18 +13,11 @@ jobs:
- name: Checkout master
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
- path: master
- persist-credentials: false
- - name: Checkout merge commit
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
- with:
- path: merge
- ref: "refs/pull/${{ github.event.number }}/merge"
persist-credentials: false
- name: Generate label config
- run: cd master && SCAN_DIR=../merge ./.github/gen_label_config.sh
+ run: ./.github/gen_label_config.sh
- name: Update labels
uses: actions/labeler@f27b608878404679385c85cfa523b85ccb86e213 # v6.1.0
with:
- configuration-path: master/.github/labeler.yml
+ configuration-path: .github/labeler.yml
sync-labels: trueWhy this scored 60/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.