diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 3bf6d63051af..d0bf5b91f258 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -58,6 +58,50 @@ describe("isPreviewRefreshShortcut", () => { }); }); +describe("previewWindowOpenAction", () => { + const details = (overrides: { + readonly url?: string; + readonly disposition?: Electron.HandlerDetails["disposition"]; + }) => ({ + url: "https://accounts.google.com/o/oauth2/auth", + disposition: "new-window" as Electron.HandlerDetails["disposition"], + ...overrides, + }); + + it("opens a real window for scripted popups so the opener survives", () => { + // OAuth SDKs read a null `window.open()` as a blocked popup, and they need + // the opener alive to receive the credential back. + expect(PreviewManager.previewWindowOpenAction(details({}))).toBe("popup"); + expect( + PreviewManager.previewWindowOpenAction(details({ url: "http://localhost:5173/auth" })), + ).toBe("popup"); + }); + + it("keeps target=_blank links in the preview tab", () => { + expect( + PreviewManager.previewWindowOpenAction(details({ disposition: "foreground-tab" })), + ).toBe("navigate"); + expect( + PreviewManager.previewWindowOpenAction(details({ disposition: "background-tab" })), + ).toBe("navigate"); + }); + + it("does not hand a window to schemes that cannot be hardened", () => { + // A popup skips the `will-attach-webview` hardening, so it only gets a window + // when its preferences can be overridden. Chromium copies the guest's + // preferences for `about:blank` and forbids overriding them. + for (const url of [ + "about:blank", + "javascript:alert(1)", + "file:///etc/passwd", + "vscode://vscode-remote/ssh-remote+box/tmp", + "not a url", + ]) { + expect(PreviewManager.previewWindowOpenAction(details({ url }))).toBe("navigate"); + } + }); +}); + const { browserWindowConstructor, createFromPath, diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index 0d90e0175fe3..c1adbe7df774 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -434,6 +434,61 @@ const APP_FORWARDED_SHORTCUTS: ReadonlyArray<{ { key: "w", meta: true, shift: false, control: false }, ]); +/** + * Protocols a preview page may open in a real popup window. + * + * `about:blank` stays out: Chromium skips browser-side navigation for it, so the + * child copies the guest's `contextIsolation: false` preferences and Electron + * gives no way to override them. Those popups keep loading in the preview tab. + * + * Deliberately not `ElectronShell.parseSafeExternalUrl`: that also admits + * `vscode://vscode-remote/...` deep links, which belong in `shell.openExternal` + * and not in a window spawned by a third-party page in the preview. + */ +const POPUP_PROTOCOLS = new Set(["http:", "https:"]); + +const isPopupUrl = (rawUrl: string): boolean => { + try { + return POPUP_PROTOCOLS.has(new URL(rawUrl).protocol); + } catch { + return false; + } +}; + +/** + * Preferences for a popup a preview page opens. + * + * A popup is not a webview attach, so the `will-attach-webview` hardening in + * `DesktopWindow` never sees it, and an unoverridden child would inherit the + * guest's relaxed posture: the picker preload needs `contextIsolation: false` + * to share `globalThis` with the previewed page, and no OAuth provider should + * get that. The window keeps the opener and the guest session either way. + */ +const POPUP_WINDOW_OPTIONS = { + webPreferences: { + contextIsolation: true, + nodeIntegration: false, + sandbox: true, + }, +} satisfies Electron.BrowserWindowConstructorOptions; + +/** + * Decides what a preview page's `window.open` should do. + * + * `"popup"` opens a real window, which scripted popups need: denying them makes + * `window.open()` return `null` (OAuth SDKs report that as a blocked popup), and + * navigating the preview tab instead destroys the opener the popup has to + * `postMessage` its result back to. + * + * `target="_blank"` links arrive as a tab disposition and keep loading in the + * preview tab, which is what people expect from a link inside a preview. + */ +export const previewWindowOpenAction = (details: { + readonly url: string; + readonly disposition: Electron.HandlerDetails["disposition"]; +}): "popup" | "navigate" => + details.disposition === "new-window" && isPopupUrl(details.url) ? "popup" : "navigate"; + export const isPreviewRefreshShortcut = (input: Electron.Input): boolean => input.type === "keyDown" && input.key.toLowerCase() === "r" && @@ -1661,6 +1716,12 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ], }); }); + // A popup opens with Electron's default handler, so the page inside it could + // otherwise spawn native windows without limit. Nothing in an OAuth flow + // opens a second popup, so the chain stops at the first one. + const windowCreated = (window: Electron.BrowserWindow): void => { + window.webContents.setWindowOpenHandler(() => ({ action: "deny" })); + }; const beforeInput = (event: Electron.Event, input: Electron.Input): void => { if (isPreviewRefreshShortcut(input)) { event.preventDefault(); @@ -1686,6 +1747,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function wc.off("did-stop-loading", sync); wc.off("did-fail-load", failed as never); wc.off("audio-state-changed", audioStateChanged); + wc.off("did-create-window", windowCreated); wc.off("before-input-event", beforeInput); wc.ipc.off(HUMAN_INPUT_CHANNEL, humanInput); wc.ipc.off(MOUSE_NAVIGATE_CHANNEL, mouseNavigate); @@ -1704,14 +1766,18 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function wc.on("audio-state-changed", audioStateChanged); wc.ipc.on(HUMAN_INPUT_CHANNEL, humanInput); wc.ipc.on(MOUSE_NAVIGATE_CHANNEL, mouseNavigate); - wc.setWindowOpenHandler(({ url }) => { + wc.setWindowOpenHandler((details) => { + if (previewWindowOpenAction(details) === "popup") { + return { action: "allow", overrideBrowserWindowOptions: POPUP_WINDOW_OPTIONS }; + } runFork( attemptPromise({ operation: "openPreviewWindow", tabId, webContentsId: wc.id }, () => - wc.loadURL(url), + wc.loadURL(details.url), ).pipe(Effect.ignore), ); return { action: "deny" }; }); + wc.on("did-create-window", windowCreated); wc.on("before-input-event", beforeInput); }); yield* Ref.update(attachedRef, (attached) => diff --git a/apps/web/src/browser/HostedBrowserWebview.tsx b/apps/web/src/browser/HostedBrowserWebview.tsx index ae0526abb15f..6b85ebc829ee 100644 --- a/apps/web/src/browser/HostedBrowserWebview.tsx +++ b/apps/web/src/browser/HostedBrowserWebview.tsx @@ -92,7 +92,6 @@ export function HostedBrowserWebview(props: { const setWebviewRef = useCallback((node: HTMLElement | null) => { webviewRef.current = node as ElectronWebview | null; - if (node && !node.hasAttribute("allowpopups")) node.setAttribute("allowpopups", "true"); }, []); useEffect(() => { @@ -259,6 +258,10 @@ export function HostedBrowserWebview(props: {