diff --git a/src/main/ssh/ssh-remote-powershell.ts b/src/main/ssh/ssh-remote-powershell.ts index 8c94fd3c483..420223ced29 100644 --- a/src/main/ssh/ssh-remote-powershell.ts +++ b/src/main/ssh/ssh-remote-powershell.ts @@ -11,14 +11,24 @@ export { // to leave room for the `/c` wrapper sshd adds before cmd.exe counts the line. const WINDOWS_REMOTE_COMMAND_LINE_BUDGET_CHARS = 8_000 -export function powerShellCommand(script: string): string { - const inline = encodedPowerShellCommand(script) +/** + * `pwsh.exe` is PowerShell 7. It is not present on a stock Windows install, so it is only ever + * chosen after a probe — but where it exists it reads a redirected stdin correctly, which Windows + * PowerShell 5.1 does not (see `system-ssh-file-binary-transfer.ts`). + */ +export type WindowsPowerShellExecutable = 'powershell.exe' | 'pwsh.exe' + +export function powerShellCommand( + script: string, + executable: WindowsPowerShellExecutable = 'powershell.exe' +): string { + const inline = encodedPowerShellCommand(script, executable) if (inline.length <= WINDOWS_REMOTE_COMMAND_LINE_BUDGET_CHARS) { return inline } // Why: these scripts are repetitive enough that gzip beats the UTF-16LE tax by // ~4x, which is the difference between a line cmd.exe runs and one it refuses. - const compressed = encodedPowerShellCommand(selfExtractingPowerShellScript(script)) + const compressed = encodedPowerShellCommand(selfExtractingPowerShellScript(script), executable) if (compressed.length > WINDOWS_REMOTE_COMMAND_LINE_BUDGET_CHARS) { throw new Error( `Remote Windows command needs ${compressed.length} characters; Orca budgets ${WINDOWS_REMOTE_COMMAND_LINE_BUDGET_CHARS} for a line sshd hands to cmd.exe, which itself refuses more than ${CMD_EXE_COMMAND_LINE_MAX_CHARS}.` @@ -27,8 +37,8 @@ export function powerShellCommand(script: string): string { return compressed } -function encodedPowerShellCommand(script: string): string { - return `powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand ${encodePowerShellCommand(script)}` +function encodedPowerShellCommand(script: string, executable: WindowsPowerShellExecutable): string { + return `${executable} -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand ${encodePowerShellCommand(script)}` } /** Orca-prefixed names so the payload can never shadow the bootstrap's own state. */ diff --git a/src/main/ssh/ssh-remote-windows-command-line-limit.test.ts b/src/main/ssh/ssh-remote-windows-command-line-limit.test.ts index c104078238e..fa34a8fbda7 100644 --- a/src/main/ssh/ssh-remote-windows-command-line-limit.test.ts +++ b/src/main/ssh/ssh-remote-windows-command-line-limit.test.ts @@ -4,6 +4,10 @@ import { CMD_EXE_COMMAND_LINE_MAX_CHARS } from '../providers/windows-shell-args' import { getRemoteHostPlatform } from './ssh-remote-platform' import { tryStealInstallLockCommand } from './ssh-relay-install-lock-commands' import { decodeRemotePowerShellScript, powerShellCommand } from './ssh-remote-powershell' +import { + makeWindowsPublishStagedFileCommand, + makeWindowsWriteFileCommand +} from './system-ssh-windows-file-write' import { cleanupOwnedRelayUploadStageCommand, promoteOwnedRelayUploadStageCommand, @@ -38,6 +42,17 @@ describe('Windows remote command line limit', () => { [ 'steal stale install lock', tryStealInstallLockCommand(windows, 'C:\\Users\\orca\\.orca-remote\\relay', 1_200) + ], + // F11 flagged these two as uncovered. They carry one path literal each, so they are the file + // commands whose length a caller can actually move. + ['write file', makeWindowsWriteFileCommand('C:\\Users\\orca\\.orca-remote\\relay.js')], + [ + 'publish staged file', + makeWindowsPublishStagedFileCommand( + 'C:\\Users\\orca\\.orca-remote\\relay.js.orca-partial-0123456789ab', + 'C:\\Users\\orca\\.orca-remote\\relay.js', + 'create' + ) ] ])('keeps the %s command inside what sshd\u2019s cmd.exe accepts', (_name, command) => { expect(command.length).toBeLessThanOrEqual(CMD_EXE_COMMAND_LINE_MAX_CHARS) @@ -76,3 +91,32 @@ describe('Windows remote command line limit', () => { ) }) }) + +/** + * F11 asked whether a pathological path could reach the budget, and what happens if it does. + * Measured: the inline encoding crosses 8000 at roughly 2500 high-entropy path characters — an + * order of magnitude past what Windows itself accepts — and the failure is a throw before any ssh + * is spawned, never a hang. + */ +describe('Windows file command budget headroom', () => { + it('absorbs a path far longer than Windows will accept', () => { + const deep = `C:\\Users\\orca\\${'segment\\'.repeat(30)}relay.js` + + expect(deep.length).toBeGreaterThan(260) + expect(makeWindowsWriteFileCommand(deep).length).toBeLessThanOrEqual( + CMD_EXE_COMMAND_LINE_MAX_CHARS + ) + }) + + it('throws rather than spawning a line cmd.exe would refuse', () => { + // Random segments so gzip cannot rescue it, which is the only way to reach the ceiling at all. + const incompressible = Array.from( + { length: 400 }, + (_unused, index) => `${index}-${Math.random().toString(36).slice(2)}` + ).join('\\') + + expect(() => makeWindowsWriteFileCommand(`C:\\${incompressible}\\f.bin`)).toThrow( + /Orca budgets 8000/ + ) + }) +}) diff --git a/src/main/ssh/ssh-system-fallback.test.ts b/src/main/ssh/ssh-system-fallback.test.ts index c366e899bf6..2a80ab031cf 100644 --- a/src/main/ssh/ssh-system-fallback.test.ts +++ b/src/main/ssh/ssh-system-fallback.test.ts @@ -709,9 +709,13 @@ describe('spawnSystemSsh', () => { expect(args[standaloneControlIdx + 1]).toBe('none') }) - it('writes files to Windows system SSH targets with PowerShell stdin bytes', async () => { - const proc = createEventedProcess() - spawnMock.mockImplementation(() => closeOnceSpawned(proc)) + it('sends Windows file writes over sftp, not through a remote PowerShell stdin', async () => { + const spawned: EventedProcess[] = [] + spawnMock.mockImplementation(() => { + const proc = createEventedProcess() + spawned.push(proc) + return closeOnceSpawned(proc) + }) const hostPlatform = getRemoteHostPlatform('win32-x64') const promise = writeFileViaSystemSsh( @@ -722,16 +726,22 @@ describe('spawnSystemSsh', () => { ) await expect(promise).resolves.toBeUndefined() - const args = spawnMock.mock.calls[0][1] as string[] - const remoteCommand = args.at(-1) ?? '' - expect(remoteCommand).toContain('powershell.exe') - expect(remoteCommand).not.toContain('/bin/sh') - expect(proc.stdin.end).toHaveBeenCalledWith(Buffer.from('0.1.0', 'utf-8')) + // #16432, re-measured: Windows PowerShell 5.1 can lose a redirected stdin for good when a read + // finds it momentarily empty, so the bytes must not travel that way at all. + const batch = String(spawned[0]!.stdin.end.mock.calls[0]?.[0] ?? '') + expect(batch).toContain('put ') + expect(batch).toContain('/C:/Users/me/.orca-remote/relay/.version.orca-partial-') + const sftpArgs = spawnMock.mock.calls[0][1] as string[] + expect(sftpArgs).toContain('-b') + // The rename that publishes it reads the staged file, never a pipe. + const publish = (spawnMock.mock.calls[1][1] as string[]).at(-1) ?? '' + expect(publish).toContain('powershell.exe') + expect(decodePowerShellCommand(publish)).toContain('[System.IO.File]::Move($staging, $path)') + expect(publish).not.toContain('/bin/sh') }) - it('writes binary buffers to Windows system SSH targets with CreateNew mode', async () => { - const proc = createEventedProcess() - spawnMock.mockImplementation(() => closeOnceSpawned(proc)) + it('enforces an exclusive Windows buffer write at the rename, where it is atomic', async () => { + spawnMock.mockImplementation(() => closeOnceSpawned(createEventedProcess())) const hostPlatform = getRemoteHostPlatform('win32-x64') const promise = writeBufferViaSystemSsh( @@ -742,12 +752,11 @@ describe('spawnSystemSsh', () => { ) await expect(promise).resolves.toBeUndefined() - const args = spawnMock.mock.calls[0][1] as string[] - const remoteCommand = args.at(-1) ?? '' - expect(remoteCommand).toContain('powershell.exe') - expect(decodePowerShellCommand(remoteCommand)).toContain('CreateNew') - expect(remoteCommand).not.toContain('/bin/sh') - expect(proc.stdin.end).toHaveBeenCalledWith(Buffer.from('png')) + const publish = decodePowerShellCommand((spawnMock.mock.calls[1][1] as string[]).at(-1) ?? '') + // `File::Move` raising on an existing destination is what carries the exclusive contract now; + // a `CreateNew` on the staged file would only refuse a leftover of our own. + expect(publish).toContain('[System.IO.File]::Move($staging, $path)') + expect(publish).not.toContain('[System.IO.File]::Delete($path)') }) it('downloads files from Windows system SSH targets with PowerShell stdout bytes', async () => { @@ -779,8 +788,7 @@ describe('spawnSystemSsh', () => { }) it('forces standalone SSH for Windows file writes when requested', async () => { - const proc = createEventedProcess() - spawnMock.mockImplementation(() => closeOnceSpawned(proc)) + spawnMock.mockImplementation(() => closeOnceSpawned(createEventedProcess())) const hostPlatform = getRemoteHostPlatform('win32-x64') const promise = writeFileViaSystemSsh( @@ -791,10 +799,14 @@ describe('spawnSystemSsh', () => { ) await expect(promise).resolves.toBeUndefined() - const args = spawnMock.mock.calls[0][1] as string[] - const standaloneControlIdx = args.indexOf('-S') + const sftpArgs = spawnMock.mock.calls[0][1] as string[] + // sftp's own `-S` names a program to run, so the same request has to be spelled as an option. + expect(sftpArgs).not.toContain('-S') + expect(sftpArgs).toContain('ControlPath=none') + const publishArgs = spawnMock.mock.calls[1][1] as string[] + const standaloneControlIdx = publishArgs.indexOf('-S') expect(standaloneControlIdx).toBeGreaterThan(-1) - expect(args[standaloneControlIdx + 1]).toBe('none') + expect(publishArgs[standaloneControlIdx + 1]).toBe('none') }) it('uploads a Windows directory as a mkdir batch plus per-file writes, never one blob', async () => { @@ -819,19 +831,20 @@ describe('spawnSystemSsh', () => { rmSync(localDir, { recursive: true, force: true }) } + // #16432: directories first, then the file — but both over sftp now, so the only PowerShell + // left is the rename that publishes the staged file, which reads a file rather than a pipe. + const mkdirBatch = String(spawned[0]!.stdin.end.mock.calls[0]?.[0] ?? '') + expect(mkdirBatch).toBe('-mkdir "/C:/Users/me/.orca-remote/relay"\n') + const putBatch = String(spawned[1]!.stdin.end.mock.calls[0]?.[0] ?? '') + expect(putBatch).toContain('put ') + expect(putBatch).toContain('/C:/Users/me/.orca-remote/relay/relay.js.orca-partial-') const commands = spawnMock.mock.calls.map((call) => (call[1] as string[]).at(-1) ?? '') - // #16432: directories first (metadata only), then the file bytes on their own stdin. One batch - // meant base64-ing the whole bundle into a single PowerShell string, which the remote never read. - expect(commands).toHaveLength(2) - expect(commands.every((command) => command.includes('powershell.exe'))).toBe(true) expect(commands.every((command) => !command.includes('/bin/sh'))).toBe(true) expect(commands.join('\n')).not.toContain('tar -xzf') - expect(JSON.parse(spawned[0].stdin.end.mock.calls[0]?.[0] as string)).toEqual([ - 'C:/Users/me/.orca-remote/relay' - ]) - expect(Buffer.from(spawned[1].stdin.end.mock.calls[0]?.[0] as Buffer).toString('utf-8')).toBe( - 'console.log("relay")' - ) + // Nothing base64s the bundle into one PowerShell string any more, and nothing reads one. + expect( + commands.some((command) => decodePowerShellCommand(command).includes('OpenStandardInput')) + ).toBe(false) }) it('forces standalone SSH for Windows upload packages when requested', async () => { @@ -855,9 +868,9 @@ describe('spawnSystemSsh', () => { } const args = spawnMock.mock.calls[0][1] as string[] - const standaloneControlIdx = args.indexOf('-S') - expect(standaloneControlIdx).toBeGreaterThan(-1) - expect(args[standaloneControlIdx + 1]).toBe('none') + // The first spawn is the sftp client, whose own `-S` names a program to run. + expect(args).not.toContain('-S') + expect(args).toContain('ControlPath=none') }) it('throws when no system ssh is found', () => { diff --git a/src/main/ssh/system-ssh-file-binary-transfer.ts b/src/main/ssh/system-ssh-file-binary-transfer.ts index b0c5b662ed1..149d10dfbd3 100644 --- a/src/main/ssh/system-ssh-file-binary-transfer.ts +++ b/src/main/ssh/system-ssh-file-binary-transfer.ts @@ -1,5 +1,7 @@ import { constants, createWriteStream } from 'node:fs' -import { lstat, open } from 'node:fs/promises' +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' @@ -16,6 +18,16 @@ import { 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 @@ -74,13 +86,16 @@ export async function writeBufferViaSystemSsh( ): Promise { throwIfAborted(options?.signal) if (options?.hostPlatform && isWindowsRemoteHost(options.hostPlatform)) { - await writeWindowsBytesViaSystemSsh( + await writeWindowsRemoteFile( target, remotePath, - contents.length, - (offset, maxBytes) => - Promise.resolve(contents.subarray(offset, Math.min(offset + maxBytes, contents.length))), - options + { + totalBytes: contents.length, + readChunk: (offset, maxBytes) => + Promise.resolve(contents.subarray(offset, Math.min(offset + maxBytes, contents.length))), + withLocalFile: (send) => withTemporaryLocalFile(contents, send) + }, + options ?? {} ) return } @@ -127,20 +142,19 @@ export async function uploadFileViaSystemSsh( throwIfAborted(options?.signal) if (options?.hostPlatform && isWindowsRemoteHost(options.hostPlatform)) { - // #16432: a Windows host cannot take a whole file through one stdin, however the local side - // paces it — see WINDOWS_STDIN_WRITE_CHUNK_BYTES. This is the path that carries the large - // files, so it is the one that has to be chunked and bounded. - await writeWindowsBytesViaSystemSsh( - target, - remotePath, - openedStat.size, - async (offset, maxBytes) => { + // 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) }, - options - ) + // 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 } @@ -173,158 +187,56 @@ export async function uploadFileViaSystemSsh( } /** - * #16432: Windows PowerShell 5.1 stops draining a redirected stdin over a non-pty ssh exec - * somewhere between 50KB and 1MB, depending on the host's `DefaultShell`, and it hangs rather than - * failing. The reporter measured that on both constructs he tried — `[Console]::In.ReadToEnd()` and - * `new IO.StreamReader([Console]::OpenStandardInput())`, the latter reading incrementally, which is - * why the limit cannot be attributed to materializing the payload. `Stream.CopyTo` reads the same - * `[Console]::OpenStandardInput()` object with the same incremental `Read` loop, so nothing in it - * escapes that limit either: no single write may exceed what one stdin is known to carry. + * #16432, re-measured: the constraint is not a size limit, and it is not cmd.exe's. * - * 32KB is an order of magnitude under the low end of the measured range, and under 50KB, which the - * reporter measured succeeding against a stream reader on the worse of the two `DefaultShell` - * settings. - */ -export const WINDOWS_STDIN_WRITE_CHUNK_BYTES = 32 * 1024 - -/** No Windows stdin write should ever outlive this; a wedged PowerShell never closes on its own. */ -export const WINDOWS_STDIN_WRITE_TIMEOUT_MS = 60_000 - -/** Suffix for the path a multi-exec Windows write lands on before it is published by rename. */ -export const WINDOWS_STAGED_WRITE_SUFFIX = '.orca-partial' - -/** - * Splits one logical Windows write into stdin-sized execs. + * 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 write that needs more than one exec cannot land on the destination directly: a chunk failing - * mid-file would leave a truncated artifact under the real name with nothing marking it incomplete, - * and the retry would then meet its own leftovers — under `exclusive` the retry's `CreateNew` fails - * on them. Multi-exec creates therefore land on a staging path and are published by a rename, which - * is also where `exclusive` is enforced: once, at the destination, instead of smeared across the - * first chunk. A caller-requested append cannot be staged without reading the remote file back, so - * it keeps writing straight through, as its own protocol already implies. + * - 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. */ -async function writeWindowsBytesViaSystemSsh( - target: SshTarget, - remotePath: string, - totalBytes: number, - readChunk: (offset: number, maxBytes: number) => Promise, - options: SystemSshWriteBufferOptions -): Promise { - throwIfAborted(options.signal) - const staged = !options.append && totalBytes > WINDOWS_STDIN_WRITE_CHUNK_BYTES - const writePath = staged ? `${remotePath}${WINDOWS_STAGED_WRITE_SUFFIX}` : remotePath - let offset = 0 - // An empty write still has to run: it is what creates (or truncates) the file. - do { - const chunk = await readChunk(offset, WINDOWS_STDIN_WRITE_CHUNK_BYTES) - if (chunk.length === 0 && offset < totalBytes) { - throw new Error(`Source ran short during upload of ${remotePath}`) - } - await writeWindowsChunkViaSystemSsh( - target, - writePath, - chunk, - { - ...options, - append: staged ? offset > 0 : options.append === true || offset > 0, - exclusive: staged ? false : options.exclusive === true && offset === 0 - }, - offset - ) - offset += chunk.length - } while (offset < totalBytes) - if (staged) { - await publishWindowsStagedWrite(target, writePath, remotePath, options) + +/** A staged write is materialized locally first when the source is a buffer rather than a file. */ +async function withTemporaryLocalFile( + contents: Buffer, + send: (localPath: string) => Promise +): Promise { + 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(() => {}) } } -async function writeWindowsChunkViaSystemSsh( - target: SshTarget, - remotePath: string, - chunk: Buffer, - options: SystemSshWriteBufferOptions, - offset: number -): Promise { - throwIfAborted(options.signal) - const channel = spawnSystemSshCommand(target, makeWindowsWriteFileCommand(remotePath, options), { - wrapCommand: false, - ...getSystemSshBuildArgsFromOperationOptions(options) - }) - const closePromise = awaitWithSystemSshAbort( - options.signal, - () => channel.close(), - waitForChannelClose( - channel, - `write ${remotePath} at offset ${offset}`, - WINDOWS_STDIN_WRITE_TIMEOUT_MS - ) - ) - if (!options.signal?.aborted) { - channel.stdin.end(chunk) - } - await closePromise -} - -async function publishWindowsStagedWrite( - target: SshTarget, - stagingPath: string, - remotePath: string, - options: SystemSshWriteBufferOptions -): Promise { - throwIfAborted(options.signal) - const channel = spawnSystemSshCommand( - target, - makeWindowsPublishStagedFileCommand(stagingPath, remotePath, options.exclusive === true), - { wrapCommand: false, ...getSystemSshBuildArgsFromOperationOptions(options) } - ) - const closePromise = awaitWithSystemSshAbort( - options.signal, - () => channel.close(), - waitForChannelClose(channel, `publish ${remotePath}`, WINDOWS_STDIN_WRITE_TIMEOUT_MS) - ) - if (!options.signal?.aborted) { - channel.stdin.end() - } - await closePromise -} - -function makeWindowsWriteFileCommand( - remotePath: string, - options?: { append?: boolean; exclusive?: boolean } -): string { - const fileMode = options?.append ? 'Append' : options?.exclusive ? 'CreateNew' : 'Create' - return powerShellCommand( - [ - '$ErrorActionPreference = "Stop"', - `$path = ${powerShellLiteral(remotePath)}`, - '$parent = [System.IO.Path]::GetDirectoryName($path)', - 'if ($parent) { $null = [System.IO.Directory]::CreateDirectory($parent) }', - '$inputStream = [Console]::OpenStandardInput()', - `$outputStream = [System.IO.File]::Open($path, [System.IO.FileMode]::${fileMode}, [System.IO.FileAccess]::Write, [System.IO.FileShare]::None)`, - 'try { $inputStream.CopyTo($outputStream) } finally { $outputStream.Dispose() }' - ].join('; ') - ) -} - -// `File::Move` throws when the destination exists, which is exactly the exclusive contract; the -// non-exclusive caller asked to replace, so it deletes first (a no-op on an absent path). -function makeWindowsPublishStagedFileCommand( - stagingPath: string, - remotePath: string, - exclusive: boolean -): string { - return powerShellCommand( - [ - '$ErrorActionPreference = "Stop"', - `$staging = ${powerShellLiteral(stagingPath)}`, - `$path = ${powerShellLiteral(remotePath)}`, - ...(exclusive ? [] : ['[System.IO.File]::Delete($path)']), - '[System.IO.File]::Move($staging, $path)' - ].join('; ') - ) -} - function makePosixWriteFileCommand( remotePath: string, options?: { append?: boolean; exclusive?: boolean } diff --git a/src/main/ssh/system-ssh-file-transfer.ts b/src/main/ssh/system-ssh-file-transfer.ts index f728c0eab02..052ac5a79a3 100644 --- a/src/main/ssh/system-ssh-file-transfer.ts +++ b/src/main/ssh/system-ssh-file-transfer.ts @@ -27,6 +27,8 @@ import { WINDOWS_STDIN_WRITE_TIMEOUT_MS, writeBufferViaSystemSsh } from './system-ssh-file-binary-transfer' +import { isSftpUnavailableError, makeDirectoriesViaSftp } from './system-ssh-sftp-transfer' +import { getWindowsRemoteWriteCapabilities } from './system-ssh-windows-write-capabilities' type SystemSshOperationOptions = SystemSshBuildArgsOptions & { signal?: AbortSignal @@ -161,9 +163,14 @@ async function collectWindowsUploadPlan( return plan } -// Why the JSON envelope survives here: a path list is metadata, so this payload stays in the -// hundreds of bytes even for a deep tree. Batched anyway, so a pathological tree cannot walk back -// into the same stdin size that wedges PowerShell. +/** + * Creates the upload's directories, preferring sftp's own `mkdir`. + * + * The PowerShell fallback keeps the JSON envelope, batched under one stdin's worth: a path list is + * metadata, so it stays in the hundreds of bytes even for a deep tree. It is still a redirected + * stdin read though, so on Windows PowerShell 5.1 it carries the same defect as any other — which + * is why sftp is tried first even for a payload this small. + */ async function createWindowsUploadDirectories( target: SshTarget, directories: readonly string[], @@ -175,23 +182,17 @@ async function createWindowsUploadDirectories( if (batch.length === 0) { return } + const pending = batch const payload = JSON.stringify(batch) batch = [] batchBytes = 0 throwIfAborted(options.signal) - const channel = spawnSystemSshCommand(target, makeWindowsCreateDirectoriesCommand(), { - wrapCommand: false, - ...getSystemSshBuildArgsFromOperationOptions(options) - }) - const closePromise = awaitWithSystemSshAbort( - options.signal, - () => channel.close(), - waitForChannelClose(channel, 'windows relay upload mkdir', WINDOWS_STDIN_WRITE_TIMEOUT_MS) + await getWindowsRemoteWriteCapabilities(target).runWithFallback( + 'sftp-subsystem', + () => makeDirectoriesViaSftp(target, pending, options), + () => createWindowsUploadDirectoriesViaPowerShell(target, payload, options), + isSftpUnavailableError ) - if (!options.signal?.aborted) { - channel.stdin.end(payload) - } - await closePromise } for (const directory of directories) { const entryBytes = Buffer.byteLength(directory) + 4 @@ -204,12 +205,33 @@ async function createWindowsUploadDirectories( await flush() } +async function createWindowsUploadDirectoriesViaPowerShell( + target: SshTarget, + payload: string, + options: SystemSshOperationOptions +): Promise { + const channel = spawnSystemSshCommand(target, makeWindowsCreateDirectoriesCommand(), { + wrapCommand: false, + ...getSystemSshBuildArgsFromOperationOptions(options) + }) + const closePromise = awaitWithSystemSshAbort( + options.signal, + () => channel.close(), + waitForChannelClose(channel, 'windows relay upload mkdir', WINDOWS_STDIN_WRITE_TIMEOUT_MS) + ) + if (!options.signal?.aborted) { + channel.stdin.end(payload) + } + await closePromise +} + function makeWindowsCreateDirectoriesCommand(): string { return powerShellCommand( [ '$ErrorActionPreference = "Stop"', - // The reporter measured this reader surviving 50KB where `[Console]::In` wedged at the same - // size (#16432); the batch above stays under that. + // Reached only where the host has no sftp subsystem. Windows PowerShell 5.1 can lose a + // redirected stdin for good when a read finds it empty (#16432); a batch this small usually + // arrives in one piece, and "usually" is exactly why sftp is preferred. '$reader = New-Object System.IO.StreamReader([Console]::OpenStandardInput())', 'try { $json = $reader.ReadToEnd() } finally { $reader.Dispose() }', 'if ([string]::IsNullOrWhiteSpace($json)) { return }', diff --git a/src/main/ssh/system-ssh-sftp-args.test.ts b/src/main/ssh/system-ssh-sftp-args.test.ts new file mode 100644 index 00000000000..94db6ebbea5 --- /dev/null +++ b/src/main/ssh/system-ssh-sftp-args.test.ts @@ -0,0 +1,100 @@ +/** + * `buildSshArgs` is shared with the sftp client, and three of its flags mean something else there. + * Every case below is a silent wrong-target rather than an error if the translation is skipped, + * which is why the fallback is "refuse and use another transport", never "pass it through". + */ +import { describe, expect, it } from 'vitest' +import { + SftpArgTranslationError, + translateSshArgsToSftpArgs, + withSftpKeepalive +} from './system-ssh-sftp-args' + +describe('translateSshArgsToSftpArgs', () => { + it('sends the port as an option, since sftp -p preserves mtimes instead', () => { + const args = translateSshArgsToSftpArgs(['-p', '2222', '--', 'dev@win.example']) + + expect(args).toEqual(['-o', 'Port=2222', '--', 'dev@win.example']) + }) + + it('spells ControlPath=none out, since sftp -S names a program to run', () => { + // `sftp -S none` would try to exec a binary called `none`. + const args = translateSshArgsToSftpArgs(['-S', 'none', '--', 'dev@win.example']) + + expect(args).toEqual(['-o', 'ControlPath=none', '--', 'dev@win.example']) + }) + + it('refuses any other -S, which would hand sftp an ssh binary Orca did not choose', () => { + expect(() => translateSshArgsToSftpArgs(['-S', '/tmp/ctl.sock'])).toThrow( + SftpArgTranslationError + ) + }) + + it('drops -T, which sftp does not have', () => { + expect(translateSshArgsToSftpArgs(['-T', '--', 'host'])).toEqual(['--', 'host']) + }) + + it('passes through the flags both clients spell the same way', () => { + const args = translateSshArgsToSftpArgs([ + '-F', + '/tmp/config', + '-o', + 'BatchMode=yes', + '-i', + '/tmp/key', + '-J', + 'jump.example', + '--', + 'dev@win.example' + ]) + + expect(args).toEqual([ + '-F', + '/tmp/config', + '-o', + 'BatchMode=yes', + '-i', + '/tmp/key', + '-J', + 'jump.example', + '--', + 'dev@win.example' + ]) + }) + + it('takes everything after -- as the destination without reinterpreting it', () => { + // A host literally named `-p` is not a flag once `--` has been seen. + expect(translateSshArgsToSftpArgs(['--', '-p'])).toEqual(['--', '-p']) + }) + + it('refuses an unknown flag rather than guessing what sftp would do with it', () => { + // The point of the throw: a flag added to buildSshArgs later must degrade to another + // transport, not reach sftp carrying a different meaning. + expect(() => translateSshArgsToSftpArgs(['-A', '--', 'host'])).toThrow(SftpArgTranslationError) + }) + + it('refuses a value flag with no value', () => { + expect(() => translateSshArgsToSftpArgs(['-o'])).toThrow(SftpArgTranslationError) + }) +}) + +describe('withSftpKeepalive', () => { + it('asks OpenSSH to notice a dead peer, since the transfer itself has no wall-clock bound', () => { + expect(withSftpKeepalive(['--', 'host'])).toEqual([ + '-o', + 'ServerAliveInterval=15', + '-o', + 'ServerAliveCountMax=3', + '--', + 'host' + ]) + }) + + it('leaves a caller-stated keepalive policy alone', () => { + const args = withSftpKeepalive(['-o', 'ServerAliveInterval=60', '--', 'host']) + + expect(args.filter((arg) => arg.startsWith('ServerAliveInterval'))).toEqual([ + 'ServerAliveInterval=60' + ]) + }) +}) diff --git a/src/main/ssh/system-ssh-sftp-args.ts b/src/main/ssh/system-ssh-sftp-args.ts new file mode 100644 index 00000000000..55cb01184dc --- /dev/null +++ b/src/main/ssh/system-ssh-sftp-args.ts @@ -0,0 +1,84 @@ +/** + * Rewrites `buildSshArgs` output for the sftp(1) client. + * + * Three flags ssh and sftp share spell different things: sftp's `-p` is "preserve mtime", its `-S` + * names the ssh binary to run, and it has no `-T` at all. Passing ssh's list through unchanged + * would silently connect to the wrong port and try to exec a program called `none`. + * + * Anything this table does not recognize throws. A flag added to `buildSshArgs` later must degrade + * to the non-sftp transfer path, never reach sftp carrying a different meaning. + */ + +/** `buildSshArgs` emitted a flag with no sftp equivalent; the caller should use another transport. */ +export class SftpArgTranslationError extends Error { + constructor(flag: string) { + super(`No sftp equivalent for system ssh argument ${JSON.stringify(flag)}`) + this.name = 'SftpArgTranslationError' + } +} + +/** Flags whose spelling and meaning are identical in both clients. */ +const PASSTHROUGH_VALUE_FLAGS = new Set(['-F', '-o', '-i', '-J']) + +export function translateSshArgsToSftpArgs(sshArgs: readonly string[]): string[] { + const sftpArgs: string[] = [] + let index = 0 + while (index < sshArgs.length) { + const flag = sshArgs[index]! + if (flag === '--') { + // Everything after `--` is the destination, which both clients spell the same way. + sftpArgs.push(...sshArgs.slice(index)) + return sftpArgs + } + const value = sshArgs[index + 1] + if (PASSTHROUGH_VALUE_FLAGS.has(flag)) { + if (value === undefined) { + throw new SftpArgTranslationError(flag) + } + sftpArgs.push(flag, value) + index += 2 + continue + } + if (flag === '-T') { + // sftp never allocates a tty, so ssh's "no tty" request has nothing to translate to. + index += 1 + continue + } + if (flag === '-p') { + if (value === undefined) { + throw new SftpArgTranslationError(flag) + } + sftpArgs.push('-o', `Port=${value}`) + index += 2 + continue + } + if (flag === '-S') { + // ssh's `-S none` is ControlPath=none; sftp's `-S` would run a binary called `none`. + if (value !== 'none') { + throw new SftpArgTranslationError(flag) + } + sftpArgs.push('-o', 'ControlPath=none') + index += 2 + continue + } + throw new SftpArgTranslationError(flag) + } + return sftpArgs +} + +/** + * A transfer that stalls mid-stream has no per-write bound to catch it, so ask OpenSSH to notice a + * dead peer itself. Only added when the caller has not already stated a keepalive policy. + */ +export function withSftpKeepalive(sftpArgs: readonly string[]): string[] { + const hasOption = (name: string): boolean => + sftpArgs.some((arg, position) => sftpArgs[position - 1] === '-o' && arg.startsWith(`${name}=`)) + const keepalive: string[] = [] + if (!hasOption('ServerAliveInterval')) { + keepalive.push('-o', 'ServerAliveInterval=15') + } + if (!hasOption('ServerAliveCountMax')) { + keepalive.push('-o', 'ServerAliveCountMax=3') + } + return [...keepalive, ...sftpArgs] +} diff --git a/src/main/ssh/system-ssh-sftp-path.test.ts b/src/main/ssh/system-ssh-sftp-path.test.ts new file mode 100644 index 00000000000..e196c2e3e8f --- /dev/null +++ b/src/main/ssh/system-ssh-sftp-path.test.ts @@ -0,0 +1,59 @@ +/** + * Both functions here guard against the same measured failure: sftp's batch lexer treats `\` as an + * escape, so a Windows path handed over raw is silently mis-targeted *and the client still exits + * 0*. On Windows 11 / OpenSSH 10.0p2, `put src C:\Users\neil\qt\a.bin` created a file literally + * named `C` in the start directory and reported success. + */ +import { describe, expect, it } from 'vitest' +import { + quoteSftpBatchArgument, + toSftpRemotePath, + UnsupportedSftpPathError +} from './system-ssh-sftp-path' + +describe('toSftpRemotePath', () => { + it('roots a drive path under /, which is the namespace the Windows sftp-server exposes', () => { + // `pwd` in that session reports `/C:/Users/dev`. + expect(toSftpRemotePath('C:/Users/dev/f.bin')).toBe('/C:/Users/dev/f.bin') + }) + + it('accepts a path already in that namespace unchanged', () => { + expect(toSftpRemotePath('/C:/Users/dev/f.bin')).toBe('/C:/Users/dev/f.bin') + }) + + it('converts the separators Orca stores paths with', () => { + expect(toSftpRemotePath('C:\\Users\\dev\\f.bin')).toBe('/C:/Users/dev/f.bin') + }) + + it('declines a UNC path rather than guessing where it lands', () => { + // A guess here writes real bytes to the wrong place; declining falls back to another transport. + expect(() => toSftpRemotePath('//server/share/f.bin')).toThrow(UnsupportedSftpPathError) + }) + + it('declines a relative path, which would resolve against the session start directory', () => { + expect(() => toSftpRemotePath('Users/dev/f.bin')).toThrow(UnsupportedSftpPathError) + }) +}) + +describe('quoteSftpBatchArgument', () => { + it('escapes the backslashes in a Windows client local path', () => { + // Unescaped, sftp reads this as C:srcf.bin and fails to find the source. + expect(quoteSftpBatchArgument('C:\\src\\f.bin')).toBe('"C:\\\\src\\\\f.bin"') + }) + + it('keeps a path with spaces as one argument', () => { + expect(quoteSftpBatchArgument('/tmp/two words.bin')).toBe('"/tmp/two words.bin"') + }) + + it('escapes an embedded quote, which would otherwise end the argument early', () => { + expect(quoteSftpBatchArgument('/tmp/dq".bin')).toBe('"/tmp/dq\\".bin"') + }) + + it('refuses a line break, which would split one batch command into two', () => { + expect(() => quoteSftpBatchArgument('/tmp/a\nrm -rf b')).toThrow(UnsupportedSftpPathError) + }) + + it('refuses a NUL, which truncates the argument', () => { + expect(() => quoteSftpBatchArgument('/tmp/a\0b')).toThrow(UnsupportedSftpPathError) + }) +}) diff --git a/src/main/ssh/system-ssh-sftp-path.ts b/src/main/ssh/system-ssh-sftp-path.ts new file mode 100644 index 00000000000..2b5bfe53f02 --- /dev/null +++ b/src/main/ssh/system-ssh-sftp-path.ts @@ -0,0 +1,46 @@ +import { normalizeWindowsRemotePath } from './ssh-remote-platform' + +/** + * A path this transfer cannot express to sftp. Callers treat it as "use another transport", never + * as a transfer failure. + */ +export class UnsupportedSftpPathError extends Error { + constructor(path: string) { + super(`Path cannot be addressed over sftp: ${JSON.stringify(path)}`) + this.name = 'UnsupportedSftpPathError' + } +} + +/** + * Converts a Windows remote path to the namespace OpenSSH's Windows sftp-server exposes, which + * roots every drive under `/`: `C:/Users/dev/f` is `/C:/Users/dev/f`, and `pwd` there reports + * `/C:/Users/dev`. + */ +export function toSftpRemotePath(remotePath: string): string { + const normalized = normalizeWindowsRemotePath(remotePath) + if (/^\/[a-zA-Z]:\//.test(normalized)) { + return normalized + } + if (/^[a-zA-Z]:\//.test(normalized)) { + return `/${normalized}` + } + // UNC (`//server/share`) and relative paths have no settled mapping in this namespace, and a + // guess here writes real bytes to the wrong place. Decline instead. + throw new UnsupportedSftpPathError(remotePath) +} + +/** + * Quotes one argument of an sftp batch line. + * + * Escaping is load-bearing, not cosmetic: sftp's batch lexer treats `\` as an escape even inside + * double quotes, so an unescaped Windows local path `C:\src\f.bin` is read as `C:srcf.bin`, and an + * unescaped destination `C:\Users\dev\f.bin` writes a file literally named `C` in the start + * directory — while sftp still exits 0. Both measured on Windows 11 / OpenSSH 10.0p2. + */ +export function quoteSftpBatchArgument(value: string): string { + if (/[\n\r\0]/.test(value)) { + // A line break would split one batch command into two; NUL truncates the argument. + throw new UnsupportedSftpPathError(value) + } + return `"${value.replace(/([\\"])/g, '\\$1')}"` +} diff --git a/src/main/ssh/system-ssh-sftp-transfer.ts b/src/main/ssh/system-ssh-sftp-transfer.ts new file mode 100644 index 00000000000..2e5c6f6dcb1 --- /dev/null +++ b/src/main/ssh/system-ssh-sftp-transfer.ts @@ -0,0 +1,176 @@ +import { accessSync, constants, existsSync, statSync } from 'node:fs' +import { posix, win32 } from 'node:path' +import type { SshTarget } from '../../shared/ssh-types' +import { buildSshArgs, type SystemSshBuildArgsOptions } from './system-ssh-args' +import { findSystemSsh } from './system-ssh-binary' +import { + SftpArgTranslationError, + translateSshArgsToSftpArgs, + withSftpKeepalive +} from './system-ssh-sftp-args' +import { + quoteSftpBatchArgument, + toSftpRemotePath, + UnsupportedSftpPathError +} from './system-ssh-sftp-path' +import { throwIfAborted } from './system-ssh-operation-lifecycle' +import { runProcess } from '../../shared/child-process/run-process' + +/** The host answered, but not with an sftp subsystem. The caller must fall back, not fail. */ +export class SftpSubsystemUnavailableError extends Error { + constructor(detail: string) { + super(`Remote host has no usable sftp subsystem: ${detail}`) + this.name = 'SftpSubsystemUnavailableError' + } +} + +/** + * True for the errors that mean "this host cannot do sftp", as opposed to "this transfer failed". + * Deliberately narrow: a permission denial or a missing directory is a real failure that must + * surface, not a reason to retry the whole upload down a slower path. + */ +export function isSftpUnavailableError(error: unknown): boolean { + return ( + error instanceof SftpSubsystemUnavailableError || + error instanceof SftpArgTranslationError || + error instanceof UnsupportedSftpPathError + ) +} + +function systemSftpCandidates(sshPath: string | null, platform: NodeJS.Platform): string[] { + const pathApi = platform === 'win32' ? win32 : posix + const executable = platform === 'win32' ? 'sftp.exe' : 'sftp' + const candidates: string[] = [] + // Why the ssh binary's own directory first: a host with two OpenSSH installs must pair the sftp + // client with the ssh that `buildSshArgs` was built for, not whichever one PATH happens to reach. + if (sshPath) { + candidates.push(pathApi.join(pathApi.dirname(sshPath), executable)) + } + if (platform === 'win32') { + const systemRoot = process.env.SystemRoot || process.env.WINDIR + if (systemRoot) { + candidates.push(win32.join(systemRoot, 'System32', 'OpenSSH', executable)) + } + } else { + candidates.push('/usr/bin/sftp', '/usr/local/bin/sftp', '/opt/homebrew/bin/sftp') + } + return candidates +} + +/** Locate the sftp client paired with the system ssh binary. Returns null when there is none. */ +export function findSystemSftp(): string | null { + if (process.env.ORCA_SYSTEM_SFTP_PATH) { + return process.env.ORCA_SYSTEM_SFTP_PATH + } + const sshPath = findSystemSsh() + for (const candidate of systemSftpCandidates(sshPath, process.platform)) { + try { + if (!statSync(candidate).isFile()) { + continue + } + if (process.platform !== 'win32') { + accessSync(candidate, constants.X_OK) + } + return candidate + } catch { + continue + } + } + return findSftpOnPath() +} + +function findSftpOnPath(): string | null { + const pathValue = process.env.PATH + if (!pathValue) { + return null + } + const pathApi = process.platform === 'win32' ? win32 : posix + const executable = process.platform === 'win32' ? 'sftp.exe' : 'sftp' + for (const entry of pathValue.split(pathApi.delimiter)) { + const directory = entry.trim().replace(/^"|"$/g, '') + if (!directory) { + continue + } + const candidate = pathApi.join(directory, executable) + if (existsSync(candidate)) { + return candidate + } + } + return null +} + +/** + * OpenSSH prints this when the server refuses the subsystem — a host with `Subsystem sftp` + * commented out, or an internal-sftp block that does not apply to this user. + */ +const SUBSYSTEM_REFUSED_PATTERN = /subsystem request failed|no such file or directory.*sftp-server/i + +export type SftpBatchOptions = SystemSshBuildArgsOptions & { signal?: AbortSignal } + +/** + * Runs one sftp batch script. + * + * The script goes to the *local* sftp client's stdin, which is the point: no remote process ever + * reads a redirected stdin, so none of this rides the Windows PowerShell stdin defect. + */ +export async function runSftpBatch( + target: SshTarget, + commands: readonly string[], + options?: SftpBatchOptions +): Promise { + throwIfAborted(options?.signal) + const sftpPath = findSystemSftp() + if (!sftpPath) { + throw new SftpSubsystemUnavailableError('no sftp client binary found alongside ssh') + } + const args = withSftpKeepalive(translateSshArgsToSftpArgs(buildSshArgs(target, options))) + let result + try { + result = await runProcess({ + program: sftpPath, + args: ['-b', '-', ...args], + // `-b -` takes the script on stdin, and that stdin is the *local* client's — no remote + // process reads a pipe anywhere in this transfer, which is the whole point of preferring it. + input: `${commands.join('\n')}\n`, + // Why no timeout: a large upload is legitimately slow, and a wall-clock cap would fail a + // healthy transfer on a slow link. A dead peer is caught by the ServerAlive options instead. + timeoutMs: null, + signal: options?.signal + }) + } catch (error) { + // A client that will not start is "this host cannot do sftp" from the caller's side, not a + // transfer failure: the payload never left. Falling back is the only useful answer. + throw new SftpSubsystemUnavailableError( + `sftp client at ${sftpPath} could not be started: ${error instanceof Error ? error.message : String(error)}` + ) + } + if (result.code === 0) { + return + } + throwIfAborted(options?.signal) + const detail = result.stderr.trim() + if (SUBSYSTEM_REFUSED_PATTERN.test(detail)) { + throw new SftpSubsystemUnavailableError(detail) + } + throw new Error(`sftp batch failed (exit ${result.code}): ${detail}`) +} + +/** + * Creates remote directories, parents first. + * + * `-mkdir` keeps sftp going when a directory is already there; batch mode otherwise aborts the + * whole script on the first non-zero status, which for an idempotent tree walk is not a failure. + */ +export function makeDirectoriesViaSftp( + target: SshTarget, + remoteDirectories: readonly string[], + options?: SftpBatchOptions +): Promise { + const commands = remoteDirectories.map( + (directory) => `-mkdir ${quoteSftpBatchArgument(toSftpRemotePath(directory))}` + ) + if (commands.length === 0) { + return Promise.resolve() + } + return runSftpBatch(target, commands, options) +} diff --git a/src/main/ssh/system-ssh-windows-file-write.ts b/src/main/ssh/system-ssh-windows-file-write.ts new file mode 100644 index 00000000000..7b2cb1f015e --- /dev/null +++ b/src/main/ssh/system-ssh-windows-file-write.ts @@ -0,0 +1,121 @@ +import { randomBytes } from 'node:crypto' +import { powerShellCommand, powerShellLiteral } from './ssh-remote-powershell' +import { normalizeWindowsRemotePath } from './ssh-remote-platform' + +/** + * Suffix marking the path a Windows write lands on before it is published by rename. + * + * The random tail is the fix for a measured harm, not decoration. A write that loses contact with + * the host leaves a remote process that may still hold the staging file open exclusively, and + * `docs/reference/ssh-execution-boundary.md` is explicit that losing contact is not evidence that + * process died — so the retry must not reuse the name it may still own. A fresh name per attempt + * means a retry never meets its predecessor's lock; the abandoned file is cleaned up best-effort + * and never treated as proof of anything. + */ +export const WINDOWS_STAGED_WRITE_SUFFIX = '.orca-partial' + +export function makeWindowsStagingPath(remotePath: string): string { + return `${remotePath}${WINDOWS_STAGED_WRITE_SUFFIX}-${randomBytes(6).toString('hex')}` +} + +export type WindowsPublishMode = 'create' | 'exclusive' | 'append' + +/** + * Publishes a staged upload onto its real name. + * + * Every branch reads the staged *file*, never a redirected stdin, which is what makes this safe on + * a host whose Windows PowerShell 5.1 cannot drain a piped stdin. `File::Move` throws when the + * destination exists, which is exactly the exclusive contract; the replacing caller deletes first + * (a no-op on an absent path). + */ +export function makeWindowsPublishStagedFileCommand( + stagingPath: string, + remotePath: string, + mode: WindowsPublishMode +): string { + const preamble = [ + '$ErrorActionPreference = "Stop"', + `$staging = ${powerShellLiteral(stagingPath)}`, + `$path = ${powerShellLiteral(remotePath)}`, + '$parent = [System.IO.Path]::GetDirectoryName($path)', + 'if ($parent) { $null = [System.IO.Directory]::CreateDirectory($parent) }' + ] + if (mode === 'append') { + return powerShellCommand( + [ + ...preamble, + '$in = [System.IO.File]::OpenRead($staging)', + '$out = [System.IO.File]::Open($path, [System.IO.FileMode]::Append, [System.IO.FileAccess]::Write, [System.IO.FileShare]::None)', + 'try { $in.CopyTo($out) } finally { $out.Dispose(); $in.Dispose() }', + '[System.IO.File]::Delete($staging)' + ].join('; ') + ) + } + return powerShellCommand( + [ + ...preamble, + ...(mode === 'exclusive' ? [] : ['[System.IO.File]::Delete($path)']), + '[System.IO.File]::Move($staging, $path)' + ].join('; ') + ) +} + +/** Best-effort removal of a staged file whose write was abandoned. Never asserts the writer died. */ +export function makeWindowsDiscardStagedFileCommand(stagingPath: string): string { + return powerShellCommand( + [ + // Deliberately not `Stop`: the previous writer may still hold this file, and that is a + // possibility to tolerate, not an error to report. The unique staging name means a leftover + // blocks nothing; sweeping it is housekeeping. + '$ErrorActionPreference = "SilentlyContinue"', + `$staging = ${powerShellLiteral(stagingPath)}`, + '[System.IO.File]::Delete($staging)' + ].join('; ') + ) +} + +/** + * The ancestor directories of a Windows remote path, drive root first. + * + * sftp's `mkdir` creates one level, so a batch has to name each level itself. The drive root is + * excluded: `-mkdir "/C:/"` is not a directory anyone creates. + */ +export function windowsRemoteAncestorDirectories(remotePath: string): string[] { + const normalized = normalizeWindowsRemotePath(remotePath) + const segments = normalized.split('/') + segments.pop() + const ancestors: string[] = [] + // Start past the drive (`C:`) or the UNC host, which are never created. + for (let depth = 2; depth <= segments.length; depth += 1) { + const directory = segments.slice(0, depth).join('/') + if (directory) { + ancestors.push(directory) + } + } + return ancestors +} + +/** + * `[Console]::OpenStandardInput()` into a `FileStream`, used only by the two stdin fallbacks. + * + * On Windows PowerShell 5.1 this is the defective read; see the strategy comment in + * `system-ssh-file-binary-transfer.ts`. It is correct under PowerShell 7. + */ +export function makeWindowsWriteFileCommand( + remotePath: string, + options?: { append?: boolean; exclusive?: boolean; executable?: 'powershell.exe' | 'pwsh.exe' } +): string { + const fileMode = options?.append ? 'Append' : options?.exclusive ? 'CreateNew' : 'Create' + return powerShellCommand( + [ + '$ErrorActionPreference = "Stop"', + `$path = ${powerShellLiteral(remotePath)}`, + '$parent = [System.IO.Path]::GetDirectoryName($path)', + 'if ($parent) { $null = [System.IO.Directory]::CreateDirectory($parent) }', + '$inputStream = [Console]::OpenStandardInput()', + `$outputStream = [System.IO.File]::Open($path, [System.IO.FileMode]::${fileMode}, [System.IO.FileAccess]::Write, [System.IO.FileShare]::None)`, + 'try { $inputStream.CopyTo($outputStream) } finally { $outputStream.Dispose() }' + ].join('; '), + options?.executable ?? 'powershell.exe' + ) +} diff --git a/src/main/ssh/system-ssh-windows-upload.test.ts b/src/main/ssh/system-ssh-windows-upload.test.ts index 207c3e2df8e..82fed2a9b57 100644 --- a/src/main/ssh/system-ssh-windows-upload.test.ts +++ b/src/main/ssh/system-ssh-windows-upload.test.ts @@ -1,29 +1,40 @@ /** - * #16432: the Windows relay upload pushed the whole bundle into one PowerShell stdin, which - * Windows PowerShell 5.1 cannot drain over a non-pty ssh exec — the remote blocks forever, and - * `waitForChannelClose()` had no timeout, so the UI sat at "Connecting…" with no error. Covered - * here: no write exceeds one stdin's worth on any Windows path (bundle upload *and* single-file - * upload, which is the one that carries large files), a partial write never lands under the real - * name, and a remote that never closes fails instead of hanging. + * #16432. The original fix chunked the payload because the constraint was believed to be a ~50KB + * cmd.exe stdin ceiling. Re-measured on Windows 11 26200.9168 / OpenSSH_for_Windows_10.0p2, it is + * not a size limit and 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, taking + * both the remaining data and the EOF with it. It is probabilistic per such read — identical 2MB + * payloads died at 167936, 270336 and 372736 — so a 32KB chunk still failed 15 times in 120 under + * load, while `findstr` took 2,016,000 bytes through one exec on the same host. + * + * So the covering property is no longer "every write is small". It is "the bytes do not cross a + * remote process's stdin at all": sftp first, PowerShell 7 next, and Windows PowerShell 5.1 last, + * bounded and loud. The staging-and-rename discipline is kept on every path, with a unique staging + * name per attempt so a retry never meets a predecessor's lock. */ import { EventEmitter } from 'node:events' import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs' -import { rm } from 'node:fs/promises' +import { readFile, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { PassThrough, Writable } from 'node:stream' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type * as SystemSshOperationLifecycle from './system-ssh-operation-lifecycle' -const { spawnSystemSshCommandMock, waitForChannelCloseSpy } = vi.hoisted(() => ({ +const { spawnSystemSshCommandMock, waitForChannelCloseSpy, runProcessMock } = vi.hoisted(() => ({ spawnSystemSshCommandMock: vi.fn(), - waitForChannelCloseSpy: vi.fn() + waitForChannelCloseSpy: vi.fn(), + runProcessMock: vi.fn() })) vi.mock('./system-ssh-command', () => ({ spawnSystemSshCommand: spawnSystemSshCommandMock })) +vi.mock('../../shared/child-process/run-process', () => ({ + runProcess: runProcessMock +})) + // Delegates to the real implementation; the spy only records whether each wait was given a bound. vi.mock('./system-ssh-operation-lifecycle', async (importActual) => { const actual = (await importActual()) as typeof SystemSshOperationLifecycle @@ -41,6 +52,11 @@ import { } from './system-ssh-file-binary-transfer' import { waitForChannelClose } from './system-ssh-operation-lifecycle' import { getRemoteHostPlatform } from './ssh-remote-platform' +import { + clearWindowsRemoteWriteCapabilitiesForTests, + getWindowsRemoteWriteCapabilities +} from './system-ssh-windows-write-capabilities' +import { explainWindowsPowerShellStdinFailure } from './system-ssh-windows-write-strategy' import type { SshTarget } from '../../shared/ssh-types' type FakeChannel = EventEmitter & { @@ -50,7 +66,12 @@ type FakeChannel = EventEmitter & { written: Buffer } -const target = { id: 'win-1', host: 'win.example', username: 'dev' } as unknown as SshTarget +const target = { + id: 'win-1', + host: 'win.example', + username: 'dev', + port: 22 +} as unknown as SshTarget const hostPlatform = getRemoteHostPlatform('win32-x64') const remoteRoot = 'C:/Users/dev/.orca-remote' @@ -78,96 +99,212 @@ function createFakeChannel(onEnd: (channel: FakeChannel) => void): FakeChannel { return channel } -type RecordedCommand = { script: string; stdin: Buffer } +type RecordedCommand = { script: string; executable: string; stdin: Buffer } +type RecordedSftpBatch = { args: string[]; script: string } -describe('Windows upload stdin framing', () => { - let localDir: string - const commands: RecordedCommand[] = [] - /** Index of the spawn that should report a non-zero exit, to model a chunk failing mid-file. */ - let failAtSpawn = -1 +const sftpBatches: RecordedSftpBatch[] = [] +const commands: RecordedCommand[] = [] +/** Index of the exec that should report a non-zero exit, to model a chunk failing mid-file. */ +let failAtSpawn = -1 +let localDir: string - const fileWrites = (): RecordedCommand[] => - commands.filter((command) => command.script.includes('FileMode]::')) - const writtenPath = (command: RecordedCommand): string => - /\$path = '((?:[^']|'')*)'/.exec(command.script)?.[1].replace(/''/g, "'") ?? '' - const fileMode = (command: RecordedCommand): string | undefined => - /FileMode\]::(\w+)/.exec(command.script)?.[1] +const fileWrites = (): RecordedCommand[] => + commands.filter((command) => command.script.includes('OpenStandardInput')) +const writtenPath = (command: RecordedCommand): string => + /\$path = '((?:[^']|'')*)'/.exec(command.script)?.[1]?.replace(/''/g, "'") ?? '' +const fileMode = (command: RecordedCommand): string | undefined => + /FileMode\]::(\w+)/.exec(command.script)?.[1] +const putLines = (): string[] => + sftpBatches.flatMap((batch) => batch.script.split('\n').filter((line) => line.startsWith('put '))) +const putDestination = (line: string): string => /put "(?:[^"]*)" "([^"]*)"/.exec(line)?.[1] ?? '' +const putSource = (line: string): string => /put "([^"]*)"/.exec(line)?.[1] ?? '' - beforeEach(() => { - commands.length = 0 - failAtSpawn = -1 - waitForChannelCloseSpy.mockClear() - localDir = mkdtempSync(join(tmpdir(), 'orca-win-upload-')) - spawnSystemSshCommandMock.mockReset() - spawnSystemSshCommandMock.mockImplementation((_target: SshTarget, command: string) => { - const spawnIndex = spawnSystemSshCommandMock.mock.calls.length - 1 - return createFakeChannel((channel) => { - commands.push({ script: decodePowerShellCommand(command), stdin: channel.written }) - setImmediate(() => - spawnIndex === failAtSpawn - ? channel.emit('close', 1, null) - : channel.emit('close', 0, null) - ) +/** Makes every sftp batch succeed, recording what it was asked to do. */ +function acceptSftp(): void { + runProcessMock.mockImplementation( + async (spec: { args: string[]; input: string; program: string }) => { + const script = spec.input + sftpBatches.push({ args: spec.args, script }) + // Model the real client: `put` copies the local file, so read it while it still exists. + for (const line of script.split('\n').filter((entry) => entry.startsWith('put '))) { + await readFile(putSource(line)) + } + return { code: 0, signal: null, stdout: '', stderr: '', timedOut: false } + } + ) +} + +/** Models a host whose sshd has no `Subsystem sftp` line. */ +function refuseSftp(): void { + runProcessMock.mockImplementation(async (spec: { args: string[]; input: string }) => { + sftpBatches.push({ args: spec.args, script: spec.input }) + return { + code: 255, + signal: null, + stdout: '', + stderr: 'subsystem request failed on channel 0\nConnection closed', + timedOut: false + } + }) +} + +/** Models a host with no PowerShell 7, which cmd.exe reports as an unrecognized command. */ +function refusePwsh(): void { + spawnSystemSshCommandMock.mockImplementation((_target: SshTarget, command: string) => { + const spawnIndex = spawnSystemSshCommandMock.mock.calls.length - 1 + const executable = command.split(' ')[0] ?? '' + return createFakeChannel((channel) => { + commands.push({ + script: decodePowerShellCommand(command), + executable, + stdin: channel.written + }) + setImmediate(() => { + if (executable === 'pwsh.exe') { + channel.stderr.write( + "'pwsh.exe' is not recognized as an internal or external command,\noperable program or batch file." + ) + channel.emit('close', 9009, null) + return + } + channel.emit('close', spawnIndex === failAtSpawn ? 1 : 0, null) }) }) }) +} - afterEach(async () => { - await rm(localDir, { recursive: true, force: true }) +beforeEach(() => { + commands.length = 0 + sftpBatches.length = 0 + failAtSpawn = -1 + clearWindowsRemoteWriteCapabilitiesForTests() + waitForChannelCloseSpy.mockClear() + localDir = mkdtempSync(join(tmpdir(), 'orca-win-upload-')) + process.env.ORCA_SYSTEM_SFTP_PATH = '/usr/bin/sftp' + runProcessMock.mockReset() + acceptSftp() + spawnSystemSshCommandMock.mockReset() + spawnSystemSshCommandMock.mockImplementation((_target: SshTarget, command: string) => { + const spawnIndex = spawnSystemSshCommandMock.mock.calls.length - 1 + return createFakeChannel((channel) => { + commands.push({ + script: decodePowerShellCommand(command), + executable: command.split(' ')[0] ?? '', + stdin: channel.written + }) + setImmediate(() => + spawnIndex === failAtSpawn ? channel.emit('close', 1, null) : channel.emit('close', 0, null) + ) + }) }) +}) - it('never pushes a whole artifact bundle into one PowerShell stdin', async () => { - mkdirSync(join(localDir, 'node'), { recursive: true }) - // Comfortably past the ~50KB point at which the reporter measured PowerShell 5.1 wedging. - writeFileSync(join(localDir, 'node', 'relay.js'), Buffer.alloc(600 * 1024, 0x61)) - writeFileSync(join(localDir, 'index.js'), Buffer.alloc(300 * 1024, 0x62)) +afterEach(async () => { + delete process.env.ORCA_SYSTEM_SFTP_PATH + await rm(localDir, { recursive: true, force: true }) +}) - await uploadDirectoryViaSystemSsh(target, localDir, remoteRoot, { hostPlatform }) - - const largest = Math.max(...commands.map((command) => command.stdin.length)) - expect(largest).toBeLessThanOrEqual(WINDOWS_STDIN_WRITE_CHUNK_BYTES) - // The base64 + JSON envelope is gone entirely: nothing reads the bundle as one string. - expect(commands.some((command) => command.script.includes('FromBase64String'))).toBe(false) - // `[Console]::In` wedged at 50KB where the stream reader did not, so the mkdir batch — the one - // payload still read as a string — must use the reader the reporter measured surviving. - expect(commands.some((command) => command.script.includes('[Console]::In.ReadToEnd()'))).toBe( - false - ) - expect( - commands.filter((command) => command.script.includes('StreamReader([Console]::')) - ).toHaveLength(1) - }) - - it('bounds the single-file upload too, which is the path large files take', async () => { - const contents = Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES * 3 + 11, 0x64) +describe('Windows upload over sftp', () => { + it('moves the payload without any remote process reading a stdin', async () => { + const contents = Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES * 60 + 11, 0x64) const localPath = join(localDir, 'big.node') writeFileSync(localPath, contents) await uploadFileViaSystemSsh(target, localPath, `${remoteRoot}/big.node`, { hostPlatform }) - const writes = fileWrites() - expect(writes).toHaveLength(4) - expect(Math.max(...writes.map((write) => write.stdin.length))).toBe( - WINDOWS_STDIN_WRITE_CHUNK_BYTES - ) - expect(Buffer.concat(writes.map((write) => write.stdin)).equals(contents)).toBe(true) - // A wedged PowerShell never closes on its own, so no wait on this path may be unbounded. - expect( - waitForChannelCloseSpy.mock.calls.every((call) => call[2] === WINDOWS_STDIN_WRITE_TIMEOUT_MS) - ).toBe(true) + // The defect is a remote stdin read; the fix is that there is not one. + expect(fileWrites()).toHaveLength(0) + expect(putLines()).toHaveLength(1) + // One transfer, not 61 execs: the whole point of the change. + expect(sftpBatches).toHaveLength(1) }) - it('writes every byte of every artifact across the chunked writes', async () => { - const contents = Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES * 2 + 17, 0x63) - writeFileSync(join(localDir, 'relay.js'), contents) + it('creates the parent chain and sends the payload in one round trip', async () => { + writeFileSync(join(localDir, 'relay.js'), 'x') - await uploadDirectoryViaSystemSsh(target, localDir, remoteRoot, { hostPlatform }) + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/a/b/relay.js`, { + hostPlatform + }) - const writes = fileWrites() - expect(writes).toHaveLength(3) - expect(Buffer.concat(writes.map((write) => write.stdin)).equals(contents)).toBe(true) - // Only the first write creates the staging file; the rest must extend it or it is truncated. - expect(writes.map(fileMode)).toEqual(['Create', 'Append', 'Append']) + expect(sftpBatches).toHaveLength(1) + expect(sftpBatches[0]!.script.split('\n').filter(Boolean)).toEqual([ + '-mkdir "/C:/Users"', + '-mkdir "/C:/Users/dev"', + '-mkdir "/C:/Users/dev/.orca-remote"', + '-mkdir "/C:/Users/dev/.orca-remote/a"', + '-mkdir "/C:/Users/dev/.orca-remote/a/b"', + expect.stringContaining('put ') as unknown as string + ]) + }) + + it('addresses the destination in the drive-rooted namespace sftp exposes', async () => { + writeFileSync(join(localDir, 'relay.js'), 'x') + + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/relay.js`, { + hostPlatform + }) + + // A backslash destination silently writes a file named `C` and still exits 0, so the leading + // slash and forward separators are correctness, not style. + expect(putDestination(putLines()[0]!)).toMatch( + /^\/C:\/Users\/dev\/\.orca-remote\/relay\.js\.orca-partial-[0-9a-f]{12}$/ + ) + }) + + it('never lands a partial under the real name, and publishes by rename', async () => { + writeFileSync(join(localDir, 'relay.js'), 'x') + const remotePath = `${remoteRoot}/relay.js` + + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), remotePath, { hostPlatform }) + + expect(putDestination(putLines()[0]!)).not.toBe(`/C:${remotePath.slice(2)}`) + const publish = commands.at(-1)! + expect(publish.script).toContain('[System.IO.File]::Move($staging, $path)') + expect(publish.script).toContain('[System.IO.File]::Delete($path)') + // The publish reads the staged file, never a pipe, so it is safe on PowerShell 5.1. + expect(publish.script).not.toContain('OpenStandardInput') + }) + + it('gives every attempt its own staging name, so a retry cannot meet a predecessor lock', async () => { + writeFileSync(join(localDir, 'relay.js'), 'x') + + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/relay.js`, { + hostPlatform + }) + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/relay.js`, { + hostPlatform + }) + + const [first, second] = putLines().map(putDestination) + expect(first).toContain(WINDOWS_STAGED_WRITE_SUFFIX) + // Losing contact is not evidence the previous writer died, so the name must not be reused. + expect(second).not.toBe(first) + }) + + it('enforces exclusive at the rename, where it is atomic', async () => { + writeFileSync(join(localDir, 'import.bin'), 'x') + + await uploadFileViaSystemSsh(target, join(localDir, 'import.bin'), `${remoteRoot}/import.bin`, { + hostPlatform, + exclusive: true + }) + + const publish = commands.at(-1)! + expect(publish.script).toContain('[System.IO.File]::Move($staging, $path)') + expect(publish.script).not.toContain('[System.IO.File]::Delete($path)') + }) + + it('appends by concatenating the staged file, not by piping bytes to the remote', async () => { + await writeBufferViaSystemSsh(target, `${remoteRoot}/log.bin`, Buffer.from('tail'), { + hostPlatform, + append: true + }) + + expect(fileWrites()).toHaveLength(0) + const publish = commands.at(-1)! + expect(publish.script).toContain('FileMode]::Append') + expect(publish.script).toContain('$in.CopyTo($out)') + expect(publish.script).toContain('[System.IO.File]::Delete($staging)') }) it('still creates an empty artifact on the host', async () => { @@ -175,80 +312,232 @@ describe('Windows upload stdin framing', () => { await uploadDirectoryViaSystemSsh(target, localDir, remoteRoot, { hostPlatform }) - expect(fileWrites().map(writtenPath)).toEqual([`${remoteRoot}/empty.txt`]) - expect(fileWrites()[0].stdin).toHaveLength(0) - expect(fileMode(fileWrites()[0])).toBe('Create') + expect(putLines()).toHaveLength(1) + expect(commands.at(-1)!.script).toContain('[System.IO.File]::Move($staging, $path)') }) - it('lands a multi-chunk write on a staging path and publishes it by rename', async () => { - const remotePath = `${remoteRoot}/relay.js` - writeFileSync(join(localDir, 'relay.js'), Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES + 1)) + it('writes a buffer through a 0600 temp file that does not outlive the transfer', async () => { + const seen: { path: string; contents: Buffer }[] = [] + runProcessMock.mockImplementation(async (spec: { args: string[]; input: string }) => { + sftpBatches.push({ args: spec.args, script: spec.input }) + for (const line of spec.input.split('\n').filter((entry) => entry.startsWith('put '))) { + const path = putSource(line) + seen.push({ path, contents: await readFile(path) }) + } + return { code: 0, signal: null, stdout: '', stderr: '', timedOut: false } + }) + + await writeBufferViaSystemSsh(target, `${remoteRoot}/version`, Buffer.from('1.2.3'), { + hostPlatform + }) + + expect(seen).toHaveLength(1) + expect(seen[0]!.contents.toString()).toBe('1.2.3') + await expect(readFile(seen[0]!.path)).rejects.toThrow() + }) + + it('creates upload directories over sftp rather than a PowerShell stdin batch', async () => { + mkdirSync(join(localDir, 'node'), { recursive: true }) + writeFileSync(join(localDir, 'node', 'relay.js'), 'x') await uploadDirectoryViaSystemSsh(target, localDir, remoteRoot, { hostPlatform }) - // Nothing touches the real name until every byte is on the host. - expect(fileWrites().map(writtenPath)).toEqual([ - `${remotePath}${WINDOWS_STAGED_WRITE_SUFFIX}`, - `${remotePath}${WINDOWS_STAGED_WRITE_SUFFIX}` - ]) - const publish = commands.at(-1)! - expect(publish.script).toContain('[System.IO.File]::Move($staging, $path)') - expect(publish.script).toContain('[System.IO.File]::Delete($path)') + expect(commands.some((command) => command.script.includes('StreamReader([Console]::'))).toBe( + false + ) + expect(sftpBatches[0]!.script).toContain('-mkdir "/C:/Users/dev/.orca-remote"') + }) + + it('sweeps the staged bytes when the publish is the thing that fails', async () => { + writeFileSync(join(localDir, 'import.bin'), 'x') + // An exclusive conflict is the ordinary way to get here: the payload is on the host, and the + // rename that would have given it a name refuses. + spawnSystemSshCommandMock.mockImplementation((_target: SshTarget, command: string) => { + const script = decodePowerShellCommand(command) + return createFakeChannel((channel) => { + commands.push({ script, executable: command.split(' ')[0] ?? '', stdin: channel.written }) + const failed = script.includes('::Move($staging, $path)') + setImmediate(() => channel.emit('close', failed ? 1 : 0, null)) + }) + }) + + await expect( + uploadFileViaSystemSsh(target, join(localDir, 'import.bin'), `${remoteRoot}/import.bin`, { + hostPlatform, + exclusive: true + }) + ).rejects.toThrow() + + const sweep = commands.at(-1)! + expect(sweep.script).toContain('[System.IO.File]::Delete($staging)') + // Tolerated, not asserted: the previous writer may still hold the file, and losing contact is + // not evidence it died. + expect(sweep.script).toContain('$ErrorActionPreference = "SilentlyContinue"') + }) + + it('reports a cancelled transfer as an abort, not as a failed one', async () => { + writeFileSync(join(localDir, 'relay.js'), 'x') + const controller = new AbortController() + // runProcess reports the kill as a non-zero exit rather than throwing, so without checking the + // signal first a user pressing cancel is indistinguishable from the transfer genuinely failing. + runProcessMock.mockImplementation(async (spec: { args: string[]; input: string }) => { + sftpBatches.push({ args: spec.args, script: spec.input }) + controller.abort() + return { code: 255, signal: 'SIGTERM', stdout: '', stderr: '', timedOut: false } + }) + + let error: Error | undefined + try { + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/relay.js`, { + hostPlatform, + signal: controller.signal + }) + } catch (thrown) { + error = thrown as Error + } + + expect(error?.name).toBe('AbortError') + expect(error?.message).not.toContain('sftp batch failed') + // A cancel is also not evidence about the host, so it must not send later writes to the slow + // path, and must not fall through to the defective reader now. + expect(getWindowsRemoteWriteCapabilities(target).shouldTry('sftp-subsystem')).toBe(true) + expect(fileWrites()).toHaveLength(0) + }) + + it('translates the ssh argument list rather than passing it to a client that reads it differently', async () => { + writeFileSync(join(localDir, 'relay.js'), 'x') + + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/relay.js`, { + hostPlatform, + disableControlMaster: true + }) + + const args = sftpBatches[0]!.args + // sftp's `-T` does not exist, its `-p` preserves mtime, and its `-S` names a program to run. + expect(args).not.toContain('-T') + expect(args).not.toContain('-p') + expect(args).not.toContain('-S') + expect(args).toContain('ControlPath=none') + expect(args).toContain('ServerAliveInterval=15') + }) +}) + +describe('Windows upload on a host with no sftp subsystem', () => { + beforeEach(() => { + refuseSftp() + }) + + it('falls back rather than failing the transfer', async () => { + const contents = Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES + 5, 0x61) + writeFileSync(join(localDir, 'relay.js'), contents) + + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/relay.js`, { + hostPlatform + }) + + expect(Buffer.concat(fileWrites().map((write) => write.stdin)).equals(contents)).toBe(true) + }) + + it('remembers the refusal, so a multi-file upload probes once', async () => { + writeFileSync(join(localDir, 'a.js'), 'a') + writeFileSync(join(localDir, 'b.js'), 'b') + writeFileSync(join(localDir, 'c.js'), 'c') + + await uploadDirectoryViaSystemSsh(target, localDir, remoteRoot, { hostPlatform }) + + // One refusal is enough; re-probing per file is a wasted round trip on every file. + expect(sftpBatches).toHaveLength(1) + }) + + it('does not spend a sweep round trip when sftp declined before moving any bytes', async () => { + writeFileSync(join(localDir, 'relay.js'), 'x') + + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/relay.js`, { + hostPlatform + }) + + // A refused subsystem staged nothing, so there is nothing to delete — and on a host without + // sftp that sweep would otherwise be paid on every single write. + expect(commands.some((command) => command.script.includes('Delete($staging)'))).toBe(false) + }) + + it('prefers PowerShell 7, which reads a redirected stdin correctly', async () => { + writeFileSync(join(localDir, 'relay.js'), Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES * 3)) + + await uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/relay.js`, { + hostPlatform + }) + + expect(fileWrites().map((write) => write.executable)).toEqual(['pwsh.exe']) + // PowerShell 7 took 2MB through one exec when measured, so chunking it buys nothing. + expect(fileWrites()[0]!.stdin).toHaveLength(WINDOWS_STDIN_WRITE_CHUNK_BYTES * 3) + }) + + it('bounds every write when only Windows PowerShell 5.1 is available', async () => { + refusePwsh() + const contents = Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES * 3 + 11, 0x64) + writeFileSync(join(localDir, 'big.node'), contents) + + await uploadFileViaSystemSsh(target, join(localDir, 'big.node'), `${remoteRoot}/big.node`, { + hostPlatform + }) + + const writes = fileWrites().filter((write) => write.executable === 'powershell.exe') + expect(writes).toHaveLength(4) + expect(Math.max(...writes.map((write) => write.stdin.length))).toBe( + WINDOWS_STDIN_WRITE_CHUNK_BYTES + ) + expect(Buffer.concat(writes.map((write) => write.stdin)).equals(contents)).toBe(true) + expect(writes.map(fileMode)).toEqual(['Create', 'Append', 'Append', 'Append']) + // A wedged PowerShell never closes on its own, so no wait on this path may be unbounded. + expect( + waitForChannelCloseSpy.mock.calls.every((call) => call[2] === WINDOWS_STDIN_WRITE_TIMEOUT_MS) + ).toBe(true) + }) + + it('remembers that PowerShell 7 is absent instead of re-probing per chunk', async () => { + refusePwsh() + writeFileSync(join(localDir, 'big.node'), Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES * 3)) + + await uploadFileViaSystemSsh(target, join(localDir, 'big.node'), `${remoteRoot}/big.node`, { + hostPlatform + }) + + expect(fileWrites().filter((write) => write.executable === 'pwsh.exe')).toHaveLength(1) }) it('leaves no truncated file under the real name when a chunk fails mid-file', async () => { writeFileSync(join(localDir, 'relay.js'), Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES * 3)) - // Spawns: 0 = mkdir batch, 1..3 = chunk writes. Fail the second chunk. - failAtSpawn = 2 + // Spawn 0 is the pwsh write; fail it and every retry beneath it. + failAtSpawn = 0 await expect( - uploadDirectoryViaSystemSsh(target, localDir, remoteRoot, { hostPlatform }) + uploadFileViaSystemSsh(target, join(localDir, 'relay.js'), `${remoteRoot}/relay.js`, { + hostPlatform + }) ).rejects.toThrow() expect(fileWrites().map(writtenPath)).not.toContain(`${remoteRoot}/relay.js`) expect(commands.some((command) => command.script.includes('::Move('))).toBe(false) }) +}) - it('enforces exclusive once at the rename, so a retry is not blocked by its own leftovers', async () => { - const localPath = join(localDir, 'import.bin') - writeFileSync(localPath, Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES + 1)) +describe('last-resort Windows PowerShell failure reporting', () => { + it('names the host limitation and its remedy, not just the timeout', () => { + const timeout = new Error('write C:/x at offset 0 timed out after 60000ms with no response') - await uploadFileViaSystemSsh(target, localPath, `${remoteRoot}/import.bin`, { - hostPlatform, - exclusive: true - }) + const explained = explainWindowsPowerShellStdinFailure(timeout) as Error - // CreateNew on chunk one would fail against a leftover staging file from a failed attempt; - // `File::Move` raising on an existing destination is what carries the exclusive contract. - expect(fileWrites().map(fileMode)).toEqual(['Create', 'Append']) - const publish = commands.at(-1)! - expect(publish.script).toContain('[System.IO.File]::Move($staging, $path)') - expect(publish.script).not.toContain('[System.IO.File]::Delete($path)') + // "timed out" alone sends the user to retry a network they cannot fix; the fix is host-side. + expect(explained.message).toContain('Windows PowerShell 5.1') + expect(explained.message).toContain('Subsystem sftp sftp-server.exe') + expect(explained.cause).toBe(timeout) }) - it('keeps a single-chunk write on the destination, with the caller mode intact', async () => { - await writeBufferViaSystemSsh(target, `${remoteRoot}/version`, Buffer.from('1.2.3'), { - hostPlatform, - exclusive: true - }) + it('leaves a real failure alone, so a permission error is not reported as a host limitation', () => { + const denied = new Error('write C:/x at offset 0 failed (exit 1): Access to the path is denied') - expect(fileWrites()).toHaveLength(1) - expect(writtenPath(fileWrites()[0])).toBe(`${remoteRoot}/version`) - expect(fileMode(fileWrites()[0])).toBe('CreateNew') - expect(commands.some((command) => command.script.includes('::Move('))).toBe(false) - }) - - it('appends onto the destination rather than staging, since append cannot be staged', async () => { - const remotePath = `${remoteRoot}/log.bin` - await writeBufferViaSystemSsh( - target, - remotePath, - Buffer.alloc(WINDOWS_STDIN_WRITE_CHUNK_BYTES + 1), - { hostPlatform, append: true } - ) - - expect(fileWrites().map(writtenPath)).toEqual([remotePath, remotePath]) - expect(fileWrites().map(fileMode)).toEqual(['Append', 'Append']) + expect(explainWindowsPowerShellStdinFailure(denied)).toBe(denied) }) }) diff --git a/src/main/ssh/system-ssh-windows-write-capabilities.test.ts b/src/main/ssh/system-ssh-windows-write-capabilities.test.ts new file mode 100644 index 00000000000..ad723d592b0 --- /dev/null +++ b/src/main/ssh/system-ssh-windows-write-capabilities.test.ts @@ -0,0 +1,79 @@ +/** + * Whether a Windows host has an sftp subsystem is a fact about that host, so the cache is keyed by + * the endpoint that executes rather than by Orca's target id — otherwise a hardened host is + * re-probed once per file, and two targets pointing at one machine learn the same fact twice. + */ +import { afterEach, describe, expect, it } from 'vitest' +import type { SshTarget } from '../../shared/ssh-types' +import { + clearWindowsRemoteWriteCapabilitiesForTests, + getWindowsRemoteWriteCapabilities, + getWindowsRemoteWriteExecutionHostKey +} from './system-ssh-windows-write-capabilities' + +const asTarget = (fields: Partial): SshTarget => fields as SshTarget + +afterEach(() => { + clearWindowsRemoteWriteCapabilitiesForTests() +}) + +describe('getWindowsRemoteWriteExecutionHostKey', () => { + it('gives two targets on one endpoint the same key', () => { + const first = asTarget({ id: 'a', host: 'win.example', username: 'dev', port: 22 }) + const second = asTarget({ id: 'b', host: 'win.example', username: 'dev', port: 22 }) + + // A target re-created under a new id has not changed what the host supports. + expect(getWindowsRemoteWriteExecutionHostKey(first)).toBe( + getWindowsRemoteWriteExecutionHostKey(second) + ) + }) + + it('separates hosts, ports and users', () => { + const base = { id: 'a', host: 'win.example', username: 'dev', port: 22 } + const keys = [ + asTarget(base), + asTarget({ ...base, host: 'other.example' }), + asTarget({ ...base, port: 2222 }), + asTarget({ ...base, username: 'ops' }) + ].map(getWindowsRemoteWriteExecutionHostKey) + + expect(new Set(keys).size).toBe(4) + }) + + it('keys a config alias by the alias, since ssh_config decides where it lands', () => { + const alias = asTarget({ id: 'a', host: 'stale.example', configHost: 'winbox' }) + + expect(getWindowsRemoteWriteExecutionHostKey(alias)).toBe('config:winbox') + }) +}) + +describe('getWindowsRemoteWriteCapabilities', () => { + it('shares one cache across targets that reach the same host', () => { + const first = asTarget({ id: 'a', host: 'win.example', username: 'dev', port: 22 }) + const second = asTarget({ id: 'b', host: 'win.example', username: 'dev', port: 22 }) + + getWindowsRemoteWriteCapabilities(first).rememberUnsupported('sftp-subsystem') + + expect(getWindowsRemoteWriteCapabilities(second).shouldTry('sftp-subsystem')).toBe(false) + }) + + it('does not let one host answer for another', () => { + const hardened = asTarget({ id: 'a', host: 'hardened.example', username: 'dev', port: 22 }) + const ordinary = asTarget({ id: 'b', host: 'ordinary.example', username: 'dev', port: 22 }) + + getWindowsRemoteWriteCapabilities(hardened).rememberUnsupported('sftp-subsystem') + + expect(getWindowsRemoteWriteCapabilities(ordinary).shouldTry('sftp-subsystem')).toBe(true) + }) + + it('keeps the two capabilities independent', () => { + const target = asTarget({ id: 'a', host: 'win.example', username: 'dev', port: 22 }) + const capabilities = getWindowsRemoteWriteCapabilities(target) + + capabilities.rememberUnsupported('pwsh') + + // No PowerShell 7 says nothing about whether the host will serve sftp. + expect(capabilities.shouldTry('sftp-subsystem')).toBe(true) + expect(capabilities.shouldTry('pwsh')).toBe(false) + }) +}) diff --git a/src/main/ssh/system-ssh-windows-write-capabilities.ts b/src/main/ssh/system-ssh-windows-write-capabilities.ts new file mode 100644 index 00000000000..dcd03f19807 --- /dev/null +++ b/src/main/ssh/system-ssh-windows-write-capabilities.ts @@ -0,0 +1,52 @@ +import type { SshTarget } from '../../shared/ssh-types' +import { CapabilityProbeCache } from '../../shared/capability-probe-cache' + +/** + * Whether a Windows host can take a file write over the sftp subsystem, and whether it has a + * PowerShell 7 to fall back to. Both are host facts, so they are cached per execution host rather + * than per transfer — a hardened host with `Subsystem sftp` removed must not be re-probed on every + * file of a multi-file upload. + */ +export type WindowsRemoteWriteCapability = 'sftp-subsystem' | 'pwsh' + +// Why re-probe at all: an admin can enable the subsystem, or install PowerShell 7, without the +// user restarting Orca. Long enough that a hardened host costs one failed probe per half hour. +export const WINDOWS_WRITE_CAPABILITY_RETRY_INTERVAL_MS = 30 * 60_000 + +const capabilitiesByExecutionHost = new Map< + string, + CapabilityProbeCache +>() + +/** + * Keyed by the endpoint that executes, not by target id: two Orca targets pointing at one host + * describe the same sshd, and a target re-created under a new id has not changed what that host + * supports. A config alias is its own key because ssh_config, not Orca, resolves where it lands. + */ +export function getWindowsRemoteWriteExecutionHostKey(target: SshTarget): string { + if (target.configHost) { + return `config:${target.configHost}` + } + const port = target.port ?? 22 + return target.username + ? `host:${target.username}@${target.host}:${port}` + : `host:${target.host}:${port}` +} + +export function getWindowsRemoteWriteCapabilities( + target: SshTarget +): CapabilityProbeCache { + const key = getWindowsRemoteWriteExecutionHostKey(target) + let cache = capabilitiesByExecutionHost.get(key) + if (!cache) { + cache = new CapabilityProbeCache( + WINDOWS_WRITE_CAPABILITY_RETRY_INTERVAL_MS + ) + capabilitiesByExecutionHost.set(key, cache) + } + return cache +} + +export function clearWindowsRemoteWriteCapabilitiesForTests(): void { + capabilitiesByExecutionHost.clear() +} diff --git a/src/main/ssh/system-ssh-windows-write-strategy.ts b/src/main/ssh/system-ssh-windows-write-strategy.ts new file mode 100644 index 00000000000..7df9f800925 --- /dev/null +++ b/src/main/ssh/system-ssh-windows-write-strategy.ts @@ -0,0 +1,301 @@ +import type { SshTarget } from '../../shared/ssh-types' +import { getSystemSshBuildArgsFromOperationOptions } from './system-ssh-args' +import { spawnSystemSshCommand } from './system-ssh-command' +import { + awaitWithSystemSshAbort, + throwIfAborted, + waitForChannelClose +} from './system-ssh-operation-lifecycle' +import { isSftpUnavailableError, runSftpBatch } from './system-ssh-sftp-transfer' +import { quoteSftpBatchArgument, toSftpRemotePath } from './system-ssh-sftp-path' +import { getWindowsRemoteWriteCapabilities } from './system-ssh-windows-write-capabilities' +import { + makeWindowsDiscardStagedFileCommand, + makeWindowsPublishStagedFileCommand, + makeWindowsStagingPath, + makeWindowsWriteFileCommand, + windowsRemoteAncestorDirectories, + type WindowsPublishMode +} from './system-ssh-windows-file-write' + +/** No Windows stdin write should ever outlive this; a wedged PowerShell never closes on its own. */ +export const WINDOWS_STDIN_WRITE_TIMEOUT_MS = 60_000 + +/** + * Bound on one stdin write for the last-resort Windows PowerShell 5.1 path. + * + * Measured on Windows 11 26200 / OpenSSH 10.0p2: a 32KB write still hangs 15 times in 120 under + * load, and no smaller value removes the risk. The defect is per blocking read, not per byte, so + * shrinking the chunk trades one risky read for more execs that each carry their own. This is a + * damage bound on a path known to be unreliable, not a safe size. + */ +export const WINDOWS_STDIN_WRITE_CHUNK_BYTES = 32 * 1024 + +export type WindowsWriteOptions = Parameters< + typeof getSystemSshBuildArgsFromOperationOptions +>[0] & { + signal?: AbortSignal + append?: boolean + exclusive?: boolean +} + +/** Bytes to write, plus a way to present them to sftp, which can only send a local file. */ +export type WindowsWriteSource = { + totalBytes: number + readChunk: (offset: number, maxBytes: number) => Promise + withLocalFile: (send: (localPath: string) => Promise) => Promise +} + +function publishMode(options: WindowsWriteOptions): WindowsPublishMode { + return options.append ? 'append' : options.exclusive === true ? 'exclusive' : 'create' +} + +/** + * Writes one file to a Windows host, preferring transports that do not push bytes through a remote + * PowerShell's stdin. + * + * Order, and why: sftp carries the whole payload in one transfer and never has a remote process + * read a pipe. Measured on Windows 11 / OpenSSH 10.0p2: 1.9MB in a median 315ms over sftp against + * 0 of 6 completions on the chunked path, whose best case was ~62 execs at ~350ms each. PowerShell + * 7 reads a redirected stdin correctly but is not installed by default. Windows PowerShell 5.1 is + * always present and is the defective reader, so it is last and it is bounded. + * + * Every transport stages under a unique name and publishes by rename, so no partial write is ever + * visible under the real name and no retry inherits a predecessor's lock. + */ +export async function writeWindowsRemoteFile( + target: SshTarget, + remotePath: string, + source: WindowsWriteSource, + options: WindowsWriteOptions +): Promise { + throwIfAborted(options.signal) + const capabilities = getWindowsRemoteWriteCapabilities(target) + await capabilities.runWithFallback( + 'sftp-subsystem', + () => writeViaSftp(target, remotePath, source, options), + () => writeViaRemoteStdin(target, remotePath, source, options), + isSftpUnavailableError + ) +} + +/** + * Stages under a name nothing else can own, publishes it, and sweeps the staging file if either + * step fails. + * + * Shared by both transports so the cleanup contract cannot drift between them: a failed publish — + * an exclusive conflict is the ordinary case — leaves bytes on the host that no longer have a + * purpose, and the sweep is what stops them accumulating. + */ +async function stageThenPublish( + target: SshTarget, + remotePath: string, + options: WindowsWriteOptions, + stage: (stagingPath: string) => Promise, + nothingStaged: (error: unknown) => boolean = () => false +): Promise { + const stagingPath = makeWindowsStagingPath(remotePath) + try { + await stage(stagingPath) + await publishStagedWrite(target, stagingPath, remotePath, options) + } catch (error) { + // A transport that declined before it moved any bytes has nothing to sweep, and sweeping + // anyway would spend a round trip on every write to a host that has no sftp subsystem. + if (!nothingStaged(error)) { + await discardStagedWrite(target, stagingPath, options) + } + throw error + } +} + +function writeViaSftp( + target: SshTarget, + remotePath: string, + source: WindowsWriteSource, + options: WindowsWriteOptions +): Promise { + const mkdirs = windowsRemoteAncestorDirectories(remotePath).map( + (directory) => `-mkdir ${quoteSftpBatchArgument(toSftpRemotePath(directory))}` + ) + return stageThenPublish( + target, + remotePath, + options, + (stagingPath) => + source.withLocalFile((localPath) => + // One round trip: the parent chain and the payload travel in the same batch. + runSftpBatch( + target, + [ + ...mkdirs, + `put ${quoteSftpBatchArgument(localPath)} ${quoteSftpBatchArgument(toSftpRemotePath(stagingPath))}` + ], + options + ) + ), + isSftpUnavailableError + ) +} + +function writeViaRemoteStdin( + target: SshTarget, + remotePath: string, + source: WindowsWriteSource, + options: WindowsWriteOptions +): Promise { + const capabilities = getWindowsRemoteWriteCapabilities(target) + return stageThenPublish(target, remotePath, options, (stagingPath) => + capabilities.runWithFallback( + 'pwsh', + () => writeStdinChunks(target, stagingPath, source, options, 'pwsh.exe'), + () => writeStdinChunks(target, stagingPath, source, options, 'powershell.exe'), + isPwshUnavailableError + ) + ) +} + +/** + * PowerShell 7 takes the whole payload in one exec — measured at 2MB — so only the 5.1 path pays + * for chunking, and only because a bounded write is the most that path can be trusted with. + */ +async function writeStdinChunks( + target: SshTarget, + stagingPath: string, + source: WindowsWriteSource, + options: WindowsWriteOptions, + executable: 'powershell.exe' | 'pwsh.exe' +): Promise { + const chunkBytes = + executable === 'pwsh.exe' ? Math.max(source.totalBytes, 1) : WINDOWS_STDIN_WRITE_CHUNK_BYTES + let offset = 0 + // An empty write still has to run: it is what creates the staged file. + do { + const chunk = await source.readChunk(offset, chunkBytes) + if (chunk.length === 0 && offset < source.totalBytes) { + throw new Error(`Source ran short during upload of ${stagingPath}`) + } + await writeOneStdinChunk( + target, + stagingPath, + chunk, + { ...options, append: offset > 0, exclusive: false }, + offset, + executable + ) + offset += chunk.length + } while (offset < source.totalBytes) +} + +async function writeOneStdinChunk( + target: SshTarget, + stagingPath: string, + chunk: Buffer, + options: WindowsWriteOptions, + offset: number, + executable: 'powershell.exe' | 'pwsh.exe' +): Promise { + throwIfAborted(options.signal) + const channel = spawnSystemSshCommand( + target, + makeWindowsWriteFileCommand(stagingPath, { + append: options.append, + exclusive: options.exclusive, + executable + }), + { wrapCommand: false, ...getSystemSshBuildArgsFromOperationOptions(options) } + ) + const closePromise = awaitWithSystemSshAbort( + options.signal, + () => channel.close(), + waitForChannelClose( + channel, + `write ${stagingPath} at offset ${offset}`, + WINDOWS_STDIN_WRITE_TIMEOUT_MS + ) + ).catch((error: unknown) => { + throw executable === 'powershell.exe' ? explainWindowsPowerShellStdinFailure(error) : error + }) + if (!options.signal?.aborted) { + channel.stdin.end(chunk) + } + await closePromise +} + +/** + * Names the cause on the one path that can hang, so the failure is not just "timed out". + * + * A user seeing this needs to know it is a host limitation with a host-side remedy, not a network + * fault they should retry into. + */ +export function explainWindowsPowerShellStdinFailure(error: unknown): unknown { + const message = error instanceof Error ? error.message : String(error) + if (!/timed out/i.test(message)) { + return error + } + return new Error( + `${message}\nWindows PowerShell 5.1 can lose a redirected stdin permanently when a read finds it momentarily empty, so this write cannot be made reliable from the client. Enable the sftp subsystem on the host (sshd_config: "Subsystem sftp sftp-server.exe"), or install PowerShell 7, and Orca will use it automatically.`, + { cause: error instanceof Error ? error : undefined } + ) +} + +function isPwshUnavailableError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error) + // cmd.exe's "not recognized" and sshd's exit 9009 both mean "no pwsh here". A timeout does not: + // that is the stdin defect, and PowerShell 7 does not have it, so it must not be cached as absent. + return /is not recognized as an internal or external command|9009|CommandNotFoundException/i.test( + message + ) +} + +async function publishStagedWrite( + target: SshTarget, + stagingPath: string, + remotePath: string, + options: WindowsWriteOptions +): Promise { + await runWindowsCommandWithoutStdin( + target, + makeWindowsPublishStagedFileCommand(stagingPath, remotePath, publishMode(options)), + `publish ${remotePath}`, + options + ) +} + +async function discardStagedWrite( + target: SshTarget, + stagingPath: string, + options: WindowsWriteOptions +): Promise { + try { + await runWindowsCommandWithoutStdin( + target, + makeWindowsDiscardStagedFileCommand(stagingPath), + `discard ${stagingPath}`, + { ...options, signal: undefined } + ) + } catch { + // Housekeeping only. The staging name is unique, so a leftover blocks nothing, and a failure + // here says nothing about whether the abandoned writer is still alive. + } +} + +function runWindowsCommandWithoutStdin( + target: SshTarget, + command: string, + label: string, + options: WindowsWriteOptions +): Promise { + const channel = spawnSystemSshCommand(target, command, { + wrapCommand: false, + ...getSystemSshBuildArgsFromOperationOptions(options) + }) + const closePromise = awaitWithSystemSshAbort( + options.signal, + () => channel.close(), + waitForChannelClose(channel, label, WINDOWS_STDIN_WRITE_TIMEOUT_MS) + ) + if (!options.signal?.aborted) { + channel.stdin.end() + } + return closePromise +}