diff --git a/node_modules b/node_modules new file mode 120000 index 00000000000..d5c09f8e76b --- /dev/null +++ b/node_modules @@ -0,0 +1 @@ +/Users/nwparker/orca/workspaces/orca/bug-basher/node_modules \ No newline at end of file diff --git a/src/main/browser/agent-browser-bridge.test.ts b/src/main/browser/agent-browser-bridge.test.ts index 13de446dc76..ed0ab74dc11 100644 --- a/src/main/browser/agent-browser-bridge.test.ts +++ b/src/main/browser/agent-browser-bridge.test.ts @@ -1512,6 +1512,87 @@ describe('AgentBrowserBridge', () => { expect(chunks).toEqual(['z'.repeat(AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES), 'qq']) }) + // ── Cross-worktree text-injection guard ── + + describe('scoped target for text-mutating commands', () => { + function twoWorktreeBridge(): AgentBrowserBridge { + const tabs = new Map([ + ['tab-a', 1], + ['tab-b', 2] + ]) + const worktrees = new Map([ + ['tab-a', 'wt-1'], + ['tab-b', 'wt-2'] + ]) + const wc1 = mockWebContents(1, 'https://a.com', 'A') + const wc2 = mockWebContents(2, 'https://b.com', 'B') + webContentsFromIdMock.mockImplementation((id: number) => (id === 1 ? wc1 : wc2)) + return new AgentBrowserBridge(mockBrowserManager(tabs, worktrees)) + } + + function sessionNamesUsed(): string[] { + return execFileMock.mock.calls + .filter((call: unknown[]) => (call[1] as string[]).includes('--session')) + .map((call: unknown[]) => { + const args = call[1] as string[] + return args[args.indexOf('--session') + 1] + }) + } + + it.each([ + ['inserttext', (b: AgentBrowserBridge) => b.keyboardInsertText('x', undefined, undefined)], + ['type', (b: AgentBrowserBridge) => b.type('x', undefined, undefined)], + ['fill', (b: AgentBrowserBridge) => b.fill('@input', 'x', undefined, undefined)] + ])( + 'refuses %s when worktrees are ambiguous instead of routing to the global active tab', + async (_name, run) => { + const b = twoWorktreeBridge() + // Why: simulates the user viewing worktree B's tab, which sets the global + // active webContents — the bug would route the agent's text there. + b.onTabChanged(2, 'wt-2') + succeedWith({ inserted: true }) + + await expect(run(b)).rejects.toMatchObject({ + code: 'browser_target_ambiguous' + }) + // Must not have dispatched the command to worktree B's session. + expect(sessionNamesUsed()).not.toContain('orca-tab-tab-b') + } + ) + + it('auto-scopes inserttext to the lone worktree that has a live tab', async () => { + const tabs = new Map([['tab-a', 1]]) + const worktrees = new Map([['tab-a', 'wt-1']]) + const wc1 = mockWebContents(1, 'https://a.com', 'A') + webContentsFromIdMock.mockReturnValue(wc1) + const b = new AgentBrowserBridge(mockBrowserManager(tabs, worktrees)) + succeedWith({ inserted: true }) + + await b.keyboardInsertText('x', undefined, undefined) + + expect(sessionNamesUsed()).toContain('orca-tab-tab-a') + }) + + it('throws browser_no_tab for inserttext when no live tab exists', async () => { + const b = new AgentBrowserBridge(mockBrowserManager(new Map())) + await expect(b.keyboardInsertText('x', undefined, undefined)).rejects.toMatchObject({ + code: 'browser_no_tab' + }) + }) + + it('keeps read-only snapshot on the lenient global active-tab fallback', async () => { + const b = twoWorktreeBridge() + // Why: read/navigation commands intentionally keep the global fallback so + // discovery still works without a worktree; only text writes are guarded. + b.onTabChanged(2, 'wt-2') + succeedWith({ snapshot: 'tree' }) + + await b.snapshot(undefined, undefined) + + expect(sessionNamesUsed()).toContain('orca-tab-tab-b') + }) + }) + // ── Cookie command arg building ── it('builds cookie set args with all options', async () => { diff --git a/src/main/browser/agent-browser-bridge.ts b/src/main/browser/agent-browser-bridge.ts index 60a7019b724..d34b365f6c4 100644 --- a/src/main/browser/agent-browser-bridge.ts +++ b/src/main/browser/agent-browser-bridge.ts @@ -117,6 +117,9 @@ type AgentBrowserExecOptions = { type EnqueueTargetedCommandOptions = { ensureSession?: boolean ensureVisible?: boolean + // Why: text-mutating commands must never fall back to the global active tab, + // which can point at a different worktree the user is currently viewing. + requireScopedTarget?: boolean } type AgentBrowserBridgeOptions = { @@ -764,27 +767,32 @@ export class AgentBrowserBridge { // Agent-browser's fill and click also fail for the same reason. // Workaround: use agent-browser's focus to resolve the ref, then set the value // directly via chunked JS and dispatch input/change events for React/framework compat. - return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => { - await this.execAgentBrowser(sessionName, ['focus', element]) - await this.execAgentBrowser(sessionName, [ - 'eval', - focusedValueSetExpression(JSON.stringify('')) - ]) - for (const chunk of iterateBrowserTextInsertionChunks( - value, - AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES - )) { + return this.enqueueTargetedCommand( + worktreeId, + browserPageId, + async (sessionName) => { + await this.execAgentBrowser(sessionName, ['focus', element]) await this.execAgentBrowser(sessionName, [ 'eval', - focusedValueSetExpression(JSON.stringify(chunk), { append: true }) + focusedValueSetExpression(JSON.stringify('')) ]) - } - await this.execAgentBrowser(sessionName, [ - 'eval', - focusedValueSetExpression(JSON.stringify(''), { append: true, dispatchEvents: true }) - ]) - return { filled: element } as BrowserFillResult - }) + for (const chunk of iterateBrowserTextInsertionChunks( + value, + AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES + )) { + await this.execAgentBrowser(sessionName, [ + 'eval', + focusedValueSetExpression(JSON.stringify(chunk), { append: true }) + ]) + } + await this.execAgentBrowser(sessionName, [ + 'eval', + focusedValueSetExpression(JSON.stringify(''), { append: true, dispatchEvents: true }) + ]) + return { filled: element } as BrowserFillResult + }, + { requireScopedTarget: true } + ) } async type( @@ -793,15 +801,20 @@ export class AgentBrowserBridge { browserPageId?: string ): Promise { await assertClipboardTextWriteWithinLimitWithYield(input) - return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => { - for (const chunk of iterateBrowserTextInsertionChunks( - input, - AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES - )) { - await this.execAgentBrowser(sessionName, ['keyboard', 'type', chunk]) - } - return { typed: true } as BrowserTypeResult - }) + return this.enqueueTargetedCommand( + worktreeId, + browserPageId, + async (sessionName) => { + for (const chunk of iterateBrowserTextInsertionChunks( + input, + AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES + )) { + await this.execAgentBrowser(sessionName, ['keyboard', 'type', chunk]) + } + return { typed: true } as BrowserTypeResult + }, + { requireScopedTarget: true } + ) } async select( @@ -878,16 +891,21 @@ export class AgentBrowserBridge { browserPageId?: string ): Promise { await assertClipboardTextWriteWithinLimitWithYield(text) - return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => { - let result: unknown = { inserted: true } - for (const chunk of iterateBrowserTextInsertionChunks( - text, - AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES - )) { - result = await this.execAgentBrowser(sessionName, ['keyboard', 'inserttext', chunk]) - } - return result - }) + return this.enqueueTargetedCommand( + worktreeId, + browserPageId, + async (sessionName) => { + let result: unknown = { inserted: true } + for (const chunk of iterateBrowserTextInsertionChunks( + text, + AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES + )) { + result = await this.execAgentBrowser(sessionName, ['keyboard', 'inserttext', chunk]) + } + return result + }, + { requireScopedTarget: true } + ) } // ── Mouse commands ── @@ -1841,7 +1859,7 @@ export class AgentBrowserBridge { execute: (sessionName: string, target: ResolvedBrowserCommandTarget) => Promise, options: EnqueueTargetedCommandOptions = {} ): Promise { - const target = this.resolveCommandTarget(worktreeId, browserPageId) + const target = this.resolveCommandTarget(worktreeId, browserPageId, options.requireScopedTarget) const sessionName = `orca-tab-${target.browserPageId}` if (options.ensureSession !== false) { @@ -1962,10 +1980,13 @@ export class AgentBrowserBridge { private resolveCommandTarget( worktreeId?: string, - browserPageId?: string + browserPageId?: string, + requireScopedTarget = false ): ResolvedBrowserCommandTarget { if (!browserPageId) { - return this.resolveActiveTab(worktreeId) + return requireScopedTarget + ? this.resolveScopedActiveTab(worktreeId) + : this.resolveActiveTab(worktreeId) } const tabs = this.getRegisteredTabs(worktreeId) @@ -2039,6 +2060,40 @@ export class AgentBrowserBridge { ) } + // Why: text-mutating commands (inserttext/type/fill) must not silently fall + // back to the global active tab when no worktree was resolved — that tab can + // belong to a worktree the user is currently viewing, so a goal-loop agent in + // another worktree would inject text into the user's foreground webview and + // steal OS focus. A scoped (worktreeId-bearing) call is already safe because + // the candidate set is pre-filtered to that worktree, so defer to the lenient + // resolver. An unscoped call instead requires an unambiguous target: scope to + // the lone worktree with live tabs, or refuse rather than guess. + private resolveScopedActiveTab(worktreeId?: string): ResolvedBrowserCommandTarget { + if (worktreeId) { + return this.resolveActiveTab(worktreeId) + } + + const worktreesWithLiveTabs = new Set() + for (const [tabId, wcId] of this.getRegisteredTabs(undefined)) { + if (this.getWebContents(wcId)) { + worktreesWithLiveTabs.add(this.browserManager.getWorktreeIdForTab(tabId)) + } + } + + if (worktreesWithLiveTabs.size === 0) { + throw new BrowserError('browser_no_tab', 'No browser tab open in this worktree') + } + if (worktreesWithLiveTabs.size > 1) { + throw new BrowserError( + 'browser_target_ambiguous', + 'Multiple worktrees have browser tabs open; pass --worktree to target text insertion safely' + ) + } + + const [onlyWorktreeId] = worktreesWithLiveTabs + return this.resolveActiveTab(onlyWorktreeId) + } + private async ensureSession( sessionName: string, browserPageId: string, diff --git a/src/main/runtime/orca-runtime-browser.test.ts b/src/main/runtime/orca-runtime-browser.test.ts index 9197c1694ab..9a2f611cfaf 100644 --- a/src/main/runtime/orca-runtime-browser.test.ts +++ b/src/main/runtime/orca-runtime-browser.test.ts @@ -505,4 +505,27 @@ describe('RuntimeBrowserCommands headless offscreen routing', () => { }) expect(closeTab).not.toHaveBeenCalled() }) + + it('forwards an unresolved worktree to the bridge unchanged for keyboard inserttext', async () => { + const { RuntimeBrowserCommands } = await import('./orca-runtime-browser') + // Why: when no --worktree is passed (or cwd is outside a managed worktree), + // worktreeId arrives undefined. The bridge — not the runtime — owns the + // cross-worktree guard, so verify the undefined scope is threaded through + // intact rather than silently widened here. + const keyboardInsertText = vi.fn().mockResolvedValue({ inserted: true }) + const bridge = { + getRegisteredTabs: vi.fn(() => new Map([['page-1', 100]])), + keyboardInsertText + } as unknown as AgentBrowserBridge + const commands = new RuntimeBrowserCommands( + createHost({ + getAgentBrowserBridge: () => bridge, + getAuthoritativeWindow: vi.fn(() => ({ webContents: { send: vi.fn() } }) as never) + }) + ) + + await commands.browserKeyboardInsertText({ text: 'hello' }) + + expect(keyboardInsertText).toHaveBeenCalledWith('hello', undefined, undefined) + }) })