fix(desktop): route attachment downloads through Electron native system on Linux (#2441)

* fix(desktop): route attachment downloads through Electron native system on Linux

Replaces shell.openExternal with webContents.downloadURL for attachment
downloads in the Electron desktop app. On Linux/Ubuntu, opening a
CloudFront URL serving Content-Type: text/html via the system browser
causes the browser to render the HTML inline instead of downloading.
Electron's native downloadURL shows a save dialog and saves the file
directly, fixing HTML downloads regardless of Content-Type.

* test(views): update desktop download test to match the new downloadURL bridge

The test still referenced the old openExternal bridge. Updated it to
assert desktopAPI.downloadURL() instead.

* fix(desktop): add URL scheme allowlist to download IPC handler

Addresses review feedback on PR #2441.

The file:download-url IPC handler called webContents.downloadURL
directly, bypassing the http/https allowlist enforced by
openExternalSafely. Adds downloadURLSafely() alongside the existing
openExternalSafely wrapper, reuses the same isSafeExternalHttpUrl
check, and extends the ESLint no-restricted-syntax rule to ban direct
webContents.downloadURL calls.

Also handles nits: observable warning on null mainWindow, removes dead
openExternal field from DesktopBridge, adds desktop-branch failure test.
This commit is contained in:
Shalin
2026-05-12 23:44:33 -07:00
committed by GitHub
parent b87e54850a
commit 7fcc8159ba
7 changed files with 77 additions and 26 deletions

View File

@@ -10,10 +10,11 @@ export default [
globals: { ...globals.node },
},
},
// Security: every renderer-controlled URL that reaches the OS shell must
// flow through openExternalSafely in src/main/external-url.ts (scheme
// allowlist). Enforce it statically so a direct shell.openExternal call
// cannot silently regress the protection.
// Security: every renderer-controlled URL that reaches the OS shell or the
// native download system must flow through the safe wrappers in
// src/main/external-url.ts (scheme allowlist). Enforce it statically so
// direct shell.openExternal / webContents.downloadURL calls cannot silently
// regress the protection.
{
files: ["src/main/**/*.ts"],
rules: {
@@ -25,6 +26,12 @@ export default [
message:
"Do not call shell.openExternal directly. Use openExternalSafely from './external-url' so the http/https allowlist stays enforced.",
},
{
selector:
"CallExpression[callee.object.property.name='webContents'][callee.property.name='downloadURL']",
message:
"Do not call webContents.downloadURL directly. Use downloadURLSafely from './external-url' so the http/https allowlist stays enforced.",
},
],
},
},

View File

@@ -1,4 +1,4 @@
import { shell } from "electron";
import { shell, type BrowserWindow } from "electron";
// True when the URL parses and uses http/https — the only schemes we let
// reach `shell.openExternal`. Scheme comparison is safe because the WHATWG
@@ -19,6 +19,19 @@ export function openExternalSafely(url: string): Promise<void> | void {
return shell.openExternal(url);
}
// Canonical wrapper around webContents.downloadURL. All renderer-controlled
// URLs that trigger a native download MUST flow through here; direct calls
// to `webContents.downloadURL` elsewhere in the main process are banned by
// the no-restricted-syntax rule in apps/desktop/eslint.config.mjs.
// Reuses the same http/https allowlist as openExternalSafely.
export function downloadURLSafely(win: BrowserWindow, url: string): void {
if (getHttpProtocol(url) === null) {
console.warn(`[security] blocked downloadURL: ${describeScheme(url)}`);
return;
}
win.webContents.downloadURL(url);
}
function getHttpProtocol(url: string): "http:" | "https:" | null {
try {
const { protocol } = new URL(url);

View File

@@ -5,7 +5,7 @@ import { electronApp, optimizer, is } from "@electron-toolkit/utils";
import fixPath from "fix-path";
import { setupAutoUpdater } from "./updater";
import { setupDaemonManager } from "./daemon-manager";
import { openExternalSafely } from "./external-url";
import { openExternalSafely, downloadURLSafely } from "./external-url";
import { installContextMenu } from "./context-menu";
import { getAppVersion } from "./app-version";
import { loadRuntimeConfig } from "./runtime-config-loader";
@@ -288,6 +288,14 @@ if (!gotTheLock) {
return openExternalSafely(url);
});
ipcMain.handle("file:download-url", (_event, url: string) => {
if (!mainWindow) {
console.warn("[download] ignored file:download-url — mainWindow torn down");
return;
}
downloadURLSafely(mainWindow, url);
});
// Sync IPC: app version + normalized OS for preload. Sync (not invoke) so
// preload can attach the values to `desktopAPI.appInfo` before any renderer
// code reads them, ensuring the very first HTTP request from the renderer

View File

@@ -19,6 +19,9 @@ interface DesktopAPI {
onInviteOpen: (callback: (invitationId: string) => void) => () => void;
/** Open a URL in the default browser. */
openExternal: (url: string) => Promise<void>;
/** Download a file by URL through Electron's native download system.
* Shows a native save dialog. On non-desktop platforms this is undefined. */
downloadURL: (url: string) => Promise<void>;
/** Hide macOS traffic lights for full-screen modals; restore when false. */
setImmersiveMode: (immersive: boolean) => Promise<void>;
/** Show a native OS notification for a new inbox item. */

View File

@@ -89,6 +89,11 @@ const desktopAPI = {
},
/** Open a URL in the default browser */
openExternal: (url: string) => ipcRenderer.invoke("shell:openExternal", url),
/** Download a file by URL through Electron's native download system.
* Shows a save dialog and saves to disk. Unlike openExternal, this
* avoids browser rendering of HTML files on Linux.
* On non-desktop platforms this property is undefined. */
downloadURL: (url: string) => ipcRenderer.invoke("file:download-url", url),
/** Toggle immersive mode — hide macOS traffic lights for full-screen modals */
setImmersiveMode: (immersive: boolean) =>
ipcRenderer.invoke("window:setImmersive", immersive),

View File

@@ -9,10 +9,6 @@ vi.mock("@multica/core/api", () => ({
api: { getAttachment: getAttachmentMock },
}));
vi.mock("../platform", () => ({
openExternal: vi.fn(),
}));
vi.mock("sonner", () => ({
toast: { error: vi.fn(), success: vi.fn() },
}));
@@ -22,7 +18,6 @@ vi.mock("../i18n", () => ({
}));
import { useDownloadAttachment } from "./use-download-attachment";
import { openExternal } from "../platform";
import { toast } from "sonner";
const SIGNED_URL =
@@ -82,9 +77,10 @@ describe("useDownloadAttachment (web)", () => {
});
describe("useDownloadAttachment (desktop)", () => {
it("skips the placeholder tab and hands the signed URL to openExternal", async () => {
(window as unknown as { desktopAPI: { openExternal: () => void } }).desktopAPI = {
openExternal: vi.fn(),
it("skips the placeholder tab and hands the signed URL to the desktop download bridge", async () => {
const downloadURL = vi.fn();
(window as unknown as { desktopAPI: { downloadURL: typeof downloadURL } }).desktopAPI = {
downloadURL,
};
getAttachmentMock.mockResolvedValueOnce({
id: "att-1",
@@ -103,6 +99,23 @@ describe("useDownloadAttachment (desktop)", () => {
// No placeholder — Electron's setWindowOpenHandler would reject
// about:blank, so we go straight to the platform's IPC bridge.
expect(openSpy).not.toHaveBeenCalled();
expect(openExternal).toHaveBeenCalledWith(SIGNED_URL);
expect(downloadURL).toHaveBeenCalledWith(SIGNED_URL);
});
it("shows a toast when the API rejects on desktop", async () => {
const downloadURL = vi.fn();
(window as unknown as { desktopAPI: { downloadURL: typeof downloadURL } }).desktopAPI = {
downloadURL,
};
getAttachmentMock.mockRejectedValueOnce(new Error("network failure"));
const { result } = renderHook(() => useDownloadAttachment());
await act(async () => {
await result.current("att-1");
});
expect(downloadURL).not.toHaveBeenCalled();
await waitFor(() => expect(toast.error).toHaveBeenCalled());
});
});

View File

@@ -3,20 +3,19 @@
import { useCallback } from "react";
import { toast } from "sonner";
import { api } from "@multica/core/api";
import { openExternal } from "../platform";
import { useT } from "../i18n";
interface DesktopBridge {
openExternal?: (u: string) => Promise<void> | void;
downloadURL?: (u: string) => Promise<void> | void;
}
// Detected at call time, not module load — the bridge is injected by the
// Electron preload after `window` exists, and reading it lazily lets the
// same hook work in both renderers without a build-time fork.
function hasDesktopBridge(): boolean {
function hasDesktopDownloadBridge(): boolean {
if (typeof window === "undefined") return false;
const bridge = (window as unknown as { desktopAPI?: DesktopBridge }).desktopAPI;
return Boolean(bridge?.openExternal);
return Boolean(bridge?.downloadURL);
}
/**
@@ -35,11 +34,11 @@ function hasDesktopBridge(): boolean {
* spec (`dom-open` step 17) makes that return `null`, which would leave
* us nothing to navigate. We disown the opener manually after the fetch.
*
* - **Desktop**: `window.open` is intercepted by Electron's
* `setWindowOpenHandler` and routed through `openExternalSafely`, which
* rejects `about:blank`. So on desktop we fetch first, then hand the URL
* to `openExternal()` which IPCs into `shell.openExternal` and opens the
* system browser.
* - **Desktop**: uses `desktopAPI.downloadURL()` which invokes Electron's
* native `webContents.downloadURL()`, showing a save dialog and saving
* the file directly. This avoids the system browser entirely and fixes
* the Linux/Ubuntu issue where HTML files are rendered inline instead
* of being downloaded.
*/
export function useDownloadAttachment(): (attachmentId: string) => Promise<void> {
const { t } = useT("editor");
@@ -47,14 +46,17 @@ export function useDownloadAttachment(): (attachmentId: string) => Promise<void>
async (attachmentId: string) => {
const failed = () => toast.error(t(($) => $.attachment.download_failed));
if (hasDesktopBridge()) {
if (hasDesktopDownloadBridge()) {
try {
const fresh = await api.getAttachment(attachmentId);
if (!fresh.download_url) {
failed();
return;
}
openExternal(fresh.download_url);
const bridge = (
window as unknown as { desktopAPI?: DesktopBridge }
).desktopAPI;
await bridge!.downloadURL!(fresh.download_url);
} catch {
failed();
}