fix(desktop/ipc): allowlist URL scheme protocols in openURLScheme (#5056)

The 'integration.openURLScheme' IPC method invokes 'shell.openExternal'
with a renderer-supplied string after only checking that it contains
'://'. Electron's documentation explicitly warns that passing untrusted
URLs to 'shell.openExternal' is unsafe: schemes such as 'file://',
'smb://', 'ms-msdt:', 'search-ms:', 'jar:', 'res:', 'javascript:',
'data:' and 'vbscript:' have well-known abuse chains (local file
disclosure, NTLM credential theft over SMB on Windows, MSDT/Follina-style
RCE, etc.). Because the renderer process can also reach this IPC via any
XSS sink in untrusted RSS feed content, the previous validation was not
sufficient.

Replace the substring check with strict URL parsing plus an allowlist of
protocols that match the integration use-cases documented in the UI
(Obsidian, Bear, Drafts, Things, Notion, DEVONthink) plus generic
http/https/mailto. All other protocols are rejected with a clear error.

Adds vitest cases for representative dangerous schemes (verifying that
'shell.openExternal' is never invoked) and for every scheme shipped as a
built-in example, so future regressions on either side are caught.

Author: sebastionoss

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,<script>alert(1)</script>"],
+      ["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)
+    })
+  })
 })

apps/desktop/layer/main/src/ipc/services/integration.ts

@@ -81,6 +81,29 @@ export async function saveMediaToEagle(input: SaveToEagleInput): Promise<any> {
   }
 }
 
+// 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<string>([
+  "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