feat: land the revision-preflight guard, fixed and wired into contract execution
PR Pipeline — Authorize → Validate → Review → Merge / auth (pull_request) Successful in 6s
PR Pipeline — Authorize → Validate → Review → Merge / validate (pull_request) Successful in 4s
PR Pipeline — Authorize → Validate → Review → Merge / lint (pull_request) Failing after 6s
PR Pipeline — Authorize → Validate → Review → Merge / ai-review (pull_request) Successful in 2s
PR Pipeline — Authorize → Validate → Review → Merge / gate (pull_request) Skipped

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.
This commit is contained in:
root
2026-09-25 11:04:29 +00:00
parent 9d64b0bd66
commit 574cb99d76
5 changed files with 477 additions and 0 deletions
+90
View File
@@ -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 <script-path> <clone-path>`:
* 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 `<ref>:<repo-relative-path>`;
* **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`.