Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions apps/desktop/src/preview/Manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,<h1>hi</h1>",
"DaTa:text/html,<h1>hi</h1>",
"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) {
Expand Down
28 changes: 18 additions & 10 deletions apps/desktop/src/preview/Manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,13 +424,17 @@ type OpenLinkListener = (event: DesktopPreviewOpenLinkEvent) => Effect.Effect<vo
// about:blank inherits relaxed guest preferences; external editor schemes must reach the OS handler.
const POPUP_PROTOCOLS = new Set(["http:", "https:"]);

const isPopupUrl = (rawUrl: string): boolean => {
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 = {
Expand All @@ -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")
Expand Down
Loading