Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions apps/desktop/e2e/browser-tools.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const FORM = `<!doctype html><html><head><title>Form fixture</title></head><body
<label>Updates <input id="updates" type="checkbox"></label>
<label>Password <input id="password" type="password"></label>
<label>Route <input id="route" oninput="history.pushState({}, '', '/form?changed=1')"></label>
<a href="/redirect">Other website</a>
<div id="horizontal" role="region" aria-label="Wide table" tabindex="0" style="width:280px;overflow-x:auto">
<div style="width:1600px;height:100px">Wide content</div>
</div>
Expand All @@ -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()
Expand Down Expand Up @@ -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) => {
Expand Down
23 changes: 9 additions & 14 deletions apps/desktop/src/main/browser-agent/driver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ 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',
})
expect(result).toEqual({
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 () => {
Expand All @@ -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'],
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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',
Expand All @@ -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')
})

Expand Down
13 changes: 2 additions & 11 deletions apps/desktop/src/main/browser-agent/driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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,
Expand All @@ -478,7 +475,6 @@ function pageStateFor(contents: WebContents, tabId: string): BrowserPageState {
canGoForward: session.canGoForward(contents),
...(issue ? { issue } : {}),
...(mediaPermissionRequest ? { mediaPermissionRequest } : {}),
...(sitePermissionRequest ? { sitePermissionRequest } : {}),
}
}

Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -1373,7 +1367,7 @@ async function loadAgentCheckedUrlAndGetResult(
url: string
): Promise<Record<string, unknown>> {
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()
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading
Loading