From 70ac845ba539171c7f517e114dac49085942ae56 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Thu, 1 Oct 2026 18:54:06 +0100 Subject: [PATCH 1/4] ci(gates): read workflow uses/permissions with yq, not grep YAML-POLICY Y-1: a verdict about a workflow must come from a parser. Each of these gates read `uses:`/`permissions:` by line grep, which only sees block style. On a KYAML workflow they either falsely failed (the quote and comma were captured into the ref) or went blind (a pin inside `{ uses: ... }` was never seen): - validate-actions-lock.sh, lock-selfcheck.sh, check-action-pins-resolve.sh, update-actions-lock.sh: refs come from yq; an unparseable file fails closed instead of contributing zero refs. - check-workflow-duplicate-keys.sh: flow documents are normalised with yq -P before the line scanner runs (yq keeps duplicates, so the scanner still sees them). - governance-reusable.yml: top-level permissions via yq has("permissions"), with a warned grep fallback on a runner without yq. - Mustfile actions-sha-pinned: optional quote/comma in the pattern. - lock-selfcheck.sh: its existing MPL-2.0 SPDX line moves from line 41 to line 2 so the staged SPDX hook sees it (identifier unchanged). Known answers on main's block-style tree: identical ref sets for validate-actions-lock (29) and lock-selfcheck (111 pairs); check-action-pins-resolve drops exactly two `# uses:` comment examples that never execute. New KYAML cases and mutants fail against the old scripts and pass against the new ones. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W5CoaksP2Bg21HpDCgFgwS --- .githooks/validate-actions-lock.sh | 31 ++++- .github/workflows/governance-reusable.yml | 18 ++- .../contractiles/must/Mustfile.a2ml | 3 +- scripts/check-action-pins-resolve.sh | 28 +++- scripts/check-workflow-duplicate-keys.sh | 37 +++++- scripts/lock-selfcheck.sh | 25 +++- .../tests/check-action-pins-resolve-test.sh | 61 ++++++++- .../check-workflow-duplicate-keys-test.sh | 51 ++++++++ scripts/tests/lock-selfcheck-test.sh | 117 +++++++++++++++++ scripts/tests/update-actions-lock-test.sh | 48 +++++++ scripts/tests/validate-actions-lock-test.sh | 121 ++++++++++++++++++ scripts/update-actions-lock.sh | 27 ++-- 12 files changed, 533 insertions(+), 34 deletions(-) create mode 100755 scripts/tests/lock-selfcheck-test.sh create mode 100755 scripts/tests/validate-actions-lock-test.sh diff --git a/.githooks/validate-actions-lock.sh b/.githooks/validate-actions-lock.sh index 8f2d56a47..734dec811 100755 --- a/.githooks/validate-actions-lock.sh +++ b/.githooks/validate-actions-lock.sh @@ -113,13 +113,34 @@ CHECKED=0 declare -a SEEN_ABSENT=() # Collect every SHA-pinned uses: ref across all workflow files. +# +# Read with yq, never grep (YAML-POLICY Y-1). A line grep for `uses:` only +# sees block style: on a KYAML file (`uses: "owner/repo@sha", # v1`) it +# captured the quote and comma into the ref and reported a pinned, locked +# action as missing. The parser returns the scalar VALUE whatever the style, +# and never matches a `uses:` that is text inside a `run:` body. +# Measured 2026-10-01: same 29-ref set as the old grep on the block tree. +if ! command -v yq >/dev/null 2>&1; then + echo -e "${RED}[validate-actions-lock] ERROR: yq not found -- it reads the workflows (YAML-POLICY Y-1)${NC}" >&2 + exit 1 +fi +declare -a UNPARSED=() mapfile -t RAW < <( - grep -rhoE '^[[:space:]]*(-[[:space:]]+)?uses:[[:space:]]*[^[:space:]#]+@[0-9a-fA-F]{40}' \ - "$WORKFLOW_DIR"/*.yml "$WORKFLOW_DIR"/*.yaml \ - "$ACTIONS_DIR"/*/action.yml "$ACTIONS_DIR"/*/action.yaml 2>/dev/null \ - | sed -E 's/^[[:space:]]*(-[[:space:]]+)?uses:[[:space:]]*//' \ - | sort -u + for f in "$WORKFLOW_DIR"/*.yml "$WORKFLOW_DIR"/*.yaml \ + "$ACTIONS_DIR"/*/action.yml "$ACTIONS_DIR"/*/action.yaml; do + [ -f "$f" ] || continue + yq -r '.. | select(tag == "!!map") | select(has("uses")) | .uses | select(tag == "!!str")' "$f" \ + || printf '\001UNPARSED\001%s\n' "$f" + done | grep -E $'@[0-9a-fA-F]{40}$|^\001UNPARSED\001' | sort -u ) +# A file yq cannot parse is a file whose refs went unchecked: fail closed. +for ref in "${RAW[@]}"; do + case "$ref" in $'\001UNPARSED\001'*) UNPARSED+=("${ref#$'\001UNPARSED\001'}") ;; esac +done +if [ "${#UNPARSED[@]}" -gt 0 ]; then + echo -e "${RED}[validate-actions-lock] ERROR: yq could not parse: ${UNPARSED[*]}${NC}" >&2 + exit 1 +fi # A zero-input pass is the classic fake green: if ref extraction ever breaks, # this script would report success having checked nothing. If the lockfile diff --git a/.github/workflows/governance-reusable.yml b/.github/workflows/governance-reusable.yml index 0f13910e7..70626f399 100644 --- a/.github/workflows/governance-reusable.yml +++ b/.github/workflows/governance-reusable.yml @@ -1298,6 +1298,16 @@ jobs: - name: Check SPDX headers + permissions run: | failed=0 + # The permissions verdict is read with yq (YAML-POLICY Y-1): the old + # `grep -q "^permissions:"` FALSELY FAILED every KYAML workflow, + # whose keys sit indented inside `{ … }`. yq ships on GitHub-hosted + # Ubuntu; a runner without it (inputs.runs-on) keeps the old line + # test, which is still right for block-style files, and says so. + have_yq=1 + command -v yq >/dev/null 2>&1 || { + have_yq=0 + echo "::warning::yq not on this runner -- permissions read by line grep; a KYAML workflow will be misreported" + } for file in .github/workflows/*.yml .github/workflows/*.yaml; do [ -f "$file" ] || continue # ⚠ SCAN THE HEADER BLOCK, NOT LINE 1. REUSE places the identifier @@ -1317,7 +1327,13 @@ jobs: | grep -q "^# SPDX-License-Identifier:"; then echo "ERROR: $file has no SPDX-License-Identifier in its header comment block"; failed=1 fi - if ! grep -q "^permissions:" "$file"; then + if [ "$have_yq" -eq 1 ]; then + if ! verdict="$(yq 'has("permissions")' "$file" 2>&1)"; then + echo "ERROR: $file is not parseable as YAML: $verdict"; failed=1 + elif [ "$verdict" != "true" ]; then + echo "ERROR: $file missing top-level 'permissions:' declaration"; failed=1 + fi + elif ! grep -q "^permissions:" "$file"; then echo "ERROR: $file missing top-level 'permissions:' declaration"; failed=1 fi done diff --git a/.machine_readable/contractiles/must/Mustfile.a2ml b/.machine_readable/contractiles/must/Mustfile.a2ml index f3a923d7d..4d018d81a 100644 --- a/.machine_readable/contractiles/must/Mustfile.a2ml +++ b/.machine_readable/contractiles/must/Mustfile.a2ml @@ -95,7 +95,8 @@ requirements — CI and pre-commit hooks fail if any check fails. ### actions-sha-pinned - description: every GitHub Action is pinned to a 40-char commit SHA -- run: ! grep -rEn 'uses:[[:space:]]+[^@]+@(v?[0-9.]+|main|master)([[:space:]]|$)' .github/workflows/ 2>/dev/null +- run: ! grep -rEn 'uses:[[:space:]]+"?[^@"]+@(v?[0-9.]+|main|master)"?,?([[:space:]]|$)' .github/workflows/ 2>/dev/null +- notes: The optional quote and trailing comma match KYAML (YAML-POLICY Y-3), whose values are always quoted; without them a tag pin in a KYAML workflow passed unseen. - severity: critical ### jobs-have-timeout diff --git a/scripts/check-action-pins-resolve.sh b/scripts/check-action-pins-resolve.sh index 70c9ba628..580794735 100755 --- a/scripts/check-action-pins-resolve.sh +++ b/scripts/check-action-pins-resolve.sh @@ -95,10 +95,32 @@ fi # Skips local (`./`) and docker:// refs, which have no upstream commit. # kind = R: the ref points at a reusable workflow file (.github/workflows/*.yml # in the repo) — those get the ancestry probe (see header); kind = A otherwise. +# +# Values come from the YAML parser, not a line grep (YAML-POLICY Y-1): a grep +# for `uses:` extracted NOTHING from a KYAML workflow (the value is quoted), +# so its pins went unchecked while the script reported success. The parser +# also stops matching `# uses: …` usage examples in header comments, which +# never execute. Measured 2026-10-01 on main: 28 refs, the grep's 30 minus +# exactly those two comment examples. +if ! command -v yq >/dev/null 2>&1; then + echo "::error::yq not found -- it reads the workflows (YAML-POLICY Y-1)" >&2 + exit 1 +fi +unparsed="" +for wf in "$WORKFLOW_DIR"/*.yml "$WORKFLOW_DIR"/*.yaml; do + [ -f "$wf" ] || continue + yq '.' "$wf" >/dev/null 2>&1 || unparsed="$unparsed $wf" +done +if [ -n "$unparsed" ]; then + echo "::error::yq could not parse:$unparsed -- their pins would go unchecked" >&2 + exit 1 +fi pairs="$( - grep -rhoE '\buses:[[:space:]]*[A-Za-z0-9_.-]+/[A-Za-z0-9_./-]+@[0-9a-f]{40}' \ - "$WORKFLOW_DIR" 2>/dev/null \ - | sed -E 's/.*uses:[[:space:]]*//' \ + for wf in "$WORKFLOW_DIR"/*.yml "$WORKFLOW_DIR"/*.yaml; do + [ -f "$wf" ] || continue + yq -r '.. | select(tag == "!!map") | select(has("uses")) | .uses | select(tag == "!!str")' "$wf" + done \ + | grep -E '^[A-Za-z0-9_.-]+/[A-Za-z0-9_./-]+@[0-9a-f]{40}$' \ | awk -F'@' '{ split($1, p, "/"); k = ($1 ~ /\.github\/workflows\/[^\/]+\.ya?ml$/) ? "R" : "A"; print p[1] "/" p[2] "\t" $2 "\t" k }' \ | sort -u )" diff --git a/scripts/check-workflow-duplicate-keys.sh b/scripts/check-workflow-duplicate-keys.sh index a527697e1..e8de2db4a 100755 --- a/scripts/check-workflow-duplicate-keys.sh +++ b/scripts/check-workflow-duplicate-keys.sh @@ -111,10 +111,43 @@ for t in "${targets[@]}"; do fi done +# Succeed when the file is a flow-style (KYAML) document: its first line that is +# not blank, a comment or a `---` marker opens a `{` mapping or `[` sequence. +is_flow_document() { + awk ' + { line = $0; sub(/\r$/, "", line); sub(/^[ \t]+/, "", line) } + line == "" || substr(line, 1, 1) == "#" || line ~ /^---[ \t]*$/ { next } + { exit (substr(line, 1, 1) == "{" || substr(line, 1, 1) == "[") ? 0 : 1 } + END { if (NR == 0) exit 1 } + ' "$1" +} + +# WHY FLOW DOCUMENTS ARE NORMALISED FIRST. scan_one walks BLOCK structure by +# indentation; a KYAML file (YAML-POLICY Y-3) puts every sibling on its own +# line inside `{ … }`, so the walker sees each step's keys as repeats of the +# previous step's and reports phantom duplicates (14 on the provisioning pilot, +# measured 2026-10-01). `yq -P` rewrites flow as block while KEEPING duplicate +# keys (it works on the node tree, not a map), so the unchanged scanner then +# answers the same question it answers for block files. Reported line numbers +# refer to that normalised form, and the message says so. failed=0 +norm="$(mktemp)" +trap 'rm -f "$norm"' EXIT for f in "${files[@]}"; do - out="$(scan_one "$f")" || { - detail="$(printf '%s' "$out" | awk -F'|' '{printf "%s\x27%s\x27 (line %s)", sep, $1, $2; sep=", "}')" + src="$f" where="" + if is_flow_document "$f"; then + if ! yq -P '.' "$f" > "$norm" 2> "$norm.err"; then + echo "::error file=${f}::not parseable as YAML: $(head -c 300 "$norm.err")" + echo "FAIL ${f}: not parseable as YAML (yq -P): $(head -c 300 "$norm.err")" + rm -f "$norm.err" + failed=$((failed + 1)) + continue + fi + rm -f "$norm.err" + src="$norm" where=" of the block-normalised form (yq -P)" + fi + out="$(scan_one "$src")" || { + detail="$(printf '%s' "$out" | awk -F'|' -v w="$where" '{printf "%s\x27%s\x27 (line %s%s)", sep, $1, $2, w; sep=", "}')" echo "::error file=${f}::duplicate key(s): ${detail}" echo "FAIL ${f}: duplicate key(s): ${detail}" failed=$((failed + 1)) diff --git a/scripts/lock-selfcheck.sh b/scripts/lock-selfcheck.sh index 9380078b8..aa8d3562b 100755 --- a/scripts/lock-selfcheck.sh +++ b/scripts/lock-selfcheck.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 # lock-selfcheck.sh — is a given `standards` commit SAFE TO PIN A CALLER TO? # # WHY THIS EXISTS @@ -38,7 +39,6 @@ # (a garbage-collected commit is the general case). Reachability is a # SEPARATE probe and must be made against the remote. # -# SPDX-License-Identifier: MPL-2.0 set -uo pipefail @@ -74,6 +74,12 @@ normalise_ref() { overall_rc=0 +# The workflows are read with yq (YAML-POLICY Y-1); without it nothing is examined. +if ! command -v yq >/dev/null 2>&1; then + echo "lock-selfcheck: yq not found -- it reads the workflows (YAML-POLICY Y-1)" >&2 + exit 2 +fi + for SHA in "$@"; do echo "==============================================================" if ! git -C "$STANDARDS_DIR" cat-file -e "${SHA}^{commit}" 2>/dev/null; then @@ -140,13 +146,18 @@ for SHA in "$@"; do : > "$TMP/missing" : > "$TMP/unkeyed_wf" : > "$TMP/scanned" + : > "$TMP/unparsed" while IFS= read -r wf; do [ -n "$wf" ] || continue git -C "$STANDARDS_DIR" show "${SHA}:${wf}" 2>/dev/null > "$TMP/wfbody" || continue - # Extract every `uses:` value, strip inline comments and quotes. - /usr/bin/grep -hoE '^[[:space:]]*(-[[:space:]]*)?uses:[[:space:]]*[^[:space:]#]+' "$TMP/wfbody" \ - | sed -E 's/.*uses:[[:space:]]*//; s/^["\x27]//; s/["\x27]$//' \ + # Extract every `uses:` VALUE with the YAML parser (YAML-POLICY Y-1). A + # line grep only sees block style: on a KYAML workflow it captured + # `…@sha",` and reported a keyed ref as POISON. A file yq cannot parse is + # recorded and fails the SHA below -- its refs went unexamined. + # Measured 2026-10-01 on main: same 111 (workflow, ref) pairs as the grep. + { yq -r '.. | select(tag == "!!map") | select(has("uses")) | .uses | select(tag == "!!str")' "$TMP/wfbody" \ + || printf '%s\n' "$wf" >> "$TMP/unparsed"; } \ | while IFS= read -r ref; do [ -n "$ref" ] || continue case "$ref" in @@ -196,7 +207,11 @@ for SHA in "$@"; do cut -f1 "$TMP/unkeyed_wf" | sort -u | sed 's/^/ /' fi - if [ "$n_missing" -eq 0 ]; then + if [ -s "$TMP/unparsed" ]; then + echo " VERDICT: UNEXAMINED — yq could not parse $(sort -u "$TMP/unparsed" | wc -l) workflow(s); their refs were not checked:" + sort -u "$TMP/unparsed" | sed 's/^/ /' + overall_rc=1 + elif [ "$n_missing" -eq 0 ]; then echo " VERDICT: SELF-CONSISTENT — every action ref used is keyed in this SHA's own lock." echo " (Reachability at the remote is NOT proven by this check.)" else diff --git a/scripts/tests/check-action-pins-resolve-test.sh b/scripts/tests/check-action-pins-resolve-test.sh index 30ae2acda..86b15eb8d 100755 --- a/scripts/tests/check-action-pins-resolve-test.sh +++ b/scripts/tests/check-action-pins-resolve-test.sh @@ -176,10 +176,10 @@ echo "== orphan reusable pins — the four #782 witness SHAs ==" # # The old predicate passed the first three, which is exactly the class this # gate now exists to fail on. -SHA_W1=7fdc27050000000000000000000000000000000000 -SHA_W2=892497fe0000000000000000000000000000000000 +SHA_W1=7fdc270500000000000000000000000000000000 +SHA_W2=892497fe00000000000000000000000000000000 SHA_W3=4696052100000000000000000000000000000000 -SHA_W4=5b1d00220000000000000000000000000000000000 +SHA_W4=5b1d002200000000000000000000000000000000 SHA_OK=81dbf2dd00000000000000000000000000000000 mk_reusable "$TMP/w1" "$SHA_W1" @@ -290,6 +290,61 @@ YAML STUB_COMMITS=200 \ expect "a subpath pin is resolved at the repository level" 0 "Checking 2 unique action pin(s)" "$TMP/dedup" +echo +echo "== YAML syntax independence (YAML-POLICY Y-1 / Y-3) ==" + +# A KYAML (flow-style) workflow quotes its values. The old line grep extracted +# NOTHING from it and reported "nothing to check" -- a dead pin sailed through. +rm -rf "$TMP/kyaml"; mkdir -p "$TMP/kyaml/.github/workflows" +cat > "$TMP/kyaml/.github/workflows/k.yml" < "$TMP/textual/.github/workflows/t.yml" < "$TMP/long/.github/workflows/l.yml" < "$TMP/broken/.github/workflows/b.yml" +expect "an unparseable workflow fails closed" 1 "yq could not parse" "$TMP/broken" + echo echo "check-action-pins-resolve regression: $pass passed, $fail failed" [ "$fail" -eq 0 ] diff --git a/scripts/tests/check-workflow-duplicate-keys-test.sh b/scripts/tests/check-workflow-duplicate-keys-test.sh index fc61b758a..c4cb6cac3 100755 --- a/scripts/tests/check-workflow-duplicate-keys-test.sh +++ b/scripts/tests/check-workflow-duplicate-keys-test.sh @@ -133,6 +133,57 @@ jobs: runs-on: ubuntu-latest YAML +echo +echo "== KYAML / flow-style workflows (YAML-POLICY Y-3) ==" + +# A KYAML file nests every key inside `{ … }`, so the indentation scanner sees +# one scope. These cases prove the flow path normalises it first: it must not +# go blind (miss a real duplicate) and must not cry wolf on sibling scopes. + +run_case "a clean KYAML workflow is clean" 0 "clean" <<'YAML' +# SPDX-License-Identifier: MPL-2.0 +{ + name: "demo", + jobs: { + a: { runs-on: "ubuntu-latest", steps: [{ run: "true" }] }, + b: { runs-on: "ubuntu-latest", steps: [{ run: "true" }] }, + }, +} +YAML + +run_case "a nested duplicate in a KYAML workflow is rejected" 1 "'runs-on'" <<'YAML' +{ + name: "demo", + jobs: { + build: { + runs-on: "ubuntu-latest", + runs-on: "ubuntu-24.04", + }, + }, +} +YAML + +run_case "a duplicate inside a one-line flow mapping is rejected" 1 "'runs-on'" <<'YAML' +{ + name: "demo", + jobs: { build: { runs-on: "ubuntu-latest", runs-on: "ubuntu-24.04" } }, +} +YAML + +run_case "a top-level duplicate in a KYAML workflow is rejected" 1 "duplicate key(s)" <<'YAML' +{ + name: "demo", + jobs: {}, + name: "demo again", +} +YAML + +run_case "an unparseable flow workflow fails closed" 1 "" <<'YAML' +{ + name: "demo", + jobs: { +YAML + echo echo "== directory scanning ==" diff --git a/scripts/tests/lock-selfcheck-test.sh b/scripts/tests/lock-selfcheck-test.sh new file mode 100755 index 000000000..a72b1bea9 --- /dev/null +++ b/scripts/tests/lock-selfcheck-test.sh @@ -0,0 +1,117 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 +# SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell (hyperpolymath) +# +# lock-selfcheck-test.sh — fixture suite for scripts/lock-selfcheck.sh, covering +# its YAML-syntax independence (YAML-POLICY Y-1 / Y-3). +# +# lock-selfcheck reads workflows at a commit and asks whether every action ref +# is keyed in that commit's own actions.lock. Its old line grep captured +# `…@sha",` from a KYAML workflow and called a self-consistent commit POISON. +# Each case is one commit in a throwaway repo, checked via STANDARDS_DIR. +# +# Run: bash scripts/tests/lock-selfcheck-test.sh +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SELFCHECK="${SELFCHECK:-$SCRIPT_DIR/../lock-selfcheck.sh}" +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +LOCKED=3d3c42e5aac5ba805825da76410c181273ba90b1 +UNLOCKED=1111111111111111111111111111111111111111 +REPO="$WORK/repo" +WF="$REPO/.github/workflows" + +mkdir -p "$WF" +git -C "$REPO" init -q -b main . +git -C "$REPO" config user.email t@example.com +git -C "$REPO" config user.name T +git -C "$REPO" config commit.gpgsign false +cat > "$WF/actions.lock" < "$WF/ci.yml" + git -C "$REPO" add -A + git -C "$REPO" commit -q -m fixture + git -C "$REPO" rev-parse HEAD +} + +# expect