Apply CodeAnt review fixes for PR #51

- _readRepoFile opens the file first (O_NOFOLLOW, O_NONBLOCK), then checks
  the opened handle is a regular file that is still the one at the in-repo
  path, and reads from the handle. A symlink swapped in for the file or a
  parent directory after the path check can no longer redirect the read.
- Untracked symlinks that point outside the repo (or nowhere) are shown as
  link entries (mode 120000, content = link target path) instead of being
  dropped, without reading the target.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: pranavcodeant

src/utils/gitDiffHelper.js

@@ -1,4 +1,5 @@
 import fs from 'fs/promises';
+import { constants as fsConstants } from 'fs';
 import path from 'path';
 import { promisify } from 'util';
 import { exec, execFile } from 'child_process';
@@ -475,11 +476,29 @@ class GitDiffHelper {
    */
   async _readRepoFile(relPath) {
     const realRoot = await fs.realpath(this.gitRoot);
+    const isInside = p => {
+      const rel = path.relative(realRoot, p);
+      return !!rel && rel !== '..' && !rel.startsWith('..' + path.sep) && !path.isAbsolute(rel);
+    };
     const realPath = await fs.realpath(path.resolve(this.gitRoot, relPath));
-    const rel = path.relative(realRoot, realPath);
-    if (!rel || rel === '..' || rel.startsWith('..' + path.sep) || path.isAbsolute(rel)) return null;
-    if (!(await fs.stat(realPath)).isFile()) return null;
-    return fs.readFile(realPath, 'utf8');
+    if (!isInside(realPath)) return null;
+
+    // Open first and then check what was actually opened, so a symlink swapped in after the
+    // check above can't redirect the read. O_NONBLOCK keeps a FIFO from blocking the open.
+    const { O_RDONLY, O_NOFOLLOW = 0, O_NONBLOCK = 0 } = fsConstants;
+    const handle = await fs.open(realPath, O_RDONLY | O_NOFOLLOW | O_NONBLOCK);
+    try {
+      const opened = await handle.stat();
+      if (!opened.isFile()) return null;
+      // The opened file must still be the file at this path, and the path must still resolve
+      // inside the repo (a parent directory may have been swapped for a symlink)
+      const nowPath = await fs.realpath(realPath);
+      const now = await fs.stat(nowPath);
+      if (!isInside(nowPath) || now.dev !== opened.dev || now.ino !== opened.ino) return null;
+      return await handle.readFile('utf8');
+    } finally {
+      await handle.close();
+    }
   }
 
   /**
@@ -488,12 +507,21 @@ class GitDiffHelper {
    */
   async _buildUntrackedFileDiff(file) {
     try {
-      const content = await this._readRepoFile(file);
-      if (content === null) return null;
+      let content = await this._readRepoFile(file).catch(() => null);
+      let mode = '100644';
+      if (content === null) {
+        // A symlink we won't follow (it points outside the repo or nowhere): show the link
+        // itself, as git would once it's committed, using its target path and never the
+        // target's contents
+        const absolutePath = path.resolve(this.gitRoot, file);
+        if (!(await fs.lstat(absolutePath)).isSymbolicLink()) return null;
+        content = await fs.readlink(absolutePath);
+        mode = '120000';
+      }
       const lines = content.split('\n');
 
       let patchStr = `diff --git a/${file} b/${file}\n`;
-      patchStr += `new file mode 100644\n`;
+      patchStr += `new file mode ${mode}\n`;
       patchStr += `index 0000000..${this._generateRandomHash()}\n`;
       patchStr += `--- /dev/null\n`;
       patchStr += `+++ b/${file}\n`;

tests/gitDiffHelper.test.js

@@ -1,6 +1,7 @@
-import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from 'vitest';
+import { describe, it, expect, vi, beforeAll, afterAll, beforeEach, afterEach } from 'vitest';
 import { execSync } from 'child_process';
 import fs from 'fs';
+import fsp from 'fs/promises';
 import path from 'path';
 import os from 'os';
 import GitDiffHelper from '../src/utils/gitDiffHelper.js';
@@ -1205,6 +1206,49 @@ describe('GitDiffHelper integration tests', () => {
       }
     });
 
+    it('shows untracked symlinks that point outside the repo as links, not their contents', async () => {
+      const target = path.join(outsideDir, 'creds.txt');
+      fs.symlinkSync(target, path.join(testDir, 'src/link-file'));
+
+      const helper = await createHelper();
+      const diffs = await helper.getDiffBasedOnReviewConfig({ type: 'base-commit', baseCommit: 'HEAD' });
+
+      const diff = diffs.find(d => d.filename_str === 'src/link-file');
+      expect(diff).toBeDefined();
+      expect(diff.edit_type_str).toBe('ADDED');
+      expect(diff.head_file_str).toBe(target);
+      expect(diff.patch_str).toContain('new file mode 120000');
+      expect(leaked(diffs)).toEqual([]);
+    });
+
+    it('does not follow a symlink swapped in after the path check', async () => {
+      writeFile('src/race/data.txt', 'inside\n');
+      fs.writeFileSync(path.join(outsideDir, 'data.txt'), 'OUTSIDE_SECRET\n');
+      const raceDir = path.join(testDir, 'src/race');
+      const realOpen = fsp.open;
+      let swapped = false;
+      // Swap the parent directory for a link to outside the repo between the check and the open
+      const spy = vi.spyOn(fsp, 'open').mockImplementation(async (file, ...rest) => {
+        if (!swapped && String(file).endsWith(path.join('race', 'data.txt'))) {
+          swapped = true;
+          fs.renameSync(raceDir, raceDir + '-orig');
+          fs.symlinkSync(outsideDir, raceDir);
+        }
+        return realOpen(file, ...rest);
+      });
+
+      try {
+        const helper = await createHelper();
+        const diffs = await helper.getDiffBasedOnReviewConfig({ type: 'uncommitted' });
+        expect(swapped).toBe(true);
+        expect(leaked(diffs)).toEqual([]);
+      } finally {
+        spy.mockRestore();
+        fs.rmSync(raceDir, { recursive: true, force: true });
+        fs.rmSync(raceDir + '-orig', { recursive: true, force: true });
+      }
+    });
+
     it('does not read committed symlinks that point outside the repo', async () => {
       const baseHash = git('rev-parse HEAD');
       fs.symlinkSync(path.join(outsideDir, 'creds.txt'), path.join(testDir, 'src/committed-link'));