Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/main/ssh/ssh-connection-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,14 @@ export function shellEscape(s: string): string {
export function wrapRemoteCommandForPosixShell(command: string): string {
// Why: sshd asks the user's login shell to parse exec commands. Orca emits
// POSIX sh snippets; `exec` avoids leaving that shell around for relay bridges.
return `exec /bin/sh -c ${shellEscape(command)}`
if (!command.includes('\n')) {
return `exec /bin/sh -c ${shellEscape(command)}`
}
// Why: csh/tcsh login shells re-parse each line of a quoted multiline
// argument and never reach /bin/sh (#8701). Collapse to one line and let
// /bin/sh rebuild the script; eval keeps stdin free for streaming commands.
const encoded = command.replace(/\\/g, '\\\\').replace(/\n/g, '\\n')
return `exec /bin/sh -c ${shellEscape(`eval "$(printf %b ${shellEscape(encoded)})"`)}`
}

export type SshExecOptions = {
Expand Down
81 changes: 81 additions & 0 deletions src/main/ssh/ssh-remote-command-wrapping.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { spawnSync } from 'node:child_process'
import { describe, expect, it } from 'vitest'
import { wrapRemoteCommandForPosixShell } from './ssh-connection-utils'

// Why: sshd hands the wrapped command string to the user's login shell with
// `-c`, so running `<shell> -c <wrapped>` locally reproduces the remote parse
// exactly — including the csh/tcsh multiline re-parse failure from #8701.
const LOGIN_SHELLS = ['/bin/sh', '/bin/bash', '/bin/zsh', '/bin/dash', '/bin/csh', '/bin/tcsh']
const availableShells =
process.platform === 'win32'
? []
: LOGIN_SHELLS.filter((shell) => spawnSync(shell, ['-c', 'exit 0']).status === 0)

function runViaLoginShell(
loginShell: string,
wrapped: string,
input?: string
): { stdout: string; status: number | null } {
const result = spawnSync(loginShell, ['-c', wrapped], {
encoding: 'utf8',
...(input !== undefined ? { input } : {}),
timeout: 5000
})
expect(result.error).toBeUndefined()
return { stdout: result.stdout, status: result.status }
}

describe('wrapRemoteCommandForPosixShell', () => {
it('keeps single-line commands in the plain /bin/sh -c form', () => {
expect(wrapRemoteCommandForPosixShell('echo "${SHELL:-/bin/sh}"')).toBe(
`exec /bin/sh -c 'echo "\${SHELL:-/bin/sh}"'`
)
})

it('emits a single-line wrapper for multiline scripts', () => {
const wrapped = wrapRemoteCommandForPosixShell('echo one\necho two\n')
expect(wrapped).not.toContain('\n')
expect(wrapped.startsWith('exec /bin/sh -c ')).toBe(true)
})

describe.each(availableShells)('under a %s login shell', (loginShell) => {
it('runs a multiline script with quotes, backslashes, and expansions intact', () => {
const script = [
'echo START',
`name='it'\\''s %s here'`,
'for cand in "$HOME" /nonexistent',
'do',
' [ -e "$cand" ] && echo "found: $cand \\\\ $name"',
'done',
'echo END'
].join('\n')

const { stdout, status } = runViaLoginShell(
loginShell,
wrapRemoteCommandForPosixShell(script)
)
expect(status).toBe(0)
expect(stdout).toContain('START')
expect(stdout).toContain(`found: ${process.env.HOME} \\ it's %s here`)
expect(stdout).toContain('END')
})

it('propagates the script exit code', () => {
const { status } = runViaLoginShell(
loginShell,
wrapRemoteCommandForPosixShell('echo one\nexit 42\n')
)
expect(status).toBe(42)
})

it('leaves stdin available to the script', () => {
const { stdout, status } = runViaLoginShell(
loginShell,
wrapRemoteCommandForPosixShell('read line\necho "got: $line"\n'),
'stream-me\n'
)
expect(status).toBe(0)
expect(stdout).toContain('got: stream-me')
})
})
})
31 changes: 31 additions & 0 deletions src/main/ssh/ssh-remote-node-resolution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,37 @@ describe('resolveRemoteNodePath', () => {
)
})

it('uses -c and the csh which builtin for tcsh login shells', async () => {
execCommandMock
.mockResolvedValueOnce('\n') // path probe: empty
.mockResolvedValueOnce('/bin/tcsh') // $SHELL
.mockResolvedValueOnce('/home/u/.local/bin/node\n') // which node
.mockResolvedValueOnce('v20.20.2\n')

await expect(resolveRemoteNodePath(conn)).resolves.toBe('/home/u/.local/bin/node')

// Why: csh/tcsh reject the combined `-lc` flag and have no `command` builtin.
expect(execCommandMock).toHaveBeenNthCalledWith(3, conn, `'/bin/tcsh' -c 'which node'`, {
wrapCommand: false,
timeoutMs: 8_000
})
})

it('uses -c and the csh which builtin for csh login shells', async () => {
execCommandMock
.mockResolvedValueOnce('\n') // path probe: empty
.mockResolvedValueOnce('/bin/csh') // $SHELL
.mockResolvedValueOnce('/usr/local/bin/node\n') // which node
.mockResolvedValueOnce('v20.20.2\n')

await expect(resolveRemoteNodePath(conn)).resolves.toBe('/usr/local/bin/node')

expect(execCommandMock).toHaveBeenNthCalledWith(3, conn, `'/bin/csh' -c 'which node'`, {
wrapCommand: false,
timeoutMs: 8_000
})
})

it('uses /bin/sh when the remote shell expansion falls back to it', async () => {
execCommandMock
.mockResolvedValueOnce('\n') // path probe: empty
Expand Down
15 changes: 12 additions & 3 deletions src/main/ssh/ssh-remote-node-resolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,11 @@ async function tryResolveViaLoginShell(
return null
}

// Why: csh/tcsh have no `command` builtin; `which` is their PATH resolver.
const probe = isCshFamilyShell(shell) ? 'which node' : 'command -v node'
const nodePath = await execCommand(
conn,
buildCommandInShell(shell, 'command -v node'),
buildCommandInShell(shell, probe),
commandOptions({ wrapCommand: false, timeoutMs: LOGIN_SHELL_PROBE_TIMEOUT_MS }, options)
)
const candidate = nodePath.trim().split('\n')[0]
Expand All @@ -171,11 +173,18 @@ async function tryResolveViaLoginShell(
return null
}

function isCshFamilyShell(shell: string): boolean {
const shellName = shell.split('/').at(-1)
return shellName === 'csh' || shellName === 'tcsh'
}

function buildCommandInShell(shell: string, command: string): string {
const shellName = shell.split('/').at(-1)
// Why: dash and POSIX sh do not require `-l`; when $SHELL falls back to
// /bin/sh, prefer a portable command over login-shell semantics.
const mode = shellName === 'sh' || shellName === 'dash' ? '-c' : '-lc'
// /bin/sh, prefer a portable command over login-shell semantics. csh/tcsh
// reject the combined `-lc` flag outright (#8701); their non-login shells
// still read .cshrc, where EDA/HPC farms set PATH.
const mode = shellName === 'sh' || shellName === 'dash' || isCshFamilyShell(shell) ? '-c' : '-lc'
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return `${shellEscape(shell)} ${mode} ${shellEscape(command)}`
}

Expand Down
Loading