fix(desktop): harden window-open deny and cover the link-title window

Follow-ups on the cherry-picked handler:
- a throwing observer can no longer change the decision; the handler returns
  an explicit deny regardless of logging failures
- the denied-URL log carries origin only, so query tokens / signed URLs from
  attacker-controlled content never reach the persisted desktop log
- the hidden link-title window (loads arbitrary user-linked pages on render,
  had no window-open handler at all) now denies too
- tests trimmed to two invariants (proven red against the pre-fix shape)

apps/desktop/electron/link-title-window.test.ts

@@ -10,13 +10,16 @@ import {
 } from './link-title-window'
 
 function makeFakeBrowserWindow() {
-  const calls = { audioMuted: [] }
+  const calls = { audioMuted: [], windowOpenHandlers: [] }
 
   const FakeBrowserWindow = function (options) {
     this.options = options
     this.webContents = {
       setAudioMuted(value) {
         calls.audioMuted.push(value)
+      },
+      setWindowOpenHandler(handler) {
+        calls.windowOpenHandlers.push(handler)
       }
     }
   }
@@ -45,6 +48,9 @@ test('createLinkTitleWindow mutes audio so historical links never autoplay sound
 
   assert.ok(window instanceof FakeBrowserWindow)
   assert.deepEqual(calls.audioMuted, [true])
+  // GHSA-9f4c-93c8-jc8g: a page loaded for its title must not be able to pop a window.
+  assert.equal(calls.windowOpenHandlers.length, 1)
+  assert.deepEqual(calls.windowOpenHandlers[0]({ url: 'https://attacker.test/popup' }), { action: 'deny' })
 })
 
 test('createLinkTitleWindow still returns the window if muting throws', () => {

apps/desktop/electron/link-title-window.ts

@@ -3,6 +3,8 @@
 // in an offscreen window and read its title. That window loads arbitrary
 // user-linked pages, so it must never emit sound or trigger real downloads.
 
+import { createWindowOpenHandler } from './window-open-policy'
+
 export function linkTitleWindowOptions(partitionSession) {
   return {
     show: false,
@@ -34,6 +36,9 @@ export function createLinkTitleWindow(BrowserWindow, partitionSession) {
 
   try {
     window.webContents.setAudioMuted(true)
+    // Loads arbitrary user-linked pages on render; it only needs the title, so
+    // a popup from that page never has a reason to exist (GHSA-9f4c-93c8-jc8g).
+    window.webContents.setWindowOpenHandler(createWindowOpenHandler())
   } catch {
     // webContents may be unavailable in degraded/headless environments; muting
     // is best-effort and the window is destroyed within a few seconds anyway.

apps/desktop/electron/main.ts

@@ -13243,16 +13243,10 @@ function wireCommonWindowHandlers(win, { zoom = true }: { zoom?: boolean } = {})
   }
 
   installContextMenuBridge(win)
-  // Deny every window-open request and NEVER open a URL as a side effect here.
-  // Trusted external links go through the audited `hermes:openExternal` IPC
-  // channel; the only content that reaches this handler is what we did not
-  // initiate — including untrusted HTML in sandboxed `allow-scripts` iframes.
-  // Opening `details.url` here is the GHSA-9f4c-93c8-jc8g (CVE-2026-70608)
-  // vector: a sandboxed iframe with no `allow-popups` and no user gesture can
-  // force the OS browser to an attacker URL. No fixed 40.x Electron exists, so
-  // we close it at the seam. See electron/window-open-policy.ts.
+  // Always deny, never open as a side effect: GHSA-9f4c-93c8-jc8g. Trusted
+  // links arrive via `hermes:openExternal`, not here. See window-open-policy.ts.
   win.webContents.setWindowOpenHandler(
-    createWindowOpenHandler(url => rememberLog(`[window-open] denied: ${url}`))
+    createWindowOpenHandler(origin => rememberLog(`[window-open] denied: ${origin}`))
   )
   win.webContents.on('will-navigate', (event, url) => {
     if ((DEV_SERVER && url.startsWith(DEV_SERVER)) || (!DEV_SERVER && url.startsWith('file:'))) {

apps/desktop/electron/window-open-policy.ts

@@ -1,22 +1,19 @@
 /**
  * Window-open policy for every BrowserWindow's webContents.
  *
- * In the Electron desktop app, every external URL we open on purpose is routed
- * through the audited `hermes:openExternal` IPC channel (see `openExternalUrl`
- * in main.ts, which enforces an http/https/mailto scheme allowlist and guards
- * file: through the IPC path resolver). The `window.open` / `target=_blank`
- * path that reaches `setWindowOpenHandler` is therefore, inside Electron, only
- * ever driven by content we did NOT initiate — most dangerously untrusted HTML
- * rendered in sandboxed `allow-scripts` iframes (artifact previews).
+ * Every external URL the desktop opens on purpose goes through the audited
+ * `hermes:openExternal` IPC channel (`openExternalUrl` in main.ts: http/https/
+ * mailto allowlist, guarded file:). The `window.open` / `target=_blank` path
+ * that reaches `setWindowOpenHandler` is therefore only ever driven by content
+ * we did NOT initiate — most dangerously untrusted HTML in sandboxed
+ * `allow-scripts` iframes (artifact previews, inline preview directives).
  *
- * GHSA-9f4c-93c8-jc8g (CVE-2026-70608, High 7.2) lets such a sandboxed iframe
- * reach this handler with NO user interaction and WITHOUT `allow-popups`. If
- * the handler opens `details.url` as a side effect, a malicious artifact can
- * force the user's real browser to an attacker-chosen URL. There is no fixed
- * 40.x Electron release (the fix is 41.10.3+/42.0.1), so we defend at the seam
- * regardless of Electron version: deny every window-open request and NEVER open
- * a URL as a side effect here. Trusted opens keep working because they go
- * through the IPC channel, not this handler.
+ * GHSA-9f4c-93c8-jc8g (CVE-2026-70608): a sandboxed iframe without
+ * `allow-popups` and without a user gesture can still reach this handler via
+ * the OpenURL navigation path. If the handler opens `details.url` as a side
+ * effect, a malicious artifact forces the user's OS browser to an attacker URL.
+ * There is no fixed Electron 40.x, so the defence lives here regardless of the
+ * pin: deny every request and never open a URL from this handler.
  */
 
 export interface WindowOpenRequestLike {
@@ -28,28 +25,34 @@ export interface WindowOpenDecision {
 }
 
 /**
- * The security decision for a window-open request. Always deny — see the module
- * comment. Kept as a named function so the contract has one tested home and a
- * future edit that tries to reintroduce conditional opening has to defeat the
- * test rather than silently succeed.
+ * `origin` only — a denied URL can carry query credentials, signed-URL tokens
+ * or attacker-controlled text, none of which belongs in a persisted log.
  */
-export function decideWindowOpen(_request: WindowOpenRequestLike): WindowOpenDecision {
-  return { action: 'deny' }
+export function describeDeniedUrl(url: string): string {
+  try {
+    const parsed = new URL(url)
+
+    return parsed.origin === 'null' ? parsed.protocol : parsed.origin
+  } catch {
+    return '<unparseable>'
+  }
 }
 
 /**
- * Build a `setWindowOpenHandler` callback. It denies unconditionally and never
- * opens anything. `onDenied` is an optional observability hook (logging only);
- * it MUST NOT open a URL — doing so would re-open the CVE this handler closes.
+ * Build a `setWindowOpenHandler` callback that denies unconditionally.
+ * `onDenied` is logging-only and receives the sanitized origin; a throwing
+ * observer must not be able to change the decision.
  */
 export function createWindowOpenHandler(
-  onDenied?: (url: string) => void
+  onDenied?: (origin: string) => void
 ): (details: WindowOpenRequestLike) => WindowOpenDecision {
   return details => {
-    if (onDenied) {
-      onDenied(details.url)
+    try {
+      onDenied?.(describeDeniedUrl(details.url))
+    } catch {
+      // observer failure is not a reason to reconsider the decision
     }
 
-    return decideWindowOpen(details)
+    return { action: 'deny' }
   }
 }

tests-js/window-open-policy.test.ts

@@ -1,67 +1,46 @@
 /**
- * Security regression for GHSA-9f4c-93c8-jc8g (CVE-2026-70608, High 7.2):
- * "Sandboxed iframe can bypass the allow-popups restriction via the OpenURL
- * navigation path."
- *
- * A sandboxed iframe without `allow-popups` can reach a window's
- * `setWindowOpenHandler` with no user gesture. The desktop app renders
- * untrusted artifact HTML in `<iframe sandbox="allow-scripts">`, so if the
- * handler opens `details.url` as a side effect, a malicious artifact can force
- * the OS browser to an attacker URL. Electron has no fixed 40.x release, so the
- * defense lives in our handler: it must ALWAYS deny and must NEVER open a URL.
- *
- * These import the real policy module (pure, dependency-free) so the contract is
- * tested against the code main.ts actually wires in — a future edit that
- * reintroduces conditional opening has to defeat these tests.
+ * Security regression for GHSA-9f4c-93c8-jc8g (CVE-2026-70608): a sandboxed
+ * iframe without `allow-popups` can reach `setWindowOpenHandler` with no user
+ * gesture, so the handler must always deny and must never open a URL. These
+ * import the real policy module main.ts wires in.
  */
 
 import assert from 'node:assert/strict'
 
 import { describe, test } from 'vitest'
 
-import {
-  createWindowOpenHandler,
-  decideWindowOpen
-} from '../apps/desktop/electron/window-open-policy'
+import { createWindowOpenHandler, describeDeniedUrl } from '../apps/desktop/electron/window-open-policy'
 
 describe('window-open policy (GHSA-9f4c-93c8-jc8g)', () => {
-  test('decideWindowOpen always denies, regardless of URL', () => {
-    for (const url of [
-      'https://example.com',
-      'http://attacker.test/steal',
+  test('denies every scheme and reports only the sanitized origin', () => {
+    const seen: string[] = []
+    const handler = createWindowOpenHandler(origin => seen.push(origin))
+
+    const urls = [
+      'https://attacker.test/steal?token=SECRET#frag',
+      'http://attacker.test:8080/x',
       'file:///etc/passwd',
       'javascript:alert(1)',
       'custom-proto://payload',
       ''
-    ]) {
-      assert.deepEqual(decideWindowOpen({ url }), { action: 'deny' })
+    ]
+
+    for (const url of urls) {
+      assert.deepEqual(handler({ url }), { action: 'deny' })
     }
+
+    assert.equal(seen.length, urls.length)
+    assert.equal(seen[0], 'https://attacker.test')
+    assert.equal(seen[1], 'http://attacker.test:8080')
+    assert.equal(describeDeniedUrl(''), '<unparseable>')
+    assert.ok(seen.every(origin => !origin.includes('SECRET') && !origin.includes('/steal')))
   })
 
-  test('handler denies and never returns an allow action', () => {
-    const handler = createWindowOpenHandler()
-    const result = handler({ url: 'https://attacker.test/popup' })
-    assert.equal(result.action, 'deny')
-  })
-
-  test('handler surfaces the denied URL to the observability hook only', () => {
-    const denied: string[] = []
-    const handler = createWindowOpenHandler(url => denied.push(url))
-
-    const result = handler({ url: 'https://attacker.test/x' })
-
-    assert.equal(result.action, 'deny')
-    assert.deepEqual(denied, ['https://attacker.test/x'])
-  })
-
-  test('a throwing observability hook does not turn a deny into an allow', () => {
+  test('a throwing observer still yields an explicit deny', () => {
     const handler = createWindowOpenHandler(() => {
       throw new Error('logging blew up')
     })
-    // The hook is logging-only; even if it throws, the security decision must
-    // not silently become "allow". We assert the throw propagates rather than
-    // being swallowed into an open — the caller (Electron) treats a throw as
-    // deny, and crucially no URL was opened as a side effect.
-    assert.throws(() => handler({ url: 'https://attacker.test/x' }))
+
+    assert.deepEqual(handler({ url: 'https://attacker.test/x' }), { action: 'deny' })
   })
 })