From 07df61fbcf69e563dabea18c5e46ffecbdb18f11 Mon Sep 17 00:00:00 2001 From: Pedram Amini Date: Tue, 3 Feb 2026 09:34:16 -0600 Subject: [PATCH] fix(ssh): use stdin-based execution to bypass shell escaping issues This is a complete rewrite of SSH remote command execution that eliminates all shell escaping issues by sending the entire script via stdin. Previously, the SSH command was built as: ssh host '/bin/bash -c '\''cd /path && VAR='\''value'\'' cmd arg'\''' This required complex nested escaping that broke with: - Heredocs (cat << 'EOF') - Long prompts (command line length limits) - Special characters in prompts Now the SSH command is simply: ssh host /bin/bash And the entire script is piped via stdin: export PATH="$HOME/.local/bin:..." cd '/project/path' export OPENCODE_CONFIG_CONTENT='{"permission":...}' exec opencode run --format json 'prompt here' Benefits: - No shell escaping layers (stdin is binary-safe) - No command line length limits - Works with any remote shell (bash, zsh, fish) - Handles any prompt content (quotes, newlines, $, etc.) - Much simpler to debug and maintain Changes: - Add buildSshCommandWithStdin() in ssh-command-builder.ts - Update process.ts to use stdin-based SSH for all agents - Add sshStdinScript to ProcessConfig type - Update ChildProcessSpawner to send stdin script - Add comprehensive tests for new function --- .../main/utils/ssh-command-builder.test.ts | 157 +++++++++++++++++- src/main/ipc/handlers/process.ts | 131 +++------------ .../spawners/ChildProcessSpawner.ts | 13 +- src/main/process-manager/types.ts | 2 + src/main/utils/ssh-command-builder.ts | 129 ++++++++++++++ 5 files changed, 323 insertions(+), 109 deletions(-) diff --git a/src/__tests__/main/utils/ssh-command-builder.test.ts b/src/__tests__/main/utils/ssh-command-builder.test.ts index 4e9ffddb..3a0f7fa1 100644 --- a/src/__tests__/main/utils/ssh-command-builder.test.ts +++ b/src/__tests__/main/utils/ssh-command-builder.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { buildSshCommand, buildRemoteCommand } from '../../../main/utils/ssh-command-builder'; +import { + buildSshCommand, + buildRemoteCommand, + buildSshCommandWithStdin, +} from '../../../main/utils/ssh-command-builder'; import type { SshRemoteConfig } from '../../../shared/types'; import * as os from 'os'; @@ -660,4 +664,155 @@ describe('ssh-command-builder', () => { expect(remoteCommand).toContain('line3'); }); }); + + describe('buildSshCommandWithStdin', () => { + /** + * Tests for the stdin-based SSH execution approach. + * This method completely bypasses shell escaping issues by: + * 1. SSH connects and runs /bin/bash on the remote + * 2. The entire script (PATH, cd, env, command) is sent via stdin + * 3. No command-line argument parsing/escaping occurs + */ + + it('returns ssh command with /bin/bash as remote command', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run', '--format', 'json'], + }); + + expect(result.command).toBe('ssh'); + // Last arg should be /bin/bash (the remote command) + expect(result.args[result.args.length - 1]).toBe('/bin/bash'); + }); + + it('includes PATH setup in stdin script', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + expect(result.stdinScript).toBeDefined(); + expect(result.stdinScript).toContain('export PATH='); + expect(result.stdinScript).toContain('.local/bin'); + expect(result.stdinScript).toContain('/opt/homebrew/bin'); + }); + + it('includes cd command in stdin script when cwd provided', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + cwd: '/home/user/project', + }); + + expect(result.stdinScript).toContain("cd '/home/user/project'"); + }); + + it('includes environment variables in stdin script', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + env: { + OPENCODE_CONFIG_CONTENT: '{"permission":{"*":"allow"},"tools":{"question":false}}', + CUSTOM_VAR: 'test-value', + }, + }); + + expect(result.stdinScript).toContain('export OPENCODE_CONFIG_CONTENT='); + expect(result.stdinScript).toContain('export CUSTOM_VAR='); + // The JSON should be in the script (escaped with single quotes) + expect(result.stdinScript).toContain('question'); + }); + + it('includes prompt as final argument in stdin script', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run', '--format', 'json'], + prompt: 'Write hello world to a file', + }); + + expect(result.stdinScript).toContain('opencode'); + expect(result.stdinScript).toContain('Write hello world to a file'); + }); + + it('handles prompts with special characters without escaping issues', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + prompt: "What's the $PATH? Use `echo` and \"quotes\"", + }); + + // The script should contain the prompt (escaped for bash) + expect(result.stdinScript).toBeDefined(); + // Single quotes in the prompt should be escaped + expect(result.stdinScript).toContain("'\\''"); + }); + + it('handles multi-line prompts', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + prompt: 'Line 1\nLine 2\nLine 3', + }); + + expect(result.stdinScript).toContain('Line 1'); + expect(result.stdinScript).toContain('Line 2'); + expect(result.stdinScript).toContain('Line 3'); + }); + + it('uses exec to replace shell with command', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + // The script should use exec to replace the shell process + expect(result.stdinScript).toContain('exec '); + }); + + it('includes SSH options in args', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + expect(result.args).toContain('-o'); + expect(result.args).toContain('BatchMode=yes'); + expect(result.args).toContain('StrictHostKeyChecking=accept-new'); + }); + + it('includes private key when provided', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + expect(result.args).toContain('-i'); + expect(result.args).toContain('/Users/testuser/.ssh/id_ed25519'); + }); + + it('includes username@host destination', async () => { + const result = await buildSshCommandWithStdin(baseConfig, { + command: 'opencode', + args: ['run'], + }); + + expect(result.args).toContain('testuser@dev.example.com'); + }); + + it('merges remote config env with option env', async () => { + const configWithEnv = { + ...baseConfig, + remoteEnv: { REMOTE_VAR: 'from-config' }, + }; + + const result = await buildSshCommandWithStdin(configWithEnv, { + command: 'opencode', + args: ['run'], + env: { OPTION_VAR: 'from-option' }, + }); + + expect(result.stdinScript).toContain('export REMOTE_VAR='); + expect(result.stdinScript).toContain('export OPTION_VAR='); + }); + }); }); diff --git a/src/main/ipc/handlers/process.ts b/src/main/ipc/handlers/process.ts index ec0d361d..02b8cc9b 100644 --- a/src/main/ipc/handlers/process.ts +++ b/src/main/ipc/handlers/process.ts @@ -17,7 +17,7 @@ import { CreateHandlerOptions, } from '../../utils/ipcHandler'; import { getSshRemoteConfig, createSshRemoteStoreAdapter } from '../../utils/ssh-remote-resolver'; -import { buildSshCommand } from '../../utils/ssh-command-builder'; +import { buildSshCommandWithStdin } from '../../utils/ssh-command-builder'; import { buildExpandedEnv } from '../../../shared/pathUtils'; import type { SshRemoteConfig } from '../../../shared/types'; import { powerManager } from '../../power-manager'; @@ -263,11 +263,7 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void let useShell = false; let sshRemoteUsed: SshRemoteConfig | null = null; let customEnvVarsToPass: Record | undefined = effectiveCustomEnvVars; - // NOTE: We previously used heredoc for OpenCode prompts over SSH, but this approach - // failed because the heredoc syntax (cat << 'EOF' ... EOF) doesn't survive the - // single-quote escaping in buildSshCommand. Now we embed the prompt directly - // in the args and let buildRemoteCommand handle escaping. - // See: https://github.com/pedramamini/Maestro/issues/XXX + let sshStdinScript: string | undefined; if (config.sessionCustomPath) { logger.debug(`Using session-level custom path for ${config.toolType}`, LOG_CONTEXT, { @@ -323,8 +319,6 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void willUseSsh: config.toolType !== 'terminal' && config.sessionSshRemoteConfig?.enabled, }); } - let shouldSendPromptViaStdin = false; - let shouldSendPromptViaStdinRaw = false; if (config.toolType !== 'terminal' && config.sessionSshRemoteConfig?.enabled) { // Session-level SSH config provided - resolve and use it logger.info(`Using session-level SSH config`, LOG_CONTEXT, { @@ -340,108 +334,40 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void }); if (sshResult.config) { - // SSH remote is configured - wrap the command for remote execution + // SSH remote is configured - use stdin-based execution + // This completely bypasses shell escaping issues by sending the script via stdin sshRemoteUsed = sshResult.config; - // ALWAYS use stdin for SSH remote execution when there's a prompt. - // Embedding prompts in the command line causes shell escaping nightmares: - // - Multiple layers of quote escaping (local spawn, SSH, remote zsh, bash -c) - // - Embedded newlines in prompts break zsh parsing (e.g., "zsh:35: parse error") - // - Special characters like quotes, $, !, etc. need complex escaping - // Using stdin with --input-format stream-json completely bypasses all these issues. - const hasStreamJsonInput = - finalArgs.includes('--input-format') && finalArgs.includes('stream-json'); - const agentSupportsStreamJson = agent?.capabilities.supportsStreamJsonInput ?? false; - let sshArgs = finalArgs; - if (config.prompt && agentSupportsStreamJson) { - // Agent supports stream-json - always use stdin for prompts - if (!hasStreamJsonInput) { - sshArgs = [...finalArgs, '--input-format', 'stream-json']; - } - shouldSendPromptViaStdin = true; - logger.info(`Using stdin for prompt in SSH remote execution`, LOG_CONTEXT, { - sessionId: config.sessionId, - promptLength: config.prompt?.length, - reason: 'ssh-stdin-for-reliability', - hasStreamJsonInput, - }); - } else if (config.prompt && !agentSupportsStreamJson) { - // Agent doesn't support stream-json - embed prompt in command line args - // For OpenCode: prompt is a positional argument (no --p flag, no -- separator) - // For other agents: send via stdin as raw text (if they support it) - if (config.toolType === 'opencode') { - // OpenCode: add prompt as positional argument - // buildRemoteCommand will properly escape it with shellEscape() - sshArgs = [...sshArgs, config.prompt]; - logger.info(`Embedding prompt in OpenCode command args for SSH`, LOG_CONTEXT, { - sessionId: config.sessionId, - promptLength: config.prompt?.length, - }); - } else { - // Other agents: send via stdin as raw text - shouldSendPromptViaStdinRaw = true; - } - } - // - // Determine the command to run on the remote host: - // 1. If user set a session-specific custom path, use that (they configured it for the remote) - // 2. Otherwise, use the agent's binaryName (e.g., 'codex', 'claude') and let - // the remote shell's PATH resolve it. This avoids using local paths like - // '/opt/homebrew/bin/codex' which don't exist on the remote host. + // Determine the command to run on the remote host const remoteCommand = config.sessionCustomPath || agent?.binaryName || config.command; - // Decide whether we'll send input via stdin to the remote command - const useStdin = sshArgs.includes('--input-format') && sshArgs.includes('stream-json'); - const sshCommand = await buildSshCommand(sshResult.config, { + // Build the SSH command with stdin script + // The script contains PATH setup, cd, env vars, and the actual command + // This eliminates all shell escaping issues + const sshCommand = await buildSshCommandWithStdin(sshResult.config, { command: remoteCommand, - args: sshArgs, - // Use the cwd from config - this is the project directory on the remote + args: finalArgs, cwd: config.cwd, - // Pass custom environment variables to the remote command env: effectiveCustomEnvVars, - // Explicitly indicate whether stdin will be used so ssh-command-builder - // can avoid forcing a TTY for stream-json modes. - useStdin, + prompt: config.prompt, // Prompt is included in the stdin script }); commandToSpawn = sshCommand.command; argsToSpawn = sshCommand.args; + sshStdinScript = sshCommand.stdinScript; - // For SSH, env vars are passed in the remote command string, not locally + // For SSH, env vars are passed in the stdin script, not locally customEnvVarsToPass = undefined; - // On Windows, use PowerShell for SSH commands to avoid cmd.exe's 8191 character limit - // PowerShell supports up to 32,767 characters, which is needed for large prompts - if (isWindows) { - useShell = true; - shellToUse = 'powershell.exe'; - logger.info( - `Using PowerShell for SSH command on Windows to support long command lines`, - LOG_CONTEXT, - { - sessionId: config.sessionId, - commandLength: sshCommand.args.join(' ').length, - } - ); - } - - // Detailed debug logging to diagnose SSH command execution issues - logger.debug(`SSH command details for debugging`, LOG_CONTEXT, { + logger.info(`SSH command built with stdin script`, LOG_CONTEXT, { sessionId: config.sessionId, toolType: config.toolType, sshBinary: sshCommand.command, sshArgsCount: sshCommand.args.length, - sshArgsArray: sshCommand.args, - // Show the last arg which contains the wrapped remote command - remoteCommandString: sshCommand.args[sshCommand.args.length - 1], - // Show the agent command that will execute remotely - agentBinary: remoteCommand, - agentArgs: sshArgs, - agentCwd: config.cwd, - // Full invocation for copy-paste debugging - fullSshInvocation: `${sshCommand.command} ${sshCommand.args - .map((arg) => (arg.includes(' ') ? `'${arg}'` : arg)) - .join(' ')}`, + remoteCommand, + remoteCwd: config.cwd, + promptLength: config.prompt?.length, + scriptLength: sshCommand.stdinScript?.length, }); } } @@ -460,38 +386,31 @@ export function registerProcessHandlers(deps: ProcessHandlerDependencies): void command: commandToSpawn, args: argsToSpawn, // When using SSH, use user's home directory as local cwd - // The remote working directory is embedded in the SSH command itself + // The remote working directory is embedded in the SSH stdin script // This fixes ENOENT errors when session.cwd is a remote-only path cwd: sshRemoteUsed ? os.homedir() : config.cwd, // When using SSH, disable PTY (SSH provides its own terminal handling) - // and env vars are passed via the remote command string requiresPty: sshRemoteUsed ? false : agent?.requiresPty, - // When using SSH with small prompts, the prompt was already added to sshArgs above - // For large prompts or stream-json input, pass it to ProcessManager so it can send via stdin - prompt: - sshRemoteUsed && config.prompt && shouldSendPromptViaStdin - ? config.prompt - : sshRemoteUsed - ? undefined - : config.prompt, + // For SSH, prompt is included in the stdin script, not passed separately + // For local execution, pass prompt as normal + prompt: sshRemoteUsed ? undefined : config.prompt, shell: shellToUse, runInShell: useShell, shellArgs: shellArgsStr, // Shell-specific CLI args (for terminal sessions) shellEnvVars: shellEnvVars, // Shell-specific env vars (for terminal sessions) contextWindow, // Pass configured context window to process manager - // When using SSH, env vars are passed in the remote command string, not locally + // When using SSH, env vars are passed in the stdin script, not locally customEnvVars: customEnvVarsToPass, imageArgs: agent?.imageArgs, // Function to build image CLI args (for Codex, OpenCode) promptArgs: agent?.promptArgs, // Function to build prompt args (e.g., ['-p', prompt] for OpenCode) noPromptSeparator: agent?.noPromptSeparator, // Some agents don't support '--' before prompt - // For SSH with stream-json input, send prompt via stdin instead of command line - sendPromptViaStdin: shouldSendPromptViaStdin ? true : undefined, - sendPromptViaStdinRaw: shouldSendPromptViaStdinRaw ? true : undefined, // Stats tracking: use cwd as projectPath if not explicitly provided projectPath: config.cwd, // SSH remote context (for SSH-specific error messages) sshRemoteId: sshRemoteUsed?.id, sshRemoteHost: sshRemoteUsed?.host, + // SSH stdin script - the entire command is sent via stdin to /bin/bash on remote + sshStdinScript, }); logger.info(`Process spawned successfully`, LOG_CONTEXT, { diff --git a/src/main/process-manager/spawners/ChildProcessSpawner.ts b/src/main/process-manager/spawners/ChildProcessSpawner.ts index 5bfad408..4391366f 100644 --- a/src/main/process-manager/spawners/ChildProcessSpawner.ts +++ b/src/main/process-manager/spawners/ChildProcessSpawner.ts @@ -397,8 +397,17 @@ export class ChildProcessSpawner { this.exitHandler.handleError(sessionId, error); }); - // Handle stdin for batch mode and stream-json - if (isStreamJsonMode && prompt) { + // Handle stdin for SSH script, stream-json, or batch mode + if (config.sshStdinScript) { + // SSH stdin script mode: send the entire script to /bin/bash on remote + // This bypasses all shell escaping issues by piping the script via stdin + logger.debug('[ProcessManager] Sending SSH stdin script', 'ProcessManager', { + sessionId, + scriptLength: config.sshStdinScript.length, + }); + childProcess.stdin?.write(config.sshStdinScript); + childProcess.stdin?.end(); + } else if (isStreamJsonMode && prompt) { if (config.sendPromptViaStdinRaw) { // Send raw prompt via stdin logger.debug('[ProcessManager] Sending raw prompt via stdin', 'ProcessManager', { diff --git a/src/main/process-manager/types.ts b/src/main/process-manager/types.ts index e05e219e..6b2ccb03 100644 --- a/src/main/process-manager/types.ts +++ b/src/main/process-manager/types.ts @@ -34,6 +34,8 @@ export interface ProcessConfig { sendPromptViaStdin?: boolean; /** If true, send the prompt via stdin as raw text instead of command line */ sendPromptViaStdinRaw?: boolean; + /** Script to send via stdin for SSH execution (bypasses shell escaping) */ + sshStdinScript?: string; } /** diff --git a/src/main/utils/ssh-command-builder.ts b/src/main/utils/ssh-command-builder.ts index 4ac58dd8..c129954d 100644 --- a/src/main/utils/ssh-command-builder.ts +++ b/src/main/utils/ssh-command-builder.ts @@ -22,6 +22,8 @@ export interface SshCommandResult { command: string; /** Arguments for the SSH command */ args: string[]; + /** Script to send via stdin (for stdin-based execution) */ + stdinScript?: string; } /** @@ -132,6 +134,133 @@ export function buildRemoteCommand(options: RemoteCommandOptions): string { return parts.join(' && '); } +/** + * Build an SSH command that executes a script via stdin. + * + * This approach completely bypasses shell escaping issues by: + * 1. SSH connects and runs `/bin/bash` on the remote + * 2. The script (with PATH setup, cd, env vars, command) is sent via stdin + * 3. No shell parsing of command-line arguments occurs + * + * This is the preferred method for SSH remote execution as it: + * - Handles any prompt content (special chars, newlines, quotes, etc.) + * - Avoids command-line length limits + * - Works regardless of the remote user's login shell (bash, zsh, fish, etc.) + * - Eliminates the escaping nightmare of nested shell contexts + * + * @param config SSH remote configuration + * @param remoteOptions Options for the remote command + * @returns SSH command/args plus the script to send via stdin + * + * @example + * const result = await buildSshCommandWithStdin(config, { + * command: 'opencode', + * args: ['run', '--format', 'json'], + * cwd: '/home/user/project', + * env: { OPENCODE_CONFIG_CONTENT: '{"permission":{"*":"allow"}}' }, + * prompt: 'Write hello world to a file' + * }); + * // result.command = 'ssh' + * // result.args = ['-o', 'BatchMode=yes', ..., 'user@host', '/bin/bash'] + * // result.stdinScript = '#!/bin/bash\nexport PATH=...\ncd /home/user/project\nOPENCODE_CONFIG_CONTENT=... opencode run --format json "Write hello world to a file"\n' + */ +export async function buildSshCommandWithStdin( + config: SshRemoteConfig, + remoteOptions: RemoteCommandOptions & { prompt?: string } +): Promise { + const args: string[] = []; + + // Resolve the SSH binary path + const sshPath = await resolveSshPath(); + + // For stdin-based execution, we never need TTY (stdin is the script, not user input) + // TTY would interfere with piping the script + + // Private key - only add if explicitly provided + if (config.privateKeyPath && config.privateKeyPath.trim()) { + args.push('-i', expandTilde(config.privateKeyPath)); + } + + // Default SSH options - but RequestTTY is always 'no' for stdin mode + for (const [key, value] of Object.entries(DEFAULT_SSH_OPTIONS)) { + args.push('-o', `${key}=${value}`); + } + + // Port specification + if (!config.useSshConfig || config.port !== 22) { + args.push('-p', config.port.toString()); + } + + // Build destination + if (config.username && config.username.trim()) { + args.push(`${config.username}@${config.host}`); + } else { + args.push(config.host); + } + + // The remote command is just /bin/bash - it will read the script from stdin + args.push('/bin/bash'); + + // Build the script to send via stdin + const scriptLines: string[] = []; + + // PATH setup - same directories as before + scriptLines.push( + 'export PATH="$HOME/.local/bin:$HOME/bin:/usr/local/bin:/opt/homebrew/bin:$HOME/.cargo/bin:$PATH"' + ); + + // Change directory if specified + if (remoteOptions.cwd) { + // In the script context, we can use simple quoting + scriptLines.push(`cd ${shellEscape(remoteOptions.cwd)} || exit 1`); + } + + // Merge environment variables + const mergedEnv: Record = { + ...(config.remoteEnv || {}), + ...(remoteOptions.env || {}), + }; + + // Export environment variables + for (const [key, value] of Object.entries(mergedEnv)) { + if (/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key)) { + scriptLines.push(`export ${key}=${shellEscape(value)}`); + } + } + + // Build the command line + // For the script, we use simple quoting since we're not going through shell parsing layers + const cmdParts = [remoteOptions.command, ...remoteOptions.args.map((arg) => shellEscape(arg))]; + + // Add prompt as final argument if provided + if (remoteOptions.prompt) { + cmdParts.push(shellEscape(remoteOptions.prompt)); + } + + // Use exec to replace the shell with the command (cleaner process tree) + scriptLines.push(`exec ${cmdParts.join(' ')}`); + + const stdinScript = scriptLines.join('\n') + '\n'; + + logger.info('SSH command built with stdin script', '[ssh-command-builder]', { + host: config.host, + username: config.username || '(using SSH config/system default)', + port: config.port, + sshPath, + sshArgsCount: args.length, + scriptLineCount: scriptLines.length, + scriptLength: stdinScript.length, + // Show first part of script for debugging (truncate if long) + scriptPreview: stdinScript.length > 500 ? stdinScript.substring(0, 500) + '...' : stdinScript, + }); + + return { + command: sshPath, + args, + stdinScript, + }; +} + /** * Build SSH command and arguments for remote execution. *