fix(desktop): hand preview guest links to the audited opener via a guest preload
Preview-pane guest pages (Streamlit's traceback "Ask Google" / "Ask …" buttons, plain `<a target="_blank">` anchors) could not open anything: the `<webview>` has no `allowpopups`, so Chromium drops the popup before any handler runs. Opening from `setWindowOpenHandler` is banned (GHSA-9f4c-93c8-jc8g, window-open-policy.ts), so this adds an explicit click bridge instead: - main.ts installs a guest preload through `will-attach-webview`, keyed on the `persist:hermes-preview` partition only. - The preload forwards a clicked `_blank` anchor's resolved href to the host renderer via `ipcRenderer.sendToHost`; it opens nothing itself. - PreviewPane admits the URL and routes it through the existing audited `hermes:openExternal` IPC. Salvaged from PR #112959 (squash of its two commits). Fixes #112941
This commit is contained in:
@@ -537,6 +537,7 @@ let f12Blocked = false
|
||||
// ESM loader is broken on Electron 40's Node (ERR_INVALID_RETURN_PROPERTY_VALUE).
|
||||
// Dev (`npm run dev`) and prod both load the esbuild output from dist/.
|
||||
const PRELOAD_PATH = path.join(APP_ROOT, 'dist', 'electron-preload.js')
|
||||
const PREVIEW_GUEST_PRELOAD_PATH = path.join(APP_ROOT, 'dist', 'preview-guest-preload.js')
|
||||
|
||||
// Remote displays (SSH X11 forwarding, VNC, RDP) make Chromium's GPU
|
||||
// compositor flicker — accelerated layers can't be presented cleanly over the
|
||||
@@ -13631,6 +13632,37 @@ function wireCommonWindowHandlers(win, { zoom = true }: { zoom?: boolean } = {})
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Give the preview pane's `<webview>` guests a preload — and ONLY those
|
||||
* guests. The pane's webview is the one `webview` tag in the app and it
|
||||
* always carries the `persist:hermes-preview` partition, so the partition is
|
||||
* the ownership key: any future webview that does not opt into that partition
|
||||
* inherits nothing from this mechanism.
|
||||
*
|
||||
* The preload (preview-guest-preload-entry.ts) never opens anything itself.
|
||||
* It forwards a clicked `_blank` anchor to the host renderer via
|
||||
* `sendToHost`, and the pane admits the scheme and routes the URL through the
|
||||
* audited `hermes:openExternal` channel. Popup requests themselves stay
|
||||
* denied-by-omission: the webview has no `allowpopups`, and the
|
||||
* `setWindowOpenHandler` contract (GHSA-9f4c-93c8-jc8g) stays side-effect
|
||||
* free.
|
||||
*/
|
||||
function installPreviewGuestPreload() {
|
||||
app.on('web-contents-created', (_event, contents) => {
|
||||
if (contents.getType() !== 'window') {
|
||||
return
|
||||
}
|
||||
|
||||
contents.on('will-attach-webview', (_attachEvent, webPreferences, params) => {
|
||||
if (params.partition !== 'persist:hermes-preview') {
|
||||
return
|
||||
}
|
||||
|
||||
webPreferences.preload = PREVIEW_GUEST_PRELOAD_PATH
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
// Every window we open starts with `show: false` so the renderer's first themed
|
||||
// paint lands before it appears, and `ready-to-show` is what reveals it.
|
||||
// Electron 40 can drop that event entirely (electron/electron#51972) on
|
||||
@@ -18419,6 +18451,7 @@ app.whenReady().then(() => {
|
||||
installEmbedReferer()
|
||||
installRemoteHeaderRules()
|
||||
registerDeepLinkProtocol()
|
||||
installPreviewGuestPreload()
|
||||
|
||||
ensureWslWindowsFonts()
|
||||
configureSpellChecker()
|
||||
|
||||
26
apps/desktop/electron/preview-guest-preload-entry.ts
Normal file
26
apps/desktop/electron/preview-guest-preload-entry.ts
Normal file
@@ -0,0 +1,26 @@
|
||||
// Preload for the preview pane's `<webview>` guests. main.ts installs this
|
||||
// file via `will-attach-webview` on the `persist:hermes-preview` partition
|
||||
// only (see `installPreviewGuestPreload`), so no other webview inherits it.
|
||||
//
|
||||
// The guest runs with contextIsolation, so this preload shares the guest's
|
||||
// DOM but never its JavaScript world. A preview page's `target="_blank"`
|
||||
// anchors (Streamlit traceback's "Ask Google" / "Ask …" buttons —
|
||||
// #112941) are intercepted here in the DOM's capture phase and handed to the
|
||||
// host renderer via `sendToHost`; the host admits the scheme and routes the
|
||||
// URL through the audited `hermes:openExternal` channel. A guest URL never
|
||||
// becomes an Electron popup and this side never opens anything by itself.
|
||||
//
|
||||
// Deliberate scope: only anchor clicks are forwarded. A page's direct
|
||||
// `window.open` calls stay blocked (the webview has no `allowpopups`), which
|
||||
// keeps the pre-fix default for script-driven popups.
|
||||
|
||||
import { installGuestExternalHandoff } from './preview-guest-preload'
|
||||
|
||||
const electron = require('electron') as {
|
||||
ipcRenderer: { sendToHost(channel: string, ...args: unknown[]): void }
|
||||
}
|
||||
|
||||
installGuestExternalHandoff({
|
||||
addEventListener: (type, listener, capture) => document.addEventListener(type, listener as EventListener, capture),
|
||||
sendToHost: (channel, ...args) => electron.ipcRenderer.sendToHost(channel, ...args)
|
||||
})
|
||||
64
apps/desktop/electron/preview-guest-preload.test.ts
Normal file
64
apps/desktop/electron/preview-guest-preload.test.ts
Normal file
@@ -0,0 +1,64 @@
|
||||
import assert from 'node:assert/strict'
|
||||
|
||||
import { describe, test } from 'vitest'
|
||||
|
||||
import { GUEST_EXTERNAL_CHANNEL, installGuestExternalHandoff } from './preview-guest-preload'
|
||||
|
||||
function rig() {
|
||||
const sent: { channel: string; args: unknown[] }[] = []
|
||||
const listeners: { type: string; listener: (event: unknown) => void; capture?: boolean }[] = []
|
||||
|
||||
const host = {
|
||||
addEventListener: (type: 'click', listener: (event: unknown) => void, capture?: boolean) =>
|
||||
listeners.push({ type, listener, capture }),
|
||||
sendToHost: (channel: string, ...args: unknown[]) => sent.push({ args, channel })
|
||||
}
|
||||
|
||||
installGuestExternalHandoff(host)
|
||||
|
||||
return { click: (target: unknown) => listeners[0].listener({ target }), listeners, sent }
|
||||
}
|
||||
|
||||
describe('installGuestExternalHandoff', () => {
|
||||
test('registers one capture-phase click listener', () => {
|
||||
const { listeners } = rig()
|
||||
|
||||
assert.equal(listeners.length, 1)
|
||||
assert.equal(listeners[0].type, 'click')
|
||||
assert.equal(listeners[0].capture, true)
|
||||
})
|
||||
|
||||
test('forwards a clicked _blank anchor as the channel message', () => {
|
||||
const { click, sent } = rig()
|
||||
|
||||
click({
|
||||
closest: (selector: string) =>
|
||||
selector === 'a[target="_blank"]' ? { href: 'https://www.google.com/search?q=traceback' } : null
|
||||
})
|
||||
|
||||
assert.deepEqual(sent, [{ args: ['https://www.google.com/search?q=traceback'], channel: GUEST_EXTERNAL_CHANNEL }])
|
||||
})
|
||||
|
||||
test('climbs from an inner element through the shared DOM', () => {
|
||||
const { click, sent } = rig()
|
||||
|
||||
// The listener resolves the enclosing anchor itself via `closest`.
|
||||
click({
|
||||
closest: (selector: string) => (selector === 'a[target="_blank"]' ? { href: 'https://chatgpt.com/?q=why' } : null)
|
||||
})
|
||||
|
||||
assert.equal(sent.length, 1)
|
||||
assert.equal(sent[0].args[0], 'https://chatgpt.com/?q=why')
|
||||
})
|
||||
|
||||
test('ignores clicks that resolve to no _blank anchor', () => {
|
||||
const { click, sent } = rig()
|
||||
|
||||
click({ closest: () => null })
|
||||
click({ closest: (selector: string) => (selector === 'a[target="_blank"]' ? { href: '' } : null) })
|
||||
click(null)
|
||||
click({})
|
||||
|
||||
assert.deepEqual(sent, [])
|
||||
})
|
||||
})
|
||||
49
apps/desktop/electron/preview-guest-preload.ts
Normal file
49
apps/desktop/electron/preview-guest-preload.ts
Normal file
@@ -0,0 +1,49 @@
|
||||
// Logic half of the preview pane's guest preload. The wiring half lives in
|
||||
// `preview-guest-preload-entry.ts` (the bundled preload itself); this split
|
||||
// keeps the handoff rules unit-testable in a plain Node environment.
|
||||
|
||||
export const GUEST_EXTERNAL_CHANNEL = 'preview-open-external'
|
||||
|
||||
interface GuestEventTarget {
|
||||
closest(selector: string): { href: string } | null
|
||||
}
|
||||
|
||||
export interface GuestHandoffHost {
|
||||
addEventListener(type: 'click', listener: (event: unknown) => void, capture?: boolean): void
|
||||
sendToHost(channel: string, ...args: unknown[]): void
|
||||
}
|
||||
|
||||
/**
|
||||
* Wire the DOM-capture listener that forwards a clicked `_blank` anchor's
|
||||
* absolute URL to the host. `host` is injected so the entry can pass the
|
||||
* guest's `document` and `ipcRenderer`, and tests can drive the same rules
|
||||
* without Electron or a DOM.
|
||||
*/
|
||||
export function installGuestExternalHandoff(host: GuestHandoffHost): void {
|
||||
host.addEventListener(
|
||||
'click',
|
||||
event => {
|
||||
const target = (event as { target?: unknown }).target as GuestEventTarget | null
|
||||
|
||||
if (!target || typeof target.closest !== 'function') {
|
||||
return
|
||||
}
|
||||
|
||||
// `closest` climbs through the click's own DOM, which this isolated
|
||||
// preload world shares with the page: an inner `<span>` inside the
|
||||
// anchor resolves to the enclosing `<a target="_blank">` the same way
|
||||
// it would for page script.
|
||||
const anchor = target.closest('a[target="_blank"]')
|
||||
|
||||
if (!anchor || typeof anchor.href !== 'string' || anchor.href === '') {
|
||||
return
|
||||
}
|
||||
|
||||
// `anchor.href` is the browser-resolved absolute URL, not the raw
|
||||
// attribute, so relative and protocol-relative hrefs arrive fully
|
||||
// qualified for the host's scheme admission check.
|
||||
host.sendToHost(GUEST_EXTERNAL_CHANNEL, anchor.href)
|
||||
},
|
||||
true
|
||||
)
|
||||
}
|
||||
29
apps/desktop/electron/window-open-policy.test.ts
Normal file
29
apps/desktop/electron/window-open-policy.test.ts
Normal file
@@ -0,0 +1,29 @@
|
||||
import assert from 'node:assert/strict'
|
||||
|
||||
import { test } from 'vitest'
|
||||
|
||||
import { createWindowOpenHandler, describeDeniedUrl } from './window-open-policy'
|
||||
|
||||
test('host handler denies every request and reports the origin only', () => {
|
||||
const seen: string[] = []
|
||||
const handler = createWindowOpenHandler(origin => seen.push(origin))
|
||||
|
||||
assert.deepEqual(handler({ url: 'https://evil.example/path?token=x' }), { action: 'deny' })
|
||||
assert.deepEqual(handler({ url: 'file:///etc/passwd' }), { action: 'deny' })
|
||||
// Full URLs (query credentials, paths) never reach the observer.
|
||||
assert.deepEqual(seen, ['https://evil.example', 'file:'])
|
||||
})
|
||||
|
||||
test('host handler keeps denying when the observer throws', () => {
|
||||
const handler = createWindowOpenHandler(() => {
|
||||
throw new Error('observer blew up')
|
||||
})
|
||||
|
||||
assert.deepEqual(handler({ url: 'https://evil.example/' }), { action: 'deny' })
|
||||
})
|
||||
|
||||
test('describeDeniedUrl sanitizes unparseable and opaque origins', () => {
|
||||
assert.equal(describeDeniedUrl('https://example.com/x?y=1'), 'https://example.com')
|
||||
assert.equal(describeDeniedUrl('data:text/html,hi'), 'data:')
|
||||
assert.equal(describeDeniedUrl('not a url'), '<unparseable>')
|
||||
})
|
||||
@@ -4,8 +4,9 @@
|
||||
// node_modules/ or tsx at runtime.
|
||||
//
|
||||
// Output:
|
||||
// dist/electron-main.mjs (MJS bundle — entry point for packaged app)
|
||||
// dist/electron-preload.js (CJS bundle — loaded via BrowserWindow preload)
|
||||
// dist/electron-main.mjs (MJS bundle — entry point for packaged app)
|
||||
// dist/electron-preload.js (CJS bundle — loaded via BrowserWindow preload)
|
||||
// dist/preview-guest-preload.js (CJS bundle — preview <webview> guest preload)
|
||||
//
|
||||
// `electron` and `node-pty` are external (provided by the runtime / staged
|
||||
// separately via stage-native-deps).
|
||||
@@ -66,3 +67,21 @@ await build({
|
||||
logLevel: 'info',
|
||||
})
|
||||
console.log(`bundled ${preloadOut}${isDev ? ' (dev)' : ''}`)
|
||||
|
||||
// Bundle preview-guest-preload-entry.ts → dist/preview-guest-preload.js
|
||||
// (main.ts hands this path to the preview webview via will-attach-webview)
|
||||
const guestPreloadEntry = resolve(root, 'electron/preview-guest-preload-entry.ts')
|
||||
const guestPreloadOut = resolve(distDir, 'preview-guest-preload.js')
|
||||
|
||||
await build({
|
||||
entryPoints: [guestPreloadEntry],
|
||||
bundle: true,
|
||||
platform: 'node',
|
||||
format: 'cjs',
|
||||
target: 'node20',
|
||||
outfile: guestPreloadOut,
|
||||
external,
|
||||
define,
|
||||
logLevel: 'info',
|
||||
})
|
||||
console.log(`bundled ${guestPreloadOut}${isDev ? ' (dev)' : ''}`)
|
||||
|
||||
@@ -671,3 +671,79 @@ describe('PreviewPane console state', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('PreviewPane guest external handoff', () => {
|
||||
const desktopWindow = window as unknown as { hermesDesktop?: Window['hermesDesktop'] }
|
||||
const initialHermesDesktop = desktopWindow.hermesDesktop
|
||||
|
||||
afterEach(() => {
|
||||
if (initialHermesDesktop) {
|
||||
desktopWindow.hermesDesktop = initialHermesDesktop
|
||||
} else {
|
||||
delete desktopWindow.hermesDesktop
|
||||
}
|
||||
})
|
||||
|
||||
async function renderWebview() {
|
||||
let rendered!: ReturnType<typeof render>
|
||||
|
||||
await act(async () => {
|
||||
rendered = render(
|
||||
<PreviewPane
|
||||
target={{
|
||||
kind: 'url',
|
||||
label: 'Preview',
|
||||
source: 'http://localhost:8501',
|
||||
url: 'http://localhost:8501'
|
||||
}}
|
||||
/>
|
||||
)
|
||||
})
|
||||
|
||||
return rendered.container.querySelector('webview') as HTMLElement
|
||||
}
|
||||
|
||||
function guestHandoff(webview: HTMLElement, url: string) {
|
||||
act(() => {
|
||||
webview.dispatchEvent(Object.assign(new Event('ipc-message'), { args: [url], channel: 'preview-open-external' }))
|
||||
})
|
||||
}
|
||||
|
||||
it('opens an admitted guest anchor URL through the audited OS-browser channel', async () => {
|
||||
const openExternal = vi.fn(async () => undefined)
|
||||
desktopWindow.hermesDesktop = { openExternal } as unknown as Window['hermesDesktop']
|
||||
|
||||
const webview = await renderWebview()
|
||||
|
||||
guestHandoff(webview, 'https://www.google.com/search?q=traceback')
|
||||
|
||||
expect(openExternal).toHaveBeenCalledExactlyOnceWith('https://www.google.com/search?q=traceback')
|
||||
})
|
||||
|
||||
it('rejects file: and javascript: guest URLs without any OS open', async () => {
|
||||
const openExternal = vi.fn(async () => undefined)
|
||||
desktopWindow.hermesDesktop = { openExternal } as unknown as Window['hermesDesktop']
|
||||
|
||||
const webview = await renderWebview()
|
||||
|
||||
guestHandoff(webview, 'file:///etc/passwd')
|
||||
guestHandoff(webview, 'javascript:alert(1)')
|
||||
|
||||
expect(openExternal).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('ignores channels the guest preload does not own', async () => {
|
||||
const openExternal = vi.fn(async () => undefined)
|
||||
desktopWindow.hermesDesktop = { openExternal } as unknown as Window['hermesDesktop']
|
||||
|
||||
const webview = await renderWebview()
|
||||
|
||||
act(() => {
|
||||
webview.dispatchEvent(
|
||||
Object.assign(new Event('ipc-message'), { args: ['https://example.com'], channel: 'something-else' })
|
||||
)
|
||||
})
|
||||
|
||||
expect(openExternal).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
endAnnotateMode,
|
||||
flushAnnotateStack
|
||||
} from '@/lib/preview-annotate'
|
||||
import { admitPreviewExternalUrl, PREVIEW_EXTERNAL_CHANNEL } from '@/lib/preview-external'
|
||||
import { reachablePreviewUrl } from '@/lib/preview-reach'
|
||||
import { rafCoalesce } from '@/lib/raf-coalesce'
|
||||
import { cn } from '@/lib/utils'
|
||||
@@ -1026,6 +1027,25 @@ export function PreviewPane({ embedded = false, onRestartServer, reloadRequest =
|
||||
webview.setAttribute('src', target.url)
|
||||
webview.setAttribute('webpreferences', 'contextIsolation=yes,nodeIntegration=no,sandbox=yes')
|
||||
|
||||
// The guest preload (main.ts installs it on this partition) forwards a
|
||||
// clicked `_blank` anchor here. Admission is our side of the contract —
|
||||
// web/mail schemes only, so a guest page can never reach the local-file
|
||||
// opener — and the open itself goes through the audited
|
||||
// `hermes:openExternal` channel, never a popup side effect.
|
||||
const onGuestExternal = (event: Event) => {
|
||||
const detail = event as Event & { args?: unknown[]; channel?: string }
|
||||
|
||||
if (detail.channel !== PREVIEW_EXTERNAL_CHANNEL) {
|
||||
return
|
||||
}
|
||||
|
||||
const url = String(detail.args?.[0] ?? '')
|
||||
|
||||
if (admitPreviewExternalUrl(url)) {
|
||||
void window.hermesDesktop?.openExternal?.(url)
|
||||
}
|
||||
}
|
||||
|
||||
const onConsole = (event: Event) => {
|
||||
const detail = event as Event & {
|
||||
level?: number
|
||||
@@ -1206,6 +1226,7 @@ export function PreviewPane({ embedded = false, onRestartServer, reloadRequest =
|
||||
}
|
||||
|
||||
webview.addEventListener('console-message', onConsole)
|
||||
webview.addEventListener('ipc-message', onGuestExternal)
|
||||
webview.addEventListener('context-menu', onGuestContextMenu)
|
||||
webview.addEventListener('devtools-closed', onDevToolsClosed)
|
||||
webview.addEventListener('devtools-opened', onDevToolsOpened)
|
||||
@@ -1223,6 +1244,7 @@ export function PreviewPane({ embedded = false, onRestartServer, reloadRequest =
|
||||
return () => {
|
||||
annotateLoopRef.current += 1
|
||||
webview.removeEventListener('console-message', onConsole)
|
||||
webview.removeEventListener('ipc-message', onGuestExternal)
|
||||
webview.removeEventListener('context-menu', onGuestContextMenu)
|
||||
webview.removeEventListener('devtools-closed', onDevToolsClosed)
|
||||
webview.removeEventListener('devtools-opened', onDevToolsOpened)
|
||||
|
||||
34
apps/desktop/src/lib/preview-external.test.ts
Normal file
34
apps/desktop/src/lib/preview-external.test.ts
Normal file
@@ -0,0 +1,34 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { admitPreviewExternalUrl, PREVIEW_EXTERNAL_CHANNEL } from './preview-external'
|
||||
|
||||
describe('admitPreviewExternalUrl', () => {
|
||||
it('admits the web and mail schemes the guest handoff exists for', () => {
|
||||
expect(admitPreviewExternalUrl('https://www.google.com/search?q=traceback')).toBe(true)
|
||||
expect(admitPreviewExternalUrl('http://localhost:8501/')).toBe(true)
|
||||
expect(admitPreviewExternalUrl('mailto:support@example.com')).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects file: — a guest page must not reach the local-file opener', () => {
|
||||
expect(admitPreviewExternalUrl('file:///etc/passwd')).toBe(false)
|
||||
expect(admitPreviewExternalUrl('FILE:///Users/me/secret.txt')).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects script-capable and opaque schemes', () => {
|
||||
expect(admitPreviewExternalUrl('javascript:alert(1)')).toBe(false)
|
||||
expect(admitPreviewExternalUrl('data:text/html,hi')).toBe(false)
|
||||
expect(admitPreviewExternalUrl('blob:https://example.com/uuid')).toBe(false)
|
||||
expect(admitPreviewExternalUrl('chrome://settings')).toBe(false)
|
||||
})
|
||||
|
||||
it('rejects unparseable and empty input', () => {
|
||||
expect(admitPreviewExternalUrl('')).toBe(false)
|
||||
expect(admitPreviewExternalUrl(' ')).toBe(false)
|
||||
expect(admitPreviewExternalUrl('not a url')).toBe(false)
|
||||
expect(admitPreviewExternalUrl('localhost:8501')).toBe(false)
|
||||
})
|
||||
|
||||
it('uses the channel the guest preload sends on', () => {
|
||||
expect(PREVIEW_EXTERNAL_CHANNEL).toBe('preview-open-external')
|
||||
})
|
||||
})
|
||||
26
apps/desktop/src/lib/preview-external.ts
Normal file
26
apps/desktop/src/lib/preview-external.ts
Normal file
@@ -0,0 +1,26 @@
|
||||
// Admission for URLs handed up by the preview guest preload
|
||||
// (electron/preview-guest-preload.ts, `preview-open-external` channel).
|
||||
//
|
||||
// A guest page chose this URL, not the user typing it, so the scheme set is
|
||||
// strictly the web/mail subset of what main's `openExternalUrl` accepts.
|
||||
// `file:` stays out on purpose: main's opener does open local files, but that
|
||||
// capability belongs to host-driven surfaces (the artifacts panel), and an
|
||||
// untrusted preview page must never reach `shell.openPath` through us.
|
||||
|
||||
export const PREVIEW_EXTERNAL_CHANNEL = 'preview-open-external'
|
||||
|
||||
export function admitPreviewExternalUrl(rawUrl: string): boolean {
|
||||
const raw = String(rawUrl ?? '').trim()
|
||||
|
||||
if (!raw) {
|
||||
return false
|
||||
}
|
||||
|
||||
try {
|
||||
const { protocol } = new URL(raw)
|
||||
|
||||
return protocol === 'https:' || protocol === 'http:' || protocol === 'mailto:'
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user