Fix false completion, escaping checks and filename/line-count handling

README.md

@@ -492,3 +492,21 @@ code changes are needed.
 - **Loop paused unexpectedly** — check `get_loop_status` → `pending_approval`; a
   safety guard tripped. Review the change, then re-run with adjusted scope.
 ```
+
+
+### Verifier regression hardening (October 2026)
+
+- A checklist item without a machine check requires at least one actual passing
+  verification command. An empty verification list is not evidence of success.
+- File checks require an existing regular file with both lexical and resolved
+  paths inside the project. Escaping symlinks are rejected. This is a defensive
+  check, not an OS sandbox against concurrent malicious filesystem rewrites.
+- Git change capture uses NUL-delimited paths, preserving whitespace, quotes,
+  tabs/newlines and literal ` -> ` text. Renames appear as delete/add so both
+  affected paths reach the guards.
+- Untracked symlinks are not followed for line counts. A final newline no longer
+  adds a phantom line to the change totals.
+
+`npm test` runs the mocked integration suite, not Codex inference. The added tests
+use temporary synthetic Git projects. Actual model/CLI behavior is not certified
+by these checks; no model calls are needed to execute the suite.

src/verifier.ts

@@ -98,9 +98,11 @@ async function hasHead(cwd: string): Promise<boolean> {
 
 async function countLines(file: string): Promise<number> {
   try {
+    // A git symlink is a path record, not permission to read its target.
+    if (!(await fs.lstat(file)).isFile()) return 0;
     const data = await fs.readFile(file, 'utf8');
     if (data.length === 0) return 0;
-    return data.split('\n').length;
+    return data.split('\n').length - (data.endsWith('\n') ? 1 : 0);
   } catch {
     return 0;
   }
@@ -129,12 +131,12 @@ export async function getDiffStat(projectDir: string): Promise<DiffStat> {
   // numstat for tracked changes (added\tdeleted\tpath); binary => '-'
   const numstatRef = headExists ? 'HEAD' : '';
   const numstat = await git(
-    ['diff', '--numstat', ...(numstatRef ? [numstatRef] : [])],
+    ['diff', '--no-renames', '--numstat', '-z', ...(numstatRef ? [numstatRef] : [])],
     cwd,
   );
   const counts = new Map<string, { added: number; deleted: number }>();
   if (numstat) {
-    for (const line of numstat.split('\n')) {
+    for (const line of numstat.split('\0')) {
       if (!line.trim()) continue;
       const parts = line.split('\t');
       if (parts.length < 3) continue;
@@ -146,17 +148,16 @@ export async function getDiffStat(projectDir: string): Promise<DiffStat> {
 
   // porcelain status for the authoritative file+status list (incl. untracked)
   const status = await git(
-    ['status', '--porcelain=v1', '--untracked-files=all'],
+    ['status', '--porcelain=v1', '-z', '--untracked-files=all', '--no-renames'],
     cwd,
   );
   if (status) {
-    for (const line of status.split('\n')) {
+    for (const line of status.split('\0')) {
       if (!line.trim()) continue;
       const xy = line.slice(0, 2);
-      let filePart = line.slice(3);
-      // renames: "old -> new" (use the last segment in case a path contains ' -> ')
-      if (filePart.includes(' -> ')) filePart = filePart.split(' -> ').pop()!;
-      const p = filePart.trim();
+      // NUL-delimited porcelain leaves the filename exact, without C quoting.
+      // Rename detection is disabled so both old/new paths remain reviewable.
+      const p = line.slice(3);
       const isUntracked = xy === '??';
       const isDeleted = xy.includes('D');
       const statusLetter = isUntracked ? 'A' : xy.trim() || 'M';
@@ -207,6 +208,22 @@ export async function runVerification(
   return results;
 }
 
+/** Resolve an existing file only when both its lexical and real paths stay inside the project. */
+async function projectCheckFile(cwd: string, requested: string): Promise<string | null> {
+  try {
+    const inside = (root: string, file: string): boolean => {
+      const relative = path.relative(root, file);
+      return relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative);
+    };
+    const candidate = path.resolve(cwd, requested);
+    if (!inside(cwd, candidate)) return null;
+    const root = await fs.realpath(cwd);
+    const real = await fs.realpath(candidate);
+    if (!inside(root, real) || !(await fs.stat(real)).isFile()) return null;
+    return real;
+  } catch { return null; }
+}
+
 /** Evaluate a single checklist machine check against the repo. */
 export async function evaluateCheck(
   projectDir: string,
@@ -216,15 +233,16 @@ export async function evaluateCheck(
   switch (check.type) {
     case 'fileExists': {
       try {
-        await fs.access(path.resolve(cwd, check.path));
-        return true;
+        return (await projectCheckFile(cwd, check.path)) !== null;
       } catch {
         return false;
       }
     }
     case 'fileContains': {
       try {
-        const data = await fs.readFile(path.resolve(cwd, check.path), 'utf8');
+        const file = await projectCheckFile(cwd, check.path);
+        if (file === null) return false;
+        const data = await fs.readFile(file, 'utf8');
         try {
           return new RegExp(check.pattern).test(data);
         } catch {
@@ -269,7 +287,7 @@ export async function review(
       evidence = done ? describeCheck(item.check) : undefined;
     } else {
       // verification-gated item
-      done = allVerificationPassed;
+      done = verification.length > 0 && allVerificationPassed;
       evidence = done ? 'all verification commands passed' : undefined;
     }
     updated.push({ ...item, done, evidence });
@@ -313,3 +331,4 @@ function describeCheck(check: CheckSpec): string {
       return `command passed: ${check.command}`;
   }
 }
+

test/verifier-regression.test.ts

@@ -0,0 +1,81 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { promises as fs } from 'node:fs';
+import * as path from 'node:path';
+import { evaluateCheck, getDiffStat, review, runCommand } from '../src/verifier.js';
+import { makeProject } from './helpers.js';
+
+const emptyDiff = { files: [], totalAdded: 0, totalDeleted: 0, isGitRepo: true, rawSummary: '' };
+
+test('plain checklist without verification cannot claim completion', async () => {
+  const dir = await makeProject();
+  try {
+    const result = await review(dir, [{id:'a',text:'Verify manually',done:false}], [], emptyDiff);
+    assert.equal(result.review.complete, false);
+    assert.equal(result.checklist[0]?.done, false);
+  } finally { await fs.rm(dir,{recursive:true,force:true}); }
+});
+
+test('file checks reject lexical and symlink escapes', async () => {
+  const dir = await makeProject();
+  const outside = await fs.mkdtemp(path.join(path.dirname(dir), 'outside-check-'));
+  try {
+    await fs.writeFile(path.join(outside,'fixture.txt'), 'external fixture');
+    await fs.symlink(outside, path.join(dir,'external'), 'dir');
+    for (const name of [path.join(outside,'fixture.txt'), path.relative(dir,path.join(outside,'fixture.txt')), 'external/fixture.txt']) {
+      assert.equal(await evaluateCheck(dir, {type:'fileExists',path:name}), false, name);
+      assert.equal(await evaluateCheck(dir, {type:'fileContains',path:name,pattern:'external'}), false, name);
+    }
+    await fs.writeFile(path.join(dir,'safe.txt'),'safe');
+    assert.equal(await evaluateCheck(dir,{type:'fileContains',path:'safe.txt',pattern:'safe'}),true);
+  } finally { await fs.rm(dir,{recursive:true,force:true}); await fs.rm(outside,{recursive:true,force:true}); }
+});
+
+test('git filenames retain whitespace, quotes, tabs and arrow text', async () => {
+  const dir = await makeProject();
+  try {
+    const names=[' leading.txt','trailing.txt ','a\tb.txt','a\nb.txt','old -> new.txt','quote".txt'];
+    for(const name of names) await fs.writeFile(path.join(dir,name),'one\n');
+    let diff=await getDiffStat(dir);
+    assert.deepEqual(diff.files.map(f=>f.path).sort(), [...names].sort());
+    for(const file of diff.files) assert.equal(file.added,1,file.path);
+    await runCommand('git add -A && git commit -qm names',dir,30000);
+    for(const name of names) await fs.appendFile(path.join(dir,name),'two\n');
+    diff=await getDiffStat(dir);
+    assert.deepEqual(diff.files.map(f=>f.path).sort(), [...names].sort());
+    for(const file of diff.files) assert.equal(file.added,1,file.path);
+  } finally { await fs.rm(dir,{recursive:true,force:true}); }
+});
+
+test('untracked symlinks are not read as target file content', async () => {
+  const dir=await makeProject();
+  try {
+    await fs.symlink('PROJECT_GOAL.md',path.join(dir,'goal-link'));
+    const diff=await getDiffStat(dir);
+    assert.equal(diff.files.find(f=>f.path==='goal-link')?.added,0);
+  } finally { await fs.rm(dir,{recursive:true,force:true}); }
+});
+
+test('rename preserves both source deletion and destination addition', async()=> {
+  const dir=await makeProject();
+  try {
+    await fs.rename(path.join(dir,'greet.test.js'),path.join(dir,'renamed test.js'));
+    await runCommand('git add -A',dir,30000);
+    const diff=await getDiffStat(dir);
+    assert.ok(diff.files.some(f=>f.path==='greet.test.js' && f.status==='D'));
+    assert.ok(diff.files.some(f=>f.path==='renamed test.js' && f.added>0));
+  } finally { await fs.rm(dir,{recursive:true,force:true}); }
+});
+
+test('untracked line counts handle empty and unterminated files', async()=> {
+  const dir=await makeProject();
+  try {
+    await fs.writeFile(path.join(dir,'empty.txt'),'');
+    await fs.writeFile(path.join(dir,'one.txt'),'one');
+    await fs.writeFile(path.join(dir,'two.txt'),'one\ntwo\n');
+    const diff=await getDiffStat(dir);
+    for(const [name,count] of [['empty.txt',0],['one.txt',1],['two.txt',2]] as const) {
+      assert.equal(diff.files.find(f=>f.path===name)?.added,count);
+    }
+  } finally { await fs.rm(dir,{recursive:true,force:true}); }
+});