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>
This commit is contained in:
Garry Tan
2026-04-05 11:45:05 -07:00
parent 4bc4bfedfd
commit 49d7841ab3
2 changed files with 31 additions and 2 deletions
+10 -1
View File
@@ -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);
+21 -1
View File
@@ -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();