diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index f19079e46bce4..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( @@ -98,15 +104,32 @@ describe("previewWindowOpenAction", () => { it("does not hand a window to schemes that cannot be hardened", () => { 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", () => { + for (const url of [ + "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", "background-tab"] as const) { + expect(PreviewManager.previewWindowOpenAction(details({ url, disposition }))).toBe("deny"); + } + } + }); + it("denies a blank window instead of blanking the opener", () => { for (const url of ["about:blank", ""]) { for (const disposition of ["new-window", "foreground-tab"] as const) { diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index d0221e9be0e6e..9e794c29d9433 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -424,13 +424,17 @@ type OpenLinkListener = (event: DesktopPreviewOpenLinkEvent) => Effect.Effect { - try { - return POPUP_PROTOCOLS.has(new URL(rawUrl).protocol); - } catch { - return false; - } -}; +// Main-process loadURL bypasses Chromium's renderer-side restrictions on local and inline URLs. +const IN_PLACE_DENIED_PROTOCOLS = new Set([ + "about:", + "chrome:", + "data:", + "devtools:", + "file:", + "filesystem:", + "javascript:", + "view-source:", +]); /** Popups bypass will-attach-webview hardening and would inherit the picker's contextIsolation:false. */ const POPUP_WINDOW_OPTIONS = { @@ -441,15 +445,19 @@ const POPUP_WINDOW_OPTIONS = { }, } satisfies Electron.BrowserWindowConstructorOptions; -/** OAuth needs a surviving popup opener; unhardenable schemes and form POSTs must stay in place. */ +/** + * OAuth needs a surviving popup opener. `blob:`, app links (behind the session's `openExternal` + * prompt) and form POSTs stay in place. + */ export const previewWindowOpenAction = (details: { readonly url: string; readonly disposition: Electron.HandlerDetails["disposition"]; readonly postBody?: Electron.PostBody; }): "popup" | "new-tab" | "navigate" | "deny" => { + const protocol = URL.parse(details.url)?.protocol; // MSAL redirects on a denied blank popup; navigating the tab would destroy that fallback. - if (details.url === "" || details.url === "about:blank") return "deny"; - if (!isPopupUrl(details.url)) return "navigate"; + 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")