jlengrand/Maestro
1
0
Fork 0
mirror of https://github.com/jlengrand/Maestro.git synced 2026-03-10 08:31:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ssh): use bash -lc wrapper for PATH in remote execution

Browse Source 
The previous fix removed the shell wrapper entirely, which broke PATH
resolution for user-installed binaries like 'claude' in ~/.local/bin.
Exit code 127 (command not found) occurred because SSH's non-login
execution doesn't load profile files.

Now using "bash -lc" instead of "$SHELL -lc":
- bash is universally available on macOS/Linux
- bash -l sources /etc/profile, ~/.bash_profile, ~/.profile for PATH
- bash profile files rarely have syntax incompatible with -c embedding
- Avoids zsh profile issues (loops/conditionals that break -c embedding)

The double-quote escaping ensures $ variables in the inner command are
passed literally and evaluated by bash, not by SSH's outer shell.
This commit is contained in:
Pedram Amini
2026-01-31 20:45:14 -05:00
parent 8cfe9ce22e
commit fc233b9dbf
2 changed files with 35 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
src/__tests__/main/utils/ssh-command-builder.test.ts
View File

@@ -319,8 +319,11 @@ describe('ssh-command-builder', () => {
});
const lastArg = result.args[result.args.length - 1];
// Command is passed directly without shell wrapper
expect(lastArg).toBe("claude '--print' 'hello'");
// Command is wrapped in bash -lc for PATH
expect(lastArg).toContain('bash -lc');
expect(lastArg).toContain('claude');
expect(lastArg).toContain('--print');
expect(lastArg).toContain('hello');
expect(lastArg).not.toContain('cd');
});
@@ -376,15 +379,16 @@ describe('ssh-command-builder', () => {
args: ['commit', '-m', "fix: it's a bug with $VARIABLES"],
});
const remoteCommand = result.args[result.args.length - 1];
// The command is passed directly to SSH without shell wrapper
// Single quotes prevent variable expansion, making this safe
// Original: git 'commit' '-m' 'fix: it'\''s a bug with $VARIABLES'
expect(remoteCommand).toContain('git');
expect(remoteCommand).toContain('commit');
expect(remoteCommand).toContain('fix:');
// $VARIABLES is inside single quotes so it won't expand (safe)
expect(remoteCommand).toContain('$VARIABLES');
const wrappedCommand = result.args[result.args.length - 1];
// The command is wrapped in bash -lc "..." with double-quote escaping
// The inner single quotes become escaped for double-quote context
// $ signs are escaped as \$ to prevent expansion by SSH's outer shell
expect(wrappedCommand).toContain('bash -lc');
expect(wrappedCommand).toContain('git');
expect(wrappedCommand).toContain('commit');
expect(wrappedCommand).toContain('fix:');
// $VARIABLES should be escaped to prevent expansion by outer shell
expect(wrappedCommand).toContain('\\$VARIABLES');
});
});
@@ -632,12 +636,10 @@ describe('ssh-command-builder', () => {
args: ['--print', '--', "what's the $PATH variable?"],
});
const remoteCommand = result.args[result.args.length - 1];
// The prompt is inside single quotes which prevents variable expansion
// $PATH is safe because single quotes treat everything literally
expect(remoteCommand).toContain('$PATH');
// Verify the single quote in "what's" is properly escaped
expect(remoteCommand).toContain("what'\\''s");
const wrappedCommand = result.args[result.args.length - 1];
// The prompt is wrapped in bash -lc "..." with double-quote escaping
// $PATH should be escaped as \$PATH to prevent expansion
expect(wrappedCommand).toContain('\\$PATH');
});
it('handles multi-line prompts', async () => {

44
src/main/utils/ssh-command-builder.ts
View File

@@ -8,7 +8,7 @@
*/
import { SshRemoteConfig } from '../../shared/types';
import { shellEscape, buildShellCommand } from './shell-escape';
import { shellEscape, buildShellCommand, shellEscapeForDoubleQuotes } from './shell-escape';
import { expandTilde } from '../../shared/pathUtils';
import { logger } from './logger';
import { resolveSshPath } from './cliDetection';
@@ -270,35 +270,22 @@ export async function buildSshCommand(
env: Object.keys(mergedEnv).length > 0 ? mergedEnv : undefined,
});
// Wrap the command to execute via the user's login shell.
// $SHELL -lc ensures the user's full PATH (including homebrew, nvm, etc.) is available.
// -l loads login profile for PATH
// -c executes the command
// Using $SHELL respects the user's configured shell (bash, zsh, etc.)
// Wrap the command in a login shell to ensure PATH is loaded.
// We use "bash -lc" rather than "$SHELL -lc" to avoid issues with complex
// profile syntax in zsh (e.g., loops, conditionals) that can't be embedded
// in a -c command string.
//
// WHY PROFILE SOURCING IS NEEDED:
// On many systems, login shells don't automatically source interactive config files.
// We explicitly source profile and rc files to ensure PATH and environment are set up
// properly for finding agent binaries like 'claude', 'codex', etc.
// Why bash specifically:
// - bash is available on virtually all Unix systems (macOS, Linux)
// - bash -l sources /etc/profile, ~/.bash_profile, ~/.profile
// - This provides PATH for user-installed binaries like 'claude' in ~/.local/bin
// - bash profile files rarely have syntax incompatible with -c embedding
//
// CRITICAL: When Node.js spawn() passes this to SSH without shell:true, SSH runs
// the command through the remote's default shell. The key is escaping:
// 1. Double quotes around the command are NOT escaped - they delimit the -c argument
// 2. $ signs inside the command MUST be escaped as \$ so they defer to the login shell
// (shellEscapeForDoubleQuotes handles this)
// 3. Single quotes inside the command pass through unchanged
//
// Example transformation for spawn():
// Input: cd '/path' && MYVAR='value' claude --print
// After escaping: cd '/path' && MYVAR='value' claude --print (no $ to escape here)
// Wrapped: $SHELL -lc "source ~/.bashrc 2>/dev/null; cd '/path' && MYVAR='value' claude --print"
// SSH receives this as one argument, passes to remote shell
// The login shell runs with full PATH from /etc/profile, ~/.bash_profile, AND ~/.bashrc
// Pass the command directly to SSH without shell wrapper.
// SSH executes commands through the remote's login shell by default,
// which provides PATH from /etc/profile. We avoid $SHELL -c wrappers
// because profile files may contain syntax incompatible with -c embedding.
args.push(remoteCommand);
// The double-quote escaping ensures $-prefixed variables in the inner command
// are passed literally and evaluated by bash, not by SSH's outer shell.
const escapedCommand = shellEscapeForDoubleQuotes(remoteCommand);
const wrappedCommand = `bash -lc "${escapedCommand}"`;
args.push(wrappedCommand);
// Log the exact command being built - use info level so it appears in system logs
logger.info('SSH command built for remote execution', '[ssh-command-builder]', {
@@ -308,6 +295,7 @@ export async function buildSshCommand(
useSshConfig: config.useSshConfig,
privateKeyPath: config.privateKeyPath ? '***configured***' : '(using SSH config/agent)',
remoteCommand,
wrappedCommand,
sshPath,
sshArgsCount: args.length,
// Full command for debugging - escape quotes for readability