ci: stop fork pull requests running on the self-hosted runners - #522
Merged
Conversation
macos-check.yml and windows-check.yml trigger on pull_request and run on
[self-hosted, macOS, ARM64] and [self-hosted, windows, x64] with no fork
guard, on a public repository.
cargo fmt/build/clippy all execute code the pull request supplies: build.rs,
proc-macro crates, a swapped Cargo.toml dependency or [patch], and the test
bodies themselves. permissions: {} limits what the token can reach; it does
nothing about code execution on the machine.
That machine is the same one that builds, signs and uploads every macOS DMG
and runtime pack. Neither check workflow cleans its workspace and the release
workflows only remove .build and dist, so an implant in ~/.cargo, ~/.npm,
~/.rustup or the persistent _work tree would survive straight into the next
release. GitHub's public-repo default only gates first-time contributors, so
one trivial merged pull request is enough to unlock this for a later one.
Gate both jobs on the pull request coming from this repository. The
workflow_dispatch path is unaffected, so a fork's Rust change can still get
its compiler pass when a maintainer asks for one -- which keeps the reason
the trigger exists (#421: cfg-gated Rust merging without ever being compiled,
because ci.yml is entirely ubuntu-latest).
Refs #511
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
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.
Fixes #511.
macos-check.ymlandwindows-check.ymltrigger onpull_requestand run on[self-hosted, macOS, ARM64]/[self-hosted, windows, x64]with no fork guard, on a public repository.cargo fmt,cargo buildandcargo clippyall execute code the PR supplies:build.rs, proc-macro crates, a swappedCargo.tomldependency or[patch], and the test bodies.permissions: {}limits the token, not code execution.That runner is the same machine that builds, signs and uploads every macOS DMG and runtime pack. Neither check workflow cleans its workspace, and the release workflows only
rm -rf .build dist-- so an implant in~/.cargo,~/.npm,~/.rustupor the persistent_worktree survives into the next release. GitHub's public-repo default only gates first-time contributors, so one trivial merged PR unlocks this for a later one.Change
Both jobs gain:
Why not move to GitHub-hosted runners
That would lose the reason the trigger exists.
ci.ymlis entirelyubuntu-latest, and a Linux runner cannot type-check code behind#[cfg(target_os = "macos")]or#[cfg(windows)]at all -- it is stripped before semantic analysis. #421 added ~600 lines of mostly cfg-gated Rust and every CI check passed without compiling any of it.workflow_dispatchis unaffected, so a fork's Rust change can still get a real compiler pass when a maintainer triggers one.Verification
Both files parse and the guard resolves as intended:
Behaviour:
workflow_dispatchruns (first clause), same-repo PRs run (second clause), fork PRs are skipped.Worth pairing later with a workspace clean at the start of the release workflows, so a compromised check run cannot persist into a build. Not done here to keep this change minimal.