From 9e290cb4943bfa712344d686e94bab5e5f8fca18 Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 24 Oct 2026 17:40:56 -0500 Subject: [PATCH 1/3] fix(desktop): preview popups can no longer load file:, data: or javascript: URLs A page's window.open with a local or inline URL took the in-place path, which calls loadURL from the main process. That browser-initiated load reaches documents Chromium blocks the page from opening itself, and a javascript: URL from any frame ran in the top page. Deny those schemes. Co-Authored-By: Claude Opus 5.5 --- apps/desktop/src/preview/Manager.test.ts | 21 ++++++++++++--- apps/desktop/src/preview/Manager.ts | 33 ++++++++++++++++++------ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 6ded53bfaf1ba..563b7950a7ce1 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -104,15 +104,30 @@ describe("previewWindowOpenAction", () => { // when its preferences can be overridden. Chromium copies the guest's // preferences for `about:blank` and forbids overriding them. for (const url of [ - "javascript:alert(1)", - "file:///etc/passwd", "vscode://vscode-remote/ssh-remote+box/tmp", - "not a url", + "blob:https://example.com/0b6c6a5e-55d4-4b4c-9d47-8f1b0c1d2e3f", ]) { expect(PreviewManager.previewWindowOpenAction(details({ url }))).toBe("navigate"); } }); + it("denies local and inline schemes that no page could open in place itself", () => { + // `loadURL` is a browser-initiated navigation, which Chromium lets reach these + // even though it blocks a page from opening them as a top-level document. + for (const url of [ + "file:///etc/passwd", + "data:text/html,

hi

", + "javascript:alert(1)", + "view-source:https://example.com", + "chrome://gpu", + "not a url", + ]) { + for (const disposition of ["new-window", "foreground-tab"] as const) { + expect(PreviewManager.previewWindowOpenAction(details({ url, disposition }))).toBe("deny"); + } + } + }); + it("denies a blank window instead of blanking the opener", () => { // MSAL opens `about:blank` first and falls back to a redirect on `null`; // loading the blank page into the tab would destroy that fallback. diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index e9578c711ae40..03c3c6b56d84a 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -452,11 +452,27 @@ type OpenLinkListener = (event: DesktopPreviewOpenLinkEvent) => Effect.Effect { +/** + * The in-place path runs `loadURL`, a browser-initiated navigation, which Chromium lets reach + * local and inline documents a page cannot open as a top-level page itself. Any frame in the + * guest can call `window.open`, so `javascript:` would also run in the top page's origin. + */ +const IN_PLACE_DENIED_PROTOCOLS = new Set([ + "about:", + "chrome:", + "data:", + "devtools:", + "file:", + "filesystem:", + "javascript:", + "view-source:", +]); + +const urlProtocol = (rawUrl: string): string | undefined => { try { - return POPUP_PROTOCOLS.has(new URL(rawUrl).protocol); + return new URL(rawUrl).protocol; } catch { - return false; + return undefined; } }; @@ -477,17 +493,18 @@ const POPUP_WINDOW_OPTIONS = { /** * `"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. Schemes a - * popup cannot be hardened for keep loading in place, and so does a form POST with a body, - * which a new tab could only reopen as a GET. + * instead destroys the opener the popup has to `postMessage` its result back to. `blob:` and app + * links keep loading in place, where the session's `openExternal` prompt guards app links, and + * so does a form POST with a body, which a new tab could only reopen as a GET. */ export const previewWindowOpenAction = (details: { readonly url: string; readonly disposition: Electron.HandlerDetails["disposition"]; readonly postBody?: Electron.PostBody; }): "popup" | "new-tab" | "navigate" | "deny" => { - if (details.url === "" || details.url === "about:blank") return "deny"; - if (!isPopupUrl(details.url)) return "navigate"; + const protocol = urlProtocol(details.url); + if (protocol === undefined || IN_PLACE_DENIED_PROTOCOLS.has(protocol)) return "deny"; + if (!POPUP_PROTOCOLS.has(protocol)) return "navigate"; if (details.disposition === "new-window") return "popup"; return !details.postBody && (details.disposition === "foreground-tab" || details.disposition === "background-tab") From ab89876107afda3b99954f689421a7e6f24143ed Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 24 Oct 2026 20:17:12 -0500 Subject: [PATCH 2/3] fix(desktop): say which popup scheme actually loads in place In the desktop app only a data: popup got past Chromium and replaced the preview tab; file:, javascript: and view-source: popups left the page unchanged. The comments now say that, and why the other local schemes stay denied. Co-Authored-By: Claude Opus 5.5 --- apps/desktop/src/preview/Manager.test.ts | 4 ++-- apps/desktop/src/preview/Manager.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 563b7950a7ce1..64675138eafc8 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -112,8 +112,8 @@ describe("previewWindowOpenAction", () => { }); it("denies local and inline schemes that no page could open in place itself", () => { - // `loadURL` is a browser-initiated navigation, which Chromium lets reach these - // even though it blocks a page from opening them as a top-level document. + // `loadURL` is a browser-initiated load, so `data:` would get past the check that + // stops a page from opening it as a top-level document. for (const url of [ "file:///etc/passwd", "data:text/html,

hi

", diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index 03c3c6b56d84a..578191cf097e3 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -453,9 +453,9 @@ type OpenLinkListener = (event: DesktopPreviewOpenLinkEvent) => Effect.Effect Date: Mon, 26 Oct 2026 05:00:14 +0000 Subject: [PATCH 3/3] fix(desktop): simplify popup scheme validation and cover denied protocols --- apps/desktop/src/preview/Manager.test.ts | 16 ++++++++++++---- apps/desktop/src/preview/Manager.ts | 16 ++-------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 6f4085f617e3f..ccea2e2bd92e7 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -86,6 +86,12 @@ describe("previewWindowOpenAction", () => { ).toBe("popup"); }); + it("opens HTTP links in preview tabs", () => { + for (const disposition of ["foreground-tab", "background-tab"] as const) { + expect(PreviewManager.previewWindowOpenAction(details({ disposition }))).toBe("new-tab"); + } + }); + it("keeps a form POST with a body on the in-place path", () => { const postBody = { data: [], contentType: "application/x-www-form-urlencoded" }; expect( @@ -106,17 +112,19 @@ describe("previewWindowOpenAction", () => { }); it("denies local and inline schemes that no page could open in place itself", () => { - // `loadURL` is a browser-initiated load, so `data:` would get past the check that - // stops a page from opening it as a top-level document. for (const url of [ - "file:///etc/passwd", "data:text/html,

hi

", + "DaTa:text/html,

hi

", + "file:///etc/passwd", "javascript:alert(1)", "view-source:https://example.com", "chrome://gpu", + "about:srcdoc", + "devtools://devtools/bundled/inspector.html", + "filesystem:https://example.com/temporary/page.html", "not a url", ]) { - for (const disposition of ["new-window", "foreground-tab"] as const) { + for (const disposition of ["new-window", "foreground-tab", "background-tab"] as const) { expect(PreviewManager.previewWindowOpenAction(details({ url, disposition }))).toBe("deny"); } } diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index 08b8529bc52d4..9e794c29d9433 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -424,11 +424,7 @@ type OpenLinkListener = (event: DesktopPreviewOpenLinkEvent) => Effect.Effect { - try { - return new URL(rawUrl).protocol; - } catch { - return undefined; - } -}; - /** Popups bypass will-attach-webview hardening and would inherit the picker's contextIsolation:false. */ const POPUP_WINDOW_OPTIONS = { webPreferences: { @@ -466,7 +454,7 @@ export const previewWindowOpenAction = (details: { readonly disposition: Electron.HandlerDetails["disposition"]; readonly postBody?: Electron.PostBody; }): "popup" | "new-tab" | "navigate" | "deny" => { - const protocol = urlProtocol(details.url); + const protocol = URL.parse(details.url)?.protocol; // MSAL redirects on a denied blank popup; navigating the tab would destroy that fallback. if (protocol === undefined || IN_PLACE_DENIED_PROTOCOLS.has(protocol)) return "deny"; if (!POPUP_PROTOCOLS.has(protocol)) return "navigate";