Block ref injection and out-of-repo symlink reads in --base (0.5.13)

Follow-up to #50, addressing its two CodeAnt review comments, which were
resolved on the PR but whose fixes were not pushed before merge.

- --base / --base-commit refs (also reachable through the MCP
  codeant_review_local tool) were interpolated into a shell command, so
  "main; <cmd>" ran <cmd>, and "--output=<file>" made git write a file.
  Refs are now resolved with execFile + rev-parse --verify <ref>^{commit},
  leading "-" is rejected, and later git commands only see the SHA.
- Working-tree reads followed symlinks, uploading files from outside the
  repo: untracked links, files under untracked symlinked directories, and
  (since #50) committed links in --base mode. _readRepoFile now skips
  anything whose real path is outside the repo or isn't a regular file.

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

Author: pranavcodeant

changelog.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## [0.5.13] - 13/10/2026
+- Security: `--base` / `--base-commit` values (including from the MCP `codeant_review_local` tool) are resolved without a shell, and option-like values are rejected, so they can no longer run commands or pass options to git
+- Security: symlinks that point outside the repository, and files reached through them, are no longer read into review or secrets payloads. In 0.5.12, `--base` / `--base-commit` sent a committed symlink's target contents
+
 ## [0.5.12] - 13/10/2026
 - `review` / `secrets` with `--base` or `--base-commit` now include staged, unstaged, and untracked files (respecting `.gitignore`), not just commits. Previously `review --all --base main` silently skipped new files
 - With `--base` / `--base-commit`, file content is read from the working tree so it matches the reviewed diff

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "codeant-cli",
-  "version": "0.5.12",
+  "version": "0.5.13",
   "description": "Code review CLI tool",
   "type": "module",
   "repository": {

package.json

@@ -1,9 +1,10 @@
 import fs from 'fs/promises';
 import path from 'path';
 import { promisify } from 'util';
-import { exec } from 'child_process';
+import { exec, execFile } from 'child_process';
 
 const execAsync = promisify(exec);
+const execFileAsync = promisify(execFile);
 
 /**
  * Expand untracked paths from `git status --porcelain`.
@@ -468,14 +469,27 @@ class GitDiffHelper {
     return diffs;
   }
 
+  /**
+   * Read a working-tree file. Returns null if it resolves outside the repository (a symlink
+   * pointing elsewhere) or isn't a regular file, so those contents are never uploaded.
+   */
+  async _readRepoFile(relPath) {
+    const realRoot = await fs.realpath(this.gitRoot);
+    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');
+  }
+
   /**
    * Build a synthetic "new file" diff for an untracked file, which git diff can't see.
    * Returns null if the file can't be read.
    */
   async _buildUntrackedFileDiff(file) {
     try {
-      const absolutePath = path.resolve(this.gitRoot, file);
-      const content = await fs.readFile(absolutePath, 'utf8');
+      const content = await this._readRepoFile(file);
+      if (content === null) return null;
       const lines = content.split('\n');
 
       let patchStr = `diff --git a/${file} b/${file}\n`;
@@ -549,8 +563,7 @@ class GitDiffHelper {
     let resolvedRef;
     for (const candidate of [branch, `origin/${branch}`]) {
       try {
-        const { stdout } = await execAsync(`git rev-parse ${candidate}`, { cwd: this.gitRoot });
-        resolvedRef = stdout.trim();
+        resolvedRef = await this._resolveCommitSha(candidate);
         break;
       } catch (err) {
         // try next
@@ -589,15 +602,29 @@ class GitDiffHelper {
     // Resolve the commit ref (supports HEAD~3, sha, tag, etc.)
     let resolvedCommit;
     try {
-      const { stdout } = await execAsync(`git rev-parse ${commit}`, { cwd: this.gitRoot });
-      resolvedCommit = stdout.trim();
+      resolvedCommit = await this._resolveCommitSha(commit);
     } catch (err) {
       throw new Error(`Could not resolve commit: ${commit}`);
     }
 
     return this._getWorkingTreeDiffAgainst(resolvedCommit);
   }
 
+  /**
+   * Resolve a user-supplied ref (from the CLI or MCP) to a commit SHA without going through a
+   * shell, so later git commands only ever see the SHA. Throws if it doesn't name a commit.
+   */
+  async _resolveCommitSha(ref) {
+    // A leading "-" would be parsed as a git option (e.g. --output=<file>)
+    if (ref.startsWith('-')) throw new Error(`Invalid ref: ${ref}`);
+    const { stdout } = await execFileAsync(
+      'git',
+      ['rev-parse', '--verify', '--quiet', `${ref}^{commit}`],
+      { cwd: this.gitRoot }
+    );
+    return stdout.trim();
+  }
+
   /**
    * Diff a base commit against the working tree: commits since base, staged and unstaged
    * changes (including staged new files), plus untracked files not ignored by .gitignore.
@@ -1011,8 +1038,8 @@ class GitDiffHelper {
     } catch (err) {
       // Handle untracked files
       try {
-        const absoluteFilePath = path.resolve(this.gitRoot, filePath);
-        const headFileStr = await fs.readFile(absoluteFilePath, 'utf8');
+        const headFileStr = await this._readRepoFile(filePath);
+        if (headFileStr === null) throw new Error('Not a regular file inside the repository');
         const lines = headFileStr.split('\n');
 
         let patchStr = `diff --git a/${filePath} b/${filePath}\n`;
@@ -1069,9 +1096,9 @@ class GitDiffHelper {
         { cwd: this.gitRoot }
       ).then(r => r.stdout).catch(() => '');
     } else {
-      headFileStr = await fs.readFile(
-        path.join(this.gitRoot, filePath), 'utf8'
-      ).catch(() => '');
+      headFileStr = await this._readRepoFile(filePath)
+        .then(content => content ?? '')
+        .catch(() => '');
     }
 
     // Get patch

src/utils/gitDiffHelper.js

@@ -1134,4 +1134,100 @@ describe('GitDiffHelper integration tests', () => {
       ).rejects.toThrow();
     });
   });
+
+  // ─────────────────────────────────────────────────────
+  // SECURITY: user-supplied refs and symlinks
+  // ─────────────────────────────────────────────────────
+  describe('security', () => {
+    let outsideDir;
+    let startBranch;
+
+    beforeEach(() => {
+      startBranch = git('rev-parse --abbrev-ref HEAD');
+      git('checkout -- .');
+      git('reset HEAD -- .');
+      git('clean -fd');
+      outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codeant-outside-'));
+      fs.writeFileSync(path.join(outsideDir, 'creds.txt'), 'OUTSIDE_SECRET\n');
+      fs.mkdirSync(path.join(outsideDir, 'dir'));
+      fs.writeFileSync(path.join(outsideDir, 'dir', 'inner.txt'), 'OUTSIDE_DIR_SECRET\n');
+    });
+
+    afterEach(() => {
+      git('reset HEAD -- .');
+      git('checkout -- .');
+      git('clean -fd');
+      if (git('rev-parse --abbrev-ref HEAD') !== startBranch) {
+        git(`checkout ${startBranch}`);
+      }
+      fs.rmSync(outsideDir, { recursive: true, force: true });
+    });
+
+    const leaked = diffs => diffs.filter(d => /OUTSIDE_(DIR_)?SECRET/.test(d.head_file_str + d.patch_str));
+
+    it('does not run shell metacharacters in a base branch or commit', async () => {
+      const marker = path.join(outsideDir, 'pwned');
+      const helper = await createHelper();
+
+      await expect(
+        helper.getDiffBasedOnReviewConfig({ type: 'base-branch', baseBranch: `HEAD; touch ${marker}` })
+      ).rejects.toThrow('Could not resolve branch');
+      await expect(
+        helper.getDiffBasedOnReviewConfig({ type: 'base-commit', baseCommit: `HEAD$(touch ${marker})` })
+      ).rejects.toThrow('Could not resolve commit');
+
+      expect(fs.existsSync(marker)).toBe(false);
+    });
+
+    it('rejects option-like refs instead of passing them to git', async () => {
+      const written = path.join(outsideDir, 'written.txt');
+      const helper = await createHelper();
+
+      await expect(
+        helper.getDiffBasedOnReviewConfig({ type: 'base-commit', baseCommit: `--output=${written}` })
+      ).rejects.toThrow('Could not resolve commit');
+
+      expect(fs.existsSync(written)).toBe(false);
+    });
+
+    it('does not read untracked symlinks that point outside the repo', async () => {
+      fs.symlinkSync(path.join(outsideDir, 'creds.txt'), path.join(testDir, 'src/link-file'));
+      fs.symlinkSync(path.join(outsideDir, 'dir'), path.join(testDir, 'src/link-dir'));
+
+      const helper = await createHelper();
+      for (const config of [
+        { type: 'base-commit', baseCommit: 'HEAD' },
+        { type: 'uncommitted' },
+        { type: 'all' },
+      ]) {
+        const diffs = await helper.getDiffBasedOnReviewConfig(config);
+        expect(leaked(diffs)).toEqual([]);
+      }
+    });
+
+    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'));
+      git('checkout -b security-committed-link');
+      git('add src/committed-link');
+      git('commit -m "committed link"');
+
+      const helper = await createHelper();
+      const diffs = await helper.getDiffBasedOnReviewConfig({ type: 'base-commit', baseCommit: baseHash });
+
+      expect(diffs.map(d => d.filename_str)).toContain('src/committed-link');
+      expect(leaked(diffs)).toEqual([]);
+    });
+
+    it('still reads symlinks that stay inside the repo', async () => {
+      fs.symlinkSync('index.js', path.join(testDir, 'src/inside-link.js'));
+
+      const helper = await createHelper();
+      const diffs = await helper.getDiffBasedOnReviewConfig({ type: 'base-commit', baseCommit: 'HEAD' });
+
+      const diff = diffs.find(d => d.filename_str === 'src/inside-link.js');
+      expect(diff).toBeDefined();
+      expect(diff.head_file_str).toBe(readFile('src/index.js'));
+    });
+  });
 });