From 9e937df8b264a9f3ec1133a11336418329c6965b Mon Sep 17 00:00:00 2001 From: "Bing.Z" Date: Mon, 21 Sep 2026 08:30:15 +0800 Subject: [PATCH 1/2] fix(cli): protect dollar signs from PowerShell expansion in computer follow-up commands --- src/cli/computer-format.test.ts | 27 +++++++++++++++ src/cli/computer-format.ts | 9 +++-- src/cli/shell-command-quote.test.ts | 51 ++++++++++++++++++++++++++++- src/cli/shell-command-quote.ts | 37 +++++++++++++++++++-- 4 files changed, 117 insertions(+), 7 deletions(-) diff --git a/src/cli/computer-format.test.ts b/src/cli/computer-format.test.ts index 2945ba537299..a87ef93eab22 100644 --- a/src/cli/computer-format.test.ts +++ b/src/cli/computer-format.test.ts @@ -104,4 +104,31 @@ describe('formatComputerAction', () => { expect(output).toContain('Inspect with the command above') expect(output).not.toContain('Click completed') }) + + it('quotes worktree selectors safely for PowerShell follow-up command', () => { + const result: ComputerActionResult = { + snapshot: { + id: 'snap-1', + app: { name: 'App', bundleId: 'com.example.app', pid: 100 }, + window: { title: 'App', id: 42, width: 800, height: 600 }, + coordinateSpace: 'window', + treeText: 'tree', + elementCount: 5, + focusedElementId: null + }, + screenshot: null, + screenshotStatus: { state: 'skipped', reason: 'no_screenshot_flag' }, + action: { + path: 'synthetic', + verification: { state: 'verified', property: 'value' } + } + } + + const output = formatComputerAction('click', result, { + worktree: 'path:C:/work/$review', + shell: 'powershell' + }) + + expect(output).toContain("--worktree 'path:C:/work/$review'") + }) }) diff --git a/src/cli/computer-format.ts b/src/cli/computer-format.ts index 3ec34c9cfbcf..d11733b67f41 100644 --- a/src/cli/computer-format.ts +++ b/src/cli/computer-format.ts @@ -19,6 +19,7 @@ import type { ComputerSnapshotResult } from '../shared/runtime-types' import type { RuntimeRpcSuccess } from './runtime-client' +import type { AgentStartupShell } from '../shared/tui-agent-startup-shell' export function formatGetAppState(result: ComputerSnapshotResult): string { const app = result.snapshot.app @@ -201,6 +202,7 @@ export type ComputerActionFollowUpTarget = { windowId?: number windowIndex?: number restoreWindow?: boolean + shell?: AgentStartupShell } export function formatComputerAction( @@ -224,17 +226,18 @@ function formatComputerFollowUpCommand( result: ComputerActionResult, target: ComputerActionFollowUpTarget ): string { + const quote = (arg: string): string => quoteCliCommandArgument(arg, target.shell) const args = [ 'orca', 'computer', 'get-app-state', '--app', - quoteCliCommandArgument(result.snapshot.app.bundleId ?? result.snapshot.app.name) + quote(result.snapshot.app.bundleId ?? result.snapshot.app.name) ] if (target.session) { - args.push('--session', quoteCliCommandArgument(target.session)) + args.push('--session', quote(target.session)) } else if (target.worktree) { - args.push('--worktree', quoteCliCommandArgument(target.worktree)) + args.push('--worktree', quote(target.worktree)) } const windowChanged = result.action?.verification?.state === 'unverified' && diff --git a/src/cli/shell-command-quote.test.ts b/src/cli/shell-command-quote.test.ts index 06e47245af2a..6c64cb01e1b5 100644 --- a/src/cli/shell-command-quote.test.ts +++ b/src/cli/shell-command-quote.test.ts @@ -15,6 +15,55 @@ describe('quoteCliCommandArgument', () => { expect(quoteCliCommandArgument('Text Editor')).toBe("'Text Editor'") vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') - expect(quoteCliCommandArgument('Text Editor')).toBe('"Text Editor"') + expect(quoteCliCommandArgument('Text Editor')).toBe("'Text Editor'") + }) + + it('quotes values for specific Windows shell families', () => { + expect(quoteCliCommandArgument('Text Editor', { shell: 'powershell', platform: 'win32' })).toBe( + "'Text Editor'" + ) + + expect(quoteCliCommandArgument('Text Editor', { shell: 'cmd', platform: 'win32' })).toBe( + '"Text Editor"' + ) + }) + + it('protects dollar signs from PowerShell variable expansion', () => { + expect( + quoteCliCommandArgument('path:C:/work/$review', { shell: 'powershell', platform: 'win32' }) + ).toBe("'path:C:/work/$review'") + + expect( + quoteCliCommandArgument('path:C:/work/$review', { shell: 'cmd', platform: 'win32' }) + ).toBe('"path:C:/work/$review"') + }) + + it('detects shell family from environment variables on win32', () => { + expect( + quoteCliCommandArgument('path:C:/work/$review', { + platform: 'win32', + env: { ORCA_TERMINAL_WINDOWS_SHELL: 'powershell.exe' } + }) + ).toBe("'path:C:/work/$review'") + + expect( + quoteCliCommandArgument('path:C:/work/$review', { + platform: 'win32', + env: { ORCA_TERMINAL_WINDOWS_SHELL: 'cmd.exe' } + }) + ).toBe('"path:C:/work/$review"') + + expect( + quoteCliCommandArgument('path:C:/work/$review', { + platform: 'win32', + env: { ORCA_WINDOWS_SHELL: 'cmd.exe' } + }) + ).toBe('"path:C:/work/$review"') + }) + + it('escapes embedded quotes correctly for PowerShell', () => { + expect( + quoteCliCommandArgument("path:C:/work/$it's", { shell: 'powershell', platform: 'win32' }) + ).toBe("'path:C:/work/$it''s'") }) }) diff --git a/src/cli/shell-command-quote.ts b/src/cli/shell-command-quote.ts index c5dff7f62a39..ea1e44e0a0a1 100644 --- a/src/cli/shell-command-quote.ts +++ b/src/cli/shell-command-quote.ts @@ -1,9 +1,40 @@ -export function quoteCliCommandArgument(value: string): string { +import type { AgentStartupShell } from '../shared/tui-agent-startup-shell' +import { quoteWindowsCmdArgument } from '../shared/child-process/windows-command-line' +import { quotePowerShellNativeArgument } from '../shared/powershell-native-argument' +import { resolveWindowsShellStartupFamily } from '../shared/windows-terminal-shell' + +export type ShellQuoteOptions = { + platform?: NodeJS.Platform + shell?: AgentStartupShell + env?: NodeJS.ProcessEnv +} + +function resolveCliShell(platform: NodeJS.Platform, env: NodeJS.ProcessEnv): AgentStartupShell { + if (platform !== 'win32') { + return 'posix' + } + const configured = env.ORCA_TERMINAL_WINDOWS_SHELL ?? env.ORCA_WINDOWS_SHELL + return resolveWindowsShellStartupFamily(configured) +} + +export function quoteCliCommandArgument( + value: string, + options?: AgentStartupShell | ShellQuoteOptions +): string { if (/^[a-zA-Z0-9._:/@-]+$/.test(value)) { return value } - if (process.platform === 'win32') { - return `"${value.replace(/"/g, '\\"')}"` + const opts = typeof options === 'string' ? { shell: options } : (options ?? {}) + const platform = opts.platform ?? process.platform + const env = opts.env ?? process.env + const shell = opts.shell ?? resolveCliShell(platform, env) + + if (shell === 'powershell') { + // Why: PowerShell expands `$var` in double quotes; quote with native-safe single quotes. + return quotePowerShellNativeArgument(value) + } + if (shell === 'cmd') { + return quoteWindowsCmdArgument(value) } return `'${value.replaceAll("'", "'\\''")}'` } From a2fbec5f3c998f6201a98c2956e48dfcf43dadce Mon Sep 17 00:00:00 2001 From: "Bing.Z" Date: Wed, 7 Oct 2026 17:33:03 +0800 Subject: [PATCH 2/2] fix(cli): quote follow-up arguments for both PowerShell and cmd The Windows shell env vars were never set, so cmd.exe users got PowerShell single quotes. Emit one spelling that keeps $ literal in PowerShell without splitting cmd.exe, and drop the unused shell field. --- src/cli/computer-format.test.ts | 14 +-- src/cli/computer-format.ts | 9 +- src/cli/shell-command-quote.test.ts | 61 +++---------- src/cli/shell-command-quote.ts | 128 +++++++++++++++++++++------- 4 files changed, 123 insertions(+), 89 deletions(-) diff --git a/src/cli/computer-format.test.ts b/src/cli/computer-format.test.ts index a87ef93eab22..63b62aad1bc4 100644 --- a/src/cli/computer-format.test.ts +++ b/src/cli/computer-format.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import type { ComputerActionResult } from '../shared/runtime-types' import { formatComputerAction, prepareComputerCliJsonResult } from './computer-format' import { printResult } from './format' @@ -54,6 +54,10 @@ describe('prepareComputerCliJsonResult', () => { }) describe('formatComputerAction', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + it('does not treat legacy action results without metadata as completed', () => { const result: ComputerActionResult = { snapshot: { @@ -105,7 +109,7 @@ describe('formatComputerAction', () => { expect(output).not.toContain('Click completed') }) - it('quotes worktree selectors safely for PowerShell follow-up command', () => { + it('quotes worktree selectors so a pasted Windows command keeps dollar signs', () => { const result: ComputerActionResult = { snapshot: { id: 'snap-1', @@ -124,11 +128,11 @@ describe('formatComputerAction', () => { } } + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') const output = formatComputerAction('click', result, { - worktree: 'path:C:/work/$review', - shell: 'powershell' + worktree: 'path:C:/work/$review' }) - expect(output).toContain("--worktree 'path:C:/work/$review'") + expect(output).toContain('--worktree path:C:/work/$"r"eview') }) }) diff --git a/src/cli/computer-format.ts b/src/cli/computer-format.ts index d11733b67f41..3ec34c9cfbcf 100644 --- a/src/cli/computer-format.ts +++ b/src/cli/computer-format.ts @@ -19,7 +19,6 @@ import type { ComputerSnapshotResult } from '../shared/runtime-types' import type { RuntimeRpcSuccess } from './runtime-client' -import type { AgentStartupShell } from '../shared/tui-agent-startup-shell' export function formatGetAppState(result: ComputerSnapshotResult): string { const app = result.snapshot.app @@ -202,7 +201,6 @@ export type ComputerActionFollowUpTarget = { windowId?: number windowIndex?: number restoreWindow?: boolean - shell?: AgentStartupShell } export function formatComputerAction( @@ -226,18 +224,17 @@ function formatComputerFollowUpCommand( result: ComputerActionResult, target: ComputerActionFollowUpTarget ): string { - const quote = (arg: string): string => quoteCliCommandArgument(arg, target.shell) const args = [ 'orca', 'computer', 'get-app-state', '--app', - quote(result.snapshot.app.bundleId ?? result.snapshot.app.name) + quoteCliCommandArgument(result.snapshot.app.bundleId ?? result.snapshot.app.name) ] if (target.session) { - args.push('--session', quote(target.session)) + args.push('--session', quoteCliCommandArgument(target.session)) } else if (target.worktree) { - args.push('--worktree', quote(target.worktree)) + args.push('--worktree', quoteCliCommandArgument(target.worktree)) } const windowChanged = result.action?.verification?.state === 'unverified' && diff --git a/src/cli/shell-command-quote.test.ts b/src/cli/shell-command-quote.test.ts index 6c64cb01e1b5..49b33655c229 100644 --- a/src/cli/shell-command-quote.test.ts +++ b/src/cli/shell-command-quote.test.ts @@ -15,55 +15,22 @@ describe('quoteCliCommandArgument', () => { expect(quoteCliCommandArgument('Text Editor')).toBe("'Text Editor'") vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') - expect(quoteCliCommandArgument('Text Editor')).toBe("'Text Editor'") + // cmd.exe splits on spaces and does not treat single quotes as delimiters. + expect(quoteCliCommandArgument('Text Editor')).toBe('"Text Editor"') }) - it('quotes values for specific Windows shell families', () => { - expect(quoteCliCommandArgument('Text Editor', { shell: 'powershell', platform: 'win32' })).toBe( - "'Text Editor'" - ) - - expect(quoteCliCommandArgument('Text Editor', { shell: 'cmd', platform: 'win32' })).toBe( - '"Text Editor"' + it('keeps a dollar sign literal for both PowerShell and cmd.exe', () => { + vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + expect(quoteCliCommandArgument('path:C:/work/$review')).toBe('path:C:/work/$"r"eview') + expect(quoteCliCommandArgument('a b$c')).toBe('a" "b$"c"') + // cmd.exe splits on an unquoted &; single quotes would not hide it. + expect(quoteCliCommandArgument('a&b$c')).toBe('a"&"b$"c"') + expect(quoteCliCommandArgument('a$$b')).toBe('a$"$"b') + expect(quoteCliCommandArgument('path:C:/work/$$()')).toBe('path:C:/work/$"`$()"') + expect(quoteCliCommandArgument("path:C:/work/$r'x")).toBe(`path:C:/work/$"r'"x`) + expect(quoteCliCommandArgument('path:C:/work/\\\\\\ $review')).toBe( + 'path:C:/work/"\\\\\\ "$"r"eview' ) - }) - - it('protects dollar signs from PowerShell variable expansion', () => { - expect( - quoteCliCommandArgument('path:C:/work/$review', { shell: 'powershell', platform: 'win32' }) - ).toBe("'path:C:/work/$review'") - - expect( - quoteCliCommandArgument('path:C:/work/$review', { shell: 'cmd', platform: 'win32' }) - ).toBe('"path:C:/work/$review"') - }) - - it('detects shell family from environment variables on win32', () => { - expect( - quoteCliCommandArgument('path:C:/work/$review', { - platform: 'win32', - env: { ORCA_TERMINAL_WINDOWS_SHELL: 'powershell.exe' } - }) - ).toBe("'path:C:/work/$review'") - - expect( - quoteCliCommandArgument('path:C:/work/$review', { - platform: 'win32', - env: { ORCA_TERMINAL_WINDOWS_SHELL: 'cmd.exe' } - }) - ).toBe('"path:C:/work/$review"') - - expect( - quoteCliCommandArgument('path:C:/work/$review', { - platform: 'win32', - env: { ORCA_WINDOWS_SHELL: 'cmd.exe' } - }) - ).toBe('"path:C:/work/$review"') - }) - - it('escapes embedded quotes correctly for PowerShell', () => { - expect( - quoteCliCommandArgument("path:C:/work/$it's", { shell: 'powershell', platform: 'win32' }) - ).toBe("'path:C:/work/$it''s'") + expect(quoteCliCommandArgument(' $review')).toBe('" `$review"') }) }) diff --git a/src/cli/shell-command-quote.ts b/src/cli/shell-command-quote.ts index ea1e44e0a0a1..210e02587149 100644 --- a/src/cli/shell-command-quote.ts +++ b/src/cli/shell-command-quote.ts @@ -1,40 +1,106 @@ -import type { AgentStartupShell } from '../shared/tui-agent-startup-shell' -import { quoteWindowsCmdArgument } from '../shared/child-process/windows-command-line' -import { quotePowerShellNativeArgument } from '../shared/powershell-native-argument' -import { resolveWindowsShellStartupFamily } from '../shared/windows-terminal-shell' +// Literal unquoted in cmd.exe, and not a PowerShell token boundary. +const WINDOWS_BARE_SAFE = /^[A-Za-z0-9._:/@\\~*?!=+-]$/ -export type ShellQuoteOptions = { - platform?: NodeJS.Platform - shell?: AgentStartupShell - env?: NodeJS.ProcessEnv +export function quoteCliCommandArgument(value: string): string { + if (/^[a-zA-Z0-9._:/@-]+$/.test(value)) { + return value + } + if (process.platform === 'win32') { + return quoteWindowsCliArgument(value) + } + return `'${value.replaceAll("'", "'\\''")}'` } -function resolveCliShell(platform: NodeJS.Platform, env: NodeJS.ProcessEnv): AgentStartupShell { - if (platform !== 'win32') { - return 'posix' +function quoteWindowsCliArgument(value: string): string { + // No `$`: keep the historical double-quoted Windows spelling. + if (!value.includes('$')) { + return `"${value.replace(/"/g, '\\"')}"` + } + const first = value[0] ?? '' + const dotSource = first === '.' && (value[1] === '$' || value[1] === '"' || value[1] === "'") + // Why: `"`, `@`, and a dot-source `.` end the PowerShell token before the rest. + if (dotSource || first === '@' || (first !== '$' && !WINDOWS_BARE_SAFE.test(first))) { + return `"${value.replace(/[`$"]/g, (char) => `\`${char}`)}"` } - const configured = env.ORCA_TERMINAL_WINDOWS_SHELL ?? env.ORCA_WINDOWS_SHELL - return resolveWindowsShellStartupFamily(configured) + // Why `$"x"`: PowerShell expands `$` only when a variable name follows, and + // cmd concatenates a quoted character onto the surrounding text. + let quoted = '' + for (let index = 0; index < value.length; index += 1) { + const char = value[index] ?? '' + if (char === '$') { + const next = value[index + 1] + if (next && isPowerShellVariableStart(next)) { + let run = next + index += 2 + while ( + index < value.length && + value[index] !== '$' && + !WINDOWS_BARE_SAFE.test(value[index] ?? '') + ) { + run += value[index] + index += 1 + } + index -= 1 + // Why: `$"r""'"` is one quoted run, so the `""` becomes a literal quote. + quoted += `$${quoteWindowsRun(run)}` + continue + } + quoted += '$' + continue + } + if (WINDOWS_BARE_SAFE.test(char)) { + quoted += char + continue + } + let run = '' + while ( + index < value.length && + value[index] !== '$' && + !WINDOWS_BARE_SAFE.test(value[index] ?? '') + ) { + run += value[index] + index += 1 + } + index -= 1 + // Why: a `\` left outside the quotes is doubled for cmd and kept by PowerShell. + const slashes = quoted.match(/\\+$/) + if (slashes) { + quoted = quoted.slice(0, -slashes[0].length) + run = `${slashes[0]}${run}` + } + quoted += quoteWindowsRun(run) + } + return quoted } -export function quoteCliCommandArgument( - value: string, - options?: AgentStartupShell | ShellQuoteOptions -): string { - if (/^[a-zA-Z0-9._:/@-]+$/.test(value)) { - return value - } - const opts = typeof options === 'string' ? { shell: options } : (options ?? {}) - const platform = opts.platform ?? process.platform - const env = opts.env ?? process.env - const shell = opts.shell ?? resolveCliShell(platform, env) +function quoteWindowsRun(run: string): string { + return `"${escapeWindowsQuotedRun(run)}"` +} - if (shell === 'powershell') { - // Why: PowerShell expands `$var` in double quotes; quote with native-safe single quotes. - return quotePowerShellNativeArgument(value) - } - if (shell === 'cmd') { - return quoteWindowsCmdArgument(value) +function escapeWindowsQuotedRun(run: string): string { + let escaped = '' + for (let index = 0; index < run.length; index += 1) { + const char = run[index] ?? '' + const next = run[index + 1] + if (char === '"') { + escaped += '""' + continue + } + if (char === '`') { + escaped += '``' + continue + } + // Why: `"$()"` is a PowerShell subexpression. A final `$` is already literal. + if (char === '$' && next && (next === '(' || next === '{' || isPowerShellVariableStart(next))) { + escaped += '`$' + continue + } + escaped += char } - return `'${value.replaceAll("'", "'\\''")}'` + return escaped +} + +function isPowerShellVariableStart(char: string): boolean { + // Mirrors PowerShell VarNameFirst, plus other letters and digits. + return /^[A-Za-z0-9_?$^:]$/.test(char) || /[\p{L}\p{N}]/u.test(char) }