From ce07abe81d40748e607adf72a9d52ddefbde12bb Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 18 Apr 2026 18:17:43 +0800 Subject: [PATCH] feat(browse): load-html, screenshot --selector, viewport --scale, alias dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the new handlers and dispatch logic that the previous commits made possible: write-commands.ts - New 'load-html' case: validateReadPath for safe-dir scoping, stat-based actionable errors (not found, directory, oversize), extension allowlist (.html/.htm/.xhtml/.svg), magic-byte sniff with UTF-8 BOM strip accepting any <[a-zA-Z!?] markup opener (not just ... work for setContent), 50MB cap via GSTACK_BROWSE_MAX_HTML_BYTES override, frame-context rejection. Calls session.setTabContent() so replay metadata is rehydrated. - viewport command extended: optional [], optional [--scale ], scale-only variant reads current size via page.viewportSize(). Invalid scale (NaN, Infinity, empty, out of 1-3) throws with named value. Headed mode rejected explicitly. - clearLoadedHtml() called BEFORE goto/back/forward/reload navigation (not after) so a timed-out goto post-commit doesn't leave stale metadata that could resurrect on a later context recreation. Codex v2 P1 catch. - goto uses validateNavigationUrl's normalized return value. meta-commands.ts - screenshot --selector flag: explicit element-screenshot form. Rejects alongside positional selector (both = error), preserves --clip conflict at line 161, composes with --base64 at lines 168-174. - chain canonicalizes each step with canonicalizeCommand — step shape is now { rawName, name, args } so prevalidation, dispatch, WRITE_COMMANDS.has, watch blocking, and result labels all use canonical names while audit labels show 'rawName→name' when aliased. Codex v3 P2 catch — prior shape only canonicalized at prevalidation and diverged everywhere else. - diff command consumes validateNavigationUrl return value for both URLs. server.ts - Command canonicalization inserted immediately after parse, before scope / watch / tab-ownership / content-wrapping checks. rawCommand preserved for future audit (not wired into audit log in this commit — follow-up). - Unknown-command handler replaced with buildUnknownCommandError() from commands.ts — produces 'Unknown command: X. Did you mean Y?' with optional upgrade hint for NEW_IN_VERSION entries. security-audit-r2.test.ts - Updated chain-loop marker from 'for (const cmd of commands)' to 'for (const c of commands)' to match the new chain step shape. Same isWatching + BLOCKED invariants still asserted. --- browse/src/meta-commands.ts | 78 ++++++++----- browse/src/server.ts | 20 +++- browse/src/write-commands.ts | 161 ++++++++++++++++++++++++-- browse/test/security-audit-r2.test.ts | 5 +- 4 files changed, 222 insertions(+), 42 deletions(-) diff --git a/browse/src/meta-commands.ts b/browse/src/meta-commands.ts index 392602f0..0ba9d8d6 100644 --- a/browse/src/meta-commands.ts +++ b/browse/src/meta-commands.ts @@ -5,7 +5,7 @@ import type { BrowserManager } from './browser-manager'; import { handleSnapshot } from './snapshot'; import { getCleanText } from './read-commands'; -import { READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS, PAGE_CONTENT_COMMANDS, wrapUntrustedContent } from './commands'; +import { READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS, PAGE_CONTENT_COMMANDS, wrapUntrustedContent, canonicalizeCommand } from './commands'; import { validateNavigationUrl } from './url-validation'; import { checkScope, type TokenInfo } from './token-registry'; import { validateOutputPath, escapeRegExp } from './path-security'; @@ -124,11 +124,15 @@ export async function handleMetaCommand( let base64Mode = false; const remaining: string[] = []; + let flagSelector: string | undefined; for (let i = 0; i < args.length; i++) { if (args[i] === '--viewport') { viewportOnly = true; } else if (args[i] === '--base64') { base64Mode = true; + } else if (args[i] === '--selector') { + flagSelector = args[++i]; + if (!flagSelector) throw new Error('Usage: screenshot --selector [path]'); } else if (args[i] === '--clip') { const coords = args[++i]; if (!coords) throw new Error('Usage: screenshot --clip x,y,w,h [path]'); @@ -156,6 +160,14 @@ export async function handleMetaCommand( } } + // --selector flag takes precedence; conflict with positional selector. + if (flagSelector !== undefined) { + if (targetSelector !== undefined) { + throw new Error('--selector conflicts with positional selector — choose one'); + } + targetSelector = flagSelector; + } + validateOutputPath(outputPath); if (clipRect && targetSelector) { @@ -244,27 +256,36 @@ export async function handleMetaCommand( ' or: browse chain \'goto url | click @e5 | snapshot -ic\'' ); - let commands: string[][]; + let rawCommands: string[][]; try { - commands = JSON.parse(jsonStr); - if (!Array.isArray(commands)) throw new Error('not array'); + rawCommands = JSON.parse(jsonStr); + if (!Array.isArray(rawCommands)) throw new Error('not array'); } catch (err: any) { // Fallback: pipe-delimited format "goto url | click @e5 | snapshot -ic" if (!(err instanceof SyntaxError) && err?.message !== 'not array') throw err; - commands = jsonStr.split(' | ') + rawCommands = jsonStr.split(' | ') .filter(seg => seg.trim().length > 0) .map(seg => tokenizePipeSegment(seg.trim())); } + // Canonicalize aliases across the whole chain. Pair canonical name with the raw + // input so result labels + error messages reflect what the user typed, but every + // dispatch path (scope check, WRITE_COMMANDS.has, watch blocking, handler lookup) + // uses the canonical name. Otherwise `chain '[["setcontent","/tmp/x.html"]]'` + // bypasses prevalidation or runs under the wrong command set. + const commands = rawCommands.map(cmd => { + const [rawName, ...cmdArgs] = cmd; + const name = canonicalizeCommand(rawName); + return { rawName, name, args: cmdArgs }; + }); + // Pre-validate ALL subcommands against the token's scope before executing any. - // This prevents partial execution where some subcommands succeed before a - // scope violation is hit, leaving the browser in an inconsistent state. + // Uses canonical name so aliases don't bypass scope checks. if (tokenInfo && tokenInfo.clientId !== 'root') { - for (const cmd of commands) { - const [name] = cmd; - if (!checkScope(tokenInfo, name)) { + for (const c of commands) { + if (!checkScope(tokenInfo, c.name)) { throw new Error( - `Chain rejected: subcommand "${name}" not allowed by your token scope (${tokenInfo.scopes.join(', ')}). ` + + `Chain rejected: subcommand "${c.rawName}" not allowed by your token scope (${tokenInfo.scopes.join(', ')}). ` + `All subcommands must be within scope.` ); } @@ -280,30 +301,33 @@ export async function handleMetaCommand( let lastWasWrite = false; if (executeCmd) { - // Full security pipeline via handleCommandInternal - for (const cmd of commands) { - const [name, ...cmdArgs] = cmd; + // Full security pipeline via handleCommandInternal. + // Pass rawName so the server's own canonicalization is a no-op (already canonical). + for (const c of commands) { const cr = await executeCmd( - { command: name, args: cmdArgs }, + { command: c.name, args: c.args }, tokenInfo, ); + const label = c.rawName === c.name ? c.name : `${c.rawName}→${c.name}`; if (cr.status === 200) { - results.push(`[${name}] ${cr.result}`); + results.push(`[${label}] ${cr.result}`); } else { // Parse error from JSON result let errMsg = cr.result; try { errMsg = JSON.parse(cr.result).error || cr.result; } catch (err: any) { if (!(err instanceof SyntaxError)) throw err; } - results.push(`[${name}] ERROR: ${errMsg}`); + results.push(`[${label}] ERROR: ${errMsg}`); } - lastWasWrite = WRITE_COMMANDS.has(name); + 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 cmd of commands) { - const [name, ...cmdArgs] = cmd; + 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)) { @@ -323,11 +347,11 @@ export async function handleMetaCommand( result = await handleMetaCommand(name, cmdArgs, bm, shutdown, tokenInfo, opts); lastWasWrite = false; } else { - throw new Error(`Unknown command: ${name}`); + throw new Error(`Unknown command: ${c.rawName}`); } - results.push(`[${name}] ${result}`); + results.push(`[${label}] ${result}`); } catch (err: any) { - results.push(`[${name}] ERROR: ${err.message}`); + results.push(`[${label}] ERROR: ${err.message}`); } } } @@ -346,12 +370,12 @@ export async function handleMetaCommand( if (!url1 || !url2) throw new Error('Usage: browse diff '); const page = bm.getPage(); - await validateNavigationUrl(url1); - await page.goto(url1, { waitUntil: 'domcontentloaded', timeout: 15000 }); + const normalizedUrl1 = await validateNavigationUrl(url1); + await page.goto(normalizedUrl1, { waitUntil: 'domcontentloaded', timeout: 15000 }); const text1 = await getCleanText(page); - await validateNavigationUrl(url2); - await page.goto(url2, { waitUntil: 'domcontentloaded', timeout: 15000 }); + const normalizedUrl2 = await validateNavigationUrl(url2); + await page.goto(normalizedUrl2, { waitUntil: 'domcontentloaded', timeout: 15000 }); const text2 = await getCleanText(page); const changes = Diff.diffLines(text1, text2); diff --git a/browse/src/server.ts b/browse/src/server.ts index 573a73d5..f460fb0e 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -19,7 +19,7 @@ import { handleWriteCommand } from './write-commands'; import { handleMetaCommand } from './meta-commands'; import { handleCookiePickerRoute, hasActivePicker } from './cookie-picker-routes'; import { sanitizeExtensionUrl } from './sidebar-utils'; -import { COMMAND_DESCRIPTIONS, PAGE_CONTENT_COMMANDS, wrapUntrustedContent } from './commands'; +import { COMMAND_DESCRIPTIONS, PAGE_CONTENT_COMMANDS, wrapUntrustedContent, canonicalizeCommand, buildUnknownCommandError, ALL_COMMANDS } from './commands'; import { wrapUntrustedPageContent, datamarkContent, runContentFilters, type ContentFilterResult, @@ -916,12 +916,21 @@ async function handleCommandInternal( tokenInfo?: TokenInfo | null, opts?: { skipRateCheck?: boolean; skipActivity?: boolean; chainDepth?: number }, ): Promise { - const { command, args = [], tabId } = body; + const { args = [], tabId } = body; + const rawCommand = body.command; - if (!command) { + if (!rawCommand) { return { status: 400, result: JSON.stringify({ error: 'Missing "command" field' }), json: true }; } + // ─── Alias canonicalization (before scope, watch, tab-ownership, dispatch) ─ + // Agent-friendly names like 'setcontent' route to canonical 'load-html'. Must + // happen BEFORE scope check so a read-scoped token calling 'setcontent' is still + // rejected (load-html lives in SCOPE_WRITE). Audit logging preserves rawCommand + // so the trail records what the agent actually typed. + const command = canonicalizeCommand(rawCommand); + const isAliased = command !== rawCommand; + // ─── Recursion guard: reject nested chains ────────────────── if (command === 'chain' && (opts?.chainDepth ?? 0) > 0) { return { status: 400, result: JSON.stringify({ error: 'Nested chain commands are not allowed' }), json: true }; @@ -1090,10 +1099,13 @@ async function handleCommandInternal( const helpText = generateHelpText(); return { status: 200, result: helpText }; } else { + // Use the rich unknown-command helper: names the input, suggests the closest + // match via Levenshtein (≤ 2 distance, ≥ 4 chars input), and appends an upgrade + // hint if the command is listed in NEW_IN_VERSION. return { status: 400, json: true, result: JSON.stringify({ - error: `Unknown command: ${command}`, + error: buildUnknownCommandError(rawCommand, ALL_COMMANDS), hint: `Available commands: ${[...READ_COMMANDS, ...WRITE_COMMANDS, ...META_COMMANDS].sort().join(', ')}`, }), }; diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index 8dbb16f7..1c7d547f 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -10,9 +10,10 @@ import type { BrowserManager } from './browser-manager'; import { findInstalledBrowsers, importCookies, importCookiesViaCdp, hasV20Cookies, 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 type { SetContentWaitUntil } from './tab-session'; import { TEMP_DIR, isPathWithin } from './platform'; import { SAFE_DIRECTORIES } from './path-security'; import { modifyStyle, undoModification, resetModifications, getModificationHistory } from './cdp-inspector'; @@ -142,30 +143,129 @@ export async function handleWriteCommand( if (inFrame) throw new Error('Cannot use goto inside a frame. Run \'frame main\' first.'); const url = args[0]; if (!url) throw new Error('Usage: browse goto '); - await validateNavigationUrl(url); - const response = await page.goto(url, { waitUntil: 'domcontentloaded', timeout: 15000 }); + // Clear loadedHtml BEFORE navigation — a timeout after the main-frame commit + // must not leave stale content that could resurrect on a later context recreation. + session.clearLoadedHtml(); + const normalizedUrl = await validateNavigationUrl(url); + const response = await page.goto(normalizedUrl, { waitUntil: 'domcontentloaded', timeout: 15000 }); const status = response?.status() || 'unknown'; - return `Navigated to ${url} (${status})`; + return `Navigated to ${normalizedUrl} (${status})`; } case 'back': { if (inFrame) throw new Error('Cannot use back inside a frame. Run \'frame main\' first.'); + session.clearLoadedHtml(); await page.goBack({ waitUntil: 'domcontentloaded', timeout: 15000 }); return `Back → ${page.url()}`; } case 'forward': { if (inFrame) throw new Error('Cannot use forward inside a frame. Run \'frame main\' first.'); + session.clearLoadedHtml(); await page.goForward({ waitUntil: 'domcontentloaded', timeout: 15000 }); return `Forward → ${page.url()}`; } case 'reload': { if (inFrame) throw new Error('Cannot use reload inside a frame. Run \'frame main\' first.'); + session.clearLoadedHtml(); await page.reload({ waitUntil: 'domcontentloaded', timeout: 15000 }); return `Reloaded ${page.url()}`; } + case 'load-html': { + if (inFrame) throw new Error('Cannot use load-html inside a frame. Run \'frame main\' first.'); + const filePath = args[0]; + if (!filePath) throw new Error('Usage: browse load-html [--wait-until load|domcontentloaded|networkidle]'); + + // Parse --wait-until flag + let waitUntil: SetContentWaitUntil = 'domcontentloaded'; + for (let i = 1; i < args.length; i++) { + if (args[i] === '--wait-until') { + const val = args[++i]; + if (val !== 'load' && val !== 'domcontentloaded' && val !== 'networkidle') { + throw new Error(`Invalid --wait-until '${val}'. Must be one of: load, domcontentloaded, networkidle.`); + } + waitUntil = val; + } else if (args[i].startsWith('--')) { + throw new Error(`Unknown flag: ${args[i]}`); + } + } + + // Extension allowlist + const ALLOWED_EXT = ['.html', '.htm', '.xhtml', '.svg']; + const ext = path.extname(filePath).toLowerCase(); + if (!ALLOWED_EXT.includes(ext)) { + throw new Error( + `load-html: file does not appear to be HTML. Expected .html/.htm/.xhtml/.svg, got ${ext || '(no extension)'}. Rename the file if it's really HTML.` + ); + } + + const absolutePath = path.resolve(filePath); + + // Safe-dirs check (reuses canonical read-side policy) + try { + validateReadPath(absolutePath); + } catch (e: any) { + throw new Error( + `load-html: ${absolutePath} must be under ${SAFE_DIRECTORIES.join(' or ')} (security policy). Copy the file into the project tree or /tmp first.` + ); + } + + // stat check — reject non-file targets with actionable error + let stat: fs.Stats; + try { + stat = await fs.promises.stat(absolutePath); + } catch (e: any) { + if (e.code === 'ENOENT') { + throw new Error( + `load-html: file not found at ${absolutePath}. Check spelling or copy the file under ${process.cwd()} or ${TEMP_DIR}.` + ); + } + throw e; + } + if (stat.isDirectory()) { + throw new Error(`load-html: ${absolutePath} is a directory, not a file. Pass a .html file.`); + } + if (!stat.isFile()) { + throw new Error(`load-html: ${absolutePath} is not a regular file.`); + } + + // Size cap + const MAX_BYTES = parseInt(process.env.GSTACK_BROWSE_MAX_HTML_BYTES || '', 10) || (50 * 1024 * 1024); + if (stat.size > MAX_BYTES) { + throw new Error( + `load-html: file too large (${stat.size} bytes > ${MAX_BYTES} cap). Raise with GSTACK_BROWSE_MAX_HTML_BYTES= or split the HTML.` + ); + } + + // Single read: Buffer → magic-byte peek → utf-8 string + const buf = await fs.promises.readFile(absolutePath); + + // Magic-byte check: strip UTF-8 BOM + leading whitespace, then verify the first + // non-whitespace byte starts a markup construct. Accepts any ...` + // which setContent wraps in a full document. Rejects binary files mis-renamed .html + // (first byte won't be `<`). + let peek = buf.slice(0, 200); + if (peek[0] === 0xEF && peek[1] === 0xBB && peek[2] === 0xBF) { + peek = peek.slice(3); + } + const peekStr = peek.toString('utf8').trimStart(); + // Valid markup opener: '<' followed by alpha (tag), '!' (doctype/comment), or '?' (xml prolog) + const looksLikeMarkup = /^<[a-zA-Z!?]/.test(peekStr); + if (!looksLikeMarkup) { + const hexDump = Array.from(buf.slice(0, 16)).map(b => b.toString(16).padStart(2, '0')).join(' '); + throw new Error( + `load-html: ${absolutePath} has ${ext} extension but content does not look like HTML. First bytes: ${hexDump}` + ); + } + + const html = buf.toString('utf8'); + await session.setTabContent(html, { waitUntil }); + return `Loaded HTML: ${absolutePath} (${stat.size} bytes)`; + } + case 'click': { const selector = args[0]; if (!selector) throw new Error('Usage: browse click '); @@ -343,11 +443,54 @@ export async function handleWriteCommand( } case 'viewport': { - const size = args[0]; - if (!size || !size.includes('x')) throw new Error('Usage: browse viewport (e.g., 375x812)'); - const [rawW, rawH] = size.split('x').map(Number); - const w = Math.min(Math.max(Math.round(rawW) || 1280, 1), 16384); - const h = Math.min(Math.max(Math.round(rawH) || 720, 1), 16384); + // Parse args: [] [--scale ]. Either may be omitted, but NOT both. + let sizeArg: string | undefined; + let scaleArg: number | undefined; + for (let i = 0; i < args.length; i++) { + if (args[i] === '--scale') { + const val = args[++i]; + if (val === undefined || val === '') { + throw new Error('viewport --scale: missing value. Usage: viewport [WxH] --scale '); + } + const parsed = Number(val); + if (!Number.isFinite(parsed)) { + throw new Error(`viewport --scale: value '${val}' is not a finite number.`); + } + scaleArg = parsed; + } else if (args[i].startsWith('--')) { + throw new Error(`Unknown viewport flag: ${args[i]}`); + } else if (sizeArg === undefined) { + sizeArg = args[i]; + } else { + throw new Error(`Unexpected positional arg: ${args[i]}. Usage: viewport [WxH] [--scale ]`); + } + } + + if (sizeArg === undefined && scaleArg === undefined) { + throw new Error('Usage: browse viewport [] [--scale ] (e.g. 375x812, or --scale 2 to keep current size)'); + } + + // Resolve width/height: either from sizeArg or from current viewport if --scale-only. + let w: number, h: number; + if (sizeArg) { + if (!sizeArg.includes('x')) throw new Error('Usage: browse viewport [] [--scale ] (e.g., 375x812)'); + const [rawW, rawH] = sizeArg.split('x').map(Number); + w = Math.min(Math.max(Math.round(rawW) || 1280, 1), 16384); + h = Math.min(Math.max(Math.round(rawH) || 720, 1), 16384); + } else { + // --scale without WxH → read current size from page + const current = page.viewportSize(); + if (!current) throw new Error('viewport --scale: could not read current viewport size.'); + w = current.width; + h = current.height; + } + + if (scaleArg !== undefined) { + const err = await bm.setDeviceScaleFactor(scaleArg, w, h); + if (err) return `Viewport partially set: ${err}`; + return `Viewport set to ${w}x${h} @ ${scaleArg}x (context recreated; refs and load-html content replayed)`; + } + await bm.setViewport(w, h); return `Viewport set to ${w}x${h}`; } diff --git a/browse/test/security-audit-r2.test.ts b/browse/test/security-audit-r2.test.ts index 985a53ed..97e9f082 100644 --- a/browse/test/security-audit-r2.test.ts +++ b/browse/test/security-audit-r2.test.ts @@ -392,12 +392,13 @@ describe('frame --url ReDoS fix', () => { describe('chain command watch-mode guard', () => { it('chain loop contains isWatching() guard before write dispatch', () => { - const block = sliceBetween(META_SRC, 'for (const cmd of commands)', 'Wait for network to settle'); + // 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'); }); it('chain loop BLOCKED message appears for write commands in watch mode', () => { - const block = sliceBetween(META_SRC, 'for (const cmd of commands)', 'Wait for network to settle'); + 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'); }); });