Skip to content

Commit 7c220c6

Browse files
committed
fix(desktop): allow custom URL schemes
1 parent 3846c90 commit 7c220c6

2 files changed

Lines changed: 81 additions & 24 deletions

File tree

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import fsp from "node:fs/promises"
22
import os from "node:os"
33

4-
import { shell } from "electron"
4+
import { dialog, shell } from "electron"
55
import path from "pathe"
66
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
77

@@ -14,6 +14,9 @@ vi.mock("electron", () => ({
1414
shell: {
1515
openExternal: vi.fn(),
1616
},
17+
dialog: {
18+
showMessageBox: vi.fn(),
19+
},
1720
}))
1821

1922
vi.mock("electron-ipc-decorator", () => ({
@@ -73,10 +76,13 @@ describe("IntegrationService", () => {
7376

7477
describe("openURLScheme", () => {
7578
const openExternalMock = vi.mocked(shell.openExternal)
79+
const showMessageBoxMock = vi.mocked(dialog.showMessageBox)
7680

7781
beforeEach(() => {
7882
openExternalMock.mockReset()
7983
openExternalMock.mockResolvedValue()
84+
showMessageBoxMock.mockReset()
85+
showMessageBoxMock.mockResolvedValue({ checkboxChecked: false, response: 0 })
8086
})
8187

8288
it("rejects input that cannot be parsed as a URL", async () => {
@@ -110,6 +116,7 @@ describe("IntegrationService", () => {
110116
await expect(service.openURLScheme(dangerousScheme)).rejects.toThrow(
111117
/not allowed|disallowed|not permitted/i,
112118
)
119+
expect(showMessageBoxMock).not.toHaveBeenCalled()
113120
expect(openExternalMock).not.toHaveBeenCalled()
114121
},
115122
)
@@ -133,7 +140,32 @@ describe("IntegrationService", () => {
133140
await expect(service.openURLScheme(allowedScheme)).resolves.toEqual({
134141
success: true,
135142
})
143+
expect(showMessageBoxMock).not.toHaveBeenCalled()
136144
expect(openExternalMock).toHaveBeenCalledWith(allowedScheme)
137145
})
146+
147+
it.each([
148+
["logseq://x-callback-url/open?title=Test"],
149+
["my-app+folo.v2://open?title=Test"],
150+
["MYAPP://open"],
151+
["file-helper://open"],
152+
])("permits user-defined integration scheme %s", async (customScheme) => {
153+
const service = new IntegrationService()
154+
155+
await expect(service.openURLScheme(customScheme)).resolves.toEqual({
156+
success: true,
157+
})
158+
expect(showMessageBoxMock).toHaveBeenCalledOnce()
159+
expect(openExternalMock).toHaveBeenCalledWith(customScheme)
160+
})
161+
162+
it("does not open a user-defined integration scheme when confirmation is canceled", async () => {
163+
showMessageBoxMock.mockResolvedValue({ checkboxChecked: false, response: 1 })
164+
const service = new IntegrationService()
165+
166+
await expect(service.openURLScheme("logseq://open")).rejects.toThrow(/not opened/i)
167+
expect(showMessageBoxMock).toHaveBeenCalledOnce()
168+
expect(openExternalMock).not.toHaveBeenCalled()
169+
})
138170
})
139171
})

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

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import { existsSync } from "node:fs"
22
import fsp from "node:fs/promises"
33

4-
import { shell } from "electron"
4+
import { dialog, shell } from "electron"
55
import { IpcMethod, IpcService } from "electron-ipc-decorator"
66
import path from "pathe"
77

8+
import { t } from "~/lib/i18n"
89
import { store } from "~/lib/store"
910
import { logger } from "~/logger"
1011

@@ -81,14 +82,7 @@ export async function saveMediaToEagle(input: SaveToEagleInput): Promise<any> {
8182
}
8283
}
8384

84-
// Allowlist of URL scheme protocols that `openURLScheme` is permitted to hand
85-
// off to `shell.openExternal`. The list intentionally covers the integrations
86-
// shipped in the UI (Obsidian, Bear, Drafts, Things, Notion, DEVONthink) plus
87-
// generic web/mail schemes, while excluding dangerous protocols such as
88-
// `file:`, `smb:`, `ms-msdt:`, `search-ms:`, `jar:`, `res:`, `javascript:`,
89-
// `data:`, `vbscript:`, which have known abuse chains when invoked from
90-
// untrusted content.
91-
const ALLOWED_URL_SCHEME_PROTOCOLS = new Set<string>([
85+
const BUILT_IN_URL_SCHEME_PROTOCOLS = new Set<string>([
9286
"http",
9387
"https",
9488
"mailto",
@@ -100,8 +94,39 @@ const ALLOWED_URL_SCHEME_PROTOCOLS = new Set<string>([
10094
"x-devonthink",
10195
])
10296

103-
function isAllowedURLSchemeProtocol(protocol: string): boolean {
104-
return ALLOWED_URL_SCHEME_PROTOCOLS.has(protocol)
97+
// Protocols that must never be handed to `shell.openExternal`, even after user
98+
// confirmation. Matches are exact so custom protocols such as `file-helper`
99+
// remain usable.
100+
const DISALLOWED_URL_SCHEME_PROTOCOLS = new Set<string>([
101+
"data",
102+
"file",
103+
"jar",
104+
"javascript",
105+
"ms-msdt",
106+
"res",
107+
"search-ms",
108+
"smb",
109+
"vbscript",
110+
])
111+
112+
function isDisallowedURLSchemeProtocol(protocol: string): boolean {
113+
return DISALLOWED_URL_SCHEME_PROTOCOLS.has(protocol)
114+
}
115+
116+
async function confirmUserDefinedURLScheme(protocol: string): Promise<boolean> {
117+
const result = await dialog.showMessageBox({
118+
type: "warning",
119+
title: t("dialog.openExternalApp.title"),
120+
message: t("dialog.openExternalApp.message", {
121+
url: `${protocol}://`,
122+
interpolation: { escapeValue: false },
123+
}),
124+
buttons: [t("dialog.open"), t("dialog.cancel")],
125+
defaultId: 1,
126+
cancelId: 1,
127+
})
128+
129+
return result.response === 0
105130
}
106131

107132
export class IntegrationService extends IpcService {
@@ -406,14 +431,9 @@ ${content}
406431

407432
try {
408433
// Parse and validate the protocol up-front. `shell.openExternal` will
409-
// happily dispatch any scheme the OS has registered a handler for,
410-
// including `file://`, `smb://`, `ms-msdt:`, `search-ms:`, `jar:`,
411-
// `res:`, etc. Several of those have well-documented exploit chains
412-
// (NTLM credential theft over SMB, MSDT/Follina RCE on Windows,
413-
// local-file disclosure via file://). The Electron docs explicitly
414-
// warn against passing untrusted URLs to `shell.openExternal`, so we
415-
// enforce a strict allowlist of schemes that the integrations UI is
416-
// intended to support.
434+
// happily dispatch any scheme the OS has registered a handler for. Keep
435+
// known dangerous protocols blocked while allowing user-configured app
436+
// schemes such as `logseq://` or `ulysses://`.
417437
let protocol: string
418438
try {
419439
protocol = new URL(scheme).protocol.replace(/:$/, "").toLowerCase()
@@ -425,10 +445,15 @@ ${content}
425445
throw new Error("Invalid URL scheme format. Must include protocol (e.g., 'app://')")
426446
}
427447

428-
if (!isAllowedURLSchemeProtocol(protocol)) {
429-
throw new Error(
430-
`URL scheme "${protocol}://" is not allowed. Allowed schemes: ${[...ALLOWED_URL_SCHEME_PROTOCOLS].sort().join(", ")}.`,
431-
)
448+
if (isDisallowedURLSchemeProtocol(protocol)) {
449+
throw new Error(`URL scheme "${protocol}://" is not allowed.`)
450+
}
451+
452+
if (
453+
!BUILT_IN_URL_SCHEME_PROTOCOLS.has(protocol) &&
454+
!(await confirmUserDefinedURLScheme(protocol))
455+
) {
456+
throw new Error(`URL scheme "${protocol}://" was not opened.`)
432457
}
433458

434459
// Log URL scheme execution (mask sensitive data)

0 commit comments

Comments
 (0)