From bdb7709fcb5b434bbed68a69fde648da77e05403 Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Fri, 13 Nov 2026 22:26:42 +0000 Subject: [PATCH] fix(browser): keep agent-browser exec on Orca's page and orcad Chromium headless exec passed raw agent-browser argv through after stripping only --cdp and --session, so connect, tab/window, close, set viewport, batch items and global launch options could move the helper to another browser or create targets behind Orca's tab model. Refuse them in one shared gate used by the desktop bridge and orcad's external-Chromium provider. orcad's external-Chromium helper inherited AGENT_BROWSER_* and read the user's ~/.agent-browser/config.json and ./agent-browser.json, so headed or extensions settings opened a visible window. Drop inherited settings, point AGENT_BROWSER_CONFIG at an empty Orca-owned file and pin --headed false. --- .../agent-browser/agent-browser-bridge.ts | 8 +- ...gent-browser-passthrough-arguments.test.ts | 39 ++++ .../agent-browser-passthrough-arguments.ts | 185 ++++++++++++++++-- .../external-chromium-browser-session.test.ts | 34 +++- .../external-chromium-browser-session.ts | 33 +++- .../external-chromium-command-arguments.ts | 75 +------ 6 files changed, 288 insertions(+), 86 deletions(-) create mode 100644 src/main/browser/agent-browser/protocol/agent-browser-passthrough-arguments.test.ts 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 ||