diff --git a/.github/tests/ci-merge-group-isolation-bad-fixture.yaml b/.github/tests/ci-merge-group-isolation-bad-fixture.yaml index 2a046f1..658d932 100644 --- a/.github/tests/ci-merge-group-isolation-bad-fixture.yaml +++ b/.github/tests/ci-merge-group-isolation-bad-fixture.yaml @@ -16,7 +16,6 @@ jobs: ci-required-checks: if: "${{ always() }}" runs-on: ubuntu-latest - permissions: - contents: read + permissions: {} steps: - run: echo "Required gate" diff --git a/.github/tests/test-ci-merge-group-isolation.sh b/.github/tests/test-ci-merge-group-isolation.sh index d028d85..705d208 100755 --- a/.github/tests/test-ci-merge-group-isolation.sh +++ b/.github/tests/test-ci-merge-group-isolation.sh @@ -45,12 +45,12 @@ required_condition="$(yq -r '.jobs.ci-required-checks.if // ""' "$ci")" fail "ci-required-checks must remain the always-running merge-queue completion gate" required_permissions="$( - yq -r \ - '.jobs.ci-required-checks.permissions | to_entries | map(.key + "=" + .value) | sort | join(",")' \ + yq -o=json -I=0 \ + '.jobs.ci-required-checks.permissions' \ "$ci" )" -[[ "$required_permissions" == "contents=read" ]] || - fail "ci-required-checks must retain only contents=read; got: $required_permissions" +[[ "$required_permissions" == "{}" ]] || + fail "ci-required-checks must retain zero token permissions; got: $required_permissions" required_job="$(yq -o=json -I=0 '.jobs.ci-required-checks' "$ci")" if grep -qF 'secrets.' <<<"$required_job"; then diff --git a/.github/tests/test-ci-required-checks-boundary.sh b/.github/tests/test-ci-required-checks-boundary.sh new file mode 100644 index 0000000..26b1279 --- /dev/null +++ b/.github/tests/test-ci-required-checks-boundary.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash +set -euo pipefail + +workflow=${1:-.github/workflows/ci.yaml} +gate='.jobs.ci-required-checks' + +fail() { + printf 'ci-required-checks boundary: %s\n' "$1" >&2 + exit 1 +} + +[ -f "$workflow" ] || fail "workflow not found: $workflow" + +permissions=$(yq -o=json -I=0 "$gate.permissions" "$workflow") +[ "$permissions" = '{}' ] || + fail "the required gate must have no token permissions, got: $permissions" + +uses_steps=$(yq -r "$gate.steps[]? | select(.uses != null) | .uses" "$workflow") +[ -z "$uses_steps" ] || + fail "the required gate must not execute checked-out or external actions, got: $uses_steps" + +step_count=$(yq -r "[$gate.steps[]? | select(.name == \"📊 Summarize workflow result\")] | length" "$workflow") +[ "$step_count" = '1' ] || + fail "expected exactly one inline summary step, got: $step_count" + +script=$(yq -r "$gate.steps[] | select(.name == \"📊 Summarize workflow result\") | .run" "$workflow") +[ -n "$script" ] && [ "$script" != 'null' ] || + fail "the inline summary step has no executable script" + +run_case() { + local label=$1 input=$2 expected=$3 needle=$4 output rc=0 + + output=$(JOB_RESULTS="$input" bash -c "$script" 2>&1) || rc=$? + + if [ "$expected" = pass ] && [ "$rc" -ne 0 ]; then + fail "$label should pass, got exit $rc: $output" + fi + if [ "$expected" = fail ] && [ "$rc" -eq 0 ]; then + fail "$label should fail closed, got exit 0: $output" + fi + if [[ "$output" != *"$needle"* ]]; then + fail "$label should explain the result with '$needle', got: $output" + fi +} + +run_case 'success and skipped results' 'success skipped' pass 'all jobs succeeded or were skipped' +run_case 'failed result' 'success failure' fail 'failed or was cancelled' +run_case 'cancelled result' 'cancelled' fail 'failed or was cancelled' +run_case 'unknown result' 'success pending' fail "unknown job result: 'pending'" +run_case 'empty result list' '' fail 'no job results were provided' + +printf 'ci-required-checks boundary and behavior are enforced\n' diff --git a/.github/tests/test-zizmor-routing.sh b/.github/tests/test-zizmor-routing.sh index c20b168..070250d 100644 --- a/.github/tests/test-zizmor-routing.sh +++ b/.github/tests/test-zizmor-routing.sh @@ -141,8 +141,8 @@ aggregate_needs="$( aggregate_results="$( yq -r ' .jobs.ci-required-checks.steps[] - | select(.uses == "./aggregate-job-checks") - | .with["job-results"] + | select(.name == "📊 Summarize workflow result") + | .env.JOB_RESULTS ' "$ci" | grep -oE 'needs\.[a-z0-9-]*zizmor[a-z0-9-]*\.result' | sed -E 's/^needs\.//; s/\.result$//' | diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 69147f7..969e701 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -17,8 +17,8 @@ jobs: # predicate: an explicit exclusion, or the push-only Zizmor self-test. Merge refs contain # PR-controlled local actions and fixtures, so test jobs must never run them with # repository secrets or write-capable tokens. Only `CI - Required Checks` completes - # the merge-queue requirement; it receives no repository secrets and retains only - # contents: read. + # the merge-queue requirement; it receives no repository secrets, has no token + # permissions, and aggregates the results inline without checking out candidate code. # # The suite is also skipped for a release-please # release commit — BOTH the release-please PR (head branch `release-please--*`) AND @@ -26,7 +26,7 @@ jobs: # `github.head_ref` is empty so the branch-name check alone misses it). Those commits # only bump versions + the changelog — already tested in the PRs that introduced each # change — so there is nothing new to test. The `CI - Required Checks` gate still - # runs (aggregate-job-checks treats skipped as success), keeping the required status + # runs (the inline gate treats skipped as success), keeping the required status # check green so the release auto-merges. # ── approve-pr ────────────────────────────────────────────── test-approve-pr: @@ -131,7 +131,8 @@ jobs: runs-on: ubuntu-latest # The underlying action only produces a diff on pull_request events; on push / # merge_group there is no PR to review, so the test is PR-gated (skipped - # otherwise, which aggregate-job-checks tolerates — like test-approve-pr). + # otherwise, which the inline required-check collector tolerates — like + # test-approve-pr). if: "${{ github.event_name != 'merge_group' && !startsWith(github.head_ref, 'release-please--') && !startsWith(github.event.head_commit.message, 'chore(main): release ') && startsWith(github.event_name, 'pull_request') }}" permissions: contents: read @@ -1912,6 +1913,10 @@ jobs: shell: bash run: bash .github/tests/test-ci-merge-group-isolation-blocks.sh + - name: 🔒 Check required status gate stays workspace-independent + shell: bash + run: bash .github/tests/test-ci-required-checks-boundary.sh + - name: 📋 Check ci.yaml test wiring is complete shell: bash run: | @@ -1921,9 +1926,9 @@ jobs: # that does `uses: ./`; # (1b) every reusable workflow (workflow_call) is exercised by a job # that does `uses: ./.github/workflows/.yaml`; and - # (2) ci-required-checks.needs and the `job-results` input of its - # ./aggregate-job-checks step name the SAME set of jobs — a job - # in `needs:` but missing from `job-results` has its failure + # (2) ci-required-checks.needs and its trusted inline JOB_RESULTS + # environment value name the SAME set of jobs — a job in + # `needs:` but missing from `JOB_RESULTS` has its failure # silently ignored (the single required check stays green). # yq is preinstalled on the GitHub-hosted ubuntu-latest runner image. ci=".github/workflows/ci.yaml" @@ -1950,12 +1955,12 @@ jobs: fi done - # (2) ci-required-checks: needs <-> job-results parity + # (2) ci-required-checks: needs <-> inline JOB_RESULTS parity needs="$(yq -r '.jobs.ci-required-checks.needs[]' "$ci" | sort -u)" results="$( yq -r '.jobs.ci-required-checks.steps[] - | select(.uses == "./aggregate-job-checks") - | .with["job-results"]' "$ci" \ + | select(.name == "📊 Summarize workflow result") + | .env.JOB_RESULTS' "$ci" \ | grep -oE 'needs\.[a-z0-9-]+\.result' \ | sed -E 's/needs\.(.*)\.result/\1/' | sort -u )" @@ -1964,7 +1969,7 @@ jobs: while IFS= read -r j; do [[ -z "$j" ]] && continue if ! grep -qxF "$j" <<<"$results"; then - echo "::error file=$ci::job '$j' is in ci-required-checks.needs but missing from the aggregate-job-checks job-results input — its failure would be silently ignored" + echo "::error file=$ci::job '$j' is in ci-required-checks.needs but missing from the trusted inline JOB_RESULTS value — its failure would be silently ignored" status=1 fi done <<<"$needs" @@ -1973,13 +1978,13 @@ jobs: while IFS= read -r j; do [[ -z "$j" ]] && continue if ! grep -qxF "$j" <<<"$needs"; then - echo "::error file=$ci::job '$j' is referenced in job-results but not in ci-required-checks.needs — its result will never be populated" + echo "::error file=$ci::job '$j' is referenced in JOB_RESULTS but not in ci-required-checks.needs — its result will never be populated" status=1 fi done <<<"$results" if [[ "$status" -eq 0 ]]; then - echo "CI coverage parity OK: every action and reusable workflow has a test job, and ci-required-checks needs <-> job-results are in sync." + echo "CI coverage parity OK: every action and reusable workflow has a test job, and ci-required-checks needs <-> JOB_RESULTS are in sync." fi exit "$status" @@ -3280,17 +3285,13 @@ jobs: - test-run-dotnet-tests-gate-lockstep - test-run-dotnet-tests-coverage-inline-lockstep runs-on: ubuntu-latest - permissions: - contents: read + permissions: {} steps: - - name: 📑 Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - - - uses: ./aggregate-job-checks - with: - job-results: >- + # Keep the required gate inline: checked-out pull request actions are attacker-controlled. + - name: 📊 Summarize workflow result + shell: bash + env: + JOB_RESULTS: >- ${{ needs.test-approve-pr.result }} ${{ needs.test-approve-pr-credential-boundary.result }} ${{ needs.test-cleanup-ghcr-packages.result }} @@ -3381,3 +3382,28 @@ jobs: ${{ needs.test-run-dotnet-tests-blocks.result }} ${{ needs.test-run-dotnet-tests-gate-lockstep.result }} ${{ needs.test-run-dotnet-tests-coverage-inline-lockstep.result }} + run: | + set -Eeuo pipefail + set -f + if [[ ! "$JOB_RESULTS" =~ [^[:space:]] ]]; then + echo "❌ CI - Required Checks — no job results were provided." + exit 1 + fi + read -r -a results <<< "$JOB_RESULTS" + + for result in "${results[@]}"; do + case "$result" in + success|skipped) + ;; + failure|cancelled) + echo "❌ CI - Required Checks — at least one job failed or was cancelled." + exit 1 + ;; + *) + echo "❌ CI - Required Checks — unknown job result: '$result'." + exit 1 + ;; + esac + done + + echo "✅ CI - Required Checks — all jobs succeeded or were skipped." diff --git a/AGENTS.md b/AGENTS.md index 785cb1a..e853000 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,7 +78,7 @@ Because composite actions and reusable workflows now live together, one componen 1. Create `/action.yaml` and `/README.md` (template in CONTRIBUTING.md) 2. Add a row to the Actions table in [README.md](README.md) -3. Add a `test-` job to `.github/workflows/ci.yaml` (`persist-credentials: false` on checkout), wired into `ci-required-checks` (both `needs:` and the `job-results` input of its `./aggregate-job-checks` step) +3. Add a `test-` job to `.github/workflows/ci.yaml` (`persist-credentials: false` on checkout), wired into `ci-required-checks` (both `needs:` and the inline summary step's `JOB_RESULTS` value) 4. Run `zizmor` locally before pushing ## Adding / Changing a Reusable Workflow @@ -109,7 +109,7 @@ on: ### Test jobs -Actions and reusable workflows are exercised as jobs inside [`ci.yaml`](.github/workflows/ci.yaml): `test-` jobs call the action via `uses: ./`; `[Test] - ` jobs call the workflow via `uses: ./.github/workflows/.yaml` with safe parameters (dry-run, fixtures from `.github/tests/` or `.github/fixtures/` — never destructive). Every new action/workflow gets a job, wired into the `ci-required-checks` job (display `CI - Required Checks`) in **two** places: the `needs:` list **and** `${{ needs..result }}` in the `job-results` input of its `./aggregate-job-checks` step. `ci-required-checks` runs `if: ${{ always() }}` and fails if any listed result is not `success`, so it is the single required status check — a job added to `needs:` but omitted from `job-results` would have its failure silently ignored. The `lint-ci-coverage-parity` job **guards this**: it fails the PR if any composite action lacks a `uses: ./` test job, if any reusable workflow (`workflow_call`) lacks a `uses: ./.github/workflows/.yaml` test job, or if `ci-required-checks.needs` and the `job-results` input ever name different sets of jobs (so the silent-ignore footgun can't recur). When a reusable-workflow test-job id would collide with an action's (`test-dependency-review`, `test-run-dotnet-tests`), the workflow job carries a `-workflow` suffix. +Actions and reusable workflows are exercised as jobs inside [`ci.yaml`](.github/workflows/ci.yaml): `test-` jobs call the action via `uses: ./`; `[Test] - ` jobs call the workflow via `uses: ./.github/workflows/.yaml` with safe parameters (dry-run, fixtures from `.github/tests/` or `.github/fixtures/` — never destructive). Every new action/workflow gets a job, wired into the `ci-required-checks` job (display `CI - Required Checks`) in **two** places: the `needs:` list **and** `${{ needs..result }}` in the inline summary step's `JOB_RESULTS` value. `ci-required-checks` runs `if: ${{ always() }}`, holds `permissions: {}`, executes no checked-out action, and fails if any listed result is not `success` or `skipped`, so it is the single required status check — a job added to `needs:` but omitted from `JOB_RESULTS` would have its failure silently ignored. The `lint-ci-coverage-parity` job **guards this**: it fails the PR if any composite action lacks a `uses: ./` test job, if any reusable workflow (`workflow_call`) lacks a `uses: ./.github/workflows/.yaml` test job, if `ci-required-checks.needs` and `JOB_RESULTS` name different sets of jobs, or if the gate regains a workspace-dependent step (so the silent-ignore and candidate-code footguns cannot recur). When a reusable-workflow test-job id would collide with an action's (`test-dependency-review`, `test-run-dotnet-tests`), the workflow job carries a `-workflow` suffix. ### Shipping a new capability behind an opt-in flag (feature-flag-first)