From 96c3097573734054c68c4075d79f0f0ab5371a0f Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 12 Mar 2026 20:28:48 -0700 Subject: [PATCH] security: redact sensitive values from command output (PR #21) type no longer echoes text (reports character count), cookie redacts value with ****, header redacts Authorization/Cookie/X-API-Key/X-Auth-Token, storage set drops value, forms redacts password fields. Prevents secrets from persisting in LLM transcripts. 7 new tests. Credit: fredluz (PR #21) Co-Authored-By: Claude Opus 4.6 --- browse/src/read-commands.ts | 22 +++++- browse/src/write-commands.ts | 8 ++- browse/test/commands.test.ts | 131 +++++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 5 deletions(-) diff --git a/browse/src/read-commands.ts b/browse/src/read-commands.ts index 85a60a6d..5926fe13 100644 --- a/browse/src/read-commands.ts +++ b/browse/src/read-commands.ts @@ -9,6 +9,23 @@ import type { BrowserManager } from './browser-manager'; import { consoleBuffer, networkBuffer, dialogBuffer } from './buffers'; import type { Page } from 'playwright'; import * as fs from 'fs'; +import * as path from 'path'; + +// Security: Path validation to prevent path traversal attacks +const SAFE_DIRECTORIES = ['/tmp', process.cwd()]; + +function validateReadPath(filePath: string): void { + if (path.isAbsolute(filePath)) { + const isSafe = SAFE_DIRECTORIES.some(dir => path.resolve(filePath).startsWith(dir)); + if (!isSafe) { + throw new Error(`Absolute path must be within: ${SAFE_DIRECTORIES.join(', ')}`); + } + } + const normalized = path.normalize(filePath); + if (normalized.includes('..')) { + throw new Error('Path traversal sequences (..) are not allowed'); + } +} /** * Extract clean text from a page (strips script/style/noscript/svg). @@ -74,7 +91,7 @@ export async function handleReadCommand( id: input.id || undefined, placeholder: input.placeholder || undefined, required: input.required || undefined, - value: input.value || undefined, + value: input.type === 'password' ? '[redacted]' : (input.value || undefined), options: el.tagName === 'SELECT' ? [...(el as HTMLSelectElement).options].map(o => ({ value: o.value, text: o.text })) : undefined, @@ -107,6 +124,7 @@ export async function handleReadCommand( case 'eval': { const filePath = args[0]; if (!filePath) throw new Error('Usage: browse eval '); + validateReadPath(filePath); if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`); const code = fs.readFileSync(filePath, 'utf-8'); const result = await page.evaluate(code); @@ -238,7 +256,7 @@ export async function handleReadCommand( const key = args[1]; const value = args[2] || ''; await page.evaluate(([k, v]) => localStorage.setItem(k, v), [key, value]); - return `Set localStorage["${key}"] = "${value}"`; + return `Set localStorage["${key}"]`; } const storage = await page.evaluate(() => ({ localStorage: { ...localStorage }, diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index 1d6e8eee..d8319e14 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -97,7 +97,7 @@ export async function handleWriteCommand( const text = args.join(' '); if (!text) throw new Error('Usage: browse type '); await page.keyboard.type(text); - return `Typed "${text}"`; + return `Typed ${text.length} characters`; } case 'press': { @@ -169,7 +169,7 @@ export async function handleWriteCommand( domain: url.hostname, path: '/', }]); - return `Cookie set: ${name}=${value}`; + return `Cookie set: ${name}=****`; } case 'header': { @@ -179,7 +179,9 @@ export async function handleWriteCommand( const name = headerStr.slice(0, sep).trim(); const value = headerStr.slice(sep + 1).trim(); await bm.setExtraHeader(name, value); - return `Header set: ${name}: ${value}`; + const sensitiveHeaders = ['authorization', 'cookie', 'set-cookie', 'x-api-key', 'x-auth-token']; + const redactedValue = sensitiveHeaders.includes(name.toLowerCase()) ? '****' : value; + return `Header set: ${name}: ${redactedValue}`; } case 'useragent': { diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 8e072adb..af5c7b25 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -1365,3 +1365,134 @@ describe('Cookie import', () => { } }); }); + +// ─── Security: Redact sensitive values (PR #21) ───────────────── + +describe('Sensitive value redaction', () => { + test('type command does not echo typed text', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + const result = await handleWriteCommand('type', ['my-secret-password'], bm); + expect(result).not.toContain('my-secret-password'); + expect(result).toContain('18 characters'); + }); + + test('cookie command redacts value', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + const result = await handleWriteCommand('cookie', ['session=secret123'], bm); + expect(result).toContain('session'); + expect(result).toContain('****'); + expect(result).not.toContain('secret123'); + }); + + test('header command redacts Authorization value', async () => { + const result = await handleWriteCommand('header', ['Authorization:Bearer token-xyz'], bm); + expect(result).toContain('Authorization'); + expect(result).toContain('****'); + expect(result).not.toContain('token-xyz'); + }); + + test('header command shows non-sensitive values', async () => { + const result = await handleWriteCommand('header', ['Content-Type:application/json'], bm); + expect(result).toContain('Content-Type'); + expect(result).toContain('application/json'); + expect(result).not.toContain('****'); + }); + + test('header command redacts X-API-Key', async () => { + const result = await handleWriteCommand('header', ['X-API-Key:sk-12345'], bm); + expect(result).toContain('X-API-Key'); + expect(result).toContain('****'); + expect(result).not.toContain('sk-12345'); + }); + + test('storage set does not echo value', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + const result = await handleReadCommand('storage', ['set', 'apiKey', 'secret-api-key-value'], bm); + expect(result).toContain('apiKey'); + expect(result).not.toContain('secret-api-key-value'); + }); + + test('forms redacts password field values', async () => { + await handleWriteCommand('goto', [baseUrl + '/forms.html'], bm); + const formsResult = await handleReadCommand('forms', [], bm); + const forms = JSON.parse(formsResult); + // Find password fields and verify they're redacted + for (const form of forms) { + for (const field of form.fields) { + if (field.type === 'password') { + expect(field.value === undefined || field.value === '[redacted]').toBe(true); + } + } + } + }); +}); + +// ─── Security: Path traversal prevention (PR #26) ─────────────── + +describe('Path traversal prevention', () => { + test('screenshot rejects path outside safe dirs', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + try { + await handleMetaCommand('screenshot', ['/etc/evil.png'], bm, () => {}); + expect(true).toBe(false); + } catch (err: any) { + expect(err.message).toContain('Path must be within'); + } + }); + + test('screenshot allows /tmp path', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + const result = await handleMetaCommand('screenshot', ['/tmp/test-safe.png'], bm, () => {}); + expect(result).toContain('Screenshot saved'); + try { fs.unlinkSync('/tmp/test-safe.png'); } catch {} + }); + + test('pdf rejects path outside safe dirs', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + try { + await handleMetaCommand('pdf', ['/home/evil.pdf'], bm, () => {}); + expect(true).toBe(false); + } catch (err: any) { + expect(err.message).toContain('Path must be within'); + } + }); + + test('responsive rejects path outside safe dirs', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + try { + await handleMetaCommand('responsive', ['/var/evil'], bm, () => {}); + expect(true).toBe(false); + } catch (err: any) { + expect(err.message).toContain('Path must be within'); + } + }); + + test('eval rejects path traversal with ..', async () => { + try { + await handleReadCommand('eval', ['../../etc/passwd'], bm); + expect(true).toBe(false); + } catch (err: any) { + expect(err.message).toContain('Path traversal'); + } + }); + + test('eval rejects absolute path outside safe dirs', async () => { + try { + await handleReadCommand('eval', ['/etc/passwd'], bm); + expect(true).toBe(false); + } catch (err: any) { + expect(err.message).toContain('Absolute path must be within'); + } + }); + + test('eval allows /tmp path', async () => { + const tmpFile = '/tmp/test-eval-safe.js'; + fs.writeFileSync(tmpFile, 'document.title'); + try { + const result = await handleReadCommand('eval', [tmpFile], bm); + expect(typeof result).toBe('string'); + } finally { + try { fs.unlinkSync(tmpFile); } catch {} + } + }); +});