diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index 05ed38014..44ac8e336 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -1,8 +1,6 @@ import { spawn } from "child_process"; import { Readable, Writable } from "stream"; import * as path from "path"; -import * as os from "os"; -import * as crypto from "crypto"; import { Shescape } from "shescape"; import type { Runtime, @@ -24,6 +22,7 @@ import { expandTildeForSSH, cdCommandForSSH } from "./tildeExpansion"; import { getProjectName } from "../utils/runtime/helpers"; import { getErrorMessage } from "../utils/errors"; import { execAsync } from "../utils/disposableExec"; +import { getControlPath } from "./sshConnectionPool"; /** * Shescape instance for bash shell escaping. @@ -61,10 +60,10 @@ export class SSHRuntime implements Runtime { constructor(config: SSHRuntimeConfig) { this.config = config; - // Generate unique control path for SSH connection multiplexing - // This allows multiple SSH sessions to reuse a single TCP connection - const randomId = crypto.randomBytes(8).toString("hex"); - this.controlPath = path.join(os.tmpdir(), `cmux-ssh-${randomId}`); + // Get deterministic controlPath from connection pool + // Multiple SSHRuntime instances with same config share the same controlPath, + // enabling ControlMaster to multiplex SSH connections across operations + this.controlPath = getControlPath(config); } /** @@ -372,6 +371,12 @@ export class SSHRuntime implements Runtime { args.push("-o", "LogLevel=ERROR"); } + // Add ControlMaster options for connection multiplexing + // This ensures git bundle transfers also reuse the master connection + args.push("-o", "ControlMaster=auto"); + args.push("-o", `ControlPath=${this.controlPath}`); + args.push("-o", "ControlPersist=60"); + if (includeHost) { args.push(this.config.host); } @@ -890,27 +895,6 @@ export class SSHRuntime implements Runtime { return { success: false, error: `Failed to delete directory: ${getErrorMessage(error)}` }; } } - - /** - * Cleanup SSH control socket on disposal - * Note: ControlPersist will automatically close the master connection after timeout, - * but we try to clean up immediately for good hygiene - */ - dispose(): void { - try { - // Send exit command to master connection (if it exists) - // This is a best-effort cleanup - the socket will auto-cleanup anyway - const exitArgs = ["-O", "exit", "-o", `ControlPath=${this.controlPath}`, this.config.host]; - - const exitProc = spawn("ssh", exitArgs, { stdio: "ignore" }); - - // Don't wait for it - fire and forget - exitProc.unref(); - } catch (error) { - // Ignore errors - control socket will timeout naturally - log.debug(`SSH control socket cleanup failed (non-fatal): ${getErrorMessage(error)}`); - } - } } /** diff --git a/src/runtime/sshConnectionPool.test.ts b/src/runtime/sshConnectionPool.test.ts new file mode 100644 index 000000000..811715084 --- /dev/null +++ b/src/runtime/sshConnectionPool.test.ts @@ -0,0 +1,132 @@ +import * as os from "os"; +import { getControlPath } from "./sshConnectionPool"; +import type { SSHRuntimeConfig } from "./SSHRuntime"; + +describe("sshConnectionPool", () => { + describe("getControlPath", () => { + test("identical configs produce same controlPath", () => { + const config: SSHRuntimeConfig = { + host: "test.example.com", + srcBaseDir: "/work", + }; + const path1 = getControlPath(config); + const path2 = getControlPath(config); + + expect(path1).toBe(path2); + }); + + test("different hosts produce different controlPaths", () => { + const path1 = getControlPath({ + host: "host1.example.com", + srcBaseDir: "/work", + }); + const path2 = getControlPath({ + host: "host2.example.com", + srcBaseDir: "/work", + }); + + expect(path1).not.toBe(path2); + }); + + test("different ports produce different controlPaths", () => { + const config1: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + port: 22, + }; + const config2: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + port: 2222, + }; + + expect(getControlPath(config1)).not.toBe(getControlPath(config2)); + }); + + test("different identityFiles produce different controlPaths", () => { + const config1: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + identityFile: "/path/to/key1", + }; + const config2: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + identityFile: "/path/to/key2", + }; + + expect(getControlPath(config1)).not.toBe(getControlPath(config2)); + }); + + test("different srcBaseDirs produce different controlPaths", () => { + const config1: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work1", + }; + const config2: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work2", + }; + + expect(getControlPath(config1)).not.toBe(getControlPath(config2)); + }); + + test("controlPath is in tmpdir with expected format", () => { + const config: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + }; + const controlPath = getControlPath(config); + + expect(controlPath).toContain(os.tmpdir()); + expect(controlPath).toMatch(/cmux-ssh-[a-f0-9]{12}$/); + }); + + test("missing port defaults to 22 in hash calculation", () => { + const config1: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + port: 22, + }; + const config2: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + // port omitted, should default to 22 + }; + + expect(getControlPath(config1)).toBe(getControlPath(config2)); + }); + + test("missing identityFile defaults to 'default' in hash calculation", () => { + const config1: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + identityFile: undefined, + }; + const config2: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + // identityFile omitted + }; + + expect(getControlPath(config1)).toBe(getControlPath(config2)); + }); + }); +}); + +describe("username isolation", () => { + test("controlPath includes local username to prevent cross-user collisions", () => { + // This test verifies that os.userInfo().username is included in the hash + // On multi-user systems, different users connecting to the same remote + // would get different controlPaths, preventing permission errors + const config: SSHRuntimeConfig = { + host: "test.com", + srcBaseDir: "/work", + }; + const controlPath = getControlPath(config); + + // The path should be deterministic for this user + expect(controlPath).toBe(getControlPath(config)); + expect(controlPath).toMatch(/^\/tmp\/cmux-ssh-[a-f0-9]{12}$/); + }); +}); diff --git a/src/runtime/sshConnectionPool.ts b/src/runtime/sshConnectionPool.ts new file mode 100644 index 000000000..7e3bdf2f5 --- /dev/null +++ b/src/runtime/sshConnectionPool.ts @@ -0,0 +1,59 @@ +/** + * SSH Connection Pool - Stateless + * + * Generates deterministic ControlPath from SSH config to enable connection + * multiplexing across SSHRuntime instances targeting the same host. + * + * Design: + * - Pure function: same config → same controlPath + * - No state: filesystem is the state + * - No cleanup: ControlPersist + OS handle it + */ + +import * as crypto from "crypto"; +import * as path from "path"; +import * as os from "os"; +import type { SSHRuntimeConfig } from "./SSHRuntime"; + +/** + * Get deterministic controlPath for SSH config. + * Multiple calls with identical config return the same path, + * enabling ControlMaster to multiplex connections. + * + * Socket files are created by SSH and cleaned up automatically: + * - ControlPersist=60: Removes socket 60s after last use + * - OS: Cleans /tmp on reboot + * + * Includes local username in hash to prevent cross-user collisions on + * multi-user systems (different users connecting to same remote would + * otherwise generate same socket path, causing permission errors). + */ +export function getControlPath(config: SSHRuntimeConfig): string { + const key = makeConnectionKey(config); + const hash = hashKey(key); + return path.join(os.tmpdir(), `cmux-ssh-${hash}`); +} + +/** + * Generate stable key from config. + * Identical configs produce identical keys. + * Includes local username to prevent cross-user socket collisions. + */ +function makeConnectionKey(config: SSHRuntimeConfig): string { + const parts = [ + os.userInfo().username, // Include local user to prevent cross-user collisions + config.host, + config.port?.toString() ?? "22", + config.srcBaseDir, + config.identityFile ?? "default", + ]; + return parts.join(":"); +} + +/** + * Generate deterministic hash for controlPath naming. + * Uses first 12 chars of SHA-256 for human-readable uniqueness. + */ +function hashKey(key: string): string { + return crypto.createHash("sha256").update(key).digest("hex").substring(0, 12); +}