diff --git a/browse/src/meta-commands.ts b/browse/src/meta-commands.ts index 4bd0faae7..4d596975f 100644 --- a/browse/src/meta-commands.ts +++ b/browse/src/meta-commands.ts @@ -667,40 +667,13 @@ export async function handleMetaCommand( lastWasWrite = WRITE_COMMANDS.has(c.name); } } else { - // Fallback: direct dispatch (CLI mode, no server context) - const { handleReadCommand } = await import('./read-commands'); - const { handleWriteCommand } = await import('./write-commands'); - - for (const c of commands) { - const name = c.name; - const cmdArgs = c.args; - const label = c.rawName === name ? name : `${c.rawName}→${name}`; - try { - let result: string; - if (WRITE_COMMANDS.has(name)) { - if (bm.isWatching()) { - result = 'BLOCKED: write commands disabled in watch mode'; - } else { - result = await handleWriteCommand(name, cmdArgs, session, bm); - } - lastWasWrite = true; - } else if (READ_COMMANDS.has(name)) { - result = await handleReadCommand(name, cmdArgs, session); - if (PAGE_CONTENT_COMMANDS.has(name)) { - result = wrapUntrustedContent(result, bm.getCurrentUrl()); - } - lastWasWrite = false; - } else if (META_COMMANDS.has(name)) { - result = await handleMetaCommand(name, cmdArgs, bm, shutdown, tokenInfo, opts); - lastWasWrite = false; - } else { - throw new Error(`Unknown command: ${c.rawName}`); - } - results.push(`[${label}] ${result}`); - } catch (err: any) { - results.push(`[${label}] ERROR: ${err.message}`); - } - } + // No fallback dispatcher. The old direct-dispatch branch here + // re-implemented command routing WITHOUT the server pipeline's + // security gates (scope, domain, tab ownership, rate limit, hidden + // element stripping, scoped-token enveloping, JS-origin assertion). + // It was unreachable in production (server.ts always passes + // executeCommand) and one boolean away from being live. + throw new Error('chain requires the browse server (no executeCommand context)'); } // Wait for network to settle after write commands before returning diff --git a/browse/src/read-commands.ts b/browse/src/read-commands.ts index 4e1371a17..1186c711a 100644 --- a/browse/src/read-commands.ts +++ b/browse/src/read-commands.ts @@ -212,7 +212,7 @@ export async function handleReadCommand( command: string, args: string[], session: TabSession, - bm?: BrowserManager, + bm: BrowserManager, ): Promise { const page = session.getPage(); // Frame-aware target for content extraction @@ -293,7 +293,7 @@ export async function handleReadCommand( const { outPath, raw, rest } = parseOutArgs(args); const expr = rest[0]; if (!expr) throw new Error('Usage: browse js [--out ] [--raw]'); - if (bm) assertJsOriginAllowed(bm, page.url()); + assertJsOriginAllowed(bm, page.url()); const wrapped = wrapForEvaluate(expr); const result = await target.evaluate(wrapped); const str = resultToString(result); @@ -308,7 +308,7 @@ export async function handleReadCommand( const { outPath, raw, rest } = parseOutArgs(args); const filePath = rest[0]; if (!filePath) throw new Error('Usage: browse eval [--out ] [--raw]'); - if (bm) assertJsOriginAllowed(bm, page.url()); + assertJsOriginAllowed(bm, page.url()); validateReadPath(filePath); if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`); const code = fs.readFileSync(filePath, 'utf-8'); diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index 9382cb27e..de3b6fbe0 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -12,6 +12,7 @@ import { resolveServerScript } from '../src/cli'; import { handleReadCommand as _handleReadCommand, parseOutArgs, hasOutArg, resultToString } from '../src/read-commands'; import { handleWriteCommand as _handleWriteCommand } from '../src/write-commands'; import { handleMetaCommand } from '../src/meta-commands'; +import { WRITE_COMMANDS, READ_COMMANDS, META_COMMANDS, PAGE_CONTENT_COMMANDS, wrapUntrustedContent } from '../src/commands'; import { consoleBuffer, networkBuffer, dialogBuffer, addConsoleEntry, addNetworkEntry, addDialogEntry, CircularBuffer } from '../src/buffers'; import * as fs from 'fs'; import { spawn } from 'child_process'; @@ -19,10 +20,41 @@ import * as path from 'path'; // Thin wrappers that bridge old test calls (bm as 3rd arg) to new signatures (session + bm) const handleReadCommand = (cmd: string, args: string[], b: BrowserManager) => - _handleReadCommand(cmd, args, b.getActiveSession()); + _handleReadCommand(cmd, args, b.getActiveSession(), b); const handleWriteCommand = (cmd: string, args: string[], b: BrowserManager) => _handleWriteCommand(cmd, args, b.getActiveSession(), b); +// Chain routes every subcommand through the server's executeCommand pipeline in +// production (the direct-dispatch fallback was deleted — it skipped the security +// gates). Tests mirror the pipeline minimally: real handlers + trust-wrapping, +// server-shaped {status, result} envelope. +function makeChainExecute(b: BrowserManager) { + return async (body: { command: string; args?: string[] }) => { + const name = body.command; + const args = body.args ?? []; + try { + let result: string; + if (WRITE_COMMANDS.has(name)) { + result = await _handleWriteCommand(name, args, b.getActiveSession(), b); + } else if (READ_COMMANDS.has(name)) { + result = await _handleReadCommand(name, args, b.getActiveSession(), b); + if (PAGE_CONTENT_COMMANDS.has(name)) { + result = wrapUntrustedContent(result, b.getCurrentUrl()); + } + } else if (META_COMMANDS.has(name)) { + result = await handleMetaCommand(name, args, b, async () => {}); + } else { + return { status: 404, result: JSON.stringify({ error: `Unknown command: ${name}` }) }; + } + return { status: 200, result }; + } catch (err: any) { + return { status: 500, result: JSON.stringify({ error: err.message }) }; + } + }; +} +const chainMeta = (b: BrowserManager, args: string[]) => + handleMetaCommand('chain', args, b, async () => {}, null, { executeCommand: makeChainExecute(b) }); + // ─── Pure arg-parser + result-conversion unit tests (no browser) ─── describe('parseOutArgs / hasOutArg', () => { test('--out splits the flag from the positional', () => { @@ -804,7 +836,7 @@ describe('Chain', () => { ['js', 'document.title'], ['css', 'h1', 'color'], ]); - const result = await handleMetaCommand('chain', [commands], bm, async () => {}); + const result = await chainMeta(bm, [commands]); expect(result).toContain('[goto]'); expect(result).toContain('Test Page - Basic'); expect(result).toContain('[css]'); @@ -812,7 +844,7 @@ describe('Chain', () => { test('chain wraps page-content sub-commands with trust markers', async () => { await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); - const result = await handleMetaCommand('chain', ['text'], bm, async () => {}); + const result = await chainMeta(bm, ['text']); expect(result).toContain('BEGIN UNTRUSTED EXTERNAL CONTENT'); expect(result).toContain('END UNTRUSTED EXTERNAL CONTENT'); }); @@ -821,7 +853,7 @@ describe('Chain', () => { const commands = JSON.stringify([ ['goto', 'http://localhost:1/unreachable'], ]); - const result = await handleMetaCommand('chain', [commands], bm, async () => {}); + const result = await chainMeta(bm, [commands]); expect(result).toContain('[goto] ERROR:'); expect(result).not.toContain('Unknown meta command'); expect(result).not.toContain('Unknown read command'); @@ -1505,14 +1537,14 @@ describe('Errors', () => { test('chain with invalid JSON falls back to pipe format', async () => { // Non-JSON input is now treated as pipe-delimited format // 'not json' → [["not", "json"]] → "not" is unknown command → error in result - const result = await handleMetaCommand('chain', ['not json'], bm, async () => {}); + const result = await chainMeta(bm, ['not json']); expect(result).toContain('ERROR'); expect(result).toContain('Unknown command: not'); }); test('chain with no arg throws', async () => { try { - await handleMetaCommand('chain', [], bm, async () => {}); + await chainMeta(bm, []); expect(true).toBe(false); } catch (err: any) { expect(err.message).toContain('Usage'); @@ -2006,7 +2038,7 @@ describe('Chain with cookie-import', () => { const commands = JSON.stringify([ ['cookie-import', tmpCookies], ]); - const result = await handleMetaCommand('chain', [commands], bm, async () => {}); + const result = await chainMeta(bm, [commands]); expect(result).toContain('[cookie-import]'); expect(result).toContain('Loaded 1 cookie'); } finally { @@ -2051,24 +2083,14 @@ describe('Network idle', () => { describe('Chain pipe format', () => { test('pipe-delimited commands work', async () => { - const result = await handleMetaCommand( - 'chain', - [`goto ${baseUrl}/basic.html | js document.title`], - bm, - async () => {} - ); + const result = await chainMeta(bm, [`goto ${baseUrl}/basic.html | js document.title`]); expect(result).toContain('[goto]'); expect(result).toContain('[js]'); expect(result).toContain('Test Page - Basic'); }); test('pipe format with quoted args', async () => { - const result = await handleMetaCommand( - 'chain', - [`goto ${baseUrl}/forms.html | fill #email "pipe@test.com"`], - bm, - async () => {} - ); + const result = await chainMeta(bm, [`goto ${baseUrl}/forms.html | fill #email "pipe@test.com"`]); expect(result).toContain('[fill]'); expect(result).toContain('Filled'); // Verify the fill actually worked @@ -2081,18 +2103,13 @@ describe('Chain pipe format', () => { ['goto', baseUrl + '/basic.html'], ['js', 'document.title'], ]); - const result = await handleMetaCommand('chain', [commands], bm, async () => {}); + const result = await chainMeta(bm, [commands]); expect(result).toContain('[goto]'); expect(result).toContain('Test Page - Basic'); }); test('pipe format with unknown command includes error', async () => { - const result = await handleMetaCommand( - 'chain', - ['bogus command'], - bm, - async () => {} - ); + const result = await chainMeta(bm, ['bogus command']); expect(result).toContain('ERROR'); expect(result).toContain('Unknown command: bogus'); }); @@ -2569,14 +2586,14 @@ describe('Command aliases', () => { test('setcontent alias routes to load-html via chain', async () => { // Chain canonicalizes aliases end-to-end; verifies the dispatch path - const result = await handleMetaCommand('chain', [JSON.stringify([['setcontent', aliasFix]])], bm, async () => {}); + const result = await chainMeta(bm, [JSON.stringify([['setcontent', aliasFix]])]); expect(result).toContain('Loaded HTML:'); const text = await handleReadCommand('text', [], bm); expect(text).toContain('alias routing ok'); }); test('set-content (hyphenated) alias also routes', async () => { - const result = await handleMetaCommand('chain', [JSON.stringify([['set-content', aliasFix]])], bm, async () => {}); + const result = await chainMeta(bm, [JSON.stringify([['set-content', aliasFix]])]); expect(result).toContain('Loaded HTML:'); }); }); diff --git a/browse/test/security-audit-r2.test.ts b/browse/test/security-audit-r2.test.ts index 9af4bcb6f..fc827af53 100644 --- a/browse/test/security-audit-r2.test.ts +++ b/browse/test/security-audit-r2.test.ts @@ -339,15 +339,18 @@ describe('frame --url ReDoS fix', () => { // ─── Task 7: watch-mode guard in chain command ─────────────────────────────── describe('chain command watch-mode guard', () => { - it('chain loop contains isWatching() guard before write dispatch', () => { - // Post-alias refactor: loop iterates over canonicalized `c of commands`. - const block = sliceBetween(META_SRC, 'for (const c of commands)', 'Wait for network to settle'); - expect(block).toContain('isWatching'); + // The direct-dispatch fallback (which carried its own isWatching() guard) + // was deleted — it skipped every OTHER server gate. Chain subcommands now + // route exclusively through executeCommand -> handleCommandInternal, whose + // watch-mode write gate covers them. Pin both halves of that contract. + it('chain has no direct-dispatch fallback (executeCommand is mandatory)', () => { + const block = sliceBetween(META_SRC, 'const executeCmd = opts?.executeCommand', 'Wait for network to settle'); + expect(block).toContain('chain requires the browse server (no executeCommand context)'); + expect(block).not.toContain('handleWriteCommand('); }); - it('chain loop BLOCKED message appears for write commands in watch mode', () => { - const block = sliceBetween(META_SRC, 'for (const c of commands)', 'Wait for network to settle'); - expect(block).toContain('BLOCKED: write commands disabled in watch mode'); + it('server pipeline blocks write commands in watch mode (covers chain subcommands)', () => { + expect(SERVER_SRC).toMatch(/isWatching\(\)\s*&&\s*isWriteInvocation\(command, args\)/); }); });