Skip to content

fix(ci-conformance): pin the script to the caller's pin, keep policy live - #26

Open
krisarmstrong wants to merge 1 commit into
mainfrom
ci/pin-conformance-script
Open

fix(ci-conformance): pin the script to the caller's pin, keep policy live#26
krisarmstrong wants to merge 1 commit into
mainfrom
ci/pin-conformance-script

Conversation

@krisarmstrong

Copy link
Copy Markdown
Contributor

Summary

Implements option 3 from #22split the pin by what the file is.

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. One commit here changed what ran in all four
repos on their next CI run, with no per-repo review and no tag bump.

file is now
scripts/check-ci-conformance.py code — a bug reds the whole fleet, on unrelated PRs pinned to the caller's own pin
policy/*.txt data — instant fleet rollout is the reason it is shared deliberately live, via an explicit second checkout

The 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 with
no manual bump, avoiding the chicken-and-egg of a self-referencing tag.

⚠️ Inert until a repo bumps its pin. Consumers stay on v1.3.0's behaviour
until they adopt a tag containing this, so merging cannot red the fleet.

Linked Issue

Closes #22

Testing Evidence

job.workflow_sha was verified empirically, not taken from docs, because
the failure mode is silent: an empty ref: is not an error to
actions/checkout — it resolves to the default branch, i.e. exactly the
behaviour being removed. A throwaway reusable-workflow probe was pushed to this
branch, run, and then reverted (it is not in the diff):

=== toJSON(job) ===
{
  "check_run_id": 95863145317,
  "workflow_ref": "MustardSeedNetworks/.github/.github/workflows/zz-probe-reusable.yml@refs/heads/ci/pin-conformance-script",
  "workflow_sha": "b257660062b860e08681a8a3aaf4e9d0b6fabfc4",
  "workflow_repository": "MustardSeedNetworks/.github",
  "workflow_file_path": ".github/workflows/zz-probe-reusable.yml",
  "status": "success"
}
=== github.workflow_ref ===
MustardSeedNetworks/.github/.github/workflows/zz-probe-caller.yml@refs/heads/ci/pin-conformance-script

b257660… was the reusable workflow's own commit. Note job.workflow_ref
points at the reusable file while github.workflow_ref points at the
caller — 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):

### 1. default (policy beside the script) — unchanged behaviour:
  exit 0 PASS
### 2. explicit override at the real policy dir — must behave identically:
  exit 0 PASS
### 3. override pointing at a missing dir — must FAIL LOUDLY, not skip:
::error::CI_CONFORMANCE_POLICY_DIR='/nonexistent/policy' is not a directory; refusing to fall back and silently skip the policy checks
missing-dir exit code: 1

Case 3 is the one that matters: main() skips every policy check when the
directory 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 RollupState tampered makes the gate fire against real seed source:

$ CI_CONFORMANCE_POLICY_DIR=/tmp/badpolicy python3 .../check-ci-conformance.py
::error::ui/src/ui/StatusRollup.tsx:28: `RollupState` has drifted from the fleet vocabulary
    found:    export type RollupState = 'ok' | 'warn' | 'crit' | 'unknown';
    expected: export type RollupState = 'ok' | 'warn' | 'crit';

ci-conformance: 1 finding(s)
exit=1

Workflow gates (yaml.safe_load accepts duplicate keys, actionlint does not,
so both are run):

$ python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci-conformance.yml'))"
safe_load OK
$ actionlint -ignore 'SC2129' .github/workflows/ci-conformance.yml
actionlint OK
$ zizmor --min-severity high .github/workflows/ci-conformance.yml
No findings to report. Good job!

Security and Release Checklist

  • No new authority. Both checkouts keep persist-credentials: false;
    job permissions: unchanged (contents: read).
  • Strictly reduces what can change under a pin — the script can no
    longer drift from the reviewed commit. Policy retains today's behaviour.
  • No fail-open introduced. The new override raises rather than falling
    back; proven above. The pre-existing if policy.exists() skip is
    unchanged for the default path.
  • Merging cannot red the fleet — consumers pin by SHA and are unaffected
    until they bump.
  • Action pins unchanged (actions/checkout@3d3c42e5…); no new actions.

⚠️ One judgement call to confirm, flagged rather than buried: this adds
.github/actionlint.yaml ignoring one message. actionlint v1.7.12 models the
job context as only {check_run_id, container, services, status} and so
reports job.workflow_sha as undefined. The probe above shows the property is
real 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.

…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci-conformance is SHA-pinned but executes policy and script from main

1 participant