Files
orca/src/main/ssh/system-ssh-file-binary-transfer.ts
T
Neil 2ee507d744 fix(ssh): move Windows file writes off PowerShell 5.1 stdin onto sftp (#18596)
* fix(ssh): move Windows file writes off PowerShell 5.1 stdin onto sftp

#16432 was fixed by chunking writes to 32KB, on the belief that a
`DefaultShell=cmd.exe` host caps one stdin at roughly 50KB. Re-measured on
Windows 11 26200.9168 / OpenSSH_for_Windows_10.0p2, that premise is wrong in
both directions, and the chunking does not fix the hang.

The real constraint: a read on Windows PowerShell 5.1's redirected-stdin handle
over a non-pty ssh exec can die permanently when it finds the stream momentarily
empty, taking both the remaining data and the EOF with it. It is probabilistic
per such read — not a size threshold, and not certain on the first one. Measured
by swapping the copy loop for a counting reader:

  a 1.5s gap before any byte    -> 0 bytes received, 6 of 6
  1 byte, 1.5s gap, then 32767  -> exactly 1 byte
  32768, 1.5s gap, then 32768   -> exactly 32768
  a continuous 2MB              -> 167936 / 270336 / 372736

Those three 2MB figures are one payload run three times under the same
conditions, which is what rules out a threshold. Independently reproduced by a
second harness where one 1.9MB counted read completed through 39 reads and
another died after 11.

A payload that fits one burst usually presents only one read that can find the
stream empty, which is why 32KB mostly works — and it still failed 15 times in
120 under load, and 1 in 40 on a quiet host. Neither rate survives the 62 execs
a 1.9MB file needs: even 2.5% compounds to about four uploads in five failing.
No chunk size helps, because the defect is per blocking read, not per byte.
Three controls on the same host, same DefaultShell, rule out both a size limit
and cmd.exe: `findstr` took 2,016,000 bytes through one exec's stdin, sftp moved
1.9MB 5/5, and PowerShell 7 took 2MB in one exec.

Windows writes now go over the sftp subsystem, whose batch script is read by
the *local* client, so no remote process reads a pipe at all. PowerShell 7 is
the fallback where sftp is unavailable, and Windows PowerShell 5.1 is last,
still bounded, and now reports the host limitation and its remedy instead of a
bare timeout.

Measured on the same host, through this code: 1.9MB x20 all succeeded,
hash-verified, median 315ms, against 0/6 before. 32KB x120 zero hangs, against
15/120.

Also:
- Stage under a unique name per attempt. An abandoned write leaves a remote
  process that may still hold the staging file, and losing contact is not
  evidence it died (docs/reference/ssh-execution-boundary.md), so a retry must
  not reuse a name its predecessor may own. Sweep is best-effort and never
  treated as proof of anything.
- Create upload directories over sftp too; the JSON mkdir batch rode the same
  defective read.
- Cover makeWindowsWriteFileCommand and the publish command against the
  8000-char budget, which F11 flagged as untested.

* fix(ssh): replace the staged Windows write atomically, and translate ssh -l

Three review findings, all on the failure path that the success-path
measurements say nothing about.

CodeRabbit, Critical: the publish deleted the destination before moving the
staged file onto it, so a failed move destroyed the user's existing file and
left a window where a reader saw no file at all. That is worse than the
truncated partial the staging discipline exists to prevent. Now File.Replace
(Win32 ReplaceFile, atomic), falling back to a plain Move only when the
destination is absent — and that race is safe, because a destination appearing
in between makes Move throw with the staged file preserved. The exclusive
branch already had it right: Move throwing on an existing destination is the
exclusive contract. Append stays non-atomic and now says why.

buildSshArgs can emit '-l <username>' for a config alias no Host block claims,
and the translator threw on it. isSftpUnavailableError read that throw as 'this
host cannot do sftp', so those hosts fell back to the defective PowerShell 5.1
path and had the refusal cached against them for 30 minutes, silently. '-l' now
maps to '-o User=', with a test for the exact argument shape buildSshArgs
produces in that case.

CodeRabbit, minor: two assertions passed on an absent observation — an
unmatched regex yields '' and every() is true of an empty list. Both now assert
the positive form first, and the same audit was applied to the three other
some()/every() assertions in the file. The temp-file test now asserts mode 0600
rather than only that the file is cleaned up.

* fix(ssh): keep a path sftp cannot spell from becoming a verdict about the host

Audit of isSftpUnavailableError, prompted by the '-l' gap having the same
shape: a per-operation condition being written into a per-host cache that
holds for 30 minutes.

It had a second instance, and this one was mine. UnsupportedSftpPathError was
classified as 'this host cannot do sftp', but it is thrown for a UNC or
relative destination and for any path sftp's batch lexer cannot quote --
including a *local* filename containing a newline, which POSIX clients allow.
One such file would have routed every later Windows write to that host down
the defective PowerShell 5.1 path for the rest of the cache window.

The host verdict is now only the errors that really are host-scoped: a refused
subsystem, a client that will not start, and an untranslatable argument list.
A path refusal falls back for that one write and leaves the cache alone, in
both the file-write and directory-creation paths.

Revert-tested. Removing the operation-scoped catch fails all three new tests,
whether or not the predicate is also widened. Widening the predicate alone
does not fail them, correctly: with the catch in place the predicate no longer
gates that path, so keeping it narrow is defence-in-depth rather than the live
mechanism.

Flag audit at the same time: -F, -o, -T, -S, -p, -i, -J, -l and -- are now the
complete set buildSshArgs can emit, and all are handled.

* fix(ssh): make the atomic publish actually run, and unroll the mkdir batch

Two runtime defects that only a real host could surface. Both were invisible
to unit tests that assert the shape of the generated command string, because
both are PowerShell rejecting an argument at execution time.

File.Replace was passed a bare $null for destinationBackupFileName. PowerShell
coerces $null to an empty string when binding a .NET string parameter, and
Replace rejects that with 'The path is not of a legal form' -- so every
create-mode publish failed. The Critical fix was inert as shipped. Now
[NullString]::Value, which is the construct that exists for this.

Measured on awin, same staging-file lock, opposite outcomes:

  old publish  rc=1  destination MISSING          <- prior contents destroyed
  new publish  rc=1  destination PRESENT, sha 7f06b7e0... unchanged
  control, destination present, no lock  rc=0  replaced exactly
  control, destination absent, no lock   rc=0  Move fallback created it

End-to-end through the real uploader afterwards: 1.9MB x15 all hashes exact,
median 303ms; overwrite of an existing destination exact both times.

Separately, the PowerShell mkdir fallback could not create a tree of more than
one directory. '@($json | ConvertFrom-Json)' wraps the parsed array in another
array, so the loop variable binds to the whole thing and [string] of it is the
paths joined by spaces. It only ever worked for a one-element batch, where
stringifying a single-element array happens to yield the element -- which is
why no existing test caught it. Pre-existing on main; fixed here because this
PR puts that command on the fallback tier and claims the ladder works.
Both tiers now verified live against a three-directory tree.
2026-09-04 01:22:09 -07:00

260 lines
9.4 KiB
TypeScript

import { constants, createWriteStream } from 'node:fs'
import { lstat, mkdtemp, open, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import type { Writable } from 'node:stream'
import { pipeline } from 'node:stream/promises'
import type { SshTarget } from '../../shared/ssh-types'
import { shellEscape } from './ssh-connection-utils'
import {
getSystemSshBuildArgsFromOperationOptions,
type SystemSshBuildArgsOptions
} from './system-ssh-args'
import { spawnSystemSshCommand } from './system-ssh-command'
import { isWindowsRemoteHost, type RemoteHostPlatform } from './ssh-remote-platform'
import { powerShellCommand, powerShellLiteral } from './ssh-remote-powershell'
import {
awaitWithSystemSshAbort,
throwIfAborted,
waitForChannelClose
} from './system-ssh-operation-lifecycle'
import {
writeWindowsRemoteFile,
type WindowsWriteSource
} from './system-ssh-windows-write-strategy'
export {
WINDOWS_STDIN_WRITE_CHUNK_BYTES,
WINDOWS_STDIN_WRITE_TIMEOUT_MS
} from './system-ssh-windows-write-strategy'
export { WINDOWS_STAGED_WRITE_SUFFIX } from './system-ssh-windows-file-write'
type SystemSshOperationOptions = SystemSshBuildArgsOptions & {
signal?: AbortSignal
hostPlatform?: RemoteHostPlatform
}
type SystemSshWriteBufferOptions = SystemSshOperationOptions & {
append?: boolean
exclusive?: boolean
}
type SystemSshUploadFileOptions = SystemSshOperationOptions & {
exclusive?: boolean
}
export async function downloadFileViaSystemSsh(
target: SshTarget,
remotePath: string,
localPath: string,
options?: SystemSshOperationOptions
): Promise<void> {
throwIfAborted(options?.signal)
const isWindows = options?.hostPlatform && isWindowsRemoteHost(options.hostPlatform)
const command = isWindows
? makeWindowsReadFileCommand(remotePath)
: `cat ${shellEscape(remotePath)}`
const channel = spawnSystemSshCommand(target, command, {
wrapCommand: !isWindows,
...getSystemSshBuildArgsFromOperationOptions(options)
})
const output = createWriteStream(localPath, { flags: 'wx' })
try {
await awaitWithSystemSshAbort(
options?.signal,
() => {
channel.close()
output.destroy()
},
Promise.all([
waitForChannelClose(channel, `download ${remotePath}`),
pipeline(channel, output)
])
)
} catch (error) {
channel.close()
output.destroy()
throw error
}
}
export async function writeBufferViaSystemSsh(
target: SshTarget,
remotePath: string,
contents: Buffer,
options?: SystemSshWriteBufferOptions
): Promise<void> {
throwIfAborted(options?.signal)
if (options?.hostPlatform && isWindowsRemoteHost(options.hostPlatform)) {
await writeWindowsRemoteFile(
target,
remotePath,
{
totalBytes: contents.length,
readChunk: (offset, maxBytes) =>
Promise.resolve(contents.subarray(offset, Math.min(offset + maxBytes, contents.length))),
withLocalFile: (send) => withTemporaryLocalFile(contents, send)
},
options ?? {}
)
return
}
const channel = spawnSystemSshCommand(
target,
makePosixWriteFileCommand(remotePath, options),
getSystemSshBuildArgsFromOperationOptions(options)
)
const closePromise = awaitWithSystemSshAbort(
options?.signal,
() => channel.close(),
waitForChannelClose(channel, `write ${remotePath}`)
)
if (!options?.signal?.aborted) {
channel.stdin.end(contents)
}
await closePromise
}
export async function uploadFileViaSystemSsh(
target: SshTarget,
localPath: string,
remotePath: string,
options?: SystemSshUploadFileOptions
): Promise<void> {
throwIfAborted(options?.signal)
const sourceStat = await lstat(localPath)
if (sourceStat.isSymbolicLink() || !sourceStat.isFile()) {
throw new Error(`Unsupported upload source: ${localPath}`)
}
const handle = await open(localPath, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0))
try {
const openedStat = await handle.stat()
if (
!openedStat.isFile() ||
openedStat.size !== sourceStat.size ||
(sourceStat.ino !== 0 && openedStat.ino !== 0 && openedStat.ino !== sourceStat.ino) ||
(sourceStat.dev !== 0 && openedStat.dev !== 0 && openedStat.dev !== sourceStat.dev)
) {
throw new Error(`File changed during upload: ${localPath}`)
}
throwIfAborted(options?.signal)
if (options?.hostPlatform && isWindowsRemoteHost(options.hostPlatform)) {
// This is the path that carries the large files, so it is the one the transport choice is
// made for; see the #16432 note below.
const source: WindowsWriteSource = {
totalBytes: openedStat.size,
readChunk: async (offset, maxBytes) => {
const buffer = Buffer.allocUnsafe(Math.min(maxBytes, openedStat.size - offset))
const { bytesRead } = await handle.read(buffer, 0, buffer.length, offset)
return buffer.subarray(0, bytesRead)
},
// The verified local file is already exactly the payload, so sftp sends it as is.
withLocalFile: (send) => send(localPath)
}
await writeWindowsRemoteFile(target, remotePath, source, options ?? {})
return
}
const channel = spawnSystemSshCommand(
target,
makePosixWriteFileCommand(remotePath, options),
getSystemSshBuildArgsFromOperationOptions(options)
)
const input = handle.createReadStream({ autoClose: false })
try {
await awaitWithSystemSshAbort(
options?.signal,
() => {
input.destroy()
channel.close()
},
Promise.all([
waitForChannelClose(channel, `upload ${remotePath}`),
pipeline(input, channel.stdin as Writable)
])
)
} catch (error) {
input.destroy()
channel.close()
throw error
}
} finally {
await handle.close()
}
}
/**
* #16432, re-measured: the constraint is not a size limit, and it is not cmd.exe's.
*
* A read on Windows PowerShell 5.1's redirected-stdin handle over a non-pty ssh exec can die
* permanently when it finds the stream momentarily empty: no further bytes arrive, and no EOF ever
* does. It is probabilistic per such read — not a size threshold, and not certain on the first one.
* Measured on Windows 11 26200.9168 / OpenSSH_for_Windows_10.0p2 with `DefaultShell = cmd.exe`, by
* replacing the copy loop with a counting reader:
*
* - a 1.5s gap before any byte, which forces the first read to find nothing -> 0 bytes, 6 of 6
* - one byte, a 1.5s gap, then 32767 more -> exactly 1 byte, then nothing
* - 32768, a 1.5s gap, then 32768 more -> exactly 32768, then nothing
* - a continuous 2MB -> 167936 / 270336 / 372736, then nothing
*
* Those three 2MB death points are one payload run three times under the same conditions, which is
* what rules out a threshold: a stream that died at a fixed point would not vary by 2x. Independently reproduced by
* a second harness, where one 1.9MB counted read survived 39 reads to completion and another died
* after 11 — same construct, same payload.
*
* A payload small enough to arrive in one burst usually presents only one read that can find the
* stream empty (the one waiting for EOF), which is why 32KB mostly works: it still failed 15 times
* in 120 with the host under load, and 1 in 40 on a quiet one. Neither rate is survivable across
* the 62 execs a 1.9MB file needs — even 2.5% compounds to roughly four uploads in five failing —
* and no chunk size helps, because the client does not control whether its bytes arrive together.
*
* The same host, same `DefaultShell`, same connection pattern contradicts every size-limit reading:
* `findstr` took 2,016,000 bytes through one exec's stdin, and PowerShell 7 took 2MB. So cmd.exe is
* not the ceiling and neither is ~50KB. Writes now go over sftp, which moves the whole payload
* without any remote process reading a pipe; see `system-ssh-windows-write-strategy.ts` for the
* fallback order.
*
* Successes are never partial. Across every run in both harnesses a failed write hung; not one
* produced a short file, so this defect cannot silently truncate an upload.
*/
/** A staged write is materialized locally first when the source is a buffer rather than a file. */
async function withTemporaryLocalFile<T>(
contents: Buffer,
send: (localPath: string) => Promise<T>
): Promise<T> {
const directory = await mkdtemp(join(tmpdir(), 'orca-win-upload-'))
const localPath = join(directory, 'payload.bin')
try {
// 0600: the payload can be repository content, and tmpdir is shared on every platform.
await writeFile(localPath, contents, { mode: 0o600 })
return await send(localPath)
} finally {
await rm(directory, { recursive: true, force: true }).catch(() => {})
}
}
function makePosixWriteFileCommand(
remotePath: string,
options?: { append?: boolean; exclusive?: boolean }
): string {
const redirection = options?.append ? '>>' : '>'
const noclobber = !options?.append && options?.exclusive ? 'set -C; ' : ''
return `${noclobber}cat ${redirection} ${shellEscape(remotePath)}`
}
function makeWindowsReadFileCommand(remotePath: string): string {
return powerShellCommand(
[
'$ErrorActionPreference = "Stop"',
`$path = ${powerShellLiteral(remotePath)}`,
'$src = [System.IO.File]::OpenRead($path)',
'$dst = [Console]::OpenStandardOutput()',
'try { $src.CopyTo($dst) } finally { $src.Dispose() }'
].join('; ')
)
}