From 49d7841ab30d5057fee410c398b653e444431341 Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Sun, 5 Apr 2026 11:45:05 -0700
Subject: [PATCH] fix(browse): add path validation to upload command (#821)

Add isPathWithin() and path traversal checks to the upload command,
blocking file exfiltration via crafted upload paths. Uses existing
SAFE_DIRECTORIES constant instead of a local copy. Adds 3 regression tests.

Co-authored-by: garagon <garagon@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 browse/src/write-commands.ts        | 11 ++++++++++-
 browse/test/path-validation.test.ts | 22 +++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts
index 5314795e..a569ce6b 100644
--- a/browse/src/write-commands.ts
+++ b/browse/src/write-commands.ts
@@ -399,9 +399,18 @@ export async function handleWriteCommand(
       const [selector, ...filePaths] = args;
       if (!selector || filePaths.length === 0) throw new Error('Usage: browse upload <selector> <file1> [file2...]');
 
-      // Validate all files exist before upload
+      // Validate paths are within safe directories (same check as cookie-import)
       for (const fp of filePaths) {
         if (!fs.existsSync(fp)) throw new Error(`File not found: ${fp}`);
+        if (path.isAbsolute(fp)) {
+          const resolvedFp = path.resolve(fp);
+          if (!SAFE_DIRECTORIES.some(dir => isPathWithin(resolvedFp, dir))) {
+            throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
+          }
+        }
+        if (path.normalize(fp).includes('..')) {
+          throw new Error('Path traversal sequences (..) are not allowed');
+        }
       }
 
       const resolved = await bm.resolveRef(selector);
diff --git a/browse/test/path-validation.test.ts b/browse/test/path-validation.test.ts
index 8a26436c..55447f40 100644
--- a/browse/test/path-validation.test.ts
+++ b/browse/test/path-validation.test.ts
@@ -1,7 +1,7 @@
 import { describe, it, expect } from 'bun:test';
 import { validateOutputPath } from '../src/meta-commands';
 import { validateReadPath } from '../src/read-commands';
-import { symlinkSync, unlinkSync, writeFileSync } from 'fs';
+import { readFileSync, symlinkSync, unlinkSync, writeFileSync } from 'fs';
 import { tmpdir } from 'os';
 import { join } from 'path';
 
@@ -35,6 +35,26 @@ describe('validateOutputPath', () => {
   });
 });
 
+describe('upload command path validation', () => {
+  const src = readFileSync(join(__dirname, '..', 'src', 'write-commands.ts'), 'utf-8');
+
+  it('validates upload paths with isPathWithin', () => {
+    const uploadBlock = src.slice(src.indexOf("case 'upload'"), src.indexOf("case 'dialog-accept'"));
+    expect(uploadBlock).toContain('isPathWithin');
+  });
+
+  it('blocks path traversal in upload', () => {
+    const uploadBlock = src.slice(src.indexOf("case 'upload'"), src.indexOf("case 'dialog-accept'"));
+    expect(uploadBlock).toContain("'..'");
+  });
+
+  it('checks absolute paths against safe directories', () => {
+    const uploadBlock = src.slice(src.indexOf("case 'upload'"), src.indexOf("case 'dialog-accept'"));
+    expect(uploadBlock).toContain('path.isAbsolute');
+    expect(uploadBlock).toContain('SAFE_DIRECTORIES');
+  });
+});
+
 describe('validateReadPath', () => {
   it('allows absolute paths within /tmp', () => {
     expect(() => validateReadPath('/tmp/script.js')).not.toThrow();