fix(security): path validation bypass in upload and cookie-import commands

browse/src/write-commands.ts

@@ -10,7 +10,7 @@ import type { BrowserManager } from './browser-manager';
 import { findInstalledBrowsers, importCookies, listSupportedBrowserNames } from './cookie-import-browser';
 import { generatePickerCode } from './cookie-picker-routes';
 import { validateNavigationUrl } from './url-validation';
-import { validateOutputPath } from './path-security';
+import { validateOutputPath, validateReadPath } from './path-security';
 import * as fs from 'fs';
 import * as path from 'path';
 import { TEMP_DIR } from './platform';
@@ -394,19 +394,10 @@ export async function handleWriteCommand(
       const [selector, ...filePaths] = args;
       if (!selector || filePaths.length === 0) throw new Error('Usage: browse upload <selector> <file1> [file2...]');
 
-      // Validate paths are within safe directories (same check as cookie-import)
+      // Validate paths through shared path-security module (resolve + realpath + boundary check)
       for (const fp of filePaths) {
         if (!fs.existsSync(fp)) throw new Error(`File not found: ${fp}`);
-        if (path.isAbsolute(fp)) {
-          let resolvedFp: string;
-          try { resolvedFp = fs.realpathSync(path.resolve(fp)); } catch { 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');
-        }
+        validateReadPath(fp);
       }
 
       const resolved = await session.resolveRef(selector);
@@ -441,17 +432,8 @@ export async function handleWriteCommand(
     case 'cookie-import': {
       const filePath = args[0];
       if (!filePath) throw new Error('Usage: browse cookie-import <json-file>');
-      // Path validation — prevent reading arbitrary files
-      if (path.isAbsolute(filePath)) {
-        const safeDirs = [TEMP_DIR, process.cwd()];
-        const resolved = path.resolve(filePath);
-        if (!safeDirs.some(dir => isPathWithin(resolved, dir))) {
-          throw new Error(`Path must be within: ${safeDirs.join(', ')}`);
-        }
-      }
-      if (path.normalize(filePath).includes('..')) {
-        throw new Error('Path traversal sequences (..) are not allowed');
-      }
+      // Path validation through shared path-security module (resolve + realpath + boundary check)
+      validateReadPath(filePath);
       if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`);
       const raw = fs.readFileSync(filePath, 'utf-8');
       let cookies: any[];

browse/test/path-validation.test.ts

@@ -39,20 +39,31 @@ describe('validateOutputPath', () => {
 describe('upload command path validation', () => {
   const src = readFileSync(join(__dirname, '..', 'src', 'write-commands.ts'), 'utf-8');
 
-  it('validates upload paths with isPathWithin', () => {
+  it('uses validateReadPath from path-security module', () => {
     const uploadBlock = src.slice(src.indexOf("case 'upload'"), src.indexOf("case 'dialog-accept'"));
-    expect(uploadBlock).toContain('isPathWithin');
+    expect(uploadBlock).toContain('validateReadPath');
   });
 
-  it('blocks path traversal in upload', () => {
+  it('does not use inline path traversal checks (replaced by path-security)', () => {
     const uploadBlock = src.slice(src.indexOf("case 'upload'"), src.indexOf("case 'dialog-accept'"));
-    expect(uploadBlock).toContain("'..'");
+    // Inline checks replaced by validateReadPath which handles resolve + realpath + boundary
+    expect(uploadBlock).not.toContain('includes');
+    expect(uploadBlock).not.toContain('SAFE_DIRECTORIES');
   });
+});
 
-  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('cookie-import command path validation', () => {
+  const src = readFileSync(join(__dirname, '..', 'src', 'write-commands.ts'), 'utf-8');
+
+  it('uses validateReadPath from path-security module', () => {
+    const importBlock = src.slice(src.indexOf("case 'cookie-import':"), src.indexOf("case 'cookie-import-browser':"));
+    expect(importBlock).toContain('validateReadPath');
+  });
+
+  it('does not use inline path traversal checks (replaced by path-security)', () => {
+    const importBlock = src.slice(src.indexOf("case 'cookie-import':"), src.indexOf("case 'cookie-import-browser':"));
+    expect(importBlock).not.toContain("includes('..')");
+    expect(importBlock).not.toContain('isPathWithin');
   });
 });