From 6ae3fbc89b9f2fa4b217a29929fe5772fa97cb11 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:42:19 -0700 Subject: [PATCH] fix(browser): open target=_blank file links from a local page in an Orca tab (#34057) --- ...browser-manager-guest-navigation-policy.ts | 8 ++-- .../browser-manager-guest-popup-policy.ts | 14 +++++-- .../browser-manager-popup-child.test.ts | 3 +- ...owser-manager-popup-openerless-tab.test.ts | 37 ++++++++++++++++++- .../browser-manager-popup-routing.test.ts | 6 ++- 5 files changed, 57 insertions(+), 11 deletions(-) diff --git a/src/main/browser/browser-manager-guest-navigation-policy.ts b/src/main/browser/browser-manager-guest-navigation-policy.ts index f9220917af4..5ea8306881e 100644 --- a/src/main/browser/browser-manager-guest-navigation-policy.ts +++ b/src/main/browser/browser-manager-guest-navigation-policy.ts @@ -3,7 +3,10 @@ import { toSecureCertificateEndpoint } from '../../shared/browser-url' import { isChromiumInternalErrorUrl } from './browser-manager-types' -import { BrowserManagerGuestPopupPolicy } from './browser-manager-guest-popup-policy' +import { + BrowserManagerGuestPopupPolicy, + isLocalFileLinkFromLocalPage +} from './browser-manager-guest-popup-policy' import { allowPageUnload } from './browser-guest-unload-policy' import { initiatingFrameUrl } from './browser-external-app-launch' @@ -32,8 +35,7 @@ export abstract class BrowserManagerGuestNavigationPolicy extends BrowserManager } const willNavigateHandler = (event: Electron.Event, url: string): void => { - // Permit local file links without allowing web pages into the filesystem. - if (url.startsWith('file:') && guest.getURL().startsWith('file:')) { + if (isLocalFileLinkFromLocalPage(guest, url)) { return } navigationGuard(event, url) diff --git a/src/main/browser/browser-manager-guest-popup-policy.ts b/src/main/browser/browser-manager-guest-popup-policy.ts index 5d031b6de94..3a9207d79b5 100644 --- a/src/main/browser/browser-manager-guest-popup-policy.ts +++ b/src/main/browser/browser-manager-guest-popup-policy.ts @@ -33,16 +33,17 @@ export abstract class BrowserManagerGuestPopupPolicy extends BrowserManagerNavig const ownerContext = this.resolvePopupOwnerContext(guest.id) const browserTabId = ownerContext?.browserTabId ?? null const externalUrl = normalizeExternalBrowserUrl(url) + const newTabUrl = externalUrl ?? (isLocalFileLinkFromLocalPage(guest, url) ? url : null) // Opener-dependent requests retain a child window instead of becoming tabs. if ( ownerContext && - externalUrl && + newTabUrl && isNewBrowserTabPopupIntent({ url, frameName, disposition, postBody }) ) { // Scripted opens persist as tabs, so opens without observed input consume a budget. if (!clicked && !this.tryConsumePageInitiatedTab(ownerContext.rootGuestWebContentsId)) { this.forwardOrQueuePopupEvent(guest.id, { - origin: safeOrigin(externalUrl), + origin: safeOrigin(newTabUrl), action: 'blocked' }) return { action: 'deny' } @@ -50,12 +51,12 @@ export abstract class BrowserManagerGuestPopupPolicy extends BrowserManagerNavig if ( this.openLinkInOrcaTab( ownerContext.browserTabId, - externalUrl, + newTabUrl, disposition !== 'background-tab' ) ) { this.forwardOrQueuePopupEvent(guest.id, { - origin: safeOrigin(externalUrl), + origin: safeOrigin(newTabUrl), action: 'opened-in-orca' }) } @@ -169,3 +170,8 @@ export abstract class BrowserManagerGuestPopupPolicy extends BrowserManagerNavig return openPopupWithOriginBar(options, url, () => false).contentWebContents } } + +// Web pages must not reach the filesystem; only a local page may link to local files. +export function isLocalFileLinkFromLocalPage(guest: Electron.WebContents, url: string): boolean { + return url.startsWith('file:') && guest.getURL().startsWith('file:') +} diff --git a/src/main/browser/browser-manager-popup-child.test.ts b/src/main/browser/browser-manager-popup-child.test.ts index 9807e9c3e71..a31fd7c3266 100644 --- a/src/main/browser/browser-manager-popup-child.test.ts +++ b/src/main/browser/browser-manager-popup-child.test.ts @@ -74,7 +74,8 @@ describe('browserManager popup child policies', () => { setWindowOpenHandler: childSetWindowOpenHandlerMock, on: childOnMock, off: childOffMock, - openDevTools: childOpenDevToolsMock + openDevTools: childOpenDevToolsMock, + getURL: vi.fn(() => 'https://identity.example.com/login') } const guest = { id: 404, diff --git a/src/main/browser/browser-manager-popup-openerless-tab.test.ts b/src/main/browser/browser-manager-popup-openerless-tab.test.ts index 2708af73b55..d2a2e7fa15a 100644 --- a/src/main/browser/browser-manager-popup-openerless-tab.test.ts +++ b/src/main/browser/browser-manager-popup-openerless-tab.test.ts @@ -77,10 +77,11 @@ function popupContents(opener: Pick | const guestSession = { marker: 'synthetic-opener-session' } const rendererSendMock = vi.fn() -function attachClickedGuest(): WindowOpenHandler { +function attachClickedGuest(pageUrl = 'https://app.example.com/'): WindowOpenHandler { const guest = { id: 2200, session: guestSession, + getURL: vi.fn(() => pageUrl), isDestroyed: vi.fn(() => false), isCurrentlyAudible: vi.fn(() => false), getType: vi.fn(() => 'webview'), @@ -234,3 +235,37 @@ describe('tab-shaped OAuth opens without an opener', () => { }) }) }) + +describe('target=_blank links to local files', () => { + const link = { frameName: '', features: '', disposition: 'foreground-tab' } + + beforeEach(() => { + resetBrowserManagerMocks(browserMocks) + resetBrowserManagerState() + rendererSendMock.mockReset() + }) + + it('opens them in an Orca tab from a local page', () => { + const handler = attachClickedGuest('file:///tmp/site/index.html') + + expect(handler({ url: 'file:///tmp/site/detail.html#pain', ...link })).toEqual({ + action: 'deny' + }) + expect(routedTabs()).toEqual([ + { browserPageId: 'browser-1', url: 'file:///tmp/site/detail.html#pain' } + ]) + }) + + it('keeps web pages out of the filesystem', () => { + const handler = attachClickedGuest('https://example.com/') + + expect(handler({ url: 'file:///etc/passwd', ...link })).toEqual({ action: 'deny' }) + expect(routedTabs()).toEqual([]) + expect(openPopupWithOriginBarMock).not.toHaveBeenCalled() + expect(rendererSendMock).toHaveBeenCalledWith('browser:popup', { + browserPageId: 'browser-1', + origin: 'null', + action: 'blocked' + }) + }) +}) diff --git a/src/main/browser/browser-manager-popup-routing.test.ts b/src/main/browser/browser-manager-popup-routing.test.ts index aa79cae268f..d0d1c5eb479 100644 --- a/src/main/browser/browser-manager-popup-routing.test.ts +++ b/src/main/browser/browser-manager-popup-routing.test.ts @@ -83,7 +83,8 @@ describe('browserManager', () => { on: guestOnMock, prependListener: vi.fn(), off: guestOffMock, - openDevTools: guestOpenDevToolsMock + openDevTools: guestOpenDevToolsMock, + getURL: vi.fn(() => 'https://example.com') } webContentsFromIdMock.mockReturnValue(guest) @@ -689,7 +690,8 @@ describe('browserManager', () => { on: guestOnMock, prependListener: vi.fn(), off: guestOffMock, - openDevTools: guestOpenDevToolsMock + openDevTools: guestOpenDevToolsMock, + getURL: vi.fn(() => 'https://example.com') } webContentsFromIdMock.mockImplementation((id: number) => { if (id === guest.id) {