diff --git a/src/main/browser/agent-browser/agent-browser-bridge.ts b/src/main/browser/agent-browser/agent-browser-bridge.ts index b746a98d472..521d9132ecd 100644 --- a/src/main/browser/agent-browser/agent-browser-bridge.ts +++ b/src/main/browser/agent-browser/agent-browser-bridge.ts @@ -14,8 +14,8 @@ import type { } from '../../../shared/browser/runtime/runtime-browser-contracts' import { BrowserError } from '../browser-error' import { - parseShellArgs, - stripAgentBrowserTargetArgs + assertAgentBrowserPassthroughStaysOnPage, + parseShellArgs } from './protocol/agent-browser-passthrough-arguments' import { AgentBrowserBridgeInteractionCommands } from './commands/agent-browser-bridge-interaction-commands' import { sendGuestCdpCommand } from '../cdp/debugger/guest-cdp-command' @@ -272,8 +272,8 @@ export class AgentBrowserBridge extends AgentBrowserBridgeInteractionCommands { } async exec(command: string, worktreeId?: string, browserPageId?: string): Promise { - // Passthrough arguments must not override Orca's selected page or CDP proxy. - const args = stripAgentBrowserTargetArgs(parseShellArgs(command.trim())) + const args = parseShellArgs(command.trim()) + assertAgentBrowserPassthroughStaysOnPage(args) return this.enqueueTargetedCommand( worktreeId, browserPageId, diff --git a/src/main/browser/agent-browser/protocol/agent-browser-passthrough-arguments.test.ts b/src/main/browser/agent-browser/protocol/agent-browser-passthrough-arguments.test.ts new file mode 100644 index 00000000000..1b7684b7541 --- /dev/null +++ b/src/main/browser/agent-browser/protocol/agent-browser-passthrough-arguments.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest' +import { assertAgentBrowserPassthroughStaysOnPage } from './agent-browser-passthrough-arguments' + +describe('assertAgentBrowserPassthroughStaysOnPage', () => { + it.each([ + [['connect', '9222']], + [['tab', 'new']], + [['window', 'new']], + [['close']], + [['set', 'viewport', '800', '600']], + [['set', 'device', 'iPhone 15']], + [['click', '@e1', '--new-tab']], + [['get', 'text', '@e3', '--cdp', 'ws://127.0.0.1:9222']], + [['get', 'url', '--session=other']], + [['snapshot', '--auto-connect']], + [['open', 'https://example.com', '-p', 'browserbase']], + [['fill', '@e1', '--headed']], + [['batch', 'get title', 'connect 9222']], + [['batch', '--bail', 'tab new']], + [['batch', 'c\\onnect 9222']], + [['batch', 'batch "tab new"']], + [['batch', 'get url --cdp 9222']] + ])('refuses %j', (args) => { + expect(() => assertAgentBrowserPassthroughStaysOnPage(args)).toThrow( + expect.objectContaining({ code: 'invalid_argument' }) + ) + }) + + it.each([ + [['get', 'text', '@e3']], + [['read']], + [['is', 'visible', '@e3']], + [['set', 'media', 'dark']], + [['wait', '--text', 'Done']], + [['batch', '--bail', 'get title', 'snapshot -i']] + ])('runs %j', (args) => { + expect(() => assertAgentBrowserPassthroughStaysOnPage(args)).not.toThrow() + }) +}) diff --git a/src/main/browser/agent-browser/protocol/agent-browser-passthrough-arguments.ts b/src/main/browser/agent-browser/protocol/agent-browser-passthrough-arguments.ts index 1ba5a18c9cc..e63976c041c 100644 --- a/src/main/browser/agent-browser/protocol/agent-browser-passthrough-arguments.ts +++ b/src/main/browser/agent-browser/protocol/agent-browser-passthrough-arguments.ts @@ -1,3 +1,5 @@ +import { BrowserError } from '../../browser-error' + export function parseShellArgs(input: string): string[] { const args: string[] = [] let current = '' @@ -25,18 +27,177 @@ export function parseShellArgs(input: string): string[] { return args } -export function stripAgentBrowserTargetArgs(args: string[]): string[] { - const stripped: string[] = [] - for (let index = 0; index < args.length; index++) { - const arg = args[index] - if (arg === '--cdp' || arg === '--session') { - index++ - continue +/** Splits a command line the way agent-browser splits each `batch` command (backslash escapes, quotes). */ +export function splitAgentBrowserCommandLine(input: string): string[] { + const args: string[] = [] + let value = '' + let quote: '"' | "'" | null = null + let escaped = false + for (const char of input.trim()) { + if (escaped) { + value += char + escaped = false + } else if (char === '\\' && quote !== "'") { + escaped = true + } else if (quote) { + if (char === quote) { + quote = null + } else { + value += char + } + } else if (char === '"' || char === "'") { + quote = char + } else if (/\s/.test(char)) { + if (value) { + args.push(value) + value = '' + } + } else { + value += char + } + } + if (escaped) { + value += '\\' + } + if (quote) { + throw new BrowserError('invalid_argument', 'Browser command has an unclosed quote.') + } + if (value) { + args.push(value) + } + return args +} + +// agent-browser's global options (0.39 through 0.43). It reads them anywhere in argv, even after +// a command, and they choose the browser, session and launch, which Orca owns. +const GLOBAL_OPTIONS = new Set([ + '--action-policy', + '--allow-file-access', + '--allowed-domains', + '--annotate', + '--args', + '--auto-connect', + '--ca-cert', + '--capture', + '--cdp', + '--cdp-headers', + '--color-scheme', + '--config', + '--confirm-actions', + '--confirm-interactive', + '--content-boundaries', + '--debug', + '--device', + '--download-path', + '--enable', + '--engine', + '--executable-path', + '--extension', + '--fix', + '--headed', + '--headers', + '--hide-scrollbars', + '--idle-timeout', + '--ignore-https-errors', + '--init-script', + '--input-mode', + '--json', + '--max-output', + '--model', + '--namespace', + '--no-auto-connect', + '--no-auto-dialog', + '--no-ca-cert', + '--no-pin-tab', + '--no-webmcp', + '--offline', + '--pin-tab', + '--profile', + '--provider', + '--proxy', + '--proxy-bypass', + '--quick', + '--quiet', + '--relaunch', + '--restore', + '--restore-check-fn', + '--restore-check-text', + '--restore-check-url', + '--restore-save', + '--screenshot-dir', + '--screenshot-format', + '--screenshot-quality', + '--session', + '--session-name', + '--state', + '--use-system-ca', + '--user-agent', + '--verbose', + '--webgpu', + '-p', + '-q', + '-v' +]) + +const TAB_HINT = 'Use the typed tab commands (tab list, tab create, tab switch, tab close).' +const VIEWPORT_HINT = 'Use the typed viewport or set device command.' +const NOT_THE_PAGE = 'It acts outside the selected Orca page.' + +// Commands that leave Orca's page: connect/tab/window/close retarget or create targets +// behind Orca's tab model, and the rest act on the machine or agent-browser itself. +const REFUSED_COMMANDS = new Map([ + ['connect', NOT_THE_PAGE], + ['tab', TAB_HINT], + ['window', TAB_HINT], + ['close', TAB_HINT], + ['quit', TAB_HINT], + ['exit', TAB_HINT], + ['state', NOT_THE_PAGE], + ['session', NOT_THE_PAGE], + ['install', NOT_THE_PAGE], + ['upgrade', NOT_THE_PAGE], + ['doctor', NOT_THE_PAGE], + ['dashboard', NOT_THE_PAGE], + ['profiles', NOT_THE_PAGE], + ['plugin', NOT_THE_PAGE], + ['plugins', NOT_THE_PAGE], + ['mcp', NOT_THE_PAGE], + ['chat', NOT_THE_PAGE] +]) + +function refuse(what: string, hint: string): never { + throw new BrowserError('invalid_argument', `exec does not run ${what}. ${hint}`) +} + +/** Refuses raw agent-browser lines that would move the helper off Orca's selected page. */ +export function assertAgentBrowserPassthroughStaysOnPage(args: readonly string[]): void { + for (const arg of args) { + const option = arg.split('=')[0] + if (GLOBAL_OPTIONS.has(option)) { + refuse(`agent-browser's global option ${option}`, 'Orca sets the browser and session.') + } + } + const [command, subcommand] = args + if (!command) { + return + } + const refusal = REFUSED_COMMANDS.get(command) + if (refusal) { + refuse(`\`${command}\``, refusal) + } + // Both resize the real window around the page (Browser.setContentsSize). + if (command === 'set' && (subcommand === 'viewport' || subcommand === 'device')) { + refuse(`\`set ${subcommand}\``, VIEWPORT_HINT) + } + if (command === 'click' && args.includes('--new-tab')) { + refuse('`click --new-tab`', TAB_HINT) + } + if (command === 'batch') { + // agent-browser runs every operand except `--bail` as its own command line. + for (const step of args.slice(1)) { + if (step !== '--bail') { + assertAgentBrowserPassthroughStaysOnPage(splitAgentBrowserCommandLine(step)) + } } - if (arg.startsWith('--cdp=') || arg.startsWith('--session=')) { - continue - } - stripped.push(arg) } - return stripped } diff --git a/src/main/orcad/browser-providers/chromium/external-chromium-browser-session.test.ts b/src/main/orcad/browser-providers/chromium/external-chromium-browser-session.test.ts index 144fd9edec3..a25b07c0164 100644 --- a/src/main/orcad/browser-providers/chromium/external-chromium-browser-session.test.ts +++ b/src/main/orcad/browser-providers/chromium/external-chromium-browser-session.test.ts @@ -12,7 +12,8 @@ vi.mock('node:fs/promises', () => ({ readFile: vi.fn(async () => Buffer.from('')), realpath: vi.fn(async (path: string) => path), access: vi.fn(async () => undefined), - rm: vi.fn(async () => undefined) + rm: vi.fn(async () => undefined), + writeFile: vi.fn(async () => undefined) })) import { ExternalChromiumBrowserSession } from './external-chromium-browser-session' @@ -41,6 +42,37 @@ describe('orcad external-chromium agent-browser environment', () => { vi.mocked(mkdir).mockClear() }) + it('launches headless with only Orca agent-browser settings', async () => { + runProcessMock.mockResolvedValue({ + code: 0, + signal: null, + stdout: JSON.stringify([{ success: true, result: { url: 'about:blank' } }]), + stderr: '', + timedOut: false + }) + vi.stubEnv('AGENT_BROWSER_HEADED', '1') + vi.stubEnv('agent_browser_extensions', '/ext') + vi.stubEnv('AGENT_BROWSER_SOCKET_DIR', '/tmp/ab-sockets') + try { + await new ExternalChromiumBrowserSession( + '/opt/orca/agent-browser', + { executablePath: BASE.executablePath, provider: 'chromium' }, + '/state' + ).run(['get', 'url']) + } finally { + vi.unstubAllEnvs() + } + const [spec] = runProcessMock.mock.calls[0] + const headed = spec.args?.indexOf('--headed') ?? -1 + expect(spec.args?.slice(headed, headed + 2)).toEqual(['--headed', 'false']) + expect(spec.env).toMatchObject({ + AGENT_BROWSER_SOCKET_DIR: '/tmp/ab-sockets', + AGENT_BROWSER_CONFIG: join('/state', 'agent-browser-config.json') + }) + expect(spec.env).not.toHaveProperty('AGENT_BROWSER_HEADED') + expect(spec.env).not.toHaveProperty('agent_browser_extensions') + }) + it('reports a ref whose element is gone as stale', async () => { runProcessMock.mockResolvedValue({ code: 1, diff --git a/src/main/orcad/browser-providers/chromium/external-chromium-browser-session.ts b/src/main/orcad/browser-providers/chromium/external-chromium-browser-session.ts index c014d87e5a7..6cd40c4a6f5 100644 --- a/src/main/orcad/browser-providers/chromium/external-chromium-browser-session.ts +++ b/src/main/orcad/browser-providers/chromium/external-chromium-browser-session.ts @@ -1,5 +1,5 @@ import { createHash, randomUUID } from 'node:crypto' -import { chmod, mkdir, readFile, rm } from 'node:fs/promises' +import { chmod, mkdir, readFile, rm, writeFile } from 'node:fs/promises' import { join } from 'node:path' import { z } from 'zod' import { BrowserError } from '../../../browser/browser-error' @@ -80,15 +80,26 @@ export function externalChromiumAgentBrowserEnvironment(options: { executablePath: string profilePath: string sessionName: string + configPath: string browserArgs?: readonly string[] }): NodeJS.ProcessEnv { + // Only the socket location is inherited: a user's AGENT_BROWSER_HEADED or _EXTENSIONS would open a + // visible window. Windows env keys ignore case. + const env = Object.fromEntries( + Object.entries(options.inheritedEnv).filter(([key]) => { + const upper = key.toUpperCase() + return !upper.startsWith('AGENT_BROWSER_') || upper === 'AGENT_BROWSER_SOCKET_DIR' + }) + ) return { - ...options.inheritedEnv, + ...env, AGENT_BROWSER_IDLE_TIMEOUT_MS: options.inheritedEnv.AGENT_BROWSER_IDLE_TIMEOUT_MS?.trim() || '0', AGENT_BROWSER_EXECUTABLE_PATH: options.executablePath, AGENT_BROWSER_PROFILE: options.profilePath, AGENT_BROWSER_SESSION: options.sessionName, + // An explicit config skips ~/.agent-browser/config.json and ./agent-browser.json. + AGENT_BROWSER_CONFIG: options.configPath, AGENT_BROWSER_ARGS: options.browserArgs?.join('\n') ?? '' } } @@ -105,6 +116,7 @@ export class ExternalChromiumBrowserSession { private readonly identity: string private readonly sessionName: string private profilePathResolution: Promise | null = null + private configPathWrite: Promise | null = null constructor( private readonly agentBrowserPath: string, @@ -127,6 +139,17 @@ export class ExternalChromiumBrowserSession { return this.profilePathResolution } + // Empty, so agent-browser's defaults (headless, no extensions) apply instead of a user's config. + private configPath(): Promise { + this.configPathWrite ??= (async () => { + const path = join(this.statePath, 'agent-browser-config.json') + await mkdir(this.statePath, { recursive: true, mode: 0o700 }) + await writeFile(path, '{}\n', { mode: 0o600 }) + return path + })() + return this.configPathWrite + } + async start(signal?: AbortSignal): Promise { signal?.throwIfAborted() const profilePath = await this.profilePath() @@ -267,7 +290,10 @@ export class ExternalChromiumBrowserSession { signal?.throwIfAborted() const profilePath = await this.profilePath() signal?.throwIfAborted() - const args = ['--session', this.sessionName, '--profile', profilePath] + const configPath = await this.configPath() + signal?.throwIfAborted() + // Pinned per command so a daemon started headed by an older environment relaunches headless. + const args = ['--session', this.sessionName, '--profile', profilePath, '--headed', 'false'] if (this.launch.browserArgs?.length) { args.push('--args', this.launch.browserArgs.join('\n')) } @@ -277,6 +303,7 @@ export class ExternalChromiumBrowserSession { executablePath: this.launch.executablePath, profilePath, sessionName: this.sessionName, + configPath, browserArgs: this.launch.browserArgs }) const result = await runProcess({ diff --git a/src/main/orcad/browser-providers/chromium/external-chromium-command-arguments.ts b/src/main/orcad/browser-providers/chromium/external-chromium-command-arguments.ts index 8f7fa05b8d7..1fcdad2e183 100644 --- a/src/main/orcad/browser-providers/chromium/external-chromium-command-arguments.ts +++ b/src/main/orcad/browser-providers/chromium/external-chromium-command-arguments.ts @@ -1,69 +1,9 @@ -import { BrowserError } from '../../../browser/browser-error' import { agentBrowserSetMediaArguments } from '../../../browser/agent-browser/protocol/agent-browser-set-media-arguments' import { agentBrowserWaitArguments } from '../../../browser/agent-browser/protocol/agent-browser-wait-arguments' -const TARGET_OVERRIDE_FLAGS: Record = { - '--args': true, - '--cdp': true, - '--executable-path': true, - '--profile': true, - '--session': true -} -const TARGET_OVERRIDE_FLAG_PREFIXES = Object.keys(TARGET_OVERRIDE_FLAGS).map((flag) => `${flag}=`) - -function parseExecArguments(input: string): string[] { - const args: string[] = [] - let value = '' - let quote: '"' | "'" | null = null - let escaped = false - for (const char of input.trim()) { - if (escaped) { - value += char - escaped = false - } else if (char === '\\' && quote !== "'") { - escaped = true - } else if (quote) { - if (char === quote) { - quote = null - } else { - value += char - } - } else if (char === '"' || char === "'") { - quote = char - } else if (/\s/.test(char)) { - if (value) { - args.push(value) - value = '' - } - } else { - value += char - } - } - if (escaped) { - value += '\\' - } - if (quote) { - throw new BrowserError('invalid_argument', 'Browser command has an unclosed quote.') - } - if (value) { - args.push(value) - } - return args -} -function stripTargetOverrideArguments(args: string[]): string[] { - const stripped: string[] = [] - for (let index = 0; index < args.length; index++) { - const arg = args[index] - if (TARGET_OVERRIDE_FLAGS[arg]) { - index++ - continue - } - if (TARGET_OVERRIDE_FLAG_PREFIXES.some((prefix) => arg.startsWith(prefix))) { - continue - } - stripped.push(arg) - } - return stripped -} +import { + assertAgentBrowserPassthroughStaysOnPage, + splitAgentBrowserCommandLine +} from '../../../browser/agent-browser/protocol/agent-browser-passthrough-arguments' export function externalChromiumWaitCommand( params: Record @@ -214,8 +154,11 @@ export function externalChromiumCommandArguments( return ['network', 'har', 'start'] case 'browserCaptureStop': return ['network', 'har', 'stop'] - case 'browserExec': - return stripTargetOverrideArguments(parseExecArguments(text('command'))) + case 'browserExec': { + const args = splitAgentBrowserCommandLine(text('command')) + assertAgentBrowserPassthroughStaysOnPage(args) + return args + } case 'browserSetViewport': if ( params.mobile === true ||