Repository navigation
Conversation
Fork PRs can't run CI jobs that need secrets via the normal pull_request trigger. Add workflow_dispatch inputs and a resolve-ref job that resolve a PR number or ref, threaded into the existing CI jobs via cat-github-actions' gem_ci.yml/gem_acceptance.yml ref input (puppetlabs/cat-github-actions#191). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
acceptance called the repo-local custom_acceptance.yml, which had no ref input, so a manual dispatch with pr_number/ref would silently run acceptance against the wrong code while spec correctly tested the resolved ref. Add the same optional ref input/checkout override used in cat-github-actions' gem_ci.yml/gem_acceptance.yml, and thread it through from ci.yml. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Mutable refs can cause secret-bearing jobs to execute different, potentially unreviewed revisions.
1 open issue
What changed in this PR
Adds manual CI dispatch for fork PRs or explicit refs, allowing secret-dependent tests to run against the selected code.
Changes:
- Adds optional
pr_numberandrefdispatch inputs and a shared ref resolver. - Passes the resolved ref to both spec and acceptance jobs.
- Adds an optional checkout override to the local PowerShell acceptance workflow.
| File | Description |
|---|---|
.github/workflows/custom_acceptance.yml |
Accepts an optional ref and applies it during checkout. |
.github/workflows/ci.yml |
Resolves manual dispatch inputs and passes the target to both test workflows. |
Review effort: Balanced
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 SHA once via the GitHub API (PR head SHA, matching the module-track fork-PR precedent) and thread that everywhere instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pinning to .head.sha fixed the mutable-ref race but silently changed what gets tested: the normal pull_request trigger (and this PR's own test plan) tests the merge result via refs/pull/<n>/merge, not the contributor's raw tip. A manual dispatch could pass while the PR actually conflicts with the target branch. Resolve merge_commit_sha instead (still immutable once captured), and fail 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>
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
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 a ref (or empty string, meaning "no override, behave exactly as before").ref: ${{ needs.resolve-ref.outputs.ref }}into thespecjob, which callspuppetlabs/cat-github-actions/.github/workflows/gem_ci.yml@main(already using@main, which has therefinput added inpuppetlabs/cat-github-actions#191— no pin bump needed).acceptancecalls a repo-local reusable workflow (./.github/workflows/custom_acceptance.yml, kept around just to force acceptance tests underpowershell.exe), notcat-github-actions'gem_acceptance.yml. It initially had no way to accept a ref override, which meant a manual dispatch withpr_number/refwould runspecagainst the right code but silently runacceptanceagainst whatever ref triggered the dispatch instead — worse than not running, since it'd look like the PR's acceptance tests passed without ever testing its code. Added the same optionalrefinput/checkout override tocustom_acceptance.ymland threaded it through fromci.yml, so both jobs now honor the resolved ref.Test plan
cimanually withrefset to a branch/SHA and confirm bothspecandacceptancecheck out that refcimanually withpr_numberset to a fork PR and confirm both jobs run againstrefs/pull/<n>/mergewith secrets availablepush/pull_requestruns onmainare unaffected (ref input is empty, behaving exactly as before)🤖 Generated with Claude Code