diff --git a/src/main/ssh/relay-socket-path-limit-shell.integration.test.ts b/src/main/ssh/relay-socket-path-limit-shell.integration.test.ts index 7dfff201f49..ce08556c3d7 100644 --- a/src/main/ssh/relay-socket-path-limit-shell.integration.test.ts +++ b/src/main/ssh/relay-socket-path-limit-shell.integration.test.ts @@ -4,7 +4,10 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { promisify } from 'node:util' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { resolveShortRelaySocketDirCommand } from './relay-socket-path-limit' +import { + resolveShortRelaySocketDirCommand, + shortRelayVersionSegment +} from './relay-socket-path-limit' const run = promisify(execFile) @@ -13,9 +16,14 @@ const run = promisify(execFile) describe('short relay socket dir guard, against a real shell', () => { let root: string + const VERSION_SEGMENT = shortRelayVersionSegment('relay-0.1.0+test') + // Retarget the generated script at a sandbox instead of the real /tmp path. function scriptFor(dir: string): string { - return resolveShortRelaySocketDirCommand().replace(/^dir=.*$/m, `dir=${JSON.stringify(dir)}`) + return resolveShortRelaySocketDirCommand(VERSION_SEGMENT).replace( + /^dir=.*$/m, + `dir=${JSON.stringify(dir)}` + ) } async function attempt(dir: string): Promise<{ ok: boolean; stdout: string }> { @@ -35,10 +43,28 @@ describe('short relay socket dir guard, against a real shell', () => { await rm(root, { recursive: true, force: true }) }) - it('creates the directory when it does not exist', async () => { + it('creates the directory and its version segment when neither exists', async () => { const dir = join(root, 'fresh') - expect((await attempt(dir)).ok).toBe(true) + const attempted = await attempt(dir) + expect(attempted.ok).toBe(true) expect((await stat(dir)).mode & 0o777).toBe(0o700) + // The segment is what keeps a later build off the path this one binds. + expect((await stat(join(dir, VERSION_SEGMENT))).mode & 0o777).toBe(0o700) + expect(attempted.stdout.trim().endsWith(`${dir}/${VERSION_SEGMENT}`)).toBe(true) + }) + + it('refuses a planted symlink in the version segment without following it', async () => { + const dir = join(root, 'mine') + const victim = join(root, 'segment-victim') + await mkdir(dir) + await chmod(dir, 0o700) + await mkdir(victim) + await chmod(victim, 0o755) + await symlink(victim, join(dir, VERSION_SEGMENT)) + + const before = (await stat(victim)).mode & 0o777 + expect((await attempt(dir)).ok).toBe(false) + expect((await stat(victim)).mode & 0o777).toBe(before) }) it('adopts a directory we already own at 0700, so reconnects keep working', async () => { diff --git a/src/main/ssh/relay-socket-path-limit.test.ts b/src/main/ssh/relay-socket-path-limit.test.ts index b8be7c13a52..7334ff9d914 100644 --- a/src/main/ssh/relay-socket-path-limit.test.ts +++ b/src/main/ssh/relay-socket-path-limit.test.ts @@ -68,8 +68,10 @@ import { parseShortRelaySocketDir, remoteSocketPathFitsLimit, remoteUnixSocketPathByteLimit, + shortRelayVersionSegment, SHORT_RELAY_SOCKET_DIR_PREFIX } from './relay-socket-path-limit' +import { supersededRelayEndpointListCommand } from './ssh-relay-superseded-endpoints' import { getRemoteHostPlatform } from './ssh-remote-platform' import type { SshConnection } from './ssh-connection' @@ -80,6 +82,9 @@ const WINDOWS = getRemoteHostPlatform('win32-x64') // The reporter's host: a managed-hosting container whose $HOME is 45 bytes (#10726). const LONG_HOME = '/var/www/611f7cf9-f715-49e6-91d9-0ffac1d7c4c0' +/** Matches the version this suite's mocked build reports. */ +const RELAY_VERSION_DIR_NAME = 'relay-0.1.0+8d4e15ad63eb' + function makeMockConnection(): SshConnection { return { canRunConcurrentExecCommands: vi.fn().mockReturnValue(true), @@ -133,13 +138,24 @@ describe('remote unix socket path limit', () => { }) it('accepts only the marker line as the short directory', () => { + const segment = shortRelayVersionSegment(RELAY_VERSION_DIR_NAME) expect( parseShortRelaySocketDir( - 'Welcome to Ubuntu\nORCA-RELAY-SHORT-SOCKET-DIR /tmp/.orca-relay-1000\n' + `Welcome to Ubuntu\nORCA-RELAY-SHORT-SOCKET-DIR /tmp/.orca-relay-1000/${segment}\n`, + segment ) - ).toBe('/tmp/.orca-relay-1000') - expect(parseShortRelaySocketDir('mkdir: permission denied\n')).toBeNull() - expect(parseShortRelaySocketDir('ORCA-RELAY-SHORT-SOCKET-DIR /etc\n')).toBeNull() + ).toBe(`/tmp/.orca-relay-1000/${segment}`) + expect(parseShortRelaySocketDir('mkdir: permission denied\n', segment)).toBeNull() + expect( + parseShortRelaySocketDir(`ORCA-RELAY-SHORT-SOCKET-DIR /etc/${segment}\n`, segment) + ).toBeNull() + // A directory belonging to another build must not be adopted as this build's. + expect( + parseShortRelaySocketDir( + `ORCA-RELAY-SHORT-SOCKET-DIR /tmp/.orca-relay-1000/${shortRelayVersionSegment('relay-9.9.9+other')}\n`, + segment + ) + ).toBeNull() }) }) @@ -156,7 +172,9 @@ describe('relay launch with a long remote $HOME', () => { .mockResolvedValueOnce(LONG_HOME) .mockResolvedValueOnce('ORCA-NATIVE-DEPS-OK') .mockResolvedValueOnce('') // launch namespace marker - .mockResolvedValueOnce(`ORCA-RELAY-SHORT-SOCKET-DIR ${SHORT_RELAY_SOCKET_DIR_PREFIX}1000`) + .mockResolvedValueOnce( + `ORCA-RELAY-SHORT-SOCKET-DIR ${SHORT_RELAY_SOCKET_DIR_PREFIX}1000/${shortRelayVersionSegment(RELAY_VERSION_DIR_NAME)}` + ) .mockResolvedValueOnce('DEAD') .mockResolvedValueOnce('READY') .mockResolvedValue('') @@ -172,10 +190,32 @@ describe('relay launch with a long remote $HOME', () => { ) expect(sockPath.startsWith(`${SHORT_RELAY_SOCKET_DIR_PREFIX}1000/`)).toBe(true) expect(result.sockPath).toBe(sockPath) - // The hashed socket name survives intact, so two targets cannot collide. + // The hashed socket name survives intact, so two targets cannot collide -- and the + // build's version segment sits above it, so the next Orca release binds a path of + // its own instead of the one this relay is still holding. expect(sockPath).toBe( - `${SHORT_RELAY_SOCKET_DIR_PREFIX}1000/${relaySocketNameForInstanceId('ssh-target-1')}` + `${SHORT_RELAY_SOCKET_DIR_PREFIX}1000/${shortRelayVersionSegment(RELAY_VERSION_DIR_NAME)}/${relaySocketNameForInstanceId('ssh-target-1')}` ) + expect(shortRelayVersionSegment('relay-0.1.0+next')).not.toBe( + shortRelayVersionSegment(RELAY_VERSION_DIR_NAME) + ) + }) + + it('sweeps superseded relays under the short base too, but never the live one', () => { + const currentShortSocketDir = `${SHORT_RELAY_SOCKET_DIR_PREFIX}1000/${shortRelayVersionSegment(RELAY_VERSION_DIR_NAME)}` + const script = supersededRelayEndpointListCommand({ + remoteHome: LONG_HOME, + currentRelayDir: `${LONG_HOME}/.orca-remote/${RELAY_VERSION_DIR_NAME}`, + sockName: relaySocketNameForInstanceId('ssh-target-1'), + currentShortSocketDir + }) + + // A relocated orphan lives outside $HOME, so the sweep that exists to make orphans + // visible has to look at the short base as well. + expect(script).toContain(`short_base="${SHORT_RELAY_SOCKET_DIR_PREFIX}$(id -u 2>/dev/null)"`) + expect(script).toContain('"$short_base"/relay-*/"$sock_name"') + expect(script).toContain(`short_current='${currentShortSocketDir}'`) + expect(script).toContain('[ -n "$short_current" ] && [ "$dir" = "$short_current" ] && continue') }) it('leaves the socket in the versioned relay dir when it already fits', async () => { @@ -205,5 +245,6 @@ describe('relay launch with a long remote $HOME', () => { const script = vi.mocked(execCommand).mock.calls[0]?.[1] as string expect(script).toContain(`short_base="${SHORT_RELAY_SOCKET_DIR_PREFIX}$(id -u 2>/dev/null)"`) + expect(script).toContain('"$short_base"/relay-*/"$sock_name"') }) }) diff --git a/src/main/ssh/relay-socket-path-limit.ts b/src/main/ssh/relay-socket-path-limit.ts index 3cc2d6fbe67..417366e2bf6 100644 --- a/src/main/ssh/relay-socket-path-limit.ts +++ b/src/main/ssh/relay-socket-path-limit.ts @@ -9,6 +9,7 @@ * * Windows relays bind named pipes (`\\.\pipe\...`), which have no `sun_path` limit. */ +import { createHash } from 'node:crypto' import { isWindowsRemoteHost, type RemoteHostPlatform } from './ssh-remote-platform' /** @@ -37,19 +38,38 @@ export function shortRelaySocketDirForUid(uid: string): string { return `${SHORT_RELAY_SOCKET_DIR_PREFIX}${uid}` } +/** + * The version segment the relocated socket lives under, named to match the version + * directories in `$HOME/.orca-remote` so one sweep pattern covers both bases. + * + * Why it has to exist: `relaySocketNameForInstanceId` hashes the *target*, not the + * build, so the filename alone is version-independent. Under `$HOME` the enclosing + * `relay-` directory supplies that dimension; without it here, the next + * Orca build would bind the exact path the previous build's relay still holds. The + * daemon handshake compares build hashes exactly, so that meeting is a version + * mismatch — and if the incumbent holds live work, `resolveRelayEndpointBeforeRelaunch` + * raises `RelayEndpointHeldError` and the user cannot connect at all until the old + * relay is stopped. The version is hashed rather than spelled out because the whole + * point of this base is a bounded length. + */ +export function shortRelayVersionSegment(relayVersionDirName: string): string { + return `relay-${createHash('sha256').update(relayVersionDirName).digest('hex').slice(0, 12)}` +} + /** * The whole hashed socket name is kept — shortening happens by replacing the * variable-length directory, never by truncating the hash, so two targets on one * host can never land on the same socket. */ -export function shortRelaySocketPath(shortDir: string, sockName: string): string { - return `${shortDir}/${sockName}` +export function shortRelaySocketPath(shortVersionDir: string, sockName: string): string { + return `${shortVersionDir}/${sockName}` } const SHORT_DIR_MARKER = 'ORCA-RELAY-SHORT-SOCKET-DIR' /** - * Create (or adopt) the per-uid short socket directory and print it. + * Create (or adopt) the per-uid short socket directory and its version segment, and + * print the segment's path. * * Validate before mutating, never the other way round: an unconditional `chmod` follows a * symlink, so a path planted by another user would have its *target's* mode rewritten before @@ -58,35 +78,49 @@ const SHORT_DIR_MARKER = 'ORCA-RELAY-SHORT-SOCKET-DIR' * proves — via `ls -ldn`, which reports the entry itself rather than what it points at — that * it is a real directory, owned by this uid, already 0700. Nothing else is touched. */ -export function resolveShortRelaySocketDirCommand(): string { +export function resolveShortRelaySocketDirCommand(versionSegment: string): string { return [ 'uid=$(id -u) || exit 1', `dir="${SHORT_RELAY_SOCKET_DIR_PREFIX}$uid"`, 'umask 077', - 'if mkdir "$dir" 2>/dev/null; then', + ...adoptOwnedDirectoryCommand('$dir'), + // The version segment is validated the same way rather than trusted: `$dir` being + // 0700 and ours does not prove what an earlier run left inside it still is. + `ver="$dir/${versionSegment}"`, + ...adoptOwnedDirectoryCommand('$ver'), + `printf '%s %s\n' '${SHORT_DIR_MARKER}' "$ver"` + ].join('\n') +} + +function adoptOwnedDirectoryCommand(target: string): string[] { + return [ + `if mkdir "${target}" 2>/dev/null; then`, ' :', 'else', // Why the sub(): ls decorates the mode with a trailing marker for extended attributes (@), // ACLs (+) or an SELinux context (.), so an exact match would refuse a directory we own. - ' entry=$(ls -ldn "$dir" 2>/dev/null | awk \'NR==1{sub(/[.@+]$/, "", $1); print $1" "$3}\')', + ` entry=$(ls -ldn "${target}" 2>/dev/null | awk 'NR==1{sub(/[.@+]$/, "", $1); print $1" "$3}')`, ' case "$entry" in', ' "drwx------ $uid") ;;', ' *) exit 1 ;;', ' esac', - 'fi', - `printf '%s %s\\n' '${SHORT_DIR_MARKER}' "$dir"` - ].join('\n') + 'fi' + ] } /** Tolerates login-shell banner noise ahead of the marker line. */ -export function parseShortRelaySocketDir(output: string): string | null { +export function parseShortRelaySocketDir(output: string, versionSegment: string): string | null { for (const line of output.split('\n')) { const trimmed = line.trim() if (!trimmed.startsWith(`${SHORT_DIR_MARKER} `)) { continue } const dir = trimmed.slice(SHORT_DIR_MARKER.length + 1).trim() - if (dir.startsWith(`${SHORT_RELAY_SOCKET_DIR_PREFIX}`) && !/[\r\n]/.test(dir)) { + if ( + dir.startsWith(`${SHORT_RELAY_SOCKET_DIR_PREFIX}`) && + dir.endsWith(`/${versionSegment}`) && + !/[\r\n]/.test(dir) + ) { return dir } } diff --git a/src/main/ssh/sftp-stream-late-error.test.ts b/src/main/ssh/sftp-stream-late-error.test.ts index 6b3e04601da..4ed3bcf8b32 100644 --- a/src/main/ssh/sftp-stream-late-error.test.ts +++ b/src/main/ssh/sftp-stream-late-error.test.ts @@ -5,7 +5,7 @@ import { EventEmitter } from 'node:events' import { PassThrough } from 'node:stream' import type { SFTPWrapper } from 'ssh2' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { uploadBuffer, uploadFile, writeStringViaSftp } from './sftp-upload' +import { uploadBuffer, uploadFile, writeStringViaSftp, writeStringsViaSftp } from './sftp-upload' import { writeRelayFile } from './ssh-relay-install-transfers' import type { SshConnection } from './ssh-connection' import { getRemoteHostPlatform } from './ssh-remote-platform' @@ -66,6 +66,46 @@ describe('late SFTP stream errors', () => { expect(() => stream.emit('error', sftpNoSuchFileError())).not.toThrow() }) + // The failure mode a per-file loop reintroduces: writeStringViaSftp removes its own + // session listener at each settle, so a session that ran N transfers ends up with zero + // listeners while it is still open and still able to deliver a STATUS reply. + it('does not throw when a session error arrives after a multi-file write settles', async () => { + const sftp = Object.assign(new EventEmitter(), { + createWriteStream: () => { + const stream = new PassThrough() + stream.resume() + return stream + }, + end: () => {} + }) as unknown as SFTPWrapper + + await writeStringsViaSftp({ sftp: () => Promise.resolve(sftp) }, [ + { path: '/home/user/.local/bin/orca', contents: '#!/bin/sh\n' }, + { path: '/home/user/.local/bin/orca.mjs', contents: 'export {}\n' } + ]) + + expect(() => sftp.emit('error', sftpNoSuchFileError())).not.toThrow() + }) + + it('still rejects a multi-file write with a session error raised during it', async () => { + const sftp = Object.assign(new EventEmitter(), { + createWriteStream: () => { + const stream = new PassThrough() + queueMicrotask(() => sftp.emit('error', sftpNoSuchFileError())) + return stream + }, + end: () => {} + }) as unknown as SFTPWrapper + + // The latch must sit behind the transfer's own prepended listener, or a real + // mid-transfer failure would be swallowed into a hang. + await expect( + writeStringsViaSftp({ sftp: () => Promise.resolve(sftp) }, [ + { path: '/home/user/.local/bin/orca', contents: '#!/bin/sh\n' } + ]) + ).rejects.toThrow('file does not exist') + }) + it('still rejects with the SFTP error when it arrives during the transfer', async () => { const stream = new PassThrough() stream.resume() @@ -83,6 +123,28 @@ describe('late SFTP stream errors', () => { }) describe('sandboxed SFTP subsystem diagnosis', () => { + it('leaves a permission refusal as itself rather than blaming a chroot', async () => { + // SSH_FX_PERMISSION_DENIED is a mode/ownership refusal on a path the subsystem can + // see -- a read-only home, a root-owned parent, a quota. Rewriting it into "your + // bastion chroots SFTP" sends the user to fix ProxyJump for a chmod. + const conn = { + writeFile: () => Promise.reject(Object.assign(new Error('permission denied'), { code: 3 })) + } as unknown as SshConnection + + const failure: unknown = await writeRelayFile( + conn, + getRemoteHostPlatform('linux-x64'), + '/home/user/.orca-remote/relay-1/.version', + 'v1' + ).then( + () => null, + (err: unknown) => err + ) + + expect((failure as Error).message).toBe('permission denied') + expect(failure).not.toHaveProperty('sandboxedSftpNamespace') + }) + it('replaces the bare SFTP status with an actionable relay-install message', async () => { const conn = { writeFile: () => Promise.reject(sftpNoSuchFileError()) diff --git a/src/main/ssh/sftp-stream-late-error.ts b/src/main/ssh/sftp-stream-late-error.ts index 7241e3190df..1630668f874 100644 --- a/src/main/ssh/sftp-stream-late-error.ts +++ b/src/main/ssh/sftp-stream-late-error.ts @@ -18,14 +18,18 @@ * guarantee one level down, on the streams. */ -/** SSH_FX_* status codes ssh2 copies onto the `Error` it builds from a STATUS reply. */ +/** SSH_FX_* status code ssh2 copies onto the `Error` it builds from a STATUS reply. */ const SSH_FX_NO_SUCH_FILE = 2 -const SSH_FX_PERMISSION_DENIED = 3 type ErrorEmitter = { on(event: 'error', listener: (err: Error) => void): unknown } +type SftpSessionEmitter = ErrorEmitter & { + once(event: 'close', listener: () => void): unknown + removeListener(event: 'error', listener: (err: Error) => void): unknown +} + export type SftpStreamErrorLatch = { /** Call once the transfer has settled; any error after this point is the late one. */ markTransferSettled(): void @@ -52,9 +56,34 @@ export function latchLateSftpStreamErrors( } } +/** + * Hold one `'error'` listener on the SFTP *session* for as long as the session lives. + * + * A transfer that attaches and removes its own session listener — `writeStringViaSftp` + * does, so a session error can reject the write in flight — leaves the emitter with zero + * listeners between transfers and after the last one. A late STATUS reply arriving in + * that window is the synchronous throw described above. Errors during a transfer still + * reach that transfer first: it prepends its listener ahead of this one. + * + * Attach this once, right after `conn.sftp()`, on every path that runs transfers over a + * session it owns. + */ +export function latchLateSftpSessionErrors(sftp: SftpSessionEmitter): void { + const swallowLateSftpError = (): void => {} + sftp.on('error', swallowLateSftpError) + sftp.once('close', () => sftp.removeListener('error', swallowLateSftpError)) +} + +/** + * A chrooted SFTP subsystem answers a path outside its namespace with + * `SSH_FX_NO_SUCH_FILE`, because the path genuinely does not exist in the view it + * serves. `SSH_FX_PERMISSION_DENIED` is not that: it is an ordinary mode/ownership + * refusal on a path the subsystem *can* see — a read-only home, a root-owned parent, + * a quota — and rewriting it into "your bastion chroots SFTP" would send the user to + * fix ProxyJump for a `chmod`. + */ export function isSandboxedSftpNamespaceError(error: unknown): boolean { - const code = (error as { code?: unknown } | null)?.code - return code === SSH_FX_NO_SUCH_FILE || code === SSH_FX_PERMISSION_DENIED + return (error as { code?: unknown } | null)?.code === SSH_FX_NO_SUCH_FILE } /** diff --git a/src/main/ssh/sftp-upload.ts b/src/main/ssh/sftp-upload.ts index a28ce707105..514df81ed1d 100644 --- a/src/main/ssh/sftp-upload.ts +++ b/src/main/ssh/sftp-upload.ts @@ -4,7 +4,11 @@ import { lstat, open, readdir, realpath } from 'node:fs/promises' import { isAbsolute, join as pathJoin, relative, sep } from 'node:path' import { finished } from 'node:stream/promises' import type { SFTPWrapper } from 'ssh2' -import { latchLateSftpStreamErrors, type SftpStreamErrorLatch } from './sftp-stream-late-error' +import { + latchLateSftpSessionErrors, + latchLateSftpStreamErrors, + type SftpStreamErrorLatch +} from './sftp-stream-late-error' export function mkdirSftp( sftp: SFTPWrapper, @@ -187,6 +191,29 @@ export function writeStringViaSftp( }) } +/** + * Write several files over one SFTP session, ending it when they are all done. + * + * Owns the session's late-error latch, which is why a caller must not hand-roll this + * loop: `writeStringViaSftp` drops its own session listener at each settle, so between + * files and after the last one the emitter would carry none, and a late STATUS reply + * throws synchronously out of ssh2's parser into main (#15479). + */ +export async function writeStringsViaSftp( + conn: { sftp(): Promise }, + files: readonly { path: string; contents: string }[] +): Promise { + const sftp = await conn.sftp() + latchLateSftpSessionErrors(sftp) + try { + for (const file of files) { + await writeStringViaSftp(sftp, file.path, file.contents) + } + } finally { + sftp.end() + } +} + export async function uploadDirectory( sftp: SFTPWrapper, localDir: string, diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index 99890503267..af7346f5d35 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -89,7 +89,9 @@ import { parseShortRelaySocketDir, remoteSocketPathFitsLimit, resolveShortRelaySocketDirCommand, - shortRelaySocketPath + shortRelaySocketPath, + shortRelayVersionSegment, + SHORT_RELAY_SOCKET_DIR_PREFIX } from './relay-socket-path-limit' import { isSshSessionLimitError } from './ssh-session-limit-error' import { @@ -604,6 +606,13 @@ async function deployAndLaunchRelayAttempt( remoteHome, currentRelayDir: remoteRelayDir, sockName: relaySocketNameForInstanceId(relayInstanceId), + // Set only when this launch relocated past sun_path; the sweep must not reap + // the socket the transport it just handed back is talking to. + ...(launched.sockPath.startsWith(SHORT_RELAY_SOCKET_DIR_PREFIX) + ? { + currentShortSocketDir: launched.sockPath.slice(0, launched.sockPath.lastIndexOf('/')) + } + : {}), nodePath: launched.nodePath }) ) @@ -1648,7 +1657,7 @@ async function launchRelay( // Why: a long remote $HOME pushes the default endpoint past sun_path and bind fails with a bare `listen EINVAL` (#10726). const sockFile = remoteSocketPathFitsLimit(hostPlatform, defaultSockFile) ? defaultSockFile - : await resolveShortPosixRelaySocketPath(conn, sockName, defaultSockFile, signal) + : await resolveShortPosixRelaySocketPath(conn, remoteDir, sockName, defaultSockFile, signal) if (isWindowsRemoteHost(hostPlatform)) { const activePipeMarkerPath = windowsActivePipeMarkerPath(hostPlatform, remoteDir, sockName) @@ -1799,23 +1808,27 @@ async function launchRelay( * * The hashed socket name is preserved in full: only the directory shrinks, so the * short form stays deterministic per target and cannot collide with another target. + * The version directory's identity comes along as a hashed segment, so a later build + * still binds a path of its own rather than the one its predecessor is holding. */ async function resolveShortPosixRelaySocketPath( conn: SshConnection, + remoteDir: string, sockName: string, defaultSockFile: string, signal?: AbortSignal ): Promise { - const output = await execCommand(conn, resolveShortRelaySocketDirCommand(), { signal }).catch( - (err: unknown) => { - if (isUnconfirmedSshCommandTermination(err)) { - throw err - } - signal?.throwIfAborted() - return '' + const versionSegment = shortRelayVersionSegment(remoteDir.slice(remoteDir.lastIndexOf('/') + 1)) + const output = await execCommand(conn, resolveShortRelaySocketDirCommand(versionSegment), { + signal + }).catch((err: unknown) => { + if (isUnconfirmedSshCommandTermination(err)) { + throw err } - ) - const shortDir = parseShortRelaySocketDir(output) + signal?.throwIfAborted() + return '' + }) + const shortDir = parseShortRelaySocketDir(output, versionSegment) if (!shortDir) { throw new Error( `Relay socket path ${defaultSockFile} exceeds the remote Unix socket limit and no short socket directory could be created on the host.` diff --git a/src/main/ssh/ssh-relay-install-transfers.ts b/src/main/ssh/ssh-relay-install-transfers.ts index 7ca1dd35a82..6f8ede57984 100644 --- a/src/main/ssh/ssh-relay-install-transfers.ts +++ b/src/main/ssh/ssh-relay-install-transfers.ts @@ -15,7 +15,8 @@ import { import type { RemoteHostPlatform } from './ssh-remote-platform' import { describeSandboxedSftpFailure, - isSandboxedSftpNamespaceError + isSandboxedSftpNamespaceError, + latchLateSftpSessionErrors } from './sftp-stream-late-error' export type RelayTransferOptions = { @@ -105,7 +106,6 @@ async function runSftpFallbackTransfer( transfer: (sftp: SFTPWrapper) => Promise ): Promise { const sftp = await conn.sftp(options?.signal) - const swallowLateSftpError = (): void => {} let sftpEndRequested = false const endSftp = (): void => { if (!sftpEndRequested) { @@ -113,9 +113,7 @@ async function runSftpFallbackTransfer( sftp.end() } } - // A late session 'error' after settle would otherwise be unhandled and crash main. - sftp.on('error', swallowLateSftpError) - sftp.once('close', () => sftp.removeListener('error', swallowLateSftpError)) + latchLateSftpSessionErrors(sftp) try { await raceSftpFileTransferWithAbort( transfer(sftp), diff --git a/src/main/ssh/ssh-relay-reset.ts b/src/main/ssh/ssh-relay-reset.ts index 570687486bf..3d585b525f2 100644 --- a/src/main/ssh/ssh-relay-reset.ts +++ b/src/main/ssh/ssh-relay-reset.ts @@ -16,7 +16,7 @@ export async function forceStopRelayForTarget( // Why: a long $HOME moves the socket to the sun_path-safe short base (#10726); reset must reach it there too. `short_base="${SHORT_RELAY_SOCKET_DIR_PREFIX}$(id -u 2>/dev/null)"`, 'if [ -d "$base" ] || [ -d "$short_base" ]; then', - ' for sock in "$base"/relay-*/"$sock_name" "$base"/"$sock_name" "$short_base"/"$sock_name"; do', + ' for sock in "$base"/relay-*/"$sock_name" "$base"/"$sock_name" "$short_base"/relay-*/"$sock_name"; do', ' [ -S "$sock" ] || continue', ' pid=""', // Why: lsof ORs selectors by default; -a prevents reset from targeting diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index a8a87a222f2..82652407862 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -5,7 +5,7 @@ import { randomUUID } from 'node:crypto' import type { BrowserWindow } from 'electron' import { deployAndLaunchRelay } from './ssh-relay-deploy' import { execCommand } from './ssh-relay-deploy-helpers' -import { writeStringViaSftp } from './sftp-upload' +import { writeStringsViaSftp } from './sftp-upload' import { isRelayVersionMismatchError } from './ssh-relay-version-mismatch-error' import { isRelayEndpointHeldError } from './ssh-relay-endpoint-incumbent' import { forgetRelayNodePtyRepairs, recoverRelayNodePtyForSpawn } from './ssh-relay-node-pty-repair' @@ -1414,14 +1414,7 @@ export class SshRelaySession { await conn.writeFile(file.path, file.contents, { hostPlatform }) } } else { - const sftp = await conn.sftp() - try { - for (const file of plan.files) { - await writeStringViaSftp(sftp, file.path, file.contents) - } - } finally { - sftp.end() - } + await writeStringsViaSftp(conn, plan.files) } for (const command of plan.postWriteCommands) { await execCommand(conn, command, { wrapCommand: !isWindowsRemoteHost(hostPlatform) }) diff --git a/src/main/ssh/ssh-relay-superseded-endpoints.ts b/src/main/ssh/ssh-relay-superseded-endpoints.ts index 1a9554f7b82..4b1ad5637ef 100644 --- a/src/main/ssh/ssh-relay-superseded-endpoints.ts +++ b/src/main/ssh/ssh-relay-superseded-endpoints.ts @@ -19,6 +19,7 @@ import type { SshConnection } from './ssh-connection' import { shellEscape } from './ssh-connection-utils' import { RELAY_REMOTE_DIR } from './relay-protocol' +import { SHORT_RELAY_SOCKET_DIR_PREFIX } from './relay-socket-path-limit' import { execCommand } from './ssh-relay-deploy-helpers' import { describeRelayEndpointIncumbent, @@ -53,6 +54,8 @@ export type SupersededRelaySweepOptions = { currentRelayDir: string /** Stable per-target socket filename, from `relaySocketNameForInstanceId`. */ sockName: string + /** Set only when this launch relocated its socket; that directory is never swept. */ + currentShortSocketDir?: string nodePath: string signal?: AbortSignal } @@ -63,15 +66,23 @@ export function supersededRelayEndpointListCommand(options: { remoteHome: string currentRelayDir: string sockName: string + currentShortSocketDir?: string }): string { return [ `base=${shellEscape(`${options.remoteHome}/${RELAY_REMOTE_DIR}`)}`, `sock_name=${shellEscape(options.sockName)}`, `current=${shellEscape(options.currentRelayDir)}`, - 'for sock in "$base"/relay-*/"$sock_name"; do', + // Why the second base: a host whose `$HOME` pushes the endpoint past `sun_path` binds + // under `/tmp/.orca-relay-/relay-/` instead (relay-socket-path-limit.ts). + // Those orphans are the same population this sweep exists to make visible, and the + // `$HOME` glob cannot see them. The uid is resolved on the host; the client never knows it. + `short_current=${shellEscape(options.currentShortSocketDir ?? '')}`, + `short_base="${SHORT_RELAY_SOCKET_DIR_PREFIX}$(id -u 2>/dev/null)"`, + 'for sock in "$base"/relay-*/"$sock_name" "$short_base"/relay-*/"$sock_name"; do', ' [ -S "$sock" ] || continue', ' dir=${sock%/*}', ' [ "$dir" = "$current" ] && continue', + ' [ -n "$short_current" ] && [ "$dir" = "$short_current" ] && continue', ' printf \'%s\\n\' "$sock"', 'done' ].join('\n') @@ -79,7 +90,14 @@ export function supersededRelayEndpointListCommand(options: { /** Remove a socket inode proven to have no holder, so version-dir GC can reclaim the tree. */ export function removeStaleRelayEndpointCommand(sockPath: string): string { - return `rm -f ${shellEscape(sockPath)}` + const remove = `rm -f ${shellEscape(sockPath)}` + if (!sockPath.startsWith(SHORT_RELAY_SOCKET_DIR_PREFIX)) { + return remove + } + // `gcOldRelayVersions` only walks `$HOME/.orca-remote`, so nothing else would ever + // reclaim a relocated version segment. `rmdir` fails while another target of the same + // build still has a socket there, which is exactly the condition for keeping it. + return `${remove}; rmdir ${shellEscape(sockPath.slice(0, sockPath.lastIndexOf('/')))} 2>/dev/null || true` } export function classifySupersededRelay(