Skip to content

Commit 79836b0

Browse files
waleedlatif1waleedlatif1
committed
improvement(desktop): remove per-task website approval prompts
1 parent 2e08162 commit 79836b0

15 files changed

Lines changed: 121 additions & 907 deletions

File tree

‎apps/desktop/e2e/browser-tools.spec.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const FORM = `<!doctype html><html><head><title>Form fixture</title></head><body
2121
<label>Updates <input id="updates" type="checkbox"></label>
2222
<label>Password <input id="password" type="password"></label>
2323
<label>Route <input id="route" oninput="history.pushState({}, '', '/form?changed=1')"></label>
24+
<a href="/redirect">Other website</a>
2425
<div id="horizontal" role="region" aria-label="Wide table" tabindex="0" style="width:280px;overflow-x:auto">
2526
<div style="width:1600px;height:100px">Wide content</div>
2627
</div>
@@ -40,6 +41,11 @@ test.describe('browser tools', () => {
4041
test.beforeAll(async () => {
4142
server = createServer(async (request, response) => {
4243
const path = new URL(request.url ?? '/', 'http://127.0.0.1').pathname
44+
if (path === '/redirect') {
45+
response.writeHead(302, { Location: `${origin.replace('127.0.0.1', 'localhost')}/landing` })
46+
response.end()
47+
return
48+
}
4349
if (path === '/api/desktop/tool/authorize') {
4450
let body = ''
4551
for await (const chunk of request) body += chunk.toString()
@@ -189,6 +195,28 @@ test.describe('browser tools', () => {
189195
expect(await formState()).toMatchObject({ name: '', route: 'change route' })
190196
})
191197

198+
test('follows a link and cross-origin redirect without a website approval prompt', async () => {
199+
await openForm()
200+
await app.evaluate(async ({ webContents }, url) => {
201+
const page = webContents.getAllWebContents().find((contents) => contents.getURL() === url)
202+
if (!page) throw new Error('Missing browser fixture')
203+
await page.executeJavaScript("document.querySelector('a').click()")
204+
}, `${origin}/form`)
205+
206+
const destination = `${origin.replace('127.0.0.1', 'localhost')}/landing`
207+
await expect
208+
.poll(() =>
209+
app.evaluate(
210+
({ webContents }, url) =>
211+
webContents.getAllWebContents().some((contents) => contents.getURL() === url),
212+
destination
213+
)
214+
)
215+
.toBe(true)
216+
expect(await app.evaluate(({ BrowserWindow }) => BrowserWindow.getAllWindows().length)).toBe(1)
217+
await expect(window.getByRole('heading')).toHaveText('Browser tools fixture')
218+
})
219+
192220
test('stops when a new popup exceeds the page summary limit', async () => {
193221
const ref = await openForm()
194222
await app.evaluate(async ({ webContents }, origin) => {

‎apps/desktop/src/main/browser-agent/driver.test.ts‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,15 @@ describe('executeTool', () => {
7777
})
7878

7979
it('validates navigation URLs before touching the session', async () => {
80-
const grant = vi.spyOn(session, 'grantSiteOriginForAgentNavigation')
80+
const prepare = vi.spyOn(session, 'prepareExplicitNavigation')
8181
const result = await driver.executeTool('chat-test', 'browser_navigate', {
8282
url: 'file:///etc/passwd',
8383
})
8484
expect(result).toEqual({
8585
ok: false,
8686
error: 'URL must be absolute and start with http:// or https://',
8787
})
88-
expect(grant).not.toHaveBeenCalled()
88+
expect(prepare).not.toHaveBeenCalled()
8989
})
9090

9191
it('reports missing required parameters by name', async () => {
@@ -94,8 +94,7 @@ describe('executeTool', () => {
9494
expect(result.error).toMatch(/Missing required parameter "url"/)
9595
})
9696

97-
it('grants only SSRF-checked agent navigation destinations before loading them', async () => {
98-
const grant = vi.spyOn(session, 'grantSiteOriginForAgentNavigation')
97+
it('loads SSRF-checked agent navigation destinations', async () => {
9998
const navigations = [
10099
['browser_navigate', 'http://127.0.0.1:4011/navigate'],
101100
['browser_open_url', 'http://127.0.0.1:4012/open'],
@@ -106,9 +105,9 @@ describe('executeTool', () => {
106105
await expect(driver.executeTool('chat-test', tool, { url })).resolves.toMatchObject({
107106
ok: true,
108107
})
109-
expect(grant).toHaveBeenCalledWith(expect.anything(), url)
108+
const contents = session.requireAutomationTab().view.webContents
109+
expect(contents.loadURL).toHaveBeenCalledWith(url)
110110
}
111-
expect(grant).toHaveBeenCalledTimes(navigations.length)
112111
})
113112

114113
it('keeps the 400ms hydration grace without rediscovering a completed load', async () => {
@@ -1140,8 +1139,8 @@ describe('executeTool', () => {
11401139
expect(respond).toHaveBeenCalledWith('request-1', true)
11411140
})
11421141

1143-
it('routes an exact renderer site decision through the scoped session boundary', async () => {
1144-
const respond = vi.spyOn(session, 'respondToSitePermission').mockReturnValue(true)
1142+
it('ignores retired site decisions without changing tab ownership', async () => {
1143+
const claim = vi.spyOn(session, 'claimActiveTabForUser')
11451144

11461145
await driver.handlePanelAction('chat-test', {
11471146
action: 'respond-site-permission',
@@ -1153,22 +1152,18 @@ describe('executeTool', () => {
11531152
requestId: 'request-2',
11541153
})
11551154

1156-
expect(respond).toHaveBeenCalledOnce()
1157-
expect(respond).toHaveBeenCalledWith('request-1', true)
1155+
expect(claim).not.toHaveBeenCalled()
11581156
})
11591157

1160-
it('grants only the exact origin entered through the user omnibox', async () => {
1158+
it('loads the exact URL entered through the user omnibox', async () => {
11611159
await driver.executeTool('chat-test', 'browser_open_tab', {})
11621160
const contents = session.requireTab().view.webContents
1163-
const grant = vi.spyOn(session, 'grantSiteOriginForUserNavigation')
11641161

11651162
await driver.handlePanelAction('chat-test', {
11661163
action: 'navigate',
11671164
url: 'https://docs.example/private?token=secret',
11681165
})
11691166

1170-
expect(grant).toHaveBeenCalledOnce()
1171-
expect(grant).toHaveBeenCalledWith(contents, 'https://docs.example/private?token=secret')
11721167
expect(contents.loadURL).toHaveBeenCalledWith('https://docs.example/private?token=secret')
11731168
})
11741169

‎apps/desktop/src/main/browser-agent/driver.ts‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,6 @@ export interface DriverCallbacks {
194194
onPageState: (state: BrowserPageState) => void
195195
onTabsState: (state: BrowserTabsState) => void
196196
onSessionStatus: (alive: boolean, scopeId: string) => void
197-
/** Whether a live renderer for the scope registered support for the consent prompt. */
198-
sitePermissionPromptSupported?: (scopeId: string) => boolean
199197
/** Whether the active tab shows a login form Sim holds a credential for. */
200198
onFillAvailability: (available: boolean, scopeId: string) => void
201199
/** Live native download state for one isolated browser scope. */
@@ -467,7 +465,6 @@ function recordNotice(notice: string): void {
467465
function pageStateFor(contents: WebContents, tabId: string): BrowserPageState {
468466
const issue = session.pageIssueForContents(contents)
469467
const mediaPermissionRequest = session.mediaPermissionRequestForContents(contents)
470-
const sitePermissionRequest = session.sitePermissionRequestForScope()
471468
return {
472469
scopeId: session.getBrowserScopeId(),
473470
tabId,
@@ -478,7 +475,6 @@ function pageStateFor(contents: WebContents, tabId: string): BrowserPageState {
478475
canGoForward: session.canGoForward(contents),
479476
...(issue ? { issue } : {}),
480477
...(mediaPermissionRequest ? { mediaPermissionRequest } : {}),
481-
...(sitePermissionRequest ? { sitePermissionRequest } : {}),
482478
}
483479
}
484480

@@ -661,8 +657,6 @@ export function initDriver(
661657
void fillCoordinator()?.refreshAvailability(true)
662658
},
663659
onPageStateChanged: pushPageState,
664-
sitePermissionPromptSupported: (scopeId) =>
665-
driverCallbacks?.sitePermissionPromptSupported?.(scopeId) === true,
666660
onTabsChanged: pushTabsState,
667661
onTabThemeChanged: (contents, theme) => {
668662
void cdp.setColorScheme(contents, theme).catch((error) => {
@@ -1373,7 +1367,7 @@ async function loadAgentCheckedUrlAndGetResult(
13731367
url: string
13741368
): Promise<Record<string, unknown>> {
13751369
session.prepareExplicitNavigation(contents)
1376-
if (!session.grantSiteOriginForAgentNavigation(contents, url)) {
1370+
if (contents.isDestroyed()) {
13771371
throw new ToolError('The tab was closed before navigation could start.')
13781372
}
13791373
const beforeUrl = contents.getURL()
@@ -4766,9 +4760,7 @@ export async function handlePanelAction(
47664760
return
47674761
}
47684762
if (action.action === 'respond-site-permission') {
4769-
if (typeof action.requestId === 'string' && typeof action.allowed === 'boolean') {
4770-
session.respondToSitePermission(action.requestId, action.allowed)
4771-
}
4763+
/** Older renderers can still send a response to the retired task-navigation prompt. */
47724764
return
47734765
}
47744766
// Navigate bootstraps the session: the user can open the panel manually
@@ -4779,7 +4771,6 @@ export async function handlePanelAction(
47794771
session.claimActiveTabForUser()
47804772
const contents = session.ensureTab().view.webContents
47814773
session.prepareExplicitNavigation(contents)
4782-
session.grantSiteOriginForUserNavigation(contents, action.url)
47834774
void contents.loadURL(action.url).catch(() => {})
47844775
}
47854776
return

0 commit comments

Comments
 (0)