Skip to content

Commit a05103f

Browse files
authored
fix(asvs): refuse an abbreviated anchor SHA — checkout reads a short ref as a branch name (#134)
* fix(asvs): refuse an abbreviated anchor SHA -- checkout reads a short ref as a branch name actions/checkout resolves ref: as a BRANCH OR TAG name unless it is the full 40-character hash. An abbreviated anchor_commit therefore dies with A branch or tag with the name 8f01cef could not be found -- emitted after it ran git branch --list --remote origin/8f01cef8 and git tag --list 8f01cef, neither of which is what the field means. check_pinning now refuses anything that is not [0-9a-f]{40}, so this is caught at authoring time instead of in CI. Two tests, the negative one proved red first. FOURTH instance of one defect in this tool in a day, and all four are the same statement: the gate failed for a reason having nothing to do with the property it claims to measure. drift gate compared a CI-injected SHA -> could never pass engine checkout bare SHA, no full history -> could never resolve anchor commit pinned to a branch-only commit -> died when the branch went anchor commit abbreviated -> read as a branch name Worth recording rather than quietly fixing: I wrote the invariant that names this class today, in this tools own docstrings, and then shipped four violations of it. The discipline is not enforceable by attention -- every one was caught by CI going red. Refusing the short SHA moves this one from caught-late to cannot-be-written. * fix(asvs): an absence claim must prove its pattern CAN fire, not just that it is quiet The V5 sweep authored every absence claim in the chapter as a prose narration of a shell command -- pattern = "rg -n 'tar.extractall' messagefoundry/ -> exit 1 (zero hits)" where the field wanted `tar\.extractall\(`. The field's type is `str`; prose is a valid `str`. The shape permitted it, and only a detector (the BLIND check) caught it -- nine claims, one chapter, all of which would have shipped proving nothing. `mutation` closes it: the realistic REINTRODUCTION the pattern claims to exclude, and the pattern must actually match it. Prose cannot fire on its own stated reintroduction, so a prose pattern stops being detectable-and-caught and becomes unwritable. INERT is checked BEFORE the corpus, because a live positive control must not launder a pattern that could never have matched anything. BLIND and INERT are different failures and the messages say so: BLIND means the search could not have SEEN the thing; INERT means the pattern could not have MATCHED it. THE TRAP INSIDE THIS FIX, recorded because it is more dangerous than the bug: the obvious convenience is to derive `mutation` from `pattern` -- unescape it, strip the anchors, default it in. That makes the check VACUOUS by construction. A value generated from the thing it validates always satisfies it, so it would pass forever, on every claim, including the prose ones, while looking like the strongest check in the file. That is the same defect class this closes, arriving through the fix. Hence no default, a load-time refusal that says author it, and a docstring that says why. The cost, accepted rather than dodged: 47 absence claims already exist without mutations, and this grandfathers none of them. They must be authored -- not generated -- before the anchor moves to a commit carrying this code. A required check that exempts everything already written is the `partial` verdict I would give anyone else. Both new tests were proved red first by neutering ONLY the new guard, leaving the rest of the module intact, so they fail for the reason claimed rather than because the file stopped importing. * fix(asvs): the mutation guidance contaminated the corpus it tells you to search This module lives INSIDE the corpus that absence patterns are searched over (scripts/ is one of the four scanned package dirs). The guidance string I added alongside the new `mutation` field carried a literal archive-extraction call as an example -- so the example itself became a real corpus hit. Two live absence claims (5.2.5, 5.3.3) flipped to FALSE the moment they were backfilled: their patterns exclude archive extraction, and the thing they matched was this file's own advice about how to write them. The engine is unaffected and still archive-safe; the defect was entirely in the checker's prose. THIRD instance today of a remedy carrying the disease, and the most literal of the three: - a drift-gate fix that pinned the tree, so the gate could no longer see drift - a mutation derived from its own pattern, vacuous by construction (caught first) - the DOCUMENTATION of a check breaking the check Found by the backfill run, which reported both failures rather than tuning the two claims to pass -- had it 'fixed' them by narrowing the patterns, the contamination would have survived and two real archive claims would have been silently weakened. The example now lives only in the Absence docstring, in escaped-regex form, which cannot self-match. The regression test is an INSTANCE lock, not a class lock, and says so: it pins the literal that actually bit. The general rule -- write examples as escaped regex -- is not mechanically enforceable. The test was proved red twice. The first attempt was a BAD red: reintroducing the literal inside a string broke the file syntactically, so the test failed on a collection error rather than on the assertion. Re-done as a comment, which contaminates the corpus identically while leaving the module parseable -- the test then failed on its own assertion, which is the only red worth anything. * fix(asvs): an ambiguous anchor certifies nothing -- require the token to be unique in its file MEASURED FIRST, because I had just asserted this was unmeasurable. 46 of 292 anchors (15%) cite a token that occurs more than once in its file: occurrences: {1: 246, 2: 20, 3: 11, 4: 5, 5: 2, 7: 3, 10: 1, 11: 1, 21: 1, 89: 1, 101: 1} The tail is not 'at risk'. `await conn.rollback()` occurs 101 times in sqlserver.py, so ANY line number in that file lands within +/-40 of some occurrence: the anchor CANNOT fail and certifies nothing. It has been standing as evidence for 2.3.3 for weeks, green throughout. Same for the 89x sibling. Those two were not anchors degrading toward hollow -- they were already hollow and reporting success. This also closes a defect in the REPAIR path rather than the detection path, which is a shape the taxonomy did not have. Everything else catalogued today fails by staying silent; this one fails by speaking CORRECTLY and then handing you a repair it cannot validate. After ADR 0154 landed, 11 anchors went stale and the gate reported all 11 -- correct. But re-anchoring to the nearest occurrence would have silently mis-anchored two: 'UPDATE sessions SET revoked_at=' now has 2 occurrences 19 lines apart -- one the keep-N revoke, one a DIFFERENT method ('sign out everywhere else'). Each falls inside the other's window, so the check accepts either. A wrong repair is strictly harder to catch than the fault it replaces: stale-and-broken is re-detected on the next run, stale-but-resolving never is. And the repair is exactly where suspicion lapses, because the tool has just proved it works. Uniqueness makes the ambiguity UNREPRESENTABLE rather than undetectable: with one occurrence there is no wrong one to pick, and the 101x anchor is refused at authoring time instead of certifying a cell indefinitely. Both tests proved against a neutered guard -- the negative reds, and the positive still passes, so the check is not simply rejecting everything. That second half matters here: a guard that rejects all anchors would also make the negative test green. No grandfathering. The 46 need longer tokens and I am fixing all 46 before this merges, same reason as the 47-claim mutation backfill. Recorded because it is the reason this exists: I told a peer session the hollow fraction was 'unobservable from inside the system' and could not be measured without redoing the work. That was a claim generalised one step past the evidence, made in a message about claims generalised one step past the evidence. The measurement is fifteen lines. 'Unmeasurable' ends inquiry the way a green light does, and is more comfortable because it sounds like rigour about limits. * fix(asvs): a decided verdict with no evidence at all was invisible to the gate 14 of 59 decided cells carry ZERO anchors and ZERO absence claims. Nothing about them is verified, and the gate could not say so, because check_anchors iterates the evidence a cell HAS. A cell with none is skipped and fails nothing: the check could only ever validate evidence that existed, never assert that it must exist. 1.2.2 3.7.3 4.2.1 4.4.1 6.3.3 7.1.3 7.5.2 11.3.3 11.7.1 12.2.2 13.3.2 16.3.2 16.4.2 (+1) Most are inherited from the prose lineage -- verdicts that were REACHED but never ANCHORED -- and ten of them are the same cells the 2026-07-31 reconciliation recovered after they were dropped from an enumeration. They came back with verdicts attached and no evidence, which is the exact conflation ADR 0156 exists to prevent: a guess wearing a verdict's clothes. docs/ASVS-ASSESSMENT-METHOD.md section 3 says 'every non-unverified cell carries at least one anchor'. It was policy, and nothing enforced it. FOURTH 'cannot observe itself' defect in this tool, and the only one about absence of evidence rather than quality of it. The other three were the frozen checkout, the paths filter that excluded the workflow, and the trigger set that excluded the subject. This one is subtler: it is not that the check was pointed at the wrong thing, it is that the check's DOMAIN was the set of things that already existed. A rule quantified over what is present cannot notice what is missing. An absence claim counts as evidence: a `fail` is often proved by absence rather than presence, so the rule is >=1 anchor OR >=1 absence claim. Both halves tested. Two existing fixtures went red when this landed, correctly -- they carried decided verdicts with no evidence. Fixed the fixtures rather than the rule, and said so in the helper's docstring so the next person does not read the placeholder anchor as cargo. MEASURED CORRECTION to something I told a peer: they suggested the hollow-anchor problem meant cell 2.3.3 was 'certified on anchors incapable of failing' and asked whether the survey percentage was ever real. Measured: NO decided cell rests entirely on ambiguous anchors (2.3.3 has 4 ambiguous of 8), so the count is not overstated on that basis. The real hole was this one, which neither of us was looking at -- worse in kind, since 14 cells have no evidence at all rather than weak evidence.
1 parent 82b4a94 commit a05103f

2 files changed

Lines changed: 322 additions & 7 deletions

File tree

scripts/asvs/scorecard.py

Lines changed: 95 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,22 @@ class Absence:
7777
absence claims survived for weeks. So an absence claim is only admissible with a
7878
``positive_control`` that must still match; if the control goes quiet the search has gone blind and
7979
the claim is void, regardless of what the pattern returns.
80+
81+
``mutation`` closes the hole the control does not: it is the realistic REINTRODUCTION this pattern
82+
claims to exclude, and the pattern must actually fire on it. A whole chapter of claims was once
83+
authored as prose narrations of shell commands — ``"rg -n 'tar.extractall' -> exit 1 (zero hits)"``
84+
where the field wanted ``tar\\.extractall\\(``. The field's type is ``str`` and prose is a valid
85+
``str``, so the shape permitted it and only a detector caught it. Requiring the pattern to match a
86+
stated reintroduction makes prose *unwritable* rather than merely detectable.
87+
88+
Do NOT derive ``mutation`` from ``pattern``. A value generated from the thing it validates
89+
satisfies the check by construction, which would make this the most authoritative-looking vacuous
90+
gate in the file — the same defect class it exists to close, arriving through the fix.
8091
"""
8192

8293
pattern: str
8394
positive_control: str
95+
mutation: str
8496

8597

8698
@dataclass(frozen=True)
@@ -171,6 +183,21 @@ def load_scorecard(path: Path) -> list[Cell]:
171183
"recording the reason for non-applicability is the one MUST in ASVS 5.0's assessment "
172184
"chapter (docs/ASVS-ASSESSMENT-METHOD.md §1)"
173185
)
186+
for a in raw.get("absence", []):
187+
if not str(a.get("mutation", "")).strip():
188+
raise ScorecardError(
189+
# NO LITERAL CODE EXAMPLE HERE. This module is inside the corpus that absence
190+
# patterns are searched over, so an illustrative call in this string becomes a
191+
# real corpus hit and reads as FALSE. The first draft used one and broke two
192+
# live claims (5.2.5, 5.3.3) the moment they were backfilled: the guidance for
193+
# a check contaminated the check. The Absence docstring carries the example in
194+
# escaped-regex form, which cannot self-match.
195+
f"cell {raw.get('id')!r}: absence claim {a.get('pattern')!r} has no `mutation` — "
196+
"state the realistic reintroduction this pattern excludes, so the pattern can "
197+
"be proved capable of firing. Author it from what the code would look like if "
198+
"the thing came back; do NOT derive it from the pattern, which makes the check "
199+
"vacuous. See the Absence docstring for a worked example"
200+
)
174201
cells.append(
175202
Cell(
176203
id=str(raw["id"]),
@@ -186,7 +213,13 @@ def load_scorecard(path: Path) -> list[Cell]:
186213
for e in raw.get("evidence", [])
187214
),
188215
absence=tuple(
189-
Absence(pattern=str(a["pattern"]), positive_control=str(a["positive_control"]))
216+
Absence(
217+
pattern=str(a["pattern"]),
218+
positive_control=str(a["positive_control"]),
219+
# No default. A missing mutation must be authored, not inferred — see the
220+
# Absence docstring on why deriving one from the pattern is worse than none.
221+
mutation=str(a["mutation"]),
222+
)
190223
for a in raw.get("absence", [])
191224
),
192225
)
@@ -231,25 +264,65 @@ def check_completeness(cells: list[Cell], corpus: dict[str, int]) -> list[str]:
231264
want = corpus.get(c.id)
232265
if want is not None and c.level != want:
233266
problems.append(f"{c.id}: level {c.level} but the corpus says L{want}")
267+
268+
# A DECIDED verdict with no evidence at all is the conflation this whole tool exists to prevent:
269+
# a guess wearing a verdict's clothes. The method is explicit — every non-`unverified` cell carries
270+
# at least one anchor — but nothing enforced it, because `check_anchors` iterates the evidence a
271+
# cell HAS. A cell with none is not checked and fails nothing; the gate could only ever validate
272+
# evidence that existed, never assert that it must. Measured when this landed: 14 of 59 decided
273+
# cells carried zero anchors AND zero absence claims, several inherited from the prose lineage
274+
# where the verdict was reached but never anchored.
275+
unevidenced = sorted(
276+
(c.id for c in cells if c.verdict in DECIDED_VERDICTS and not c.evidence and not c.absence),
277+
key=_sort_key,
278+
)
279+
if unevidenced:
280+
problems.append(
281+
f"evidence: {len(unevidenced)} decided cell(s) carry NO anchor and NO absence claim, so "
282+
"nothing about them is verified — either anchor them or return them to `unverified`, "
283+
f"which is what an unevidenced verdict actually is: {', '.join(unevidenced)}"
284+
)
234285
return problems
235286

236287

237288
def check_anchors(cells: list[Cell], root: Path, findings: Findings) -> None:
238-
"""Open every evidence anchor and assert its token still resolves.
289+
"""Open every evidence anchor and assert its token still resolves, and resolves UNAMBIGUOUSLY.
239290
240291
When the code moves, this reds a test — instead of the sentence rotting in place and the next
241292
session funding work that is already done.
293+
294+
**Uniqueness is not pedantry; it is what makes the resolution mean anything.** An ``expect`` that
295+
occurs many times in its file resolves from almost anywhere: with ``await conn.rollback()``
296+
appearing 101 times in one module, *any* line number in that file lands within ±40 of some
297+
occurrence, so the anchor cannot fail and certifies nothing. Two such anchors sat in this scorecard
298+
as evidence for weeks.
299+
300+
It also closes a defect in the REPAIR path rather than the detection path. When code moves, the
301+
check correctly reports it — but a re-anchor to the nearest occurrence can silently install a
302+
*stale-but-resolving* anchor that passes forever. That happened live: after ADR 0154 landed,
303+
``UPDATE sessions SET revoked_at=`` had two occurrences 19 lines apart — one the keep-N revoke, one
304+
a different method entirely — each inside the other's window, so the check would have accepted the
305+
wrong one. A repair is exactly where suspicion lapses, because the tool has just proved it works.
242306
"""
243307
for c in cells:
244308
for a in c.evidence:
245309
target = root / a.path
246310
if not target.is_file():
247311
findings.problems.append(f"{c.id}: evidence path {a.path} does not exist")
248312
continue
249-
lines = target.read_text(encoding="utf-8", errors="replace").splitlines()
313+
text = target.read_text(encoding="utf-8", errors="replace")
314+
lines = text.splitlines()
250315
lo = max(0, a.line - 1 - ANCHOR_WINDOW)
251316
hi = min(len(lines), a.line + ANCHOR_WINDOW)
252317
findings.checked_anchors += 1
318+
occurrences = text.count(a.expect)
319+
if occurrences > 1:
320+
findings.problems.append(
321+
f"{c.id}: {a.path}:{a.line} anchor is AMBIGUOUS — {a.expect!r} occurs "
322+
f"{occurrences} times in the file, so the line number is not load-bearing and a "
323+
"re-anchor cannot be checked. Cite a longer token that appears exactly once"
324+
)
325+
continue
253326
if a.expect in "\n".join(lines[lo:hi]):
254327
continue
255328
where = " (found elsewhere in the file)" if a.expect in "\n".join(lines) else ""
@@ -265,6 +338,14 @@ def check_absences(cells: list[Cell], root: Path, findings: Findings) -> None:
265338
for c in cells:
266339
for a in c.absence:
267340
findings.checked_absences += 1
341+
# Before asking what the corpus says, ask whether the pattern is a pattern at all. A prose
342+
# narration greps to nothing and is indistinguishable from a true absence.
343+
if not re.search(a.pattern, a.mutation):
344+
findings.problems.append(
345+
f"{c.id}: absence claim is INERT — {a.pattern!r} does not match its own stated "
346+
f"reintroduction {a.mutation!r}, so it would stay quiet if the thing came back"
347+
)
348+
continue
268349
control = _grep_count(a.positive_control, corpus_files)
269350
if control == 0:
270351
findings.problems.append(
@@ -328,6 +409,17 @@ def check_pinning(scorecard: Path, corpus: Path) -> list[str]:
328409
"pinning: [scorecard].asvs_version is missing — bare requirement ids re-point across "
329410
"ASVS versions, so the scorecard must say which version its ids mean"
330411
)
412+
anchor = str(meta.get("anchor_commit", "")).strip()
413+
# actions/checkout resolves `ref:` as a BRANCH OR TAG name unless it is a full 40-char hash, so an
414+
# abbreviated anchor fails with "A branch or tag with the name ... could not be found" -- a gate
415+
# failing for a reason with nothing to do with what it measures. Caught in CI twice; refused here.
416+
if anchor and not re.fullmatch(r"[0-9a-f]{40}", anchor):
417+
problems.append(
418+
f"pinning: [scorecard].anchor_commit {anchor!r} must be a FULL 40-character SHA -- an "
419+
"abbreviated hash is not resolvable by actions/checkout, which treats a short ref as a "
420+
"branch or tag name"
421+
)
422+
331423
declared = str(meta.get("corpus_sha256", "")).strip()
332424
actual = corpus_digest(corpus)
333425
if not declared:

0 commit comments

Comments
 (0)