Repository navigation
Add manual fork-PR CI dispatch to ci.yml - #405
Merged
Merged
Conversation
Fork PRs don't get secrets (e.g. PUPPET_FORGE_TOKEN, CODECOV_TOKEN) via the normal pull_request trigger, so CI silently skips secret-dependent steps for external contributors. Add pr_number/ref inputs to the existing workflow_dispatch trigger and a resolve-ref job that resolves either into a ref, threaded into both the spec job (gem_ci.yml@main, which gained a ref input in puppetlabs/cat-github-actions#191) and the acceptance job (the repo-local custom_acceptance.yml, which needed the same ref input added since it's not one of the shared cat-github-actions workflows). workflow_dispatch is already gated to users with repo write access, which is the trust boundary — no label/approval system is layered on top, intentionally lightweight for this low-traffic tool repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refs/pull/<n>/merge is mutable: GitHub repoints it on every new push to the PR, and spec/acceptance check it out independently, with acceptance only starting after spec finishes. A fork author could push a malicious commit in that window and have acceptance silently execute it with PUPPET_FORGE_TOKEN in scope, instead of the commit a maintainer actually reviewed before dispatching. Resolve pr_number/ref to a concrete, immutable commit SHA once via the GitHub API instead — specifically the PR's merge commit (matching the module-track fork-PR precedent and the semantics of the normal pull_request trigger, which also tests the merge result rather than the raw head), failing explicitly if the PR isn't mergeable yet or has conflicts rather than silently testing the wrong commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #405 +/- ##
=======================================
Coverage 88.98% 88.98%
=======================================
Files 6 6
Lines 853 853
=======================================
Hits 759 759
Misses 94 94 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Summary
Replaces #402 with the same end result and a clean 2-commit history (identical final diff, verified — #402 accumulated some since-corrected mistakes across iterative review rounds that don't need to stay in permanent history). The Puppet-8-nightly-CI fixes that were bundled into #402 are split out to #404, since they're an unrelated, pre-existing problem.
PUPPET_FORGE_TOKEN,CODECOV_TOKEN) via the normalpull_requesttrigger, so CI silently skips secret-dependent steps for external contributors.pr_number/refinputs to the existingworkflow_dispatchtrigger, plus aresolve-refjob that resolves either into an immutable commit SHA (or empty string, meaning "no override, behave exactly as before").pr_number, resolves the PR's merge commit SHA (not the mutablerefs/pull/<n>/mergeref, and not the raw head SHA) via the GitHub API, failing explicitly if the PR isn't mergeable yet or has conflicts — this matches the normalpull_requesttrigger's test semantics while staying immutable across job boundaries, sincespecandacceptancecheck out independently andacceptanceonly starts oncespecfinishes (a mutable ref could let a fork author push a new, unreviewed commit in that window and have it silently execute with secrets in scope).specjob (gem_ci.yml@main, which gained arefinput in Add optional ref input to gem_ci and gem_acceptance cat-github-actions#191) and theacceptancejob (the repo-localcustom_acceptance.yml, which needed the samerefinput added since it isn't one of the sharedcat-github-actionsworkflows).workflow_dispatchis already gated to users with repo write access — that's the trust boundary. No label/approval system layered on top, intentionally lightweight for this low-traffic tool repo.Test plan
cimanually withrefset to a branch/SHA and confirmspec/acceptancecheck out that refcimanually withpr_numberset to a fork PR and confirm both jobs run against the resolved merge commit SHA with secrets availablepush/pull_requestruns onmainare unaffected (ref input is empty, behaving exactly as before)🤖 Generated with Claude Code