feat(terminal): 在终端打开支持自定义选择终端应用(Ghostty/iTerm2 等)#569
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a configurable terminal launcher feature, unifying terminal launching logic across macOS, Linux, and Windows into a new terminalLauncher.ts utility, and adding corresponding configuration UI to the general settings page. The code review identified critical security vulnerabilities, specifically potential AppleScript injection in the macOS Terminal and iTerm2 presets, and shell injection in runAppleScript (recommending execFileAsync over execAsync). Additionally, the reviewer pointed out a bug in asynchronous error handling where returning promises without await bypasses the try-catch block, a type safety issue in parseCommandString, and provided corresponding test updates.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| build: (p) => ` | ||
| tell application "Terminal" | ||
| activate | ||
| do script "cd " & quoted form of "${p}" | ||
| end tell | ||
| ` |
There was a problem hiding this comment.
此处直接将路径 p 插值到 AppleScript 的双引号字符串中:quoted form of "${p}"。
如果路径 p 中包含双引号 " 或反斜杠 \,不仅会导致 AppleScript 语法解析错误,还可能引发 AppleScript 注入攻击(例如恶意构造的路径可以执行任意 AppleScript 代码)。
虽然使用了 quoted form of,但它是在 AppleScript 运行时对 Shell 进行转义,无法保护 AppleScript 自身的字符串字面量免受注入。
建议在插值前,对路径中的反斜杠 \ 和双引号 " 进行安全转义。
build: (p) => {
const safePath = p.replaceAll('\\', '\\\\').replaceAll('\"', '\\\"')
return `
tell application "Terminal"
activate
do script "cd " & quoted form of "${safePath}"
end tell
`
}| build: (p) => ` | ||
| tell application "iTerm" | ||
| activate | ||
| tell (create window with default profile) | ||
| write session "cd " & quoted form of "${p}" | ||
| end tell | ||
| end tell | ||
| ` |
There was a problem hiding this comment.
与 Terminal 预设类似,此处直接将路径 p 插值到 AppleScript 的双引号字符串中,存在 AppleScript 注入攻击 的风险。
建议在插值前,对路径中的反斜杠 \ 和双引号 " 进行安全转义。
build: (p) => {
const safePath = p.replaceAll('\\', '\\\\').replaceAll('\"', '\\\"')
return `
tell application "iTerm"
activate
tell (create window with default profile)
write session "cd " & quoted form of "${safePath}"
end tell
end tell
`
}| async function runAppleScript(script: string): Promise<void> { | ||
| const escaped = script.replace(/'/g, "'\\''") | ||
| await execAsync(`osascript -e '${escaped}'`) | ||
| } |
| if (terminal === 'custom') { | ||
| const parsed = parseCustomCommand(customCommand ?? '') | ||
| if (parsed) { | ||
| return runCli(parsed.command, applyPathToArgs(parsed.args, folderPath)) | ||
| } | ||
| // 自定义命令为空 → 回退默认 | ||
| return executePreset(resolvePreset('default', process.platform), folderPath) | ||
| } | ||
|
|
||
| return executePreset(resolvePreset(terminal, process.platform), folderPath) |
There was a problem hiding this comment.
在 async 函数中,如果在 try-catch 块内直接 return 一个 Promise(例如 runCli 或 executePreset 的返回值)而没有使用 await,那么当该 Promise 发生 reject 时,当前函数内的 catch 块将无法捕获到该异常。异常会直接抛给调用方,导致此处的 try-catch 容错逻辑失效。
建议在 return 时加上 await,以确保 openInTerminal 内部的 catch 块能够正确捕获并处理启动失败的异常,安全地返回 false。
// 自定义命令优先
if (terminal === 'custom') {
const parsed = parseCustomCommand(customCommand ?? '')
if (parsed) {
return await runCli(parsed.command, applyPathToArgs(parsed.args, folderPath))
}
// 自定义命令为空 → 回退默认
return await executePreset(resolvePreset('default', process.platform), folderPath)
}
return await executePreset(resolvePreset(terminal, process.platform), folderPath)| // ==================== 命令字符串解析(内联,避免耦合 commandLauncher)==================== | ||
|
|
||
| /** 将命令字符串拆分为 [可执行文件, 参数列表],处理引号 */ | ||
| function parseCommandString(cmd: string): [string, string[]] { |
There was a problem hiding this comment.
在 parseCommandString 中,如果输入的命令为空或仅包含空格,parts 数组将为空,此时 parts[0] 为 undefined。
然而,该函数的返回类型声明为 [string, string[]],这在类型安全上存在隐患(运行时 parts[0] 可能为 undefined,但 TypeScript 认为它是 string)。
建议将返回类型修改为 [string | undefined, string[]],以确保类型安全。
| function parseCommandString(cmd: string): [string, string[]] { | |
| function parseCommandString(cmd: string): [string | undefined, string[]] { |
| import { exec, spawn } from 'child_process' | ||
| import { promisify } from 'util' | ||
| import databaseAPI from '../api/shared/database' | ||
|
|
||
| const execAsync = promisify(exec) |
There was a problem hiding this comment.
为了更安全地执行 osascript,建议引入 execFile 并使用 promisify 将其包装为异步函数。
| import { exec, spawn } from 'child_process' | |
| import { promisify } from 'util' | |
| import databaseAPI from '../api/shared/database' | |
| const execAsync = promisify(exec) | |
| import { exec, spawn, execFile } from 'child_process' | |
| import { promisify } from 'util' | |
| import databaseAPI from '../api/shared/database' | |
| const execAsync = promisify(exec) | |
| const execFileAsync = promisify(execFile) |
| const { mockDbGet, mockSpawn, mockExec } = vi.hoisted(() => ({ | ||
| mockDbGet: vi.fn(), | ||
| mockSpawn: vi.fn(), | ||
| mockExec: vi.fn((...args: unknown[]) => { | ||
| const cb = args[args.length - 1] | ||
| if (typeof cb === 'function') cb(null, { stdout: '', stderr: '' }) | ||
| }) | ||
| })) | ||
|
|
||
| vi.mock('child_process', () => ({ spawn: mockSpawn, exec: mockExec })) | ||
| vi.mock('../../src/main/api/shared/database', () => ({ | ||
| default: { dbGet: mockDbGet, dbPut: vi.fn() } | ||
| })) |
There was a problem hiding this comment.
由于主进程代码中建议将 runAppleScript 改为使用更安全的 execFile,我们需要在测试中 mock 并断言 execFile 的调用,以确保单测能够顺利通过。
const { mockDbGet, mockSpawn, mockExec, mockExecFile } = vi.hoisted(() => ({
mockDbGet: vi.fn(),
mockSpawn: vi.fn(),
mockExec: vi.fn((...args: unknown[]) => {
const cb = args[args.length - 1]
if (typeof cb === 'function') cb(null, { stdout: '', stderr: '' })
}),
mockExecFile: vi.fn((...args: unknown[]) => {
const cb = args[args.length - 1]
if (typeof cb === 'function') cb(null, { stdout: '', stderr: '' })
})
}))
vi.mock('child_process', () => ({ spawn: mockSpawn, exec: mockExec, execFile: mockExecFile }))
vi.mock('../../src/main/api/shared/database', () => ({
default: { dbGet: mockDbGet, dbPut: vi.fn() }
}))| it('自定义命令为空 → 回退默认(mac 走 applescript,不 spawn)', async () => { | ||
| mockDbGet.mockReturnValue({ terminal: 'custom', terminalCustomCommand: '' }) | ||
| const ok = await openInTerminal('/x') | ||
| expect(ok).toBe(true) | ||
| expect(mockSpawn).not.toHaveBeenCalled() | ||
| expect(mockExec).toHaveBeenCalled() | ||
| // execAsync(`osascript -e '${escaped}'`) → exec(cmd, callback),cmd 内含 folderPath | ||
| expect(mockExec).toHaveBeenCalledWith(expect.stringContaining('/x'), expect.anything()) | ||
| }) |
There was a problem hiding this comment.
配合 runAppleScript 改用 execFile 的改动,此处断言也应从 mockExec 改为 mockExecFile。
it('自定义命令为空 → 回退默认(mac 走 applescript,不 spawn)', async () => {
mockDbGet.mockReturnValue({ terminal: 'custom', terminalCustomCommand: '' })
const ok = await openInTerminal('/x')
expect(ok).toBe(true)
expect(mockSpawn).not.toHaveBeenCalled()
expect(mockExecFile).toHaveBeenCalled()
// execFileAsync('osascript', ['-e', script]) → execFile(cmd, args, callback)
expect(mockExecFile).toHaveBeenCalledWith(
'osascript',
expect.arrayContaining([expect.stringContaining('/x')]),
expect.anything()
)
})- runAppleScript 改用 execFileAsync 直调 osascript,不经 shell,杜绝 shell 注入 - Terminal/iTerm2 预设路径经 escapeAppleScriptString 转义反斜杠与双引号,防 AppleScript 注入 - openInTerminal 内 return 改为 await,使 reject 能被 try/catch 捕获返回 false - parseCommandString 返回类型改为 [string | undefined, string[]] 修正类型安全 - 测试 mock 改 execFile 并断言 osascript 调用,新增 escapeAppleScriptString 测试
|
感谢 @gemini-code-assist 的评审,所有意见已在 commit 9c3453c 修复并通过验证( 逐条对应:
额外:为 |
|
感谢你的详细反馈和高质量的修复! 你不仅解决了之前指出的 AppleScript 注入和 Promise 异步处理问题,还通过引入 从代码结构来看,将终端启动逻辑统一到 目前的实现已经非常完善,完全符合高质量代码的标准。期待你的后续合并! |
Summary
「在终端打开」此前在 macOS 上硬编码系统 Terminal.app,无法选择其他终端。本 PR 将其做成可配置功能,并统一了两处重复的终端启动逻辑。
src/main/utils/terminalLauncher.ts:统一终端启动模块。包含三平台预设注册表(macOS: Terminal/Ghostty/iTerm2;Linux: gnome-terminal/konsole/xterm;Windows: wt/powershell/cmd)、自定义命令模板({path}占位符)、纯函数(可单测)与openInTerminal编排函数(读settings-general配置分发)。systemCommands.handleOpenTerminal与appleScriptHelper.openInTerminal都改为委托terminalLauncher.openInTerminal,删除迁出的 5 个函数(openTerminalOnMac/openTerminalOnLinux/tryLaunchWindowsTerminal/escapePowerShellPath/escapeCmdPath)。terminal='default',行为与改动前完全一致(Terminal.app / Linux 回退链 / Windows 回退链)。runAppleScript单引号转义修复了旧openTerminalOnMac的潜在 shell 注入;CLI/自定义命令用spawn(非 shell),{path}是 argv 级替换。Closes #568
设计与计划
docs/superpowers/specs/2026-07-02-configurable-terminal-launcher-design.mddocs/superpowers/plans/2026-07-02-configurable-terminal-launcher.mdTest Plan
pnpm typecheck:node干净(主进程)pnpm test tests/main/terminalLauncher.test.ts—— 23 测试全过(纯函数 + 编排分发 + 路径转义)pnpm test零新增失败(2 个预存在失败pluginRemovalCleanup/databasePluginIsolation在 main 上同样失败,已确认无关)pnpm build:setting(沙箱环境缺unocss未能验证,需在正常环境确认设置插件构建)pnpm build整体构建通过open -na Ghostty.app --args --working-directory={path}→ 同样打开 Ghostty与计划的已知偏离
linuxLauncher.ts的parseCommandString):避免把linuxLauncher的dialog/WindowManager/fs重依赖传染进 terminalLauncher,保持其为干净叶子模块。