From 835c742c79025c8d89dc1d2010ae06b1019da266 Mon Sep 17 00:00:00 2001 From: Samhit21 Date: Mon, 17 Aug 2026 16:35:49 -0500 Subject: [PATCH] chore(repo): replace four literal NUL bytes with the escape, gate the class (#787) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four source files carried a raw 0x00 where the six-character JS escape belongs. Identical at runtime — the byte is only ever reached for as a collision-free join separator, and the reasoning is sound each time — but it makes grep and git grep exit 1 with no output at all over the whole file, and renders the blob as Bin on GitHub whenever it lands in the first ~8000 bytes. JobRepostArchiveDialog.tsx (offset 4149) was on the dark side of that threshold; the other three read as normal text diffs while every search over them lied. Encoding-only, so no test moves. Add scripts/check-no-literal-nul.mjs to stop the class coming back. #786 shipped a fifth instance through review and PR #125 fixed a sixth in June: the construct is individually correct every time someone reaches for it, so a rule does not hold and a gate has to. It walks git ls-files, skips binary formats by declared extension — by path, deliberately, since content sniffing keys on the very byte being hunted and would skip exactly the files it exists to catch — and reports file:line:column with the byte offset. Wired into verify and into verify:quick. It belongs in the inner loop by that gate's own criterion: it costs ~50ms, and all four occurrences were in .ts under src/, which is precisely what the Stop sentinel fires on. Closes #787 --- package.json | 5 +- scripts/check-no-literal-nul.mjs | 176 ++++++++++++++++++ scripts/check-no-literal-nul.test.mjs | 123 ++++++++++++ .../features/JobRepostArchiveDialog.tsx | Bin 7426 -> 7431 bytes src/hooks/useJobSearch.ts | 4 +- src/lib/heuristics/extract/education.ts | 2 +- src/lib/heuristics/fixture-match.ts | 2 +- 7 files changed, 306 insertions(+), 6 deletions(-) create mode 100644 scripts/check-no-literal-nul.mjs create mode 100644 scripts/check-no-literal-nul.test.mjs diff --git a/package.json b/package.json index 46d9a488..d4e3e39f 100644 --- a/package.json +++ b/package.json @@ -29,11 +29,12 @@ "typecheck": "tsc -b --noEmit", "lint": "eslint .", "check:fixtures": "node scripts/check-fixture-pii.mjs", + "check:nul": "node scripts/check-no-literal-nul.mjs", "check:baselines": "node scripts/check-known-failures.mjs", "check:core": "node scripts/check-core-package.mjs", "test:coverage": "vitest run --coverage", - "verify:quick": "tsc -b --noEmit && eslint . && npm run test:changed", - "verify": "tsc -b --noEmit && eslint . && npm run check:fixtures && npm run check:baselines && npm run check:core && npm run test:changed && vite build && (fallow audit --base origin/main || echo 'fallow audit exited non-zero (report-only, ignored)')" + "verify:quick": "tsc -b --noEmit && eslint . && npm run check:nul && npm run test:changed", + "verify": "tsc -b --noEmit && eslint . && npm run check:nul && npm run check:fixtures && npm run check:baselines && npm run check:core && npm run test:changed && vite build && (fallow audit --base origin/main || echo 'fallow audit exited non-zero (report-only, ignored)')" }, "dependencies": { "@fontsource/poppins": "^5.2.7", diff --git a/scripts/check-no-literal-nul.mjs b/scripts/check-no-literal-nul.mjs new file mode 100644 index 00000000..51a3de6e --- /dev/null +++ b/scripts/check-no-literal-nul.mjs @@ -0,0 +1,176 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 The offlinecv Authors + +/** + * Literal-NUL gate (#787). Fails the build when a tracked text file carries a + * raw `0x00` byte. + * + * WHY A GATE RATHER THAN A RULE. The construct that produces these is + * individually CORRECT every time someone reaches for it: a NUL is the one byte + * that cannot occur in a résumé field, so joining a composite key with it is + * collision-free by construction. Only the ENCODING is wrong — the byte gets + * written where the six-character escape belongs, and the two are identical at + * runtime. Nothing about the reasoning is faulty, so "remember not to do this" + * does not survive contact with the next person who needs a separator. #787 + * found four occurrences on `main`; #786 shipped a fifth through review and PR + * #125 fixed a sixth in June. That is a class, and a class needs a machine. + * + * WHAT IT COSTS TO MISS ONE. Two failure modes, and the loud one is the one + * that usually does not fire: + * + * 1. SEARCH BREAKS SILENTLY, at any offset. `grep` treats the file as binary + * and exits 1 with no output at all — no match, no `Binary file matches`, + * no error. It reads exactly like "that symbol is not defined." `git grep` + * goes quiet the same way; `rg` at least says `binary file matches`. + * `grep -a` / `rg -a` recover, but only if you already suspect. + * 2. THE DIFF GOES DARK ONLY SOMETIMES. Git sniffs for binary content over + * the head of the blob, so a NUL in the first ~8000 bytes renders the file + * as `Bin` on GitHub — unreviewable, no inline comments — while a NUL past + * that renders a perfectly normal text diff over a file every `grep` lies + * about. That asymmetry is how #786's instance (offset 46379) passed + * review: the diff looked completely fine. + * + * WHY AN EXTENSION SKIP-LIST AND NOT CONTENT SNIFFING. This is the trap in + * writing this gate, and it is worth stating so nobody "improves" it into + * uselessness: the obvious implementation skips files that *look* binary, and a + * NUL byte is precisely what every binary-detection heuristic keys on. Such a + * gate skips exactly the files it exists to catch and passes forever. So the + * skip is by PATH, declared up front in `BINARY_EXTENSIONS`, and anything not on + * that list is judged on its bytes no matter what they contain. A new binary + * asset type is a deliberate edit here — which is the correct amount of friction + * for teaching a correctness gate to ignore a file. + * + * Run: npm run check:nul + */ + +import { execFileSync } from "node:child_process"; +import { readFileSync, statSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const ROOT = fileURLToPath(new URL("..", import.meta.url)); + +/** + * Paths whose bytes are legitimately binary, matched on the lowercased suffix. + * + * Every entry is a format that carries NULs as a matter of course, so scanning + * it would report thousands of findings about files nobody edits by hand. The + * live ones in this repo are the two Poppins faces under `src/assets/fonts/` + * (~15k NULs each) and the 58 PDF fixtures under `tests/fixtures/pdfs/`; the + * rest are here so the first `.png` or `.woff2` someone commits does not fail + * the build for a reason that has nothing to do with them. + */ +const BINARY_EXTENSIONS = [ + ".avif", ".docx", ".gif", ".gz", ".ico", ".jpeg", ".jpg", ".mp4", ".node", + ".otf", ".pdf", ".png", ".swf", ".tgz", ".ttf", ".wasm", ".webp", ".woff", + ".woff2", ".zip", +]; + +/** True when `relPath` names a format whose bytes are meant to be binary. */ +export function isBinaryPath(relPath) { + const lower = relPath.toLowerCase(); + return BINARY_EXTENSIONS.some((ext) => lower.endsWith(ext)); +} + +/** + * Every literal NUL in `buffer`, as `{ line, column, offset }` with 1-indexed + * line and column. + * + * Reported per occurrence rather than as a count because the fix is per site, + * and a `file:line` is the thing an editor can jump to. Line and column are + * derived by counting newlines up to the offset — the file is by definition not + * safely decodable as text, so this deliberately works on the raw bytes rather + * than on a `utf8` decode that would substitute replacement characters and + * shift every column after the first multi-byte codepoint. + */ +export function nulPositions(buffer) { + const found = []; + let lineStart = 0; + let line = 1; + + for (let i = 0; i < buffer.length; i += 1) { + if (buffer[i] === 0x0a) { + line += 1; + lineStart = i + 1; + continue; + } + if (buffer[i] !== 0x00) continue; + // Column in BYTES from the start of the line. Exact for the ASCII these + // separators always sit in, and close enough to point at in anything else. + found.push({ line, column: i - lineStart + 1, offset: i }); + } + + return found; +} + +/** Tracked, non-deleted paths, from git rather than a directory walk. */ +export function trackedFiles(root = ROOT) { + const out = execFileSync("git", ["ls-files", "-z"], { cwd: root, encoding: "buffer" }); + return out + .toString("utf8") + .split("\0") + .filter((p) => p.length > 0); +} + +/** + * Scan `paths` (repo-relative) and return one entry per offending file. + * + * A path that no longer exists on disk is skipped rather than thrown on: `git + * ls-files` reports the index, and a file staged for deletion is still in it. + */ +export function scanPaths(paths, root = ROOT) { + const offenders = []; + for (const relPath of paths) { + if (isBinaryPath(relPath)) continue; + const absPath = join(root, relPath); + try { + if (!statSync(absPath).isFile()) continue; + } catch { + continue; + } + const positions = nulPositions(readFileSync(absPath)); + if (positions.length > 0) offenders.push({ relPath, positions }); + } + return offenders; +} + +function reportOffenders(offenders, scanned) { + for (const { relPath, positions } of offenders) { + for (const { line, column, offset } of positions) { + console.error(`✗ ${relPath}:${line}:${column} — literal NUL (byte offset ${offset})`); + } + } + const total = offenders.reduce((n, o) => n + o.positions.length, 0); + console.error( + `\n${total} literal NUL byte(s) in ${offenders.length} of ${scanned} scanned file(s).\n` + + `Write the six-character escape \\u0000 instead — identical at runtime, and the file stays\n` + + `plain text so \`grep\` stops silently reporting no matches over it.\n\n` + + `Careful applying the fix: every JSON-string layer between an agent and the file (a Write\n` + + `tool, a \`gh\` comment body) decodes a typed escape straight back into a real NUL, and an\n` + + `editor renders one as nothing. Verify by counting bytes, not by eye:\n` + + ` python3 -c "print(open('','rb').read().count(b'\\x00'))"`, + ); +} + +function main() { + const tracked = trackedFiles(); + const scannable = tracked.filter((p) => !isBinaryPath(p)); + const offenders = scanPaths(tracked); + + if (offenders.length === 0) { + console.log( + `✓ no literal NUL bytes: ${scannable.length} tracked text file(s) scanned ` + + `(${tracked.length - scannable.length} binary path(s) skipped by extension).`, + ); + return; + } + + reportOffenders(offenders, scannable.length); + process.exitCode = 1; +} + +// Only scan when run as a script; importing this module (the unit tests do) +// must not kick off a repo walk. +if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) { + main(); +} diff --git a/scripts/check-no-literal-nul.test.mjs b/scripts/check-no-literal-nul.test.mjs new file mode 100644 index 00000000..0d57cc37 --- /dev/null +++ b/scripts/check-no-literal-nul.test.mjs @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 The offlinecv Authors + +/** + * Unit tests for the literal-NUL gate (#787). + * + * The load-bearing case is `scanPaths` FAILING on a file that carries a real + * `0x00` — a gate for this class is worth nothing unless it has been watched to + * fail, because the defect is invisible in every rendering (an editor draws a + * NUL as nothing, `grep` reports no match rather than an error, and the diff + * only goes dark past a byte-offset threshold). + * + * Note how the fixtures below build that byte: `String.fromCharCode(0)`, never a + * literal in this source. Writing one here would make this very file an + * occurrence of what it tests — caught by the gate on the next run, which is + * funny exactly once. + */ + +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { isBinaryPath, nulPositions, scanPaths, trackedFiles } from "./check-no-literal-nul.mjs"; + +const NUL = String.fromCharCode(0); + +describe("nulPositions", () => { + it("finds nothing in ordinary text", () => { + expect(nulPositions(Buffer.from("const key = `${a}\\u0000${b}`;\n"))).toEqual([]); + }); + + it("reports the escape-shaped source as clean — that is the fix, not the defect", () => { + // Six ASCII characters. This is what #787 replaced the four byte literals + // with, and the gate must consider it perfectly fine or the fix is unusable. + const escaped = Buffer.from("a.join(\"\\u0000\")"); + expect(escaped.includes(0x00)).toBe(false); + expect(nulPositions(escaped)).toEqual([]); + }); + + it("locates a NUL by 1-indexed line and column", () => { + const buffer = Buffer.from(`line one\nleft${NUL}right\nline three\n`); + expect(nulPositions(buffer)).toEqual([{ line: 2, column: 5, offset: 13 }]); + }); + + it("reports every occurrence, not just the first", () => { + const buffer = Buffer.from(`${NUL}a\nb${NUL}\n`); + expect(nulPositions(buffer).map((p) => p.line)).toEqual([1, 2]); + }); + + it("finds a NUL past the ~8000-byte mark where git stops sniffing", () => { + // The #786 failure mode: git only sniffs the head of the blob, so a NUL out + // here renders a normal-looking text diff while every `grep` over the file + // silently reports no matches. A gate that only read the head would miss + // exactly the instance that got through review. + const buffer = Buffer.from(`${"x".repeat(9000)}${NUL}\n`); + expect(nulPositions(buffer)).toEqual([{ line: 1, column: 9001, offset: 9000 }]); + }); +}); + +describe("isBinaryPath", () => { + it("skips the font and PDF assets that legitimately carry NULs", () => { + expect(isBinaryPath("src/assets/fonts/Poppins-Bold.ttf")).toBe(true); + expect(isBinaryPath("tests/fixtures/pdfs/latex/awesome-cv-cv.pdf")).toBe(true); + }); + + it("matches case-insensitively", () => { + expect(isBinaryPath("docs/Diagram.PNG")).toBe(true); + }); + + it("does not skip source, config or markdown", () => { + for (const path of ["src/hooks/useJobSearch.ts", "package.json", "CLAUDE.md", "scripts/x.mjs"]) { + expect(isBinaryPath(path)).toBe(false); + } + }); + + it("does not skip a text file whose name merely contains a binary extension", () => { + // `.pdf` in the middle of the name is not a `.pdf` file — matching on the + // suffix rather than a substring is what keeps `render-ats-pdf.ts` scanned. + expect(isBinaryPath("src/lib/pdf/render-ats-pdf.ts")).toBe(false); + }); +}); + +describe("scanPaths", () => { + let dir; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "offlinecv-nul-gate-")); + writeFileSync(join(dir, "clean.ts"), 'export const k = `${a}\\u0000${b}`;\n'); + writeFileSync(join(dir, "dirty.ts"), `export const k = \`\${a}${NUL}\${b}\`;\n`); + writeFileSync(join(dir, "asset.ttf"), `binary${NUL}payload`); + }); + + afterAll(() => rmSync(dir, { recursive: true, force: true })); + + it("passes a tree whose only NULs live in binary paths", () => { + expect(scanPaths(["clean.ts", "asset.ttf"], dir)).toEqual([]); + }); + + it("FAILS on a file carrying a literal NUL — the fail-before case", () => { + const offenders = scanPaths(["clean.ts", "dirty.ts", "asset.ttf"], dir); + expect(offenders).toHaveLength(1); + expect(offenders[0].relPath).toBe("dirty.ts"); + // `export const k = \`${a}` is 22 bytes, so the NUL is byte 23 of line 1. + expect(offenders[0].positions).toEqual([{ line: 1, column: 23, offset: 22 }]); + }); + + it("ignores a path in the index that is not on disk", () => { + // `git ls-files` reports the index, which still names a file staged for + // deletion. Throwing there would fail the gate for a reason unrelated to it. + expect(scanPaths(["deleted-in-worktree.ts"], dir)).toEqual([]); + }); +}); + +describe("the repository itself", () => { + it("has no literal NUL in any tracked text file", () => { + // The acceptance criterion from #787, asserted rather than described. This + // is the test that goes red if the class ever comes back, whether or not + // anyone remembered to run `npm run check:nul`. + expect(scanPaths(trackedFiles())).toEqual([]); + }); +}); diff --git a/src/components/features/JobRepostArchiveDialog.tsx b/src/components/features/JobRepostArchiveDialog.tsx index 807c8f72229d097cba207743b60ca59b9d3a6401..1506b3bddfdc80c5b1039f720882ff93e387df53 100644 GIT binary patch delta 19 ZcmZp&YPZ^8D!>*~Y5)YA%>>Ty002D$1=|1s delta 14 VcmZp-YO>m3D!|CF*;?QX4*(*21WEt^ diff --git a/src/hooks/useJobSearch.ts b/src/hooks/useJobSearch.ts index bc87feee..26fc1430 100644 --- a/src/hooks/useJobSearch.ts +++ b/src/hooks/useJobSearch.ts @@ -195,12 +195,12 @@ export function useJobSearch( // joined key list rather than the array (a new array identity every render). // Additions are NOT handled here on purpose — they need a fetch, which is // `searchPendingCompanies`'s job. - const selectedKeyList = selectedCompanies.map(companyKey).join(""); + const selectedKeyList = selectedCompanies.map(companyKey).join("\u0000"); useEffect(() => { const snapshot = rawFetchRef.current; if (!snapshot) return; const selected = new Set( - selectedKeyList === "" ? [] : selectedKeyList.split(""), + selectedKeyList === "" ? [] : selectedKeyList.split("\u0000"), ); const removed = fetchedCompanyKeys.filter((key) => !selected.has(key)); if (removed.length === 0) return; diff --git a/src/lib/heuristics/extract/education.ts b/src/lib/heuristics/extract/education.ts index 035d7ea9..b49ddaf6 100644 --- a/src/lib/heuristics/extract/education.ts +++ b/src/lib/heuristics/extract/education.ts @@ -559,7 +559,7 @@ function parseDegreeAndField(line: string): { function degreeFieldKey(line: string): string | null { if (!DEGREE_RE.test(line)) return null; const { degree, field } = parseDegreeAndField(line); - return `${degree}${field ?? ""}`.toLowerCase(); + return `${degree}\u0000${field ?? ""}`.toLowerCase(); } /** Peel a trailing "City, ST" (US) or "City, Country" (international) location diff --git a/src/lib/heuristics/fixture-match.ts b/src/lib/heuristics/fixture-match.ts index 218cf822..fe024f41 100644 --- a/src/lib/heuristics/fixture-match.ts +++ b/src/lib/heuristics/fixture-match.ts @@ -318,7 +318,7 @@ function axisKey(path: AxisPath, a: ReproArtifact, d: DerivedSignals): string { return canonicalList([...a.triggers]); case "disagreements": return canonicalList( - a.disagreements.map((x) => `${x.kind}${x.field}${x.likelyCause ?? ""}`), + a.disagreements.map((x) => `${x.kind}\u0000${x.field}\u0000${x.likelyCause ?? ""}`), ); case "sectionSource": return a.sectionSource;