From 574cb99d7670585b2dcaebacce52f24420b433d1 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 25 Sep 2026 11:04:29 +0000 Subject: [PATCH 1/2] feat: land the revision-preflight guard, fixed and wired into contract execution A contract verdict is only meaningful if it came from the merged copy. The fleet has been bitten three times on 2026-09-25 (a clone parked on a merged feature branch while executing from another clone; a script copied into the runner clone by hand; a stale local origin/master making an ancestry check report unlanded work). The control for this existed as an untracked draft and protected nobody, because it was entirely fail-open. Defect in the draft, preserved verbatim as tests/fixtures/revision-preflight.prefix.sh: git -C "$CLONE" show "origin/master:$(basename "$SCRIPT")" basename drops the scripts/ prefix, so for any script under scripts/ it queried the repo root, failed, took the "warn but don't block" branch and exited 0 - passing a script that exists in no revision at all. Reproduced: pre-fix + scripts/demo.sh under scripts/ -> 'could not resolve', EXIT=0 pre-fix + a script in no revision -> EXIT=0 Fixed guard (scripts/revision-preflight.sh): * resolves the repo-relative path inside the clone, so scripts/ paths resolve; * FAILS CLOSED - a path absent from the ref, an unresolvable ref, or a failed fetch is a failure, never a warning; * fetches the remote by default, because a stale local ref would otherwise pass a stale script as current; --no-fetch states the assumption instead of hiding it. Wiring (scripts/contract-run.sh): before executing, the wrapper runs the guard against the clone it lives in. Default CONTRACT_REVISION_PREFLIGHT=enforce withholds the verdict, alerts and exits 2 on mismatch; =warn logs and continues; =off skips. Verified live: match -> contract proceeds and PASSes; mismatch -> 'VERDICT WITHHELD', exit 2; =warn -> continues. Pinning (docs/contract-execution-pinning.md): every contract pins the clone contract-run.sh lives in - the deployed runner being /opt/contract-runner on CT 100. Documented that daily-health-digest has no contract file at all, which is why its execution copy was silently operator-chosen. Tests: tests/test_revision_preflight.sh, 15 assertions over a throwaway clone with a real bare remote. It runs the pre-fix draft against the same cases and shows it passing a ghost script, so the tests provably bite. shellcheck: scripts/revision-preflight.sh and the new test are clean. The three findings remaining in contract-run.sh (SC2086 x2, SC2034) are pre-existing and byte-identical on master. --- docs/contract-execution-pinning.md | 90 ++++++++++ scripts/contract-run.sh | 37 ++++ scripts/revision-preflight.sh | 128 ++++++++++++++ tests/fixtures/revision-preflight.prefix.sh | 40 +++++ tests/test_revision_preflight.sh | 182 ++++++++++++++++++++ 5 files changed, 477 insertions(+) create mode 100644 docs/contract-execution-pinning.md create mode 100755 scripts/revision-preflight.sh create mode 100755 tests/fixtures/revision-preflight.prefix.sh create mode 100755 tests/test_revision_preflight.sh diff --git a/docs/contract-execution-pinning.md b/docs/contract-execution-pinning.md new file mode 100644 index 0000000..f27aeb2 --- /dev/null +++ b/docs/contract-execution-pinning.md @@ -0,0 +1,90 @@ +# Contract execution pinning + +Which copy of a contract script actually ran, and how that is proven. + +## Why this exists + +Three times on 2026-09-25 a contract reported a verdict from a copy that was +not the merged one: + +1. The ops lane's own clone sat on the merged feature branch + `fix/search-stack-multi-engine-20260925` at `8b2eba4` with no `pve_auth` + fix, while it executed the daily digest from a different clone. Nothing in + the workflow noticed. +2. `scripts/search-stack-check.py` was deployed into the pinned runner clone + by hand rather than through git. +3. A stale local `origin/master` ref made an ancestry check report + "unlanded work" for a branch that had in fact merged — the same staleness + would have passed a stale script as current. + +A contract verdict is only meaningful if it came from the merged copy. The +control is `scripts/revision-preflight.sh`. + +## The rule + +**Every contract pins exactly one clone for execution: the clone that +`scripts/contract-run.sh` itself lives in.** + +`contract-run.sh` derives that from its own location (`SCRIPTS_DIR`) and checks +the script it is about to run against `origin/master` in the same clone. There +is no second path to configure, and no contract may be executed from a +hand-copied location. + +| Contract | Script | Pinned clone | +| --- | --- | --- | +| `infrastructure-monitoring` | `scripts/infra-monitoring.sh` | the clone containing `contract-run.sh` | +| `proxmox-monitor` | `scripts/proxmox-monitor.sh` | same | +| `zulip-health` | `scripts/zulip-monitor.sh` | same | +| `agent-health-check` | `scripts/agent-health-check.py` | same | +| `litellm-health` | `scripts/litellm-health-check.py` | same | +| `disk-gc-threat-response` | `scripts/disk-gc-scan.py` | same | +| `pm2-self-heal` | `scripts/pm2-self-heal.sh` | same | +| `search-stack-visibility` | `scripts/search-stack-check.py` | same | + +### The deployed runner + +The scheduler on **CT 100 (abiba)** runs contracts from +**`/opt/contract-runner`** via `/etc/cron.d/contract-runner`. That clone is the +pinned execution copy for every scheduled contract, and it must be kept current +with `master` by fast-forward. Its `origin` is a local path to the upstream +working copy, not a network remote. + +`daily-health-digest` is **not** in the table above because it has no contract +file and no mapping — it is dispatched by cron as +`fm-send.sh ops "run contract: daily-health-digest"` and was, until +2026-09-25, executed by hand from whichever clone the operator happened to be +in. Creating its contract file and pinning it to a clone is an open follow-up. + +## How the check works + +`scripts/revision-preflight.sh `: + +* resolves the **repo-relative** path of the executing script inside the clone; +* **fetches** the remote first, so a stale local ref cannot make a stale script + look current; +* compares the script's sha256 against `:`; +* **fails closed** — a path absent from the ref, an unresolvable ref, or a + failed fetch is a failure, never a warning. + +Exit `0` means verified match. Exit `1` means mismatch or unverifiable. + +## Modes in `contract-run.sh` + +| `CONTRACT_REVISION_PREFLIGHT` | Behaviour | +| --- | --- | +| unset / `enforce` (default) | withhold the verdict, alert, exit `2` | +| `warn` | log the mismatch and continue | +| `off` | skip the check entirely | + +`enforce` is the default deliberately: an unverifiable copy is indistinguishable +from a stale or hand-edited one, and a verdict from it is worse than no verdict. + +## Operating notes + +* A stale pinned clone will now make contracts **withhold** rather than report. + That is the intended failure. Recover by fast-forwarding the pinned clone: + `git -C /opt/contract-runner pull --ff-only`. +* When a contract legitimately changes, land it through the normal branch + PR + path and fast-forward the pinned clone. Do not copy files into it by hand. +* `--no-fetch` exists for offline inspection; it prints that freshness is + assumed rather than verified, and it is not used by `contract-run.sh`. diff --git a/scripts/contract-run.sh b/scripts/contract-run.sh index 04643aa..e09b418 100755 --- a/scripts/contract-run.sh +++ b/scripts/contract-run.sh @@ -18,6 +18,14 @@ # litellm-health -> scripts/litellm-health-check.py # disk-gc-threat-response -> scripts/disk-gc-scan.py # pm2-self-heal -> scripts/pm2-self-heal.sh +# search-stack-visibility -> scripts/search-stack-check.py +# +# Execution copy: every contract pins the clone this script lives in (see +# docs/contract-execution-pinning.md). Before a contract runs, this wrapper +# proves the script it is about to execute byte-matches origin/master: +# CONTRACT_REVISION_PREFLIGHT=enforce (default) refuse to report on mismatch +# CONTRACT_REVISION_PREFLIGHT=warn log the mismatch and continue +# CONTRACT_REVISION_PREFLIGHT=off skip the check entirely # # Exit codes: # 0 = contract passed @@ -111,6 +119,35 @@ echo "Started: $(date -u '+%Y-%m-%d %H:%M:%S UTC')" | tee -a "$LOG_FILE" echo "Script: $SCRIPT_PATH" | tee -a "$LOG_FILE" echo "" | tee -a "$LOG_FILE" +# ── Revision preflight ─────────────────────────────────────────────────────── +# A verdict is only meaningful if it came from the merged copy. Refuse to report +# one from a mismatched or unverifiable copy; that is a probe failure (exit 2), +# not a contract verdict, because the result would be untrustworthy. +# See docs/contract-execution-pinning.md. +REVISION_PREFLIGHT_MODE="${CONTRACT_REVISION_PREFLIGHT:-enforce}" +REPO_ROOT="$(cd "${SCRIPTS_DIR}/.." && pwd)" +if [ "$REVISION_PREFLIGHT_MODE" != "off" ] && [ -x "${SCRIPTS_DIR}/revision-preflight.sh" ]; then + if "${SCRIPTS_DIR}/revision-preflight.sh" "$SCRIPT_PATH" "$REPO_ROOT" 2>&1 | tee -a "$LOG_FILE"; then + : + elif [ "$REVISION_PREFLIGHT_MODE" = "warn" ]; then + echo "⚠️ revision preflight failed — continuing because CONTRACT_REVISION_PREFLIGHT=warn" | tee -a "$LOG_FILE" + else + echo "🚫 VERDICT WITHHELD: executing copy does not match the merged revision" | tee -a "$LOG_FILE" + ALERT_MSG="🔴 Contract $CONTRACT_NAME: revision mismatch — verdict withheld. Log: $LOG_FILE" + ZULIP_API_URL="${ZULIP_API_URL:-https://chat.sysloggh.net/api/v1}" + ZULIP_API_KEY="${ZULIP_API_KEY:-}" + ZULIP_USER="${ZULIP_USER:-abiba-bot@chat.sysloggh.net}" + if [ -n "$ZULIP_API_KEY" ] && command -v curl &> /dev/null; then + curl -sf -X POST "${ZULIP_API_URL}/messages" \ + -u "${ZULIP_USER}:${ZULIP_API_KEY}" \ + -d "type=private" \ + -d "to=9" \ + -d "content=${ALERT_MSG}" > /dev/null 2>&1 || true + fi + exit 2 + fi +fi + # Use timeout to prevent hangs (10 minutes default) TIMEOUT=600 timeout "$TIMEOUT" $INTERPRETER "$SCRIPT_PATH" 2>&1 | tee -a "$LOG_FILE" diff --git a/scripts/revision-preflight.sh b/scripts/revision-preflight.sh new file mode 100755 index 0000000..2086f6b --- /dev/null +++ b/scripts/revision-preflight.sh @@ -0,0 +1,128 @@ +#!/usr/bin/env bash +# revision-preflight.sh — prove the copy a contract is about to execute is the +# copy that is merged. +# +# Usage: +# revision-preflight.sh [options] +# +# Options: +# --ref Ref to compare against (default: origin/master) +# --no-fetch Do not refresh the ref first (see FRESHNESS below) +# --quiet Print nothing on success +# -h, --help Show this help +# +# Exit codes: +# 0 the executing script byte-matches : +# 1 MISMATCH, or the revision could not be resolved (see FAIL CLOSED) +# +# FRESHNESS +# A guard is only as good as the ref it compares against. On 2026-09-25 a +# stale local origin/master made an ancestry check on this fleet report +# "unlanded work" for a branch that had in fact merged, and it would equally +# have passed a stale script as current. So by default this guard FETCHES the +# remote before comparing. With --no-fetch it compares against whatever the +# local ref points at and says so out loud; it never silently assumes +# freshness. +# +# FAIL CLOSED +# An unresolvable path or ref is a FAILURE, never a warning. "Cannot verify" +# is precisely the state a stale or hand-edited copy produces, so treating it +# as success would defeat the guard. The original draft of this script did +# exactly that: it resolved the master revision with +# `git show origin/master:$(basename "$SCRIPT")`, which drops the scripts/ +# prefix, queries the repo root, fails, and exited 0 — passing a script that +# exists in no revision at all. +# +# WHICH CLONE +# Pass the clone the contract is actually executing from. See +# docs/contract-execution-pinning.md for which clone each contract pins. + +set -euo pipefail + +REF="origin/master" +FETCH=1 +QUIET=0 + +usage() { + sed -n '2,45p' "$0" | sed 's/^# \{0,1\}//' +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --ref) + [[ $# -ge 2 ]] || { echo "revision-preflight: --ref needs a value" >&2; exit 1; } + REF="$2"; shift 2 ;; + --no-fetch) FETCH=0; shift ;; + --quiet) QUIET=1; shift ;; + -h|--help) usage; exit 0 ;; + --) shift; break ;; + -*) echo "revision-preflight: unknown option: $1" >&2; exit 1 ;; + *) break ;; + esac +done + +if [[ $# -lt 2 ]]; then + usage >&2 + exit 1 +fi + +SCRIPT="$1" +CLONE="$2" + +say() { [[ $QUIET -eq 1 ]] || echo "$@" >&2; } +fail() { echo "❌ revision-preflight: $*" >&2; exit 1; } + +# ── 1. inputs must exist ────────────────────────────────────────────────────── +[[ -f "$SCRIPT" ]] || fail "executing script not found: $SCRIPT" +[[ -d "$CLONE" ]] || fail "clone path is not a directory: $CLONE" +git -C "$CLONE" rev-parse --git-dir >/dev/null 2>&1 \ + || fail "not a git clone: $CLONE" + +# ── 2. resolve the repo-relative path (the original defect) ─────────────────── +CLONE_ABS=$(cd "$CLONE" && pwd) +SCRIPT_ABS=$(cd "$(dirname "$SCRIPT")" && pwd)/$(basename "$SCRIPT") +case "$SCRIPT_ABS" in + "$CLONE_ABS"/*) REL="${SCRIPT_ABS#"$CLONE_ABS"/}" ;; + *) fail "script is outside the clone: $SCRIPT_ABS is not under $CLONE_ABS" ;; +esac + +# ── 3. refresh the ref so staleness cannot mask a stale script ──────────────── +if [[ $FETCH -eq 1 ]]; then + REMOTE="${REF%%/*}" + [[ "$REMOTE" == "$REF" ]] && REMOTE="origin" + if ! git -C "$CLONE" fetch --quiet "$REMOTE" 2>/dev/null; then + fail "cannot fetch '$REMOTE' in $CLONE — refusing to verify against a possibly stale '$REF'. Re-run with network access, or pass --no-fetch to compare against the local ref deliberately." + fi +else + say "⚠️ revision-preflight: --no-fetch — comparing against the LOCAL '$REF'; freshness is assumed, not verified" +fi + +# ── 4. resolve the merged revision; unresolvable is a failure ──────────────── +git -C "$CLONE" rev-parse --verify --quiet "$REF" >/dev/null \ + || fail "ref '$REF' does not resolve in $CLONE" +REF_COMMIT=$(git -C "$CLONE" rev-parse --short "$REF") + +TMPFILE=$(mktemp) +trap 'rm -f "$TMPFILE"' EXIT + +if ! git -C "$CLONE" show "$REF:$REL" > "$TMPFILE" 2>/dev/null; then + fail "'$REL' does not exist in $REF ($REF_COMMIT) — cannot verify $SCRIPT. A path that is absent from $REF can never be a merged copy." +fi + +# ── 5. compare ─────────────────────────────────────────────────────────────── +EXEC_SHA=$(sha256sum "$SCRIPT" | cut -d' ' -f1) +MERGED_SHA=$(sha256sum "$TMPFILE" | cut -d' ' -f1) + +if [[ "$EXEC_SHA" != "$MERGED_SHA" ]]; then + { + echo "❌ revision-preflight: MISMATCH — refusing to report from this copy" + echo " script: $SCRIPT_ABS" + echo " clone: $CLONE_ABS" + echo " executed: $EXEC_SHA" + echo " merged: $MERGED_SHA ($REF:$REL @ $REF_COMMIT)" + } >&2 + exit 1 +fi + +say "✅ revision-preflight: $REL matches $REF @ $REF_COMMIT ($EXEC_SHA)" +exit 0 diff --git a/tests/fixtures/revision-preflight.prefix.sh b/tests/fixtures/revision-preflight.prefix.sh new file mode 100755 index 0000000..0c5765d --- /dev/null +++ b/tests/fixtures/revision-preflight.prefix.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# Revision preflight guard: verify the script being executed matches origin/master +# Usage: revision-preflight.sh +# Returns 0 if match, 1 if mismatch (prints both revisions) + +set -euo pipefail + +SCRIPT="${1:?Usage: revision-preflight.sh }" +CLONE="${2:?Usage: revision-preflight.sh }" + +# Compute sha256 of the script being executed +EXEC_SHA=$(sha256sum "$SCRIPT" | cut -d' ' -f1) + +# Compute sha256 of the merged origin/master version +# Extract to a temp file to avoid pipe issues +TMPFILE=$(mktemp) +trap 'rm -f "$TMPFILE"' EXIT + +# Try to extract the file from origin/master +if git -C "$CLONE" show "origin/master:$(basename "$SCRIPT")" > "$TMPFILE" 2>/dev/null; then + MASTER_SHA=$(sha256sum "$TMPFILE" | cut -d' ' -f1) +else + echo "⚠️ revision-preflight: could not resolve origin/master revision for $(basename "$SCRIPT")" >&2 + exit 0 # Warn but don't block if git show fails +fi + +if [[ -z "$MASTER_SHA" || "$MASTER_SHA" == "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" ]]; then + echo "⚠️ revision-preflight: could not resolve origin/master revision for $(basename "$SCRIPT")" >&2 + exit 0 # Warn but don't block if git show fails +fi + +if [[ "$EXEC_SHA" != "$MASTER_SHA" ]]; then + echo "⚠️ revision-preflight: MISMATCH detected" >&2 + echo " Executed: $EXEC_SHA ($(basename "$SCRIPT"))" >&2 + echo " Merged: $MASTER_SHA (origin/master:$(basename "$SCRIPT"))" >&2 + exit 1 +else + echo "✅ revision-preflight: $SCRIPT matches origin/master ($EXEC_SHA)" >&2 + exit 0 +fi diff --git a/tests/test_revision_preflight.sh b/tests/test_revision_preflight.sh new file mode 100755 index 0000000..02bdefe --- /dev/null +++ b/tests/test_revision_preflight.sh @@ -0,0 +1,182 @@ +#!/usr/bin/env bash +# Behavioural tests for scripts/revision-preflight.sh +# +# Every case builds a throwaway clone with a real bare remote, so origin/master +# is genuine and the guard's fetch path is exercised. Nothing outside mktemp is +# touched. +# +# The pre-fix draft is kept at tests/fixtures/revision-preflight.prefix.sh and +# is run against the SAME cases, to prove these tests bite: the pre-fix guard +# exits 0 where the fixed guard exits 1. + +set -uo pipefail + +HERE="$(cd "$(dirname "$0")" && pwd)" +REPO="$(cd "$HERE/.." && pwd)" +GUARD="$REPO/scripts/revision-preflight.sh" +PREFIX_GUARD="$HERE/fixtures/revision-preflight.prefix.sh" + +PASS=0 +FAIL=0 +FAILED_CASES=() + +pass() { printf ' ✓ %s\n' "$1"; PASS=$((PASS + 1)); } +fail() { printf ' ✗ %s\n' "$1"; FAIL=$((FAIL + 1)); FAILED_CASES+=("$1"); } + +# Build a clone with a real remote; echo the clone path. +make_clone() { + local tmp + tmp="$(mktemp -d)" + git init --bare -q "$tmp/remote.git" + git init -q "$tmp/clone" + ( + cd "$tmp/clone" || exit 1 + git config user.email test@example.invalid + git config user.name test + mkdir -p scripts + printf '#!/bin/bash\necho hello\n' > scripts/demo.sh + chmod +x scripts/demo.sh + git add -A + git commit -qm init + git branch -M master + git remote add origin "$tmp/remote.git" + git push -q origin master + git fetch -q origin + ) + echo "$tmp/clone" +} + +echo "== revision-preflight behavioural tests ==" + +# ── 1. match → exit 0 ──────────────────────────────────────────────────────── +echo "1. matching copy" +C=$(make_clone) +if out=$("$GUARD" "$C/scripts/demo.sh" "$C" 2>&1); then + pass "matching copy exits 0" +else + fail "matching copy should exit 0 (got $?, output: $out)" +fi +if [[ -z "$( "$GUARD" --quiet "$C/scripts/demo.sh" "$C" 2>&1 )" ]]; then + pass "--quiet prints nothing on a match" +else + fail "--quiet should print nothing on a match" +fi +rm -rf "$(dirname "$C")" + +# ── 2. mismatch → exit 1 and names both hashes ─────────────────────────────── +echo "2. mismatched copy" +C=$(make_clone) +printf '#!/bin/bash\necho TAMPERED\n' > "$C/scripts/demo.sh" +out=$("$GUARD" "$C/scripts/demo.sh" "$C" 2>&1); rc=$? +if [[ $rc -eq 1 ]]; then pass "mismatch exits 1"; else fail "mismatch should exit 1 (got $rc)"; fi +if [[ "$out" == *"MISMATCH"* ]]; then pass "mismatch says MISMATCH"; else fail "mismatch should say MISMATCH"; fi +if [[ "$out" == *"executed:"* && "$out" == *"merged:"* ]]; then + pass "mismatch prints both revisions" +else + fail "mismatch should print both revisions" +fi +rm -rf "$(dirname "$C")" + +# ── 3. paths under scripts/ resolve (the original basename defect) ─────────── +echo "3. repo-relative path resolution" +C=$(make_clone) +if "$GUARD" --quiet "$C/scripts/demo.sh" "$C" >/dev/null 2>&1; then + pass "script under scripts/ resolves against origin/master" +else + fail "script under scripts/ must resolve (basename defect)" +fi +rm -rf "$(dirname "$C")" + +# ── 4. script that exists in NO revision → must fail ───────────────────────── +echo "4. untracked script present in no revision" +C=$(make_clone) +printf '#!/bin/bash\necho never committed\n' > "$C/scripts/ghost.sh" +out=$("$GUARD" "$C/scripts/ghost.sh" "$C" 2>&1); rc=$? +if [[ $rc -eq 1 ]]; then pass "ghost script exits 1"; else fail "ghost script must exit 1 (got $rc)"; fi +if [[ "$out" == *"does not exist in"* ]]; then + pass "ghost script says it is absent from the ref" +else + fail "ghost script should say it is absent from the ref" +fi +rm -rf "$(dirname "$C")" + +# ── 5. unresolvable ref → must fail ────────────────────────────────────────── +echo "5. unresolvable ref" +C=$(make_clone) +out=$("$GUARD" --no-fetch --ref origin/nope "$C/scripts/demo.sh" "$C" 2>&1); rc=$? +if [[ $rc -eq 1 ]]; then pass "unresolvable ref exits 1"; else fail "unresolvable ref must exit 1 (got $rc)"; fi +rm -rf "$(dirname "$C")" + +# ── 6. missing script → must fail ──────────────────────────────────────────── +echo "6. missing script" +C=$(make_clone) +out=$("$GUARD" "$C/scripts/nope.sh" "$C" 2>&1); rc=$? +if [[ $rc -eq 1 ]]; then pass "missing script exits 1"; else fail "missing script must exit 1 (got $rc)"; fi +rm -rf "$(dirname "$C")" + +# ── 7. script outside the clone → must fail ────────────────────────────────── +echo "7. script outside the clone" +C=$(make_clone) +OUTSIDE=$(mktemp) +printf '#!/bin/bash\necho outside\n' > "$OUTSIDE" +out=$("$GUARD" "$OUTSIDE" "$C" 2>&1); rc=$? +if [[ $rc -eq 1 ]]; then pass "outside script exits 1"; else fail "outside script must exit 1 (got $rc)"; fi +rm -f "$OUTSIDE"; rm -rf "$(dirname "$C")" + +# ── 8. --no-fetch states the freshness assumption ──────────────────────────── +echo "8. --no-fetch states its assumption" +C=$(make_clone) +out=$("$GUARD" --no-fetch "$C/scripts/demo.sh" "$C" 2>&1) +if [[ "$out" == *"freshness is assumed"* ]]; then + pass "--no-fetch states the freshness assumption" +else + fail "--no-fetch should state the freshness assumption" +fi +rm -rf "$(dirname "$C")" + +# ── 9. the pre-fix guard must FAIL these same cases (proves the tests bite) ── +echo "9. pre-fix draft fails the same cases (bite proof)" +if [[ ! -f "$PREFIX_GUARD" ]]; then + fail "pre-fix fixture missing: $PREFIX_GUARD" +else + # 9a. repo-relative path: pre-fix drops scripts/ and cannot resolve + C=$(make_clone) + out=$("$PREFIX_GUARD" "$C/scripts/demo.sh" "$C" 2>&1); rc=$? + if [[ $rc -eq 0 && "$out" == *"could not resolve"* ]]; then + pass "pre-fix: exits 0 and cannot resolve scripts/demo.sh (defect confirmed)" + else + fail "pre-fix should exit 0 with 'could not resolve' (got rc=$rc)" + fi + rm -rf "$(dirname "$C")" + + # 9b. ghost script: pre-fix passes a script that exists in no revision + C=$(make_clone) + printf '#!/bin/bash\necho never committed\n' > "$C/scripts/ghost.sh" + out=$("$PREFIX_GUARD" "$C/scripts/ghost.sh" "$C" 2>&1); rc=$? + if [[ $rc -eq 0 ]]; then + pass "pre-fix: PASSES a ghost script that exists in no revision (defect confirmed)" + else + fail "pre-fix was expected to wrongly pass the ghost script (got rc=$rc)" + fi + rm -rf "$(dirname "$C")" + + # 9c. a file that DOES exist at the repo root still works pre-fix, showing + # the defect is specific to nested paths + C=$(make_clone) + printf '#!/bin/bash\necho root\n' > "$C/rootlevel.sh" + ( cd "$C" && git add rootlevel.sh && git commit -qm root && git push -q origin master && git fetch -q origin ) + if "$PREFIX_GUARD" "$C/rootlevel.sh" "$C" >/dev/null 2>&1; then + pass "pre-fix: root-level path resolves (so the defect is the basename, not git)" + else + fail "pre-fix should resolve a root-level tracked file" + fi + rm -rf "$(dirname "$C")" +fi + +echo +echo " passed: $PASS failed: $FAIL" +if [[ $FAIL -gt 0 ]]; then + printf ' FAILED: %s\n' "${FAILED_CASES[@]}" + exit 1 +fi +echo "All revision-preflight tests passed." -- 2.54.0 From c5a6dcd42a21457bcc7dbd3539218cd567f257d3 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 25 Sep 2026 11:29:04 +0000 Subject: [PATCH 2/2] fix(revision-preflight): default to warn, name the refusal class, bound the fetch Bounded correction round on PR #134 after a PASS-WITH-FINDINGS review whose Finding 4 is High. The guard's purpose and its fail-closed fix stand; the problem was that with 'enforce' as the default it gates EVERY scheduled contract, and three legitimate states produce a refusal - a clone legitimately ahead of origin/master mid-review, a detached HEAD, and an offline or failed fetch - so any of them would turn the fleet's monitoring into withheld verdicts. That risk outweighs the staleness the guard catches. 1. DEFAULT IS NOW 'warn'. 'enforce' remains available and documented. The criteria for flipping the default later are written into the doc as a decision with evidence - a sustained window (30 days / 200+ runs) with zero mismatch:* and zero cannot-verify:* refusals, no fetch blips, and a pinned clone demonstrably kept current - explicitly as its own change, not a silent flip. 2. 'COULD NOT CHECK' IS NOW DISTINGUISHABLE FROM 'THIS COPY IS WRONG'. Every non-zero exit prints a machine-readable REASON= line: cannot-verify:fetch-failed | cannot-verify:ref-unresolvable (exit 2) mismatch:path-absent | mismatch:content mismatch:detached-head | mismatch:clone-ahead (exit 1) detached-head and clone-ahead are named separately because they are legitimate states, far less alarming than a hand-edited file. clone-ahead requires HEAD to be STRICTLY ahead; an uncommitted edit on a commit that IS the ref is a plain content mismatch (my own first cut got this wrong and the new test 7d caught it). 3. THE DEFAULT FETCH IS BOUNDED: --fetch-timeout, default 20s, 0 = unbounded, and a missing 'timeout' binary is itself a cannot-verify rather than an unbounded fetch inside a scheduled contract. 4. TEST COVERAGE ADDED for every new class: fetch failure, fetch timeout (asserted to return promptly under a 1s bound), unresolvable ref, detached HEAD, clone-ahead, genuine content mismatch, and the contract-run.sh default. The pre-fix draft fixture comparisons are kept: 31 passed, 0 failed. 5. MERGE-TIME SEQUENCE documented: fast-forward /opt/contract-runner, confirm clean, prove a contract runs and reports. Baseline recorded as of today - firstmate has already fast-forwarded it to 9faffe4 - with the note that an untracked file blocks a fast-forward even when byte-identical. Live behaviour re-verified on the real runner path: default: REASON=mismatch:clone-ahead -> 'continuing because ...=warn' -> VERDICT: PASS, exit 0 enforce: REASON=mismatch:clone-ahead -> 'VERDICT WITHHELD: mismatch:clone-ahead', exit 2 MANDATORY CHECKS (master went red once from a credential-SHAPED string, so these are now run on every shippable branch): bash scripts/prose-lint.sh -> LINT PASSED (18 warning(s)) secret scan -> secret scan clean (tree; 34 allowlisted, 24 inert value(s) ignored); No committed credentials shellcheck revision-preflight.sh -> clean shellcheck test_revision_preflight.sh -> clean shellcheck contract-run.sh -> SC2034 x1, SC2086 x2 - byte-identical on master, i.e. pre-existing, none introduced tests/test_probe_drift.py::test_prose_lint_accepts_report_format_with_provenance fails both before and after this branch (it runs prose-lint from a temp CWD and cannot find its sibling secret-scan.sh). Pre-existing, unrelated, not fixed here. --- docs/contract-execution-pinning.md | 95 +++++++++++++++++-- scripts/contract-run.sh | 60 +++++++----- scripts/revision-preflight.sh | 145 ++++++++++++++++++++++------- tests/test_revision_preflight.sh | 108 ++++++++++++++++++++- 4 files changed, 341 insertions(+), 67 deletions(-) diff --git a/docs/contract-execution-pinning.md b/docs/contract-execution-pinning.md index f27aeb2..1a9a886 100644 --- a/docs/contract-execution-pinning.md +++ b/docs/contract-execution-pinning.md @@ -61,28 +61,107 @@ in. Creating its contract file and pinning it to a clone is an open follow-up. * resolves the **repo-relative** path of the executing script inside the clone; * **fetches** the remote first, so a stale local ref cannot make a stale script - look current; + look current — bounded by `--fetch-timeout` (default 20s) so a hung remote + cannot block a scheduled contract; * compares the script's sha256 against `:`; * **fails closed** — a path absent from the ref, an unresolvable ref, or a failed fetch is a failure, never a warning. -Exit `0` means verified match. Exit `1` means mismatch or unverifiable. +### Exit codes and reason classes + +The guard distinguishes **"I could not check"** from **"this copy is wrong"**, +and every non-zero exit prints a machine-readable `REASON=` line before +the human text, because a warning nobody can classify is not actionable — and +the flip to `enforce` (below) depends on being able to read these apart. + +| exit | `REASON=` | meaning | +| --- | --- | --- | +| 0 | — | verified match | +| 2 | `cannot-verify:fetch-failed` | remote unreachable, failed, or timed out | +| 2 | `cannot-verify:ref-unresolvable` | `` does not exist in the clone | +| 1 | `mismatch:path-absent` | the script does not exist in `` | +| 1 | `mismatch:content` | the script differs from `` | +| 1 | `mismatch:detached-head` | the clone is on a detached HEAD | +| 1 | `mismatch:clone-ahead` | local HEAD is strictly ahead of `` (mid-review) | + +`detached-head` and `clone-ahead` are named separately on purpose: they are +*legitimate* states that merely fail to be "the merged copy", and they are far +less alarming than a hand-edited file. `clone-ahead` requires HEAD to be +**strictly** ahead — an uncommitted edit on a commit that *is* the ref is a +plain `content` mismatch. ## Modes in `contract-run.sh` | `CONTRACT_REVISION_PREFLIGHT` | Behaviour | | --- | --- | -| unset / `enforce` (default) | withhold the verdict, alert, exit `2` | -| `warn` | log the mismatch and continue | +| unset / **`warn` (default)** | log the refusal and its class, then still report | +| `enforce` | withhold the verdict, alert, exit `2` | | `off` | skip the check entirely | -`enforce` is the default deliberately: an unverifiable copy is indistinguishable -from a stale or hand-edited one, and a verdict from it is worse than no verdict. +**The default is `warn`, deliberately.** The guard gates *every* scheduled +contract, and three legitimate situations would otherwise turn the whole +fleet's monitoring into withheld verdicts: a clone legitimately ahead of +`origin/master` mid-review, a detached HEAD, and an offline or failed fetch. +That is a bigger risk than the staleness the guard exists to catch. `warn` +keeps the signal loud and classified in every run's log without letting the +monitoring go dark. + +### Criteria for flipping the default to `enforce` + +Do not flip it on preference. Flip it when the evidence says the false-refusal +rate is low enough, as its own small change with its own review: + +1. the guard has run across **every scheduled contract** for a sustained period + (suggested: 30 consecutive days, or 200+ contract runs) with **zero** + `mismatch:*` and **zero** `cannot-verify:*` refusals in the per-run logs; +2. no `cannot-verify:fetch-failed` arising from ordinary network blips in that + window — if the pinned clone's remote is not reliably reachable, `enforce` + will withhold rather than report; +3. the pinned runner clone is demonstrably kept current by fast-forward, so + `mismatch:clone-ahead` is a genuine fault rather than routine procedure. + +The evidence for the flip is the `REASON=` lines already written into +`/var/log/contract-runs/`. Until then the default stays `warn`. + +## Merge-time sequence (do this whenever this repo merges) + +**Baseline as of 2026-09-25:** `/opt/contract-runner` is already +fast-forwarded to master `9faffe4`, so the pinned runner clone is current +today. This sequence exists to keep it that way. + +After any merge to `master`: + +```bash +# 1. fast-forward the pinned runner clone on CT 100 +git -C /opt/contract-runner pull --ff-only + +# 2. confirm it is current and clean +git -C /opt/contract-runner log --oneline -1 +git -C /opt/contract-runner status --porcelain # expect no output + +# 3. prove a contract runs and reports normally +CONTRACT_RUN_LOG_DIR=/tmp/preflight-proof \ + bash /opt/contract-runner/scripts/contract-run.sh search-stack-visibility +echo "EXIT=$?" # expect 0, and 'revision-preflight: … matches origin/master' +``` + +A contract that reports a `REASON=mismatch:*` refusal here means the runner +clone is stale or locally edited — fast-forward it rather than reaching for +`CONTRACT_REVISION_PREFLIGHT=off`. + +**Note on untracked files:** git refuses to fast-forward over an untracked file +even when its content is byte-identical to the incoming version +(`The following untracked working tree files would be overwritten by merge`). +A dirty clone will therefore block step 1. Resolve it by removing or stashing +the untracked paths first — that is exactly what blocked a clone on 2026-09-25. ## Operating notes -* A stale pinned clone will now make contracts **withhold** rather than report. - That is the intended failure. Recover by fast-forwarding the pinned clone: +* Under the default `warn`, a stale pinned clone still produces verdicts but + every run logs the refusal and its class. Read those lines; do not ignore + them. +* Under `enforce`, a stale pinned clone **withholds**. That is the intended + failure. Recover by fast-forwarding: `git -C /opt/contract-runner pull --ff-only`. * When a contract legitimately changes, land it through the normal branch + PR path and fast-forward the pinned clone. Do not copy files into it by hand. diff --git a/scripts/contract-run.sh b/scripts/contract-run.sh index e09b418..3b6ec8d 100755 --- a/scripts/contract-run.sh +++ b/scripts/contract-run.sh @@ -23,9 +23,11 @@ # Execution copy: every contract pins the clone this script lives in (see # docs/contract-execution-pinning.md). Before a contract runs, this wrapper # proves the script it is about to execute byte-matches origin/master: -# CONTRACT_REVISION_PREFLIGHT=enforce (default) refuse to report on mismatch -# CONTRACT_REVISION_PREFLIGHT=warn log the mismatch and continue -# CONTRACT_REVISION_PREFLIGHT=off skip the check entirely +# CONTRACT_REVISION_PREFLIGHT=warn (default) log a refusal, still report +# CONTRACT_REVISION_PREFLIGHT=enforce refuse to report on a mismatch +# CONTRACT_REVISION_PREFLIGHT=off skip the check entirely +# A refusal names its class: cannot-verify:fetch-failed|ref-unresolvable, +# or mismatch:content|path-absent|detached-head|clone-ahead. # # Exit codes: # 0 = contract passed @@ -120,32 +122,44 @@ echo "Script: $SCRIPT_PATH" | tee -a "$LOG_FILE" echo "" | tee -a "$LOG_FILE" # ── Revision preflight ─────────────────────────────────────────────────────── -# A verdict is only meaningful if it came from the merged copy. Refuse to report -# one from a mismatched or unverifiable copy; that is a probe failure (exit 2), -# not a contract verdict, because the result would be untrustworthy. +# A verdict is only meaningful if it came from the merged copy. This reports +# whether the executing copy matches, and distinguishes "could not check" from +# "this copy is wrong" so an operator can tell them apart. # See docs/contract-execution-pinning.md. -REVISION_PREFLIGHT_MODE="${CONTRACT_REVISION_PREFLIGHT:-enforce}" +# +# Default is WARN, not enforce: the guard gates every scheduled contract, and a +# legitimate state (branch mid-review, detached HEAD, briefly offline) would +# otherwise turn the whole fleet's monitoring into withheld verdicts. The +# criteria for flipping the default to enforce are written down in the doc. +REVISION_PREFLIGHT_MODE="${CONTRACT_REVISION_PREFLIGHT:-warn}" REPO_ROOT="$(cd "${SCRIPTS_DIR}/.." && pwd)" if [ "$REVISION_PREFLIGHT_MODE" != "off" ] && [ -x "${SCRIPTS_DIR}/revision-preflight.sh" ]; then - if "${SCRIPTS_DIR}/revision-preflight.sh" "$SCRIPT_PATH" "$REPO_ROOT" 2>&1 | tee -a "$LOG_FILE"; then - : - elif [ "$REVISION_PREFLIGHT_MODE" = "warn" ]; then - echo "⚠️ revision preflight failed — continuing because CONTRACT_REVISION_PREFLIGHT=warn" | tee -a "$LOG_FILE" + PREFLIGHT_OUT="$(mktemp)" + if "${SCRIPTS_DIR}/revision-preflight.sh" "$SCRIPT_PATH" "$REPO_ROOT" >"$PREFLIGHT_OUT" 2>&1; then + cat "$PREFLIGHT_OUT" | tee -a "$LOG_FILE" else - echo "🚫 VERDICT WITHHELD: executing copy does not match the merged revision" | tee -a "$LOG_FILE" - ALERT_MSG="🔴 Contract $CONTRACT_NAME: revision mismatch — verdict withheld. Log: $LOG_FILE" - ZULIP_API_URL="${ZULIP_API_URL:-https://chat.sysloggh.net/api/v1}" - ZULIP_API_KEY="${ZULIP_API_KEY:-}" - ZULIP_USER="${ZULIP_USER:-abiba-bot@chat.sysloggh.net}" - if [ -n "$ZULIP_API_KEY" ] && command -v curl &> /dev/null; then - curl -sf -X POST "${ZULIP_API_URL}/messages" \ - -u "${ZULIP_USER}:${ZULIP_API_KEY}" \ - -d "type=private" \ - -d "to=9" \ - -d "content=${ALERT_MSG}" > /dev/null 2>&1 || true + cat "$PREFLIGHT_OUT" | tee -a "$LOG_FILE" + PREFLIGHT_REASON="$(grep -m1 '^REASON=' "$PREFLIGHT_OUT" | cut -d= -f2-)" + [ -n "$PREFLIGHT_REASON" ] || PREFLIGHT_REASON="unclassified" + if [ "$REVISION_PREFLIGHT_MODE" = "enforce" ]; then + echo "🚫 VERDICT WITHHELD: $PREFLIGHT_REASON" | tee -a "$LOG_FILE" + ALERT_MSG="🔴 Contract $CONTRACT_NAME: revision preflight REFUSED ($PREFLIGHT_REASON) — verdict withheld. Log: $LOG_FILE" + ZULIP_API_URL="${ZULIP_API_URL:-https://chat.sysloggh.net/api/v1}" + ZULIP_API_KEY="${ZULIP_API_KEY:-}" + ZULIP_USER="${ZULIP_USER:-abiba-bot@chat.sysloggh.net}" + if [ -n "$ZULIP_API_KEY" ] && command -v curl &> /dev/null; then + curl -sf -X POST "${ZULIP_API_URL}/messages" \ + -u "${ZULIP_USER}:${ZULIP_API_KEY}" \ + -d "type=private" \ + -d "to=9" \ + -d "content=${ALERT_MSG}" > /dev/null 2>&1 || true + fi + rm -f "$PREFLIGHT_OUT" + exit 2 fi - exit 2 + echo "⚠️ revision preflight: $PREFLIGHT_REASON — continuing because CONTRACT_REVISION_PREFLIGHT=$REVISION_PREFLIGHT_MODE" | tee -a "$LOG_FILE" fi + rm -f "$PREFLIGHT_OUT" fi # Use timeout to prevent hangs (10 minutes default) diff --git a/scripts/revision-preflight.sh b/scripts/revision-preflight.sh index 2086f6b..5b0f370 100755 --- a/scripts/revision-preflight.sh +++ b/scripts/revision-preflight.sh @@ -6,29 +6,45 @@ # revision-preflight.sh [options] # # Options: -# --ref Ref to compare against (default: origin/master) -# --no-fetch Do not refresh the ref first (see FRESHNESS below) -# --quiet Print nothing on success -# -h, --help Show this help +# --ref Ref to compare against (default: origin/master) +# --no-fetch Do not refresh the ref first (see FRESHNESS) +# --fetch-timeout Bound the default fetch (default: 20; 0 = no bound) +# --quiet Print nothing on success +# -h, --help Show this help # # Exit codes: # 0 the executing script byte-matches : -# 1 MISMATCH, or the revision could not be resolved (see FAIL CLOSED) +# 1 the copy is NOT the merged one -> REASON=mismatch: +# 2 the check could not be performed -> REASON=cannot-verify: +# +# Every non-zero exit prints one machine-readable line +# REASON= +# followed by the human explanation. The two top-level classes are deliberately +# distinct: "I could not check" is a different situation from "this copy is +# wrong", and an operator must never have to guess which they are looking at. +# +# cannot-verify:fetch-failed the remote could not be reached (or timed out) +# cannot-verify:ref-unresolvable does not exist in the clone +# mismatch:path-absent the script does not exist in +# mismatch:content the script differs from +# mismatch:detached-head the clone is on a detached HEAD +# mismatch:clone-ahead local HEAD is ahead of (mid-review?) # # FRESHNESS # A guard is only as good as the ref it compares against. On 2026-09-25 a # stale local origin/master made an ancestry check on this fleet report # "unlanded work" for a branch that had in fact merged, and it would equally # have passed a stale script as current. So by default this guard FETCHES the -# remote before comparing. With --no-fetch it compares against whatever the +# remote before comparing, bounded by --fetch-timeout so a hung remote cannot +# block a scheduled contract. With --no-fetch it compares against whatever the # local ref points at and says so out loud; it never silently assumes # freshness. # # FAIL CLOSED # An unresolvable path or ref is a FAILURE, never a warning. "Cannot verify" # is precisely the state a stale or hand-edited copy produces, so treating it -# as success would defeat the guard. The original draft of this script did -# exactly that: it resolved the master revision with +# as success would defeat the guard. The original draft did exactly that: it +# resolved the master revision with # `git show origin/master:$(basename "$SCRIPT")`, which drops the scripts/ # prefix, queries the repo root, fails, and exited 0 — passing a script that # exists in no revision at all. @@ -42,71 +58,115 @@ set -euo pipefail REF="origin/master" FETCH=1 QUIET=0 +FETCH_TIMEOUT="${REVISION_PREFLIGHT_FETCH_TIMEOUT:-20}" usage() { - sed -n '2,45p' "$0" | sed 's/^# \{0,1\}//' + sed -n '2,55p' "$0" | sed 's/^# \{0,1\}//' } while [[ $# -gt 0 ]]; do case "$1" in --ref) - [[ $# -ge 2 ]] || { echo "revision-preflight: --ref needs a value" >&2; exit 1; } + [[ $# -ge 2 ]] || { echo "revision-preflight: --ref needs a value" >&2; exit 2; } REF="$2"; shift 2 ;; --no-fetch) FETCH=0; shift ;; + --fetch-timeout) + [[ $# -ge 2 ]] || { echo "revision-preflight: --fetch-timeout needs a value" >&2; exit 2; } + FETCH_TIMEOUT="$2"; shift 2 ;; --quiet) QUIET=1; shift ;; -h|--help) usage; exit 0 ;; --) shift; break ;; - -*) echo "revision-preflight: unknown option: $1" >&2; exit 1 ;; + -*) echo "revision-preflight: unknown option: $1" >&2; exit 2 ;; *) break ;; esac done if [[ $# -lt 2 ]]; then usage >&2 - exit 1 + exit 2 fi SCRIPT="$1" CLONE="$2" say() { [[ $QUIET -eq 1 ]] || echo "$@" >&2; } -fail() { echo "❌ revision-preflight: $*" >&2; exit 1; } + +# refuse -> the copy is not the merged one +refuse() { + local class="$1"; shift + echo "REASON=mismatch:${class}" >&2 + echo "❌ revision-preflight: MISMATCH (${class}) — refusing to report from this copy" >&2 + for line in "$@"; do echo " $line" >&2; done + exit 1 +} + +# unverifiable -> the check could not be performed +unverifiable() { + local class="$1"; shift + echo "REASON=cannot-verify:${class}" >&2 + echo "❌ revision-preflight: CANNOT VERIFY (${class}) — refusing to report unverified" >&2 + for line in "$@"; do echo " $line" >&2; done + exit 2 +} # ── 1. inputs must exist ────────────────────────────────────────────────────── -[[ -f "$SCRIPT" ]] || fail "executing script not found: $SCRIPT" -[[ -d "$CLONE" ]] || fail "clone path is not a directory: $CLONE" -git -C "$CLONE" rev-parse --git-dir >/dev/null 2>&1 \ - || fail "not a git clone: $CLONE" +if [[ ! -f "$SCRIPT" ]]; then + refuse "path-absent" "executing script not found: $SCRIPT" +fi +if [[ ! -d "$CLONE" ]]; then + unverifiable "ref-unresolvable" "clone path is not a directory: $CLONE" +fi +if ! git -C "$CLONE" rev-parse --git-dir >/dev/null 2>&1; then + unverifiable "ref-unresolvable" "not a git clone: $CLONE" +fi # ── 2. resolve the repo-relative path (the original defect) ─────────────────── CLONE_ABS=$(cd "$CLONE" && pwd) SCRIPT_ABS=$(cd "$(dirname "$SCRIPT")" && pwd)/$(basename "$SCRIPT") case "$SCRIPT_ABS" in "$CLONE_ABS"/*) REL="${SCRIPT_ABS#"$CLONE_ABS"/}" ;; - *) fail "script is outside the clone: $SCRIPT_ABS is not under $CLONE_ABS" ;; + *) refuse "content" "script is outside the clone: $SCRIPT_ABS is not under $CLONE_ABS" ;; esac -# ── 3. refresh the ref so staleness cannot mask a stale script ──────────────── +# ── 3. refresh the ref, bounded, so a hung remote cannot block a contract ──── if [[ $FETCH -eq 1 ]]; then REMOTE="${REF%%/*}" [[ "$REMOTE" == "$REF" ]] && REMOTE="origin" - if ! git -C "$CLONE" fetch --quiet "$REMOTE" 2>/dev/null; then - fail "cannot fetch '$REMOTE' in $CLONE — refusing to verify against a possibly stale '$REF'. Re-run with network access, or pass --no-fetch to compare against the local ref deliberately." + FETCH_CMD=(git -C "$CLONE" fetch --quiet "$REMOTE") + if [[ "$FETCH_TIMEOUT" != "0" ]]; then + if ! command -v timeout >/dev/null 2>&1; then + unverifiable "fetch-failed" \ + "cannot bound the fetch: 'timeout' is not available" \ + "refusing to run an unbounded fetch inside a scheduled contract" + fi + FETCH_CMD=(timeout --signal=TERM --kill-after=5 "$FETCH_TIMEOUT" "${FETCH_CMD[@]}") + fi + if ! "${FETCH_CMD[@]}" 2>/dev/null; then + unverifiable "fetch-failed" \ + "could not fetch '$REMOTE' in $CLONE_ABS (bound: ${FETCH_TIMEOUT}s)" \ + "cannot compare against a possibly stale '$REF'" \ + "re-run with network access, raise --fetch-timeout, or pass --no-fetch deliberately" fi else say "⚠️ revision-preflight: --no-fetch — comparing against the LOCAL '$REF'; freshness is assumed, not verified" fi # ── 4. resolve the merged revision; unresolvable is a failure ──────────────── -git -C "$CLONE" rev-parse --verify --quiet "$REF" >/dev/null \ - || fail "ref '$REF' does not resolve in $CLONE" +if ! git -C "$CLONE" rev-parse --verify --quiet "$REF" >/dev/null; then + unverifiable "ref-unresolvable" \ + "ref '$REF' does not resolve in $CLONE_ABS" \ + "the clone may never have fetched, or the ref name may be wrong" +fi REF_COMMIT=$(git -C "$CLONE" rev-parse --short "$REF") TMPFILE=$(mktemp) trap 'rm -f "$TMPFILE"' EXIT if ! git -C "$CLONE" show "$REF:$REL" > "$TMPFILE" 2>/dev/null; then - fail "'$REL' does not exist in $REF ($REF_COMMIT) — cannot verify $SCRIPT. A path that is absent from $REF can never be a merged copy." + refuse "path-absent" \ + "'$REL' does not exist in $REF ($REF_COMMIT)" \ + "a path absent from $REF can never be a merged copy" \ + "script: $SCRIPT_ABS" fi # ── 5. compare ─────────────────────────────────────────────────────────────── @@ -114,14 +174,35 @@ EXEC_SHA=$(sha256sum "$SCRIPT" | cut -d' ' -f1) MERGED_SHA=$(sha256sum "$TMPFILE" | cut -d' ' -f1) if [[ "$EXEC_SHA" != "$MERGED_SHA" ]]; then - { - echo "❌ revision-preflight: MISMATCH — refusing to report from this copy" - echo " script: $SCRIPT_ABS" - echo " clone: $CLONE_ABS" - echo " executed: $EXEC_SHA" - echo " merged: $MERGED_SHA ($REF:$REL @ $REF_COMMIT)" - } >&2 - exit 1 + DETAIL=("script: $SCRIPT_ABS" + "clone: $CLONE_ABS" + "executed: $EXEC_SHA" + "merged: $MERGED_SHA ($REF:$REL @ $REF_COMMIT)") + + # Name WHY it differs: a detached HEAD or a branch legitimately ahead of the + # ref is a much more benign situation than a hand-edited file, and the + # operator must be able to tell them apart. + if ! git -C "$CLONE" symbolic-ref -q HEAD >/dev/null 2>&1; then + DETAIL+=("note: the clone is on a DETACHED HEAD, so the executing copy") + DETAIL+=(" cannot be attributed to any branch") + refuse "detached-head" "${DETAIL[@]}" + fi + + HEAD_REF=$(git -C "$CLONE" symbolic-ref -q --short HEAD || echo "HEAD") + # Strictly ahead: equal commits are not "ahead", and an uncommitted edit on a + # commit that IS the ref must fall through to a plain content mismatch. + REF_OID=$(git -C "$CLONE" rev-parse "$REF" 2>/dev/null || echo "") + HEAD_OID=$(git -C "$CLONE" rev-parse HEAD 2>/dev/null || echo "") + if [[ -n "$REF_OID" && "$REF_OID" != "$HEAD_OID" ]] \ + && git -C "$CLONE" merge-base --is-ancestor "$REF" HEAD 2>/dev/null; then + AHEAD=$(git -C "$CLONE" rev-list --count "$REF..HEAD" 2>/dev/null || echo "?") + DETAIL+=("note: '$HEAD_REF' is AHEAD of $REF by $AHEAD commit(s)") + DETAIL+=(" (a legitimate mid-review state, not a hand-edited file)") + refuse "clone-ahead" "${DETAIL[@]}" + fi + + DETAIL+=("branch: $HEAD_REF") + refuse "content" "${DETAIL[@]}" fi say "✅ revision-preflight: $REL matches $REF @ $REF_COMMIT ($EXEC_SHA)" diff --git a/tests/test_revision_preflight.sh b/tests/test_revision_preflight.sh index 02bdefe..c947cf0 100755 --- a/tests/test_revision_preflight.sh +++ b/tests/test_revision_preflight.sh @@ -100,21 +100,65 @@ else fi rm -rf "$(dirname "$C")" -# ── 5. unresolvable ref → must fail ────────────────────────────────────────── +reason_of() { grep -m1 '^REASON=' <<<"$1" | cut -d= -f2-; } + +# ── 5. unresolvable ref → CANNOT VERIFY (exit 2) ───────────────────────────── echo "5. unresolvable ref" C=$(make_clone) out=$("$GUARD" --no-fetch --ref origin/nope "$C/scripts/demo.sh" "$C" 2>&1); rc=$? -if [[ $rc -eq 1 ]]; then pass "unresolvable ref exits 1"; else fail "unresolvable ref must exit 1 (got $rc)"; fi +if [[ $rc -eq 2 ]]; then pass "unresolvable ref exits 2 (cannot verify)"; else fail "unresolvable ref must exit 2 (got $rc)"; fi +if [[ "$(reason_of "$out")" == "cannot-verify:ref-unresolvable" ]]; then + pass "unresolvable ref names cannot-verify:ref-unresolvable" +else + fail "unresolvable ref should name its class (got: $(reason_of "$out"))" +fi rm -rf "$(dirname "$C")" -# ── 6. missing script → must fail ──────────────────────────────────────────── +# ── 5b. fetch failure → CANNOT VERIFY, and it names that ───────────────────── +echo "5b. fetch failed" +C=$(make_clone) +( cd "$C" && git remote set-url origin /nonexistent/definitely-not-a-repo ) +out=$("$GUARD" "$C/scripts/demo.sh" "$C" 2>&1); rc=$? +if [[ $rc -eq 2 ]]; then pass "fetch failure exits 2 (cannot verify)"; else fail "fetch failure must exit 2 (got $rc)"; fi +if [[ "$(reason_of "$out")" == "cannot-verify:fetch-failed" ]]; then + pass "fetch failure names cannot-verify:fetch-failed" +else + fail "fetch failure should name its class (got: $(reason_of "$out"))" +fi +if [[ "$out" == *"bound:"* ]]; then pass "fetch failure reports the bound"; else fail "fetch failure should report the bound"; fi +rm -rf "$(dirname "$C")" + +# ── 5c. fetch timeout → CANNOT VERIFY, bounded (never hangs) ───────────────── +echo "5c. fetch timeout is bounded" +C=$(make_clone) +# a remote that will never answer: a fifo-backed git daemon is overkill, so use +# a black-hole address with a 1s bound and assert we return promptly. +( cd "$C" && git remote set-url origin http://10.255.255.1:9/never.git ) +start=$(date +%s) +out=$("$GUARD" --fetch-timeout 1 "$C/scripts/demo.sh" "$C" 2>&1); rc=$? +elapsed=$(( $(date +%s) - start )) +if [[ $rc -eq 2 ]]; then pass "timeout exits 2 (cannot verify)"; else fail "timeout must exit 2 (got $rc)"; fi +if [[ "$(reason_of "$out")" == "cannot-verify:fetch-failed" ]]; then + pass "timeout names cannot-verify:fetch-failed" +else + fail "timeout should name its class (got: $(reason_of "$out"))" +fi +if [[ $elapsed -le 10 ]]; then pass "timeout returned promptly (${elapsed}s, bound 1s)"; else fail "timeout did not bound the fetch (${elapsed}s)"; fi +rm -rf "$(dirname "$C")" + +# ── 6. missing script → MISMATCH:path-absent (exit 1) ──────────────────────── echo "6. missing script" C=$(make_clone) out=$("$GUARD" "$C/scripts/nope.sh" "$C" 2>&1); rc=$? if [[ $rc -eq 1 ]]; then pass "missing script exits 1"; else fail "missing script must exit 1 (got $rc)"; fi +if [[ "$(reason_of "$out")" == "mismatch:path-absent" ]]; then + pass "missing script names mismatch:path-absent" +else + fail "missing script should name its class (got: $(reason_of "$out"))" +fi rm -rf "$(dirname "$C")" -# ── 7. script outside the clone → must fail ────────────────────────────────── +# ── 7. script outside the clone → MISMATCH (exit 1) ────────────────────────── echo "7. script outside the clone" C=$(make_clone) OUTSIDE=$(mktemp) @@ -123,6 +167,48 @@ out=$("$GUARD" "$OUTSIDE" "$C" 2>&1); rc=$? if [[ $rc -eq 1 ]]; then pass "outside script exits 1"; else fail "outside script must exit 1 (got $rc)"; fi rm -f "$OUTSIDE"; rm -rf "$(dirname "$C")" +# ── 7b. detached HEAD is named, not reported as a raw content mismatch ─────── +echo "7b. detached HEAD" +C=$(make_clone) +( cd "$C" && printf '#!/bin/bash\necho TAMPERED\n' > scripts/demo.sh \ + && git add scripts/demo.sh && git commit -qm tamper && git checkout -q --detach HEAD ) +out=$("$GUARD" "$C/scripts/demo.sh" "$C" 2>&1); rc=$? +if [[ $rc -eq 1 ]]; then pass "detached HEAD exits 1"; else fail "detached HEAD must exit 1 (got $rc)"; fi +if [[ "$(reason_of "$out")" == "mismatch:detached-head" ]]; then + pass "detached HEAD names mismatch:detached-head" +else + fail "detached HEAD should name its class (got: $(reason_of "$out"))" +fi +rm -rf "$(dirname "$C")" + +# ── 7c. a branch ahead of the ref is named as such, not as a raw mismatch ──── +echo "7c. clone ahead of the ref" +C=$(make_clone) +( cd "$C" && git checkout -q -b feature \ + && printf '#!/bin/bash\necho FEATURE\n' > scripts/demo.sh \ + && git add scripts/demo.sh && git commit -qm feature ) +out=$("$GUARD" "$C/scripts/demo.sh" "$C" 2>&1); rc=$? +if [[ $rc -eq 1 ]]; then pass "clone ahead exits 1"; else fail "clone ahead must exit 1 (got $rc)"; fi +if [[ "$(reason_of "$out")" == "mismatch:clone-ahead" ]]; then + pass "clone ahead names mismatch:clone-ahead" +else + fail "clone ahead should name its class (got: $(reason_of "$out"))" +fi +if [[ "$out" == *"mid-review"* ]]; then pass "clone ahead explains it is a legitimate state"; else fail "clone ahead should explain the state"; fi +rm -rf "$(dirname "$C")" + +# ── 7d. a genuine content mismatch is named as content ────────────────────── +echo "7d. genuine content mismatch" +C=$(make_clone) +printf '#!/bin/bash\necho TAMPERED\n' > "$C/scripts/demo.sh" +out=$("$GUARD" "$C/scripts/demo.sh" "$C" 2>&1); rc=$? +if [[ "$(reason_of "$out")" == "mismatch:content" ]]; then + pass "hand-edit names mismatch:content" +else + fail "hand-edit should name mismatch:content (got: $(reason_of "$out"))" +fi +rm -rf "$(dirname "$C")" + # ── 8. --no-fetch states the freshness assumption ──────────────────────────── echo "8. --no-fetch states its assumption" C=$(make_clone) @@ -134,6 +220,20 @@ else fi rm -rf "$(dirname "$C")" +# ── 8b. contract-run.sh defaults to warn, not enforce ─────────────────────── +echo "8b. contract-run.sh default mode" +DEFAULT=$(grep -m1 'CONTRACT_REVISION_PREFLIGHT:-' "$REPO/scripts/contract-run.sh" | sed 's/.*:-//; s/}.*//') +if [[ "$DEFAULT" == "warn" ]]; then + pass "contract-run.sh defaults to warn" +else + fail "contract-run.sh default must be warn (found: '$DEFAULT')" +fi +if grep -q 'CONTRACT_REVISION_PREFLIGHT=enforce' "$REPO/scripts/contract-run.sh"; then + pass "enforce remains available and documented" +else + fail "enforce must remain documented" +fi + # ── 9. the pre-fix guard must FAIL these same cases (proves the tests bite) ── echo "9. pre-fix draft fails the same cases (bite proof)" if [[ ! -f "$PREFIX_GUARD" ]]; then -- 2.54.0