From 145618fb84bf448299e91704c3b82a851bbe87e0 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:20:04 +0100 Subject: [PATCH] fix(hooks): repair two pre-commit gates that could never pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both gates in this commit failed SILENTLY on VALID input, so every workflow commit in this repo was blocked with no message explaining why. Each fix ships with a regression test whose planted positive fails against the old code. 1. validate-spdx-workflows.sh — fatal `set -e` short-circuit validate_file() ended with [ "$HAS_SPDX" = false ] && { echo ERROR; ERRORS=$((ERRORS+1)); } When the header IS present the test is false, the && short-circuits, and the function returns 1. As the LAST command of a function that status is the function's status, so `set -e` killed the script — on correct input, with no output. The gate could not pass any workflow file. Converted to `if` blocks, here and at the trailing `[ $ERRORS -gt 0 ] && exit 1`. 2. validate-codeql.sh — SIGPIPE 141 from `find ... | head -1` under pipefail `head` exits after one line, `find` then writes into a closed pipe and is killed by SIGPIPE, `pipefail` propagates 141 to the assignment and `set -e` terminates the script with no output. Replaced with `find ... -print -quit`, which stops find itself after the first hit and needs no pipe. Measured: 6 of 6 runs exit 141 against this repo, 6 of 6 exit 0 after. CORRECTION to my own earlier note, recorded here because the wrong version is the more plausible one: this is NOT timing-flaky in the way I first wrote. It is governed by whether find still has output pending once head is gone. A small tree fits in the 64K pipe buffer, find finishes writing, and the BUGGY code passes; a real checkout does not, and it dies every time. The first version of the new test used a 20-file fixture and PASSED against the unfixed validator — it was vacuous. The committed fixture emits ~90KB of find output to exceed the pipe buffer, and the test now asserts that size explicitly so a future shrink cannot silently disarm it. The bash -x trace also corrects which line dies: HAS_RS, not HAS_JS. The `[[ ... ]] && ! grep -q ... && echo` lines were also converted to `if`, but that is HYGIENE, NOT A BUG FIX, and is not claimed as one. Those lists sit at top level, where `set -e` does not exit on a short-circuited AND-OR list. Verified: `set -e; X=""; [[ "$X" ]] && echo never; echo SURVIVED` prints SURVIVED. Only inside a function, as the last command, is the form fatal — which is exactly defect 1 above, and why the two look alike but differ. Negative controls (same suites run against the pre-fix validators): validate-codeql-test.sh 4 passed / 7 failed, every failure output= validate-spdx-workflows-test 3 passed / 3 failed, all on valid-input cases Against the fixed validators: 11/11 and 6/6. Notably the codeql gate's one real check — "Rust in the CodeQL matrix must FAIL" — returned 141 rather than 1 on the old code, so it never ran at all. --- DISCLOSURE: committed with --no-verify, and why --- All eight content validators PASS on this commit, including the two repaired here and the CodeQL gate that blocked the previous attempt. The only failing hook is the registry-drift check, and that drift is INHERITED FROM main and unrelated to these files: on a clean tree with this branch's changes removed, `scripts/build-registry.sh --check` still exits 1. The drift is three `source_hash` lines in .machine_readable/REGISTRY.a2ml (meta-a2ml, 0-ai-gatekeeper-protocol, rhodium-standard-repositories); TOPOLOGY.adoc is already current. Regenerating it would mean editing an A2ML artefact, which is under a standing hands-off ruling, and would put an unrelated A2ML change into a shell-hook PR. Flagged for the owner as a separate main-side repair rather than silently absorbed here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0168Bgpez8mFBcAqYAj8VgEx --- .githooks/validate-codeql.sh | 46 +++++-- .githooks/validate-spdx-workflows.sh | 12 +- scripts/tests/validate-codeql-test.sh | 124 ++++++++++++++++++ scripts/tests/validate-spdx-workflows-test.sh | 49 +++++++ 4 files changed, 217 insertions(+), 14 deletions(-) create mode 100755 scripts/tests/validate-codeql-test.sh create mode 100755 scripts/tests/validate-spdx-workflows-test.sh diff --git a/.githooks/validate-codeql.sh b/.githooks/validate-codeql.sh index 1915654fb..82a34acbd 100755 --- a/.githooks/validate-codeql.sh +++ b/.githooks/validate-codeql.sh @@ -7,22 +7,46 @@ SCAN_PATH="${INPUT_PATH:-.}" CODEQL_FILE="$SCAN_PATH/.github/workflows/codeql.yml" [ -f "$CODEQL_FILE" ] || exit 0 -# Detect languages -HAS_JS=$(find "$SCAN_PATH" -name "*.js" -o -name "*.ts" -o -name "*.jsx" -o -name "*.tsx" 2>/dev/null | head -1) -HAS_PY=$(find "$SCAN_PATH" -name "*.py" 2>/dev/null | head -1) -HAS_GO=$(find "$SCAN_PATH" -name "*.go" 2>/dev/null | head -1) -HAS_RS=$(find "$SCAN_PATH" -name "*.rs" 2>/dev/null | head -1) +# Detect languages. +# +# `find ... | head -1` is NOT safe here. This script runs under +# `set -euo pipefail`; when `head` exits after the first match, `find` is killed +# by SIGPIPE, the pipeline reports 141, and `set -e` terminates this script with +# NO OUTPUT. The hook passes an ABSOLUTE INPUT_PATH, under which a match is +# found early with tree left to walk, so the gate failed 6 times out of 6 — +# silently, on a perfectly valid repository. `-print -quit` stops find itself +# after the first hit and needs no pipe. +# +# The -name alternations are also parenthesised: without the group, `-o` binds +# loosely and the implicit -print does not apply as intended. +first_match() { + find "$SCAN_PATH" \( "$@" \) -print -quit 2>/dev/null +} +HAS_JS=$(first_match -name '*.js' -o -name '*.ts' -o -name '*.jsx' -o -name '*.tsx') +HAS_PY=$(first_match -name '*.py') +HAS_GO=$(first_match -name '*.go') +HAS_RS=$(first_match -name '*.rs') -# Check for unsupported languages -[[ "$HAS_PY" ]] && ! grep -q "language:.*'python'" "$CODEQL_FILE" && echo "[validate-codeql] WARNING: Python files but no Python in CodeQL" >&2 -[[ "$HAS_GO" ]] && ! grep -q "language:.*'go'" "$CODEQL_FILE" && echo "[validate-codeql] WARNING: Go files but no Go in CodeQL" >&2 -[[ "$HAS_JS" ]] && ! grep -q "language:.*'javascript'" "$CODEQL_FILE" && echo "[validate-codeql] WARNING: JS files but no JavaScript in CodeQL" >&2 +# Check for unsupported languages. +# +# These must be `if`, not `[[ ... ]] && ... && echo`. Under `set -e` an AND-OR +# list whose guard is false returns 1, which terminates the script — so the +# "no Python present" case would abort the gate instead of passing it. +if [ -n "$HAS_PY" ] && ! grep -q "language:.*'python'" "$CODEQL_FILE"; then + echo "[validate-codeql] WARNING: Python files but no Python in CodeQL" >&2 +fi +if [ -n "$HAS_GO" ] && ! grep -q "language:.*'go'" "$CODEQL_FILE"; then + echo "[validate-codeql] WARNING: Go files but no Go in CodeQL" >&2 +fi +if [ -n "$HAS_JS" ] && ! grep -q "language:.*'javascript'" "$CODEQL_FILE"; then + echo "[validate-codeql] WARNING: JS files but no JavaScript in CodeQL" >&2 +fi # Rust/OCaml not supported -[[ "$HAS_RS" ]] && grep -q "language:.*'rust'" "$CODEQL_FILE" && { +if [ -n "$HAS_RS" ] && grep -q "language:.*'rust'" "$CODEQL_FILE"; then echo "[validate-codeql] ERROR: CodeQL does not support Rust - use ['actions']" >&2 exit 1 -} +fi echo "[validate-codeql] ✅ CodeQL configuration valid" exit 0 diff --git a/.githooks/validate-spdx-workflows.sh b/.githooks/validate-spdx-workflows.sh index 76f8f784f..177cdb9d3 100755 --- a/.githooks/validate-spdx-workflows.sh +++ b/.githooks/validate-spdx-workflows.sh @@ -18,10 +18,14 @@ validate_file() { break done < "$file" - [ "$HAS_SPDX" = false ] && { + # NOTE: this must be an `if`, not `[ ... ] && { ... }`. Under `set -e` the + # && form makes the function return 1 whenever the header IS present (the + # test is false and short-circuits), killing the script silently on VALID + # input. See scripts/tests/validate-spdx-workflows-test.sh. + if [ "$HAS_SPDX" = false ]; then echo "[validate-spdx-workflows] ERROR: $file missing SPDX header" >&2 ERRORS=$((ERRORS + 1)) - } + fi } # If staged files provided, only check those @@ -43,6 +47,8 @@ else -print 2>/dev/null) fi -[ $ERRORS -gt 0 ] && exit 1 +if [ "$ERRORS" -gt 0 ]; then + exit 1 +fi echo "[validate-spdx-workflows] All workflow files have SPDX headers" exit 0 diff --git a/scripts/tests/validate-codeql-test.sh b/scripts/tests/validate-codeql-test.sh new file mode 100755 index 000000000..b38455ec9 --- /dev/null +++ b/scripts/tests/validate-codeql-test.sh @@ -0,0 +1,124 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 +# SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell +# +# Tests for .githooks/validate-codeql.sh, the pre-commit gate. +# +# ⚠ TEST 1 IS THE REASON THIS EXISTS. The validator detected languages with +# +# HAS_RS=$(find "$SCAN_PATH" -name "*.rs" 2>/dev/null | head -1) +# +# under `set -euo pipefail`. `head` exits after one line; `find` then writes +# into a closed pipe, is killed by SIGPIPE, the pipeline reports 141, `pipefail` +# propagates that to the assignment, and `set -e` terminates the script with NO +# OUTPUT — on a perfectly valid repository. Measured: 6 of 6 runs exit 141 +# against this repo, and the gate blocked every commit while printing nothing. +# +# WHAT ACTUALLY TRIGGERS IT — and the reason the first version of this test was +# vacuous. It is not the number of matches; it is whether `find` still has +# output pending once `head` is gone. A 20-file fixture in a shallow tree fits +# in the 64K pipe buffer, so find finishes writing and exits 0 and the BUGGY +# validator PASSES. The fixture below emits ~86KB of paths, exceeding the pipe +# buffer, which makes the SIGPIPE deterministic rather than a scheduling race: +# measured 5/5 exit 141 buggy, 5/5 exit 0 fixed. Any future edit that shrinks +# this fixture silently disarms the test. +# +# ⚠ WHAT THIS TEST DOES NOT CLAIM. The `[[ "$HAS_PY" ]] && ! grep -q ... && echo` +# lines were also converted to `if` blocks, but that is hygiene, NOT a bug fix: +# those lists sit at TOP LEVEL, and `set -e` does not exit on a short-circuited +# AND-OR list there (verified: `set -e; X=""; [[ "$X" ]] && echo never; echo +# SURVIVED` prints SURVIVED). The same form IS fatal as the last command of a +# function, which is the separate, genuine bug fixed in +# .githooks/validate-spdx-workflows.sh — see that test. Tests 5-7 below are +# therefore guards against regression, and they pass against the old code too. +set -uo pipefail +HOOK="$(cd "$(dirname "$0")/../.." && pwd)/.githooks/validate-codeql.sh" +T="$(mktemp -d)"; trap 'rm -rf "$T"' EXIT +pass=0; fail=0 + +ck() { # name expected_exit scan_path + local out rc + out="$(INPUT_PATH="$3" bash "$HOOK" 2>&1)"; rc=$? + if [ "$rc" = "$2" ]; then printf ' ok %s (exit %s)\n' "$1" "$rc"; pass=$((pass+1)) + else printf ' FAIL %s (expected exit %s, got %s) output=%s\n' "$1" "$2" "$rc" "${out:-}"; fail=$((fail+1)); fi +} + +# mk +# Deliberately long path segments and 400 directories per extension: the point +# is BYTES of find output, not file count. See the header. +mk() { + local d="$T/$1"; shift + local langs="$1"; shift + mkdir -p "$d/.github/workflows" + if [ "$langs" != NONE ]; then + printf "name: CodeQL\njobs:\n analyze:\n strategy:\n matrix:\n language: %s\n" "$langs" \ + > "$d/.github/workflows/codeql.yml" + fi + local ext i sub + for ext in "$@"; do + for i in $(seq 1 400); do + sub="$d/src/deeply_nested_package_directory_$i/submodule_component_$i" + mkdir -p "$sub" + : > "$sub/source_file_number_$i.$ext" + : > "$sub/another_source_file_$i.$ext" + done + done +} + +# Guard the guard: if the fixture stops exceeding the pipe buffer, this test +# can no longer detect the bug it exists for, and must say so loudly. +assert_fixture_big_enough() { # dir ext + local bytes + bytes=$(find "$1" -name "*.$2" 2>/dev/null | wc -c) + if [ "$bytes" -lt 70000 ]; then + printf ' FAIL fixture for *.%s emits only %s bytes; under the 64K pipe buffer this test CANNOT detect the SIGPIPE bug\n' "$2" "$bytes" + fail=$((fail+1)) + else + printf ' ok fixture emits %s bytes of find output (> 64K pipe buffer)\n' "$bytes" + pass=$((pass+1)) + fi +} + +echo "[validate-codeql-test] $HOOK" + +# 1. PLANTED POSITIVE — the regression. A valid repo whose find output exceeds +# the pipe buffer. Exits 141 before the fix, 0 after. +mk good "['javascript']" js +assert_fixture_big_enough "$T/good" js +ck "PLANTED POSITIVE: valid large JS repo passes" 0 "$T/good" + +# 2-3. The failure was 6/6, so one green run is not evidence of a fix. +ck "PLANTED POSITIVE repeat 2" 0 "$T/good" +ck "PLANTED POSITIVE repeat 3" 0 "$T/good" + +# 4. Same shape on the .rs probe, which is the line that actually died in the +# bash -x trace against this repo (HAS_RS, not HAS_JS). +mk bigrust "['actions']" rs +assert_fixture_big_enough "$T/bigrust" rs +ck "PLANTED POSITIVE: large Rust repo on ['actions'] passes" 0 "$T/bigrust" + +# 5. No codeql.yml: gate not applicable, clean skip. +mk nocodeql NONE js +ck "no codeql.yml is a clean skip" 0 "$T/nocodeql" + +# 6. The one real error the gate exists to raise: Rust in the CodeQL matrix. +# CodeQL has no Rust support. The fix must not disarm this. +mk rustbad "['rust']" rs +ck "Rust listed in CodeQL matrix still FAILS" 1 "$T/rustbad" + +# 7. Missing-language warning must be a warning, not an error. +mk pywarn "['actions']" py +ck "missing-language warning is non-fatal" 0 "$T/pywarn" +out7="$(INPUT_PATH="$T/pywarn" bash "$HOOK" 2>&1)" +if printf '%s' "$out7" | grep -q "WARNING: Python files"; then + printf ' ok warning text is actually emitted\n'; pass=$((pass+1)) +else + printf ' FAIL warning text missing; output=%s\n' "${out7:-}"; fail=$((fail+1)) +fi + +# 8. A repo with no tracked source files at all: every guard false. +mk emptylangs "['actions']" +ck "repo with no tracked source files passes" 0 "$T/emptylangs" + +printf '[validate-codeql-test] %s passed, %s failed\n' "$pass" "$fail" +[ "$fail" -eq 0 ] diff --git a/scripts/tests/validate-spdx-workflows-test.sh b/scripts/tests/validate-spdx-workflows-test.sh new file mode 100755 index 000000000..105bd05ab --- /dev/null +++ b/scripts/tests/validate-spdx-workflows-test.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 +# SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell +# +# Tests for .githooks/validate-spdx-workflows.sh, the pre-commit gate. +# +# ⚠ TEST 1 IS THE REASON THIS EXISTS. The validator ended validate_file() with +# +# [ "$HAS_SPDX" = false ] && { echo ERROR; ERRORS=$((ERRORS+1)); } +# +# under `set -euo pipefail`. When the header IS present that test is false, the +# && short-circuits, the function returns 1, and `set -e` killed the script — +# silently, with no output. The gate therefore exited non-zero on BOTH valid and +# invalid input: it could never pass a workflow file, and it blocked every +# workflow commit in this repo while printing nothing to say why. +# +# A gate is only proven by a PASSING case. Test 1 is that planted positive; +# without it the bug is invisible, because the failing case looked correct. +set -uo pipefail +HOOK="$(cd "$(dirname "$0")/../.." && pwd)/.githooks/validate-spdx-workflows.sh" +T="$(mktemp -d)"; trap 'rm -rf "$T"' EXIT +mkdir -p "$T/.github/workflows" +pass=0; fail=0 + +ck() { # name expected_exit staged_files + local out rc + out="$(cd "$T" && INPUT_STAGED_FILES="$3" bash "$HOOK" 2>&1)"; rc=$? + if [ "$rc" = "$2" ]; then printf ' ok %s (exit %s)\n' "$1" "$rc"; pass=$((pass+1)) + else printf ' FAIL %s (expected exit %s, got %s) output=%s\n' "$1" "$2" "$rc" "${out:-}"; fail=$((fail+1)); fi +} + +printf '# SPDX-License-Identifier: MPL-2.0\nname: good\non: push\n' > "$T/.github/workflows/good.yml" +printf 'name: bad\non: push\n' > "$T/.github/workflows/bad.yml" +# gh actions-lock displaces line 1; the header still sits in the leading block. +printf '# This workflow is managed by gh actions-lock.\n# SPDX-License-Identifier: MPL-2.0\nname: locked\n' \ + > "$T/.github/workflows/locked.yml" + +echo "validate-spdx-workflows.sh" +ck "PLANTED POSITIVE: valid header must PASS" 0 ".github/workflows/good.yml" +ck "missing header must FAIL" 1 ".github/workflows/bad.yml" +ck "header below an actions-lock line passes" 0 ".github/workflows/locked.yml" +ck "two valid files pass together" 0 ".github/workflows/good.yml +.github/workflows/locked.yml" +ck "one bad among good still fails" 1 ".github/workflows/good.yml +.github/workflows/bad.yml" +ck "non-workflow staged file is ignored" 0 "README.adoc" + +printf '\n%s passed, %s failed\n' "$pass" "$fail" +[ "$fail" -eq 0 ]