fix(gate): the wire-contract search shelled out to grep and read its failure as a finding
gates / gates (push) Successful in 33s
gates / gates (push) Successful in 33s
CI convicted ALL 174 checked tags while the pre-push hook was green. Cause, read from the run log rather than guessed at the second attempt: the search used `grep -rnE --include=…`, and the CI runner's image carries python3 and git and deliberately little else — its grep does not support `--include`, so stdout was empty and the gate read empty as "the tag is absent". That is a gate silently treating a tool failure as a finding, which is worse than no gate, and it is exactly the error-swallowing this repo forbids. A green from it would have been just as untrustworthy as the red. Fixed by removing the dependency, not by working around it: the search is now pure Python — one token index per receiving repo, built in a single pass, no subprocess. Faster too (one walk instead of ~350 greps), and unreadable-file / empty-repo cases now exit 2 INCONCLUSIVE rather than reporting absence. THE BEFORE CAPTURE WAS RE-VERIFIED, NOT RE-GENERATED — the stronger claim. All 40 fields recorded in BEFORE.md were re-tested against the new implementation: agree=40, disagree=0, i.e. exactly the four this session fixed are now present and the other 36 still absent. The number 40 stands under both implementations; only the mechanism changed. The whole-token property survives by construction — a token index treats `healed_at` and `privsep_healed_at` as distinct tokens. This is the THIRD instrument defect this gate's own controls caught before it was trusted, after the substring false negative and the dr_recipe over-opacity. The first two were caught by re-finding the known instances; this one by the CI-versus-hook disagreement the workflow's alarm mail explicitly says outranks whatever the push was for.
This commit is contained in:
@@ -75,3 +75,33 @@ exit=1
|
||||
SELFTEST OK — the gate convicts a planted unreachable tag and the plant is the only difference.
|
||||
exit=0
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ⚠ The search implementation changed AFTER this capture, and the capture was re-verified
|
||||
|
||||
The run above used `grep -rnE --include=…` to test whether a tag occurs in the receiving repo. That
|
||||
**works on a workstation and returns nothing on the CI runner**, whose image carries python3 and git
|
||||
and deliberately little else — its `grep` does not support `--include`. Empty stdout was then read as
|
||||
"the tag is absent", so the gate convicted **all 174** checked tags and CI went red while the
|
||||
pre-push hook was green (runs 260–262).
|
||||
|
||||
That is the gate silently reading a tool failure as a finding, which is worse than no gate, and it is
|
||||
the error-swallowing this repo's rules forbid. The search is now **pure Python**: one token index per
|
||||
receiving repo, no subprocess, no external dependency.
|
||||
|
||||
**This capture was NOT re-generated — it is re-verified**, which is the stronger claim. Every one of
|
||||
the 40 fields recorded above was re-tested against the new implementation:
|
||||
|
||||
```
|
||||
fields recorded in BEFORE.md: 40
|
||||
agree=40 disagree=0
|
||||
```
|
||||
|
||||
— i.e. the new implementation finds exactly the four this session fixed (`operator_key_configured`,
|
||||
`wg_handshake_age_s`, `healed_at`, `escrow_stale`) present, and the other 36 still absent. **The
|
||||
number 40 stands under both implementations**; only the mechanism and its portability changed.
|
||||
|
||||
The whole-token property also survives by construction: a token index treats `healed_at` and
|
||||
`privsep_healed_at` as distinct tokens, so the substring false negative that the control caught
|
||||
cannot come back.
|
||||
|
||||
@@ -59,7 +59,6 @@ gate must publish its holes.
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
|
||||
@@ -338,23 +337,50 @@ def walk(by_dir, by_name, start_dir, start_type, seen=None, prefix=""):
|
||||
return out
|
||||
|
||||
|
||||
def receiver_has(tag, repo_root):
|
||||
"""Whole-token match, NOT substring.
|
||||
TOKEN_RE = re.compile(r"[A-Za-z0-9_]+")
|
||||
|
||||
A plain `grep -F healed_at` also matches `privsep_healed_at`, so a genuinely dropped field is
|
||||
reported as received. That false NEGATIVE was found by running this gate's control (R-260's
|
||||
list) and noticing one known field missing from the output — which is the whole argument for
|
||||
making a gate re-find the instances it was written for before trusting it.
|
||||
|
||||
def receiver_tokens(repo_root):
|
||||
"""Every `[A-Za-z0-9_]+` token in the receiver's PRODUCTION Go and templates, as a set.
|
||||
|
||||
WHOLE-TOKEN, NOT SUBSTRING. A substring test reports a genuinely dropped field as received:
|
||||
`healed_at` occurs inside `privsep_healed_at`. That false negative was caught by this gate's own
|
||||
control — R-260 named `healed_at`, so its absence from the first run's output was the tell.
|
||||
|
||||
PURE PYTHON, NO `grep`. The first version shelled out to `grep -rnE --include=...`, which works
|
||||
on a workstation and returns NOTHING on the CI runner, whose image carries python3 and git and
|
||||
deliberately little else — its grep does not support `--include`. Empty stdout was then read as
|
||||
"the tag is absent", so the gate convicted ALL 174 checked tags and CI went red while the
|
||||
pre-push hook was green. **A gate that silently reads a tool failure as a finding is worse than
|
||||
no gate**, and swallowing that error is the exact thing this repo's rules forbid. Reading the
|
||||
files here removes the dependency, is portable, and is one pass instead of ~350 subprocesses.
|
||||
|
||||
Test files and `testdata/` are excluded deliberately: a tag present only in a fixture is not
|
||||
decodable by production — and that is R-262 exactly, where the cross-repo golden carries two
|
||||
fields no hub struct reads.
|
||||
"""
|
||||
pattern = r"(^|[^A-Za-z0-9_])" + re.escape(tag) + r"([^A-Za-z0-9_]|$)"
|
||||
r = subprocess.run(
|
||||
["grep", "-rnE", "--include=*.go", "--include=*.html", pattern, repo_root],
|
||||
capture_output=True, text=True)
|
||||
for line in r.stdout.splitlines():
|
||||
if "_test.go" in line or "/testdata/" in line:
|
||||
continue
|
||||
return True
|
||||
return False
|
||||
toks = set()
|
||||
n = 0
|
||||
for dp, dn, fn in os.walk(repo_root):
|
||||
dn[:] = [d for d in dn if d not in (".git", "vendor", "node_modules", "testdata")]
|
||||
for f in fn:
|
||||
if f.endswith("_test.go"):
|
||||
continue
|
||||
if not (f.endswith(".go") or f.endswith(".html")):
|
||||
continue
|
||||
p = os.path.join(dp, f)
|
||||
try:
|
||||
with open(p, encoding="utf-8", errors="replace") as fh:
|
||||
toks.update(TOKEN_RE.findall(fh.read()))
|
||||
except OSError as e:
|
||||
# never swallowed: an unreadable source file makes the answer unknown, not "absent"
|
||||
die("wire-contract gate INCONCLUSIVE: cannot read %s: %s" % (p, e))
|
||||
n += 1
|
||||
if n == 0:
|
||||
die("wire-contract gate INCONCLUSIVE: no production .go/.html found under %s — a receiver "
|
||||
"with no source cannot be searched, and an empty search is not evidence of absence."
|
||||
% repo_root)
|
||||
return toks
|
||||
|
||||
|
||||
def run(root_override=None, quiet=False):
|
||||
@@ -368,6 +394,8 @@ def run(root_override=None, quiet=False):
|
||||
" This gate compares two repositories; it cannot pass without both." % (label, path))
|
||||
|
||||
indexes = {k: build_index(v) for k, v in repos.items()}
|
||||
# one pass per receiving repo, not one subprocess per tag
|
||||
rtokens = {k: receiver_tokens(v) for k, v in repos.items()}
|
||||
convictions = []
|
||||
checked = skipped = 0
|
||||
|
||||
@@ -396,7 +424,7 @@ def run(root_override=None, quiet=False):
|
||||
skipped += 1
|
||||
continue
|
||||
checked += 1
|
||||
if not receiver_has(tag, repos[receiver]):
|
||||
if tag not in rtokens[receiver]:
|
||||
missing.append((tag, dotted))
|
||||
if missing:
|
||||
convictions.append((label, receiver, missing))
|
||||
|
||||
Reference in New Issue
Block a user