fix(ci-conformance): pin the script to the caller's pin, keep policy live - #26
Open
krisarmstrong wants to merge 1 commit into
Open
fix(ci-conformance): pin the script to the caller's pin, keep policy live#26krisarmstrong wants to merge 1 commit into
krisarmstrong wants to merge 1 commit into
Conversation
…live Callers write `ci-conformance.yml@<sha> # vX.Y.Z`, but the job checked out this repo with no `ref:` and ran the script from that checkout. So the workflow DEFINITION was pinned while the script and policy that actually executed were live from the default branch. The pin implied an immutability it did not provide: one commit here changed what ran in all four repos on their next CI run, with no per-repo review and no tag bump. #22 offered three options. This takes the third — split them by what they are: The SCRIPT is code. A bug in it reds the whole fleet, including on unrelated PRs, so it now resolves to `job.workflow_sha` — the very commit the caller pinned the workflow to. A repo's `@<sha>` now covers the script too, and no manual bump is needed to keep the two in step. The POLICY is data. Rolling a policy change out to four repos at once is the reason a shared policy exists; requiring four PRs and a tag would defeat it. It stays live from the default branch, now via an explicit second checkout rather than as an unexamined side effect of the first. `job.workflow_sha` is the reusable workflow's own commit, not the caller's (`github.sha`). It was verified empirically before being relied on, because the failure mode is silent: an empty `ref:` is not an error to actions/checkout, it resolves to the default branch — precisely the behaviour being removed. A guard step now fails loudly if it is ever empty. The script gains CI_CONFORMANCE_POLICY_DIR to read policy from elsewhere. It raises when the override does not resolve, because main() skips every policy check when the directory is absent: a typo'd path would otherwise turn the linter-floor and status-vocabulary gates into silent no-ops while still reporting success. Also adds .github/actionlint.yaml. actionlint v1.7.12 still models the `job` context as only {check_run_id, container, services, status} and flags `job.workflow_sha` as undefined. The probe output proving otherwise is recorded in that file so the ignore can be removed when the linter catches up. Inert until a repo bumps its pin: consumers stay on v1.3.0's behaviour until they adopt a tag containing this. Closes #22
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
Implements option 3 from #22 — split the pin by what the file is.
Callers write
ci-conformance.yml@<sha> # vX.Y.Z, but the job checked out thisrepo with no
ref:and ran the script from that checkout. So the workflowdefinition was pinned while the script and policy that actually executed
were live from the default branch. One commit here changed what ran in all four
repos on their next CI run, with no per-repo review and no tag bump.
scripts/check-ci-conformance.pypolicy/*.txtThe script resolves to
job.workflow_sha— the reusable workflow's own commit.So a repo's
@<sha>now covers the script too, and the two stay in step withno manual bump, avoiding the chicken-and-egg of a self-referencing tag.
until they adopt a tag containing this, so merging cannot red the fleet.
Linked Issue
Closes #22
Testing Evidence
job.workflow_shawas verified empirically, not taken from docs, becausethe failure mode is silent: an empty
ref:is not an error toactions/checkout— it resolves to the default branch, i.e. exactly thebehaviour being removed. A throwaway reusable-workflow probe was pushed to this
branch, run, and then reverted (it is not in the diff):
b257660…was the reusable workflow's own commit. Notejob.workflow_refpoints at the reusable file while
github.workflow_refpoints at thecaller — which is the distinction this change depends on. A guard step
fails loudly if the value is ever empty, so the silent fallback cannot return.
The script's new override, proven all three ways (run against a real seed
checkout):
Case 3 is the one that matters:
main()skips every policy check when thedirectory is absent, so a typo'd path would otherwise turn the linter-floor and
status-vocabulary gates into silent no-ops while still reporting success.
And proof the overridden directory is genuinely the one consulted — a policy
copy with
RollupStatetampered makes the gate fire against real seed source:Workflow gates (
yaml.safe_loadaccepts duplicate keys,actionlintdoes not,so both are run):
Security and Release Checklist
persist-credentials: false;job
permissions:unchanged (contents: read).longer drift from the reviewed commit. Policy retains today's behaviour.
back; proven above. The pre-existing
if policy.exists()skip isunchanged for the default path.
until they bump.
actions/checkout@3d3c42e5…); no new actions..github/actionlint.yamlignoring one message. actionlint v1.7.12 models thejobcontext as only{check_run_id, container, services, status}and soreports
job.workflow_shaas undefined. The probe above shows the property isreal at runtime, so this suppresses a stale linter schema, not a finding —
the evidence is recorded in the file itself so the ignore can be dropped when
actionlint catches up. Say the word if you would rather restructure to avoid the
expression instead.
Not merged or tagged by me — #22 framed this as a fleet-governance call, so
it wants your sign-off. It needs a tag (v1.5.0) before any repo can adopt it.