diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f7b4a707..eb8fb8f0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -163,6 +163,7 @@ ### Security +- Limit the programs the desktop app may start to the exact command lines of the supported coding agents, the MCP server, and the Harness companion. On Windows the app could run any command through `cmd /c`, so any code running in the editor's webview could start arbitrary programs. - Update the desktop app to Tauri 2.12, which binds large IPC channel responses to the webview that requested them instead of letting another webview fetch them (GHSA-w28w-mhc8-qvjv). - Install a desktop update only when its signature names the version the update server announces, so a tampered update manifest cannot pair a newer version number with an older signed build. - Update `@xmldom/xmldom` to 0.9.12, which fixes quadratic-time and quadratic-memory parsing of crafted SVG and XML and reports malformed end tags instead of accepting them. diff --git a/desktop/AGENTS.md b/desktop/AGENTS.md index 5a8a719e9..8bacb5d2c 100644 --- a/desktop/AGENTS.md +++ b/desktop/AGENTS.md @@ -6,6 +6,7 @@ Check `desktop/Cargo.toml`, `desktop/capabilities/**`, and `desktop/tauri.conf.j - Dev tools: add or use a menu item to toggle them; do not rely on keyboard shortcuts. - `desktop/src/credentials.rs` stores secrets in the native system credential store; failures must surface, never fall back to browser or plaintext storage (`src/AGENTS.md`, Settings). - ACP and harness process changes require checking `desktop/capabilities/**`. +- A `shell:allow-spawn` entry pins the whole command line: no `"args": true`, and a Windows `.cmd` shim runs through its own `cmd-` entry with fixed `/c …` arguments, which `resolvePlatformCommand` selects (`tests/engine/tauri/command.test.ts`). - `desktop/generated/menu.json` is produced by `bun run generate:tauri-menu` from `src/app/shell/menu/schema.ts`; do not edit or import it directly. - Run `bun run generate:icons --target desktop` before direct Cargo checks; native icons are generated, not committed. - `build_fig_file` performs `.fig` export on desktop; the browser path uses fflate (`packages/fig/AGENTS.md`). diff --git a/desktop/capabilities/default.json b/desktop/capabilities/default.json index 86ca5fc2b..234640066 100644 --- a/desktop/capabilities/default.json +++ b/desktop/capabilities/default.json @@ -68,12 +68,16 @@ { "identifier": "shell:allow-spawn", "allow": [ - { "name": "claude-agent-acp", "cmd": "claude-agent-acp", "args": true }, - { "name": "codex-acp", "cmd": "codex-acp", "args": true }, - { "name": "gemini", "cmd": "gemini", "args": true }, - { "name": "openpencil-mcp-http", "cmd": "openpencil-mcp-http", "args": true }, + { "name": "claude-agent-acp", "cmd": "claude-agent-acp", "args": false }, + { "name": "codex-acp", "cmd": "codex-acp", "args": false }, + { "name": "gemini", "cmd": "gemini", "args": ["--acp"] }, + { "name": "openpencil-mcp-http", "cmd": "openpencil-mcp-http", "args": false }, { "name": "openpencil-harness", "cmd": "openpencil-harness", "args": false }, - { "name": "cmd", "cmd": "cmd", "args": true } + { "name": "cmd-claude-agent-acp", "cmd": "cmd", "args": ["/c", "claude-agent-acp"] }, + { "name": "cmd-codex-acp", "cmd": "cmd", "args": ["/c", "codex-acp"] }, + { "name": "cmd-gemini", "cmd": "cmd", "args": ["/c", "gemini", "--acp"] }, + { "name": "cmd-openpencil-mcp-http", "cmd": "cmd", "args": ["/c", "openpencil-mcp-http"] }, + { "name": "cmd-openpencil-harness", "cmd": "cmd", "args": ["/c", "openpencil-harness"] } ] }, "shell:allow-stdin-write", diff --git a/src/app/tauri/command.ts b/src/app/tauri/command.ts index c2ad1e054..f36c1996e 100644 --- a/src/app/tauri/command.ts +++ b/src/app/tauri/command.ts @@ -6,8 +6,12 @@ * `openpencil-mcp-http`) are `.cmd` shims. The Rust spawner behind * `@tauri-apps/plugin-shell` only resolves real executables, so launching a * `.cmd` directly fails with ENOENT. Routing through `cmd /c` lets the shell - * resolve the shim via PATHEXT. `cmd` is allowlisted in - * desktop/capabilities/default.json. + * resolve the shim via PATHEXT. + * + * `command` is the name of a shell scope entry in desktop/capabilities/default.json, and + * each scope entry pins the whole command line. On Windows the entry for `name` is + * `cmd-`, which runs exactly `cmd /c `: a scope that let `cmd` take any + * arguments would let the webview run any command. * * The `userAgent` argument is injectable for testing; it defaults to the live * navigator value at runtime, falling back to an empty string in non-browser @@ -24,7 +28,7 @@ export function resolvePlatformCommand( userAgent: string = detectUserAgent() ): { command: string; args: string[] } { if (userAgent.includes('Windows')) { - return { command: 'cmd', args: ['/c', command, ...args] } + return { command: `cmd-${command}`, args: ['/c', command, ...args] } } return { command, args } } diff --git a/tests/engine/tauri/command.test.ts b/tests/engine/tauri/command.test.ts index 929a9af61..d09b44474 100644 --- a/tests/engine/tauri/command.test.ts +++ b/tests/engine/tauri/command.test.ts @@ -1,23 +1,30 @@ import { describe, expect, test } from 'bun:test' +import { readFileSync } from 'node:fs' + +import * as v from 'valibot' + +import { ACP_AGENTS } from '@open-pencil/core/constants' import { resolvePlatformCommand } from '@/app/tauri/command' +import { repoPath } from '#tests/helpers/paths' + const WINDOWS_UA = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36 Edg/120.0.0.0' const MAC_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko)' describe('resolvePlatformCommand', () => { - test('wraps a bare command in cmd /c on Windows', () => { + test('wraps a bare command in cmd /c through its own scope entry on Windows', () => { expect(resolvePlatformCommand('claude-agent-acp', [], WINDOWS_UA)).toEqual({ - command: 'cmd', + command: 'cmd-claude-agent-acp', args: ['/c', 'claude-agent-acp'] }) }) test('preserves extra args after the command on Windows', () => { expect(resolvePlatformCommand('gemini', ['--acp'], WINDOWS_UA)).toEqual({ - command: 'cmd', + command: 'cmd-gemini', args: ['/c', 'gemini', '--acp'] }) }) @@ -43,3 +50,67 @@ describe('resolvePlatformCommand', () => { }) }) }) + +const ShellScope = v.object({ + permissions: v.array( + v.union([ + v.string(), + v.object({ + identifier: v.string(), + allow: v.optional( + v.array( + v.object({ + name: v.optional(v.string()), + cmd: v.optional(v.string()), + args: v.optional(v.union([v.boolean(), v.array(v.unknown())])) + }) + ) + ) + }) + ]) + ) +}) + +function spawnScope() { + const text = readFileSync(repoPath('desktop/capabilities/default.json'), 'utf8') + const capability = v.parse(v.pipe(v.string(), v.parseJson(), ShellScope), text) + const spawn = capability.permissions.find( + (permission) => typeof permission !== 'string' && permission.identifier === 'shell:allow-spawn' + ) + return typeof spawn === 'string' ? [] : (spawn?.allow ?? []) +} + +describe('shell scope', () => { + // Every program the app starts, with the arguments it starts it with. + const spawns: [string, string[]][] = [ + ...ACP_AGENTS.map((agent): [string, string[]] => [agent.command, agent.args]), + ['openpencil-mcp-http', []], + ['openpencil-harness', []] + ] + + for (const userAgent of [WINDOWS_UA, MAC_UA]) { + for (const [name, args] of spawns) { + test(`allows exactly ${name} ${args.join(' ')} on ${userAgent === MAC_UA ? 'macOS' : 'Windows'}`, () => { + const resolved = resolvePlatformCommand(name, args, userAgent) + const entry = spawnScope().find((candidate) => candidate.name === resolved.command) + expect(entry?.cmd).toBe(userAgent === WINDOWS_UA ? 'cmd' : name) + expect(entry?.args === false ? [] : entry?.args).toEqual(resolved.args) + }) + } + } + + test('allows nothing but those programs', () => { + const expected = [WINDOWS_UA, MAC_UA].flatMap((userAgent) => + spawns.map(([name, args]) => resolvePlatformCommand(name, args, userAgent).command) + ) + expect( + spawnScope() + .map((entry) => entry.name) + .sort() + ).toEqual([...new Set(expected)].sort()) + }) + + test('lets no program take arbitrary arguments', () => { + expect(spawnScope().filter((entry) => entry.args === true)).toEqual([]) + }) +}) diff --git a/tests/engine/tauri/harness-process.test.ts b/tests/engine/tauri/harness-process.test.ts index d48d195de..b5622964e 100644 --- a/tests/engine/tauri/harness-process.test.ts +++ b/tests/engine/tauri/harness-process.test.ts @@ -52,7 +52,7 @@ describe('Harness sidecar process', () => { ).not.toContain('secret') }) - test('routes the npm launcher through cmd on Windows', async () => { + test('routes the npm launcher through its cmd scope entry on Windows', async () => { const originalNavigator = Object.getOwnPropertyDescriptor(globalThis, 'navigator') try { Object.defineProperty(globalThis, 'navigator', { @@ -62,7 +62,7 @@ describe('Harness sidecar process', () => { await mockTauriIPC((cmd, args) => { if (cmd === 'plugin:shell|spawn') { expect(args).toMatchObject({ - program: 'cmd', + program: 'cmd-openpencil-harness', args: ['/c', 'openpencil-harness'], options: { encoding: 'raw', env: {} } }) diff --git a/tests/engine/tauri/processes.test.ts b/tests/engine/tauri/processes.test.ts index e9d08288d..60c129bc5 100644 --- a/tests/engine/tauri/processes.test.ts +++ b/tests/engine/tauri/processes.test.ts @@ -61,7 +61,7 @@ describe('Tauri process helpers', () => { expect(calls[2]?.args).toEqual({ cmd: 'killChild', pid: 42 }) }) - test('starts Windows ACP command shims through cmd', async () => { + test('starts Windows ACP command shims through their cmd scope entry', async () => { Object.defineProperty(globalThis, 'navigator', { configurable: true, value: { userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' } @@ -69,7 +69,7 @@ describe('Tauri process helpers', () => { await mockTauriIPC((cmd, args) => { if (cmd === 'plugin:shell|spawn') { expect(args).toMatchObject({ - program: 'cmd', + program: 'cmd-agent-cli', args: ['/c', 'agent-cli', '--stdio'], options: { encoding: 'raw', env: {} } })