From 53d2f1d8b68792ba3c2b3dc36a1cec06c349f449 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:22:54 +0100 Subject: [PATCH] ci(y1): drift script and sha-pins hook read uses: refs with yq YAML-POLICY Y-1: a verdict about a workflow comes from a YAML parser, never a line grep. The two remaining line-grep readers move to the same selector as #1115: yq -r '.. | select(tag == "!!map") | select(has("uses")) | .uses | select(tag == "!!str")' scripts/check-lockfile-drift.sh - A quoted KYAML ref (`uses: "a/b@v1",`) never matched the grep, so a drifted KYAML workflow scanned clean. It now reports drift. - `uses:` text inside a `run:` body was read as a ref (false drift). - A workflow that does not parse, or parses with no `jobs:` map (an unclosed `name: "X` quote swallowing the file), is UNEXAMINED: exit 2, never "clean". yq missing is exit 2. .githooks/validate-sha-pins.sh - A quoted local ref (`uses: "./x"`) was false-failed as unpinned. - A one-line flow step (`- { uses: a/b@v1 }`) was never checked. - An unparseable workflow, or one with no `jobs:` map, fails closed. - Fixes a fixture typo the new guard caught (docker.yml had no jobs:). Known answer: sha-pins error counts identical on all 172 workflow/action files on main (92 vendored mirror files compared via a neutral copy). Every drift difference across 101 estate repos is old-was-wrong: run-body decoys or invalid YAML the old script certified clean. Tests: scripts/tests/check-lockfile-drift-test.sh (new, 8 cases) and 7 new cases in validate-sha-pins-test.sh. The old scripts fail 4 of each, including the KYAML mutant. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W5CoaksP2Bg21HpDCgFgwS --- .githooks/validate-sha-pins.sh | 49 ++++++-- scripts/check-lockfile-drift.sh | 51 ++++++-- scripts/tests/check-lockfile-drift-test.sh | 132 +++++++++++++++++++++ scripts/tests/validate-sha-pins-test.sh | 56 ++++++++- 4 files changed, 265 insertions(+), 23 deletions(-) create mode 100755 scripts/tests/check-lockfile-drift-test.sh diff --git a/.githooks/validate-sha-pins.sh b/.githooks/validate-sha-pins.sh index 3d5c1ee9d..51980ee0b 100755 --- a/.githooks/validate-sha-pins.sh +++ b/.githooks/validate-sha-pins.sh @@ -31,6 +31,7 @@ # rather than skipped silently. The scope matches .githooks/validate-actions-lock.sh, # the sibling hook this repo also runs in CI (.github/workflows/actions-lock-gate.yml). set -euo pipefail +command -v yq >/dev/null 2>&1 || { echo "[validate-sha-pins] ERROR: yq not found -- required to read workflows (YAML-POLICY Y-1)" >&2; exit 1; } SCAN_PATH="${INPUT_PATH:-.}" STAGED_FILES="${INPUT_STAGED_FILES:-}" VENDORED_MARK="rhodium-standard-repositories/" @@ -40,9 +41,11 @@ SKIPPED=0 is_vendored() { case "$1" in *"${VENDORED_MARK}"*) return 0 ;; *) return 1 ;; esac; } -# One line of a workflow, emitted per unpinned ref. Local paths (`./`, `../`) are -# this repo's own composite actions and `docker://` refs are container images, not -# actions -- neither is modelled by actions.lock, which keys actions only. +# Prints one `:uses: ` record per ref in that is neither a +# full-SHA pin, a local path, nor a docker:// image (or one record naming a +# parse failure). Empty output means the file is clean. Local paths (`./`, +# `../`) are this repo's own composite actions and `docker://` refs are container +# images, not actions -- neither is modelled by actions.lock, which keys actions only. UNPINNED_FILTER() { # ONE pipeline, and the first selector matches any NON-BLANK ref. # @@ -63,15 +66,40 @@ UNPINNED_FILTER() { # carried a live `$/` ref the whole time. A second reader of one stdin is # never a second chance. # + # 3. (YAML-POLICY Y-1) The selector was a line grep, which saw only block + # style: a quoted ref (`uses: "./x"`) slipped past the local-path arm and + # was false-failed, a one-line flow step (`- { uses: a/b@v1 }`) was never + # matched at all, and `uses:` text inside a `run:` body was read as a ref. + # Refs now come from the YAML parser, one `:uses: ` record each; + # a file that does not parse is reported as such, never scanned clean. + # # An unknown-shaped ref must be REPORTED, never skipped. The exemptions below # are the only way out, and each one is explicit. - grep -nE '^[[:space:]]*(-[[:space:]]*)?uses:[[:space:]]+[^[:space:]]' \ - | grep -vE 'uses:[[:space:]]+[./]' \ - | grep -vE 'uses:[[:space:]]+docker://' \ - | grep -vE 'uses:[[:space:]]+[^[:space:]@]+@[0-9a-f]{40}([^0-9a-f]|$)' \ - || true + local file=$1 refs + if ! refs=$(yq -r '.. | select(tag == "!!map") | select(has("uses")) | .uses | select(tag == "!!str") | ((line | tostring) + ":" + .)' "$file" 2>&1); then + printf '0:uses: \n' "$(printf '%s' "$refs" | head -1)" + return 0 + fi + # An unclosed quote can swallow a workflow into one scalar that still parses, + # leaving no jobs and no refs: that is a broken file, not a clean one. + case "$file" in + *.github/workflows/*) + if [ "$(yq -r '.jobs | tag' "$file" 2>/dev/null)" != '!!map' ]; then + printf '0:uses: \n' + return 0 + fi ;; + esac + [ -n "$refs" ] || return 0 + printf '%s\n' "$refs" \ + | sed -E 's/^([0-9]+):/\1:uses: /' \ + | { /usr/bin/grep -E '^[0-9]+:uses: [^[:space:]]' || true; } \ + | { /usr/bin/grep -vE '^[0-9]+:uses: [./]' || true; } \ + | { /usr/bin/grep -vE '^[0-9]+:uses: docker://' || true; } \ + | { /usr/bin/grep -vE '^[0-9]+:uses: [^[:space:]@]+@[0-9a-f]{40}$' || true; } } +# Reports every unpinned or invalid `uses:` ref in to stderr and adds +# one to ERRORS per ref. validate_file() { local file="$1" rec lineno body while IFS= read -r rec; do @@ -81,6 +109,9 @@ validate_file() { body="${body#"${body%%[![:space:]]*}"}" echo "[validate-sha-pins] ERROR: $file:$lineno: ${body}" >&2 case "$body" in + *'&2 + ;; *'uses:'*'$/'*) # Name the real fault. `$/...` is not an unpinned ref, it is not valid # `uses:` syntax at all, so the workflow dies at STARTUP and no job of @@ -96,7 +127,7 @@ validate_file() { ;; esac ERRORS=$((ERRORS + 1)) - done < <(UNPINNED_FILTER < "$file") + done < <(UNPINNED_FILTER "$file") } if [ -n "$STAGED_FILES" ]; then diff --git a/scripts/check-lockfile-drift.sh b/scripts/check-lockfile-drift.sh index babfa2495..97bc27dac 100755 --- a/scripts/check-lockfile-drift.sh +++ b/scripts/check-lockfile-drift.sh @@ -49,7 +49,8 @@ set -eo pipefail # file can be appended straight into the report) # Exit: 0 = no drift (or no lockfile — not this script's business) # 1 = drift found -# 2 = usage / environment error +# 2 = usage / environment error (incl. yq missing), or a workflow +# that does not parse as YAML (UNEXAMINED, never "clean") REPO_DIR="${1:-.}" REPO_SLUG="${2:-$(basename "$REPO_DIR")}" @@ -63,12 +64,16 @@ WFDIR="$REPO_DIR/.github/workflows" # has none and runs fine), so absence is NOT evidence of a fault. [ -f "$LOCK" ] || { echo "[drift] no actions.lock in $REPO_DIR — out of scope (see mode 1)"; exit 0; } +command -v yq >/dev/null 2>&1 || { echo "[drift] yq not found — required to read workflows (YAML-POLICY Y-1)" >&2; exit 2; } + drift=0 checked=0 +unexamined=0 want_tmp="$(mktemp)" have_tmp="$(mktemp)" -trap 'rm -f "$want_tmp" "$have_tmp"' EXIT +uses_tmp="$(mktemp)" +trap 'rm -f "$want_tmp" "$have_tmp" "$uses_tmp"' EXIT for wf in "$WFDIR"/*.yml "$WFDIR"/*.yaml; do [ -f "$wf" ] || continue @@ -80,14 +85,30 @@ for wf in "$WFDIR"/*.yml "$WFDIR"/*.yaml; do # those legitimately carry a bare `[]` entry and are not drift # * strip sub-action paths: `github/codeql-action/init@v1` is recorded in # the lockfile as `github/codeql-action@v1` - # Strip comments FIRST. A commented-out `# - uses: foo@sha` is dead code; - # flagging it is a false positive, and a detector that cries wolf is ignored. - sed -E 's/^[[:space:]]*#.*$//' "$wf" 2>/dev/null \ - | grep -oE "uses:[[:space:]]*[A-Za-z0-9_.-]+/[A-Za-z0-9_./-]+@[A-Za-z0-9._-]+" 2>/dev/null \ - | sed -E 's/uses:[[:space:]]*//' \ - | grep -v '/\.github/workflows/' \ + # Read `uses:` values with the YAML parser (YAML-POLICY Y-1). The old line + # grep could not see a quoted KYAML value (`uses: "a/b@v1",`) and silently + # skipped the workflow; it also matched `uses:` text inside a `run:` body. + # A parser never sees comments, so commented-out steps stay ignored. Only + # owner/repo[/path]@ref values count: local `./` actions and `docker://` + # images have no lockfile entry. An unparseable workflow is counted as + # UNEXAMINED and makes the run exit 2 rather than pass as clean. + if ! yq -r '.. | select(tag == "!!map") | select(has("uses")) | .uses | select(tag == "!!str")' "$wf" > "$uses_tmp" 2>/dev/null; then + echo "[drift] $base: not parseable as YAML — UNEXAMINED" >&2 + unexamined=$((unexamined + 1)) + continue + fi + # An unclosed quote (`name: "X` -- measured in 007-lang oracle-fuzz.yml) + # can swallow the rest of the file into one scalar and still PARSE, leaving + # no jobs and no refs. GitHub cannot run that file, so it is not "clean". + if [ "$(yq -r '.jobs | tag' "$wf" 2>/dev/null)" != '!!map' ]; then + echo "[drift] $base: parses, but has no jobs: map — UNEXAMINED" >&2 + unexamined=$((unexamined + 1)) + continue + fi + { /usr/bin/grep -E '^[A-Za-z0-9_.-]+/[A-Za-z0-9_./-]+@[A-Za-z0-9._-]+$' "$uses_tmp" || true; } \ + | { /usr/bin/grep -v '/\.github/workflows/' || true; } \ | sed -E 's#^([^/]+/[^/@]+)(/[^@]*)?@#\1@#' \ - | sort -u > "$want_tmp" || true + | sort -u > "$want_tmp" [ -s "$want_tmp" ] || continue @@ -100,7 +121,7 @@ for wf in "$WFDIR"/*.yml "$WFDIR"/*.yaml; do while read -r want; do [ -n "$want" ] || continue - grep -qxF "$want" "$have_tmp" && continue + /usr/bin/grep -qxF "$want" "$have_tmp" && continue name="${want%@*}" ref="${want#*@}" @@ -108,13 +129,13 @@ for wf in "$WFDIR"/*.yml "$WFDIR"/*.yaml; do # Only DRIFT if the lockfile knows this action at a *different* version. # Absent entirely is mode 2/3, or a verified-creator action that # legitimately needs no entry (e.g. Swatinem/rust-cache) — not drift. - have="$(grep -m1 -F "$name@" "$have_tmp")" || continue + have="$(/usr/bin/grep -m1 -F "$name@" "$have_tmp")" || continue # A workflow may pin by 40-char SHA while the lockfile records a TAG. # That is the same action in two notations, NOT drift. The lockfile # resolves every entry to `commit: 'sha1-<40hex>'`, so compare against # that rather than against the tag string. - if printf '%s' "$ref" | grep -qE '^[0-9a-f]{40}$'; then + if printf '%s' "$ref" | /usr/bin/grep -qE '^[0-9a-f]{40}$'; then resolved="$(awk -v k=" '$have':" ' $0 == k { on = 1; next } on && /commit:/ { gsub(/.*sha1-|.$/, ""); print; exit } @@ -131,8 +152,14 @@ done if [ "$drift" -gt 0 ]; then echo "[drift] $drift drifted entry/entries across $checked workflow(s) in $REPO_DIR" >&2 echo "[drift] fix: run \`gh actions-lock\` then restore SPDX to line 1" >&2 + [ "$unexamined" -eq 0 ] || echo "[drift] also: $unexamined workflow(s) could not be parsed — not examined" >&2 exit 1 fi +if [ "$unexamined" -gt 0 ]; then + echo "[drift] $unexamined workflow(s) in $REPO_DIR could not be parsed — verdict withheld" >&2 + exit 2 +fi + echo "[drift] clean — $checked workflow(s) checked in $REPO_DIR" >&2 exit 0 diff --git a/scripts/tests/check-lockfile-drift-test.sh b/scripts/tests/check-lockfile-drift-test.sh new file mode 100755 index 000000000..8a24fce12 --- /dev/null +++ b/scripts/tests/check-lockfile-drift-test.sh @@ -0,0 +1,132 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 +# SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell (hyperpolymath) +# +# check-lockfile-drift-test.sh — fixture suite for scripts/check-lockfile-drift.sh, +# covering its YAML-syntax independence (YAML-POLICY Y-1 / Y-3). +# +# The script used to line-grep `uses:[[:space:]]*owner/repo@ref`. A KYAML +# workflow quotes the value (`uses: "owner/repo@ref",`), so the grep matched +# nothing and a drifted KYAML workflow scanned CLEAN. It also read `uses:` text +# inside a `run:` body as a ref, and called an unparseable workflow clean. +# Each case runs the script on a fixture repo; exit 0 clean, 1 drift, 2 withheld. +# +# Run: bash scripts/tests/check-lockfile-drift-test.sh +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +DRIFT="${DRIFT:-$SCRIPT_DIR/../check-lockfile-drift.sh}" +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +LOCKED=3d3c42e5aac5ba805825da76410c181273ba90b1 +WF="$WORK/repo/.github/workflows" +mkdir -p "$WF" +cat > "$WF/actions.lock" < — writes stdin to ci.yml, runs the script +# on the fixture repo, and checks its exit code and, unless "-", an output substring. +expect() { + local label=$1 want=$2 needle=$3 out rc + cat > "$WF/ci.yml" + out="$(bash "$DRIFT" "$WORK/repo" fixture 2>&1)"; rc=$? + if [ "$rc" -ne "$want" ]; then + echo "FAIL: $label — expected exit $want, got $rc" + printf '%s\n' "$out" | sed 's/^/ | /' + fail=$((fail + 1)); return + fi + if [ "$needle" != "-" ] && ! printf '%s' "$out" | /usr/bin/grep -qF -- "$needle"; then + echo "FAIL: $label — exit $rc correct, but output lacked '$needle'" + printf '%s\n' "$out" | sed 's/^/ | /' + fail=$((fail + 1)); return + fi + echo "PASS: $label"; pass=$((pass + 1)) +} + +expect "block control: the locked tag is clean" 0 "clean" <<'EOF' +jobs: + a: + steps: + - uses: actions/checkout@v7.0.1 +EOF + +expect "block control: the locked commit by SHA is clean" 0 "clean" < "$T/.github/workflows/branch.yml" printf 'name: local\non: push\njobs:\n a:\n steps:\n - uses: ./local-action\n - uses: ./.github/actions/signed-push\n' \ > "$T/.github/workflows/local.yml" -printf 'name: docker\non:\n a:\n container:\n image: node:20\n steps:\n - uses: docker://ghcr.io/org/img:1.2.3\n' \ +printf 'name: docker\non: push\njobs:\n a:\n container:\n image: node:20\n steps:\n - uses: docker://ghcr.io/org/img:1.2.3\n' \ > "$T/.github/workflows/docker.yml" printf 'name: short\non: push\njobs:\n a:\n steps:\n - uses: actions/checkout@abc123 # not a full SHA\n' \ > "$T/.github/workflows/short.yml" @@ -77,5 +77,57 @@ ckf "vendored RSR mirror is scoped out (see Debtfile)" 0 "$T/vend" mkdir -p "$T/empty" ckf "refuses to certify a tree with 0 workflow files" 1 "$T/empty" +# YAML-POLICY Y-1: refs come from the parser, not a line grep. The old grep +# false-failed a quoted local ref, never saw a one-line flow step, read `uses:` +# text inside a `run:` body as a ref, and passed a file it could not parse. +cat > "$T/.github/workflows/kyaml.yml" < "$T/.github/workflows/kyaml-mutant.yml" < "$T/.github/workflows/quoted-local.yml" +printf 'name: flow\non: push\njobs:\n a:\n steps:\n - { uses: actions/checkout@v7.0.1 }\n' \ + > "$T/.github/workflows/flow-step.yml" +printf 'name: decoy\non: push\njobs:\n a:\n steps:\n - uses: actions/checkout@%s # v7.0.1\n - run: |\n echo "uses: actions/cache@v6.1.0"\n' "$S40" \ + > "$T/.github/workflows/decoy.yml" +printf '{\n jobs: {\n a: { steps: [ { uses: "actions/checkout@v7.0.1" },\n' \ + > "$T/.github/workflows/broken.yml" +# The quote closes on a later line, so the file PARSES as one `name` scalar with +# no jobs (measured: 007-lang oracle-fuzz.yml). GitHub cannot run it. +printf 'name: "CHECK: swallowed\non: push\njobs:\n a:\n steps:\n - uses: actions/checkout@v7.0.1\n - run: echo hi"\n' \ + > "$T/.github/workflows/swallowed.yml" + +ck "KYAML: pinned + quoted local ref PASSES" 0 ".github/workflows/kyaml.yml" +ck "KYAML mutant: an unpinned flow ref FAILS" 1 ".github/workflows/kyaml-mutant.yml" +ck "quoted local \"./x\" is a local ref, not unpinned" 0 ".github/workflows/quoted-local.yml" +ck "one-line { uses: } flow step with a tag FAILS" 1 ".github/workflows/flow-step.yml" +ck "uses: text inside a run: body is not a ref" 0 ".github/workflows/decoy.yml" +ck "unparseable workflow FAILS closed" 1 ".github/workflows/broken.yml" +ck "quote-swallowed workflow with no jobs FAILS closed" 1 ".github/workflows/swallowed.yml" + printf '\n%s passed, %s failed\n' "$pass" "$fail" [ "$fail" -eq 0 ]