mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(ssh): make POSIX relay wrapper survive csh/tcsh login shells (#8709)
Reconstruct remote POSIX commands with bounded printf arguments so non-POSIX SSH login shells can forward them without requiring remote base64. Preserve relay and system-SSH stdin, and centralize login-shell flag selection for csh/tcsh compatibility.\n\nValidated against real csh and tcsh OpenSSH targets with built-in and system SSH, including cold relay deployment, stdin upload, PTY I/O, file mutation, and reconnect.
This commit is contained in:
@@ -110,10 +110,44 @@ export function shellEscape(s: string): string {
|
||||
return `'${s.replace(/'/g, "'\\''")}'`
|
||||
}
|
||||
|
||||
const REMOTE_COMMAND_CHUNK_MAX_BYTES = 1_024
|
||||
const REMOTE_COMMAND_PRINTF_ESCAPED_BYTES = new Set([0x21, 0x27, 0x5c])
|
||||
|
||||
function encodeRemoteCommandForPrintf(command: string): string[] {
|
||||
const chunks: string[] = []
|
||||
let chunk = ''
|
||||
let chunkBytes = 0
|
||||
for (const character of command) {
|
||||
const codePoint = character.codePointAt(0)!
|
||||
const isSafePrintableAscii =
|
||||
codePoint >= 0x20 && codePoint <= 0x7e && !REMOTE_COMMAND_PRINTF_ESCAPED_BYTES.has(codePoint)
|
||||
const encodedCharacter =
|
||||
codePoint > 0x7f || isSafePrintableAscii
|
||||
? character
|
||||
: `\\0${codePoint.toString(8).padStart(3, '0')}`
|
||||
const encodedBytes = codePoint > 0x7f ? Buffer.byteLength(character) : encodedCharacter.length
|
||||
if (chunkBytes + encodedBytes > REMOTE_COMMAND_CHUNK_MAX_BYTES) {
|
||||
chunks.push(chunk)
|
||||
chunk = ''
|
||||
chunkBytes = 0
|
||||
}
|
||||
chunk += encodedCharacter
|
||||
chunkBytes += encodedBytes
|
||||
}
|
||||
chunks.push(chunk)
|
||||
return chunks
|
||||
}
|
||||
|
||||
/** Wrap a POSIX snippet into one line that non-POSIX SSH login shells can forward. */
|
||||
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)}`
|
||||
// Why: csh/tcsh split multiline SSH exec strings before /bin/sh sees them.
|
||||
// POSIX printf rebuilds bounded argument chunks without consuming relay stdin.
|
||||
const encodedChunks = encodeRemoteCommandForPrintf(command)
|
||||
const decodeAndRun =
|
||||
'decoded=$(printf %b "$@" && printf _) || exit $?; ' +
|
||||
'decoded=${decoded%_}; exec /bin/sh -c "$decoded"'
|
||||
const chunkArguments = encodedChunks.map(shellEscape).join(' ')
|
||||
return `exec /bin/sh -c ${shellEscape(decodeAndRun)} orca-command ${chunkArguments}`
|
||||
}
|
||||
|
||||
export type SshExecOptions = {
|
||||
|
||||
@@ -657,15 +657,19 @@ describe('SshConnection', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('wraps exec commands in /bin/sh so non-POSIX login shells do not parse relay snippets', async () => {
|
||||
it('wraps exec commands as a single line that csh/tcsh login shells cannot break', async () => {
|
||||
const conn = new SshConnection(createTarget(), createCallbacks())
|
||||
await conn.connect()
|
||||
|
||||
await conn.exec("cd '/tmp' && ('/usr/bin/node' -e 'console.log(1)' || echo MISSING)")
|
||||
const original = "cd '/tmp' && ('/usr/bin/node' -e 'console.log(1)' || echo MISSING)"
|
||||
await conn.exec(original)
|
||||
|
||||
expect(clientInstances[0].lastExecCommand).toBe(
|
||||
"exec /bin/sh -c 'cd '\\''/tmp'\\'' && ('\\''/usr/bin/node'\\'' -e '\\''console.log(1)'\\'' || echo MISSING)'"
|
||||
)
|
||||
const wrapped = clientInstances[0].lastExecCommand!
|
||||
// Why: sshd lets the login shell parse this first, so raw newlines let
|
||||
// csh/tcsh split the command before /bin/sh receives it (issue #8701).
|
||||
expect(wrapped).not.toContain('\n')
|
||||
expect(wrapped).toMatch(/^exec \/bin\/sh -c '.*printf %b .*' orca-command /)
|
||||
expect(wrapped).not.toContain('base64')
|
||||
})
|
||||
|
||||
it('can execute native remote commands without the POSIX shell wrapper', async () => {
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
import { shellEscape } from './ssh-connection-utils'
|
||||
|
||||
const COMMAND_ONLY_SHELLS = new Set(['sh', 'dash', 'csh', 'tcsh'])
|
||||
|
||||
/** Build a command using the startup mode supported by the configured login shell. */
|
||||
export function buildSshLoginShellCommand(shell: string, command: string): string {
|
||||
const shellName = shell.split('/').at(-1)
|
||||
// Why: csh/tcsh reject combined -lc, while sh/dash do not need login mode here.
|
||||
const mode = shellName && COMMAND_ONLY_SHELLS.has(shellName) ? '-c' : '-lc'
|
||||
return `${shellEscape(shell)} ${mode} ${shellEscape(command)}`
|
||||
}
|
||||
@@ -0,0 +1,94 @@
|
||||
import { existsSync } from 'node:fs'
|
||||
import { spawnSync } from 'node:child_process'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { shellEscape, wrapRemoteCommandForPosixShell } from './ssh-connection-utils'
|
||||
|
||||
describe('wrapRemoteCommandForPosixShell', () => {
|
||||
it('emits one physical line without requiring a remote decoder binary', () => {
|
||||
const wrapped = wrapRemoteCommandForPosixShell("printf 'a\\n'\nprintf 'b\\n'")
|
||||
expect(wrapped).not.toContain('\n')
|
||||
expect(wrapped).toContain('printf %b "$@"')
|
||||
expect(wrapped).not.toContain('base64')
|
||||
})
|
||||
|
||||
it('does not expand long commands made only of parser-safe ASCII', () => {
|
||||
const command = `: #${'x'.repeat(7_000)}`
|
||||
expect(wrapRemoteCommandForPosixShell(command).length - command.length).toBeLessThan(200)
|
||||
})
|
||||
|
||||
it('does not octal-expand literal UTF-8 near SSH command-length limits', () => {
|
||||
const command = `: #${'☃'.repeat(7_000)}`
|
||||
expect(wrapRemoteCommandForPosixShell(command).length - command.length).toBeLessThan(200)
|
||||
})
|
||||
|
||||
it('does not octal-expand quoted shell metacharacters near command-length limits', () => {
|
||||
const command = `: #${'$"`'.repeat(2_000)}`
|
||||
expect(wrapRemoteCommandForPosixShell(command).length - command.length).toBeLessThan(200)
|
||||
})
|
||||
|
||||
it.skipIf(process.platform === 'win32')(
|
||||
'reconstructs multiline UTF-8 and shell metacharacters exactly',
|
||||
() => {
|
||||
const expected = "line one\nline 'two' \"$`\\ ! ☃\n"
|
||||
const command = `printf '%s' ${shellEscape(expected)}`
|
||||
const result = runThroughShell('/bin/sh', command)
|
||||
|
||||
expect(result.status).toBe(0)
|
||||
expect(result.stderr).toBe('')
|
||||
expect(result.stdout).toBe(expected)
|
||||
}
|
||||
)
|
||||
|
||||
it.skipIf(process.platform === 'win32')('keeps stdin free for relay protocol bytes', () => {
|
||||
const command = 'IFS= read -r frame; printf \'frame:%s\' "$frame"'
|
||||
const result = runThroughShell('/bin/sh', command, 'rpc-payload\n')
|
||||
|
||||
expect(result.status).toBe(0)
|
||||
expect(result.stdout).toBe('frame:rpc-payload')
|
||||
})
|
||||
|
||||
it.skipIf(process.platform === 'win32' || !existsSync('/bin/tcsh'))(
|
||||
'keeps every login-shell word bounded for long metacharacter-heavy commands',
|
||||
() => {
|
||||
const expected = '$"`'.repeat(2_000)
|
||||
const command = `printf '%s' ${shellEscape(expected)}`
|
||||
const result = runThroughShell('/bin/tcsh', command)
|
||||
|
||||
expect(result.status).toBe(0)
|
||||
expect(result.stderr).toBe('')
|
||||
expect(result.stdout).toBe(expected)
|
||||
}
|
||||
)
|
||||
|
||||
for (const shell of [
|
||||
'/bin/sh',
|
||||
'/bin/bash',
|
||||
'/bin/dash',
|
||||
'/bin/zsh',
|
||||
'/bin/ksh',
|
||||
'/bin/csh',
|
||||
'/bin/tcsh',
|
||||
'/usr/bin/fish',
|
||||
'/usr/local/bin/fish',
|
||||
'/opt/homebrew/bin/fish'
|
||||
]) {
|
||||
it.skipIf(process.platform === 'win32' || !existsSync(shell))(
|
||||
`survives the real ${shell} parser`,
|
||||
() => {
|
||||
const expected = "first\nsecond '$`\\n\\\\ ! ☃\n"
|
||||
const command = `printf '%s' ${shellEscape(expected)}`
|
||||
const result = runThroughShell(shell, command)
|
||||
expect(result.status).toBe(0)
|
||||
expect(result.stderr).toBe('')
|
||||
expect(result.stdout).toBe(expected)
|
||||
}
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
function runThroughShell(shell: string, command: string, input?: string) {
|
||||
return spawnSync(shell, ['-c', wrapRemoteCommandForPosixShell(command)], {
|
||||
encoding: 'utf8',
|
||||
input
|
||||
})
|
||||
}
|
||||
@@ -272,6 +272,41 @@ describe('resolveRemoteNodePath', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('uses -c (not -lc) for a csh login shell, which rejects combined -lc', async () => {
|
||||
// Why: csh/tcsh reject `-lc` and mis-handle `-l` here ("Bad : modifier",
|
||||
// issue #8701), so the login-shell probe must drop to plain `-c`.
|
||||
execCommandMock
|
||||
.mockResolvedValueOnce('\n') // path probe: empty
|
||||
.mockResolvedValueOnce('/bin/csh\n') // $SHELL
|
||||
.mockResolvedValueOnce('/usr/local/bin/node\n')
|
||||
.mockResolvedValueOnce('v20.0.0\n')
|
||||
|
||||
await expect(resolveRemoteNodePath(conn)).resolves.toBe('/usr/local/bin/node')
|
||||
expect(execCommandMock).toHaveBeenNthCalledWith(3, conn, `'/bin/csh' -c 'command -v node'`, {
|
||||
wrapCommand: false,
|
||||
timeoutMs: 8_000
|
||||
})
|
||||
})
|
||||
|
||||
it('uses -c (not -lc) for a tcsh login shell', async () => {
|
||||
execCommandMock
|
||||
.mockResolvedValueOnce('\n') // path probe: empty
|
||||
.mockResolvedValueOnce('/usr/bin/tcsh\n') // $SHELL
|
||||
.mockResolvedValueOnce('/usr/local/bin/node\n')
|
||||
.mockResolvedValueOnce('v20.0.0\n')
|
||||
|
||||
await expect(resolveRemoteNodePath(conn)).resolves.toBe('/usr/local/bin/node')
|
||||
expect(execCommandMock).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
conn,
|
||||
`'/usr/bin/tcsh' -c 'command -v node'`,
|
||||
{
|
||||
wrapCommand: false,
|
||||
timeoutMs: 8_000
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
// ── Failure ───────────────────────────────────────────────────────────
|
||||
|
||||
it('throws when both strategies find no usable node', async () => {
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from './ssh-remote-node-install-guidance'
|
||||
import { execCommand } from './ssh-relay-deploy-helpers'
|
||||
import { isSshSessionLimitError } from './ssh-session-limit-error'
|
||||
import { buildSshLoginShellCommand } from './ssh-login-shell-command'
|
||||
|
||||
// Why: the relay requires Node.js 18+. Version managers like nvm keep every
|
||||
// installed version on disk, so a naive "highest version" glob can hand back
|
||||
@@ -150,7 +151,7 @@ async function tryResolveViaLoginShell(
|
||||
|
||||
const nodePath = await execCommand(
|
||||
conn,
|
||||
buildCommandInShell(shell, 'command -v node'),
|
||||
buildSshLoginShellCommand(shell, 'command -v node'),
|
||||
commandOptions({ wrapCommand: false, timeoutMs: LOGIN_SHELL_PROBE_TIMEOUT_MS }, options)
|
||||
)
|
||||
const candidate = nodePath.trim().split('\n')[0]
|
||||
@@ -171,14 +172,6 @@ async function tryResolveViaLoginShell(
|
||||
return null
|
||||
}
|
||||
|
||||
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'
|
||||
return `${shellEscape(shell)} ${mode} ${shellEscape(command)}`
|
||||
}
|
||||
|
||||
// Returns true if `nodePath` runs and reports Node >= MIN_NODE_MAJOR.
|
||||
// Caches nothing — this runs at most a few times per resolution (one per
|
||||
// candidate), and the exec round-trip dominates.
|
||||
|
||||
@@ -441,9 +441,18 @@ describe('spawnSystemSsh', () => {
|
||||
it('spawns a remote command through the system ssh target', () => {
|
||||
spawnSystemSshCommand(createTarget({ configHost: 'fdpass-host' }), 'echo hello')
|
||||
|
||||
// Why: the remote command stays on one line so csh/tcsh login shells cannot
|
||||
// split it before /bin/sh receives it.
|
||||
const args = spawnMock.mock.calls[0][1] as string[]
|
||||
expect(args).toContain('--')
|
||||
expect(args).toContain('deploy@fdpass-host')
|
||||
const wrapped = args.at(-1)!
|
||||
expect(wrapped).not.toContain('\n')
|
||||
expect(wrapped).toContain('printf %b "$@"')
|
||||
expect(wrapped).not.toContain('base64')
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
SYSTEM_SSH_PATH,
|
||||
expect.arrayContaining(['--', 'deploy@fdpass-host', "exec /bin/sh -c 'echo hello'"]),
|
||||
expect.any(Array),
|
||||
expect.objectContaining({ stdio: ['pipe', 'pipe', 'pipe'] })
|
||||
)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user