diff --git a/.githooks/validate-spdx.sh b/.githooks/validate-spdx.sh index 14843162..53a194b9 100755 --- a/.githooks/validate-spdx.sh +++ b/.githooks/validate-spdx.sh @@ -7,37 +7,39 @@ SCAN_PATH="${INPUT_PATH:-.}" STAGED_FILES="${INPUT_STAGED_FILES:-}" ERRORS=0 -# The extension allowlist is the SINGLE source of truth for "is this a source -# file we require an SPDX header on". It MUST be applied in both modes: staged -# mode previously passed $STAGED_FILES through unfiltered, so any commit that -# touched a non-source file was judged by a rule written for source files. The -# machine-generated .github/workflows/actions.lock ("Do not edit by hand") -# carries no SPDX header and has any added header stripped on the next -# regeneration, so that omission blocked EVERY commit touching the lockfile -- -# which is why a Dependabot-caused lockfile desync could sit unrepaired. +# The single authority for "does this path need an SPDX header". +# +# ⚠ THIS FUNCTION EXISTS BECAUSE THE TWO MODES BELOW USED TO ASK DIFFERENT +# QUESTIONS. Full-scan mode filtered by source extension inside `find`; staged +# mode had NO filter at all (`FILES_TO_CHECK=$STAGED_FILES`) and so demanded a +# header on EVERY staged path. That is not a cosmetic split, because only the +# staged branch is reachable: this validator is called from .githooks/pre-commit +# and from no workflow at all, so the mode that HAD the filter never ran and the +# mode that ran had none. +# +# The concrete casualty was .github/workflows/actions.lock. It is generated by +# `gh actions-lock` ("Do not edit by hand"), carries no header, and is restamped +# on every regeneration — so a header added by hand does not survive. No commit +# touching the lockfile could pass pre-commit, which is why a Dependabot-caused +# lockfile desync could sit unrepaired long enough to recur (see #746). +# +# Keep ONE list. If a mode ever needs a different rule, that is a new function +# with a name saying so, never a second copy of these patterns. is_source_file() { case "$1" in - # NOTE: *.json is deliberately NOT here. The test below requires a '#' - # comment line, which JSON has no syntax for, so the rule was unsatisfiable - # by construction and never satisfied: both .machine_readable/*.json files - # already on main carry zero SPDX headers. Removing it restores truth - # rather than weakening the check. Same defect class as the staged-mode - # filter bug fixed in #804, one extension over. - *.rs|*.res|*.js|*.ts|*.sh|*.bash|*.zig|*.ex|*.exs|*.gleam) return 0 ;; - *.ml|*.mli|*.adb|*.ads|*.ncl|*.toml|*.yaml|*.yml) return 0 ;; + *.rs|*.res|*.js|*.ts|*.sh|*.bash|*.zig|*.ex|*.exs|*.gleam|\ + *.ml|*.mli|*.adb|*.ads|*.ncl|*.toml|*.json|*.yaml|*.yml) return 0 ;; *) return 1 ;; esac } +# If staged files provided, only check those if [ -n "$STAGED_FILES" ]; then FILES_TO_CHECK=$STAGED_FILES else + # Every candidate file; is_source_file below decides which ones count. FILES_TO_CHECK=$(find "$SCAN_PATH" -path '*/.git/*' -prune -o -path '*/node_modules/*' -prune -o \ - -type f \( -name '*.rs' -o -name '*.res' -o -name '*.js' -o -name '*.ts' -o -name '*.sh' \ - -o -name '*.bash' -o -name '*.zig' -o -name '*.ex' -o -name '*.exs' -o -name '*.gleam' \ - -o -name '*.ml' -o -name '*.mli' -o -name '*.adb' -o -name '*.ads' -o -name '*.ncl' \ - -o -name '*.toml' -o -name '*.yaml' -o -name '*.yml' \ - \) -print 2>/dev/null || true) + -type f -print 2>/dev/null || true) fi [ -z "$FILES_TO_CHECK" ] && exit 0 @@ -45,7 +47,7 @@ fi for file in $FILES_TO_CHECK; do [ -f "$file" ] || continue is_source_file "$file" || continue - + # Check for SPDX header in first 10 lines if ! head -10 "$file" | grep -qE '^# SPDX-License-Identifier:'; then echo "[validate-spdx] ERROR: $file missing SPDX header" >&2 diff --git a/.github/workflows/actions.lock b/.github/workflows/actions.lock index 5abc4ad7..32745eed 100644 --- a/.github/workflows/actions.lock +++ b/.github/workflows/actions.lock @@ -192,7 +192,7 @@ dependencies: owner_id: 47606891 repo_id: 331103973 'github/codeql-action@b96794f015dfd88f77b49b1c93e0fa7110f94c63': - ref: 'v4.38.0' + ref: 'b96794f015dfd88f77b49b1c93e0fa7110f94c63' commit: 'sha1-b96794f015dfd88f77b49b1c93e0fa7110f94c63' owner_id: 9919 repo_id: 259445878 diff --git a/scripts/tests/validate-spdx-test.sh b/scripts/tests/validate-spdx-test.sh new file mode 100755 index 00000000..76949b6e --- /dev/null +++ b/scripts/tests/validate-spdx-test.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 +# SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell +# +# Tests for .githooks/validate-spdx.sh, the pre-commit gate. +# +# ⚠ TEST 1 IS THE REASON THIS EXISTS, and it FAILS against the previous version. +# +# The validator had two modes asking two different questions. Full-scan mode +# filtered by source extension inside `find`; staged mode did not filter at all: +# +# FILES_TO_CHECK=$STAGED_FILES # every staged path, whatever it is +# +# Only the staged branch is reachable in practice — this validator is invoked by +# .githooks/pre-commit and by no workflow — so the mode that HAD the filter never +# ran, and the mode that ran had none. +# +# The casualty was .github/workflows/actions.lock: machine-generated by +# `gh actions-lock`, headerless by construction, and restamped on every +# regeneration so a hand-added header does not survive. Every commit touching the +# lockfile was therefore unpassable, which is how a Dependabot-caused lockfile +# desync survived long enough to recur (precedent: #746). +# +# Test 1 is the planted positive for that: staging a headerless .lock file must +# PASS. Test 4 is the parity check that the two modes now ask the same question, +# because a filter that exists in only one mode is what caused this. +set -uo pipefail +HOOK="$(cd "$(dirname "$0")/../.." && pwd)/.githooks/validate-spdx.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 +} + +ck_scan() { # name expected_exit path + local out rc + out="$(cd "$T" && 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 +} + +# A real actions.lock preamble: the tool's own banner, and no SPDX line anywhere. +printf "# This file is machine-generated by \`gh actions-lock\`.\n# Do not edit by hand; run \`gh actions-lock\` to update.\nversion: 'v0.0.2'\n" \ + > "$T/.github/workflows/actions.lock" +printf '# SPDX-License-Identifier: MPL-2.0\necho ok\n' > "$T/good.sh" +printf 'echo bad\n' > "$T/bad.sh" +printf 'name: bad\non: push\n' > "$T/.github/workflows/bad.yml" +printf 'binary-ish payload, not source\n' > "$T/README.md" + +echo "validate-spdx.sh" +ck "PLANTED POSITIVE: headerless actions.lock must PASS" 0 ".github/workflows/actions.lock" +ck "negative control: headerless .sh must FAIL" 1 "good.sh bad.sh" +ck "negative control: headerless .yml must FAIL" 1 ".github/workflows/bad.yml" +ck "valid header must PASS" 0 "good.sh" +ck "non-source README.md must PASS" 0 "README.md" +ck "lockfile alongside a valid source file must PASS" 0 ".github/workflows/actions.lock good.sh" + +# PARITY: both modes must agree about the same tree. A tree whose only headerless +# files are non-source must pass a full scan exactly as it passes staged mode. +mkdir -p "$T/parity" +cp "$T/.github/workflows/actions.lock" "$T/parity/actions.lock" +cp "$T/README.md" "$T/parity/README.md" +cp "$T/good.sh" "$T/parity/good.sh" +ck_scan "PARITY: full-scan mode ignores the same non-source files" 0 "parity" + +printf '\n%s passed, %s failed\n' "$pass" "$fail" +[ "$fail" -eq 0 ] || exit 1