diff --git a/src/main/browser/agent-browser-bridge-execution.ts b/src/main/browser/agent-browser-bridge-execution.ts index a2f11fce47a..31f5a191ede 100644 --- a/src/main/browser/agent-browser-bridge-execution.ts +++ b/src/main/browser/agent-browser-bridge-execution.ts @@ -176,7 +176,6 @@ export abstract class AgentBrowserBridgeExecution extends AgentBrowserBridgeTabs protected closeStaleAgentBrowserSession(sessionName: string): Promise { if ( canSkipAgentBrowserSessionReset({ - platform: process.platform, ownsSocketDirectory: this.ownsAgentBrowserSocketDirectory, socketDirectory: this.agentBrowserEnv.AGENT_BROWSER_SOCKET_DIR, sessionName diff --git a/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts b/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts index 0b028538c8a..2955cb66263 100644 --- a/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts +++ b/src/main/browser/agent-browser-bridge-session-lifecycle.test.ts @@ -81,6 +81,14 @@ function closeCallCount(): number { describe('AgentBrowserBridge', () => { let bridge: AgentBrowserBridge + // The mocked fs has no mkdirSync, so the constructor never claims a socket directory itself. + function ownSocketDirectory(): void { + Object.assign(bridge, { + ownsAgentBrowserSocketDirectory: true, + agentBrowserEnv: { AGENT_BROWSER_SOCKET_DIR: '/tmp/orca-ab-test' } + }) + } + beforeEach(() => { resetAgentBrowserBridgeMocks({ webContentsFromIdMock, @@ -89,35 +97,28 @@ describe('AgentBrowserBridge', () => { stdinWrites, cdpWsProxyInstances: CdpWsProxyMock.instances }) + // Default to a socket that exists so an unprepared test still takes the reset path. + lstatSyncMock.mockReset() + lstatSyncMock.mockReturnValue({}) bridge = new AgentBrowserBridge(mockBrowserManager()) bridge.setActiveTab(100) }) it('snapshots a fresh owned session without launching a helper just to close it', async () => { - vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') - Object.assign(bridge, { - ownsAgentBrowserSocketDirectory: true, - agentBrowserEnv: { AGENT_BROWSER_SOCKET_DIR: '/tmp/orca-ab-test' } - }) + ownSocketDirectory() lstatSyncMock.mockImplementation(() => { throw Object.assign(new Error('No socket'), { code: 'ENOENT' }) }) webContentsFromIdMock.mockReturnValue(mockWebContents(100)) succeedWith({ snapshot: 'ready' }) - try { - expect(await bridge.snapshot()).toMatchObject({ snapshot: 'ready' }) - expect(closeCallCount()).toBe(0) - } finally { - vi.restoreAllMocks() - } + expect(await bridge.snapshot()).toMatchObject({ snapshot: 'ready' }) + expect(closeCallCount()).toBe(0) + expect(lstatSyncMock).toHaveBeenCalledWith('/tmp/orca-ab-test/orca-tab-tab-1.sock') }) it('fails closed when stale agent-browser session ownership cannot be reset', async () => { - Object.assign(bridge, { - ownsAgentBrowserSocketDirectory: true, - agentBrowserEnv: { AGENT_BROWSER_SOCKET_DIR: '/tmp/orca-ab-test' } - }) + ownSocketDirectory() lstatSyncMock.mockReturnValue({}) vi.useFakeTimers() try { diff --git a/src/main/browser/agent-browser-session-reset.test.ts b/src/main/browser/agent-browser-session-reset.test.ts index 974fa348cc9..b38822d5175 100644 --- a/src/main/browser/agent-browser-session-reset.test.ts +++ b/src/main/browser/agent-browser-session-reset.test.ts @@ -6,27 +6,28 @@ vi.mock('node:fs', () => ({ lstatSync })) import { canSkipAgentBrowserSessionReset } from './agent-browser-session-reset' const owned = { - platform: 'darwin' as const, ownsSocketDirectory: true, socketDirectory: '/tmp/orca-ab-profile', sessionName: 'orca-tab-page' } +const socketPath = join(owned.socketDirectory, 'orca-tab-page.sock') beforeEach(() => { lstatSync.mockReset() }) -it.each(['darwin', 'linux'] as const)('skips an absent owned socket on %s', (platform) => { +it('skips an absent owned socket', () => { lstatSync.mockImplementation(() => { throw Object.assign(new Error('No socket'), { code: 'ENOENT' }) }) - expect(canSkipAgentBrowserSessionReset({ ...owned, platform })).toBe(true) - expect(lstatSync).toHaveBeenCalledWith(join(owned.socketDirectory, 'orca-tab-page.sock')) + expect(canSkipAgentBrowserSessionReset(owned)).toBe(true) + expect(lstatSync).toHaveBeenCalledWith(socketPath) }) it('requires reset when a socket or symlink exists', () => { lstatSync.mockReturnValue({}) expect(canSkipAgentBrowserSessionReset(owned)).toBe(false) + expect(lstatSync).toHaveBeenCalledWith(socketPath) }) it.each(['EACCES', 'EIO', 'ENOTDIR'])('requires reset for %s', (code) => { @@ -34,14 +35,15 @@ it.each(['EACCES', 'EIO', 'ENOTDIR'])('requires reset for %s', (code) => { throw Object.assign(new Error('Socket inspection failed'), { code }) }) expect(canSkipAgentBrowserSessionReset(owned)).toBe(false) + expect(lstatSync).toHaveBeenCalledWith(socketPath) }) +// Windows and inherited socket directories both arrive as ownsSocketDirectory: false. it.each([ - { platform: 'win32' as const }, { ownsSocketDirectory: false }, { socketDirectory: undefined }, - { socketDirectory: 'relative' }, { sessionName: '../other' }, + { sessionName: 'has space' }, { sessionName: '' } ])('requires reset without an owned Unix socket address: %j', (override) => { expect(canSkipAgentBrowserSessionReset({ ...owned, ...override })).toBe(false) diff --git a/src/main/browser/agent-browser-session-reset.ts b/src/main/browser/agent-browser-session-reset.ts index a6240c03612..8c6b7545f02 100644 --- a/src/main/browser/agent-browser-session-reset.ts +++ b/src/main/browser/agent-browser-session-reset.ts @@ -1,28 +1,30 @@ import { lstatSync } from 'node:fs' -import { basename, isAbsolute, join } from 'node:path' +import { join } from 'node:path' +// agent-browser's own session-name rule; doubles as a traversal fence for the `join` below. +const SAFE_SESSION_NAME = /^[A-Za-z0-9_-]+$/ + +/** + * True when no daemon can be holding `sessionName`, so closing it would only start one. + * + * Only an Orca-derived socket directory proves that (`ownsSocketDirectory`): it is a + * private per-profile `/tmp` directory, never an inherited one shared with a second + * profile, and never Windows, which uses named pipes and leaves no socket to inspect. + */ export function canSkipAgentBrowserSessionReset(options: { - platform: NodeJS.Platform ownsSocketDirectory: boolean socketDirectory: string | undefined sessionName: string }): boolean { const { socketDirectory, sessionName } = options - if ( - options.platform === 'win32' || - !options.ownsSocketDirectory || - !socketDirectory || - !isAbsolute(socketDirectory) || - !sessionName || - basename(sessionName) !== sessionName - ) { + if (!options.ownsSocketDirectory || !socketDirectory || !SAFE_SESSION_NAME.test(sessionName)) { return false } try { lstatSync(join(socketDirectory, `${sessionName}.sock`)) return false } catch (error) { - // An absent owned socket cannot be reused; permission and other failures prove nothing. + // Only a proven-absent socket is safe to skip; permission and other failures prove nothing. return (error as NodeJS.ErrnoException).code === 'ENOENT' } }