diff --git a/apps/desktop/layer/main/src/ipc/services/integration.test.ts b/apps/desktop/layer/main/src/ipc/services/integration.test.ts index d4c60c08f..718a10c77 100644 --- a/apps/desktop/layer/main/src/ipc/services/integration.test.ts +++ b/apps/desktop/layer/main/src/ipc/services/integration.test.ts @@ -1,8 +1,9 @@ import fsp from "node:fs/promises" import os from "node:os" +import { shell } from "electron" import path from "pathe" -import { afterEach, describe, expect, it, vi } from "vitest" +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" import { IntegrationService } from "./integration" @@ -69,4 +70,72 @@ describe("IntegrationService", () => { fsp.stat(path.join(vaultPath, "KAWA DESIGN 少女前线2:追放 索米·雪兔献礼 1")), ).rejects.toThrow() }) + + describe("openURLScheme", () => { + const openExternalMock = vi.mocked(shell.openExternal) + + beforeEach(() => { + openExternalMock.mockReset() + openExternalMock.mockResolvedValue() + }) + + it("rejects input that cannot be parsed as a URL", async () => { + const service = new IntegrationService() + + await expect(service.openURLScheme("not-a-url")).rejects.toThrow( + /Invalid URL scheme/i, + ) + expect(openExternalMock).not.toHaveBeenCalled() + }) + + // These are the dangerous protocols that previously slipped through the + // "contains ://" guard and reached shell.openExternal verbatim. + // shell.openExternal docs explicitly warn that passing untrusted URLs is + // unsafe — file://, smb://, search-ms:, ms-msdt:, jar:, res:, etc. have + // been used in real-world RCE / NTLM-credential-theft chains. + it.each([ + ["file:///etc/passwd"], + ["FILE:///etc/passwd"], + ["smb://attacker.example/share"], + ["jar:http://attacker.example/x.jar!/"], + ["res://shell32.dll/1"], + ["ms-msdt:/id PCWDiagnostic"], + ["search-ms:query=secret"], + ["javascript:alert(1)"], + ["data:text/html,"], + ["vbscript:msgbox(1)"], + ])( + "blocks dangerous scheme %s and does not invoke shell.openExternal", + async (dangerousScheme) => { + const service = new IntegrationService() + + await expect(service.openURLScheme(dangerousScheme)).rejects.toThrow( + /not allowed|disallowed|not permitted/i, + ) + expect(openExternalMock).not.toHaveBeenCalled() + }, + ) + + // The integration UI ships these schemes as built-in examples + // (see url-scheme-handler.ts#getExamples) plus generic web/mail. + // They must keep working after the fix. + it.each([ + ["https://example.com"], + ["http://example.com/path?q=1"], + ["mailto:user@example.com"], + ["obsidian://new?vault=MyVault&name=Test"], + ["bear://x-callback-url/create?title=Test"], + ["things:///add?title=Test"], + ["notion://new?title=Test"], + ["x-devonthink://createText?title=Test"], + ["drafts://x-callback-url/create?text=Test"], + ])("permits known integration scheme %s", async (allowedScheme) => { + const service = new IntegrationService() + + await expect(service.openURLScheme(allowedScheme)).resolves.toEqual({ + success: true, + }) + expect(openExternalMock).toHaveBeenCalledWith(allowedScheme) + }) + }) }) diff --git a/apps/desktop/layer/main/src/ipc/services/integration.ts b/apps/desktop/layer/main/src/ipc/services/integration.ts index 90e97f1f2..047476e53 100644 --- a/apps/desktop/layer/main/src/ipc/services/integration.ts +++ b/apps/desktop/layer/main/src/ipc/services/integration.ts @@ -81,6 +81,29 @@ export async function saveMediaToEagle(input: SaveToEagleInput): Promise { } } +// Allowlist of URL scheme protocols that `openURLScheme` is permitted to hand +// off to `shell.openExternal`. The list intentionally covers the integrations +// shipped in the UI (Obsidian, Bear, Drafts, Things, Notion, DEVONthink) plus +// generic web/mail schemes, while excluding dangerous protocols such as +// `file:`, `smb:`, `ms-msdt:`, `search-ms:`, `jar:`, `res:`, `javascript:`, +// `data:`, `vbscript:`, which have known abuse chains when invoked from +// untrusted content. +const ALLOWED_URL_SCHEME_PROTOCOLS = new Set([ + "http", + "https", + "mailto", + "obsidian", + "bear", + "drafts", + "things", + "notion", + "x-devonthink", +]) + +function isAllowedURLSchemeProtocol(protocol: string): boolean { + return ALLOWED_URL_SCHEME_PROTOCOLS.has(protocol) +} + export class IntegrationService extends IpcService { static override readonly groupName = "integration" @@ -382,11 +405,32 @@ ${content} const requestId = Math.random().toString(36).slice(2, 8) try { - // Validate URL scheme format - if (!scheme.includes("://")) { + // Parse and validate the protocol up-front. `shell.openExternal` will + // happily dispatch any scheme the OS has registered a handler for, + // including `file://`, `smb://`, `ms-msdt:`, `search-ms:`, `jar:`, + // `res:`, etc. Several of those have well-documented exploit chains + // (NTLM credential theft over SMB, MSDT/Follina RCE on Windows, + // local-file disclosure via file://). The Electron docs explicitly + // warn against passing untrusted URLs to `shell.openExternal`, so we + // enforce a strict allowlist of schemes that the integrations UI is + // intended to support. + let protocol: string + try { + protocol = new URL(scheme).protocol.replace(/:$/, "").toLowerCase() + } catch { throw new Error("Invalid URL scheme format. Must include protocol (e.g., 'app://')") } + if (!protocol) { + throw new Error("Invalid URL scheme format. Must include protocol (e.g., 'app://')") + } + + if (!isAllowedURLSchemeProtocol(protocol)) { + throw new Error( + `URL scheme "${protocol}://" is not allowed. Allowed schemes: ${[...ALLOWED_URL_SCHEME_PROTOCOLS].sort().join(", ")}.`, + ) + } + // Log URL scheme execution (mask sensitive data) const safeScheme = scheme.replaceAll(/(\?|&)([^=]+)=([^&]+)/g, (_, prefix, key, value) => // Mask potential sensitive query parameters @@ -399,7 +443,7 @@ ${content} logger.info(`[URLScheme:${requestId}] Opening URL scheme`, { scheme: safeScheme, - protocol: scheme.split("://")[0], + protocol, }) // Use Electron's shell.openExternal to open URL scheme