fix(browser): open target=_blank file links from a local page in an Orca tab (#34057)

This commit is contained in:
Neil
2026-10-26 22:42:19 -07:00
committed by GitHub
parent 1fdf89685e
commit 6ae3fbc89b
5 changed files with 57 additions and 11 deletions
@@ -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)
@@ -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:')
}
@@ -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,
@@ -77,10 +77,11 @@ function popupContents(opener: Pick<Electron.WebFrameMain, 'frameTreeNodeId'> |
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'
})
})
})
@@ -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) {