diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index b20e99213c..592e9f43ff 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -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 `` 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() diff --git a/apps/desktop/electron/preview-guest-preload-entry.ts b/apps/desktop/electron/preview-guest-preload-entry.ts new file mode 100644 index 0000000000..ddeb120350 --- /dev/null +++ b/apps/desktop/electron/preview-guest-preload-entry.ts @@ -0,0 +1,26 @@ +// Preload for the preview pane's `` 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) +}) diff --git a/apps/desktop/electron/preview-guest-preload.test.ts b/apps/desktop/electron/preview-guest-preload.test.ts new file mode 100644 index 0000000000..136e9c3e5c --- /dev/null +++ b/apps/desktop/electron/preview-guest-preload.test.ts @@ -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, []) + }) +}) diff --git a/apps/desktop/electron/preview-guest-preload.ts b/apps/desktop/electron/preview-guest-preload.ts new file mode 100644 index 0000000000..672ac25e7a --- /dev/null +++ b/apps/desktop/electron/preview-guest-preload.ts @@ -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 `` inside the + // anchor resolves to the enclosing `` 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 + ) +} diff --git a/apps/desktop/electron/window-open-policy.test.ts b/apps/desktop/electron/window-open-policy.test.ts new file mode 100644 index 0000000000..0edbd54b0d --- /dev/null +++ b/apps/desktop/electron/window-open-policy.test.ts @@ -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'), '') +}) diff --git a/apps/desktop/scripts/bundle-electron-main.mjs b/apps/desktop/scripts/bundle-electron-main.mjs index b613b98ca2..a119f4a9d4 100644 --- a/apps/desktop/scripts/bundle-electron-main.mjs +++ b/apps/desktop/scripts/bundle-electron-main.mjs @@ -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 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)' : ''}`) diff --git a/apps/desktop/src/app/chat/right-rail/preview-pane.test.tsx b/apps/desktop/src/app/chat/right-rail/preview-pane.test.tsx index 77bc58bfe7..cc222db5f8 100644 --- a/apps/desktop/src/app/chat/right-rail/preview-pane.test.tsx +++ b/apps/desktop/src/app/chat/right-rail/preview-pane.test.tsx @@ -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 + + await act(async () => { + rendered = render( + + ) + }) + + 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() + }) +}) diff --git a/apps/desktop/src/app/chat/right-rail/preview-pane.tsx b/apps/desktop/src/app/chat/right-rail/preview-pane.tsx index 14c74432b7..a3d6fb7511 100644 --- a/apps/desktop/src/app/chat/right-rail/preview-pane.tsx +++ b/apps/desktop/src/app/chat/right-rail/preview-pane.tsx @@ -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) diff --git a/apps/desktop/src/lib/preview-external.test.ts b/apps/desktop/src/lib/preview-external.test.ts new file mode 100644 index 0000000000..6a87e6b652 --- /dev/null +++ b/apps/desktop/src/lib/preview-external.test.ts @@ -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') + }) +}) diff --git a/apps/desktop/src/lib/preview-external.ts b/apps/desktop/src/lib/preview-external.ts new file mode 100644 index 0000000000..393f189265 --- /dev/null +++ b/apps/desktop/src/lib/preview-external.ts @@ -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 + } +}