diff --git a/apps/desktop/e2e/browser-tools.spec.ts b/apps/desktop/e2e/browser-tools.spec.ts index 55e8c7021de..d7edc1bf521 100644 --- a/apps/desktop/e2e/browser-tools.spec.ts +++ b/apps/desktop/e2e/browser-tools.spec.ts @@ -21,6 +21,7 @@ const FORM = `Form fixtureUpdates + Other website
Wide content
@@ -40,6 +41,11 @@ test.describe('browser tools', () => { test.beforeAll(async () => { server = createServer(async (request, response) => { const path = new URL(request.url ?? '/', 'http://127.0.0.1').pathname + if (path === '/redirect') { + response.writeHead(302, { Location: `${origin.replace('127.0.0.1', 'localhost')}/landing` }) + response.end() + return + } if (path === '/api/desktop/tool/authorize') { let body = '' for await (const chunk of request) body += chunk.toString() @@ -189,6 +195,28 @@ test.describe('browser tools', () => { expect(await formState()).toMatchObject({ name: '', route: 'change route' }) }) + test('follows a link and cross-origin redirect without a website approval prompt', async () => { + await openForm() + await app.evaluate(async ({ webContents }, url) => { + const page = webContents.getAllWebContents().find((contents) => contents.getURL() === url) + if (!page) throw new Error('Missing browser fixture') + await page.executeJavaScript("document.querySelector('a').click()") + }, `${origin}/form`) + + const destination = `${origin.replace('127.0.0.1', 'localhost')}/landing` + await expect + .poll(() => + app.evaluate( + ({ webContents }, url) => + webContents.getAllWebContents().some((contents) => contents.getURL() === url), + destination + ) + ) + .toBe(true) + expect(await app.evaluate(({ BrowserWindow }) => BrowserWindow.getAllWindows().length)).toBe(1) + await expect(window.getByRole('heading')).toHaveText('Browser tools fixture') + }) + test('stops when a new popup exceeds the page summary limit', async () => { const ref = await openForm() await app.evaluate(async ({ webContents }, origin) => { diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index bd5763cd810..5cb48a1cd3c 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -77,7 +77,7 @@ describe('executeTool', () => { }) it('validates navigation URLs before touching the session', async () => { - const grant = vi.spyOn(session, 'grantSiteOriginForAgentNavigation') + const prepare = vi.spyOn(session, 'prepareExplicitNavigation') const result = await driver.executeTool('chat-test', 'browser_navigate', { url: 'file:///etc/passwd', }) @@ -85,7 +85,7 @@ describe('executeTool', () => { ok: false, error: 'URL must be absolute and start with http:// or https://', }) - expect(grant).not.toHaveBeenCalled() + expect(prepare).not.toHaveBeenCalled() }) it('reports missing required parameters by name', async () => { @@ -94,8 +94,7 @@ describe('executeTool', () => { expect(result.error).toMatch(/Missing required parameter "url"/) }) - it('grants only SSRF-checked agent navigation destinations before loading them', async () => { - const grant = vi.spyOn(session, 'grantSiteOriginForAgentNavigation') + it('loads SSRF-checked agent navigation destinations', async () => { const navigations = [ ['browser_navigate', 'http://127.0.0.1:4011/navigate'], ['browser_open_url', 'http://127.0.0.1:4012/open'], @@ -106,9 +105,9 @@ describe('executeTool', () => { await expect(driver.executeTool('chat-test', tool, { url })).resolves.toMatchObject({ ok: true, }) - expect(grant).toHaveBeenCalledWith(expect.anything(), url) + const contents = session.requireAutomationTab().view.webContents + expect(contents.loadURL).toHaveBeenCalledWith(url) } - expect(grant).toHaveBeenCalledTimes(navigations.length) }) it('keeps the 400ms hydration grace without rediscovering a completed load', async () => { @@ -1140,8 +1139,8 @@ describe('executeTool', () => { expect(respond).toHaveBeenCalledWith('request-1', true) }) - it('routes an exact renderer site decision through the scoped session boundary', async () => { - const respond = vi.spyOn(session, 'respondToSitePermission').mockReturnValue(true) + it('ignores retired site decisions without changing tab ownership', async () => { + const claim = vi.spyOn(session, 'claimActiveTabForUser') await driver.handlePanelAction('chat-test', { action: 'respond-site-permission', @@ -1153,22 +1152,18 @@ describe('executeTool', () => { requestId: 'request-2', }) - expect(respond).toHaveBeenCalledOnce() - expect(respond).toHaveBeenCalledWith('request-1', true) + expect(claim).not.toHaveBeenCalled() }) - it('grants only the exact origin entered through the user omnibox', async () => { + it('loads the exact URL entered through the user omnibox', async () => { await driver.executeTool('chat-test', 'browser_open_tab', {}) const contents = session.requireTab().view.webContents - const grant = vi.spyOn(session, 'grantSiteOriginForUserNavigation') await driver.handlePanelAction('chat-test', { action: 'navigate', url: 'https://docs.example/private?token=secret', }) - expect(grant).toHaveBeenCalledOnce() - expect(grant).toHaveBeenCalledWith(contents, 'https://docs.example/private?token=secret') expect(contents.loadURL).toHaveBeenCalledWith('https://docs.example/private?token=secret') }) diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index 83d06102de3..807aaf81444 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -194,8 +194,6 @@ export interface DriverCallbacks { onPageState: (state: BrowserPageState) => void onTabsState: (state: BrowserTabsState) => void onSessionStatus: (alive: boolean, scopeId: string) => void - /** Whether a live renderer for the scope registered support for the consent prompt. */ - sitePermissionPromptSupported?: (scopeId: string) => boolean /** Whether the active tab shows a login form Sim holds a credential for. */ onFillAvailability: (available: boolean, scopeId: string) => void /** Live native download state for one isolated browser scope. */ @@ -467,7 +465,6 @@ function recordNotice(notice: string): void { function pageStateFor(contents: WebContents, tabId: string): BrowserPageState { const issue = session.pageIssueForContents(contents) const mediaPermissionRequest = session.mediaPermissionRequestForContents(contents) - const sitePermissionRequest = session.sitePermissionRequestForScope() return { scopeId: session.getBrowserScopeId(), tabId, @@ -478,7 +475,6 @@ function pageStateFor(contents: WebContents, tabId: string): BrowserPageState { canGoForward: session.canGoForward(contents), ...(issue ? { issue } : {}), ...(mediaPermissionRequest ? { mediaPermissionRequest } : {}), - ...(sitePermissionRequest ? { sitePermissionRequest } : {}), } } @@ -661,8 +657,6 @@ export function initDriver( void fillCoordinator()?.refreshAvailability(true) }, onPageStateChanged: pushPageState, - sitePermissionPromptSupported: (scopeId) => - driverCallbacks?.sitePermissionPromptSupported?.(scopeId) === true, onTabsChanged: pushTabsState, onTabThemeChanged: (contents, theme) => { void cdp.setColorScheme(contents, theme).catch((error) => { @@ -1373,7 +1367,7 @@ async function loadAgentCheckedUrlAndGetResult( url: string ): Promise> { session.prepareExplicitNavigation(contents) - if (!session.grantSiteOriginForAgentNavigation(contents, url)) { + if (contents.isDestroyed()) { throw new ToolError('The tab was closed before navigation could start.') } const beforeUrl = contents.getURL() @@ -4766,9 +4760,7 @@ export async function handlePanelAction( return } if (action.action === 'respond-site-permission') { - if (typeof action.requestId === 'string' && typeof action.allowed === 'boolean') { - session.respondToSitePermission(action.requestId, action.allowed) - } + /** Older renderers can still send a response to the retired task-navigation prompt. */ return } // Navigate bootstraps the session: the user can open the panel manually @@ -4779,7 +4771,6 @@ export async function handlePanelAction( session.claimActiveTabForUser() const contents = session.ensureTab().view.webContents session.prepareExplicitNavigation(contents) - session.grantSiteOriginForUserNavigation(contents, action.url) void contents.loadURL(action.url).catch(() => {}) } return diff --git a/apps/desktop/src/main/browser-agent/session.test.ts b/apps/desktop/src/main/browser-agent/session.test.ts index 2a0a48e58fd..39e1bc2683b 100644 --- a/apps/desktop/src/main/browser-agent/session.test.ts +++ b/apps/desktop/src/main/browser-agent/session.test.ts @@ -106,7 +106,6 @@ function freshSession( onTabCreated: vi.fn(), onActiveTabChanged: vi.fn(), onPageStateChanged: vi.fn(), - sitePermissionPromptSupported: vi.fn(() => true), onTabsChanged: vi.fn(), onTabThemeChanged: vi.fn(), onTabNavigated: vi.fn(), @@ -841,53 +840,64 @@ describe('browser-agent session', () => { } }) - it('extends an in-flight background restore without restarting its load', async () => { - vi.useFakeTimers() - try { - const tabs = Array.from({ length: 4 }, (_, index) => ({ - url: `https://active-restore-${index}.example/`, - })) - const { persistence } = memoryBrowserPersistence({ - 'chat-active-restore': { v: 1, tabs, activeIndex: 0, downloads: [] }, - }) - const createdContents: MockView['webContents'][] = [] - const selectedLoads: Array<() => void> = [] - session = freshSession( - win, - { - onTabCreated: (webContents) => { - const contents = webContents as unknown as MockView['webContents'] - const index = createdContents.push(contents) - 1 - contents.loadURL.mockImplementation( - () => - new Promise((resolve) => { - if (index === 1) selectedLoads.push(resolve) - }) - ) + it.each(['loaded', 'timed-out'] as const)( + 'gives a late foreground promotion its full loading window (%s)', + async (outcome) => { + vi.useFakeTimers() + try { + const tabs = Array.from({ length: 4 }, (_, index) => ({ + url: `https://active-restore-${index}.example/`, + })) + const { persistence } = memoryBrowserPersistence({ + 'chat-active-restore': { v: 1, tabs, activeIndex: 0, downloads: [] }, + }) + const createdContents: MockView['webContents'][] = [] + const selectedLoads: Array<() => void> = [] + session = freshSession( + win, + { + onTabCreated: (webContents) => { + const contents = webContents as unknown as MockView['webContents'] + const index = createdContents.push(contents) - 1 + contents.loadURL.mockImplementation( + () => + new Promise((resolve) => { + if (index === 1) selectedLoads.push(resolve) + }) + ) + }, }, - }, - persistence - ) - - const selected = session.withBrowserScope('chat-active-restore', () => { - session.restoreBrowserSession() - return session.switchAutomationTab('2') - }) - const selection = session.withBrowserScope('chat-active-restore', () => - session.waitForPendingTabRestore(selected) - ) + persistence + ) - expect(createdContents[1].loadURL).toHaveBeenCalledOnce() - expect(createdContents[1].stop).not.toHaveBeenCalled() - await vi.advanceTimersByTimeAsync(15_000) - expect(createdContents[1].stop).not.toHaveBeenCalled() + session.withBrowserScope('chat-active-restore', () => session.restoreBrowserSession()) + await vi.advanceTimersByTimeAsync(14_000) + const selected = session.withBrowserScope('chat-active-restore', () => + session.switchAutomationTab('2') + ) + const selection = session.withBrowserScope('chat-active-restore', () => + session.waitForPendingTabRestore(selected) + ) - selectedLoads[0]?.() - await expect(selection).resolves.toBe(true) - } finally { - vi.useRealTimers() + expect(createdContents[1].loadURL).toHaveBeenCalledOnce() + expect(createdContents[1].stop).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(19_999) + expect(createdContents[1].stop).not.toHaveBeenCalled() + + if (outcome === 'loaded') { + selectedLoads[0]?.() + await expect(selection).resolves.toBe(true) + } else { + session.withBrowserScope('chat-active-restore', () => session.switchAutomationTab('2')) + await vi.advanceTimersByTimeAsync(1) + await expect(selection).resolves.toBe(false) + expect(createdContents[1].stop).toHaveBeenCalledOnce() + } + } finally { + vi.useRealTimers() + } } - }) + ) it('queues a fifth foreground restore without preempting another foreground restore', async () => { const snapshots = Object.fromEntries( @@ -1031,123 +1041,39 @@ describe('browser-agent session', () => { } }) - it('gives a redirected background restore its complete site-decision window', async () => { + it('does not extend a background restore timeout for cross-origin redirects', async () => { vi.useFakeTimers() try { - const tabs = [ - { url: 'http://127.0.0.1:4601/active' }, - { url: 'http://127.0.0.1:4601/background' }, - ] const { persistence } = memoryBrowserPersistence({ - 'chat-stale-restore-prompt': { v: 1, tabs, activeIndex: 0, downloads: [] }, - }) - const createdContents: MockView['webContents'][] = [] - session = freshSession( - win, - { - onTabCreated: (webContents) => { - const contents = webContents as unknown as MockView['webContents'] - createdContents.push(contents) - contents.loadURL.mockImplementation(() => new Promise(() => {})) - }, + 'chat-test': { + v: 1, + tabs: [ + { url: 'http://127.0.0.1:4601/active' }, + { url: 'http://127.0.0.1:4601/background' }, + ], + activeIndex: 0, + downloads: [], }, - persistence - ) - - session.withBrowserScope('chat-stale-restore-prompt', () => session.restoreBrowserSession()) - session.activateBrowserScope('chat-stale-restore-prompt') - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const background = createdContents[1] - const redirected = beginMainFrameRequest(background, 'http://127.0.0.1:4602/redirect') - await vi.advanceTimersByTimeAsync(0) - expect( - session.withBrowserScope('chat-stale-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toMatchObject({ origin: 'http://127.0.0.1:4602' }) - - await vi.advanceTimersByTimeAsync(15_000) - - expect(background.stop).not.toHaveBeenCalled() - expect( - session.withBrowserScope('chat-stale-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toBeDefined() - - await vi.advanceTimersByTimeAsync(5_000) - - await expect(redirected).resolves.toEqual({ cancel: true }) - expect( - session.withBrowserScope('chat-stale-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toBeUndefined() - expect(background.stop).not.toHaveBeenCalled() - - await vi.advanceTimersByTimeAsync(15_000) - - expect(background.stop).toHaveBeenCalledOnce() - } finally { - vi.useRealTimers() - } - }) - - it('does not let repeated redirect prompts extend a restore without bound', async () => { - vi.useFakeTimers() - try { - const tabs = [ - { url: 'http://127.0.0.1:4611/active' }, - { url: 'http://127.0.0.1:4611/background' }, - ] - const { persistence } = memoryBrowserPersistence({ - 'chat-bounded-restore-prompt': { v: 1, tabs, activeIndex: 0, downloads: [] }, }) - const createdContents: MockView['webContents'][] = [] + const created: MockView['webContents'][] = [] session = freshSession( win, { onTabCreated: (webContents) => { const contents = webContents as unknown as MockView['webContents'] - createdContents.push(contents) + created.push(contents) contents.loadURL.mockImplementation(() => new Promise(() => {})) }, }, persistence ) - - session.withBrowserScope('chat-bounded-restore-prompt', () => session.restoreBrowserSession()) - session.activateBrowserScope('chat-bounded-restore-prompt') - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const background = createdContents[1] - const firstRedirect = beginMainFrameRequest(background, 'http://127.0.0.1:4612/first') - await vi.advanceTimersByTimeAsync(0) - expect( - session.withBrowserScope('chat-bounded-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toMatchObject({ origin: 'http://127.0.0.1:4612' }) - - await vi.advanceTimersByTimeAsync(20_000) - await expect(firstRedirect).resolves.toEqual({ cancel: true }) - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const secondRedirect = beginMainFrameRequest(background, 'http://127.0.0.1:4613/second', 2) - await vi.advanceTimersByTimeAsync(0) - expect( - session.withBrowserScope('chat-bounded-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toMatchObject({ origin: 'http://127.0.0.1:4613' }) - + session.restoreBrowserSession() + const background = created[1] + await expect( + beginMainFrameRequest(background, 'http://127.0.0.1:4602/login') + ).resolves.toEqual({ cancel: false }) await vi.advanceTimersByTimeAsync(15_000) - expect(background.stop).toHaveBeenCalledOnce() - await expect(secondRedirect).resolves.toEqual({ cancel: true }) - expect( - session.withBrowserScope('chat-bounded-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toBeUndefined() } finally { vi.useRealTimers() } @@ -2789,7 +2715,7 @@ describe('browser-agent session', () => { expect(onTabCreated).toHaveBeenLastCalledWith(userTab?.view.webContents) }) - it('does not treat an untrusted page popup as user authorization for its origin', async () => { + it('lets internal page popups navigate after the network check', async () => { panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) const source = (session.ensureTab().view as unknown as MockView).webContents const openWindow = source.setWindowOpenHandler.mock.calls[0]?.[0] as (details: { @@ -2801,13 +2727,8 @@ describe('browser-agent session', () => { const popup = (session.activeTab()?.view as unknown as MockView).webContents const request = beginMainFrameRequest(popup, destination) - await vi.waitFor(() => - expect(session.sitePermissionRequestForScope()).toMatchObject({ - origin: 'http://127.0.0.1:4099', - }) - ) - session.respondToSitePermission(session.sitePermissionRequestForScope()?.requestId ?? '', false) - await expect(request).resolves.toEqual({ cancel: true }) + await expect(request).resolves.toEqual({ cancel: false }) + expect(dialog.showMessageBox).not.toHaveBeenCalled() }) it('blocks controlled pages from moving or resizing the desktop window', () => { @@ -3095,210 +3016,50 @@ describe('browser-agent session', () => { } }) - it('holds a new top-level origin for an exact task-scoped user decision', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - const first = beginMainFrameRequest( - contents, - 'http://127.0.0.1:4101/private?token=secret#fragment' - ) - - await vi.waitFor(() => { - expect(session.sitePermissionRequestForScope()).toMatchObject({ - tabId: '1', - origin: 'http://127.0.0.1:4101', - }) - }) - const prompt = session.sitePermissionRequestForScope() - expect(prompt).not.toHaveProperty('url') - expect(win.focus).toHaveBeenCalled() - expect(win.webContents.focus).toHaveBeenCalled() - expect(session.respondToSitePermission(prompt?.requestId ?? '', true)).toBe(true) - await expect(first).resolves.toEqual({ cancel: false }) - - await expect( - beginMainFrameRequest(contents, 'http://127.0.0.1:4101/another?different=secret', 2) - ).resolves.toEqual({ cancel: false }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - - const otherOrigin = beginMainFrameRequest(contents, 'http://127.0.0.1:4102/', 3) - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - expect(session.respondToSitePermission('not-the-live-request', true)).toBe(false) - const otherPrompt = session.sitePermissionRequestForScope() - expect(session.respondToSitePermission(otherPrompt?.requestId ?? '', false)).toBe(true) - await expect(otherOrigin).resolves.toEqual({ cancel: true }) - }) - - it('allows an SSRF-checked agent destination without granting a cross-origin redirect', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) + it('allows public and loopback cross-origin navigation without a task prompt', async () => { const contents = (session.ensureTab().view as unknown as MockView).webContents - const destination = 'http://127.0.0.1:4111/agent-path?token=secret' - - expect( - session.grantSiteOriginForAgentNavigation(contents as unknown as WebContents, destination) - ).toBe(true) - await expect(beginMainFrameRequest(contents, destination)).resolves.toEqual({ cancel: false }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - - const redirect = beginMainFrameRequest(contents, 'http://127.0.0.1:4112/redirected', 2) - await vi.waitFor(() => - expect(session.sitePermissionRequestForScope()).toMatchObject({ - origin: 'http://127.0.0.1:4112', - }) - ) - const prompt = session.sitePermissionRequestForScope() - expect(session.respondToSitePermission(prompt?.requestId ?? '', false)).toBe(true) - await expect(redirect).resolves.toEqual({ cancel: true }) - }) - - it('uses a native exact-origin prompt when the active renderer lacks prompt support', async () => { - session = freshSession(win, { - sitePermissionPromptSupported: vi.fn(() => false), - }) - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ - response: 1, - checkboxChecked: false, - }) - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - - const request = beginMainFrameRequest( - contents, - 'http://127.0.0.1:4151/private?token=secret#fragment' - ) - - await expect(request).resolves.toEqual({ cancel: false }) - expect(dialog.showMessageBox).toHaveBeenCalledWith( - win, - expect.objectContaining({ - buttons: ['Block', 'Allow'], - defaultId: 0, - cancelId: 0, - message: 'Allow this browser task to open http://127.0.0.1:4151?', - }) - ) - expect(JSON.stringify(vi.mocked(dialog.showMessageBox).mock.lastCall)).not.toContain('secret') - expect(session.sitePermissionRequestForScope()).toBeUndefined() - }) - - it('attaches the native fallback to the window that owns the visible panel', async () => { - const panelOwner = mainWindowMock() - session = freshSession(win, { - sitePermissionPromptSupported: vi.fn(() => false), - }) - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ - response: 0, - checkboxChecked: false, - }) - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }, panelOwner) - const contents = (session.ensureTab().view as unknown as MockView).webContents - - await expect(beginMainFrameRequest(contents, 'http://127.0.0.1:4155/private')).resolves.toEqual( - { cancel: true } - ) - - expect(dialog.showMessageBox).toHaveBeenCalledWith(panelOwner, expect.any(Object)) - }) - - it('denies a new site prompt immediately when its scope is hidden or inactive', async () => { - const hiddenContents = (session.ensureTab().view as unknown as MockView).webContents - - await expect( - beginMainFrameRequest(hiddenContents, 'http://127.0.0.1:4156/hidden') - ).resolves.toEqual({ cancel: true }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const inactiveContents = session.withBrowserScope( - 'chat-inactive', - () => session.ensureTab().view as unknown as MockView - ).webContents - await expect( - beginMainFrameRequest(inactiveContents, 'http://127.0.0.1:4157/inactive') - ).resolves.toEqual({ cancel: true }) - expect( - session.withBrowserScope('chat-inactive', () => session.sitePermissionRequestForScope()) - ).toBeUndefined() - }) - - it('does not show the native fallback when the active renderer owns the prompt', async () => { - vi.mocked(dialog.showMessageBox).mockClear() - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - const request = beginMainFrameRequest(contents, 'http://127.0.0.1:4152/docs') - - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - + for (const url of [ + 'https://public.example/', + 'https://redirect.example/login', + 'http://127.0.0.1:4101/', + 'http://localhost:4102/', + ]) { + await expect(beginMainFrameRequest(contents, url)).resolves.toEqual({ cancel: false }) + } expect(dialog.showMessageBox).not.toHaveBeenCalled() - const prompt = session.sitePermissionRequestForScope() - expect(session.respondToSitePermission(prompt?.requestId ?? '', false)).toBe(true) - await expect(request).resolves.toEqual({ cancel: true }) }) - it('revalidates a native allow decision after the held request becomes stale', async () => { - session = freshSession(win, { - sitePermissionPromptSupported: vi.fn(() => false), - }) - vi.mocked(dialog.showMessageBox).mockClear() - let answerPrompt: ((result: { response: number; checkboxChecked: boolean }) => void) | undefined - vi.mocked(dialog.showMessageBox).mockImplementationOnce( - () => - new Promise((resolve) => { - answerPrompt = resolve + it.each(['mainFrame', 'subFrame'])( + 'retains private-network checks for %s navigation', + async (resourceType) => { + const contents = (session.ensureTab().view as unknown as MockView).webContents + for (const url of ['http://169.254.169.254/', 'http://10.0.0.1/', 'file:///tmp/example']) { + await expect(beginSubresourceRequest(contents, url, resourceType)).resolves.toEqual({ + cancel: true, }) - ) - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - const request = beginMainFrameRequest(contents, 'http://127.0.0.1:4153/held') - await vi.waitFor(() => expect(dialog.showMessageBox).toHaveBeenCalled()) - const signal = vi.mocked(dialog.showMessageBox).mock.lastCall?.at(-1)?.signal - expect(signal?.aborted).toBe(false) - - mainFrameNavigationStarted(contents, false, 'http://127.0.0.1:4154/replacement') - await expect(request).resolves.toEqual({ cancel: true }) - expect(signal?.aborted).toBe(true) - answerPrompt?.({ response: 1, checkboxChecked: false }) - - const retried = beginMainFrameRequest(contents, 'http://127.0.0.1:4153/retried', 2) - await expect(retried).resolves.toEqual({ cancel: true }) - expect(dialog.showMessageBox).toHaveBeenCalledTimes(2) - }) - - it('keeps the held request alive through its own navigation-start event', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - const destination = 'http://127.0.0.1:4201/docs' - const request = beginMainFrameRequest(contents, destination) - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - - mainFrameNavigationStarted(contents, false, `${destination}#section`) - const prompt = session.sitePermissionRequestForScope() - expect(prompt).toBeDefined() - expect(session.respondToSitePermission(prompt?.requestId ?? '', true)).toBe(true) - await expect(request).resolves.toEqual({ cancel: false }) - - const replaced = beginMainFrameRequest(contents, 'http://127.0.0.1:4202/', 2) - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - mainFrameNavigationStarted(contents, false, 'http://127.0.0.1:4203/') - await expect(replaced).resolves.toEqual({ cancel: true }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - }) + } + mockLookup.mockResolvedValue([{ address: '192.168.0.1', family: 4 }]) + await expect( + beginSubresourceRequest(contents, 'https://private-redirect.example/', resourceType) + ).resolves.toEqual({ cancel: true }) + mockLookup.mockRejectedValue(new Error('DNS unavailable')) + await expect( + beginSubresourceRequest(contents, 'https://unresolved-redirect.example/', resourceType) + ).resolves.toEqual({ cancel: true }) + } + ) - it('invalidates a held site decision before an explicit replacement navigation', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) + it('allows checked navigation in hidden and inactive tasks without a prompt', async () => { const contents = (session.ensureTab().view as unknown as MockView).webContents - const held = beginMainFrameRequest(contents, 'http://127.0.0.1:4204/held') - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - const requestId = session.sitePermissionRequestForScope()?.requestId - - session.prepareExplicitNavigation(contents as unknown as WebContents) - - await expect(held).resolves.toEqual({ cancel: true }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - expect(session.respondToSitePermission(requestId ?? '', true)).toBe(false) + panel.setPanelBounds(null) + session.activateBrowserScope('another-task') + await expect(beginMainFrameRequest(contents, 'http://127.0.0.1:4201/')).resolves.toEqual({ + cancel: false, + }) + expect(dialog.showMessageBox).not.toHaveBeenCalled() }) - it('seeds restored origins before loading while still holding a new redirect origin', async () => { + it('allows restored pages and their checked cross-origin redirects', async () => { const restoredUrl = 'http://127.0.0.1:4301/restored?private=value' const { persistence } = memoryBrowserPersistence({ 'chat-test': { @@ -3315,56 +3076,9 @@ describe('browser-agent session', () => { expect(contents.loadURL).toHaveBeenCalledWith(restoredUrl) await expect(beginMainFrameRequest(contents, restoredUrl)).resolves.toEqual({ cancel: false }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() const redirected = beginMainFrameRequest(contents, 'http://127.0.0.1:4302/login', 2) - await vi.waitFor(() => - expect(session.sitePermissionRequestForScope()).toMatchObject({ - origin: 'http://127.0.0.1:4302', - }) - ) - const prompt = session.sitePermissionRequestForScope() - session.respondToSitePermission(prompt?.requestId ?? '', false) - await expect(redirected).resolves.toEqual({ cancel: true }) - }) - - it('bounds task grants and fails closed when a main-frame request cannot map to a live tab', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - for (let index = 0; index <= 64; index += 1) { - expect( - session.grantSiteOriginForUserNavigation( - contents as unknown as WebContents, - `http://127.0.0.1:${4400 + index}/private` - ) - ).toBe(true) - } - - const evicted = beginMainFrameRequest(contents, 'http://127.0.0.1:4400/again') - await vi.waitFor(() => - expect(session.sitePermissionRequestForScope()).toMatchObject({ - origin: 'http://127.0.0.1:4400', - }) - ) - session.respondToSitePermission(session.sitePermissionRequestForScope()?.requestId ?? '', false) - await expect(evicted).resolves.toEqual({ cancel: true }) - - const handler = contents.session.webRequest.onBeforeRequest.mock.calls[0]?.[0] - const unmapped = new Promise<{ cancel: boolean }>((resolve) => { - handler( - { - id: 99, - url: 'http://127.0.0.1:4499/', - method: 'GET', - resourceType: 'mainFrame', - referrer: '', - timestamp: Date.now(), - uploadData: [], - }, - resolve - ) - }) - await expect(unmapped).resolves.toEqual({ cancel: true }) + await expect(redirected).resolves.toEqual({ cancel: false }) }) it('blocks an image hostname that resolves to a private address', async () => { @@ -3380,34 +3094,6 @@ describe('browser-agent session', () => { }) }) - it('default-denies pending site requests on timeout, tab close, and stale-document approval', async () => { - vi.useFakeTimers() - try { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const tab = session.ensureTab() - const contents = (tab.view as unknown as MockView).webContents - const timedOut = beginMainFrameRequest(contents, 'http://127.0.0.1:4501/') - await vi.advanceTimersByTimeAsync(0) - await vi.advanceTimersByTimeAsync(20_000) - await expect(timedOut).resolves.toEqual({ cancel: true }) - - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const stale = beginMainFrameRequest(contents, 'http://127.0.0.1:4502/', 2) - await vi.advanceTimersByTimeAsync(0) - const stalePrompt = session.sitePermissionRequestForScope() - contents.getURL.mockReturnValue('https://changed.example/') - expect(session.respondToSitePermission(stalePrompt?.requestId ?? '', true)).toBe(true) - await expect(stale).resolves.toEqual({ cancel: true }) - - const closing = beginMainFrameRequest(contents, 'http://127.0.0.1:4503/', 3) - await vi.advanceTimersByTimeAsync(0) - session.closeTab(tab.id) - await expect(closing).resolves.toEqual({ cancel: true }) - } finally { - vi.useRealTimers() - } - }) - it('leaves nothing of the signed-out user behind in the browser profile', async () => { const clearStorageData = vi.fn(async () => {}) const clearCache = vi.fn(async () => {}) diff --git a/apps/desktop/src/main/browser-agent/session.ts b/apps/desktop/src/main/browser-agent/session.ts index 9538bff0b81..e183137c9a0 100644 --- a/apps/desktop/src/main/browser-agent/session.ts +++ b/apps/desktop/src/main/browser-agent/session.ts @@ -10,7 +10,6 @@ import type { BrowserMediaPermissionRequest, BrowserOmniboxFocusMode, BrowserPageIssue, - BrowserSitePermissionRequest, BrowserTabState, BrowserTabsState, BrowserTheme, @@ -70,7 +69,6 @@ import { } from '@/main/browser-agent/url-guard' import { browserUserAgent } from '@/main/browser-agent/user-agent' import type { BrowserSessionSnapshot } from '@/main/desktop-chat-session-store' -import { showShellDialog } from '@/main/dialogs' import { suggestedFilename, uniqueDownloadPath } from '@/main/downloads' import { type FocusedResourceShortcut, @@ -98,7 +96,6 @@ export interface AgentTab { recoveringUnresponsive?: boolean pendingMediaPermission?: PendingMediaPermission mediaPermissionGrant?: MediaPermissionGrant - pendingSitePermission?: PendingSitePermission lastRealUserGestureAt?: number } @@ -114,19 +111,6 @@ interface MediaPermissionGrant { devices: Set } -interface PendingSitePermission { - request: BrowserSitePermissionRequest - /** Exact committed document from which the suspended request originated. */ - documentUrl: string - /** Exact destination retained only in main-process memory for receipt validation. */ - destinationUrl: string - contents: WebContents - networkRequestId: number - resolve: (allowed: boolean) => void - timeout: ReturnType - nativePromptController?: AbortController -} - export interface BrowserSessionPersistence { load: (scopeId: string) => BrowserSessionSnapshot | null save: (scopeId: string, snapshot: BrowserSessionSnapshot) => boolean @@ -161,8 +145,6 @@ export interface AgentSessionEvents { onActiveTabChanged: (contents: WebContents) => void /** The active tab's recoverable page state changed without a navigation. */ onPageStateChanged: (contents: WebContents) => void - /** Whether the current app renderer can present and answer a site-origin prompt. */ - sitePermissionPromptSupported: (scopeId: string) => boolean /** The tab list or active tab changed. */ onTabsChanged: () => void /** Sim's appearance preference changed for an existing tab. */ @@ -200,8 +182,6 @@ const BACKGROUND_TAB_RESTORE_TIMEOUT_MS = 15_000 const FOREGROUND_TAB_RESTORE_TIMEOUT_MS = 20_000 const MEDIA_PERMISSION_GESTURE_WINDOW_MS = 10_000 const MEDIA_PERMISSION_PROMPT_TIMEOUT_MS = 30_000 -const SITE_PERMISSION_PROMPT_TIMEOUT_MS = 20_000 -const MAX_SITE_ORIGIN_GRANTS_PER_SCOPE = 64 export type BrowserShortcut = 'focus-omnibox' | 'new-tab' | 'close-tab' | 'find' @@ -269,8 +249,6 @@ interface BrowserScopeState { */ findingTabId: string | null findingRequestId: number | null - /** Memory-bounded, task-local origins explicitly reached or approved by the user. */ - siteOriginGrants: Map } function createBrowserScopeState(): BrowserScopeState { @@ -291,7 +269,6 @@ function createBrowserScopeState(): BrowserScopeState { automationNeedsAttention: false, findingTabId: null, findingRequestId: null, - siteOriginGrants: new Map(), } } @@ -444,7 +421,6 @@ interface PendingTabRestore { settled: boolean requeueAfterPreemption: boolean cancelLoad?: () => void - grantSitePermissionGrace?: () => void promoteToForeground?: () => void } @@ -1229,11 +1205,6 @@ function mediaOrigin(candidate: unknown): string | null { } } -function withoutUrlFragment(url: string): string { - const fragmentIndex = url.indexOf('#') - return fragmentIndex < 0 ? url : url.slice(0, fragmentIndex) -} - function requestedMediaDevices(candidate: unknown): BrowserMediaDevice[] | null { if (!Array.isArray(candidate) || candidate.length === 0) return null const devices = new Set() @@ -1348,211 +1319,6 @@ export async function respondToMediaPermission(requestId: string, allowed: boole publishPageIssue(tab) } -function grantSiteOrigin(state: BrowserScopeState, origin: string): void { - state.siteOriginGrants.delete(origin) - state.siteOriginGrants.set(origin, true) - while (state.siteOriginGrants.size > MAX_SITE_ORIGIN_GRANTS_PER_SCOPE) { - const oldest = state.siteOriginGrants.keys().next().value - if (typeof oldest !== 'string') break - state.siteOriginGrants.delete(oldest) - } -} - -function hasSiteOriginGrant(state: BrowserScopeState, origin: string): boolean { - if (!state.siteOriginGrants.has(origin)) return false - grantSiteOrigin(state, origin) - return true -} - -function publishSitePermissionState(scopeId: string): void { - const resolved = resolveBrowserScopeId(scopeId) - const state = browserScopeStates.get(resolved) - if (!state) return - const active = state.tabs.find((tab) => tab.id === state.activeTabId) - if (active && !active.view.webContents.isDestroyed()) { - withBrowserScope(resolved, () => events?.onPageStateChanged(active.view.webContents)) - } -} - -function settleSitePermission(tab: AgentTab, allowed: boolean, publish = true): boolean { - const pending = tab.pendingSitePermission - if (!pending) return false - tab.pendingSitePermission = undefined - clearTimeout(pending.timeout) - pending.nativePromptController?.abort() - pending.resolve(allowed) - if (publish) publishSitePermissionState(tab.scopeId) - return true -} - -function scopedTabForRequest(details: { - webContents?: WebContents - webContentsId?: number -}): { scopeId: string; tab: AgentTab } | null { - if (details.webContents) return scopedTabForContents(details.webContents) - if (typeof details.webContentsId !== 'number') return null - for (const [scopeId, state] of browserScopeStates) { - const tab = state.tabs.find( - (candidate) => candidate.view.webContents.id === details.webContentsId - ) - if (tab) return { scopeId, tab } - } - return null -} - -/** Highest-priority exact site request: visible tab, automation tab, then task tab order. */ -export function sitePermissionRequestForScope(): BrowserSitePermissionRequest | undefined { - const state = browserScopeState() - const active = state.tabs.find((tab) => tab.id === state.activeTabId)?.pendingSitePermission - if (active) return active.request - const automation = state.tabs.find( - (tab) => tab.id === state.automationTabId - )?.pendingSitePermission - if (automation) return automation.request - return state.tabs.find((tab) => tab.pendingSitePermission)?.pendingSitePermission?.request -} - -function grantSiteOriginForExplicitNavigation(contents: WebContents, destination: string): boolean { - const scoped = scopedTabForContents(contents) - const origin = mediaOrigin(destination) - if (!scoped || !origin) return false - const state = browserScopeStates.get(scoped.scopeId) - if (!state || scoped.tab.view.webContents !== contents || contents.isDestroyed()) return false - grantSiteOrigin(state, origin) - return true -} - -/** Grants only the destination origin entered through a native-activation-gated user action. */ -export function grantSiteOriginForUserNavigation( - contents: WebContents, - destination: string -): boolean { - return grantSiteOriginForExplicitNavigation(contents, destination) -} - -/** Grants the exact destination origin after the browser driver has completed its SSRF check. */ -export function grantSiteOriginForAgentNavigation( - contents: WebContents, - destination: string -): boolean { - return grantSiteOriginForExplicitNavigation(contents, destination) -} - -/** Applies a response only to the exact live task, tab, document, and suspended network request. */ -export function respondToSitePermission(requestId: string, allowed: boolean): boolean { - const scopeId = getBrowserScopeId() - const state = browserScopeStates.get(scopeId) - const tab = state?.tabs.find( - (candidate) => candidate.pendingSitePermission?.request.requestId === requestId - ) - const pending = tab?.pendingSitePermission - if (!state || !tab || !pending) return false - - if (!allowed) return settleSitePermission(tab, false) - - const contents = tab.view.webContents - const live = - !contents.isDestroyed() && - pending.contents === contents && - pending.request.tabId === tab.id && - pending.documentUrl === contents.getURL() && - mediaOrigin(pending.destinationUrl) === pending.request.origin && - scopeId === resolveBrowserScopeId(tab.scopeId) && - scopeId === getActiveBrowserScopeId() && - isPanelVisible() - if (!live) return settleSitePermission(tab, false) - - grantSiteOrigin(state, pending.request.origin) - return settleSitePermission(tab, true) -} - -async function requestSitePermission(details: { - id: number - url: string - webContents?: WebContents - webContentsId?: number -}): Promise { - const origin = mediaOrigin(details.url) - const scoped = scopedTabForRequest(details) - if (!origin || !scoped || suspendedBrowserScopes.has(scoped.scopeId)) return false - const state = browserScopeStates.get(scoped.scopeId) - const contents = scoped.tab.view.webContents - if (!state || contents.isDestroyed()) return false - - if (mediaOrigin(contents.getURL()) === origin || hasSiteOriginGrant(state, origin)) return true - if (scoped.scopeId !== getActiveBrowserScopeId() || !isPanelVisible()) return false - const win = panelWindow() - if (!win || win.isDestroyed()) return false - - settleSitePermission(scoped.tab, false, false) - revokeTabMediaPermissions(scoped.tab, false) - const request: BrowserSitePermissionRequest = { - requestId: generateId(), - tabId: scoped.tab.id, - origin, - } - const allowed = new Promise((resolve) => { - scoped.tab.pendingSitePermission = { - request, - documentUrl: contents.getURL(), - destinationUrl: details.url, - contents, - networkRequestId: details.id, - resolve, - timeout: setTimeout( - bindToBrowserScope(scoped.scopeId, () => { - const pending = scoped.tab.pendingSitePermission - if ( - pending?.request.requestId !== request.requestId || - pending.networkRequestId !== details.id - ) { - return - } - settleSitePermission(scoped.tab, false) - }), - SITE_PERMISSION_PROMPT_TIMEOUT_MS - ), - } - }) - scoped.tab.pendingRestore?.grantSitePermissionGrace?.() - if (events?.sitePermissionPromptSupported(scoped.scopeId)) { - win.focus() - win.webContents.focus() - publishSitePermissionState(scoped.scopeId) - } else { - const nativePromptController = new AbortController() - const pending = scoped.tab.pendingSitePermission - if (!pending || pending.request.requestId !== request.requestId) return await allowed - pending.nativePromptController = nativePromptController - void showShellDialog(win, { - type: 'warning', - buttons: ['Block', 'Allow'], - defaultId: 0, - cancelId: 0, - noLink: true, - signal: nativePromptController.signal, - message: `Allow this browser task to open ${request.origin}?`, - detail: 'Only allow this site if it is expected for the current task.', - }) - .then(({ response }) => { - withBrowserScope(scoped.scopeId, () => { - respondToSitePermission(request.requestId, response === 1) - }) - }) - .catch((error) => { - if (!nativePromptController.signal.aborted) { - logger.warn('Could not present the native site permission prompt', { - error: getErrorMessage(error), - }) - } - withBrowserScope(scoped.scopeId, () => { - respondToSitePermission(request.requestId, false) - }) - }) - } - return await allowed -} - /** * Default-deny hardening for the agent partition. Site permissions remain * denied apart from ALLOWED_SITE_PERMISSIONS. Media is granted only after a @@ -1677,25 +1443,7 @@ function configureAgentPartition(ses: Session): void { logger.warn('Could not answer an agent request', { error: getErrorMessage(error) }) } } - if (details.resourceType === 'mainFrame') { - void checkAgentUrl(details.url) - .then(async (guard) => { - if (!guard.ok) { - logger.warn('Blocked agent document navigation to a private host') - settle(true) - return - } - settle(!(await requestSitePermission(details))) - }) - .catch((error) => { - // Fail closed: an unexpected rejection must cancel, never leave the - // request suspended with no callback. - logger.error('Agent SSRF check failed; cancelling request', { error }) - settle(true) - }) - return - } - if (details.resourceType === 'subFrame') { + if (details.resourceType === 'mainFrame' || details.resourceType === 'subFrame') { void checkAgentUrl(details.url) .then((guard) => { if (!guard.ok) logger.warn('Blocked agent document navigation to a private host') @@ -2168,14 +1916,10 @@ export function stopFindInActiveTab(focusPage: boolean): void { * inside the browser resource rather than spawn a native window, and both are * reached from an untrusted page, so the scheme is checked here once. */ -function openTabWithUrl( - url: string, - { agentOwned, userAuthorized }: { agentOwned: boolean; userAuthorized: boolean } -): void { +function openTabWithUrl(url: string, { agentOwned }: { agentOwned: boolean }): void { if (!/^https?:\/\//i.test(url)) return try { const tab = agentOwned ? addAutomationTab() : addTab() - if (userAuthorized) grantSiteOriginForUserNavigation(tab.view.webContents, url) void tab.view.webContents.loadURL(url).catch(() => {}) } catch (error) { logger.warn('Could not open a link in a new browser tab', { @@ -2227,10 +1971,7 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV contents.setUserAgent(browserUserAgent()) attachAgentContextMenu(contents, { addToChat: (text) => withBrowserScope(scopeId, () => addPageSelectionToChat(contents, text)), - openTab: (url) => - withBrowserScope(scopeId, () => - openTabWithUrl(url, { agentOwned: false, userAuthorized: true }) - ), + openTab: (url) => withBrowserScope(scopeId, () => openTabWithUrl(url, { agentOwned: false })), defaultZoomFactor: getBrowserDefaultZoomFactor, }) @@ -2290,7 +2031,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV withBrowserScope(scopeId, () => openTabWithUrl(details.url, { agentOwned: agentOwnsPopupFrom(contents), - userAuthorized: false, }) ) return { action: 'deny' } @@ -2319,7 +2059,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV return } dismissFind(tab.id) - settleSitePermission(tab, false) revokeTabMediaPermissions(tab, false) tab.pageIssue = { kind: 'crashed', @@ -2337,7 +2076,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV const tab = tabs.find((entry) => entry.view === view) if (!tab || tab.pageIssue?.kind === 'crashed') return dismissFind(tab.id) - settleSitePermission(tab, false) revokeTabMediaPermissions(tab, false) tab.pageIssue = { kind: 'unresponsive', @@ -2444,13 +2182,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV if (!details.isMainFrame) return const tab = tabs.find((entry) => entry.view === view) if (tab) { - if ( - tab.pendingSitePermission && - withoutUrlFragment(tab.pendingSitePermission.destinationUrl) !== - withoutUrlFragment(details.url) - ) { - settleSitePermission(tab, false) - } revokeTabMediaPermissions(tab) } notePageNavigationStarted(contents) @@ -2472,7 +2203,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV bindToBrowserScope(scopeId, () => { const tab = tabs.find((entry) => entry.view === view) if (tab) { - settleSitePermission(tab, false) revokeTabMediaPermissions(tab, false) } events?.onTabClosed(contents) @@ -2718,19 +2448,14 @@ function loadPendingTabRestore(pending: PendingTabRestore, timeoutMs: number): P return new Promise((resolve) => { let settled = false let timeout: ReturnType | undefined - const startedAt = Date.now() - let hardDeadlineAt = startedAt + timeoutMs + SITE_PERMISSION_PROMPT_TIMEOUT_MS - let deadlineAt = startedAt + timeoutMs + let deadlineAt = Date.now() + timeoutMs let foregroundDeadlineGranted = pending.priority === 'foreground' - let sitePermissionGraceGranted = false const finish = (loaded: boolean) => { if (settled) return settled = true if (timeout) clearTimeout(timeout) pending.cancelLoad = undefined - pending.grantSitePermissionGrace = undefined pending.promoteToForeground = undefined - if (!loaded) settleSitePermission(pending.tab, false) if ( loaded && isPendingTabRestoreLive(pending) && @@ -2767,21 +2492,10 @@ function loadPendingTabRestore(pending: PendingTabRestore, timeoutMs: number): P if (timeout) clearTimeout(timeout) timeout = setTimeout(() => stopLoad(true), Math.max(0, deadlineAt - Date.now())) } - pending.grantSitePermissionGrace = () => { - if (settled || sitePermissionGraceGranted) return - sitePermissionGraceGranted = true - deadlineAt = Math.min(deadlineAt + SITE_PERMISSION_PROMPT_TIMEOUT_MS, hardDeadlineAt) - scheduleDeadline() - } pending.promoteToForeground = () => { if (settled || foregroundDeadlineGranted) return foregroundDeadlineGranted = true - hardDeadlineAt = - startedAt + FOREGROUND_TAB_RESTORE_TIMEOUT_MS + SITE_PERMISSION_PROMPT_TIMEOUT_MS - deadlineAt = Math.min( - Math.max(deadlineAt, Date.now() + FOREGROUND_TAB_RESTORE_TIMEOUT_MS), - hardDeadlineAt - ) + deadlineAt = Date.now() + FOREGROUND_TAB_RESTORE_TIMEOUT_MS scheduleDeadline() } scheduleDeadline() @@ -2946,7 +2660,6 @@ export async function waitForPendingTabRestore(tab: AgentTab): Promise export function prepareExplicitNavigation(contents: WebContents): void { const tab = tabForContents(contents) if (!tab) return - settleSitePermission(tab, false) tab.pendingRestoreUrl = undefined discardPendingTabRestore(tab) } @@ -3019,7 +2732,6 @@ export function restoreBrowserSession(): void { nextTabId: state.nextTabId, restored: state.restored, lastPersistedSnapshot: state.lastPersistedSnapshot, - siteOriginGrants: new Map(state.siteOriginGrants), } const previousDownloads = browserDownloadsByScope.get(scopeId) const restoredTabs: AgentTab[] = [] @@ -3034,8 +2746,6 @@ export function restoreBrowserSession(): void { for (const { entry } of selectedEntries) { const tab = addTabInternal({ activate: false, notify: false }) tab.pendingRestoreUrl = entry.url - const restoredOrigin = mediaOrigin(entry.url) - if (restoredOrigin) grantSiteOrigin(state, restoredOrigin) restoredTabs.push(tab) restoredLoads.push({ tab, url: entry.url }) } @@ -3056,7 +2766,6 @@ export function restoreBrowserSession(): void { state.nextTabId = previousState.nextTabId state.restored = previousState.restored state.lastPersistedSnapshot = previousState.lastPersistedSnapshot - state.siteOriginGrants = previousState.siteOriginGrants if (previousDownloads) browserDownloadsByScope.set(scopeId, previousDownloads) else browserDownloadsByScope.delete(scopeId) applyActiveTabThrottling() @@ -3145,7 +2854,6 @@ export function reopenClosedTab(): AgentTab | null { // onBeforeRequest still runs the full DNS-resolving SSRF check on the // document load. Pre-checking would only buy a nicer error, and there is // no model to report one to — this path is a user keystroke. - grantSiteOriginForUserNavigation(tab.view.webContents, url) void tab.view.webContents.loadURL(url).catch(() => {}) } return tab @@ -3235,7 +2943,6 @@ export function closeTab( clearAutomationIndicatorsForTab(tabId) const [tab] = tabs.splice(index, 1) discardPendingTabRestore(tab) - settleSitePermission(tab, false) revokeTabMediaPermissions(tab, false) recentlyClosedTabUrls.unshift(sanitizeRestorableUrl(tabUrl(tab)) ?? 'about:blank') if (recentlyClosedTabUrls.length > MAX_RECENTLY_CLOSED_TABS) { @@ -3264,7 +2971,6 @@ export function closeTab( persistBrowserSession() events?.onTabsChanged() if (!hasSession()) { - currentScope.siteOriginGrants.clear() events?.onSessionClosed() } } @@ -3420,7 +3126,6 @@ function closeLiveTabs(): void { dismissFind(currentScope.findingTabId) for (const tab of tabs.splice(0)) { discardPendingTabRestore(tab) - settleSitePermission(tab, false, false) revokeTabMediaPermissions(tab, false) detachIfAttached(tab.view) if (!tab.view.webContents.isDestroyed()) { @@ -3433,7 +3138,6 @@ function closeLiveTabs(): void { currentScope.automationActive = false currentScope.automationNeedsAttention = false currentScope.visibleTabUserSelected = false - currentScope.siteOriginGrants.clear() clearFocusedBrowserTab() } diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 49127cf305d..b273e50f1eb 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -717,8 +717,6 @@ function main(): void { onSessionStatus: (alive, scopeId) => { scopeEvents.sendBrowser(scopeId, 'browser-agent:session-status', alive, scopeId) }, - sitePermissionPromptSupported: (scopeId) => - scopeEvents.browserSitePermissionPromptSupported(scopeId), onFillAvailability: (available, scopeId) => { scopeEvents.sendBrowser(scopeId, 'browser-credentials:fill-availability', { available, diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index cace1224b65..83deccabb27 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -299,7 +299,6 @@ describe('registerIpcHandlers', () => { scopeEvents: { activateBrowser: vi.fn(), activateTerminal: vi.fn(), - registerBrowserSitePermissionPromptSupport: vi.fn(), sendBrowser: vi.fn(), sendTerminal: vi.fn(), }, @@ -1047,56 +1046,19 @@ describe('registerIpcHandlers', () => { panelAction.mockRestore() }) - it('requires trusted input for site grants and user-origin navigation', async () => { + it('requires trusted input for user navigation', async () => { const { invoke, on } = collectHandlers() const panelAction = vi.spyOn(browserDriver, 'handlePanelAction').mockResolvedValue() const handler = on.get('browser-agent:panel-action') + const action = { action: 'navigate', url: 'https://docs.example/page' } - await invoke.get('browser-agent:activate-scope')?.(inactiveAppEvent, 'chat-sites') - handler?.( - inactiveAppEvent, - { action: 'respond-site-permission', requestId: 'request-1', allowed: true }, - 'chat-sites' - ) - handler?.( - inactiveAppEvent, - { action: 'respond-site-permission', requestId: 'request-1', allowed: false }, - 'chat-sites' - ) - handler?.( - inactiveAppEvent, - { action: 'navigate', url: 'https://docs.example/private' }, - 'chat-sites' - ) + await invoke.get('browser-agent:activate-scope')?.(inactiveAppEvent, 'chat-navigation') + handler?.(inactiveAppEvent, action, 'chat-navigation') + expect(panelAction).not.toHaveBeenCalled() - expect(panelAction).toHaveBeenCalledOnce() - expect(panelAction).toHaveBeenCalledWith('chat-sites', { - action: 'respond-site-permission', - requestId: 'request-1', - allowed: false, - }) - - await invoke.get('browser-agent:activate-scope')?.(activeAppEvent, 'chat-sites') - handler?.( - activeAppEvent, - { action: 'respond-site-permission', requestId: 'request-2', allowed: true }, - 'chat-sites' - ) - handler?.( - activeAppEvent, - { action: 'navigate', url: 'https://docs.example/private' }, - 'chat-sites' - ) - - expect(panelAction).toHaveBeenNthCalledWith(2, 'chat-sites', { - action: 'respond-site-permission', - requestId: 'request-2', - allowed: true, - }) - expect(panelAction).toHaveBeenNthCalledWith(3, 'chat-sites', { - action: 'navigate', - url: 'https://docs.example/private', - }) + await invoke.get('browser-agent:activate-scope')?.(activeAppEvent, 'chat-navigation') + handler?.(activeAppEvent, action, 'chat-navigation') + expect(panelAction).toHaveBeenCalledExactlyOnceWith('chat-navigation', action) panelAction.mockRestore() }) @@ -1123,20 +1085,6 @@ describe('registerIpcHandlers', () => { panelAction.mockRestore() }) - it('accepts site permission prompt support only from the app renderer', () => { - const { on } = collectHandlers() - const register = on.get('browser-agent:register-site-permission-prompt-support') - - register?.(evilEvent) - expect(deps.scopeEvents.registerBrowserSitePermissionPromptSupport).not.toHaveBeenCalled() - - register?.(appEvent) - expect(deps.scopeEvents.registerBrowserSitePermissionPromptSupport).toHaveBeenCalledOnce() - expect(deps.scopeEvents.registerBrowserSitePermissionPromptSupport).toHaveBeenCalledWith( - appSender - ) - }) - it('ignores browser-agent panel actions from outside the app origin', () => { const { on } = collectHandlers() const handler = on.get('browser-agent:panel-action') @@ -1376,11 +1324,10 @@ describe('registerIpcHandlers', () => { it('atomically creates and navigates a canonical user URL only from trusted input', async () => { const tabsState = { scopeId: 'chat-links', tabs: [], activeTabId: '2' } - const tabContents = { loadURL: vi.fn(async () => {}) } + const tabContents = { loadURL: vi.fn(async () => {}), isDestroyed: () => false } const add = vi.spyOn(browserSession, 'addTab').mockReturnValue({ view: { webContents: tabContents }, } as never) - const grant = vi.spyOn(browserSession, 'grantSiteOriginForUserNavigation').mockReturnValue(true) const peek = vi.spyOn(browserSession, 'peekTabsState').mockReturnValue(tabsState) const { invoke } = collectHandlers() @@ -1394,7 +1341,6 @@ describe('registerIpcHandlers', () => { ).resolves.toEqual(tabsState) expect(add).toHaveBeenCalledOnce() - expect(grant).toHaveBeenCalledWith(tabContents, CANONICAL_BROWSER_URL) expect(tabContents.loadURL).toHaveBeenCalledWith(CANONICAL_BROWSER_URL) for (const url of INVALID_BROWSER_URLS) { @@ -1415,7 +1361,6 @@ describe('registerIpcHandlers', () => { expect(add).toHaveBeenCalledOnce() add.mockRestore() - grant.mockRestore() peek.mockRestore() }) diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index e55dcb7a2c7..4586567439a 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -52,7 +52,6 @@ import { addTab, findInActiveTab, getBrowserDownloadsState, - grantSiteOriginForUserNavigation, peekTabsState, reorderTab, setBrowserAppTheme, @@ -335,11 +334,7 @@ export interface IpcDeps { terminal: TerminalRegistry scopeEvents: Pick< ScopedEventRouter, - | 'activateBrowser' - | 'activateTerminal' - | 'registerBrowserSitePermissionPromptSupport' - | 'sendBrowser' - | 'sendTerminal' + 'activateBrowser' | 'activateTerminal' | 'sendBrowser' | 'sendTerminal' > settings: DesktopSettingsService getWindowState: (sender: WebContents) => DesktopWindowState @@ -928,15 +923,6 @@ export function registerIpcHandlers(deps: IpcDeps): void { }) }, }, - 'browser-agent:register-site-permission-prompt-support': { - kind: 'send', - gate: 'app-origin', - requires: 'browser', - passSender: true, - handler: (sender) => { - deps.scopeEvents.registerBrowserSitePermissionPromptSupport(sender as WebContents) - }, - }, 'browser-agent:open-url': { kind: 'invoke', gate: 'app-origin', @@ -953,7 +939,7 @@ export function registerIpcHandlers(deps: IpcDeps): void { } return withBrowserScope(scope, () => { const tab = addTab() - if (!grantSiteOriginForUserNavigation(tab.view.webContents, destination)) { + if (tab.view.webContents.isDestroyed()) { return peekTabsState() } void tab.view.webContents.loadURL(destination).catch(() => {}) @@ -1146,11 +1132,7 @@ export function registerIpcHandlers(deps: IpcDeps): void { needsUserActivation: ([action]) => { if (!isRecordLike(action)) return false if (action.action === 'navigate') return true - return ( - (action.action === 'respond-media-permission' || - action.action === 'respond-site-permission') && - action.allowed === true - ) + return action.action === 'respond-media-permission' && action.allowed === true }, handler: (sender, action, rawScope) => { const scope = activeRendererScope(browserScopeBySender, sender as WebContents, rawScope) diff --git a/apps/desktop/src/main/scoped-event-router.test.ts b/apps/desktop/src/main/scoped-event-router.test.ts index 700d6dbc3d5..7020b526b32 100644 --- a/apps/desktop/src/main/scoped-event-router.test.ts +++ b/apps/desktop/src/main/scoped-event-router.test.ts @@ -37,10 +37,6 @@ class FakeContents { this.emit('destroyed') } - markDestroyed(): void { - this.destroyed = true - } - private emit(channel: string, ...args: unknown[]): void { for (const listener of [...(this.listeners.get(channel) ?? [])]) listener(...args) } @@ -52,76 +48,6 @@ function webContents(): { fake: FakeContents; contents: WebContents } { } describe('ScopedEventRouter', () => { - it('defaults old renderers to no site permission prompt support', () => { - const router = new ScopedEventRouter() - const renderer = webContents() - - router.activateBrowser(renderer.contents, 'chat-a') - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(false) - }) - - it('recognizes an active renderer site permission prompt handshake', () => { - const router = new ScopedEventRouter() - const renderer = webContents() - - router.registerBrowserSitePermissionPromptSupport(renderer.contents) - router.activateBrowser(renderer.contents, 'chat-a') - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(true) - }) - - it('requires a fresh site permission prompt handshake after renderer reload', () => { - const router = new ScopedEventRouter() - const renderer = webContents() - - router.registerBrowserSitePermissionPromptSupport(renderer.contents) - router.activateBrowser(renderer.contents, 'chat-a') - renderer.fake.navigate() - router.activateBrowser(renderer.contents, 'chat-a') - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(false) - - router.registerBrowserSitePermissionPromptSupport(renderer.contents) - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(true) - }) - - it('rejects ambiguous site prompts when two live renderers share a scope', () => { - const router = new ScopedEventRouter() - const first = webContents() - const second = webContents() - - router.activateBrowser(first.contents, 'chat-a') - router.activateBrowser(second.contents, 'chat-a') - router.registerBrowserSitePermissionPromptSupport(first.contents) - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(false) - - router.registerBrowserSitePermissionPromptSupport(second.contents) - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(false) - }) - - it('recovers site prompt support after an extra recipient moves or is destroyed', () => { - const router = new ScopedEventRouter() - const stable = webContents() - const moving = webContents() - - router.registerBrowserSitePermissionPromptSupport(stable.contents) - router.registerBrowserSitePermissionPromptSupport(moving.contents) - router.activateBrowser(stable.contents, 'chat-a') - router.activateBrowser(moving.contents, 'chat-a') - router.activateBrowser(moving.contents, 'chat-b') - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(true) - - router.activateBrowser(moving.contents, 'chat-a') - moving.fake.markDestroyed() - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(true) - }) - it('sends resource events only to renderers activated for the matching scope', () => { const router = new ScopedEventRouter() const chatA = webContents() diff --git a/apps/desktop/src/main/scoped-event-router.ts b/apps/desktop/src/main/scoped-event-router.ts index 7d36ca68bc5..62541b098b2 100644 --- a/apps/desktop/src/main/scoped-event-router.ts +++ b/apps/desktop/src/main/scoped-event-router.ts @@ -14,34 +14,6 @@ export class ScopedEventRouter { private readonly browser = this.createSurfaceRoutes() private readonly terminal = this.createSurfaceRoutes() private readonly observedContents = new WeakSet() - private readonly sitePermissionPromptRenderers = new WeakSet() - - /** Records an active renderer handshake without trusting a shell-bundled preload flag. */ - registerBrowserSitePermissionPromptSupport(contents: WebContents): void { - this.sitePermissionPromptRenderers.add(contents) - this.observe(contents) - } - - /** True only when exactly one live renderer owns the scope and registered prompt support. */ - browserSitePermissionPromptSupported(scopeId: string): boolean { - const recipients = this.browser.contentsByScope.get(scopeId) - if (!recipients) return false - let liveRecipientCount = 0 - let supported = false - for (const contents of [...recipients]) { - if (contents.isDestroyed()) { - this.forget(contents) - continue - } - if (this.browser.activeByContents.get(contents) !== scopeId) { - this.removeFromScope(this.browser, contents, scopeId) - continue - } - liveRecipientCount++ - supported ||= this.sitePermissionPromptRenderers.has(contents) - } - return liveRecipientCount === 1 && supported - } activateBrowser(contents: WebContents, scopeId: string): void { this.activate(this.browser, contents, scopeId) @@ -94,7 +66,6 @@ export class ScopedEventRouter { } private forget(contents: WebContents): void { - this.sitePermissionPromptRenderers.delete(contents) this.forgetSurface(this.browser, contents) this.forgetSurface(this.terminal, contents) } diff --git a/apps/desktop/src/preload/index.test.ts b/apps/desktop/src/preload/index.test.ts index 5854b1b624b..9e7f1a313d1 100644 --- a/apps/desktop/src/preload/index.test.ts +++ b/apps/desktop/src/preload/index.test.ts @@ -28,7 +28,7 @@ describe('desktop preload bridge', () => { if (!exposed) throw new Error('Expected the desktop preload API to be exposed') expect(exposed.browserAgent.supportsAtomicPanelOcclusion).toBe(true) - exposed.browserAgent.registerSitePermissionPromptSupport?.() + expect(exposed.browserAgent.registerSitePermissionPromptSupport).toBeUndefined() await exposed.browserAgent.cancelTool?.('tool-1', 'chat-default') await exposed.browserAgent.cancelActiveTool?.('chat-reloaded') await exposed.browserAgent.setPanelOccluded(true, 'chat-default') @@ -46,7 +46,6 @@ describe('desktop preload bridge', () => { ['browser-agent:search-suggestions', 'sim ai'], ['desktop:settings:set-browser-search-suggestions', false], ]) - expect(send).toHaveBeenCalledWith('browser-agent:register-site-permission-prompt-support') }) it('exposes native microphone settings only on supported platforms', async () => { diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index e4e78d8a19b..34c37732078 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -196,9 +196,6 @@ const api: SimDesktopApi = { }, browserAgent: { supportsAtomicPanelOcclusion: true, - registerSitePermissionPromptSupport: (): void => { - ipcRenderer.send('browser-agent:register-site-permission-prompt-support') - }, executeTool: ( toolCallId: string, tool: BrowserToolName, diff --git a/packages/browser-protocol/src/index.ts b/packages/browser-protocol/src/index.ts index 91195dc6a84..67c956b2dc4 100644 --- a/packages/browser-protocol/src/index.ts +++ b/packages/browser-protocol/src/index.ts @@ -236,6 +236,7 @@ export interface BrowserPanelAction { | 'zoom-out' | 'zoom-reset' | 'respond-media-permission' + /** Compatibility response for installed shells with the retired navigation gate. */ | 'respond-site-permission' | 'takeover-done' /** Absolute URL for `navigate` (typed into the panel's URL bar). */ @@ -265,7 +266,7 @@ export interface BrowserMediaPermissionRequest { devices: BrowserMediaDevice[] } -/** One ungranted top-level origin transition awaiting explicit user consent. */ +/** Legacy navigation request emitted only by installed shells with per-task site consent. */ export interface BrowserSitePermissionRequest { requestId: string /** Exact tab whose suspended request will be resumed or cancelled. */ @@ -288,7 +289,7 @@ export interface BrowserPageState { issue?: BrowserPageIssue /** Main-frame media request awaiting a renderer-owned permission prompt. */ mediaPermissionRequest?: BrowserMediaPermissionRequest - /** Ungranted top-level origin transition awaiting a renderer-owned permission prompt. */ + /** Legacy request from installed shells that still require a site-origin prompt. */ sitePermissionRequest?: BrowserSitePermissionRequest } diff --git a/packages/desktop-bridge/contract-snapshot.ts b/packages/desktop-bridge/contract-snapshot.ts index f81d290db5f..c52c10df42b 100644 --- a/packages/desktop-bridge/contract-snapshot.ts +++ b/packages/desktop-bridge/contract-snapshot.ts @@ -251,6 +251,7 @@ export interface BrowserPanelAction { | 'zoom-out' | 'zoom-reset' | 'respond-media-permission' + /** Compatibility response for installed shells with the retired navigation gate. */ | 'respond-site-permission' | 'takeover-done' /** Absolute URL for `navigate` (typed into the panel's URL bar). */ @@ -280,7 +281,7 @@ export interface BrowserMediaPermissionRequest { devices: BrowserMediaDevice[] } -/** One ungranted top-level origin transition awaiting explicit user consent. */ +/** Legacy navigation request emitted only by installed shells with per-task site consent. */ export interface BrowserSitePermissionRequest { requestId: string /** Exact tab whose suspended request will be resumed or cancelled. */ @@ -303,7 +304,7 @@ export interface BrowserPageState { issue?: BrowserPageIssue /** Main-frame media request awaiting a renderer-owned permission prompt. */ mediaPermissionRequest?: BrowserMediaPermissionRequest - /** Ungranted top-level origin transition awaiting a renderer-owned permission prompt. */ + /** Legacy request from installed shells that still require a site-origin prompt. */ sitePermissionRequest?: BrowserSitePermissionRequest } @@ -1066,8 +1067,8 @@ export interface SimDesktopBrowserAgentApi { /** New shells can atomically force-hide a native page before renderer effects paint. */ readonly supportsAtomicPanelOcclusion?: true /** - * Confirms that this renderer can present and answer site-origin prompts. - * Optional for compatibility with installed shells that predate site consent. + * Confirms that this renderer can present and answer legacy site-origin prompts. + * Only installed shells with the retired per-task navigation gate expose this. */ registerSitePermissionPromptSupport?(): void /** diff --git a/packages/desktop-bridge/src/index.ts b/packages/desktop-bridge/src/index.ts index f91713aec1e..e98e761dc4a 100644 --- a/packages/desktop-bridge/src/index.ts +++ b/packages/desktop-bridge/src/index.ts @@ -158,8 +158,8 @@ export interface SimDesktopBrowserAgentApi { /** New shells can atomically force-hide a native page before renderer effects paint. */ readonly supportsAtomicPanelOcclusion?: true /** - * Confirms that this renderer can present and answer site-origin prompts. - * Optional for compatibility with installed shells that predate site consent. + * Confirms that this renderer can present and answer legacy site-origin prompts. + * Only installed shells with the retired per-task navigation gate expose this. */ registerSitePermissionPromptSupport?(): void /**