diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessRequest.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessRequest.swift index 0f218e18bd45..ee08e8e1f9b8 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessRequest.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteProcessRequest.swift @@ -1,8 +1,8 @@ public import Foundation /// One subprocess invocation the coordinator hands to its process runner: -/// the executable, argv, optional environment/working directory/stdin, and -/// the timeout after which the process is terminated. +/// the executable, argv, optional environment/working directory/standard +/// input, and the timeout after which the process is terminated. public struct RemoteProcessRequest: Sendable { /// Absolute path of the executable to launch. public let executable: String @@ -15,6 +15,8 @@ public struct RemoteProcessRequest: Sendable { /// Data written to stdin (the write end is closed afterwards), or `nil` /// to attach the null device. public let stdin: Data? + /// Local file streamed to stdin, or `nil` for data/null-device input. + public let stdinFile: URL? /// Seconds after which a still-running process is terminated and the run /// fails with the legacy timeout error. public let timeout: TimeInterval @@ -33,6 +35,33 @@ public struct RemoteProcessRequest: Sendable { self.environment = environment self.currentDirectory = currentDirectory self.stdin = stdin + self.stdinFile = nil + self.timeout = timeout + } + + /// Creates a request whose standard input is streamed from a local file. + /// + /// - Parameters: + /// - executable: Absolute path of the executable to launch. + /// - arguments: Argument vector excluding the executable. + /// - environment: Process environment, or `nil` to inherit. + /// - currentDirectory: Working directory, or `nil` to inherit. + /// - stdinFile: Local file whose bytes become process standard input. + /// - timeout: Seconds after which a still-running process is terminated. + public init( + executable: String, + arguments: [String], + environment: [String: String]? = nil, + currentDirectory: URL? = nil, + stdinFile: URL, + timeout: TimeInterval + ) { + self.executable = executable + self.arguments = arguments + self.environment = environment + self.currentDirectory = currentDirectory + self.stdin = nil + self.stdinFile = stdinFile self.timeout = timeout } } diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift index 870f2dcd59ed..05b46faf9129 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Process/RemoteSessionProcessRunner.swift @@ -60,6 +60,13 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { let arguments = request.arguments let timeout = request.timeout let stdin = request.stdin + let stdinFileHandle: FileHandle? + if let stdinFile = request.stdinFile { + stdinFileHandle = try FileHandle(forReadingFrom: stdinFile) + } else { + stdinFileHandle = nil + } + defer { try? stdinFileHandle?.close() } debugLog( "remote.proc.start exec=\(URL(fileURLWithPath: executable).lastPathComponent) " + @@ -80,7 +87,9 @@ public struct RemoteSessionProcessRunner: RemoteSessionProcessRunning { process.standardOutput = stdoutPipe process.standardError = stderrPipe - if stdin != nil { + if let stdinFileHandle { + process.standardInput = stdinFileHandle + } else if stdin != nil { process.standardInput = Pipe() } else { process.standardInput = FileHandle.nullDevice diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+Bootstrap.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+Bootstrap.swift index aadbb8d9fc69..66073a91fad5 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+Bootstrap.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+Bootstrap.swift @@ -302,63 +302,6 @@ extension RemoteSessionCoordinator { return output } - func uploadRemoteDaemonBinaryLocked(localBinary: URL, location: RemoteDaemonInstallLocation) throws { - let remotePath = location.absolutePath - let remoteDirectory = location.directory - let remoteTempPath = "\(remotePath).tmp-\(UUID().uuidString.prefix(8))" - debugLog( - "remote.upload.begin local=\(localBinary.path) remoteTemp=\(remoteTempPath) remote=\(remotePath)" - ) - - let mkdirScript = "mkdir -p \(remoteDirectory.shellSingleQuoted)" - let mkdirCommand = "sh -c \(mkdirScript.shellSingleQuoted)" - let mkdirResult = try sshExec(arguments: sshCommonArguments(batchMode: true) + [configuration.destination, mkdirCommand], timeout: 12) - guard mkdirResult.status == 0 else { - let detail = Self.bestErrorLine(stderr: mkdirResult.stderr, stdout: mkdirResult.stdout) ?? "ssh exited \(mkdirResult.status)" - throw NSError(domain: "cmux.remote.daemon", code: 30, userInfo: [ - NSLocalizedDescriptionKey: "failed to create remote daemon directory: \(detail)", - ]) - } - - let scpSSHOptions = backgroundSSHOptions(configuration.sshOptions) - var scpArgs: [String] = ["-q"] - if !hasSSHOptionKey(scpSSHOptions, key: "StrictHostKeyChecking") { - scpArgs += ["-o", "StrictHostKeyChecking=accept-new"] - } - scpArgs += ["-o", "ControlMaster=no"] - if let port = configuration.port { - scpArgs += ["-P", String(port)] - } - if let identityFile = configuration.identityFile, - !identityFile.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { - scpArgs += ["-i", identityFile] - } - for option in scpSSHOptions { - scpArgs += ["-o", option] - } - scpArgs += [localBinary.path, "\(configuration.destination):\(remoteTempPath)"] - let scpResult = try scpExec(arguments: scpArgs, timeout: 45) - guard scpResult.status == 0 else { - let detail = Self.bestErrorLine(stderr: scpResult.stderr, stdout: scpResult.stdout) ?? "scp exited \(scpResult.status)" - throw NSError(domain: "cmux.remote.daemon", code: 31, userInfo: [ - NSLocalizedDescriptionKey: "failed to upload cmuxd-remote: \(detail)", - ]) - } - - let finalizeScript = """ - chmod 755 \(remoteTempPath.shellSingleQuoted) && \ - mv \(remoteTempPath.shellSingleQuoted) \(remotePath.shellSingleQuoted) - """ - let finalizeCommand = "sh -c \(finalizeScript.shellSingleQuoted)" - let finalizeResult = try sshExec(arguments: sshCommonArguments(batchMode: true) + [configuration.destination, finalizeCommand], timeout: 12) - guard finalizeResult.status == 0 else { - let detail = Self.bestErrorLine(stderr: finalizeResult.stderr, stdout: finalizeResult.stdout) ?? "ssh exited \(finalizeResult.status)" - throw NSError(domain: "cmux.remote.daemon", code: 32, userInfo: [ - NSLocalizedDescriptionKey: "failed to install remote daemon binary: \(detail)", - ]) - } - } - func helloRemoteDaemonLocked(remotePath: String) throws -> DaemonHello { let request = #"{"id":1,"method":"hello","params":{}}"# let script = "printf '%s\\n' \(request.shellSingleQuoted) | \(remotePath.shellSingleQuoted) serve --stdio" diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+DaemonUpload.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+DaemonUpload.swift new file mode 100644 index 000000000000..43ba4e73d042 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+DaemonUpload.swift @@ -0,0 +1,107 @@ +internal import CmuxFoundation +internal import Foundation + +// Installs cmuxd-remote through the same SSH exec channel used by bootstrap. +// No SFTP subsystem or remote scp executable is required: the local binary is +// streamed to `cat`, then the existing chmod-and-rename step publishes it +// atomically at the versioned destination. +extension RemoteSessionCoordinator { + func uploadRemoteDaemonBinaryLocked(localBinary: URL, location: RemoteDaemonInstallLocation) throws { + let remotePath = location.absolutePath + let remoteDirectory = location.directory + let remoteTempPath = "\(remotePath).tmp-\(UUID().uuidString.prefix(8))" + debugLog( + "remote.upload.begin transport=ssh-stdin local=\(localBinary.path) " + + "remoteTemp=\(remoteTempPath) remote=\(remotePath)" + ) + + let mkdirScript = "mkdir -p \(remoteDirectory.shellSingleQuoted)" + let mkdirCommand = "sh -c \(mkdirScript.shellSingleQuoted)" + let mkdirResult: RemoteCommandResult + do { + mkdirResult = try sshExec( + arguments: sshCommonArguments(batchMode: true) + [configuration.destination, mkdirCommand], + timeout: 12 + ) + } catch { + throw NSError(domain: "cmux.remote.daemon", code: 30, userInfo: [ + NSLocalizedDescriptionKey: String( + localized: "remoteDaemon.upload.createDirectoryFailed", + defaultValue: "failed to create remote daemon directory" + ), + ]) + } + guard mkdirResult.status == 0 else { + let detail = Self.bestErrorLine(stderr: mkdirResult.stderr, stdout: mkdirResult.stdout) ?? + "ssh exited \(mkdirResult.status)" + throw NSError(domain: "cmux.remote.daemon", code: 30, userInfo: [ + NSLocalizedDescriptionKey: String( + localized: "remoteDaemon.upload.createDirectoryFailedWithDetail", + defaultValue: "failed to create remote daemon directory: \(detail)" + ), + ]) + } + + let uploadScript = "cat > \(remoteTempPath.shellSingleQuoted)" + let uploadCommand = "sh -c \(uploadScript.shellSingleQuoted)" + let uploadResult: RemoteCommandResult + do { + uploadResult = try sshExec( + arguments: sshCommonArguments(batchMode: true) + [configuration.destination, uploadCommand], + stdinFile: localBinary, + timeout: 45 + ) + } catch { + cleanupUploadedRemotePaths([remoteTempPath]) + throw NSError(domain: "cmux.remote.daemon", code: 31, userInfo: [ + NSLocalizedDescriptionKey: String( + localized: "remoteDaemon.upload.transferFailed", + defaultValue: "failed to upload cmuxd-remote" + ), + ]) + } + guard uploadResult.status == 0 else { + cleanupUploadedRemotePaths([remoteTempPath]) + let detail = Self.bestErrorLine(stderr: uploadResult.stderr, stdout: uploadResult.stdout) ?? + "ssh exited \(uploadResult.status)" + throw NSError(domain: "cmux.remote.daemon", code: 31, userInfo: [ + NSLocalizedDescriptionKey: String( + localized: "remoteDaemon.upload.transferFailedWithDetail", + defaultValue: "failed to upload cmuxd-remote: \(detail)" + ), + ]) + } + + let finalizeScript = """ + chmod 755 \(remoteTempPath.shellSingleQuoted) && \ + mv \(remoteTempPath.shellSingleQuoted) \(remotePath.shellSingleQuoted) + """ + let finalizeCommand = "sh -c \(finalizeScript.shellSingleQuoted)" + let finalizeResult: RemoteCommandResult + do { + finalizeResult = try sshExec( + arguments: sshCommonArguments(batchMode: true) + [configuration.destination, finalizeCommand], + timeout: 12 + ) + } catch { + cleanupUploadedRemotePaths([remoteTempPath]) + throw NSError(domain: "cmux.remote.daemon", code: 32, userInfo: [ + NSLocalizedDescriptionKey: String( + localized: "remoteDaemon.upload.installFailed", + defaultValue: "failed to install remote daemon binary" + ), + ]) + } + guard finalizeResult.status == 0 else { + cleanupUploadedRemotePaths([remoteTempPath]) + let detail = Self.bestErrorLine(stderr: finalizeResult.stderr, stdout: finalizeResult.stdout) ?? + "ssh exited \(finalizeResult.status)" + throw NSError(domain: "cmux.remote.daemon", code: 32, userInfo: [ + NSLocalizedDescriptionKey: String( + localized: "remoteDaemon.upload.installFailedWithDetail", + defaultValue: "failed to install remote daemon binary: \(detail)" + ), + ]) + } + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+ProcessExecution.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+ProcessExecution.swift new file mode 100644 index 000000000000..39c7baf5dd49 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+ProcessExecution.swift @@ -0,0 +1,78 @@ +internal import Foundation + +// Blocking SSH/SCP/dev-build execution through the injected process runner. +// Calls stay on the coordinator's serial utility queue; file-backed stdin lets +// large helper binaries flow through SSH without buffering them in memory. +extension RemoteSessionCoordinator { + func sshExec( + arguments: [String], + stdin: Data? = nil, + timeout: TimeInterval = 15 + ) throws -> RemoteCommandResult { + try runProcess( + executable: "/usr/bin/ssh", + arguments: arguments, + environment: configuration.sshProcessEnvironment, + stdin: stdin, + timeout: timeout + ) + } + + func sshExec( + arguments: [String], + stdinFile: URL, + timeout: TimeInterval = 15 + ) throws -> RemoteCommandResult { + // A host or caller can configure StdinNull=yes; OpenSSH would then + // discard this file while `cat` still exits successfully. Its first + // option value wins, so pin file-backed execs before caller options. + let fileInputArguments = ["-o", "StdinNull=no"] + arguments + return try processRunner.run( + RemoteProcessRequest( + executable: "/usr/bin/ssh", + arguments: fileInputArguments, + environment: configuration.sshProcessEnvironment, + stdinFile: stdinFile, + timeout: timeout + ), + operation: nil + ) + } + + func scpExec( + arguments: [String], + timeout: TimeInterval = 30, + operation: (any RemoteTransferCancelling)? = nil + ) throws -> RemoteCommandResult { + try runProcess( + executable: "/usr/bin/scp", + arguments: arguments, + environment: configuration.sshProcessEnvironment, + stdin: nil, + timeout: timeout, + operation: operation + ) + } + + func runProcess( + executable: String, + arguments: [String], + environment: [String: String]? = nil, + currentDirectory: URL? = nil, + stdin: Data?, + timeout: TimeInterval, + operation: (any RemoteTransferCancelling)? = nil + ) throws -> RemoteCommandResult { + try processRunner.run( + RemoteProcessRequest( + executable: executable, + arguments: arguments, + environment: environment, + currentDirectory: currentDirectory, + stdin: stdin, + timeout: timeout + ), + operation: operation + ) + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator.swift index ff5b55b0ab12..a778dff079d1 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator.swift @@ -542,55 +542,6 @@ public final class RemoteSessionCoordinator: @unchecked Sendable { return message.isEmpty ? "remote daemon bootstrap failed" : message } - // MARK: - Subprocess execution (through the runner seam) - - func sshExec(arguments: [String], stdin: Data? = nil, timeout: TimeInterval = 15) throws -> RemoteCommandResult { - try runProcess( - executable: "/usr/bin/ssh", - arguments: arguments, - environment: configuration.sshProcessEnvironment, - stdin: stdin, - timeout: timeout - ) - } - - func scpExec( - arguments: [String], - timeout: TimeInterval = 30, - operation: (any RemoteTransferCancelling)? = nil - ) throws -> RemoteCommandResult { - try runProcess( - executable: "/usr/bin/scp", - arguments: arguments, - environment: configuration.sshProcessEnvironment, - stdin: nil, - timeout: timeout, - operation: operation - ) - } - - func runProcess( - executable: String, - arguments: [String], - environment: [String: String]? = nil, - currentDirectory: URL? = nil, - stdin: Data?, - timeout: TimeInterval, - operation: (any RemoteTransferCancelling)? = nil - ) throws -> RemoteCommandResult { - try processRunner.run( - RemoteProcessRequest( - executable: executable, - arguments: arguments, - environment: environment, - currentDirectory: currentDirectory, - stdin: stdin, - timeout: timeout - ), - operation: operation - ) - } - // MARK: - Debug logging func debugLog(_ message: @autoclosure () -> String) { diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteDaemonUploadStep.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteDaemonUploadStep.swift new file mode 100644 index 000000000000..ddd269422997 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteDaemonUploadStep.swift @@ -0,0 +1,7 @@ +enum RemoteDaemonUploadStep: Equatable { + case createDirectory + case upload + case finalize + case cleanup + case unknown +} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteDaemonUploadTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteDaemonUploadTests.swift new file mode 100644 index 000000000000..82efbe70f468 --- /dev/null +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteDaemonUploadTests.swift @@ -0,0 +1,377 @@ +import CmuxCore +import CmuxRemoteDaemon +import CmuxRemoteWorkspace +import Foundation +import Testing +@testable import CmuxRemoteSession + +@Suite("Remote daemon upload") +struct RemoteDaemonUploadTests { + @Test("Upload succeeds through SSH exec when SCP's SFTP transport is unavailable") + func uploadSucceedsWithoutSFTP() throws { + let fileManager = FileManager.default + let root = fileManager.temporaryDirectory.appendingPathComponent( + "cmux-remote-daemon-upload-\(UUID().uuidString)", + isDirectory: true + ) + try fileManager.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? fileManager.removeItem(at: root) } + + let localBinary = root.appendingPathComponent("cmuxd-remote", isDirectory: false) + try Data("fake daemon".utf8).write(to: localBinary) + + let runner = RecordingProcessRunner { request in + if request.executable == "/usr/bin/scp" { + return RemoteCommandResult( + status: 1, + stdout: "", + stderr: "subsystem request failed on channel 0" + ) + } + switch Self.uploadStep(for: request) { + case .createDirectory, .upload, .finalize: + return RemoteCommandResult(status: 0, stdout: "", stderr: "") + case .cleanup, .unknown: + return Self.unexpectedRequestResult(request) + } + } + let coordinator = makeCoordinator(runner: runner) + defer { coordinator.stop() } + let location = RemoteDaemonInstallLocation( + relativePath: ".cmux/bin/cmuxd-remote/test/linux-amd64/cmuxd-remote", + absolutePath: "/home/test/.cmux/bin/cmuxd-remote/test/linux-amd64/cmuxd-remote" + ) + + try coordinator.queue.sync { + try coordinator.uploadRemoteDaemonBinaryLocked( + localBinary: localBinary, + location: location + ) + } + + let requests = runner.requests + #expect(requests.map(Self.uploadStep) == [.createDirectory, .upload, .finalize]) + #expect(requests.allSatisfy { $0.executable == "/usr/bin/ssh" }) + let createDirectoryRequest = try #require( + requests.first { request in + Self.uploadStep(for: request) == .createDirectory + } + ) + #expect(createDirectoryRequest.arguments.last?.contains(location.directory) == true) + let uploadRequest = try #require( + requests.first { request in + Self.uploadStep(for: request) == .upload + } + ) + #expect(uploadRequest.stdinFile == localBinary) + let finalizeRequest = try #require( + requests.first { request in + Self.uploadStep(for: request) == .finalize + } + ) + #expect(finalizeRequest.arguments.last?.contains(location.absolutePath) == true) + } + + @Test("Upload reports SSH exec failures with their remote detail") + func uploadReportsExecFailureDetail() throws { + let fileManager = FileManager.default + let localBinary = fileManager.temporaryDirectory.appendingPathComponent( + "cmux-remote-daemon-upload-\(UUID().uuidString)", + isDirectory: false + ) + try Data("fake daemon".utf8).write(to: localBinary) + defer { try? fileManager.removeItem(at: localBinary) } + + let runner = RecordingProcessRunner { request in + switch Self.uploadStep(for: request) { + case .createDirectory, .cleanup: + return RemoteCommandResult(status: 0, stdout: "", stderr: "") + case .upload: + return RemoteCommandResult( + status: 1, + stdout: "", + stderr: "cat: remote path: Permission denied" + ) + case .finalize, .unknown: + return Self.unexpectedRequestResult(request) + } + } + let coordinator = makeCoordinator(runner: runner) + defer { coordinator.stop() } + let location = RemoteDaemonInstallLocation( + relativePath: ".cmux/bin/cmuxd-remote/test/linux-amd64/cmuxd-remote", + absolutePath: "/home/test/.cmux/bin/cmuxd-remote/test/linux-amd64/cmuxd-remote" + ) + + do { + try coordinator.queue.sync { + try coordinator.uploadRemoteDaemonBinaryLocked( + localBinary: localBinary, + location: location + ) + } + Issue.record("Expected SSH exec upload to fail") + } catch { + let nsError = error as NSError + #expect(nsError.domain == "cmux.remote.daemon") + #expect(nsError.code == 31) + #expect( + nsError.localizedDescription == + "failed to upload cmuxd-remote: cat: remote path: Permission denied" + ) + } + + let requests = runner.requests + #expect(requests.map(Self.uploadStep) == [.createDirectory, .upload, .cleanup]) + let uploadRequest = try #require( + requests.first { request in + Self.uploadStep(for: request) == .upload + } + ) + let cleanupRequest = try #require( + requests.first { request in + Self.uploadStep(for: request) == .cleanup + } + ) + let temporaryPathMarker = try #require( + Self.temporaryPathMarker(in: uploadRequest.arguments.last) + ) + #expect(cleanupRequest.arguments.last?.contains(temporaryPathMarker) == true) + #expect(cleanupRequest.arguments.last?.contains(location.absolutePath) == true) + } + + @Test("Finalization failure cleans the temporary upload and reports install detail") + func finalizationFailureCleansTemporaryUpload() throws { + let fileManager = FileManager.default + let localBinary = fileManager.temporaryDirectory.appendingPathComponent( + "cmux-remote-daemon-upload-\(UUID().uuidString)", + isDirectory: false + ) + try Data("fake daemon".utf8).write(to: localBinary) + defer { try? fileManager.removeItem(at: localBinary) } + + let runner = RecordingProcessRunner { request in + switch Self.uploadStep(for: request) { + case .createDirectory, .upload, .cleanup: + return RemoteCommandResult(status: 0, stdout: "", stderr: "") + case .finalize: + return RemoteCommandResult( + status: 1, + stdout: "", + stderr: "chmod: remote helper: Operation not permitted" + ) + case .unknown: + return Self.unexpectedRequestResult(request) + } + } + let coordinator = makeCoordinator(runner: runner) + defer { coordinator.stop() } + let location = RemoteDaemonInstallLocation( + relativePath: ".cmux/bin/cmuxd-remote/test/linux-amd64/cmuxd-remote", + absolutePath: "/home/test/.cmux/bin/cmuxd-remote/test/linux-amd64/cmuxd-remote" + ) + + do { + try coordinator.queue.sync { + try coordinator.uploadRemoteDaemonBinaryLocked( + localBinary: localBinary, + location: location + ) + } + Issue.record("Expected remote daemon finalization to fail") + } catch { + let nsError = error as NSError + #expect(nsError.domain == "cmux.remote.daemon") + #expect(nsError.code == 32) + #expect( + nsError.localizedDescription == + "failed to install remote daemon binary: chmod: remote helper: Operation not permitted" + ) + } + + let requests = runner.requests + #expect(requests.map(Self.uploadStep) == [.createDirectory, .upload, .finalize, .cleanup]) + let uploadRequest = try #require( + requests.first { request in + Self.uploadStep(for: request) == .upload + } + ) + let cleanupRequest = try #require( + requests.first { request in + Self.uploadStep(for: request) == .cleanup + } + ) + let temporaryPathMarker = try #require( + Self.temporaryPathMarker(in: uploadRequest.arguments.last) + ) + #expect(cleanupRequest.arguments.last?.contains(temporaryPathMarker) == true) + #expect(cleanupRequest.arguments.last?.contains(location.absolutePath) == true) + } + + @Test("Upload process failures do not expose arbitrary local error text") + func uploadProcessFailureSanitizesLocalDetail() throws { + try assertProcessFailureIsSanitized( + at: .upload, + expectedCode: 31, + expectedDescription: "failed to upload cmuxd-remote" + ) + } + + @Test("Directory process failures do not expose arbitrary local error text") + func directoryProcessFailureSanitizesLocalDetail() throws { + try assertProcessFailureIsSanitized( + at: .createDirectory, + expectedCode: 30, + expectedDescription: "failed to create remote daemon directory" + ) + } + + @Test("Finalization process failures do not expose arbitrary local error text") + func finalizationProcessFailureSanitizesLocalDetail() throws { + try assertProcessFailureIsSanitized( + at: .finalize, + expectedCode: 32, + expectedDescription: "failed to install remote daemon binary" + ) + } + + private func assertProcessFailureIsSanitized( + at failingStep: RemoteDaemonUploadStep, + expectedCode: Int, + expectedDescription: String + ) throws { + let fileManager = FileManager.default + let localBinary = fileManager.temporaryDirectory.appendingPathComponent( + "cmux-remote-daemon-upload-\(UUID().uuidString)", + isDirectory: false + ) + try Data("fake daemon".utf8).write(to: localBinary) + defer { try? fileManager.removeItem(at: localBinary) } + + let privateDetail = "sensitive local path /Users/example/private/key" + let runner = RecordingProcessRunner { request in + let step = Self.uploadStep(for: request) + if step == failingStep { + throw NSError(domain: "test.local.process", code: 1, userInfo: [ + NSLocalizedDescriptionKey: privateDetail, + ]) + } + switch step { + case .createDirectory, .upload, .finalize, .cleanup: + return RemoteCommandResult(status: 0, stdout: "", stderr: "") + case .unknown: + return Self.unexpectedRequestResult(request) + } + } + let coordinator = makeCoordinator(runner: runner) + defer { coordinator.stop() } + let location = RemoteDaemonInstallLocation( + relativePath: ".cmux/bin/cmuxd-remote/test/linux-amd64/cmuxd-remote", + absolutePath: "/home/test/.cmux/bin/cmuxd-remote/test/linux-amd64/cmuxd-remote" + ) + + do { + try coordinator.queue.sync { + try coordinator.uploadRemoteDaemonBinaryLocked( + localBinary: localBinary, + location: location + ) + } + Issue.record("Expected the injected process failure to propagate") + } catch { + let nsError = error as NSError + #expect(nsError.domain == "cmux.remote.daemon") + #expect(nsError.code == expectedCode) + #expect(nsError.localizedDescription == expectedDescription) + #expect(!nsError.localizedDescription.contains(privateDetail)) + } + + let expectedSteps: [RemoteDaemonUploadStep] + switch failingStep { + case .createDirectory: + expectedSteps = [.createDirectory] + case .upload: + expectedSteps = [.createDirectory, .upload, .cleanup] + case .finalize: + expectedSteps = [.createDirectory, .upload, .finalize, .cleanup] + case .cleanup, .unknown: + Issue.record("Unsupported process-failure test step: \(failingStep)") + return + } + #expect(runner.requests.map(Self.uploadStep) == expectedSteps) + } + + private static func uploadStep(for request: RemoteProcessRequest) -> RemoteDaemonUploadStep { + guard request.executable == "/usr/bin/ssh", + let command = request.arguments.last else { + return .unknown + } + if command.contains("mkdir -p ") { + return .createDirectory + } + if command.contains("cat > ") { + return .upload + } + if command.contains("chmod 755 "), command.contains("mv ") { + return .finalize + } + if command.contains("rm -f -- ") { + return .cleanup + } + return .unknown + } + + private static func temporaryPathMarker(in command: String?) -> String? { + guard let command, + let markerRange = command.range(of: ".tmp-") else { + return nil + } + let marker = command[markerRange.lowerBound...].prefix(13) + guard marker.count == 13 else { return nil } + return String(marker) + } + + private static func unexpectedRequestResult(_ request: RemoteProcessRequest) -> RemoteCommandResult { + RemoteCommandResult( + status: 97, + stdout: "", + stderr: "unexpected request: \(request.executable) \(request.arguments.last ?? "")" + ) + } + + private func makeCoordinator(runner: RecordingProcessRunner) -> RemoteSessionCoordinator { + let configuration = WorkspaceRemoteConfiguration( + destination: "test@sftp-disabled.example", + port: 2222, + identityFile: "/tmp/cmux-test-identity", + sshOptions: [], + localProxyPort: nil, + relayPort: nil, + relayID: nil, + relayToken: nil, + localSocketPath: nil, + terminalStartupCommand: nil, + preserveAfterTerminalExit: false, + persistentDaemonSlot: nil + ) + return RemoteSessionCoordinator( + host: NoopRemoteSessionHost(), + configuration: configuration, + proxyBroker: SSHOverrideUnusedRemoteProxyBroker(), + connectionBroker: NativeSSHConnectionBroker(), + manifestRepository: RemoteDaemonManifestRepository(homeDirectory: FileManager.default.temporaryDirectory), + processRunner: runner, + reachabilityProbe: SSHOverrideNoopReachabilityProbe(), + relayCommandRewriter: SSHOverridePassthroughRelayCommandRewriter(), + buildInfo: SSHOverrideStubBuildInfo(), + daemonStrings: RemoteDaemonStrings( + missingPersistentPTYCapability: "", + missingRequiredFunctionality: "" + ), + strings: RemoteSessionStrings( + connectedVMNoProxyFormat: "%@", + suspendedDetailFormat: "%@" + ) + ) + } +} diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift index 816f1e029bf3..fbcc0665e2ca 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionProcessRunnerTests.swift @@ -70,6 +70,31 @@ struct RemoteSessionProcessRunnerTests { #expect(result.stdout == "hello-stdin") } + @Test("Streams a local file through stdin") + func streamsFileStdin() throws { + let fileManager = FileManager.default + let fileURL = fileManager.temporaryDirectory.appendingPathComponent( + "cmux-process-stdin-\(UUID().uuidString)", + isDirectory: false + ) + try Data("hello-file-stdin".utf8).write(to: fileURL) + defer { try? fileManager.removeItem(at: fileURL) } + + let runner = RemoteSessionProcessRunner() + let result = try runner.run( + RemoteProcessRequest( + executable: "/bin/cat", + arguments: [], + stdinFile: fileURL, + timeout: 5 + ), + operation: nil + ) + + #expect(result.status == 0) + #expect(result.stdout == "hello-file-stdin") + } + @Test("Launch failure throws the pinned cmux.remote.process code 1") func launchFailurePinsErrorCode() { let runner = RemoteSessionProcessRunner() diff --git a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionSSHRemoteCommandOverrideTests.swift b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionSSHRemoteCommandOverrideTests.swift index 763ec23cc203..fbba3b0e8646 100644 --- a/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionSSHRemoteCommandOverrideTests.swift +++ b/Packages/macOS/CmuxRemoteSession/Tests/CmuxRemoteSessionTests/RemoteSessionSSHRemoteCommandOverrideTests.swift @@ -74,6 +74,36 @@ struct RemoteSessionSSHRemoteCommandOverrideTests { } } + @Test("File-backed SSH exec overrides a configured StdinNull") + func fileBackedSSHExecOverridesConfiguredStdinNull() throws { + let runner = RecordingProcessRunner() + let coordinator = Self.makeCoordinator( + runner: runner, + sshOptions: ["StdinNull=yes"] + ) + defer { coordinator.stop() } + + let localFile = URL(fileURLWithPath: "/tmp/cmux-test-helper") + _ = try coordinator.sshExec( + arguments: coordinator.sshCommonArguments(batchMode: true) + [ + "user@example.test", + "sh -c 'cat > remote-helper'", + ], + stdinFile: localFile, + timeout: 1 + ) + + let request = try #require(runner.requests.first) + #expect(request.stdinFile == localFile) + let overrideIndex = Self.pairIndex(request.arguments, "-o", "StdinNull=no") + let configuredIndex = Self.pairIndex(request.arguments, "-o", "StdinNull=yes") + #expect(overrideIndex != nil) + #expect(configuredIndex != nil) + if let overrideIndex, let configuredIndex { + #expect(overrideIndex < configuredIndex) + } + } + // MARK: - Helpers private static func consecutive(_ args: [String], _ a: String, _ b: String) -> Bool { @@ -110,15 +140,15 @@ struct RemoteSessionSSHRemoteCommandOverrideTests { return RemoteSessionCoordinator( host: NoopRemoteSessionHost(), configuration: configuration, - proxyBroker: UnusedRemoteProxyBroker(), + proxyBroker: SSHOverrideUnusedRemoteProxyBroker(), connectionBroker: NativeSSHConnectionBroker(), manifestRepository: RemoteDaemonManifestRepository( homeDirectory: FileManager.default.temporaryDirectory ), processRunner: runner, - reachabilityProbe: NoopReachabilityProbe(), - relayCommandRewriter: PassthroughRelayCommandRewriter(), - buildInfo: StubBuildInfo(), + reachabilityProbe: SSHOverrideNoopReachabilityProbe(), + relayCommandRewriter: SSHOverridePassthroughRelayCommandRewriter(), + buildInfo: SSHOverrideStubBuildInfo(), daemonStrings: RemoteDaemonStrings( missingPersistentPTYCapability: "", missingRequiredFunctionality: "" @@ -133,11 +163,23 @@ struct RemoteSessionSSHRemoteCommandOverrideTests { // MARK: - Stubs -/// Records every subprocess request the coordinator issues and returns a -/// canned successful result, so the argv can be asserted without touching ssh. -private final class RecordingProcessRunner: RemoteSessionProcessRunning, @unchecked Sendable { +/// Records every subprocess request and returns an injected response. +/// Synchronization: the lock guards request storage; `response` is immutable +/// and `@Sendable`, which makes the unchecked conformance safe. +final class RecordingProcessRunner: RemoteSessionProcessRunning, @unchecked Sendable { + typealias Response = @Sendable (RemoteProcessRequest) throws -> RemoteCommandResult + private let lock = NSLock() private var _requests: [RemoteProcessRequest] = [] + private let response: Response + + init( + response: @escaping Response = { _ in + RemoteCommandResult(status: 0, stdout: "", stderr: "") + } + ) { + self.response = response + } var requests: [RemoteProcessRequest] { lock.withLock { _requests } } @@ -146,11 +188,11 @@ private final class RecordingProcessRunner: RemoteSessionProcessRunning, @unchec operation: (any RemoteTransferCancelling)? ) throws -> RemoteCommandResult { lock.withLock { _requests.append(request) } - return RemoteCommandResult(status: 0, stdout: "", stderr: "") + return try response(request) } } -private struct NoopRemoteSessionHost: RemoteSessionHosting { +struct NoopRemoteSessionHost: RemoteSessionHosting { func publishConnectionState(_ state: WorkspaceRemoteConnectionState, detail: String?) {} func publishDaemonStatus(_ status: WorkspaceRemoteDaemonStatus) {} func publishProxyEndpoint(_ endpoint: BrowserProxyEndpoint?) {} @@ -159,13 +201,13 @@ private struct NoopRemoteSessionHost: RemoteSessionHosting { func publishBootstrapRemoteTTY(_ ttyName: String) {} } -private final class UnusedRemoteProxyBroker: RemoteProxyBrokering, @unchecked Sendable { +final class SSHOverrideUnusedRemoteProxyBroker: RemoteProxyBrokering, @unchecked Sendable { func acquire( configuration: WorkspaceRemoteConfiguration, remotePath: String, onUpdate: @escaping @Sendable (RemoteProxyBrokerUpdate) -> Void ) -> RemoteProxyLease { - fatalError("UnusedRemoteProxyBroker.acquire is not exercised by these tests") + fatalError("SSHOverrideUnusedRemoteProxyBroker.acquire is not exercised by these tests") } func listPTY(configuration: WorkspaceRemoteConfiguration) throws -> [[String: Any]] { [] } @@ -207,11 +249,11 @@ private final class UnusedRemoteProxyBroker: RemoteProxyBrokering, @unchecked Se command: String?, requireExisting: Bool ) throws -> RemotePTYBridgeServer.Endpoint { - fatalError("UnusedRemoteProxyBroker.startPTYBridge is not exercised by these tests") + fatalError("SSHOverrideUnusedRemoteProxyBroker.startPTYBridge is not exercised by these tests") } } -private struct NoopReachabilityProbe: RemoteHostReachabilityProbing { +struct SSHOverrideNoopReachabilityProbe: RemoteHostReachabilityProbing { func probe( destination: String, port: Int?, @@ -221,7 +263,7 @@ private struct NoopReachabilityProbe: RemoteHostReachabilityProbing { ) {} } -private struct PassthroughRelayCommandRewriter: RemoteRelayCommandRewriting { +struct SSHOverridePassthroughRelayCommandRewriter: RemoteRelayCommandRewriting { func rewriteRemoteRelayCommandLine( _ commandLine: Data, workspaceAliases: [UUID: UUID], @@ -231,7 +273,7 @@ private struct PassthroughRelayCommandRewriter: RemoteRelayCommandRewriting { } } -private struct StubBuildInfo: RemoteSessionBuildInfoProviding { +struct SSHOverrideStubBuildInfo: RemoteSessionBuildInfoProviding { func appVersion() -> String? { nil } func embeddedDaemonManifest() -> WorkspaceRemoteDaemonManifest? { nil } func executableDirectoryURL() -> URL? { nil } diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index 9c103c4555bb..859e8103d6b2 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -133726,6 +133726,108 @@ } } }, + "remoteDaemon.upload.createDirectoryFailed": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "failed to create remote daemon directory" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "リモートデーモンのディレクトリを作成できませんでした" + } + } + } + }, + "remoteDaemon.upload.createDirectoryFailedWithDetail": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "failed to create remote daemon directory: %@" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "リモートデーモンのディレクトリを作成できませんでした：%@" + } + } + } + }, + "remoteDaemon.upload.installFailed": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "failed to install remote daemon binary" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "リモートデーモンのバイナリをインストールできませんでした" + } + } + } + }, + "remoteDaemon.upload.installFailedWithDetail": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "failed to install remote daemon binary: %@" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "リモートデーモンのバイナリをインストールできませんでした：%@" + } + } + } + }, + "remoteDaemon.upload.transferFailed": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "failed to upload cmuxd-remote" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "cmuxd-remote をアップロードできませんでした" + } + } + } + }, + "remoteDaemon.upload.transferFailedWithDetail": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "failed to upload cmuxd-remote: %@" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "cmuxd-remote をアップロードできませんでした：%@" + } + } + } + }, "remotePTYAttach.error.allocationDiagnostic": { "comment": "Passthrough of the remote daemon's dynamic PTY-allocation diagnostic (device paths, devpts mount options, remediation). The detail is locale-invariant technical text, so every locale is just the substitution placeholder.", "localizations": {