diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c610e49..ab3314da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1590,3 +1590,61 @@ jobs: exit 1 fi echo "attack 2 refused: the unallowed-extension target never ran" + + # A fan-in so branch protection can require ONE stable context instead of + # eleven, several of which are matrix-interpolated. + # + # `check (22.23.2)` carries the exact pinned floor, and the comment on that + # matrix says why: a declared floor must name the exact release CI executes. + # Raising the floor therefore RENAMES the required context. With admins + # exempt today that is survivable; the moment `enforce_admins` is true, a + # renamed context means no commit can satisfy protection and `main` freezes + # with no exit except editing the settings by hand. + # + # `if: always()` is load-bearing. Without it a failed dependency SKIPS this + # job rather than failing it, and a skipped required check is not a failure + # to GitHub -- the gate would wave through exactly the runs it exists to + # stop. Each result is compared to `success` rather than to `failure` for the + # same reason: `skipped` and `cancelled` are not success. + # + # This covers `ci.yml` only. `lint` lives in `demo-lint.yml` and cannot be a + # `needs:` from here, so protection requires two contexts: `gate` and `lint`. + # Neither is matrix-interpolated, so neither can be renamed by a version bump. + gate: + name: gate + if: always() + needs: + - check + - audit + - git-matrix + - install-macos + - install-alpine + - install-script + - install-ps1 + runs-on: ubuntu-latest + steps: + - name: every job this gate fans in from succeeded + env: + RESULTS: >- + check=${{ needs.check.result }} + audit=${{ needs.audit.result }} + git-matrix=${{ needs.git-matrix.result }} + install-macos=${{ needs.install-macos.result }} + install-alpine=${{ needs.install-alpine.result }} + install-script=${{ needs.install-script.result }} + install-ps1=${{ needs.install-ps1.result }} + run: | + set -eu + failed=0 + for pair in $RESULTS; do + job=${pair%%=*} + result=${pair#*=} + if [ "$result" != "success" ]; then + echo "::error::$job did not succeed (result: $result)" + failed=1 + else + echo "$job: success" + fi + done + [ "$failed" -eq 0 ] || exit 1 + echo "all fanned-in jobs succeeded" diff --git a/scripts/check-exact-head-ci.mjs b/scripts/check-exact-head-ci.mjs index c99ac4fd..52c1953c 100644 --- a/scripts/check-exact-head-ci.mjs +++ b/scripts/check-exact-head-ci.mjs @@ -40,7 +40,7 @@ const CI_WORKFLOW_FILE_PATH = fileURLToPath(new URL(`../${CI_WORKFLOW_PATH}`, im // shell command; without this lock replacing every job body with `true` would // still look like a real successful run. Update deliberately with the CI // workflow when its reviewed job contract changes. -export const EXPECTED_CI_WORKFLOW_SHA256 = '473b1fc06750384875d7236986b34b9564b3ff250c3f6da8b4572be50d75c48a'; +export const EXPECTED_CI_WORKFLOW_SHA256 = '93d93eed99727fd422c4f462da93b9eab42b55aeab063675a59dbda4818a28b7'; // Fixed rather than inferred from returned jobs: absence must fail rather // than define itself away. `lint` only runs for pull requests and is therefore @@ -59,6 +59,12 @@ export const EXPECTED_CI_WORKFLOW_SHA256 = '473b1fc06750384875d7236986b34b9564b3 // linted, and saying only the first invites someone to add `lint` to this // list, which would block every release. export const REQUIRED_CHECKS = Object.freeze([ + // `gate` fans in from the ten below and fails unless every one succeeded. It + // is listed here as well rather than instead: this gate reads the API's job + // list, so an entry it does not know about is reported as an unexpected job, + // and a `gate` that is not named here would fail every release the moment it + // started running. + 'gate', 'check (22.23.2)', 'check (24)', 'audit', diff --git a/test/ci-gate.test.ts b/test/ci-gate.test.ts new file mode 100644 index 00000000..341949b4 --- /dev/null +++ b/test/ci-gate.test.ts @@ -0,0 +1,82 @@ +/** + * The fan-in gate exists so branch protection can require one stable context + * instead of eleven, several of which are matrix-interpolated: `check (22.23.2)` + * carries the exact pinned floor, so raising that floor RENAMES the required + * context. While admins are exempt a rename is survivable; once they are not, a + * renamed context means no commit can satisfy protection and `main` freezes with + * no exit except editing the settings by hand. + * + * That only holds while the gate fans in from EVERY job. A job added to `ci.yml` + * and not added to `needs:` is silently outside protection — the same shape as + * the failure `bench/verify.mjs` was rewritten to prevent, where a file left off + * a declared list was silently ungated. The list is default-in there; here it + * cannot be, so it is asserted instead. + */ + +import { readFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { load } from 'js-yaml'; +import { describe, expect, it } from 'vitest'; + +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); + +interface Workflow { + jobs: Record; +} + +const workflow = (file: string): Workflow => + load(readFileSync(join(REPO_ROOT, '.github/workflows', file), 'utf8')) as Workflow; + +describe('the CI fan-in gate', () => { + const ci = workflow('ci.yml'); + + it('exists and is named for use as a required context', () => { + expect(ci.jobs.gate, 'ci.yml has no gate job').toBeDefined(); + expect(ci.jobs.gate?.name).toBe('gate'); + }); + + it('fans in from every other job in the workflow', () => { + const others = Object.keys(ci.jobs).filter((id) => id !== 'gate'); + const needs = ci.jobs.gate?.needs ?? []; + const uncovered = others.filter((id) => !needs.includes(id)); + expect(uncovered, `these jobs are outside the gate: ${uncovered.join(', ')}`).toHaveLength(0); + }); + + it('names no job that does not exist', () => { + const needs = ci.jobs.gate?.needs ?? []; + const dangling = needs.filter((id) => ci.jobs[id] === undefined); + expect(dangling, `gate needs a missing job: ${dangling.join(', ')}`).toHaveLength(0); + }); + + it('runs even when a dependency fails', () => { + // Without `always()` a failed dependency SKIPS this job rather than failing + // it, and a skipped required check is not a failure to GitHub — the gate + // would wave through exactly the runs it exists to stop. + expect(ci.jobs.gate?.if).toBe('always()'); + }); + + it('treats anything other than success as failure', () => { + const body = readFileSync(join(REPO_ROOT, '.github/workflows/ci.yml'), 'utf8'); + const gate = body.slice(body.indexOf(' gate:')); + // `skipped` and `cancelled` are not success. Comparing against `failure` + // would let both through. + expect(gate).toContain('!= "success"'); + expect(gate).not.toMatch(/=\s*"failure"/); + }); + + it('lint stays a separate required context, and is not matrix-interpolated', () => { + // `lint` lives in another workflow and cannot be a `needs:` from ci.yml, so + // protection requires two contexts. Neither may carry a matrix value, or a + // version bump renames it. + const demo = workflow('demo-lint.yml'); + expect(demo.jobs.lint, 'demo-lint.yml has no lint job').toBeDefined(); + for (const [file, id] of [['ci.yml', 'gate'], ['demo-lint.yml', 'lint']] as const) { + const body = readFileSync(join(REPO_ROOT, '.github/workflows', file), 'utf8'); + const job = body.slice(body.indexOf(` ${id}:`)); + const header = job.slice(0, job.indexOf('steps:')); + expect(header, `${id} must not be matrix-interpolated`).not.toContain('matrix.'); + } + }); +}); diff --git a/test/release-publish-prerequisites.test.ts b/test/release-publish-prerequisites.test.ts index ee0f8cd3..f85e1941 100644 --- a/test/release-publish-prerequisites.test.ts +++ b/test/release-publish-prerequisites.test.ts @@ -364,7 +364,18 @@ describe('the release gate requires every job CI runs on a push', () => { for (const [name, job] of Object.entries(workflow.jobs)) { if (name === 'lint') continue; - expect(job.if, `${name} may not be skipped`).toBeUndefined(); + // The rule is that a release-required job cannot be skipped, not that it + // cannot carry a condition. `always()` is the one expression that makes a + // job unskippable: it runs whatever its dependencies did. Every other + // condition can evaluate false, and a required job that never reported is + // indistinguishable here from one that was never run. + // + // The fan-in gate needs exactly that guarantee for the opposite reason: + // without `always()` a failed dependency SKIPS it rather than failing it, + // and GitHub does not treat a skipped required check as a failure. + if (job.if !== undefined) { + expect(job.if, `${name}'s condition must be always(), or the job can skip`).toBe('always()'); + } expect(job['continue-on-error'], `${name} may not be allowed to fail`).toBeUndefined(); expect(job.steps, `${name} needs executable work`).toEqual(expect.any(Array)); expect(job.steps!.length, `${name} may not be empty`).toBeGreaterThan(0);