diff --git a/apps/desktop/eslint.config.mjs b/apps/desktop/eslint.config.mjs index 4f9f86d77d..3690c9dfda 100644 --- a/apps/desktop/eslint.config.mjs +++ b/apps/desktop/eslint.config.mjs @@ -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.", + }, ], }, }, diff --git a/apps/desktop/src/main/external-url.ts b/apps/desktop/src/main/external-url.ts index e528d1db97..58b2c80404 100644 --- a/apps/desktop/src/main/external-url.ts +++ b/apps/desktop/src/main/external-url.ts @@ -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 { 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); diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 660559a322..8c278b49bc 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -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 diff --git a/apps/desktop/src/preload/index.d.ts b/apps/desktop/src/preload/index.d.ts index 8e54ced451..0fb1ee46d0 100644 --- a/apps/desktop/src/preload/index.d.ts +++ b/apps/desktop/src/preload/index.d.ts @@ -19,6 +19,9 @@ interface DesktopAPI { onInviteOpen: (callback: (invitationId: string) => void) => () => void; /** Open a URL in the default browser. */ openExternal: (url: string) => Promise; + /** 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; /** Hide macOS traffic lights for full-screen modals; restore when false. */ setImmersiveMode: (immersive: boolean) => Promise; /** Show a native OS notification for a new inbox item. */ diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index c67d32b58e..e8015e9920 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -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), diff --git a/packages/views/editor/use-download-attachment.test.tsx b/packages/views/editor/use-download-attachment.test.tsx index ac22594fe9..4ac5e828f9 100644 --- a/packages/views/editor/use-download-attachment.test.tsx +++ b/packages/views/editor/use-download-attachment.test.tsx @@ -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()); }); }); diff --git a/packages/views/editor/use-download-attachment.ts b/packages/views/editor/use-download-attachment.ts index 559400bef6..c2dea633f0 100644 --- a/packages/views/editor/use-download-attachment.ts +++ b/packages/views/editor/use-download-attachment.ts @@ -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; + downloadURL?: (u: string) => Promise | 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 { const { t } = useT("editor"); @@ -47,14 +46,17 @@ export function useDownloadAttachment(): (attachmentId: string) => Promise 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(); }