fix(security): validate URLs in shell.openExternal and prevent path traversal by xr843 · Pull Request #13892 · CherryHQ/cherry-studio

xr843 · 2026-03-30T05:20:36Z

Summary

This PR addresses two security vulnerabilities in the Electron main process:

1. Unvalidated `shell.openExternal()` — CWE-78

All four shell.openExternal() call sites accept URLs without protocol validation. An attacker who achieves XSS (e.g., via crafted LLM responses) could trigger arbitrary protocol handlers:

shell.openExternal('file:///etc/passwd')     // open local files
shell.openExternal('ms-msdt:/id ...')         // launch system tools (Windows)
shell.openExternal('calculator:')             // launch arbitrary apps

Fix: Add isSafeExternalUrl() helper that only allows http:, https:, and mailto: protocols. Applied to all call sites:

src/main/ipc.ts — IpcChannel.Open_Website handler
src/main/services/WindowService.ts — will-navigate and setWindowOpenHandler
src/main/services/WebviewService.ts — webview setWindowOpenHandler

2. Path Traversal in `http://file/` handler — CWE-22

The setWindowOpenHandler in WindowService.ts extracts a filename from http://file/ URLs via simple string replacement and concatenates it into a file path without sanitization:

// Before: vulnerable to path traversal
const fileName = url.replace('http://file/', '')
const filePath = storageDir + '/' + fileName
shell.openPath(filePath)  // http://file/../../../etc/passwd → opens /etc/passwd

Fix: Use path.resolve() and verify the result stays within storageDir:

const filePath = path.resolve(storageDir, fileName)
if (!filePath.startsWith(path.resolve(storageDir) + path.sep)) {
  logger.warn(`Blocked path traversal attempt: ${fileName}`)
  return
}

References

…versal prevention - Add isSafeExternalUrl() helper that only allows http(s) and mailto protocols - Prevents OS command execution via custom protocol handlers (file://, ms-msdt:, etc.) - Prevents path traversal in http://file/ URL handler via path.resolve + boundary check - Applied to all 4 shell.openExternal call sites across ipc.ts, WindowService.ts, WebviewService.ts CWE-78: OS Command Injection via URL handlers CWE-22: Path Traversal

GeorgeDong32

Review Summary

安全修复方向正确，核心实现质量良好。重点关注以下问题：

安全修复覆盖度

已保护的调用点 (4/4 main process):

ipc.ts — Open_Website handler ✅
WindowService.ts — will-navigate ✅
WindowService.ts — setWindowOpenHandler ✅
WebviewService.ts — setWindowOpenHandler ✅

未保护但安全的调用点:

AppMenuService.ts (4处) — 硬编码 HTTPS URL，无需保护
AnthropicService.ts:169 — OAuth URL 始终为 HTTPS，低风险

需要关注的问题

[重要] Preload bridge bypass:
src/preload/index.ts:437 仍然无校验地暴露 shell.openExternal 给 renderer 进程。以下组件通过此路径绕过了安全检查：

HtmlArtifactsCard.tsx:45 — 打开 file:// URL（合法用途）
MacProcessTrustHintModal.tsx:17 — 打开 x-apple.systempreferences:（合法用途）

这些是合理的功能需求，但也意味着 renderer 中任何恶意代码都可以绕过本次安全修复。建议在 preload bridge 中也加入校验，或为这些合法自定义协议提供白名单。

路径遍历防护

path.resolve() + startsWith(storageDir + path.sep) 的实现是标准模式，能有效阻止 ../ 遍历攻击。代码质量良好。

其他

WebviewService 被拦截的 URL 缺少 logger.warn（详见 inline comment）
isSafeExternalUrl 缺少单元测试（详见 inline comment）
http://file/ 空文件名边界情况（详见 inline comment）

GeorgeDong32 · 2026-03-30T16:22:05Z

src/main/services/security.ts

+export function isSafeExternalUrl(url: string): boolean {
+  try {
+    const parsed = new URL(url)
+    return ALLOWED_EXTERNAL_PROTOCOLS.has(parsed.protocol)
+  } catch {
+    return false
+  }


nit: 缺少单元测试

isSafeExternalUrl 是安全边界函数，建议补充单元测试覆盖以下场景：

正常 http/https/mailto URL → true

自定义协议 file://, ms-msdt:, javascript:alert(1) → false

空字符串、纯文本、缺少协议的 URL → false (catch 分支)

大小写混合协议如 HTTP://... → 验证 new URL() 的解析行为

安全关键函数没有测试覆盖是一个风险点。

GeorgeDong32 · 2026-03-30T16:22:09Z

src/main/services/WebviewService.ts

    if (isExternal) {
-      void shell.openExternal(url)
+      if (isSafeExternalUrl(url)) {
+        void shell.openExternal(url)


minor: 被拦截的 URL 缺少日志记录

其他三个调用点（ipc.ts:188, WindowService.ts:287, WindowService.ts:329）在拦截不安全 URL 时都有 logger.warn，唯独这里没有。

当 webview 中的链接被静默丢弃时，用户看到的是"点击无反应"，调试时也无法在日志中找到线索。建议补充：

if (isSafeExternalUrl(url)) { void shell.openExternal(url) } else { logger.warn(`Blocked shell.openExternal for untrusted URL scheme: ${url}`) }

src/main/services/WindowService.ts

… guard - Add logger.warn in WebviewService when blocking untrusted URL schemes - Guard against empty fileName in http://file/ URL handling - Simplify path traversal check (remove redundant storageDir equality) - Add unit tests for isSafeExternalUrl covering allowed protocols, dangerous schemes, malformed input, and mixed-case handling

xr843 · 2026-03-30T23:26:04Z

Thanks for the thorough review @GeorgeDong32! All three points addressed:

Unit tests added — security.test.ts covers http/https/mailto (allowed), file/javascript/ms-msdt/calculator (blocked), empty/malformed input, and mixed-case protocols.
WebviewService logger.warn added — blocked URLs now log consistently across all four call sites.
Empty fileName guard — http://file/ with no filename is now explicitly rejected instead of opening the storage directory. Also simplified the path traversal check by removing the redundant storageDir equality branch.

Regarding the preload bridge concern — good observation. The file:// and x-apple.systempreferences: usage in HtmlArtifactsCard and MacProcessTrustHintModal are legitimate, but a preload-level allowlist would be a good hardening follow-up. Happy to tackle that in a separate PR if maintainers agree.

EurFelux

Review Summary

安全修复方向正确，isSafeExternalUrl 实现简洁有效，路径遍历防护采用标准模式，测试覆盖充分。

需要修复

CI 失败：src/main/ipc.ts import 排序不符合 simple-import-sort 规则

需要关注

Preload bridge 绕过：src/preload/index.ts:447 直接暴露 shell.openExternal，未经任何校验。本次 PR 保护的 4 个调用点均在 main process，但 renderer 可通过 preload bridge 完全绕过
现有功能兼容性：renderer 中有两处使用非标准协议的 openExternal 调用（file:// 用于 artifacts 预览、x-apple.systempreferences: 用于 macOS 辅助功能设置），目前因走 preload bridge 不受影响，但后续补全校验时会 break。建议在白名单设计中提前考虑这些合法用例
类型约束：建议在 IPC 参数类型层面用 template literal type 做编译期约束，运行时校验作为兜底而非唯一防线

EurFelux · 2026-03-31T06:52:20Z

src/main/ipc.ts

 import storeSyncService from './services/StoreSyncService'
 import { themeService } from './services/ThemeService'
 import VertexAIService from './services/VertexAIService'
+import { isSafeExternalUrl } from './services/security'


[CI] Import sort 错误

CI basic-checks 因 simple-import-sort/imports 失败。这个 import 的位置不对，需要运行 eslint --fix 或手动调整 import 顺序。

src/main/ipc.ts 1:1 error Run autofix to sort these imports! simple-import-sort/imports

EurFelux · 2026-03-31T06:52:44Z

src/main/services/security.ts

+export function isSafeExternalUrl(url: string): boolean {
+  try {
+    const parsed = new URL(url)
+    return ALLOWED_EXTERNAL_PROTOCOLS.has(parsed.protocol)
+  } catch {
+    return false
+  }


[重要] Preload bridge 绕过问题

isSafeExternalUrl 实现本身很好，但 src/preload/index.ts:447 仍然直接暴露 shell.openExternal 给 renderer 进程：

shell: { openExternal: (url: string, options?: Electron.OpenExternalOptions) => shell.openExternal(url, options) }

Renderer 中通过 window.api.shell.openExternal() 调用的代码完全绕过了本次安全检查。GeorgeDong32 在之前的 review 中也提到了这个问题。

建议： 在 preload bridge 中也加入 isSafeExternalUrl 校验，或者将 preload 的 shell.openExternal 改为走 IPC Open_Website 通道（已有校验）。如果有合法的自定义协议需求（如 file://、x-apple.systempreferences:），可以单独维护一个白名单。

这不阻塞合并，但建议作为 follow-up 修复。

EurFelux · 2026-03-31T06:53:04Z

src/main/services/WindowService.ts

-        shell.openPath(filePath).catch((err) => logger.error('Failed to open file:', err))
-      } else {
+        const filePath = path.resolve(storageDir, fileName)
+        // Prevent path traversal: ensure resolved path is within storageDir


[nit] 路径遍历检查的边界情况

当 storageDir 本身以 path.sep 结尾时（虽然实际中很少见），path.resolve(storageDir) + path.sep 可能会产生双斜杠。path.resolve 会去掉尾部斜杠所以实际上没问题，只是提一下。

另外，startsWith 检查在 macOS（大小写不敏感文件系统）上可能被绕过：/Users/../uSERS/... resolve 后虽然正常化了路径，但如果 storageDir 和实际 resolve 路径大小写不同时仍有理论上的风险。不过对于 Cherry Studio 的实际使用场景来说这不是问题，仅作记录。

EurFelux · 2026-03-31T06:56:41Z

src/main/services/security.ts

+ * Security utility functions for the main process.
+ */
+
+const ALLOWED_EXTERNAL_PROTOCOLS = new Set(['http:', 'https:', 'mailto:'])


[重要] 两处现有功能会被此 PR 的限制 break

PR 保护了 main process 中的 4 个 shell.openExternal 调用点，但 preload bridge（src/preload/index.ts:447）仍然直接暴露 shell.openExternal 给 renderer。以下 renderer 代码通过 preload bridge 绕过了本次安全检查，目前不受影响——但如果后续将 preload bridge 也加上校验，这两处将会 break：

1. file:// 协议 — HtmlArtifactsCard

src/renderer/src/components/CodeBlockView/HtmlArtifactsCard.tsx:45:

const filePath = `file://${path}` void window.api.shell.openExternal(filePath)

用于在外部浏览器打开 HTML artifacts 预览。file:// 不在允许列表中。

2. x-apple.systempreferences: 协议 — MacProcessTrustHintModal

src/renderer/src/pages/settings/SelectionAssistantSettings/components/MacProcessTrustHintModal.tsx:18:

void window.api.shell.openExternal('x-apple.systempreferences:com.apple.preference.security?Privacy_Accessibility')

用于打开 macOS 系统偏好设置的辅助功能页面。x-apple.systempreferences: 不在允许列表中。

建议

ALLOWED_EXTERNAL_PROTOCOLS 应考虑以应用内已有的合法协议需求为基础来设计，而不是仅凭安全直觉。可以：

直接在白名单中添加 file: 和 x-apple.systempreferences:

或者提供一个 isSafeExternalUrl(url, { allowFile: true }) 这样的 options 参数

或者将这两个调用改为走专门的 IPC 通道

无论选哪种方案，都需要在 PR 中明确说明这些现有功能不受影响或已做适配。

EurFelux · 2026-03-31T06:58:03Z

src/main/ipc.ts

+  ipcMain.handle(IpcChannel.Open_Website, (_, url: string) => {
+    if (!isSafeExternalUrl(url)) {
+      logger.warn(`Blocked shell.openExternal for untrusted URL scheme: ${url}`)
+      return
+    }
+    return shell.openExternal(url)


[建议] 在 IPC 入口处用类型约束替代运行时检查

当前做法是在运行时调用 isSafeExternalUrl 做动态拦截——调用者无法提前知道哪些 URL 会被静默丢弃。如果确实要限制协议类型，更推荐在 IPC 入口的参数类型层面做约束，例如：

// shared/IpcChannel.ts 或类似位置 type SafeExternalUrl = `http://${string}` | `https://${string}` | `mailto:${string}`

然后让 Open_Website handler 的参数类型为 SafeExternalUrl。这样：

编译期就能发现不合规的调用（如 file://、x-apple.systempreferences:）

需要使用非标准协议的调用点被迫走专门的 IPC 通道，而不是静默失败

避免用户点击后"无任何反应"的糟糕体验——静默 return 比抛错更难排查

运行时校验可以保留作为兜底，但不应是唯一防线。

xr843 added 4 commits March 30, 2026 13:19

fix(security): validate URLs before shell.openExternal in IPC handler

f3ca1c2

fix(security): validate URLs and prevent path traversal in WindowService

5cc2174

fix(security): validate URLs before shell.openExternal in WebviewService

3db6786

xr843 requested review from 0xfullex and DeJeune as code owners March 30, 2026 05:20

DeJeune approved these changes Mar 30, 2026

View reviewed changes

DeJeune requested review from EurFelux and GeorgeDong32 March 30, 2026 06:18

GeorgeDong32 reviewed Mar 30, 2026

View reviewed changes

DeJeune added the ready-to-merge This PR has sufficient reviewer approvals but is awaiting code owner approval. label Mar 31, 2026

EurFelux reviewed Mar 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(security): validate URLs in shell.openExternal and prevent path traversal#13892

fix(security): validate URLs in shell.openExternal and prevent path traversal#13892
xr843 wants to merge 5 commits intoCherryHQ:mainfrom
xr843:fix/security-shell-openexternal-path-traversal

xr843 commented Mar 30, 2026

Uh oh!

GeorgeDong32 left a comment

Uh oh!

GeorgeDong32 Mar 30, 2026

Uh oh!

GeorgeDong32 Mar 30, 2026

Uh oh!

Uh oh!

xr843 commented Mar 30, 2026

Uh oh!

EurFelux left a comment

Uh oh!

EurFelux Mar 31, 2026

Uh oh!

EurFelux Mar 31, 2026

Uh oh!

EurFelux Mar 31, 2026

Uh oh!

EurFelux Mar 31, 2026

Uh oh!

EurFelux Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

xr843 commented Mar 30, 2026

Summary

1. Unvalidated shell.openExternal() — CWE-78

2. Path Traversal in http://file/ handler — CWE-22

References

Uh oh!

GeorgeDong32 left a comment

Choose a reason for hiding this comment

Review Summary

安全修复覆盖度

需要关注的问题

路径遍历防护

其他

Uh oh!

GeorgeDong32 Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

GeorgeDong32 Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xr843 commented Mar 30, 2026

Uh oh!

EurFelux left a comment

Choose a reason for hiding this comment

Review Summary

需要修复

需要关注

Uh oh!

EurFelux Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

EurFelux Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

EurFelux Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

EurFelux Mar 31, 2026

Choose a reason for hiding this comment

1. file:// 协议 — HtmlArtifactsCard

2. x-apple.systempreferences: 协议 — MacProcessTrustHintModal

建议

Uh oh!

EurFelux Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

1. Unvalidated `shell.openExternal()` — CWE-78

2. Path Traversal in `http://file/` handler — CWE-22

1. `file://` 协议 — HtmlArtifactsCard

2. `x-apple.systempreferences:` 协议 — MacProcessTrustHintModal