diff --git a/src/shared/hermes-startup-query.ts b/src/shared/hermes-startup-query.ts index 79dd818f1..60e270507 100644 --- a/src/shared/hermes-startup-query.ts +++ b/src/shared/hermes-startup-query.ts @@ -1,3 +1,4 @@ +import { encodePowerShellCommand } from './powershell-command-encoding' import { buildShellCommandFromArgv, quoteStartupArg, @@ -14,15 +15,6 @@ const POWERSHELL_NATIVE_QUERY_VARIABLE = 'orcaHermesNativeQuery' export const ORCA_HERMES_STARTUP_QUERY_ENV = 'ORCA_HERMES_STARTUP_QUERY' -function encodePowerShellCommand(command: string): string { - let bytes = '' - for (let index = 0; index < command.length; index += 1) { - const code = command.charCodeAt(index) - bytes += String.fromCharCode(code & 0xff, code >>> 8) - } - return btoa(bytes) -} - function encodePosixEvalScript(command: string): string { return Array.from( new TextEncoder().encode(command), diff --git a/src/shared/powershell-command-encoding.ts b/src/shared/powershell-command-encoding.ts index 3d8e36aa4..aa03c0699 100644 --- a/src/shared/powershell-command-encoding.ts +++ b/src/shared/powershell-command-encoding.ts @@ -1,3 +1,11 @@ export function encodePowerShellCommand(command: string): string { - return Buffer.from(command, 'utf16le').toString('base64') + // Why: some callers (setup sequencing, Hermes startup) run in the sandboxed + // renderer where Node's Buffer is unavailable, so encode the UTF-16LE bytes + // PowerShell's -EncodedCommand expects using only renderer-safe globals. + let bytes = '' + for (let index = 0; index < command.length; index += 1) { + const code = command.charCodeAt(index) + bytes += String.fromCharCode(code & 0xff, code >>> 8) + } + return btoa(bytes) } diff --git a/src/shared/setup-agent-sequencing.test.ts b/src/shared/setup-agent-sequencing.test.ts index ebb8ab3c9..9225249a3 100644 --- a/src/shared/setup-agent-sequencing.test.ts +++ b/src/shared/setup-agent-sequencing.test.ts @@ -1,5 +1,5 @@ import { spawn } from 'node:child_process' -import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { tmpdir } from 'node:os' @@ -173,33 +173,84 @@ describe('createSequencedSetupAgentCommands', () => { nonce: 'nonce-win', waitTimeoutSeconds: 3 }) + const setupPowerShell = decodePowerShellScript(result.setupCommand) + const startupPowerShell = decodePowerShellScript(result.startupCommand) - expect(result.setupCommand).toContain('cmd.exe /d /s /v:on /c') - expect(result.setupCommand).toContain('cmd.exe /c ""C:\\repo\\.git\\orca\\setup-runner.cmd""') - expect(result.setupCommand).toContain('echo !ORCA_SETUP_NONCE!:!ORCA_SETUP_STATUS!') + expect(result.setupCommand).toContain( + 'powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand' + ) + expect(setupPowerShell).toContain("$runner = 'C:\\repo\\.git\\orca\\setup-runner.cmd'") + expect(setupPowerShell).toContain('$nonce + ":" + $setupStatus') expect(result.startupCommand.match(/powershell\.exe/g)).toHaveLength(1) - expect(result.startupCommand).toContain('powershell.exe -NoProfile -ExecutionPolicy Bypass') - expect(result.startupCommand).toContain('AddSeconds(3)') - expect(result.startupCommand).toContain('!ORCA_SETUP_STATUS!') - expect(result.startupCommand).toContain('Timed out waiting for setup before starting agent.') - expect(result.startupCommand).toContain('Setup failed; skipping agent startup.') expect(result.startupCommand).toContain( + 'powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand' + ) + expect(startupPowerShell).toContain('AddSeconds(3)') + expect(startupPowerShell).toContain('Missing setup marker path.') + expect(startupPowerShell).toContain('Timed out waiting for setup before starting agent.') + expect(startupPowerShell).toContain('Setup failed; skipping agent startup.') + expect(startupPowerShell).toContain( 'Remove-Item -LiteralPath $marker, $tmp -Force -ErrorAction SilentlyContinue' ) expect(result.startupCommand).not.toContain('%ERRORLEVEL%') - expect(result.startupCommand).not.toContain(' & ) else') - expect(result.startupCommand).not.toContain('if ""!ORCA_SETUP_STATUS!""==""124""') - expect(result.startupCommand).not.toContain('if not ""!ORCA_SETUP_STATUS!""==""0""') - expect(result.startupCommand).not.toContain( - `call !${SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV}!` - ) - expect(result.startupCommand).toContain('Invoke-Expression') + expect(startupPowerShell).toContain('Invoke-Expression') expect(result.startupCommand).not.toContain('fix !PATH! & test') expect(result.startupEnv).toEqual({ [SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV]: "codex --model gpt-5 'fix !PATH! & test'" }) }) + it.skipIf(process.platform !== 'win32')( + 'executes the native Windows setup-to-agent sequence through cmd.exe', + async () => { + const tempDir = join(makeTempDir(), 'path with spaces') + mkdirSync(tempDir) + const runnerScriptPath = join(tempDir, 'setup runner.cmd') + const startupScriptPath = join(tempDir, 'agent-startup.cmd') + const logPath = join(tempDir, 'sequence.log') + + writeFileSync( + runnerScriptPath, + ['@echo off', `>> "${logPath}" echo setup-done`, 'exit /b 0'].join('\r\n'), + 'utf8' + ) + writeFileSync( + startupScriptPath, + ['@echo off', `>> "${logPath}" echo agent-start`, 'exit /b 0'].join('\r\n'), + 'utf8' + ) + + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath, + startupCommand: `cmd.exe /d /c "${startupScriptPath}"`, + platform: 'windows', + nonce: 'windows-sequence', + waitTimeoutSeconds: 2 + }) + + const setupExit = await waitForExit( + spawnWindowsCommand(tempDir, 'run-setup.cmd', commands.setupCommand) + ) + expect(setupExit.code).toBe(0) + expect(readIfExists(`${runnerScriptPath}.windows-sequence.done`)).toBe( + 'windows-sequence:0\r\n' + ) + + const startupExit = await waitForExit( + spawnWindowsCommand( + tempDir, + 'run-startup.cmd', + commands.startupCommand, + commands.startupEnv + ) + ) + + expect(startupExit.code).toBe(0) + expect(startupExit.stderr).toContain('Waiting for setup to finish before starting agent...') + expect(readFileSync(logPath, 'utf8')).toBe('setup-done\r\nagent-start\r\n') + } + ) + it.skipIf(process.platform === 'win32')( 'ignores stale markers until the matching setup run finishes, even when startup launches first', async () => { @@ -409,6 +460,30 @@ function sleep(ms: number): Promise { }) } +function spawnWindowsCommand( + dir: string, + filename: string, + command: string, + env: Record = {} +): ReturnType { + const scriptPath = join(dir, filename) + // Why: /s strips the quotes Node adds for batch paths containing spaces; + // argv spawning still exercises cmd.exe's native parser without that loss. + writeFileSync(scriptPath, `@echo off\r\n${command}\r\nexit /b %ERRORLEVEL%\r\n`, 'utf8') + return spawn('cmd.exe', ['/d', '/c', scriptPath], { + stdio: 'pipe', + env: { ...process.env, ...env } + }) +} + +function decodePowerShellScript(command: string): string { + const encoded = command.match(/-EncodedCommand\s+([A-Za-z0-9+/=]+)/)?.[1] + if (!encoded) { + throw new Error('Missing PowerShell encoded command') + } + return Buffer.from(encoded, 'base64').toString('utf16le') +} + function waitForExit( child: ReturnType ): Promise<{ code: number | null; stderr: string }> { diff --git a/src/shared/setup-agent-sequencing.ts b/src/shared/setup-agent-sequencing.ts index 33a5f96a6..88f6c11ac 100644 --- a/src/shared/setup-agent-sequencing.ts +++ b/src/shared/setup-agent-sequencing.ts @@ -1,3 +1,4 @@ +import { encodePowerShellCommand } from './powershell-command-encoding' import { resolveSetupRunnerCommand, type SetupRunnerCommandPlatform, @@ -45,7 +46,11 @@ export function createSequencedSetupAgentCommands(args: { if (resolution.shell === 'windows') { return { - setupCommand: buildWindowsSetupCommand(resolution.command, markerPath, nonce), + setupCommand: buildWindowsSetupCommand( + resolution.runnerScriptPathForShell, + markerPath, + nonce + ), startupCommand: buildWindowsStartupCommand(markerPath, nonce, waitTimeoutSeconds), startupEnv: { [SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV]: args.startupCommand @@ -165,17 +170,33 @@ function hasUnquotedPosixCommandSeparator(command: string): boolean { return false } -function buildWindowsSetupCommand(setupCommand: string, markerPath: string, nonce: string): string { - return wrapCmd([ - `set "ORCA_SETUP_MARKER=${escapeCmdSetValue(markerPath)}"`, - `set "ORCA_SETUP_NONCE=${escapeCmdSetValue(nonce)}"`, - 'del /f /q "!ORCA_SETUP_MARKER!" "!ORCA_SETUP_MARKER!.tmp" 2>nul', - `call ${setupCommand}`, - 'set "ORCA_SETUP_STATUS=!ERRORLEVEL!"', - '> "!ORCA_SETUP_MARKER!.tmp" echo !ORCA_SETUP_NONCE!:!ORCA_SETUP_STATUS!', - 'move /y "!ORCA_SETUP_MARKER!.tmp" "!ORCA_SETUP_MARKER!" >nul', - 'exit /b !ORCA_SETUP_STATUS!' - ]) +function buildWindowsSetupCommand( + runnerScriptPath: string, + markerPath: string, + nonce: string +): string { + // Why: delayed expansion keeps path metacharacters as data when cmd invokes the batch runner. + const script = [ + `$runner = ${quotePowerShellString(runnerScriptPath)}`, + `$marker = ${quotePowerShellString(markerPath)}`, + '$tmp = $marker + ".tmp"', + `$nonce = ${quotePowerShellString(nonce)}`, + 'Remove-Item -LiteralPath $marker, $tmp -Force -ErrorAction SilentlyContinue', + '$processInfo = [System.Diagnostics.ProcessStartInfo]::new()', + '$processInfo.FileName = $env:ComSpec', + '$processInfo.Arguments = \'/d /s /v:on /c ""!ORCA_SETUP_RUNNER!""\'', + '$processInfo.UseShellExecute = $false', + '$processInfo.EnvironmentVariables["ORCA_SETUP_RUNNER"] = $runner', + '$process = [System.Diagnostics.Process]::Start($processInfo)', + '$process.WaitForExit()', + '$setupStatus = $process.ExitCode', + '$utf8 = [System.Text.UTF8Encoding]::new($false)', + '[System.IO.File]::WriteAllText($tmp, ($nonce + ":" + $setupStatus + [Environment]::NewLine), $utf8)', + 'Move-Item -LiteralPath $tmp -Destination $marker -Force', + 'exit $setupStatus' + ].join('; ') + + return encodePowerShellInvocation(script) } function buildWindowsStartupCommand( @@ -187,10 +208,15 @@ function buildWindowsStartupCommand( // Why: native Windows setup runners launch through cmd.exe, but PowerShell // gives us safe bounded file polling/parsing without a fragile batch label loop. const script = [ - '$marker = $env:ORCA_SETUP_MARKER', + `$marker = ${quotePowerShellString(markerPath)}`, + 'if ([string]::IsNullOrWhiteSpace($marker)) {', + ' [Console]::Error.WriteLine("Missing setup marker path.")', + ' exit 1', + '}', '$tmp = $marker + ".tmp"', - '$nonce = $env:ORCA_SETUP_NONCE', + `$nonce = ${quotePowerShellString(nonce)}`, `$deadline = (Get-Date).AddSeconds(${timeout})`, + '[Console]::Error.WriteLine("Waiting for setup to finish before starting agent...")', 'while ($true) {', ' if (Test-Path -LiteralPath $marker) {', ' $content = Get-Content -LiteralPath $marker -TotalCount 1', @@ -220,18 +246,11 @@ function buildWindowsStartupCommand( '}' ].join('; ') - return wrapCmd([ - `set "ORCA_SETUP_MARKER=${escapeCmdSetValue(markerPath)}"`, - `set "ORCA_SETUP_NONCE=${escapeCmdSetValue(nonce)}"`, - 'echo Waiting for setup to finish before starting agent... 1>&2', - `powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ${quoteWindowsArg(script)}`, - 'set "ORCA_SETUP_STATUS=!ERRORLEVEL!"', - 'exit /b !ORCA_SETUP_STATUS!' - ]) + return encodePowerShellInvocation(script) } -function wrapCmd(parts: string[]): string { - return `cmd.exe /d /s /v:on /c ${quoteWindowsArg(parts.join(' & '))}` +function encodePowerShellInvocation(script: string): string { + return `powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand ${encodePowerShellCommand(script)}` } function quotePosixArg(value: string): string { @@ -241,12 +260,8 @@ function quotePosixArg(value: string): string { return `'${value.replace(/'/g, `'\\''`)}'` } -function quoteWindowsArg(value: string): string { - return `"${value.replace(/"/g, '""')}"` -} - -function escapeCmdSetValue(value: string): string { - return value.replace(/"/g, '""').replace(/[%!^]/g, (char) => `^${char}`) +function quotePowerShellString(value: string): string { + return `'${value.replace(/'/g, "''")}'` } export function getSetupAgentSequenceShellForTests( diff --git a/src/shared/setup-agent-sequencing.windows.test.ts b/src/shared/setup-agent-sequencing.windows.test.ts new file mode 100644 index 000000000..b964eb241 --- /dev/null +++ b/src/shared/setup-agent-sequencing.windows.test.ts @@ -0,0 +1,174 @@ +import { spawn } from 'node:child_process' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { dirname, join } from 'node:path' +import { tmpdir } from 'node:os' + +import { afterEach, describe, expect, it } from 'vitest' + +import { + createSequencedSetupAgentCommands, + SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV +} from './setup-agent-sequencing' + +const TEMP_DIRS: string[] = [] + +afterEach(() => { + for (const dir of TEMP_DIRS.splice(0)) { + rmSync(dir, { recursive: true, force: true }) + } +}) + +describe.skipIf(process.platform !== 'win32')('Windows setup-agent sequencing', () => { + it.each([ + 'path with spaces', + 'ampersand&parentheses(test)', + 'caret^percent%bang!', + "apostrophe's directory", + 'Unicode-한글-abc' + ])('preserves the native runner path in %s', async (directoryName) => { + const tempDir = makeTempDir(directoryName) + const runnerScriptPath = join(tempDir, 'setup runner.cmd') + const startupScriptPath = join(tempDir, 'agent startup.ps1') + const logPath = join(dirname(tempDir), 'sequence.log') + const prompt = 'spaces & pipe | caret ^ percent % bang ! "quotes" Unicode 한글 trailing\\' + + writeFileSync( + runnerScriptPath, + ['@echo off', `>> "${logPath}" echo setup-done`, 'exit /b 0'].join('\r\n'), + 'utf8' + ) + writeFileSync( + startupScriptPath, + [ + 'param([string]$Value)', + '$utf8 = [System.Text.UTF8Encoding]::new($false)', + `[System.IO.File]::AppendAllText('${quotePowerShell(logPath)}', $Value + [Environment]::NewLine, $utf8)` + ].join('\r\n'), + 'utf8' + ) + + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath, + startupCommand: `& '${quotePowerShell(startupScriptPath)}' '${quotePowerShell(prompt)}'`, + platform: 'windows', + nonce: 'windows-sequence', + waitTimeoutSeconds: 2 + }) + + const setupExit = await waitForExit( + spawnWindowsCommand(dirname(tempDir), 'run setup.cmd', commands.setupCommand) + ) + expect(setupExit.code).toBe(0) + expect(readFileSync(`${runnerScriptPath}.windows-sequence.done`, 'utf8')).toBe( + 'windows-sequence:0\r\n' + ) + + const startupExit = await waitForExit( + spawnWindowsCommand( + dirname(tempDir), + 'run startup.cmd', + commands.startupCommand, + commands.startupEnv + ) + ) + expect(startupExit.code).toBe(0) + expect(startupExit.stderr).toContain('Waiting for setup to finish before starting agent...') + expect(readFileSync(logPath, 'utf8')).toBe(`setup-done\r\n${prompt}\r\n`) + }) + + it('keeps the startup command out of generated cmd.exe source', () => { + const startupCommand = 'agent --prompt "& | ^ % ! 한글 trailing\\"' + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath: 'C:\\repo\\setup-runner.cmd', + startupCommand, + platform: 'windows', + nonce: 'windows-sequence' + }) + + expect(commands.setupCommand).not.toContain(startupCommand) + expect(commands.startupCommand).not.toContain(startupCommand) + expect(commands.startupEnv?.[SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV]).toBe(startupCommand) + }) + + it('propagates setup failure without launching the agent', async () => { + const tempDir = makeTempDir('failure path & metacharacters!') + const runnerScriptPath = join(tempDir, 'setup runner.cmd') + const startupScriptPath = join(tempDir, 'agent startup.cmd') + const startupLogPath = join(dirname(tempDir), 'agent-started.log') + + writeFileSync(runnerScriptPath, '@echo off\r\nexit /b 37\r\n', 'utf8') + writeFileSync( + startupScriptPath, + `@echo off\r\necho started>"${startupLogPath}"\r\nexit /b 0\r\n`, + 'utf8' + ) + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath, + startupCommand: `cmd.exe /d /c "${startupScriptPath}"`, + platform: 'windows', + nonce: 'failed-windows-sequence', + waitTimeoutSeconds: 2 + }) + + const setupExit = await waitForExit( + spawnWindowsCommand(dirname(tempDir), 'run failed setup.cmd', commands.setupCommand) + ) + expect(setupExit.code).toBe(37) + expect(readFileSync(`${runnerScriptPath}.failed-windows-sequence.done`, 'utf8')).toBe( + 'failed-windows-sequence:37\r\n' + ) + + const startupExit = await waitForExit( + spawnWindowsCommand( + dirname(tempDir), + 'run blocked startup.cmd', + commands.startupCommand, + commands.startupEnv + ) + ) + expect(startupExit.code).toBe(37) + expect(startupExit.stderr).toContain('Setup failed; skipping agent startup.') + expect(existsSync(startupLogPath)).toBe(false) + }) +}) + +function makeTempDir(directoryName: string): string { + const root = mkdtempSync(join(tmpdir(), 'orca-setup-sequencing-')) + TEMP_DIRS.push(root) + const dir = join(root, directoryName) + mkdirSync(dir) + return dir +} + +function spawnWindowsCommand( + dir: string, + filename: string, + command: string, + env: Record = {} +): ReturnType { + const scriptPath = join(dir, filename) + writeFileSync(scriptPath, `@echo off\r\n${command}\r\nexit /b %ERRORLEVEL%\r\n`, 'utf8') + return spawn('cmd.exe', ['/d', '/c', scriptPath], { + stdio: 'pipe', + env: { ...process.env, ...env } + }) +} + +function quotePowerShell(value: string): string { + return value.replace(/'/g, "''") +} + +function waitForExit( + child: ReturnType +): Promise<{ code: number | null; stderr: string }> { + return new Promise((resolve, reject) => { + let stderr = '' + child.stderr?.on('data', (chunk: Buffer | string) => { + stderr += chunk.toString() + }) + child.once('error', reject) + child.once('close', (code) => { + resolve({ code, stderr }) + }) + }) +}