From b7f23621c82f9e25046d6c904dd766fd89c9e0af Mon Sep 17 00:00:00 2001 From: Doreen Date: Tue, 20 Oct 2026 13:57:35 +0200 Subject: [PATCH] feat: audit action pins for impostor and mismatched commits --- .github/workflows/audit-action-pins.yml | 149 ++++++++++++++++++++++ .github/workflows/test-actions.yml | 2 + README.md | 28 ++++- docs/NEW_REPO.md | 1 + flake.nix | 1 + scripts/test-audit-action-pins.sh | 157 ++++++++++++++++++++++++ 6 files changed, 337 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/audit-action-pins.yml create mode 100755 scripts/test-audit-action-pins.sh diff --git a/.github/workflows/audit-action-pins.yml b/.github/workflows/audit-action-pins.yml new file mode 100644 index 0000000..1415a34 --- /dev/null +++ b/.github/workflows/audit-action-pins.yml @@ -0,0 +1,149 @@ +name: Audit Action Pins + +# Reusable, and also run on dev-kit's own PRs. Consumers call it from a stub; +# see "Governance workflows" in the README. +# +# update-action-pins.yml makes sure every action is pinned to a SHA; this checks +# whether that SHA can be trusted. It runs zizmor's online audits over every +# workflow and composite action, and fails only on these findings: +# - impostor-commit: the SHA isn't on any branch or tag of the upstream repo, +# e.g. a fork's commit served under the parent repo's URL. +# - ref-version-mismatch: the version comment doesn't match the SHA. +# zizmor can't run a subset of its audits, so the job filters its JSON output. +# +# On pull requests only findings on lines the PR adds or changes count. A pinned +# SHA's code never changes, so a finding on a pin the PR doesn't touch comes +# from upstream moving a tag, and would otherwise block every PR. Any other +# event, such as a manual `workflow_dispatch` run, audits every pin. +# +# No `paths` filter on purpose: a required check gated by paths never reports on +# PRs outside them, so the merge stays blocked. + +permissions: + contents: read + +on: + pull_request: + workflow_call: + workflow_dispatch: + +# `bash` runs with pipefail, unlike the default `bash -e`: a failing git diff or +# jq at the head of a pipeline must fail the step, not leave an empty result +# that reads as "nothing to audit" or "no findings". +defaults: + run: + shell: bash + +jobs: + audit-pins: + name: Audit action pins + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + path: repo + # The PR's merge commit and its first parent, the base branch, to + # list the lines the PR changes. + fetch-depth: 2 + persist-credentials: false + - name: List the lines the PR changes + id: changes + if: github.event_name == 'pull_request' + working-directory: repo + run: | + # : for each line the PR adds or changes, from the hunk + # headers of the merge commit's diff against the base branch. The + # prefixes are explicit so a diff.noprefix or diff.mnemonicPrefix + # setting can't change the paths, and core.quotePath is off so + # non-ASCII paths aren't quoted and escaped. git ends the `+++` line + # with a tab when the path holds a space; the tab isn't part of it. + git -c core.quotePath=false diff --unified=0 --no-color --no-ext-diff \ + --src-prefix=a/ --dst-prefix=b/ HEAD^1 HEAD \ + | awk ' + /^diff --git / { header = 1; next } + header && /^\+\+\+ / { path = substr($0, 7); sub(/\t$/, "", path); next } + /^@@ / { + header = 0 + split(substr($3, 2), range, ",") + count = (2 in range) ? range[2] : 1 + for (i = 0; i < count; i++) print path ":" (range[1] + i) + }' > "$RUNNER_TEMP/changed-lines" + # Findings only count on changed lines, so without a changed line in a + # workflow or an action there's nothing to audit. The remaining steps + # are skipped rather than the workflow being filtered by `paths`, so + # the check still reports. + if grep -qE '^(\.github/workflows/[^/]+|(.*/)?action)\.ya?ml:[0-9]+$' "$RUNNER_TEMP/changed-lines"; then + echo "audit=true" >> "$GITHUB_OUTPUT" + else + echo "audit=false" >> "$GITHUB_OUTPUT" + echo "No changed lines in a workflow or an action." + fi + # dev-kit's flake at the commit this workflow runs from, so zizmor comes + # from the same locked nixpkgs as the dev shell. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + if: steps.changes.outputs.audit != 'false' + with: + repository: ${{ job.workflow_repository }} + ref: ${{ job.workflow_sha }} + path: dev-kit + sparse-checkout: | + flake.nix + flake.lock + sparse-checkout-cone-mode: false + persist-credentials: false + # Inlined rather than taken from setup-nix, as in flake-closure-diff: a + # reusable workflow cannot use the `./` path, which resolves against the + # caller. No Cachix: zizmor comes from cache.nixos.org. + - name: Install nix + if: steps.changes.outputs.audit != 'false' + # Upstream Nix, as in setup-nix. + uses: cachix/install-nix-action@448cde739abec1eaf0646249e3d25bb8d9bc7702 # v31.11.2 + - name: Run zizmor + if: steps.changes.outputs.audit != 'false' + working-directory: repo + env: + GH_TOKEN: ${{ github.token }} + run: | + # Without a token zizmor falls back to offline mode: it skips the + # online audits, impostor-commit and ref-version-mismatch included, + # and the check would pass without auditing anything. + if [[ -z "$GH_TOKEN" ]]; then + echo "::error::No GitHub token, so zizmor's online audits would not run." + exit 1 + fi + # --no-config: a repo's zizmor.yml must not turn these audits off. + # --no-ignores: an inline `# zizmor: ignore[...]` doesn't hide a + # finding either. Neither finding is ever fine to merge unseen. + # --no-exit-codes: fail only when zizmor itself fails, not on findings. + nix shell --inputs-from path:../dev-kit nixpkgs#zizmor --command \ + zizmor --format json-v1 --no-exit-codes --no-config --no-ignores \ + --no-progress . > "$RUNNER_TEMP/zizmor.json" + - name: Fail on untrusted pins + if: steps.changes.outputs.audit != 'false' + run: | + # An empty or truncated report would read as "no findings". + if ! jq -e 'type == "array"' "$RUNNER_TEMP/zizmor.json" > /dev/null 2>&1; then + echo "::error::zizmor's report is missing or not a JSON array." + exit 1 + fi + # On a pull request, only findings on the lines the PR changes count. + # zizmor's primary location for both audits is the `uses:` line. + scoped=false changed=/dev/null + if [[ -f "$RUNNER_TEMP/changed-lines" ]]; then + scoped=true changed="$RUNNER_TEMP/changed-lines" + fi + errors=$(jq -r --argjson scoped "$scoped" --rawfile changed "$changed" ' + ($changed | split("\n") | map({(.): true}) | add // {}) as $scope + | .[] + | select(.ident == "impostor-commit" or .ident == "ref-version-mismatch") + | ((.locations | map(select(.symbolic.kind == "Primary")) | first) // .locations[0]) as $l + | ($l.symbolic.key.Local.verbatim_path | ltrimstr("./")) as $path + | ($l.concrete.location.start_point.row + 1) as $line + | select(($scoped | not) or $scope["\($path):\($line)"]) + | "::error file=\($path),line=\($line)::\(.ident): \(.desc)\(if $l.symbolic.annotation then " (\($l.symbolic.annotation))" else "" end)"' \ + "$RUNNER_TEMP/zizmor.json" | sort -u) + if [[ -n "$errors" ]]; then + echo "$errors" + exit 1 + fi + echo "No impostor commits or mismatched version comments." diff --git a/.github/workflows/test-actions.yml b/.github/workflows/test-actions.yml index 83f29da..13db033 100644 --- a/.github/workflows/test-actions.yml +++ b/.github/workflows/test-actions.yml @@ -50,6 +50,8 @@ jobs: run: .github/actions/get-go-version/test-extract-go-version.sh - name: Test the update-action-pins check run: scripts/test-update-action-pins.sh + - name: Test the audit-action-pins steps + run: scripts/test-audit-action-pins.sh - name: Test check-go-version.sh run: scripts/test-check-go-version.sh - name: Test check-dev-kit-pins.sh diff --git a/README.md b/README.md index 77485b3..d66f679 100644 --- a/README.md +++ b/README.md @@ -342,6 +342,7 @@ them together with `DEV_KIT_VERSION`. | Workflow | Stub triggers | Needs | | --------------------------- | ------------------------------------------------------------------ | ------------------------------- | | `update-action-pins.yml` | `pull_request` | | +| `audit-action-pins.yml` | `pull_request`, `workflow_dispatch` | | | `conventional-commits.yml` | `pull_request` (opened, edited, synchronize, reopened) | `.commitlintrc.yml` in the repo | | `osv-scanner.yml` | `pull_request`, `push` to main, weekly | optional `scan-args` input | | `issues-add-labels.yaml` | `issues` (opened, reopened) | | @@ -379,6 +380,30 @@ moved the tag, or the PR swapped the SHA. Pins should name a full release tag (`# v7.0.1`, not `# v7`), since a major tag moves with every release and would trip the check on each update. +`.github/workflows/audit-action-pins.yml`: + +```yaml +name: Audit Action Pins +on: + pull_request: + workflow_dispatch: +permissions: + contents: read +jobs: + audit-action-pins: + uses: opendefensecloud/dev-kit/.github/workflows/audit-action-pins.yml@<40-char-sha> # +``` + +It runs zizmor with the job's `GITHUB_TOKEN`, which zizmor needs to look up +upstream tags and branches, and fails on an impostor commit or a version comment +that doesn't match its SHA. Neither `# zizmor: ignore[...]` comments nor a +`zizmor.yml` in the repo turn a finding off. + +On pull requests it only fails on pins the PR adds or changes: a pinned SHA's +code never changes, so a finding on an untouched pin means upstream moved a tag, +and would otherwise block every PR. A manual `workflow_dispatch` run audits +every pin, e.g. when a repo adopts the workflow. + `.github/workflows/conventional-commits.yml`: ```yaml @@ -483,7 +508,8 @@ jobs: A job from a reusable workflow reports as ` / `, so `REPO_STATUS_CHECKS` needs the new names after switching to a stub, e.g. -`update-action-pins / Check action pins`, `conventional-commits / PR Title` and +`update-action-pins / Check action pins`, +`audit-action-pins / Audit action pins`, `conventional-commits / PR Title` and `conventional-commits / Commit Messages`. osv-scanner nests Google's reusable workflow one level deeper, so its scan reports as `osv-scanner / scan-pr / osv-scan`. Update the list and rerun `make repo-settings` in the same change, or the old diff --git a/docs/NEW_REPO.md b/docs/NEW_REPO.md index 9e0c21a..fc72d1e 100644 --- a/docs/NEW_REPO.md +++ b/docs/NEW_REPO.md @@ -68,6 +68,7 @@ Copy the relevant workflows from `.github/workflows/` in this repository: | `issues-add-to-project.yml` | Adds new issues and PRs to the org project board | | `release-drafter.yaml` | Drafts release notes from merged PRs | | `update-action-pins.yml` | Fails the PR if any action is not pinned to a SHA with a version comment, or a pin's SHA changes under the same version | +| `audit-action-pins.yml` | Fails the PR if a pin it changes is an impostor commit or mismatches its version comment | If using release-drafter, also copy `.github/release-drafter.yml` (the config file). diff --git a/flake.nix b/flake.nix index 4f67689..3cd5682 100644 --- a/flake.nix +++ b/flake.nix @@ -56,6 +56,7 @@ unzip yamlfmt yq-go + zizmor ]; defaultHooks = { diff --git a/scripts/test-audit-action-pins.sh b/scripts/test-audit-action-pins.sh new file mode 100755 index 0000000..b4fb56a --- /dev/null +++ b/scripts/test-audit-action-pins.sh @@ -0,0 +1,157 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Runs the steps from audit-action-pins.yml itself (extracted with yq), so the +# test cannot drift from what CI runs; the filter runs on findings shaped like +# zizmor's json-v1 output. + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" + +# Keep the developer's git config (commit signing, hooks) out of the test repos. +export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 + +# step : prints the run script of the audit-pins step with that name. +step() { + local run + run="$(yq ".jobs.audit-pins.steps[] | select(.name == \"$1\") | .run" \ + "$ROOT/.github/workflows/audit-action-pins.yml")" + [[ -n "$run" ]] || { + echo "FAIL: step '$1' not found" >&2 + exit 1 + } + printf '%s\n' "$run" +} + +FILTER="$(step "Fail on untrusted pins")" +LIST="$(step "List the lines the PR changes")" + +fail=0 + +# report +report() { + if [[ "$2" == "$1" ]]; then + echo "ok $3" + else + echo "FAIL $3: expected $1, got $2" + fail=1 + fi +} + +# The steps below run as `bash -eo pipefail`, which `shell: bash` gives in CI. +report bash "$(yq '.defaults.run.shell' "$ROOT/.github/workflows/audit-action-pins.yml")" \ + "steps run with pipefail" + +# finding : prints one finding on .github/workflows/a.yml line 7 as +# zizmor reports it. +finding() { + jq -n --arg ident "$1" '{ + ident: $ident, + desc: "description", + locations: [{ + symbolic: {key: {Local: {verbatim_path: "./.github/workflows/a.yml"}}, kind: "Primary", annotation: null}, + concrete: {location: {start_point: {row: 6, column: 8}}} + }] + }' +} + +# check : runs the filter on as +# zizmor.json, scoped to the lines in $SCOPE (one : each) when set. +check() { + local want=$1 desc=$2 dir got + dir="$(mktemp -d)" + printf '%s' "$3" > "$dir/zizmor.json" + if [[ -n "${SCOPE+x}" ]]; then printf '%s' "$SCOPE" > "$dir/changed-lines"; fi + if RUNNER_TEMP=$dir bash -eo pipefail -c "$FILTER" > /dev/null 2>&1; then got=pass; else got=fail; fi + rm -rf "$dir" + report "$want" "$got" "$desc" +} + +# expect ...: runs the filter on a report +# holding the given findings. +expect() { + local want=$1 desc=$2 + shift 2 + if (($#)); then + check "$want" "$desc" "$(printf '%s\n' "$@" | jq -s .)" + else + check "$want" "$desc" '[]' + fi +} + +expect pass "no findings" +expect pass "other audits only" "$(finding artipacked)" "$(finding known-vulnerable-actions)" +expect fail "impostor-commit" "$(finding impostor-commit)" +expect fail "ref-version-mismatch" "$(finding ref-version-mismatch)" +expect fail "relevant finding among others" "$(finding artipacked)" "$(finding impostor-commit)" +check fail "empty report" '' +check fail "truncated report" '[{"ident": "impostor-commit"' +check fail "report that isn't an array" '{"error": "rate limited"}' +check fail "finding in an unexpected shape" '[{"ident": "impostor-commit", "desc": "description"}]' + +SCOPE=".github/workflows/a.yml:7" +expect fail "finding on a changed line" "$(finding impostor-commit)" +SCOPE=".github/workflows/a.yml:8" +expect pass "finding on an unchanged line" "$(finding ref-version-mismatch)" +SCOPE=".github/workflows/b.yml:7" +expect pass "finding on the same line of another file" "$(finding impostor-commit)" +unset SCOPE + +# expect_lines : commits both versions of (none when empty) and runs the list +# step on them. Expected lines are :, space-separated; expected +# audit is the step's `audit` output. +expect_lines() { + local desc=$1 file=$2 base=$3 head=$4 want="$5 audit=$6" dir got + dir="$(mktemp -d)" + mkdir -p "$dir/repo/$(dirname "$file")" + ( + cd "$dir/repo" + git init -q + if [[ -n "$base" ]]; then printf '%s\n' "$base" > "$file"; fi + git add -A && git -c user.name=t -c user.email=t@t commit -q --allow-empty -m base + if [[ -n "$head" ]]; then printf '%s\n' "$head" > "$file"; else rm -f "$file"; fi + git add -A && git -c user.name=t -c user.email=t@t commit -q --allow-empty -m head + ) + if (cd "$dir/repo" && RUNNER_TEMP=$dir GITHUB_OUTPUT=$dir/output bash -eo pipefail -c "$LIST") > /dev/null 2>&1; then + got="$(tr '\n' ' ' < "$dir/changed-lines" | sed 's/ $//') $(cat "$dir/output")" + else + got="(step failed)" + fi + rm -rf "$dir" + report "$want" "$got" "$desc" +} + +A=".github/workflows/a.yml" +expect_lines "changed line" "$A" "$(printf 'a\nb\nc')" "$(printf 'a\nB\nc')" "$A:2" true +expect_lines "added lines" "$A" "$(printf 'a\nc')" "$(printf 'a\nb1\nb2\nc')" "$A:2 $A:3" true +expect_lines "removed line only" "$A" "$(printf 'a\nb\nc')" "$(printf 'a\nc')" "" false +expect_lines "new file" "$A" "" "$(printf 'a\nb')" "$A:1 $A:2" true +expect_lines "deleted file" "$A" "$(printf 'a\nb')" "" "" false +expect_lines "added line that looks like a header" "$A" "a" "$(printf 'a\n++ b')" "$A:2" true + +for f in .github/workflows/a.yaml .github/actions/foo/action.yml action.yaml \ + ".github/actions/my action/action.yml" .github/actions/größe/action.yml; do + expect_lines "change to $f" "$f" "" "a" "$f:1" true +done +for f in main.go .github/dependabot.yml .github/workflows/sub/a.yml myaction.yml; do + expect_lines "change to $f" "$f" "" "a" "$f:1" false +done + +# Without the base commit, the list step must fail rather than find no changes. +dir="$(mktemp -d)" +mkdir -p "$dir/repo/.github/workflows" +( + cd "$dir/repo" + git init -q + printf 'a\n' > .github/workflows/a.yml + git add -A && git -c user.name=t -c user.email=t@t commit -q -m head +) +if (cd "$dir/repo" && RUNNER_TEMP=$dir GITHUB_OUTPUT=$dir/output bash -eo pipefail -c "$LIST") > /dev/null 2>&1; then + got=pass +else + got=fail +fi +rm -rf "$dir" +report fail "$got" "missing base commit" + +exit "$fail"