From 679cf3b489ed965788d8c17b66614af7656fc1ee Mon Sep 17 00:00:00 2001 From: SUaDtL Date: Wed, 29 Jul 2026 11:11:31 -0400 Subject: [PATCH] fix(farm): a raw NUL byte made the repo's largest source file unsearchable (#536) farm.ts carried a literal NUL character inside `parts.join(...)` - the actual byte, not the `\0` escape. ripgrep treats any file containing one as BINARY, so every content search across this repo silently skipped its largest source file and returned "binary file matches" instead of results. Hit live while investigating #515: a grep over farm.ts came back empty and the reason was not the query. git was unaffected and still is - its binary sniff reads only the first 8000 bytes and the NUL sat at 50941 - which is why diffs, blame and review always looked normal and nothing ever flagged it. NO behaviour change, and not a payload change: - farm.js rebuilds BYTE-IDENTICAL, so the emitted separator is unchanged. - the payload gate agrees: plugins/ca/tools/ is a build directory and only the declared artifact counts (#435). No ca version bump. - the crypto gate independently executed both forms under Node and confirmed sha256 over a raw-NUL join and an escape join produce the identical digest, so no setup fingerprint is invalidated and no setup cache is busted. Cleared H-09b through the auth-crypto reviewer rather than around it. The line is `createHash("sha256")`, so the guard fires correctly even though the change is one character of source spelling; the recorded pass binds exactly that one line. The first attempt was refused - I had moved the checkout out from under the reviewer mid-review, and minting then would have bound the attestation to a different tree. That refusal was right. AC-3, the durable half: a guard fails if any TRACKED file with a text extension reintroduces a raw NUL. Scoped to every tracked file rather than a list of trees, because a NUL breaks ripgrep wherever it lands and a five-tree pathspec left 335 files unscanned while the message claimed "tracked source files" unqualified. Both halves are proven by mutation, not asserted: restoring the byte is caught, AND a pathspec matching nothing is caught. The second matters because `git ls-files` exits 0 on a pathspec that matches nothing - measured - so the returncode check alone would pass vacuously forever. The scanned-count floor is the control that actually does the work. Confirmed fixed with the tool that could not read it: ripgrep now returns line-numbered matches from farm.ts. Claude-Session: https://claude.ai/code/session_01WJgVfZw7J81PB7mwpHyUxx --- .github/scripts/test_ci_impact.py | 67 +++++++++++++++++++++++++++++++ plugins/ca/tools/farm.ts | 2 +- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/.github/scripts/test_ci_impact.py b/.github/scripts/test_ci_impact.py index 9e814c19..5d38fcdc 100644 --- a/.github/scripts/test_ci_impact.py +++ b/.github/scripts/test_ci_impact.py @@ -11,6 +11,7 @@ import json import fnmatch import re +import subprocess import sys import tempfile import tomllib @@ -2500,6 +2501,72 @@ def test_every_tested_tree_has_its_coverage_command_documented(self): "these trees run tests but tech-stack.md names no coverage command for them, so " "tdd Phase 5 and refactor Phase 2/6 cannot run there: " + ", ".join(undocumented)) + def test_no_tracked_source_file_contains_a_raw_nul_byte(self): + """Issue #536: a raw NUL makes ripgrep treat a source file as BINARY. + + `farm.ts` carried one inside `parts.join("")` — written as the + literal character instead of the `\\0` escape. It behaved correctly, and + git diffed it normally (git's binary sniff reads only the first 8000 + bytes and the NUL sat well past that), so nothing caught it. But every + content search across the repo silently skipped the largest source file + in it, returning "binary file matches" instead of results. + + A search tool that quietly excludes a file is worse than one that errors: + the answer looks complete. Cheap to prevent, invisible once reintroduced. + """ + # Driven from `git ls-files`, not an rglob: the claim is about TRACKED + # sources, and a filesystem walk also picks up gitignored build output — + # an untracked `site/coverage/` was present the first time this ran. A + # generated artifact carrying a NUL would then fail this test with a + # message blaming a source file, which is a worse failure than the one it + # exists to catch. + # + # EVERY tracked file, with no pathspec. A first cut listed five trees and + # left 335 tracked files of the same extensions unscanned — `.codearbiter/`, + # `docs/`, the workflows themselves — while the failure message claimed + # "tracked source files" without qualification. A NUL breaks ripgrep + # wherever it lands, so the scope should match the claim rather than the + # other way round. The extension allowlist below is what keeps genuinely + # binary files out. + listed = subprocess.run( + ["git", "ls-files", "-z"], + cwd=REPO_ROOT, capture_output=True, timeout=120, + ) + self.assertEqual(listed.returncode, 0, "git ls-files failed, so nothing was scanned") + offenders = [] + scanned = 0 + for entry in listed.stdout.split(b"\0"): + if not entry: + continue + rel = entry.decode("utf-8", "surrogateescape") + if not rel.endswith((".ts", ".js", ".mjs", ".py", ".md", ".json", ".yml", ".sh")): + continue + path = REPO_ROOT / rel + try: + content = path.read_bytes() + except OSError: + continue + scanned += 1 + if b"\x00" in content: + offenders.append(rel) + + # A pathspec that matches nothing passes this test vacuously forever. The + # repo has hundreds of files under those roots; 100 is a floor no real + # state falls below and a typo'd pathspec cannot clear. + # `git ls-files` exits 0 on a pathspec that matches NOTHING, so the + # returncode check above cannot catch a typo — measured. This floor is + # what actually does the work. + self.assertGreater( + scanned, 500, + f"only {scanned} tracked files were scanned - the pathspec matched almost " + "nothing, so a clean result here proves nothing") + self.assertEqual( + sorted(offenders), [], + "these tracked source files contain a raw NUL byte, so ripgrep treats them as " + "binary and skips them in every content search - use the \\0 escape instead: " + + ", ".join(sorted(offenders)), + ) + def test_no_coverage_exemption_is_stale(self): """An exemption for a tree that no longer exists silently widens the hole.""" trees = set(self._tested_trees()) diff --git a/plugins/ca/tools/farm.ts b/plugins/ca/tools/farm.ts index 75aa7885..3b2d52f9 100644 --- a/plugins/ca/tools/farm.ts +++ b/plugins/ca/tools/farm.ts @@ -1060,7 +1060,7 @@ async function setupFingerprint(wt: string, t: Task, deps: RunTaskDeps): Promise const parts = [JSON.stringify(t.setup ?? [])]; for (const rel of t.setupInputs ?? []) parts.push(`${rel}=${(await deps.fileHash(path.resolve(wt, rel))) ?? "absent"}`); - return createHash("sha256").update(parts.join("")).digest("hex"); + return createHash("sha256").update(parts.join("\0")).digest("hex"); } /** Runs both setup phases for one attempt. Returns a redacted note on failure, null on success. */