Skip to content

Commit 923f36e

Browse files
committed
fix(desktop): refuse window.open app links, gate every launch on a prompt-capable renderer, and keep page-raised prompts from taking focus
1 parent 85f98bf commit 923f36e

7 files changed

Lines changed: 75 additions & 45 deletions

File tree

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

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -268,30 +268,36 @@ describe('app links', () => {
268268
frameTreeNodeId: 2,
269269
origin: 'https://ads.example',
270270
} as WebFrameMain
271-
requestExternalLaunch(contents, 'zoommtg://zoom.us/join', { initiator: frame })
272-
requestExternalLaunch(contents, 'zoommtg://zoom.us/join', {
273-
initiator: topFrame('https://ads.example'),
274-
})
275-
requestExternalLaunch(contents, 'zoommtg://zoom.us/join', { initiator: null })
271+
requestExternalLaunch(contents, 'zoommtg://zoom.us/join', frame)
272+
requestExternalLaunch(contents, 'zoommtg://zoom.us/join', topFrame('https://ads.example'))
273+
requestExternalLaunch(contents, 'zoommtg://zoom.us/join', null)
276274
expect(pagePromptForContents(contents)).toBeUndefined()
277275
expect(appLaunchNoticeForContents(contents)).toBeUndefined()
278276
})
279277

280-
it('asks after the top document is clicked, but only offers a popup passively', () => {
278+
it('asks after the top document is clicked, and otherwise only offers passively', () => {
281279
const clicked = agentContents()
282-
requestExternalLaunch(clicked, 'zoommtg://zoom.us/join', { initiator: topFrame() })
280+
requestExternalLaunch(clicked, 'zoommtg://zoom.us/join', topFrame())
283281
expect(pagePromptForContents(clicked)).toMatchObject({ kind: 'external-app', appName: 'Zoom' })
284282

285-
const popup = agentContents()
286-
requestExternalLaunch(popup, 'zoommtg://zoom.us/join', 'popup')
287-
expect(pagePromptForContents(popup)).toBeUndefined()
288-
expect(appLaunchNoticeForContents(popup)).toMatchObject({ origin: PAGE, appName: 'Zoom' })
283+
context.userActivated = false
284+
const unclicked = agentContents()
285+
requestExternalLaunch(unclicked, 'zoommtg://zoom.us/join', topFrame())
286+
expect(pagePromptForContents(unclicked)).toBeUndefined()
287+
expect(appLaunchNoticeForContents(unclicked)).toMatchObject({ origin: PAGE, appName: 'Zoom' })
288+
})
289+
290+
it('opens nothing, not even a clicked mail link, without a renderer that shows prompts', async () => {
291+
context.promptsEnabled = false
292+
requestExternalLaunch(agentContents(), 'mailto:ada@example.com', topFrame())
293+
await vi.advanceTimersByTimeAsync(0)
294+
expect(shell.openExternal).not.toHaveBeenCalled()
289295
})
290296

291297
it('opens only the first link while its prompt is pending', async () => {
292298
const contents = agentContents()
293-
requestExternalLaunch(contents, 'zoommtg://zoom.us/join?id=1', { initiator: topFrame() })
294-
requestExternalLaunch(contents, 'zoommtg://zoom.us/join?id=2', { initiator: topFrame() })
299+
requestExternalLaunch(contents, 'zoommtg://zoom.us/join?id=1', topFrame())
300+
requestExternalLaunch(contents, 'zoommtg://zoom.us/join?id=2', topFrame())
295301
expect(appLaunchNoticeForContents(contents)).toBeUndefined()
296302
answer(contents, { allowed: true, kind: 'external-app', alwaysAllow: false })
297303
await vi.advanceTimersByTimeAsync(0)
@@ -300,11 +306,11 @@ describe('app links', () => {
300306

301307
it('forgets "Always allow" on reset', async () => {
302308
const contents = agentContents()
303-
requestExternalLaunch(contents, 'zoommtg://zoom.us/join?id=1', { initiator: topFrame() })
309+
requestExternalLaunch(contents, 'zoommtg://zoom.us/join?id=1', topFrame())
304310
answer(contents, { allowed: true, kind: 'external-app', alwaysAllow: true })
305311
resetPagePromptDecisions()
306312
vi.advanceTimersByTime(20_000)
307-
requestExternalLaunch(contents, 'zoommtg://zoom.us/join?id=2', { initiator: topFrame() })
313+
requestExternalLaunch(contents, 'zoommtg://zoom.us/join?id=2', topFrame())
308314
await vi.advanceTimersByTimeAsync(0)
309315
expect(pagePromptForContents(contents)).toMatchObject({ kind: 'external-app' })
310316
expect(shell.openExternal).toHaveBeenCalledExactlyOnceWith('zoommtg://zoom.us/join?id=1')
@@ -383,7 +389,7 @@ describe('accepting an answer', () => {
383389
vi.mocked(shell.openExternal).mockClear()
384390
context.userActivated = slot === 'prompt'
385391
const contents = agentContents()
386-
requestExternalLaunch(contents, 'zoommtg://zoom.us/join', { initiator: topFrame })
392+
requestExternalLaunch(contents, 'zoommtg://zoom.us/join', topFrame)
387393
const requestId =
388394
slot === 'prompt'
389395
? shownPrompt(contents).requestId

‎apps/desktop/src/main/browser-agent/page-prompts.ts‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -486,13 +486,6 @@ export function handleSelectClientCertificate(
486486
})
487487
}
488488

489-
/**
490-
* Who asked to open another app: a navigation names its initiating frame; a
491-
* popup cannot be attributed to one, since Electron's window-open details
492-
* carry no frame.
493-
*/
494-
export type AppLaunchSource = { initiator: WebFrameMain | null | undefined } | 'popup'
495-
496489
/** The tab's own top-level document asked, not a frame inside it. */
497490
function isTopDocument(
498491
contents: WebContents,
@@ -516,16 +509,18 @@ function launchAppLink(url: string): void {
516509
}
517510

518511
/**
519-
* A browser tab asked to open a non-web URL. Agent input never launches
520-
* anything, and neither do frames. The top document's request right after the
521-
* user's own click asks them to confirm the app (mail links open directly);
522-
* a popup or a request without that click only gets a passive notice the user
523-
* can act on. A pending launch prompt refuses further requests.
512+
* A browser tab's navigation to a non-web URL, with the frame that initiated
513+
* it. Agent input never launches anything, and neither do frames; `window.open`
514+
* never reaches here, since it names no initiating frame. The top document's
515+
* request right after the user's own click asks them to confirm the app (mail
516+
* links open directly); one without that click only gets a passive notice the
517+
* user can act on. Nothing launches without a renderer that shows prompts, and
518+
* a pending launch prompt refuses further requests.
524519
*/
525520
export function requestExternalLaunch(
526521
contents: WebContents,
527522
rawUrl: string,
528-
source: AppLaunchSource
523+
initiator: WebFrameMain | null | undefined
529524
): void {
530525
if (!host || contents.isDestroyed() || !isAgentWebContents(contents)) return
531526
const target = appLinkTarget(rawUrl)
@@ -540,7 +535,7 @@ export function requestExternalLaunch(
540535
appName = displayText(app.getApplicationNameForProtocol(target.url), 80)
541536
} catch {}
542537
const context = host.tabContext(contents)
543-
if (!appName || !context) return
538+
if (!appName || !context?.promptsEnabled) return
544539
if (context.agentActedLast) {
545540
host.noticeForAgent(
546541
contents,
@@ -550,14 +545,14 @@ export function requestExternalLaunch(
550545
}
551546
const origin = parseHttpUrl(contents.getURL())?.origin
552547
if (!origin) return
553-
if (source !== 'popup' && !isTopDocument(contents, source.initiator, origin)) {
548+
if (!isTopDocument(contents, initiator, origin)) {
554549
logger.warn('Denied an app link from a frame in agent browser', { scheme: target.scheme })
555550
return
556551
}
557552
const attempts = recentAttempts(contents, 'app-launch', LAUNCH_RATE_WINDOW_MS)
558553
if (attempts.length >= MAX_LAUNCHES_PER_WINDOW) return
559554
const launchKey = `${origin}|${target.scheme}`
560-
if (source !== 'popup' && context.userActivated && !context.agentDrives) {
555+
if (context.userActivated && !context.agentDrives) {
561556
if (target.scheme === 'mailto' || alwaysAllowedLaunches.has(launchKey)) {
562557
attempts.push(Date.now())
563558
launchAppLink(target.url)

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ vi.mock('node:dns/promises', () => ({
1212
}))
1313

1414
import {
15+
app,
1516
BrowserWindow,
1617
dialog,
1718
session as electronSession,
1819
Menu,
1920
shell,
2021
WebContentsView,
2122
} from 'electron'
23+
import { appLaunchNoticeForContents } from '@/main/browser-agent/page-prompts'
2224
import * as panel from '@/main/browser-agent/panel'
2325
import { agentAppOrigin, routeAgentNavigation } from '@/main/browser-agent/registry'
2426
import * as sessionModule from '@/main/browser-agent/session'
@@ -1257,6 +1259,11 @@ describe('browser-agent session', () => {
12571259
contents.loadURL.mockClear()
12581260
expect(openHandler({ url: 'file:///etc/passwd' })).toEqual({ action: 'deny' })
12591261
expect(contents.loadURL).not.toHaveBeenCalled()
1262+
// window.open names no initiating frame, so an app link from it is never offered either.
1263+
session.enablePagePrompts()
1264+
app.getApplicationNameForProtocol = vi.fn(() => 'Zoom')
1265+
expect(openHandler({ url: 'zoommtg://zoom.us/join' })).toEqual({ action: 'deny' })
1266+
expect(appLaunchNoticeForContents(contents as unknown as WebContents)).toBeUndefined()
12601267
})
12611268

12621269
it('opens a background-disposition popup by URL and keeps cross-scheme popups denied', () => {

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ import {
6060
cancelPagePrompts,
6161
handleSelectPasskeyAccount,
6262
noteMainFrameNavigation,
63-
requestExternalLaunch,
6463
resetPagePromptDecisions,
6564
setPagePromptHost,
6665
} from '@/main/browser-agent/page-prompts'
@@ -2777,10 +2776,6 @@ function initializeTabView(
27772776
// WebContents keeps window.opener for sign-in popups that post back to it.
27782777
contents.setWindowOpenHandler((details) =>
27792778
withBrowserScope(scopeId, () => {
2780-
if (!isBlankPopupUrl(details.url) && !/^https?:\/\//i.test(details.url)) {
2781-
requestExternalLaunch(contents, details.url, 'popup')
2782-
return { action: 'deny' }
2783-
}
27842779
const agentOwned = agentOwnsPopupFrom(contents)
27852780
const background = details.disposition === 'background-tab'
27862781
const adopt = canAdoptPopup(contents, details.url)

‎apps/desktop/src/main/security-guards.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export function attachNavigationGuards(contents: WebContents, deps: GuardDeps):
4343
if (!hasAllowedAgentBrowserScheme) {
4444
event.preventDefault()
4545
if (event.isMainFrame === true) {
46-
requestExternalLaunch(contents, url, { initiator: event.initiator })
46+
requestExternalLaunch(contents, url, event.initiator)
4747
} else logger.warn('Denied non-http navigation in agent browser', { url: scrubUrl(url) })
4848
}
4949
return

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt.tsx‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,10 @@ interface PromptModalProps<Prompt extends BrowserPagePrompt> {
5656
onRespond: RespondToPrompt
5757
}
5858

59-
/** Signing in takes a click on Sign in: Enter in a field never submits credentials a keystroke meant for the page. */
59+
/**
60+
* A page raised this, so it never takes focus, and signing in takes a click on
61+
* Sign in: Enter in a field never submits credentials typed for the page.
62+
*/
6063
function HttpAuthPromptModal({ prompt, open, onRespond }: PromptModalProps<BrowserHttpAuthPrompt>) {
6164
const armed = useActivationDelay(open)
6265
const [username, setUsername] = useState('')
@@ -65,7 +68,13 @@ function HttpAuthPromptModal({ prompt, open, onRespond }: PromptModalProps<Brows
6568
const submit = () =>
6669
onRespond(prompt.requestId, { allowed: true, kind: 'http-auth', username, password })
6770
return (
68-
<ChipModal open={open} onOpenChange={(next) => !next && cancel()} srTitle='Sign in' size='sm'>
71+
<ChipModal
72+
open={open}
73+
onOpenChange={(next) => !next && cancel()}
74+
srTitle='Sign in'
75+
size='sm'
76+
focusOnOpen={false}
77+
>
6978
<ChipModalHeader onClose={cancel}>Sign in</ChipModalHeader>
7079
<ChipModalBody>
7180
<ChipModalDescription>
@@ -118,9 +127,9 @@ interface ChoicePromptModalProps {
118127
}
119128

120129
/**
121-
* Picks one of the page's offered identities. Nothing is chosen up front, Enter
122-
* dismisses, and Continue waits out the activation delay, so a keystroke meant
123-
* for the page can never send an identity.
130+
* Picks one of the page's offered identities. It never takes focus, nothing is
131+
* chosen up front, Enter dismisses, and Continue waits out the activation delay,
132+
* so a keystroke meant for the page can never send an identity.
124133
*/
125134
function ChoicePromptModal({
126135
open,
@@ -137,7 +146,13 @@ function ChoicePromptModal({
137146
const armed = useActivationDelay(open)
138147
const [choiceId, setChoiceId] = useState('')
139148
return (
140-
<ChipModal open={open} onOpenChange={(next) => !next && onCancel()} srTitle={title} size='sm'>
149+
<ChipModal
150+
open={open}
151+
onOpenChange={(next) => !next && onCancel()}
152+
srTitle={title}
153+
size='sm'
154+
focusOnOpen={false}
155+
>
141156
<ChipModalHeader onClose={onCancel}>{title}</ChipModalHeader>
142157
<ChipModalBody>
143158
<ChipModalDescription>{description}</ChipModalDescription>

‎packages/emcn/src/components/chip-modal/chip-modal.tsx‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ function focusChipModalDefaultAction(event: Event): void {
156156
focusChipModalContent(event.currentTarget as HTMLElement | null)
157157
}
158158

159+
function keepFocusOnOpen(event: Event): void {
160+
event.preventDefault()
161+
}
162+
159163
/**
160164
* Supplies implicit submission for single-line custom inputs that are not a
161165
* {@link ChipModalField}. Canonical fields handle Enter at the control so they
@@ -216,6 +220,13 @@ interface ChipModalProps {
216220
* @default false
217221
*/
218222
dismissDisabled?: boolean
223+
/**
224+
* Whether opening moves focus into the modal. Set `false` for a modal the
225+
* user did not ask for, such as a prompt a web page raised: typing meant for
226+
* whatever had focus never lands in it, and the user clicks into it instead.
227+
* @default true
228+
*/
229+
focusOnOpen?: boolean
219230
children?: React.ReactNode
220231
}
221232

@@ -276,6 +287,7 @@ function ChipModal({
276287
size = 'md',
277288
className,
278289
dismissDisabled = false,
290+
focusOnOpen = true,
279291
children,
280292
'aria-describedby': ariaDescribedBy,
281293
}: ChipModalProps) {
@@ -287,7 +299,7 @@ function ChipModal({
287299
srTitle={srTitle}
288300
size={size}
289301
dismissDisabled={dismissDisabled}
290-
onOpenAutoFocus={focusChipModalDefaultAction}
302+
onOpenAutoFocus={focusOnOpen ? focusChipModalDefaultAction : keepFocusOnOpen}
291303
aria-describedby={ariaDescribedBy}
292304
>
293305
<ChipModalSurface className={className}>{children}</ChipModalSurface>

0 commit comments

Comments
 (0)