From b60162ae45b91a4397171b1e957d0cec2d533ab3 Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Fri, 27 Mar 2026 22:14:02 -0700
Subject: [PATCH] fix: symlink bypass in validateReadPath (MEDIUM-02)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Always resolve to absolute path first (fixes relative path bypass)
- Use realpathSync to follow symlinks before boundary check
- Throw on non-ENOENT realpathSync failures (explicit over silent)
- Resolve SAFE_DIRECTORIES through realpathSync (macOS /tmp → /private/tmp)
- Resolve directory part for non-existent files (ENOENT with symlinked parent)
---
 browse/src/read-commands.ts | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/browse/src/read-commands.ts b/browse/src/read-commands.ts
index 802c3813..5615b60f 100644
--- a/browse/src/read-commands.ts
+++ b/browse/src/read-commands.ts
@@ -37,19 +37,34 @@ function wrapForEvaluate(code: string): string {
 }
 
 // Security: Path validation to prevent path traversal attacks
-const SAFE_DIRECTORIES = [TEMP_DIR, process.cwd()];
+// Resolve safe directories through realpathSync to handle symlinks (e.g., macOS /tmp → /private/tmp)
+const SAFE_DIRECTORIES = [TEMP_DIR, process.cwd()].map(d => {
+  try { return fs.realpathSync(d); } catch { return d; }
+});
 
 export function validateReadPath(filePath: string): void {
-  if (path.isAbsolute(filePath)) {
-    const resolved = path.resolve(filePath);
-    const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(resolved, dir));
-    if (!isSafe) {
-      throw new Error(`Absolute path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
+  // Always resolve to absolute first (fixes relative path symlink bypass)
+  const resolved = path.resolve(filePath);
+  // Resolve symlinks — throw on non-ENOENT errors
+  let realPath: string;
+  try {
+    realPath = fs.realpathSync(resolved);
+  } catch (err: any) {
+    if (err.code === 'ENOENT') {
+      // File doesn't exist — resolve directory part for symlinks (e.g., /tmp → /private/tmp)
+      try {
+        const dir = fs.realpathSync(path.dirname(resolved));
+        realPath = path.join(dir, path.basename(resolved));
+      } catch {
+        realPath = resolved;
+      }
+    } else {
+      throw new Error(`Cannot resolve real path: ${filePath} (${err.code})`);
     }
   }
-  const normalized = path.normalize(filePath);
-  if (normalized.includes('..')) {
-    throw new Error('Path traversal sequences (..) are not allowed');
+  const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realPath, dir));
+  if (!isSafe) {
+    throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
   }
 }