fix(scan): read the hunks and the encodings the scan claims to cover

src/scan-session.mjs

@@ -112,7 +112,8 @@ export function scanEntries(repo, engine, entries, options = {}) {
         // Binary detection still reads the blob. Count every examined byte so
         // later commits cannot reset the total budget merely by using NUL data.
         stats.bytes_scanned += blob.length;
-        if (isBinary(blob)) {
+        const utf16 = decodeUtf16(blob);
+        if (utf16 === null && isBinary(blob)) {
             // Content rules match text lines, so a binary blob carries nothing
             // they could fire on. Skipping it loses no coverage: the skip is
             // recorded as 'binary', which does not mark the scan incomplete.
@@ -122,7 +123,7 @@ export function scanEntries(repo, engine, entries, options = {}) {
         }
         stats.files_scanned += 1;
         const ranges = lineRanges.get(entry.path);
-        const matched = engine.checkContent(entry.path, blob.toString('utf8'),
+        const matched = engine.checkContent(entry.path, utf16 ?? blob.toString('utf8'),
             ranges && ranges.length ? { lineRanges: ranges } : {});
         stats.findings_total += matched.length;
         for (const finding of matched) {
@@ -240,6 +241,24 @@ function isBinary(buffer) {
     return false;
 }
 
+// A byte-order mark declares the bytes are UTF-16 text, so their NUL padding is
+// not evidence of a binary file. Returns the decoded text, or null to leave the
+// blob to isBinary. Only the mark is trusted: guessing at unmarked NUL data
+// would decode images into garbage and hand that to the content rules.
+function decodeUtf16(buffer) {
+    if (buffer.length < 4 || buffer.length % 2 !== 0) return null;
+    const [first, second] = buffer;
+    // FF FE 00 00 opens UTF-32LE, which shares the UTF-16LE mark's first two bytes.
+    if (first === 0xff && second === 0xfe && !(buffer[2] === 0 && buffer[3] === 0)) {
+        return buffer.subarray(2).toString('utf16le');
+    }
+    // swap16 mutates in place, so byte-swap a copy and leave the read batch alone.
+    if (first === 0xfe && second === 0xff) {
+        return Buffer.from(buffer.subarray(2)).swap16().toString('utf16le');
+    }
+    return null;
+}
+
 // collectLineRanges builds a Map<path, ranges[]> for all entries whose content
 // scan can be narrowed to changed hunks (W4, bug 12d-F1). It batches the work
 // into ONE `git diff --unified=0` per (commit, parent) pair rather than one diff
@@ -288,17 +307,41 @@ function collectLineRanges(repo, entries) {
         // comes from git diff --raw -z, also unquoted). Paths containing newlines
         // are not handled, but they fall back to a whole-blob scan (safe).
         let currentPath = null;
+        // Hunk bodies must never be read as headers. An added line is rendered as
+        // `+` plus its content, so source starting `++ ` arrives as `+++ ...` and
+        // otherwise passes for a new-file header, hiding every later hunk of that
+        // file behind a path nothing scans. Each `@@` header declares how many
+        // body lines follow on both sides; consume exactly that many first.
+        let pending = 0;
         for (const line of output.split('\n')) {
+            if (pending > 0) {
+                if (line.startsWith('\\')) continue; // "\ No newline at end of file"
+                if (line.startsWith('-') || line.startsWith('+')) {
+                    pending -= 1;
+                    continue;
+                }
+                pending = 0; // body ended sooner than declared: resume header parsing
+            }
+            // Content cannot forge this prefix, so it is the one reliable resync point.
+            if (line.startsWith('diff --git ')) {
+                currentPath = null;
+                continue;
+            }
             // With --no-prefix the +++ line is `+++ path` (no b/ prefix).
             const plusMatch = /^\+\+\+ (.+)$/.exec(line);
             if (plusMatch) {
                 currentPath = plusMatch[1] === '/dev/null' ? null : plusMatch[1];
                 continue;
             }
-            const match = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/.exec(line);
-            if (!match || !currentPath) continue;
-            const start = Number(match[1]);
-            const count = match[2] === undefined ? 1 : Number(match[2]);
+            const match = /^@@ -\d+(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/.exec(line);
+            if (!match) continue;
+            const removed = match[1] === undefined ? 1 : Number(match[1]);
+            const count = match[3] === undefined ? 1 : Number(match[3]);
+            // Set the budget before any bail below, or a skipped hunk re-exposes
+            // its body to the header branches above.
+            pending = removed + count;
+            if (!currentPath) continue;
+            const start = Number(match[2]);
             if (start === 0 || count === 0) continue; // pure deletion, no new content
             const range = { start, end: start + count - 1 };
             const existing = byPath.get(currentPath);

tests/scan-session.test.mjs

@@ -90,10 +90,10 @@ test('binary blobs skip whole: no content rule reads them and the scan stays com
         execFileSync('git', ['init', '-q'], { cwd: dir });
         execFileSync('git', ['config', 'user.name', 'Test'], { cwd: dir });
         execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: dir });
-        // PowerShell 5.1 still writes UTF-16LE from `>`, `Out-File` and
-        // `Tee-Object`. One NUL per character classifies the blob as binary,
-        // and no content rule reads binary bytes: the marker text below would
-        // block as plain UTF-8, and here it produces no finding.
+        // NUL-bearing bytes with no byte-order mark stay binary. Nothing here
+        // says whether they are text, and guessing costs more than it buys: a
+        // misread image decoded into garbage would hand the content rules
+        // nonsense, and a block finding on clean unstages the user's asset.
         writeFileSync(join(dir, 'deploy-notes.txt'), Buffer.from('// generated by AI\n', 'utf16le'));
         execFileSync('git', ['add', '.'], { cwd: dir });
 
@@ -108,6 +108,65 @@ test('binary blobs skip whole: no content rule reads them and the scan stays com
     }
 });
 
+// A byte-order mark says the bytes are UTF-16 text, so the NUL padding is not
+// evidence of a binary file. PowerShell 5.1 writes exactly this from `>`,
+// `Out-File` and `Tee-Object`, which is how a Windows developer ends up with an
+// AI-marker file that no content rule ever reads.
+function repositoryWithBlob(name, bytes) {
+    const dir = mkdtempSync(join(tmpdir(), 'aim-session-bom-'));
+    execFileSync('git', ['init', '-q'], { cwd: dir });
+    execFileSync('git', ['config', 'user.name', 'Test'], { cwd: dir });
+    execFileSync('git', ['config', 'user.email', 'test@example.com'], { cwd: dir });
+    writeFileSync(join(dir, name), bytes);
+    execFileSync('git', ['add', '.'], { cwd: dir });
+    return dir;
+}
+
+test('UTF-16 text with a byte-order mark is read by the content rules', () => {
+    const body = Buffer.from('// generated by AI\n', 'utf16le');
+    const dir = repositoryWithBlob('deploy-notes.txt', Buffer.concat([Buffer.from([0xff, 0xfe]), body]));
+    try {
+        const repo = openRepo(dir);
+        const result = scanEntries(repo, newEngine('strict'), trackedEntries(repo));
+        assert.equal(result.stats.files_scanned, 1, 'a BOM-marked UTF-16 blob must be scanned, not skipped');
+        assert.ok(
+            result.findings.some((f) => f.matchedRuleIds.includes('marker.ai-authored')),
+            'the marker in UTF-16LE text must produce the same finding it does in UTF-8',
+        );
+    } finally {
+        rmSync(dir, { recursive: true, force: true });
+    }
+});
+
+test('big-endian UTF-16 text with a byte-order mark is read by the content rules', () => {
+    const body = Buffer.from('// generated by AI\n', 'utf16le').swap16();
+    const dir = repositoryWithBlob('deploy-notes.txt', Buffer.concat([Buffer.from([0xfe, 0xff]), body]));
+    try {
+        const repo = openRepo(dir);
+        const result = scanEntries(repo, newEngine('strict'), trackedEntries(repo));
+        assert.ok(
+            result.findings.some((f) => f.matchedRuleIds.includes('marker.ai-authored')),
+            'the marker in UTF-16BE text must produce the same finding it does in UTF-8',
+        );
+    } finally {
+        rmSync(dir, { recursive: true, force: true });
+    }
+});
+
+test('an odd-length blob claiming a byte-order mark is left to the binary skip', () => {
+    // swap16() throws on odd lengths; the pre-commit hook must not crash on a
+    // truncated file, so an impossible UTF-16 length falls through to binary.
+    const dir = repositoryWithBlob('truncated.bin', Buffer.from([0xfe, 0xff, 0x00, 0x2f, 0x00]));
+    try {
+        const repo = openRepo(dir);
+        const result = scanEntries(repo, newEngine('strict'), trackedEntries(repo));
+        assert.equal(result.stats.skipped.binary, 1);
+        assert.equal(result.complete, true);
+    } finally {
+        rmSync(dir, { recursive: true, force: true });
+    }
+});
+
 test('git stderr from a passing blob read is captured, not printed', () => {
     const dir = mkdtempSync(join(tmpdir(), 'aim-session-stderr-'));
     try {
@@ -269,3 +328,67 @@ test('hunk-narrowing: batched multi-file diff attributes each file its own hunks
         rmSync(dir, { recursive: true, force: true });
     }
 });
+
+// A unified diff renders an added line as `+` followed by its content, so a
+// source line starting `++ ` arrives as `+++ ...` — the same shape as a new-file
+// header. Hunk bodies must be consumed by the line budget the `@@` header
+// declares, never re-read as headers, or everything after the forged line in
+// that file is attributed to a path nobody scans.
+const FORGERY_BASE = [
+    '// top of file',
+    'int i = 0;',
+    '',
+    'func Fixture() {',
+    '  step()',
+    '  step()',
+    '  step()',
+    '  // benign line',
+    '  step()',
+    '}',
+].join('\n') + '\n';
+
+function commitForgedHeader(dir, secondLine) {
+    const lines = FORGERY_BASE.split('\n');
+    lines[1] = secondLine;
+    lines[7] = '// generated by AI';
+    writeFileSync(join(dir, 'main.go'), lines.join('\n'));
+    execFileSync('git', ['add', 'main.go'], { cwd: dir });
+    execFileSync('git', ['commit', '-q', '-m', 'edit'], { cwd: dir });
+}
+
+test('a hunk body that reads like a file header does not hide the rest of the file', () => {
+    // Line 2 becomes `++ i;`, which git renders as `+++ i;`. Line 8 becomes a
+    // marker in a later hunk. The marker must still be scanned.
+    const dir = repositoryWithBase('main.go', FORGERY_BASE);
+    try {
+        commitForgedHeader(dir, '++ i;');
+
+        const repo = openRepo(dir);
+        const { head, parent } = headAndParent(dir);
+        const changes = commitChanges(repo, head, head, [parent]);
+        const result = scanEntries(repo, newEngine('strict'), changes.entries);
+        const markers = result.findings.filter((f) => f.matchedRuleIds.includes('marker.ai-authored'));
+        assert.ok(markers.length >= 1, 'a line rendering as `+++ path` must not blind the hunks after it');
+        assert.deepEqual([...new Set(markers.map((f) => f.path))], ['main.go']);
+    } finally {
+        rmSync(dir, { recursive: true, force: true });
+    }
+});
+
+test('a hunk body reading like a deleted-file header does not hide the rest of the file', () => {
+    // `++ /dev/null` renders as `+++ /dev/null`, which the header branch treats
+    // as a deletion and would drop every remaining hunk in the file.
+    const dir = repositoryWithBase('main.go', FORGERY_BASE);
+    try {
+        commitForgedHeader(dir, '++ /dev/null');
+
+        const repo = openRepo(dir);
+        const { head, parent } = headAndParent(dir);
+        const changes = commitChanges(repo, head, head, [parent]);
+        const result = scanEntries(repo, newEngine('strict'), changes.entries);
+        const markers = result.findings.filter((f) => f.matchedRuleIds.includes('marker.ai-authored'));
+        assert.ok(markers.length >= 1, 'a line rendering as `+++ /dev/null` must not blind the hunks after it');
+    } finally {
+        rmSync(dir, { recursive: true, force: true });
+    }
+});