Refuse cross-worktree global fallback for browser text insertion (#5093) (#6563)

Co-authored-by: Orca <help@stably.ai>

node_modules

@@ -0,0 +1 @@
+/Users/nwparker/orca/workspaces/orca/bug-basher/node_modules

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 () => {

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<BrowserTypeResult> {
     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<unknown> {
     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<T>,
     options: EnqueueTargetedCommandOptions = {}
   ): Promise<T> {
-    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<string | undefined>()
+    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,

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)
+  })
 })