From 33443b130b85265ed2dbfc896a82e9d9e3d686ae Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:28:30 -0700 Subject: [PATCH 01/48] cmux ssh: test that replay and reconnect filters drop OSC 52 clipboard reads Co-Authored-By: Claude Opus 5.5 --- ...connectInputByteFilterClipboardTests.swift | 71 ++++++++++++++++ ...HPTYReplayOutputFilterClipboardTests.swift | 85 +++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReplayOutputFilterClipboardTests.swift diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift new file mode 100644 index 000000000000..acb9ad4a25a7 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift @@ -0,0 +1,71 @@ +import Foundation +import Testing +@testable import CmuxFoundation + +@Suite("SSH PTY reconnect input byte filter clipboard replies") +struct SSHPTYReconnectInputByteFilterClipboardTests { + private static let terminators = ["\u{07}", "\u{1B}\\"] + + @Test("a stale OSC 52 clipboard reply is dropped and ordinary input passes", arguments: terminators) + func dropsClipboardReplyBeforeOrdinaryInput(terminator: String) { + var filter = SSHPTYReconnectInputByteFilter(enabled: true) + let reply = Data("\u{1B}]52;c;c2VjcmV0LXRva2Vu\(terminator)".utf8) + let normalInput = Data("printf keep\n".utf8) + + #expect(filter.filter(reply + normalInput) == normalInput) + #expect(!filter.isFilteringActive) + } + + @Test("a clipboard reply split at every chunk boundary is dropped", arguments: terminators) + func dropsClipboardReplySplitAcrossChunks(terminator: String) { + let reply = Data("\u{1B}]52;p;c2VjcmV0\(terminator)".utf8) + let normalInput = Data("ls\n".utf8) + for split in 1.. Date: Tue, 29 Sep 2026 17:30:29 -0700 Subject: [PATCH 02/48] cmux ssh: drop replayed OSC 52 clipboard reads and their reconnect replies Co-Authored-By: Claude Opus 5.5 --- .../SSHPTYReconnectInputByteFilter.swift | 77 ++++++++++++++----- .../SSHPTYReplayOutputFilter.swift | 48 +++++++----- 2 files changed, 87 insertions(+), 38 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift index bd82144e5e1d..6341e9503bc2 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift @@ -1,6 +1,7 @@ public import Foundation -/// Removes terminal probe replies queued while an SSH PTY bridge reconnects. +/// Removes terminal probe and OSC 52 clipboard replies queued while an SSH PTY +/// bridge reconnects. /// /// Filtering remains active only while input consists entirely of recognized /// terminal replies or EOT bytes. The first ordinary key byte ends filtering, @@ -22,10 +23,18 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { case strip(length: Int) case incomplete case passThrough + /// An OSC 52 clipboard reply whose terminator has not arrived yet. + case unterminatedClipboardReply } private var isFiltering: Bool private var pending = [UInt8]() + /// Whether the rest of an OSC 52 clipboard reply is being discarded. + /// + /// Clipboard replies carry the user's clipboard and can exceed the + /// pending-probe bound, so their bytes are dropped as they stream in + /// instead of being buffered and later flushed to the remote PTY. + private var discardingClipboardReply = false /// Creates a reconnect-input filter. /// @@ -54,6 +63,14 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { var output = Data() var index = 0 + if discardingClipboardReply { + guard let end = Self.stringTerminatorEnd(in: bytes, from: 0) else { + retainTrailingEscapeOfDiscardedReply(bytes) + return output + } + discardingClipboardReply = false + index = end + } while index < bytes.count { if bytes[index] == Self.endOfTransmission { index += 1 @@ -81,16 +98,33 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { isFiltering = false output.append(contentsOf: bytes[index...]) return output + case .unterminatedClipboardReply: + discardingClipboardReply = true + retainTrailingEscapeOfDiscardedReply(bytes) + return output } } return output } + /// Keeps a trailing ESC so a string terminator split across reads is seen. + private mutating func retainTrailingEscapeOfDiscardedReply(_ bytes: [UInt8]) { + if bytes.last == Self.escape { + pending.append(Self.escape) + } + } + /// Returns any incomplete escape sequence retained by the filter. /// /// - Returns: Pending bytes in their original order. public mutating func finish() -> Data { + if discardingClipboardReply { + // Never forward any part of a clipboard reply. + discardingClipboardReply = false + pending.removeAll(keepingCapacity: false) + return Data() + } guard !pending.isEmpty else { return Data() } @@ -110,12 +144,12 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { /// Whether an incomplete recognized sequence is awaiting more bytes. public var hasPendingInput: Bool { - isFiltering && !pending.isEmpty + isFiltering && (!pending.isEmpty || discardingClipboardReply) } /// Whether filtering is active with no partial sequence buffered. public var isFilteringAtProbeBoundary: Bool { - isFiltering && pending.isEmpty + isFiltering && pending.isEmpty && !discardingClipboardReply } /// Whether recognized reconnect-time replies are still being removed. @@ -136,7 +170,7 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { switch bytes[start + 1] { case rightBracket: - return oscColorReplySequence(in: bytes, at: start) + return oscReplySequence(in: bytes, at: start) case leftBracket: return csiProbeReplySequence(in: bytes, at: start) case dcs: @@ -169,7 +203,7 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { return .incomplete } - private static func oscColorReplySequence( + private static func oscReplySequence( in bytes: [UInt8], at start: Int ) -> SequenceMatch { @@ -189,32 +223,37 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { } guard cursor < bytes.count else { - return isOSCColorReplyCommandPrefix(command) ? .incomplete : .passThrough + return isOSCReplyCommandPrefix(command) ? .incomplete : .passThrough } guard bytes[cursor] == semicolon else { return .passThrough } - guard command == [0x31, 0x30] || command == [0x31, 0x31] || command == [0x31, 0x32] else { + let isClipboardReply = command == [0x35, 0x32] + guard isClipboardReply || + command == [0x31, 0x30] || command == [0x31, 0x31] || command == [0x31, 0x32] else { return .passThrough } - cursor += 1 + guard let end = stringTerminatorEnd(in: bytes, from: cursor + 1) else { + return isClipboardReply ? .unterminatedClipboardReply : .incomplete + } + return .strip(length: end - start) + } + + /// Returns the index just past the first BEL or ST at or after `start`. + private static func stringTerminatorEnd(in bytes: [UInt8], from start: Int) -> Int? { + var cursor = start while cursor < bytes.count { let byte = bytes[cursor] if byte == bell { - return .strip(length: cursor - start + 1) + return cursor + 1 } - if byte == escape { - guard cursor + 1 < bytes.count else { - return .incomplete - } - if bytes[cursor + 1] == backslash { - return .strip(length: cursor - start + 2) - } + if byte == escape, cursor + 1 < bytes.count, bytes[cursor + 1] == backslash { + return cursor + 2 } cursor += 1 } - return .incomplete + return nil } private static func csiProbeReplySequence( @@ -237,8 +276,10 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { return .incomplete } - private static func isOSCColorReplyCommandPrefix(_ command: [UInt8]) -> Bool { + private static func isOSCReplyCommandPrefix(_ command: [UInt8]) -> Bool { command.isEmpty || + command == [0x35] || + command == [0x35, 0x32] || command == [0x31] || command == [0x31, 0x30] || command == [0x31, 0x31] || diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift index 4b9bb90c4253..5c688e705f4c 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift @@ -16,6 +16,7 @@ public struct SSHPTYReplayOutputFilter: Sendable { private static let bell: UInt8 = 0x07 private static let backslash: UInt8 = 0x5C private static let semicolon: UInt8 = 0x3B + private static let questionMark: UInt8 = 0x3F private static let maxPendingBytes = 4 * 1024 private enum SequenceMatch { @@ -231,34 +232,41 @@ public struct SSHPTYReplayOutputFilter: Sendable { return cursor == bytes.count ? .incomplete : .passThrough } cursor += 1 - var payloadFirst: UInt8? + let payloadStart = cursor while cursor < bytes.count { guard cursor - start <= Self.maxPendingBytes else { return .passThrough } - if payloadFirst == nil { payloadFirst = bytes[cursor] } + let terminatorLength: Int if bytes[cursor] == Self.bell { - return isColorQuery(command: command, commandDigitCount: commandDigitCount, payloadFirst: payloadFirst) - ? .strip(length: cursor - start + 1) - : .passThrough - } - if bytes[cursor] == Self.escape, cursor + 1 < bytes.count, - bytes[cursor + 1] == Self.backslash { - return isColorQuery(command: command, commandDigitCount: commandDigitCount, payloadFirst: payloadFirst) - ? .strip(length: cursor - start + 2) - : .passThrough + terminatorLength = 1 + } else if bytes[cursor] == Self.escape, cursor + 1 < bytes.count, + bytes[cursor + 1] == Self.backslash { + terminatorLength = 2 + } else { + cursor += 1 + continue } - cursor += 1 + let isQuery = commandDigitCount > 0 && isOSCQuery( + command: command, + payload: bytes[payloadStart.. Bool { - commandDigitCount > 0 && - (command == 4 || command == 10 || command == 11 || command == 12) && - payloadFirst == 0x3F + private static func isOSCQuery(command: Int, payload: ArraySlice) -> Bool { + switch command { + case 4, 10, 11, 12: + return payload.first == Self.questionMark + case 52: + // Clipboard read: `52 ; ; ?`. Replaying it would make + // the local terminal send its current clipboard to the remote. + // Writes carry base64 data instead of `?` and stay untouched. + guard let separator = payload.firstIndex(of: Self.semicolon) else { return false } + return payload[payload.index(after: separator)...].elementsEqual([Self.questionMark]) + default: + return false + } } private static func dcsQuery(in bytes: [UInt8], at start: Int) -> SequenceMatch { From ae41937bfb267412798ab024a2917f62085a72a0 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:32:30 -0700 Subject: [PATCH 03/48] cmux ssh: test that a read gap inside an OSC 52 reply keeps discarding Co-Authored-By: Claude Opus 5.5 --- ...connectInputByteFilterClipboardTests.swift | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift index acb9ad4a25a7..50f2885272f8 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift @@ -48,11 +48,26 @@ struct SSHPTYReconnectInputByteFilterClipboardTests { #expect(output == normalInput) } + @Test( + "a >25 ms read gap inside a clipboard reply does not end the discard", + arguments: ["c2Vj", "c2Vj\u{1B}"] + ) + func continuationTimeoutKeepsDiscardingClipboardReply(firstPayload: String) { + var filter = SSHPTYReconnectInputByteFilter(enabled: true) + var output = filter.filter(Data("\u{1B}]52;c;\(firstPayload)".utf8)) + output.append(expireContinuationTimeout(&filter)) + + let rest = firstPayload.hasSuffix("\u{1B}") ? "\\" : "cmV0\u{07}" + let normalInput = Data("ls\n".utf8) + output.append(filter.filter(Data(rest.utf8) + normalInput)) + + #expect(output == normalInput) + } + @Test("stopping mid-reply does not forward the partial clipboard reply") func stopFilteringDropsPartialClipboardReply() { var filter = SSHPTYReconnectInputByteFilter(enabled: true) #expect(filter.filter(Data("\u{1B}]52;c;c2VjcmV0".utf8)) == Data()) - #expect(filter.hasPendingInput) #expect(filter.stopFiltering() == Data()) let normalInput = Data("ls\n".utf8) @@ -68,4 +83,12 @@ struct SSHPTYReconnectInputByteFilterClipboardTests { let later = Data("\u{1B}]52;c;c2VjcmV0\u{07}".utf8) #expect(filter.filter(later) == later) } + + /// Applies what the CLI stdin pump (`SSHPTYAttachReconnectInputFilter`) + /// does when no byte arrives within its 25 ms continuation timeout: a + /// filter reporting `hasPendingInput` is ended with `stopFiltering()` and + /// the returned bytes are forwarded to the remote PTY. + private func expireContinuationTimeout(_ filter: inout SSHPTYReconnectInputByteFilter) -> Data { + filter.hasPendingInput ? filter.stopFiltering() : Data() + } } From 98f1449d75e5895dee4c27f8c3e86b7c6ab1d482 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:33:03 -0700 Subject: [PATCH 04/48] cmux ssh: keep discarding an OSC 52 reply across a read gap Co-Authored-By: Claude Opus 5.5 --- .../SSHPTYReconnectInputByteFilter.swift | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift index 6341e9503bc2..6afcc9c83713 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift @@ -33,7 +33,8 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { /// /// Clipboard replies carry the user's clipboard and can exceed the /// pending-probe bound, so their bytes are dropped as they stream in - /// instead of being buffered and later flushed to the remote PTY. + /// instead of being buffered and later flushed to the remote PTY. See + /// ``hasPendingInput`` for how long discarding may last. private var discardingClipboardReply = false /// Creates a reconnect-input filter. @@ -142,9 +143,19 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { return input } - /// Whether an incomplete recognized sequence is awaiting more bytes. + /// Whether an incomplete probe reply is buffered awaiting more bytes. + /// + /// Callers may end filtering with ``stopFiltering()`` when this stays true + /// past a short continuation timeout, which forwards the buffered bytes. + /// A clipboard reply being discarded is deliberately excluded: a pause in + /// the middle of one must not end filtering, or the rest of the clipboard + /// would reach the remote PTY. Discarding therefore lasts until BEL/ST + /// arrives or filtering stops (the caller's reconnect deadline, + /// ``finish()`` or ``stopFiltering()``), and nothing retained is forwarded. + /// The trade-off: if the terminator is lost, input typed before that + /// deadline is discarded with the reply. public var hasPendingInput: Bool { - isFiltering && (!pending.isEmpty || discardingClipboardReply) + isFiltering && !pending.isEmpty && !discardingClipboardReply } /// Whether filtering is active with no partial sequence buffered. From 0d7f4e514117487beedef0a9225af113fb368466 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:27:33 -0700 Subject: [PATCH 05/48] cmux ssh: test that the live release manifest cannot override the embedded daemon checksum Co-Authored-By: Claude Opus 5.5 --- .../RemoteDaemonManifestRepositoryTests.swift | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonManifestRepositoryTests.swift b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonManifestRepositoryTests.swift index f01c484922f6..532664fa730a 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonManifestRepositoryTests.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonManifestRepositoryTests.swift @@ -188,7 +188,7 @@ struct RemoteDaemonManifestRepositoryTests { #expect(permissions == 0o755) } - @Test("a checksum mismatch with no live-manifest rescue throws the pinned code-28 error") + @Test("a checksum mismatch throws the pinned code-28 error") func checksumMismatchThrows() throws { let server = FakeHTTPServer() defer { server.close() } @@ -210,27 +210,37 @@ struct RemoteDaemonManifestRepositoryTests { #expect(!FileManager.default.fileExists(atPath: cacheURL.path)) } - @Test("a stale embedded checksum is rescued by the live release manifest and reported in the result") - func checksumLiveManifestFallback() throws { + @Test("a live release manifest that matches a mismatched download never overrides the embedded checksum") + func liveManifestCannotOverrideEmbeddedChecksum() throws { let server = FakeHTTPServer() defer { server.close() } let home = try temporaryHome() let repository = makeRepository(home: home) - let binary = Data("nightly-overwritten bytes".utf8) - server.setResponse(path: "/cmuxd-remote-linux-amd64", body: binary) + // Someone with write access to the release swapped both the binary and + // the live manifest; only the checksum embedded in the signed app is + // trustworthy. + let swappedBinary = Data("attacker-swapped bytes".utf8) + server.setResponse(path: "/cmuxd-remote-linux-amd64", body: swappedBinary) server.setResponse( path: "/cmuxd-remote-manifest.json", - body: Data(makeManifestJSON(port: server.port, assetPath: "/cmuxd-remote-linux-amd64", sha256: sha256Hex(binary)).utf8) + body: Data(makeManifestJSON(port: server.port, assetPath: "/cmuxd-remote-linux-amd64", sha256: sha256Hex(swappedBinary)).utf8) ) - let staleEntry = makeEntry(port: server.port, assetPath: "/cmuxd-remote-linux-amd64", sha256: String(repeating: "f", count: 64)) + let embeddedEntry = makeEntry(port: server.port, assetPath: "/cmuxd-remote-linux-amd64", sha256: String(repeating: "f", count: 64)) - let download = try repository.downloadBinary( - entry: staleEntry, - version: "0.99.0", - releaseURL: "http://127.0.0.1:\(server.port)" - ) - #expect(download.usedLiveManifestChecksumFallback) - #expect(try Data(contentsOf: download.binaryURL) == binary) + var thrown: NSError? + do { + _ = try repository.downloadBinary( + entry: embeddedEntry, + version: "0.99.0", + releaseURL: "http://127.0.0.1:\(server.port)" + ) + } catch { + thrown = error as NSError + } + #expect(thrown?.domain == "cmux.remote.daemon") + #expect(thrown?.code == 28, "a download that misses the embedded checksum must be rejected") + let cacheURL = try repository.cachedBinaryURL(version: "0.99.0", goOS: "linux", goArch: "amd64") + #expect(!FileManager.default.fileExists(atPath: cacheURL.path), "the swapped binary must not reach the upload cache") } @Test("an HTTP error status surfaces as the pinned code-26 error") From 6f5ad60592c4396e29cde74d9663e7a61aa4a2fa Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:29:15 -0700 Subject: [PATCH 06/48] cmux ssh: never let the live release manifest override the embedded daemon checksum Co-Authored-By: Claude Opus 5.5 --- .../RemoteSessionCoordinator+Bootstrap.swift | 9 +- .../RemoteDaemonManifestRepository.swift | 83 +++++-------------- .../RemoteDaemonBundledAssetsTests.swift | 1 - .../RemoteDaemonManifestRepositoryTests.swift | 26 +----- 4 files changed, 25 insertions(+), 94 deletions(-) diff --git a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+Bootstrap.swift b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+Bootstrap.swift index 200f4e9b8ea7..8d6bc553311d 100644 --- a/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+Bootstrap.swift +++ b/Packages/macOS/CmuxRemoteSession/Sources/CmuxRemoteSession/Session/RemoteSessionCoordinator+Bootstrap.swift @@ -305,14 +305,7 @@ extension RemoteSessionCoordinator { debugLog("remote.build.cached path=\(cacheURL.path)") return cacheURL } - let download = try manifestRepository.downloadBinary( - entry: entry, - version: manifest.appVersion, - releaseURL: manifest.releaseURL - ) - if download.usedLiveManifestChecksumFallback { - debugLog("remote.download.checksum-fallback: embedded manifest checksum stale, live manifest matched for \(entry.assetName)") - } + let download = try manifestRepository.downloadBinary(entry: entry, version: manifest.appVersion) debugLog("remote.build.downloaded path=\(download.binaryURL.path)") return download.binaryURL } diff --git a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Manifest/RemoteDaemonManifestRepository.swift b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Manifest/RemoteDaemonManifestRepository.swift index 54a100fd2fcc..d708a7218a4c 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Manifest/RemoteDaemonManifestRepository.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Manifest/RemoteDaemonManifestRepository.swift @@ -4,9 +4,11 @@ internal import CmuxSettings internal import CryptoKit /// Mediates the cmuxd-remote release manifest and the local cache of -/// verified daemon binaries it indexes: fetches the live manifest from a -/// release, downloads + checksum-verifies binaries, and validates/places them -/// in the shared on-disk cache the separately-signed CLI also reads. +/// verified daemon binaries it indexes: downloads binaries, verifies them +/// against the checksum pinned in the signed app's embedded manifest, and +/// validates/places them in the shared on-disk cache the separately-signed +/// CLI also reads. The live release manifest is never consulted: it shares a +/// trust domain with the release assets, so it cannot vouch for them. /// /// Faithful lift of the manifest/cache/download half of the legacy /// `WorkspaceRemoteSessionController` bootstrap path. The embedded-manifest @@ -16,19 +18,15 @@ internal import CryptoKit /// Isolation design: stateless `Sendable` value (injected `FileManager` + /// home directory only), so no actor is warranted; methods are synchronous /// and blocking by contract because the caller is the session controller's -/// serial utility queue mid-bootstrap, which cannot await. The two network -/// calls bridge `URLSession`'s callbacks with a semaphore exactly like the +/// serial utility queue mid-bootstrap, which cannot await. The network call +/// bridges `URLSession`'s callbacks with a semaphore exactly like the /// legacy code; converting them to `async` is deferred modernization for the /// coordinator phase. public struct RemoteDaemonManifestRepository: Sendable { - /// Result of ``downloadBinary(entry:version:releaseURL:)``. + /// Result of ``downloadBinary(entry:version:)``. public struct Download: Sendable { /// Final cached location of the verified binary. public let binaryURL: URL - /// True when the embedded manifest's checksum was stale and the - /// download was instead verified against the live release manifest - /// (a newer nightly overwrote the shared release asset). - public let usedLiveManifestChecksumFallback: Bool } // FileManager is documented thread-safe for these path-based operations; @@ -94,43 +92,16 @@ public struct RemoteDaemonManifestRepository: Sendable { return nil } - /// Fetches the live manifest JSON from the release, returning nil on any - /// failure (blocking; 15s request timeout, 20s overall wait). - public func fetchManifest(releaseURL: String, version: String) -> WorkspaceRemoteDaemonManifest? { - guard let manifestURL = URL(string: "\(releaseURL)/cmuxd-remote-manifest.json") else { return nil } - let request = NSMutableURLRequest(url: manifestURL) - request.timeoutInterval = 15 - request.setValue("cmux/\(version)", forHTTPHeaderField: "User-Agent") - let session = URLSession(configuration: .ephemeral) - let semaphore = DispatchSemaphore(value: 0) - // Single-assignment hand-off signalled exactly once by the data-task - // callback before the blocking wait returns (legacy bridge shape). - nonisolated(unsafe) var resultData: Data? - session.dataTask(with: request as URLRequest) { data, response, error in - defer { semaphore.signal() } - guard error == nil, - let httpResponse = response as? HTTPURLResponse, - (200...299).contains(httpResponse.statusCode) else { return } - resultData = data - }.resume() - _ = semaphore.wait(timeout: .now() + 20.0) - session.finishTasksAndInvalidate() - guard let data = resultData else { return nil } - return try? JSONDecoder().decode(WorkspaceRemoteDaemonManifest.self, from: data) - } - /// Installs a checksum-verified bundled binary when available, otherwise - /// downloads `entry`'s binary and verifies its checksum (falling back to the - /// live release manifest when the embedded checksum is stale), marks it - /// executable, and atomically installs it at the cache path (blocking; + /// downloads `entry`'s binary and verifies it against `entry`'s checksum + /// (always throwing on a mismatch), marks it executable, and atomically installs it at the cache path (blocking; /// 60s request timeout, 75s overall wait). public func downloadBinary( entry: WorkspaceRemoteDaemonManifest.Entry, - version: String, - releaseURL: String? = nil + version: String ) throws -> Download { if let binaryURL = try installBundledBinary(entry: entry, version: version) { - return Download(binaryURL: binaryURL, usedLiveManifestChecksumFallback: false) + return Download(binaryURL: binaryURL) } guard let url = URL(string: entry.downloadURL) else { throw NSError(domain: "cmux.remote.daemon", code: 25, userInfo: [ @@ -179,23 +150,14 @@ public struct RemoteDaemonManifestRepository: Sendable { ]) } - var usedLiveManifestChecksumFallback = false - let downloadedSHA = try sha256Hex(forFile: downloadedURL) - if downloadedSHA != entry.sha256.lowercased() { - // The embedded manifest's checksum doesn't match the downloaded binary. - // This can happen when a newer nightly overwrites the shared release - // asset after this build's manifest was embedded. As a fallback, fetch - // the live manifest from the release and verify against that. - if let releaseURL, - let liveManifest = fetchManifest(releaseURL: releaseURL, version: version), - let liveEntry = liveManifest.entry(goOS: entry.goOS, goArch: entry.goArch), - downloadedSHA == liveEntry.sha256.lowercased() { - usedLiveManifestChecksumFallback = true - } else { - throw NSError(domain: "cmux.remote.daemon", code: 28, userInfo: [ - NSLocalizedDescriptionKey: "remote daemon checksum mismatch for \(entry.assetName)", - ]) - } + // Only the checksum embedded in the signed app is trusted. Release + // assets are published per build and never overwritten, so a mismatch + // means a tampered or corrupted asset, never a stale pin. + guard try sha256Hex(forFile: downloadedURL) == entry.sha256.lowercased() else { + try? fileManager.removeItem(at: downloadedURL) + throw NSError(domain: "cmux.remote.daemon", code: 28, userInfo: [ + NSLocalizedDescriptionKey: "remote daemon checksum mismatch for \(entry.assetName)", + ]) } let tempURL = cacheURL.deletingLastPathComponent() @@ -205,10 +167,7 @@ public struct RemoteDaemonManifestRepository: Sendable { try fileManager.setAttributes([.posixPermissions: 0o755], ofItemAtPath: tempURL.path) try? fileManager.removeItem(at: cacheURL) try fileManager.moveItem(at: tempURL, to: cacheURL) - return Download( - binaryURL: cacheURL, - usedLiveManifestChecksumFallback: usedLiveManifestChecksumFallback - ) + return Download(binaryURL: cacheURL) } private func cacheRoot() throws -> URL { diff --git a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonBundledAssetsTests.swift b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonBundledAssetsTests.swift index a49ee1814d2e..a9054a6b9610 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonBundledAssetsTests.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonBundledAssetsTests.swift @@ -38,7 +38,6 @@ struct RemoteDaemonBundledAssetsTests { let download = try repository.downloadBinary(entry: entry, version: "test-nightly.12301") #expect(try Data(contentsOf: download.binaryURL) == binaryFixture) #expect(FileManager.default.isExecutableFile(atPath: download.binaryURL.path)) - #expect(!download.usedLiveManifestChecksumFallback) #expect(try repository.validatedCachedBinary(entry: entry, version: "test-nightly.12301") == download.binaryURL) // A second workspace may reach installation after another filled the cache. #expect(try repository.downloadBinary(entry: entry, version: "test-nightly.12301").binaryURL == download.binaryURL) diff --git a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonManifestRepositoryTests.swift b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonManifestRepositoryTests.swift index 532664fa730a..7f00888356e4 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonManifestRepositoryTests.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteDaemonManifestRepositoryTests.swift @@ -152,23 +152,6 @@ struct RemoteDaemonManifestRepositoryTests { #expect(rootExists && isDirectory.boolValue, "cache root is created eagerly") } - @Test("fetchManifest decodes a live manifest and returns nil on a non-2xx status") - func fetchManifestStatuses() throws { - let server = FakeHTTPServer() - defer { server.close() } - let home = try temporaryHome() - let repository = makeRepository(home: home) - let manifestJSON = makeManifestJSON(port: server.port, assetPath: "/bin", sha256: "abc123") - - server.setResponse(path: "/cmuxd-remote-manifest.json", body: Data(manifestJSON.utf8)) - let manifest = repository.fetchManifest(releaseURL: "http://127.0.0.1:\(server.port)", version: "0.99.0") - #expect(manifest?.releaseTag == "v0.99.0") - #expect(manifest?.entry(goOS: "linux", goArch: "amd64")?.assetName == "cmuxd-remote-linux-amd64") - - server.setResponse(path: "/cmuxd-remote-manifest.json", status: 500, body: Data()) - #expect(repository.fetchManifest(releaseURL: "http://127.0.0.1:\(server.port)", version: "0.99.0") == nil) - } - @Test("downloadBinary verifies the checksum and installs the binary executable at the cache path") func downloadHappyPath() throws { let server = FakeHTTPServer() @@ -180,7 +163,6 @@ struct RemoteDaemonManifestRepositoryTests { let entry = makeEntry(port: server.port, assetPath: "/cmuxd-remote-linux-amd64", sha256: sha256Hex(binary)) let download = try repository.downloadBinary(entry: entry, version: "0.99.0") - #expect(!download.usedLiveManifestChecksumFallback) #expect(download.binaryURL == (try repository.cachedBinaryURL(version: "0.99.0", goOS: "linux", goArch: "amd64"))) #expect(try Data(contentsOf: download.binaryURL) == binary) #expect(FileManager.default.isExecutableFile(atPath: download.binaryURL.path)) @@ -229,11 +211,9 @@ struct RemoteDaemonManifestRepositoryTests { var thrown: NSError? do { - _ = try repository.downloadBinary( - entry: embeddedEntry, - version: "0.99.0", - releaseURL: "http://127.0.0.1:\(server.port)" - ) + // The live manifest above is still served at the release URL; the + // repository must never consult it. + _ = try repository.downloadBinary(entry: embeddedEntry, version: "0.99.0") } catch { thrown = error as NSError } From 30c7650e040794ac99e8ce6b20efe9ec4f85a91b Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:31:41 -0700 Subject: [PATCH 07/48] cmux vm ssh: test that an unlaunched SSH startup script is removed Move the one-shot launcher writer into CmuxFoundation unchanged and add a regression showing that a credential-bearing launcher whose terminal never starts stays in the temporary directory. Co-Authored-By: Claude Opus 5.5 --- CLI/CMUXCLI+SSHStartupScripts.swift | 8 +-- .../SSHStartupLaunchScripts.swift | 54 +++++++++++++++++ .../SSHStartupLaunchScriptsTests.swift | 59 +++++++++++++++++++ 3 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHStartupLaunchScripts.swift create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHStartupLaunchScriptsTests.swift diff --git a/CLI/CMUXCLI+SSHStartupScripts.swift b/CLI/CMUXCLI+SSHStartupScripts.swift index 1d0e8be0eb9e..14cf1d3b1356 100644 --- a/CLI/CMUXCLI+SSHStartupScripts.swift +++ b/CLI/CMUXCLI+SSHStartupScripts.swift @@ -424,12 +424,8 @@ extension CMUXCLI { return scriptLines.joined(separator: "\n") } private func writeSSHStartupScript(_ scriptBody: String, remoteRelayPort: Int) throws -> String { - let scriptURL = FileManager.default.temporaryDirectory.appendingPathComponent( - "cmux-ssh-startup-\(remoteRelayPort)-\(UUID().uuidString.lowercased()).sh" - ) - let script = "#!/bin/sh\n\(scriptBody)\n" - try script.write(to: scriptURL, atomically: true, encoding: .utf8) - try FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: scriptURL.path) + let scriptURL = try SSHStartupLaunchScripts(directory: FileManager.default.temporaryDirectory) + .write(scriptBody: scriptBody, remoteRelayPort: remoteRelayPort) return shellQuote(scriptURL.path) } private func reusableShellStartupCommand( diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHStartupLaunchScripts.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHStartupLaunchScripts.swift new file mode 100644 index 000000000000..bc25d121f690 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHStartupLaunchScripts.swift @@ -0,0 +1,54 @@ +public import Foundation + +/// One-shot launcher scripts that start an SSH terminal, owned by the command +/// that writes them until a terminal takes them over. +/// +/// A launcher can carry a short-lived credential, so it lives in a file rather +/// than in the terminal's startup command, which the socket API can report. +/// A launcher deletes itself when it runs; one whose terminal never starts +/// must be removed by its owner. +/// +/// ```swift +/// let launchScripts = SSHStartupLaunchScripts(directory: FileManager.default.temporaryDirectory) +/// defer { launchScripts.removeUnlaunched() } +/// let script = try launchScripts.write(scriptBody: body, remoteRelayPort: 0) +/// // ... create the terminal that runs `script` ... +/// launchScripts.handOff() +/// ``` +public final class SSHStartupLaunchScripts { + private let directory: URL + private let fileManager: FileManager + + /// Creates an owner that writes launchers into `directory`. + /// + /// - Parameters: + /// - directory: Where launchers are written, normally the user's temporary directory. + /// - fileManager: The file manager used to write and remove launchers. + public init(directory: URL, fileManager: FileManager = FileManager()) { + self.directory = directory + self.fileManager = fileManager + } + + /// Writes an executable, owner-only launcher that runs `scriptBody` with `/bin/sh`. + /// + /// - Parameters: + /// - scriptBody: The shell script, without a shebang line. + /// - remoteRelayPort: The relay port, recorded in the file name for diagnostics. + /// - Returns: The launcher's file URL. + /// - Throws: An error when the launcher cannot be written. + public func write(scriptBody: String, remoteRelayPort: Int) throws -> URL { + let scriptURL = directory.appendingPathComponent( + "cmux-ssh-startup-\(remoteRelayPort)-\(UUID().uuidString.lowercased()).sh" + ) + let script = "#!/bin/sh\n\(scriptBody)\n" + try script.write(to: scriptURL, atomically: true, encoding: .utf8) + try fileManager.setAttributes([.posixPermissions: 0o700], ofItemAtPath: scriptURL.path) + return scriptURL + } + + /// Records that a terminal now runs every launcher written so far. + public func handOff() {} + + /// Removes every launcher that was not handed off to a terminal. + public func removeUnlaunched() {} +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHStartupLaunchScriptsTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHStartupLaunchScriptsTests.swift new file mode 100644 index 000000000000..32cf4138229c --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHStartupLaunchScriptsTests.swift @@ -0,0 +1,59 @@ +import CmuxFoundation +import Foundation +import Testing + +@Suite("SSH startup launch scripts") +struct SSHStartupLaunchScriptsTests { + private let credentialBody = "cmux_ssh_password_b64='c2VjcmV0'\nexec ssh cmux@example.test" + + private func makeDirectory() throws -> URL { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-launch-scripts-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: false) + return directory + } + + private func entries(in directory: URL) throws -> [String] { + try FileManager.default.contentsOfDirectory(atPath: directory.path) + } + + @Test("A launcher whose terminal never starts leaves no credential on disk") + func unlaunchedScriptIsRemoved() throws { + let directory = try makeDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let launchScripts = SSHStartupLaunchScripts(directory: directory) + + _ = try launchScripts.write(scriptBody: credentialBody, remoteRelayPort: 0) + // The workspace was reused, its startup command was replaced, or it + // failed to be created or configured, so nothing runs the launcher. + launchScripts.removeUnlaunched() + + #expect(try entries(in: directory).isEmpty) + } + + @Test("A launcher handed to a terminal stays until it runs and removes itself") + func handedOffScriptSurvives() throws { + let directory = try makeDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let launchScripts = SSHStartupLaunchScripts(directory: directory) + + let script = try launchScripts.write(scriptBody: credentialBody, remoteRelayPort: 0) + launchScripts.handOff() + launchScripts.removeUnlaunched() + + #expect(FileManager.default.fileExists(atPath: script.path)) + } + + @Test("A launcher is private to its owner") + func scriptIsOwnerOnly() throws { + let directory = try makeDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let launchScripts = SSHStartupLaunchScripts(directory: directory) + + let script = try launchScripts.write(scriptBody: credentialBody, remoteRelayPort: 0) + let permissions = try FileManager.default.attributesOfItem(atPath: script.path)[.posixPermissions] as? NSNumber + + #expect(permissions?.intValue == 0o700) + launchScripts.removeUnlaunched() + } +} From c256079ae64ce35579f2130263359a25c435660f Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:32:43 -0700 Subject: [PATCH 08/48] cmux vm ssh: remove SSH startup scripts no terminal will run The one-shot launcher can embed the Cloud VM password and only deletes itself when it runs. runSSHWithOptions now owns the launchers it writes: it removes them when the persistent Cloud path replaces the startup command, when a pinned workspace is reused, and when workspace creation or configuration fails, and hands them off only to a newly created workspace whose first terminal runs them. Co-Authored-By: Claude Opus 5.5 --- CLI/CMUXCLI+SSHStartupScripts.swift | 14 ++++++---- CLI/cmux.swift | 26 +++++++++++++++---- .../SSHStartupLaunchScripts.swift | 20 ++++++++++++-- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/CLI/CMUXCLI+SSHStartupScripts.swift b/CLI/CMUXCLI+SSHStartupScripts.swift index 14cf1d3b1356..97dbe0fa85e7 100644 --- a/CLI/CMUXCLI+SSHStartupScripts.swift +++ b/CLI/CMUXCLI+SSHStartupScripts.swift @@ -9,7 +9,8 @@ extension CMUXCLI { isShellSnippet: Bool = false, passwordCredential: String? = nil, controlPathPreflightShellFunction: String? = nil, - reconnectLimitDefault: Int = 20 + reconnectLimitDefault: Int = 20, + launchScripts: SSHStartupLaunchScripts ) throws -> String { let script = buildSSHStartupScriptBody( sshCommand: sshCommand, @@ -21,7 +22,7 @@ extension CMUXCLI { oneTimeCommand: nil, reconnectLimitDefault: reconnectLimitDefault ) - return try writeSSHStartupScript(script, remoteRelayPort: remoteRelayPort) + return try writeSSHStartupScript(script, remoteRelayPort: remoteRelayPort, launchScripts: launchScripts) } func buildReusableSSHStartupCommand( @@ -423,9 +424,12 @@ extension CMUXCLI { ] return scriptLines.joined(separator: "\n") } - private func writeSSHStartupScript(_ scriptBody: String, remoteRelayPort: Int) throws -> String { - let scriptURL = try SSHStartupLaunchScripts(directory: FileManager.default.temporaryDirectory) - .write(scriptBody: scriptBody, remoteRelayPort: remoteRelayPort) + private func writeSSHStartupScript( + _ scriptBody: String, + remoteRelayPort: Int, + launchScripts: SSHStartupLaunchScripts + ) throws -> String { + let scriptURL = try launchScripts.write(scriptBody: scriptBody, remoteRelayPort: remoteRelayPort) return shellQuote(scriptURL.path) } private func reusableShellStartupCommand( diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 8ea6ed0bb89c..cbff309701ad 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -12167,6 +12167,11 @@ struct CMUXCLI { options: sshOptions, localCommandScript: combinedLocalCommandScript ) + // One-shot launchers can carry the Cloud VM password. Only a new + // workspace's first terminal runs one, and it then deletes itself; + // every other exit removes them here. + let launchScripts = SSHStartupLaunchScripts(directory: FileManager.default.temporaryDirectory) + defer { launchScripts.removeUnlaunched() } var initialSSHStartupCommand: String var remoteTerminalSSHStartupCommand: String if let remoteTerminalBootstrapScript, !remoteTerminalBootstrapScript.isEmpty { @@ -12177,7 +12182,8 @@ struct CMUXCLI { remoteRelayPort: sshOptions.remoteRelayPort, localCommandScript: combinedLocalCommandScript, passwordCredential: sshOptions.passwordCredential, - controlPathPreflightShellFunction: controlPathPreflightShellFunction + controlPathPreflightShellFunction: controlPathPreflightShellFunction, + launchScripts: launchScripts ) remoteTerminalSSHStartupCommand = buildReusableBootstrapSSHStartupCommand( options: sshOptions, @@ -12195,7 +12201,8 @@ struct CMUXCLI { remoteRelayPort: sshOptions.remoteRelayPort, isShellSnippet: true, passwordCredential: sshOptions.passwordCredential, - controlPathPreflightShellFunction: controlPathPreflightShellFunction + controlPathPreflightShellFunction: controlPathPreflightShellFunction, + launchScripts: launchScripts ) remoteTerminalSSHStartupCommand = buildReusableSSHStartupCommand( sshCommand: rawRemoteCommandSnippet, @@ -12211,7 +12218,8 @@ struct CMUXCLI { shellFeatures: "", remoteRelayPort: sshOptions.remoteRelayPort, passwordCredential: sshOptions.passwordCredential, - controlPathPreflightShellFunction: controlPathPreflightShellFunction + controlPathPreflightShellFunction: controlPathPreflightShellFunction, + launchScripts: launchScripts ) remoteTerminalSSHStartupCommand = buildReusableSSHStartupCommand( sshCommand: startupRemoteTerminalSSHCommand, @@ -12246,6 +12254,8 @@ struct CMUXCLI { ) reusableTerminalStartupCommand = splitAttachCommand initialSSHStartupCommand = reusableTerminalStartupCommand; remoteTerminalSSHStartupCommand = reusableTerminalStartupCommand + // The attach loop fetches its own credential, so no terminal runs the launcher. + launchScripts.removeUnlaunched() } else { let splitAttachCommand = [ "env", @@ -12465,6 +12475,10 @@ struct CMUXCLI { } throw error } + if didCreateWorkspace { + // The new workspace's first terminal runs the launcher, which deletes itself. + launchScripts.handOff() + } var payload = configuredPayload @@ -12863,7 +12877,8 @@ struct CMUXCLI { remoteRelayPort: Int, localCommandScript: String? = nil, passwordCredential: String? = nil, - controlPathPreflightShellFunction: String? = nil + controlPathPreflightShellFunction: String? = nil, + launchScripts: SSHStartupLaunchScripts ) throws -> String { let commandSnippet = buildSSHBootstrapCommandSnippet( options: options, @@ -12876,7 +12891,8 @@ struct CMUXCLI { remoteRelayPort: remoteRelayPort, isShellSnippet: true, passwordCredential: passwordCredential, - controlPathPreflightShellFunction: controlPathPreflightShellFunction + controlPathPreflightShellFunction: controlPathPreflightShellFunction, + launchScripts: launchScripts ) } diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHStartupLaunchScripts.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHStartupLaunchScripts.swift index bc25d121f690..142cfaf3a5f6 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHStartupLaunchScripts.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHStartupLaunchScripts.swift @@ -18,6 +18,7 @@ public import Foundation public final class SSHStartupLaunchScripts { private let directory: URL private let fileManager: FileManager + private var unlaunched: [URL] = [] /// Creates an owner that writes launchers into `directory`. /// @@ -41,14 +42,29 @@ public final class SSHStartupLaunchScripts { "cmux-ssh-startup-\(remoteRelayPort)-\(UUID().uuidString.lowercased()).sh" ) let script = "#!/bin/sh\n\(scriptBody)\n" + // Track before writing so a failed permission change still removes the file. + unlaunched.append(scriptURL) try script.write(to: scriptURL, atomically: true, encoding: .utf8) try fileManager.setAttributes([.posixPermissions: 0o700], ofItemAtPath: scriptURL.path) return scriptURL } /// Records that a terminal now runs every launcher written so far. - public func handOff() {} + /// + /// Each launcher deletes itself when it runs, so ``removeUnlaunched()`` + /// leaves handed-off launchers in place. + public func handOff() { + unlaunched.removeAll() + } /// Removes every launcher that was not handed off to a terminal. - public func removeUnlaunched() {} + /// + /// Call it on every exit path of the command that wrote the launchers, + /// typically from a `defer`. + public func removeUnlaunched() { + for scriptURL in unlaunched { + try? fileManager.removeItem(at: scriptURL) + } + unlaunched.removeAll() + } } From 6d269253ca3e691ca3f253635401c75ecdf4adcb Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:31:30 -0700 Subject: [PATCH 09/48] cmux ssh relay: test that the remote CLI requires the relay to prove the token The remote cmux CLI only checks the challenge's relay_id, which the relay sends before authentication, then accepts any {"ok":true}. It also skips the handshake entirely when no relay credentials exist for a TCP address, which happens after transport cleanup removes .auth while shells keep CMUX_SOCKET_PATH. Another user who binds the forwarded port while it is down receives the CLI's commands and hook payloads. The Go tests stand up that impostor and expect the CLI to send nothing. The Swift test expects the relay's success line to carry an HMAC proof over the client's nonce, and an older client without a nonce to still authenticate. Co-Authored-By: Claude Opus 5.5 --- .../RemoteCLIRelayServerTests.swift | 64 +++++++++++ .../cli_relay_mutual_auth_test.go | 105 ++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 daemon/remote/cmd/cmuxd-remote/cli_relay_mutual_auth_test.go diff --git a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift index 4bb1309f497c..07a74b68df92 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift @@ -424,6 +424,70 @@ struct RemoteCLIRelayServerTests { ) } + @Test("the relay proves it holds the token to a client that sends its own nonce") + func relayProvesTokenToClient() throws { + let server = try RemoteCLIRelayServer( + localSocketPath: "/tmp/unused.sock", + relayID: "relay-1", + relayTokenHex: tokenHex, + commandRewriter: RecordingRelayRewriter() + ) + defer { server.stop() } + let port = try server.start() + let client = RelayTestClient(port: port) + defer { client.cancel() } + + #expect(client.wait { data, _ in data.contains(0x0A) }) + let challenge = try #require(client.receivedJSONLines().first) + let serverNonce = try #require(challenge["nonce"] as? String) + let token = try #require(RemoteCLIRelayServer.Session.hexData(from: tokenHex)) + let clientMAC = RemoteCLIRelayServer.Session.authMAC( + token: token, + message: Data("relay_id=relay-1\nnonce=\(serverNonce)\nversion=1".utf8) + ) + let clientNonce = String(repeating: "5a", count: 32) + let auth: [String: Any] = [ + "relay_id": "relay-1", + "mac": clientMAC.map { String(format: "%02x", $0) }.joined(), + "client_nonce": clientNonce, + ] + client.send(try JSONSerialization.data(withJSONObject: auth) + Data([0x0A])) + #expect(client.wait { data, _ in + String(decoding: data, as: UTF8.self).contains("\"ok\":true") + }) + + let result = try #require(client.receivedJSONLines().last) + let relayMACHex = try #require( + result["relay_mac"] as? String, + "The success line must carry the relay's proof of the token" + ) + let expectedRelayMAC = RemoteCLIRelayServer.Session.authMAC( + token: token, + message: Data( + "cmux-relay-server-proof\nrelay_id=relay-1\nclient_nonce=\(clientNonce)\nserver_nonce=\(serverNonce)\nversion=1".utf8 + ) + ) + #expect(relayMACHex == expectedRelayMAC.map { String(format: "%02x", $0) }.joined()) + #expect(relayMACHex != clientMAC.map { String(format: "%02x", $0) }.joined()) + } + + @Test("an older client without a nonce still authenticates") + func olderClientWithoutNonceAuthenticates() throws { + let server = try RemoteCLIRelayServer( + localSocketPath: "/tmp/unused.sock", + relayID: "relay-1", + relayTokenHex: tokenHex, + commandRewriter: RecordingRelayRewriter() + ) + defer { server.stop() } + let port = try server.start() + let client = RelayTestClient(port: port) + defer { client.cancel() } + + try authenticate(client) + #expect(client.receivedJSONLines().last?["relay_mac"] == nil) + } + @Test("a wrong MAC gets ok:false and the connection closed") func wrongMACRejected() throws { let unixServer = try FakeUnixSocketServer(response: Data()) diff --git a/daemon/remote/cmd/cmuxd-remote/cli_relay_mutual_auth_test.go b/daemon/remote/cmd/cmuxd-remote/cli_relay_mutual_auth_test.go new file mode 100644 index 000000000000..9f408ff24a72 --- /dev/null +++ b/daemon/remote/cmd/cmuxd-remote/cli_relay_mutual_auth_test.go @@ -0,0 +1,105 @@ +package main + +import ( + "bufio" + "encoding/json" + "net" + "os" + "strings" + "testing" + "time" +) + +// startImpostorRelay listens where the relay should be, like another remote +// user who bound the forwarded port while the SSH forward was down. It knows +// the relay ID (the relay sends it before authentication), accepts whatever +// MAC it gets, and reports every line the CLI sends after that. +func startImpostorRelay(t *testing.T, relayID string, sendChallenge bool) (string, <-chan string) { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + t.Cleanup(func() { ln.Close() }) + lines := make(chan string, 16) + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go func(conn net.Conn) { + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(5 * time.Second)) + reader := bufio.NewReader(conn) + if sendChallenge { + challenge, _ := json.Marshal(map[string]any{ + "protocol": "cmux-relay-auth", + "version": 1, + "relay_id": relayID, + "nonce": "impostor-nonce", + }) + _, _ = conn.Write(append(challenge, '\n')) + if _, err := reader.ReadString('\n'); err != nil { + return + } + _, _ = conn.Write([]byte(`{"ok":true}` + "\n")) + } + line, err := reader.ReadString('\n') + if strings.TrimSpace(line) != "" { + lines <- line + } + if err != nil { + return + } + _, _ = conn.Write([]byte(`{"id":1,"ok":true,"result":{}}` + "\n")) + }(conn) + } + }() + return ln.Addr().String(), lines +} + +func expectNothingSentToImpostor(t *testing.T, lines <-chan string) { + t.Helper() + select { + case line := <-lines: + t.Fatalf("CLI sent a request to a relay that never proved it holds the relay token: %s", strings.TrimSpace(line)) + case <-time.After(200 * time.Millisecond): + } +} + +func TestCLIRefusesRelayThatCannotProveTheToken(t *testing.T) { + relayID := "relay-impostor" + addr, lines := startImpostorRelay(t, relayID, true) + t.Setenv("HOME", t.TempDir()) + t.Setenv("CMUX_RELAY_ID", relayID) + t.Setenv("CMUX_RELAY_TOKEN", strings.Repeat("c3", 32)) + + code := runCLI([]string{"--socket", addr, "ping"}) + + expectNothingSentToImpostor(t, lines) + if code == 0 { + t.Fatal("ping must fail when the relay cannot prove it holds the relay token") + } +} + +func TestCLIRefusesTCPRelayWithoutCredentials(t *testing.T) { + // Transport cleanup removes .auth on disconnect, while shells keep + // CMUX_SOCKET_PATH=127.0.0.1:. Whoever binds the port next must not + // receive the CLI's requests. + addr, lines := startImpostorRelay(t, "", false) + home := t.TempDir() + if err := os.MkdirAll(home+"/.cmux/relay", 0o700); err != nil { + t.Fatal(err) + } + t.Setenv("HOME", home) + t.Setenv("CMUX_RELAY_ID", "") + t.Setenv("CMUX_RELAY_TOKEN", "") + + code := runCLI([]string{"--socket", addr, "ping"}) + + expectNothingSentToImpostor(t, lines) + if code == 0 { + t.Fatal("ping must fail when no relay credentials exist for a TCP relay address") + } +} From f4837edf4dffe72c560775b45a8808462e64eb59 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:35:00 -0700 Subject: [PATCH 10/48] cmux ssh relay: make the relay prove it holds the token The remote CLI now sends a 32-byte client nonce with its MAC and sends nothing else until the success line carries relay_mac, an HMAC-SHA256 with the relay token over "cmux-relay-server-proof", the relay ID, the client nonce and the server nonce. The label keeps the proof distinct from the client MAC, which starts with "relay_id=", so neither can be reflected as the other. The CLI checks it with hmac.Equal. It also refuses a TCP relay address with no credentials instead of sending the request unauthenticated: transport cleanup removes .auth while shells keep CMUX_SOCKET_PATH, so that path reached whoever bound the port. Version skew. The challenge stays v1 and still carries relay_id. - New CLI, older app: the older relay ignores client_nonce and replies {"ok":true} with no relay_mac, so the CLI fails closed with "relay did not prove it holds the relay token; reconnect". In practice the CLI for a relay port is the daemon that app uploaded, via ~/.cmux/relay/.daemon_path written next to the auth file, so this needs two app versions sharing a host through the cmuxd-remote-current fallback. - Older CLI, new app: an auth line without client_nonce gets the unchanged {"ok":true}, so it keeps working. Hiding relay_id before authentication would break those clients with no benefit to current ones: relay_id is no longer an authenticator once the relay must prove the token. The macOS Swift CLI's relay client in CLI/cmux.swift is unchanged and keeps working against the new relay; it is left for a follow-up. Co-Authored-By: Claude Opus 5.5 --- .../Relay/RemoteCLIRelayServer.swift | 7 ++ .../Relay/RemoteCLIRelaySession.swift | 50 ++++++++- daemon/remote/README.md | 1 + daemon/remote/cmd/cmuxd-remote/cli.go | 57 +++++++--- daemon/remote/cmd/cmuxd-remote/cli_test.go | 103 +++++++++++------- 5 files changed, 167 insertions(+), 51 deletions(-) diff --git a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelayServer.swift b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelayServer.swift index cec83bdf6502..2e68fdbea378 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelayServer.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelayServer.swift @@ -17,6 +17,13 @@ import Network /// failure-response delay (anti-timing-oracle), and every NSError /// domain/code/message must not change. /// +/// Mutual authentication: a client that adds a `client_nonce` to its MAC line +/// gets `relay_mac` in the success line, an HMAC over a +/// `cmux-relay-server-proof` label, the relay ID and both nonces. Current +/// remote CLIs require it before sending anything, so a listener another +/// remote user binds on the forwarded port cannot impersonate the relay. +/// Clients without a nonce get the unchanged v1 `{"ok":true}`. +/// /// Post-authentication, every command line is authorized by /// `RemoteRelayCommandPolicy` (GHSA-9vmv-3hjw-j28c): deny-by-default method /// allowlist, remote-owned workspace/surface targets only, no command-bearing diff --git a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift index 4ca35869feb4..2969b0cf6dc4 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift @@ -227,9 +227,30 @@ extension RemoteCLIRelayServer { return } + // A client that sends its own nonce requires the relay to prove it + // holds the token too, so a listener another remote user bound on + // the forwarded port cannot pose as the relay. Older clients send + // no nonce and get the plain success line. + var success: [String: Any] = ["ok": true] + if let clientNonceValue = object["client_nonce"] { + guard let clientNonce = clientNonceValue as? String, + Self.isValidClientNonce(clientNonce) else { + sendFailureAndClose() + return + } + let proof = Self.relayProofMAC( + token: relayToken, + relayID: relayID, + clientNonce: clientNonce, + serverNonce: challengeNonce, + version: challengeVersion + ) + success["relay_mac"] = proof.map { String(format: "%02x", $0) }.joined() + } + phase = .awaitingCommand armPhaseTimeout(for: .awaitingCommand) - sendJSONLine(["ok": true]) { [weak self] _ in + sendJSONLine(success) { [weak self] _ in guard let self else { return } self.queue.async { self.processBufferedLines() @@ -403,6 +424,33 @@ extension RemoteCLIRelayServer { Data("relay_id=\(relayID)\nnonce=\(nonce)\nversion=\(version)".utf8) } + /// The relay's proof of the token, returned to clients that send a + /// nonce. The leading label keeps it distinct from every client MAC, + /// whose message starts with `relay_id=`, so neither can be reflected + /// as the other. + static func relayProofMAC( + token: Data, + relayID: String, + clientNonce: String, + serverNonce: String, + version: Int + ) -> Data { + let message = Data( + "cmux-relay-server-proof\nrelay_id=\(relayID)\nclient_nonce=\(clientNonce)\nserver_nonce=\(serverNonce)\nversion=\(version)".utf8 + ) + return authMAC(token: token, message: message) + } + + /// Accepts 16 to 64 bytes of lowercase hex. + private static func isValidClientNonce(_ nonce: String) -> Bool { + guard (32...128).contains(nonce.utf8.count), + nonce.utf8.allSatisfy({ (0x30...0x39).contains($0) || (0x61...0x66).contains($0) }) + else { + return false + } + return hexData(from: nonce) != nil + } + static func authMAC(token: Data, message: Data) -> Data { let key = SymmetricKey(data: token) let code = HMAC.authenticationCode(for: message, using: key) diff --git a/daemon/remote/README.md b/daemon/remote/README.md index fdad7dd45a68..44db04aae111 100644 --- a/daemon/remote/README.md +++ b/daemon/remote/README.md @@ -151,6 +151,7 @@ For TCP addresses, the CLI dials once and only refreshes `~/.cmux/socket_addr` a Authenticated relay details: 1. Each SSH workspace gets its own relay ID and relay token. 2. The app runs a local loopback relay server that requires an HMAC-SHA256 challenge-response before forwarding a command to the real local Unix socket. + Authentication is mutual: the CLI sends its own nonce with its MAC and sends nothing further until the relay's success line carries `relay_mac`, an HMAC over a `cmux-relay-server-proof` label, the relay ID and both nonces. Another remote user who binds the forwarded port while it is down cannot produce it. The CLI also refuses a TCP relay address that has no relay credentials. The relay still answers clients that send no nonce with the plain v1 `{"ok":true}`. 3. The remote shell never gets direct access to the local app socket. It only gets the reverse-forwarded relay port plus `~/.cmux/relay/.auth`, which is written with `0600` permissions and removed when the relay stops. 4. Authentication is not authorization. `RemoteRelayCommandPolicy` rejects unlisted methods, command-bearing startup parameters, invalid selectors, and every parameter outside the selected method’s explicit schema. The app then verifies a request HMAC binding the originating workspace and active local SSH controller generation, and validates targets against its live remote terminal identities (`RemoteRelayAuthorizationPolicy`); aliases translate IDs but do not grant ownership. Local/browser panels in a remote workspace are excluded. Dispatch rechecks the controller generation and live ownership before acting; replacing or retiring the controller invalidates previously admitted requests. Input, close, scrollback, and selection reads also recheck the actual terminal target, and relay reads bypass cached topology responses. `surface.split` is withheld because its local fallback can spawn a Mac PTY. `surface.create`, `pane.create`, `surface.respawn`, `surface.send_key`, workspace/window/group creation, and global listing/navigation methods are denied. The `surface.resume.*` methods are not relay methods: a binding carries a command that would run on the Mac, and the app also refuses any relay-origin binding (manaflow-ai/cmux#14907). `notification.create_for_target` accepts no `reply_shape`; the app delivers a relayed notification with the relay origin, no reply, and the remote destination in its title. For a relay caller, `workspace.remote.status` and the `terminal_session_*` lifecycle methods return only `enabled`, `state`, and `connected` in `remote`. `agent.hook.enqueue` admits only Claude lifecycle events (`session-start`, `prompt-submit`, `stop`, `notification`, `session-end`, `pre-tool-use`) with `relay_backed: true` and an owned `workspace_id`/`surface_id`; the app rebuilds the hook environment from those selectors and drops host paths from the payload. `agent.hook.barrier` and decision hooks stay unavailable. Relay-side denials return `remote_relay_denied`; app-side ownership denials return `remote_relay_*_denied` without executing the requested operation. diff --git a/daemon/remote/cmd/cmuxd-remote/cli.go b/daemon/remote/cmd/cmuxd-remote/cli.go index d52e6cdcc8f4..ff64c5382fcf 100644 --- a/daemon/remote/cmd/cmuxd-remote/cli.go +++ b/daemon/remote/cmd/cmuxd-remote/cli.go @@ -1122,15 +1122,21 @@ func dialSocketUntil(addr string, refreshAddr func() string, deadline time.Time) if err != nil { return nil, err } - if auth := currentRelayAuth(connectedAddr); auth != nil { - authDeadline := deadline - if authDeadline.IsZero() { - authDeadline = time.Now().Add(5 * time.Second) - } - if err := authenticateRelayConnUntil(conn, auth, authDeadline); err != nil { - conn.Close() - return nil, err - } + // A TCP address is a forwarded relay port that another remote user + // can bind while the forward is down, so never talk to it without + // credentials that let the relay prove itself. + auth := currentRelayAuth(connectedAddr) + if auth == nil { + conn.Close() + return nil, errors.New("no relay credentials for this address; reconnect this SSH workspace") + } + authDeadline := deadline + if authDeadline.IsZero() { + authDeadline = time.Now().Add(5 * time.Second) + } + if err := authenticateRelayConnUntil(conn, auth, authDeadline); err != nil { + conn.Close() + return nil, err } return conn, nil } @@ -1199,13 +1205,19 @@ func authenticateRelayConnUntil(conn net.Conn, auth *relayAuthState, deadline ti } tokenBytes, err := hex.DecodeString(auth.RelayToken) - if err != nil { + if err != nil || len(tokenBytes) == 0 { return fmt.Errorf("invalid relay auth token") } + clientNonceBytes := make([]byte, 32) + if _, err := rand.Read(clientNonceBytes); err != nil { + return fmt.Errorf("failed to create relay auth nonce: %w", err) + } + clientNonce := hex.EncodeToString(clientNonceBytes) mac := computeRelayMAC(tokenBytes, auth.RelayID, challenge.Nonce, challenge.Version) payload, err := json.Marshal(map[string]any{ - "relay_id": auth.RelayID, - "mac": hex.EncodeToString(mac), + "relay_id": auth.RelayID, + "mac": hex.EncodeToString(mac), + "client_nonce": clientNonce, }) if err != nil { return fmt.Errorf("failed to encode relay auth response: %w", err) @@ -1219,7 +1231,8 @@ func authenticateRelayConnUntil(conn net.Conn, auth *relayAuthState, deadline ti return fmt.Errorf("failed to read relay auth result: %w", err) } var result struct { - OK bool `json:"ok"` + OK bool `json:"ok"` + RelayMAC string `json:"relay_mac"` } if err := json.Unmarshal([]byte(line), &result); err != nil { return fmt.Errorf("invalid relay auth result") @@ -1227,10 +1240,28 @@ func authenticateRelayConnUntil(conn net.Conn, auth *relayAuthState, deadline ti if !result.OK { return fmt.Errorf("relay auth rejected") } + // Anyone who connected once has seen the relay ID, so only a proof over + // this client's nonce shows the listener holds the relay token. + receivedProof, err := hex.DecodeString(result.RelayMAC) + expectedProof := computeRelayProofMAC(tokenBytes, auth.RelayID, clientNonce, challenge.Nonce, challenge.Version) + if err != nil || !hmac.Equal(receivedProof, expectedProof) { + return fmt.Errorf("relay did not prove it holds the relay token; reconnect this SSH workspace") + } _ = conn.SetDeadline(time.Time{}) return nil } +// computeRelayProofMAC is the relay's answer to the client nonce. Its label +// keeps it distinct from the client MAC, whose message starts with "relay_id=". +func computeRelayProofMAC(token []byte, relayID, clientNonce, serverNonce string, version int) []byte { + mac := hmac.New(sha256.New, token) + _, _ = io.WriteString(mac, fmt.Sprintf( + "cmux-relay-server-proof\nrelay_id=%s\nclient_nonce=%s\nserver_nonce=%s\nversion=%d", + relayID, clientNonce, serverNonce, version, + )) + return mac.Sum(nil) +} + func computeRelayMAC(token []byte, relayID, nonce string, version int) []byte { mac := hmac.New(sha256.New, token) _, _ = io.WriteString(mac, fmt.Sprintf("relay_id=%s\nnonce=%s\nversion=%d", relayID, nonce, version)) diff --git a/daemon/remote/cmd/cmuxd-remote/cli_test.go b/daemon/remote/cmd/cmuxd-remote/cli_test.go index 308475ea9b6f..e3e4d925ec31 100644 --- a/daemon/remote/cmd/cmuxd-remote/cli_test.go +++ b/daemon/remote/cmd/cmuxd-remote/cli_test.go @@ -169,6 +169,7 @@ func startMockV2SocketWithRequestCapture(t *testing.T) (string, <-chan map[strin func startMockV2TCPSocketWithResult(t *testing.T, result any) string { t.Helper() + relayID, token := useMockRelayCredentials(t) ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatalf("failed to listen on TCP: %v", err) @@ -183,13 +184,16 @@ func startMockV2TCPSocketWithResult(t *testing.T, result any) string { } go func(conn net.Conn) { defer conn.Close() - buf := make([]byte, 4096) - n, _ := conn.Read(buf) - if n == 0 { + reader := bufio.NewReader(conn) + if !serveMockRelayHandshake(conn, reader, relayID, token) { + return + } + line, _ := reader.ReadBytes('\n') + if len(line) == 0 { return } var req map[string]any - if err := json.Unmarshal(buf[:n], &req); err != nil { + if err := json.Unmarshal(line, &req); err != nil { _, _ = conn.Write([]byte(`{"ok":false,"error":{"code":"parse","message":"bad json"}}` + "\n")) return } @@ -250,41 +254,9 @@ func startMockAuthenticatedTCPSocket(t *testing.T, relayID, relayToken, response } go func(conn net.Conn) { defer conn.Close() - nonce := "testnonce" - challenge, _ := json.Marshal(map[string]any{ - "protocol": "cmux-relay-auth", - "version": 1, - "relay_id": relayID, - "nonce": nonce, - }) - _, _ = conn.Write(append(challenge, '\n')) - - reader := bufio.NewReader(conn) - line, err := reader.ReadString('\n') - if err != nil { - return - } - var authResp map[string]any - if err := json.Unmarshal([]byte(line), &authResp); err != nil { - _, _ = conn.Write([]byte(`{"ok":false}` + "\n")) - return - } - macHex, _ := authResp["mac"].(string) - receivedMAC, err := hex.DecodeString(macHex) - if err != nil { - _, _ = conn.Write([]byte(`{"ok":false}` + "\n")) - return - } - - h := hmac.New(sha256.New, relayTokenBytes) - _, _ = io.WriteString(h, fmt.Sprintf("relay_id=%s\nnonce=%s\nversion=%d", relayID, nonce, 1)) - expectedMAC := h.Sum(nil) - if !hmac.Equal(receivedMAC, expectedMAC) { - _, _ = conn.Write([]byte(`{"ok":false}` + "\n")) + if !serveMockRelayHandshake(conn, bufio.NewReader(conn), relayID, relayTokenBytes) { return } - - _, _ = conn.Write([]byte(`{"ok":true}` + "\n")) buf := make([]byte, 4096) n, _ := conn.Read(buf) _, _ = conn.Write([]byte(response)) @@ -298,6 +270,59 @@ func startMockAuthenticatedTCPSocket(t *testing.T, relayID, relayToken, response return ln.Addr().String() } +// serveMockRelayHandshake plays the app relay's side of the handshake: it +// checks the client MAC and, when the client sends a nonce, proves the token. +func serveMockRelayHandshake(conn net.Conn, reader *bufio.Reader, relayID string, token []byte) bool { + nonce := "testnonce" + challenge, _ := json.Marshal(map[string]any{ + "protocol": "cmux-relay-auth", + "version": 1, + "relay_id": relayID, + "nonce": nonce, + }) + _, _ = conn.Write(append(challenge, '\n')) + + line, err := reader.ReadString('\n') + if err != nil { + return false + } + var authResp map[string]any + if err := json.Unmarshal([]byte(line), &authResp); err != nil { + _, _ = conn.Write([]byte(`{"ok":false}` + "\n")) + return false + } + macHex, _ := authResp["mac"].(string) + receivedMAC, err := hex.DecodeString(macHex) + if err != nil { + _, _ = conn.Write([]byte(`{"ok":false}` + "\n")) + return false + } + h := hmac.New(sha256.New, token) + _, _ = io.WriteString(h, fmt.Sprintf("relay_id=%s\nnonce=%s\nversion=%d", relayID, nonce, 1)) + if !hmac.Equal(receivedMAC, h.Sum(nil)) { + _, _ = conn.Write([]byte(`{"ok":false}` + "\n")) + return false + } + result := map[string]any{"ok": true} + if clientNonce, _ := authResp["client_nonce"].(string); clientNonce != "" { + result["relay_mac"] = hex.EncodeToString(computeRelayProofMAC(token, relayID, clientNonce, nonce, 1)) + } + payload, _ := json.Marshal(result) + _, _ = conn.Write(append(payload, '\n')) + return true +} + +// useMockRelayCredentials points the CLI at the credentials the mock TCP +// relays below accept. +func useMockRelayCredentials(t *testing.T) (string, []byte) { + t.Helper() + relayID := "relay-mock" + token := strings.Repeat("d4", 32) + t.Setenv("CMUX_RELAY_ID", relayID) + t.Setenv("CMUX_RELAY_TOKEN", token) + return relayID, mustHex(t, token) +} + func mustHex(t *testing.T, value string) []byte { t.Helper() data, err := hex.DecodeString(value) @@ -320,6 +345,7 @@ func TestDialSocketRefreshesToUpdatedTCPAddressWithoutPolling(t *testing.T) { t.Fatalf("listen ready: %v", err) } defer readyListener.Close() + relayID, token := useMockRelayCredentials(t) accepted := make(chan struct{}) go func() { @@ -328,6 +354,7 @@ func TestDialSocketRefreshesToUpdatedTCPAddressWithoutPolling(t *testing.T) { if acceptErr != nil { return } + serveMockRelayHandshake(conn, bufio.NewReader(conn), relayID, token) conn.Close() }() @@ -449,10 +476,12 @@ func TestDialSocketDetection(t *testing.T) { t.Fatalf("listen: %v", err) } defer ln.Close() + relayID, token := useMockRelayCredentials(t) go func() { conn, _ := ln.Accept() if conn != nil { + serveMockRelayHandshake(conn, bufio.NewReader(conn), relayID, token) conn.Close() } }() From 4c6a00242d7c6b5bc1f9ee62d20283ec85d0ae82 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:35:30 -0700 Subject: [PATCH 11/48] cmux ssh relay: test that idle pre-auth connections cannot block the relay The relay's 16-session cap counts connections that have not authenticated, and each may sit for up to 10 seconds. Any remote user who holds 16 idle connections to the forwarded port locks out the workspace's own CLI and hooks. The test opens 64 idle connections, then expects a genuine client to still get a challenge, authenticate and round-trip a command. Co-Authored-By: Claude Opus 5.5 --- .../RemoteCLIRelayServerTests.swift | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift index 07a74b68df92..e53da61de1b1 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift @@ -254,6 +254,47 @@ struct RemoteCLIRelayServerTests { ) } + @Test("idle unauthenticated connections cannot lock out the relay's own client") + func idleUnauthenticatedConnectionsDoNotStarveAuthenticatedClient() throws { + let unixServer = try FakeUnixSocketServer(response: Data("{\"ok\":true,\"result\":42}\n".utf8)) + defer { unixServer.close() } + let server = try RemoteCLIRelayServer( + localSocketPath: unixServer.path, + relayID: "relay-1", + relayTokenHex: tokenHex, + commandRewriter: RecordingRelayRewriter() + ) + defer { server.stop() } + let port = try server.start() + var idleClients: [RelayTestClient] = [] + defer { + for client in idleClients { + client.cancel() + } + } + + // Another remote user opens more idle connections than any budget + // and never authenticates. + for _ in 0..<64 { + let client = RelayTestClient(port: port) + idleClients.append(client) + #expect(client.wait { data, closed in data.contains(0x0A) || closed }) + } + + let client = RelayTestClient(port: port) + defer { client.cancel() } + #expect( + client.wait { data, _ in data.contains(0x0A) }, + "The relay's own client must still receive a challenge" + ) + guard client.receivedJSONLines().first?["nonce"] is String else { return } + try authenticate(client) + client.send(Data((#"{"id":"relay-test","method":"system.ping","params":{}}"# + "\n").utf8)) + #expect(client.wait { data, closed in + String(decoding: data, as: UTF8.self).contains("\"result\":42") && closed + }) + } + @Test("unauthenticated relay sessions expire and release capacity") func unauthenticatedSessionsExpireAndReleaseCapacity() throws { let clock = ManualRetryClock() From 15a091626ab218348845f8f08f2b1d846b6bdd33 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:37:25 -0700 Subject: [PATCH 12/48] cmux ssh relay: give unauthenticated connections their own budget Connections that have not authenticated no longer count against the 16 authenticated sessions. They get a separate budget of 32, and when it is full a new connection evicts the oldest pending one, so idle connections from another remote user cannot keep the workspace's own CLI from getting a challenge. A session moves to the authenticated budget only after its MAC verifies; if that budget is full it gets the usual {"ok":false} after the 50ms failure floor. The pre-auth deadline drops from 10s to 5s, matching the CLI's own dial-and-handshake budget; the post-auth command deadline stays at 10s. Co-Authored-By: Claude Opus 5.5 --- .../Relay/RemoteCLIRelayServer.swift | 43 +++++++++++++-- .../Relay/RemoteCLIRelaySession.swift | 16 +++++- .../RemoteCLIRelayServerTests.swift | 52 +++++++++++++++++-- 3 files changed, 101 insertions(+), 10 deletions(-) diff --git a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelayServer.swift b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelayServer.swift index 2e68fdbea378..adae39cb313b 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelayServer.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelayServer.swift @@ -42,8 +42,13 @@ import Network /// argument. The actor/async migration is a deliberate later-phase item /// (plan: "Modernization hot-spots"). public final class RemoteCLIRelayServer: @unchecked Sendable { - /// Bounds authenticated and pre-auth relay work, including local socket waits. + /// Bounds authenticated relay work, including local socket waits. static let maximumConcurrentSessions = 16 + /// Bounds connections that have not authenticated yet. Anyone on the + /// remote host can open these, so they have their own budget and, when + /// it is full, a new connection evicts the oldest one. Idle connections + /// therefore cannot keep the workspace's own CLI from authenticating. + static let maximumPendingAuthSessions = 32 private let localSocketPath: String private let relayID: String @@ -55,6 +60,9 @@ public final class RemoteCLIRelayServer: @unchecked Sendable { private var listener: NWListener? private var sessions: [UUID: Session] = [:] + /// Unauthenticated session IDs, oldest first. + private var pendingAuthSessionIDs: [UUID] = [] + private var authenticatedSessionIDs: Set = [] private var isStopped = false private var localPort: Int? private var workspaceAliases: [UUID: UUID] = [:] @@ -180,6 +188,8 @@ public final class RemoteCLIRelayServer: @unchecked Sendable { localPort = nil let activeSessions = sessions.values sessions.removeAll() + pendingAuthSessionIDs.removeAll() + authenticatedSessionIDs.removeAll() for session in activeSessions { session.stop() } @@ -196,11 +206,14 @@ public final class RemoteCLIRelayServer: @unchecked Sendable { } private func acceptConnectionLocked(_ connection: NWConnection) { - guard !isStopped, - sessions.count < Self.maximumConcurrentSessions else { + guard !isStopped else { connection.cancel() return } + while pendingAuthSessionIDs.count >= Self.maximumPendingAuthSessions { + let oldestID = pendingAuthSessionIDs.removeFirst() + sessions.removeValue(forKey: oldestID)?.stop() + } let sessionID = UUID() let session = Session( connection: connection, @@ -211,15 +224,37 @@ public final class RemoteCLIRelayServer: @unchecked Sendable { commandEvaluator: { [weak self] commandLine in self?.evaluateCommandLineLocked(commandLine) ?? .deny("relay authorization is unavailable") }, + admitAuthenticated: { [weak self] in + self?.admitAuthenticatedSessionLocked(sessionID) ?? false + }, queue: queue, clock: clock ) { [weak self] in - self?.sessions.removeValue(forKey: sessionID) + self?.forgetSessionLocked(sessionID) } sessions[sessionID] = session + pendingAuthSessionIDs.append(sessionID) session.start() } + /// Moves a session that passed the handshake from the pre-auth budget + /// to the authenticated one, or refuses it when that budget is full. + private func admitAuthenticatedSessionLocked(_ sessionID: UUID) -> Bool { + guard sessions[sessionID] != nil, + authenticatedSessionIDs.count < Self.maximumConcurrentSessions else { + return false + } + pendingAuthSessionIDs.removeAll { $0 == sessionID } + authenticatedSessionIDs.insert(sessionID) + return true + } + + private func forgetSessionLocked(_ sessionID: UUID) { + sessions.removeValue(forKey: sessionID) + pendingAuthSessionIDs.removeAll { $0 == sessionID } + authenticatedSessionIDs.remove(sessionID) + } + /// Applies the remote-relay authorization policy first; only allowed /// commands reach the app's alias-aware rewriter and the local socket. private func evaluateCommandLineLocked(_ commandLine: Data) -> CommandDisposition { diff --git a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift index 2969b0cf6dc4..e713b27e7e03 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift @@ -66,6 +66,10 @@ extension RemoteCLIRelayServer { case closed } + /// The challenge needs one round trip over SSH, and the CLI gives the + /// whole dial and handshake five seconds, so a connection that has + /// not authenticated by then only holds a pre-auth slot. + private static let preAuthTimeoutMilliseconds = 5_000 private static let handshakeTimeoutMilliseconds = 10_000 private let connection: NWConnection @@ -74,6 +78,7 @@ extension RemoteCLIRelayServer { private let relayID: String private let relayToken: Data private let commandEvaluator: (Data) -> CommandDisposition + private let admitAuthenticated: () -> Bool private let queue: DispatchQueue private let clock: any RemoteProxyRetryClock private let onClose: () -> Void @@ -99,6 +104,7 @@ extension RemoteCLIRelayServer { relayID: String, relayToken: Data, commandEvaluator: @escaping (Data) -> CommandDisposition, + admitAuthenticated: @escaping () -> Bool, queue: DispatchQueue, clock: any RemoteProxyRetryClock, onClose: @escaping () -> Void @@ -109,6 +115,7 @@ extension RemoteCLIRelayServer { self.relayID = relayID self.relayToken = relayToken self.commandEvaluator = commandEvaluator + self.admitAuthenticated = admitAuthenticated self.queue = queue self.clock = clock self.onClose = onClose @@ -248,6 +255,10 @@ extension RemoteCLIRelayServer { success["relay_mac"] = proof.map { String(format: "%02x", $0) }.joined() } + guard admitAuthenticated() else { + sendFailureAndClose() + return + } phase = .awaitingCommand armPhaseTimeout(for: .awaitingCommand) sendJSONLine(success) { [weak self] _ in @@ -374,9 +385,12 @@ extension RemoteCLIRelayServer { private func armPhaseTimeout(for expectedPhase: Phase) { phaseTimeoutTask?.cancel() + let timeoutMilliseconds = expectedPhase == .awaitingAuth + ? Self.preAuthTimeoutMilliseconds + : Self.handshakeTimeoutMilliseconds phaseTimeoutTask = Task { [weak self, clock] in guard (try? await clock.sleep( - forMilliseconds: Self.handshakeTimeoutMilliseconds + forMilliseconds: timeoutMilliseconds )) != nil else { return } diff --git a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift index e53da61de1b1..31922c49bbb9 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift @@ -222,8 +222,8 @@ private final class RelayTestClient: @unchecked Sendable { struct RemoteCLIRelayServerTests { private let tokenHex = "00112233445566778899aabbccddeeff" - @Test("relay sessions are capacity bounded") - func relaySessionsAreCapacityBounded() throws { + @Test("authenticated relay sessions are capacity bounded") + func authenticatedSessionsAreCapacityBounded() throws { let server = try RemoteCLIRelayServer( localSocketPath: "/tmp/unused.sock", relayID: "relay-1", @@ -240,18 +240,60 @@ struct RemoteCLIRelayServerTests { } let expectedSessionCapacity = 16 + #expect(RemoteCLIRelayServer.maximumConcurrentSessions == expectedSessionCapacity) for _ in 0.. Date: Tue, 29 Sep 2026 17:50:01 -0700 Subject: [PATCH 13/48] cmux ssh: test that differently routed connections do not share a master OpenSSH's %C hashes only the endpoint, so two routes to the same user@host:port that differ in IdentityAgent or ForwardAgent resolve to one cmux socket, and routes that differ in ProxyCommand or IdentityFile lose sharing entirely. These tests describe the intended behavior: a separate, cmux-owned master per route. Co-Authored-By: Claude Opus 5.5 --- .../SSHRouteSpecificControlPathTests.swift | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHRouteSpecificControlPathTests.swift diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHRouteSpecificControlPathTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHRouteSpecificControlPathTests.swift new file mode 100644 index 000000000000..777eb255bed3 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHRouteSpecificControlPathTests.swift @@ -0,0 +1,91 @@ +import Foundation +import Testing +@testable import CmuxFoundation + +/// Two routes to the same `user@host:port` must not share one ControlMaster +/// when their security-relevant options differ, because OpenSSH's `%C` hashes +/// only the endpoint. +@Suite("SSH route-specific control paths") +struct SSHRouteSpecificControlPathTests { + private let socketDirectory = "/Users/alice/.cmux/ssh" + private var options: SSHConnectionSharingOptions { + SSHConnectionSharingOptions( + userID: 501, + controlSocketDirectoryPath: socketDirectory, + authenticationLockDirectoryPath: "/private/var/folders/cmux-tests" + ) + } + + /// `ssh -G` output for one endpoint plus route-specific lines. + private func resolvedConfiguration(hostname: String = "10.0.0.5", _ routeLines: [String]) -> String { + ([ + "user alice", + "hostname \(hostname)", + "port 22", + "controlmaster false", + "controlpersist no", + "forwardagent no", + ] + routeLines).joined(separator: "\n") + } + + /// The cmux-owned socket the CLI would hand to every later SSH command. + private func sharedControlPath(hostname: String = "10.0.0.5", _ routeLines: [String]) -> String? { + let configured = options.userConfiguredControlOptions( + fromSSHConfigOutput: resolvedConfiguration(hostname: hostname, routeLines), + baselineSSHConfigOutput: resolvedConfiguration(hostname: hostname, []), + explicitOptions: [] + ) + let merged = options.mergingDefaults(into: [], userConfiguredControlOptions: configured) + // Later helpers re-merge the serialized options; the route must survive. + #expect(options.mergingDefaults(into: merged) == merged) + return options.cmuxOwnedControlPath(in: merged) + } + + @Test("Routes that differ only in one security-relevant option use different masters", arguments: [ + (["proxycommand /usr/local/bin/network-a %h %p"], ["proxycommand /usr/local/bin/network-b %h %p"]), + (["identityfile /Users/alice/.ssh/restricted"], ["identityfile /Users/alice/.ssh/admin"]), + (["identityagent /Users/alice/.ssh/restricted-agent.sock"], ["identityagent /Users/alice/.ssh/agent.sock"]), + (["forwardagent yes"], ["forwardagent no", "identitiesonly yes"]), + ]) + func differingRoutesDoNotShareAMaster(first: [String], second: [String]) throws { + let firstPath = try #require(sharedControlPath(first)) + let secondPath = try #require(sharedControlPath(second)) + + #expect(firstPath != secondPath) + #expect(firstPath != "\(socketDirectory)/%C") + #expect(secondPath != "\(socketDirectory)/%C") + #expect(sharedControlPath(first) == firstPath) + } + + @Test("The same route options on different hosts use different masters") + func sameRouteOptionsOnDifferentHostsDoNotShare() throws { + let route = ["proxycommand /usr/local/bin/broker %h %p"] + let first = try #require(sharedControlPath(hostname: "10.0.0.5", route)) + let second = try #require(sharedControlPath(hostname: "10.0.0.6", route)) + #expect(first != second) + } + + @Test("Route-specific socket names are recognized as cmux-owned") + func routeSpecificNamesAreRecognized() throws { + let path = try #require(sharedControlPath(["proxycommand /usr/local/bin/broker %h %p"])) + + #expect(path.hasPrefix("\(socketDirectory)/")) + #expect(!path.contains("%")) + #expect(options.cmuxOwnedControlPath(in: ["ControlMaster=auto", "ControlPath=\(path)"]) == path) + #expect(options.resolvedControlMasterAuthenticationLockPath(controlPath: path) != nil) + #expect(options.resolvedControlMasterOwnershipLockPath(controlPath: path) != nil) + #expect(try shellPatternMatches(path)) + #expect(try !shellPatternMatches("/Users/alice/.ssh/" + URL(fileURLWithPath: path).lastPathComponent)) + } + + /// Runs the cleanup scripts' `case` pattern against `path`. + private func shellPatternMatches(_ path: String) throws -> Bool { + let pattern = try #require(options.resolvedControlPathShellPattern) + let process = Process() + process.executableURL = URL(fileURLWithPath: "/bin/sh") + process.arguments = ["-c", "case \"$1\" in \(pattern)) exit 0 ;; esac; exit 1", "sh", path] + try process.run() + process.waitUntilExit() + return process.terminationStatus == 0 + } +} From 887f0aea7f25c043bca573d01c9017668c212f1b Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:56:52 -0700 Subject: [PATCH 14/48] cmux ssh: give each security-relevant route its own control socket OpenSSH's %C hashes only local host, HostName, port, user and (on newer releases) ProxyJump. Routes to one endpoint that differ in IdentityAgent, ForwardAgent, IdentitiesOnly or similar options shared a cmux master, so a session meant for a restricted agent could ride one authenticated with another. Routes whose ProxyCommand, IdentityFile or host-key policy differed were isolated only by turning sharing off. The route-sensitive key list now includes the agent and key-source options, and a route-sensitive connection gets a socket named by a SHA-256 digest of its resolved ssh -G route: the endpoint plus every route-sensitive value, so ssh_config and explicit -o/-i values count alike. The name keeps %C's 40 lowercase hex characters, so the sun_path budget and every recognizer of cmux-owned sockets (Swift checks, shell case patterns, lock and broker keys) are unchanged. Without a resolved route, sharing stays off as before, and a user-supplied ControlPath is still left alone. Co-Authored-By: Claude Opus 5.5 --- CLI/cmux.swift | 7 +- .../SSHConnectionSharingOptions.swift | 98 +++++++++++++++++-- .../SSHConnectionSharingOptionsTests.swift | 3 +- .../SSHRouteSpecificControlPathTests.swift | 27 +++++ 4 files changed, 124 insertions(+), 11 deletions(-) diff --git a/CLI/cmux.swift b/CLI/cmux.swift index cbff309701ad..655a1b412e9f 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -12059,7 +12059,12 @@ struct CMUXCLI { explicitOptions: inputSSHOptions.sshOptions ) }, - routeSensitiveOptions: inputSSHOptions.identityFile.map { ["IdentityFile=\($0)"] } ?? [] + routeSensitiveOptions: inputSSHOptions.identityFile.map { ["IdentityFile=\($0)"] } ?? [], + // `%C` ignores proxy, identity and host-key options; a route that + // sets them gets a master keyed by its whole resolved route. + routeIdentifier: resolvedUserSSHConfiguration.flatMap { + sharingOptions.routeIdentifier(fromSSHConfigOutput: $0) + } ) if resolvedUserSSHConfiguration != nil { sshOptions.sshOptions = resolvedCmuxControlPathOptions(for: sshOptions) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHConnectionSharingOptions.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHConnectionSharingOptions.swift index 90aa46837e16..a1e6c8050e50 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHConnectionSharingOptions.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHConnectionSharingOptions.swift @@ -10,6 +10,11 @@ internal import Foundation /// endpoints. Workspace relay ports deliberately do not participate in the /// path: reverse forwards are individual channels on the shared master. /// +/// `%C` ignores how a connection reaches and authenticates to that endpoint. +/// A route with security-relevant options (proxy, identity, agent, host-key +/// policy and similar) therefore gets its own socket, named by a digest of +/// its resolved route, or shares nothing when no resolved route is known. +/// /// The sockets live in `~/.cmux/ssh`. OpenSSH trusts whatever socket is at /// `ControlPath`, so when no directory only this user can write to is /// available, cmux adds no sharing defaults and the user's SSH configuration @@ -21,7 +26,15 @@ public struct SSHConnectionSharingOptions: Sendable { /// no connections because no private directory is available. public let controlSocketDirectoryPath: String? private let authenticationLockDirectory: URL - private static let routeSensitiveMarker = "__cmux_route_sensitive=true" + /// Private option key carrying a resolved route's identity, or `true` + /// when the route is sensitive but no identity could be resolved. + private static let routeSensitiveMarkerKey = "__cmux_route_sensitive" + /// Resolved `ssh -G` keys naming the endpoint a route reaches. `proxyjump` + /// is here because older OpenSSH releases leave it out of `%C`. + private static let routeEndpointKeys: Set = ["user", "hostname", "port", "proxyjump"] + /// Options that change how a connection reaches or authenticates to its + /// endpoint, or what a session on the master can do there. `%C` ignores + /// all of them, so routes that differ in one must not share a master. private static let routeSensitiveKeys: Set = [ "proxycommand", "proxyjump", "identityfile", "certificatefile", "hostkeyalias", "hostkeyalgorithms", "hostbasedacceptedalgorithms", @@ -36,6 +49,11 @@ public struct SSHConnectionSharingOptions: Sendable { "addressfamily", "bindaddress", "bindinterface", "localaddress", "gssapiauthentication", "gssapikexalgorithms", "gssapiserveridentity", "gssapidelegatecredentials", "kerberosauthentication", "kerberosorlocalpasswd", + // The master's agent and key sources authenticate the connection every + // multiplexed session rides, and agent forwarding exposes that agent + // to the remote host. + "identityagent", "identitiesonly", "pkcs11provider", "securitykeyprovider", + "forwardagent", "forwardx11trusted", "proxyusefdpass", ] /// Creates an option merger for the current local user, creating @@ -120,9 +138,11 @@ public struct SSHConnectionSharingOptions: Sendable { /// ``userConfiguredControlOptions(fromSSHConfigOutput:explicitOptions:)``. /// - routeSensitiveOptions: Values that make the route-specific socket /// necessary when a route identifier is available. - /// - routeIdentifier: Stable opaque identity for the complete route. - /// Route-sensitive options use a private socket derived from this - /// value; without it they remain unshared. + /// - routeIdentifier: Stable opaque identity for the complete route, such + /// as ``routeIdentifier(fromSSHConfigOutput:)``. Route-sensitive + /// options use a private socket derived from this value, or else from + /// the route resolved into `userConfiguredControlOptions`; without + /// either they remain unshared. /// - Returns: Effective explicit options for native SSH commands. public func mergingDefaults( into options: [String], @@ -131,15 +151,23 @@ public struct SSHConnectionSharingOptions: Sendable { routeIdentifier: String? = nil ) -> [String] { let resolver = SSHAgentSocketResolver() + let routeMarkerValue = resolver.optionValue( + named: Self.routeSensitiveMarkerKey, + in: userConfiguredControlOptions ?? [] + ) let routeSensitive = !routeSensitiveOptions.isEmpty || options.contains { option in guard let key = resolver.optionKey(option) else { return false } return Self.routeSensitiveKeys.contains(key) } - || userConfiguredControlOptions?.contains(where: { SSHAgentSocketResolver().optionKey($0) == Self.routeSensitiveMarker.split(separator: "=").first.map(String.init) }) == true + || routeMarkerValue != nil + // A caller's identity wins; otherwise use the route `ssh -G` resolved. + // The `true` placeholder means no identity is known. + let effectiveRouteIdentifier = routeIdentifier + ?? routeMarkerValue.flatMap { Self.isRouteDigest($0) ? $0 : nil } var merged = options.compactMap { option -> String? in let trimmed = option.trimmingCharacters(in: .whitespacesAndNewlines) - guard !trimmed.isEmpty, SSHAgentSocketResolver().optionKey(trimmed) != Self.routeSensitiveMarker.split(separator: "=").first.map(String.init) else { return nil } + guard !trimmed.isEmpty, resolver.optionKey(trimmed) != Self.routeSensitiveMarkerKey else { return nil } return trimmed } let controlKeys = ["ControlMaster", "ControlPath", "ControlPersist"] @@ -189,8 +217,8 @@ public struct SSHConnectionSharingOptions: Sendable { } return merged } - if let routeIdentifier, - let routeControlPath = routeSpecificControlPath(for: routeIdentifier) { + if let effectiveRouteIdentifier, + let routeControlPath = routeSpecificControlPath(for: effectiveRouteIdentifier) { if controlMaster == nil { merged.append("ControlMaster=auto") } @@ -241,7 +269,56 @@ public struct SSHConnectionSharingOptions: Sendable { return merged } + /// Returns a stable identity for the complete route `ssh -G` resolved. + /// + /// The identity covers the endpoint (`user`, `hostname`, `port`, + /// `proxyjump`) and every security-relevant option in OpenSSH's resolved + /// form, so explicit `-o`/`-i` values and ssh_config values count alike. + /// Aliases that resolve to the same route share an identity. Pass it as + /// `routeIdentifier` to + /// ``mergingDefaults(into:userConfiguredControlOptions:routeSensitiveOptions:routeIdentifier:)``. + /// + /// - Parameter output: Standard output from `ssh -G ` run + /// with the caller's explicit options. + /// - Returns: A lowercase hex digest, or `nil` when the output names no host. + public func routeIdentifier(fromSSHConfigOutput output: String) -> String? { + var entries: [(key: String, value: String)] = [] + for line in output.split(whereSeparator: \.isNewline) { + let parts = line.split(maxSplits: 1, whereSeparator: \.isWhitespace) + guard parts.count == 2 else { continue } + let key = parts[0].lowercased() + guard Self.routeEndpointKeys.contains(key) || Self.routeSensitiveKeys.contains(key) else { + continue + } + entries.append((key, parts[1].trimmingCharacters(in: .whitespacesAndNewlines))) + } + guard entries.contains(where: { $0.key == "hostname" && !$0.value.isEmpty }) else { + return nil + } + // Sort by key only: repeated keys such as `identityfile` keep the + // order OpenSSH tries them in. + let canonical = entries.enumerated() + .sorted { ($0.element.key, $0.offset) < ($1.element.key, $1.offset) } + .map { "\($0.element.key) \($0.element.value)" } + .joined(separator: "\n") + return SHA256.hash(data: Data(("cmux-ssh-route-v1\n" + canonical).utf8)) + .map { String(format: "%02x", $0) } + .joined() + } + + /// Whether `value` is an identity from ``routeIdentifier(fromSSHConfigOutput:)``. + private static func isRouteDigest(_ value: String) -> Bool { + value.utf8.count == 64 && value.utf8.allSatisfy { byte in + (UInt8(ascii: "0")...UInt8(ascii: "9")).contains(byte) + || (UInt8(ascii: "a")...UInt8(ascii: "f")).contains(byte) + } + } + /// Returns a private, deterministic socket path for one route identity. + /// + /// The name has the length and alphabet of a `%C` expansion, so it fits + /// the same `sun_path` budget and every recognizer of cmux-owned sockets + /// (Swift checks, shell `case` patterns, lock and broker keys) accepts it. private func routeSpecificControlPath(for routeIdentifier: String) -> String? { guard let controlSocketDirectoryPath, !routeIdentifier.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { @@ -335,7 +412,10 @@ public struct SSHConnectionSharingOptions: Sendable { "ControlPersist=\(values["controlpersist"] ?? "no")", ] } - if routeSensitive { result.append(Self.routeSensitiveMarker) } + if routeSensitive { + let identity = routeIdentifier(fromSSHConfigOutput: output) ?? "true" + result.append("\(Self.routeSensitiveMarkerKey)=\(identity)") + } return result } diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHConnectionSharingOptionsTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHConnectionSharingOptionsTests.swift index 85962fd65be4..625743298768 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHConnectionSharingOptionsTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHConnectionSharingOptionsTests.swift @@ -430,7 +430,8 @@ struct SSHConnectionSharingOptionsTests { @Test("An explicitly disabled master gets no sharing defaults") func preservesDisabledControlMaster() { - let supplied = ["ControlMaster=no", "ForwardAgent=yes"] + // ForwardAgent is route-sensitive, which also pins ControlPath=none. + let supplied = ["ControlMaster=no", "ServerAliveInterval=30"] #expect(options.mergingDefaults(into: supplied) == supplied) #expect(options.cmuxOwnedControlPath(in: [ diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHRouteSpecificControlPathTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHRouteSpecificControlPathTests.swift index 777eb255bed3..c8446b258931 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHRouteSpecificControlPathTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHRouteSpecificControlPathTests.swift @@ -65,6 +65,33 @@ struct SSHRouteSpecificControlPathTests { #expect(first != second) } + @Test("Explicit route options use the route resolved by ssh -G", arguments: [ + ("ProxyCommand=/usr/local/bin/network-a %h %p", "ProxyCommand=/usr/local/bin/network-b %h %p"), + ("IdentityFile=/Users/alice/.ssh/restricted", "IdentityFile=/Users/alice/.ssh/admin"), + ]) + func explicitRouteOptionsUseResolvedRoute(first: String, second: String) throws { + // `ssh -G` echoes explicit options in lowercase-key form. + func controlPath(_ option: String) -> String? { + let parts = option.split(separator: "=", maxSplits: 1) + let output = resolvedConfiguration(["\(parts[0].lowercased()) \(parts[1])"]) + let merged = options.mergingDefaults( + into: [option], + userConfiguredControlOptions: nil, + routeIdentifier: options.routeIdentifier(fromSSHConfigOutput: output) + ) + return options.cmuxOwnedControlPath(in: merged) + } + let firstPath = try #require(controlPath(first)) + let secondPath = try #require(controlPath(second)) + #expect(firstPath != secondPath) + #expect(controlPath(first) == firstPath) + } + + @Test("A resolved route without a host name keeps sharing disabled") + func unresolvedRouteDisablesSharing() { + #expect(options.routeIdentifier(fromSSHConfigOutput: "proxycommand /usr/local/bin/broker") == nil) + } + @Test("Route-specific socket names are recognized as cmux-owned") func routeSpecificNamesAreRecognized() throws { let path = try #require(sharedControlPath(["proxycommand /usr/local/bin/broker %h %p"])) From 622c9b2493705214bc09ec5f5ea205bea6d52fd5 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:50:12 -0700 Subject: [PATCH 15/48] cmux ssh relay: share the relay handshake between the app and the macOS CLI The relay in the app and the macOS CLI each built the relay MAC message, hex coding and constant-time comparison on their own. Move them into CmuxFoundation as RemoteRelayAuthentication, and move the CLI's side of the handshake into RemoteRelayClientHandshake, which takes line I/O from the caller so it can be tested against a fake relay without a CLI build. No behavior change: the CLI still sends relay_id and mac and accepts any {"ok":true}, with the same error messages. Co-Authored-By: Claude Opus 5.5 --- CLI/cmux.swift | 100 ++++++------------ .../RemoteRelayAuthentication.swift | 91 ++++++++++++++++ .../RemoteRelayClientHandshake.swift | 73 +++++++++++++ .../Relay/RemoteCLIRelaySession.swift | 56 ++-------- 4 files changed, 209 insertions(+), 111 deletions(-) create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayAuthentication.swift create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 655a1b412e9f..1c050dc0ef27 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -3573,7 +3573,7 @@ final class SocketClient { let environment = ProcessInfo.processInfo.environment if let relayID = trimmedEnvValue(environment["CMUX_RELAY_ID"]), let relayTokenHex = trimmedEnvValue(environment["CMUX_RELAY_TOKEN"]), - let relayToken = hexData(from: relayTokenHex) { + let relayToken = RemoteRelayAuthentication.hexData(from: relayTokenHex) { return RelayCredentials(relayID: relayID, relayToken: relayToken) } @@ -3583,37 +3583,13 @@ final class SocketClient { let authObject = try? JSONSerialization.jsonObject(with: authData) as? [String: Any], let relayID = trimmedEnvValue(authObject["relay_id"] as? String), let relayTokenHex = trimmedEnvValue(authObject["relay_token"] as? String), - let relayToken = hexData(from: relayTokenHex) else { + let relayToken = RemoteRelayAuthentication.hexData(from: relayTokenHex) else { throw CLIError(message: "Missing relay auth metadata for \(endpoint.host):\(endpoint.port)") } return RelayCredentials(relayID: relayID, relayToken: relayToken) } - private static func hexData(from string: String) -> Data? { - let normalized = string.trimmingCharacters(in: .whitespacesAndNewlines) - guard !normalized.isEmpty, - normalized.count.isMultiple(of: 2) else { - return nil - } - - var data = Data(capacity: normalized.count / 2) - var cursor = normalized.startIndex - while cursor < normalized.endIndex { - let next = normalized.index(cursor, offsetBy: 2) - guard let byte = UInt8(normalized[cursor.. String { - data.map { String(format: "%02x", $0) }.joined() - } - private func connectToRelay( endpoint: RelayEndpoint, responseTimeout: TimeInterval? = nil, @@ -3734,47 +3710,41 @@ final class SocketClient { responseTimeout: TimeInterval, deadline: Date ) throws { - let challengeLine = try readLine( - responseTimeout: responseTimeout, - deadline: deadline - ) - guard let challengeData = challengeLine.data(using: .utf8), - let challenge = try JSONSerialization.jsonObject(with: challengeData) as? [String: Any], - (challenge["protocol"] as? String) == "cmux-relay-auth", - let version = challenge["version"] as? Int, - let relayID = challenge["relay_id"] as? String, - relayID == credentials.relayID, - let nonce = challenge["nonce"] as? String, - !nonce.isEmpty else { - throw CLIError(message: "Invalid relay authentication challenge") - } - - let authMessage = Data("relay_id=\(relayID)\nnonce=\(nonce)\nversion=\(version)".utf8) - let key = SymmetricKey(data: credentials.relayToken) - let mac = Data(HMAC.authenticationCode(for: authMessage, using: key)) - let authPayload = try JSONSerialization.data(withJSONObject: [ - "relay_id": relayID, - "mac": Self.hexString(from: mac), - ]) - try configureSocketWriteSafety(remainingSocketTimeout( - responseTimeout: responseTimeout, - deadline: deadline - )) - try writeAllNonBlocking( - authPayload + Data([0x0A]), - deadline: deadline, - timeoutMessage: "Relay command timed out", - failureMessage: "Failed to write to relay socket" + let handshake = RemoteRelayClientHandshake( + relayID: credentials.relayID, + relayToken: credentials.relayToken ) + do { + try handshake.perform( + readLine: { + try readLine(responseTimeout: responseTimeout, deadline: deadline) + }, + writeLine: { line in + try configureSocketWriteSafety(remainingSocketTimeout( + responseTimeout: responseTimeout, + deadline: deadline + )) + try writeAllNonBlocking( + line, + deadline: deadline, + timeoutMessage: "Relay command timed out", + failureMessage: "Failed to write to relay socket" + ) + } + ) + } catch let failure as RemoteRelayClientHandshake.Failure { + throw CLIError(message: Self.relayHandshakeFailureMessage(failure)) + } + } - let authResponseLine = try readLine( - responseTimeout: responseTimeout, - deadline: deadline - ) - guard let authResponseData = authResponseLine.data(using: .utf8), - let authResponse = try JSONSerialization.jsonObject(with: authResponseData) as? [String: Any], - (authResponse["ok"] as? Bool) == true else { - throw CLIError(message: "Relay authentication failed") + private static func relayHandshakeFailureMessage( + _ failure: RemoteRelayClientHandshake.Failure + ) -> String { + switch failure { + case .invalidChallenge: + return "Invalid relay authentication challenge" + case .rejected: + return "Relay authentication failed" } } diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayAuthentication.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayAuthentication.swift new file mode 100644 index 000000000000..9ecf06aa563c --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayAuthentication.swift @@ -0,0 +1,91 @@ +internal import CryptoKit +public import Foundation + +/// Message and MAC construction for the `cmux ssh` CLI relay handshake. +/// +/// The relay in the app and the macOS cmux CLI both build their MACs here, so +/// the two sides of the handshake cannot drift. The Go remote CLI +/// (`daemon/remote/cmd/cmuxd-remote/cli.go`) builds the same byte strings. +public enum RemoteRelayAuthentication { + /// Value of the challenge line's `protocol` field. + public static let protocolName = "cmux-relay-auth" + + /// The client's MAC over the relay's challenge. + /// + /// - Parameters: + /// - token: Relay token shared by the relay and its clients. + /// - relayID: Relay ID from the challenge. + /// - nonce: Relay nonce from the challenge. + /// - version: Protocol version from the challenge. + public static func clientMAC(token: Data, relayID: String, nonce: String, version: Int) -> Data { + hmac(token: token, message: "relay_id=\(relayID)\nnonce=\(nonce)\nversion=\(version)") + } + + /// The relay's proof that it holds the token, answering a client nonce. + /// + /// The leading label keeps it distinct from every client MAC, whose + /// message starts with `relay_id=`, so neither can be reflected as the + /// other. + /// + /// - Parameters: + /// - token: Relay token shared by the relay and its clients. + /// - relayID: Relay ID from the challenge. + /// - clientNonce: Hex nonce the client sent with its MAC. + /// - serverNonce: Relay nonce from the challenge. + /// - version: Protocol version from the challenge. + public static func relayProofMAC( + token: Data, + relayID: String, + clientNonce: String, + serverNonce: String, + version: Int + ) -> Data { + hmac( + token: token, + message: "cmux-relay-server-proof\nrelay_id=\(relayID)\nclient_nonce=\(clientNonce)\nserver_nonce=\(serverNonce)\nversion=\(version)" + ) + } + + /// Constant-time equality for secrets and MACs. + /// + /// Inputs of different lengths are unequal; the length itself is not + /// treated as secret. + public static func constantTimeEqual(_ lhs: Data, _ rhs: Data) -> Bool { + guard lhs.count == rhs.count else { return false } + var difference: UInt8 = 0 + for (left, right) in zip(lhs, rhs) { + difference |= left ^ right + } + return difference == 0 + } + + /// Constant-time equality for UTF-8 strings such as bridge tokens. + public static func constantTimeEqual(_ lhs: String, _ rhs: String) -> Bool { + constantTimeEqual(Data(lhs.utf8), Data(rhs.utf8)) + } + + /// Decodes an even-length hex string, or returns `nil`. + public static func hexData(from string: String) -> Data? { + let normalized = string.trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalized.isEmpty, normalized.count.isMultiple(of: 2) else { return nil } + var data = Data(capacity: normalized.count / 2) + var cursor = normalized.startIndex + while cursor < normalized.endIndex { + let next = normalized.index(cursor, offsetBy: 2) + guard let byte = UInt8(normalized[cursor.. String { + data.map { String(format: "%02x", $0) }.joined() + } + + static func hmac(token: Data, message: String) -> Data { + let key = SymmetricKey(data: token) + return Data(HMAC.authenticationCode(for: Data(message.utf8), using: key)) + } +} diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift new file mode 100644 index 000000000000..8f888330f59c --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift @@ -0,0 +1,73 @@ +public import Foundation + +/// The client side of the `cmux ssh` CLI relay handshake. +/// +/// The macOS cmux CLI runs it on a remote Mac, where `CMUX_SOCKET_PATH` names +/// a forwarded TCP port on the remote loopback. The caller owns the socket and +/// its deadlines and passes line I/O in; nothing but the handshake is written +/// until ``perform(readLine:writeLine:)`` returns. +public struct RemoteRelayClientHandshake: Sendable { + /// Why the handshake refused to continue. + public enum Failure: Error, Equatable, Sendable { + /// The first line is not a challenge for this relay ID. + case invalidChallenge + /// The relay rejected the client's MAC. + case rejected + } + + private let relayID: String + private let relayToken: Data + + /// Creates a handshake for one relay. + /// + /// - Parameters: + /// - relayID: Relay ID from the relay's credentials. + /// - relayToken: Relay token from the relay's credentials. + public init(relayID: String, relayToken: Data) { + self.relayID = relayID + self.relayToken = relayToken + } + + /// Runs the handshake. + /// + /// - Parameters: + /// - readLine: Returns the next line from the relay, without its newline. + /// - writeLine: Writes one line to the relay; the data ends with a newline. + public func perform( + readLine: () throws -> String, + writeLine: (Data) throws -> Void + ) throws { + let challengeLine = try readLine() + guard let challenge = Self.jsonObject(challengeLine), + (challenge["protocol"] as? String) == RemoteRelayAuthentication.protocolName, + let version = challenge["version"] as? Int, + let challengeRelayID = challenge["relay_id"] as? String, + challengeRelayID == relayID, + let nonce = challenge["nonce"] as? String, + !nonce.isEmpty else { + throw Failure.invalidChallenge + } + + let mac = RemoteRelayAuthentication.clientMAC( + token: relayToken, + relayID: relayID, + nonce: nonce, + version: version + ) + let payload = try JSONSerialization.data(withJSONObject: [ + "relay_id": relayID, + "mac": RemoteRelayAuthentication.hexString(from: mac), + ]) + try writeLine(payload + Data([0x0A])) + + guard let result = Self.jsonObject(try readLine()), + (result["ok"] as? Bool) == true else { + throw Failure.rejected + } + } + + private static func jsonObject(_ line: String) -> [String: Any]? { + guard let data = line.data(using: .utf8) else { return nil } + return (try? JSONSerialization.jsonObject(with: data)) as? [String: Any] + } +} diff --git a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift index e713b27e7e03..be09005f6d41 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift @@ -227,9 +227,13 @@ extension RemoteCLIRelayServer { return } - let message = Self.authMessage(relayID: relayID, nonce: challengeNonce, version: challengeVersion) - let expectedMAC = Self.authMAC(token: relayToken, message: message) - guard Self.constantTimeEqual(receivedMAC, expectedMAC) else { + let expectedMAC = RemoteRelayAuthentication.clientMAC( + token: relayToken, + relayID: relayID, + nonce: challengeNonce, + version: challengeVersion + ) + guard RemoteRelayAuthentication.constantTimeEqual(receivedMAC, expectedMAC) else { sendFailureAndClose() return } @@ -245,14 +249,14 @@ extension RemoteCLIRelayServer { sendFailureAndClose() return } - let proof = Self.relayProofMAC( + let proof = RemoteRelayAuthentication.relayProofMAC( token: relayToken, relayID: relayID, clientNonce: clientNonce, serverNonce: challengeNonce, version: challengeVersion ) - success["relay_mac"] = proof.map { String(format: "%02x", $0) }.joined() + success["relay_mac"] = RemoteRelayAuthentication.hexString(from: proof) } guard admitAuthenticated() else { @@ -434,27 +438,6 @@ extension RemoteCLIRelayServer { onClose() } - private static func authMessage(relayID: String, nonce: String, version: Int) -> Data { - Data("relay_id=\(relayID)\nnonce=\(nonce)\nversion=\(version)".utf8) - } - - /// The relay's proof of the token, returned to clients that send a - /// nonce. The leading label keeps it distinct from every client MAC, - /// whose message starts with `relay_id=`, so neither can be reflected - /// as the other. - static func relayProofMAC( - token: Data, - relayID: String, - clientNonce: String, - serverNonce: String, - version: Int - ) -> Data { - let message = Data( - "cmux-relay-server-proof\nrelay_id=\(relayID)\nclient_nonce=\(clientNonce)\nserver_nonce=\(serverNonce)\nversion=\(version)".utf8 - ) - return authMAC(token: token, message: message) - } - /// Accepts 16 to 64 bytes of lowercase hex. private static func isValidClientNonce(_ nonce: String) -> Bool { guard (32...128).contains(nonce.utf8.count), @@ -471,27 +454,8 @@ extension RemoteCLIRelayServer { return Data(code) } - private static func constantTimeEqual(_ lhs: Data, _ rhs: Data) -> Bool { - guard lhs.count == rhs.count else { return false } - var diff: UInt8 = 0 - for index in lhs.indices { - diff |= lhs[index] ^ rhs[index] - } - return diff == 0 - } - static func hexData(from string: String) -> Data? { - let normalized = string.trimmingCharacters(in: .whitespacesAndNewlines) - guard normalized.count.isMultiple(of: 2), !normalized.isEmpty else { return nil } - var data = Data(capacity: normalized.count / 2) - var cursor = normalized.startIndex - while cursor < normalized.endIndex { - let next = normalized.index(cursor, offsetBy: 2) - guard let byte = UInt8(normalized[cursor.. String? { From b8b1e7339433913366bf0898b25ddab4100fc7f6 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:53:14 -0700 Subject: [PATCH 16/48] cmux ssh relay: test that the macOS CLI requires the relay to prove the token On a remote Mac the cmux CLI talks to the relay through a forwarded loopback port. While the forward is down another user on that Mac can bind the port, answer {"ok":true} and receive the CLI's commands and hook payloads. The CLI's handshake accepts that answer today. These fake-relay tests expect the handshake to send a fresh 32-byte client_nonce and to refuse an answer without relay_mac, with a relay_mac made with another token, or with one replayed from another client nonce, before the command is written. An interop test runs the shared handshake against the real RemoteCLIRelayServer. Co-Authored-By: Claude Opus 5.5 --- .../RemoteRelayClientHandshakeTests.swift | 132 ++++++++++++++++++ .../RemoteCLIRelayServerTests.swift | 46 ++++++ 2 files changed, 178 insertions(+) create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift new file mode 100644 index 000000000000..e5cdad3a7ece --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift @@ -0,0 +1,132 @@ +import CryptoKit +import Foundation +import Testing + +@testable import CmuxFoundation + +/// A relay listener driven by the client's line I/O. It answers the auth line +/// with `result(authLine)` and records everything the client writes. +private final class FakeRelay { + let relayID: String + let serverNonce = "0123456789abcdef0123456789abcdef" + private let result: ([String: Any]) -> String + private var pendingLines: [String] + private(set) var receivedLines: [String] = [] + + init(relayID: String, result: @escaping ([String: Any]) -> String) { + self.relayID = relayID + self.result = result + pendingLines = [ + #"{"protocol":"cmux-relay-auth","version":1,"relay_id":"\#(relayID)","nonce":"\#(serverNonce)"}"#, + ] + } + + func readLine() throws -> String { + guard !pendingLines.isEmpty else { throw POSIXError(.ECONNRESET) } + return pendingLines.removeFirst() + } + + func writeLine(_ data: Data) throws { + let line = String(decoding: data, as: UTF8.self) + .trimmingCharacters(in: .newlines) + receivedLines.append(line) + if receivedLines.count == 1 { + let object = (try? JSONSerialization.jsonObject(with: Data(line.utf8))) as? [String: Any] ?? [:] + pendingLines.append(result(object)) + } + } +} + +@Suite +struct RemoteRelayClientHandshakeTests { + private static let relayID = "relay-handshake-test" + private static let token = Data((0..<32).map { UInt8($0) }) + private static let request = #"{"id":"1","method":"notification.create","params":{"title":"secret"}}"# + + /// Independent oracle for the relay's proof, built from the wire format + /// rather than the shared helper. + private static func proofHex(clientNonce: String, serverNonce: String, token: Data = token) -> String { + let message = "cmux-relay-server-proof\nrelay_id=\(relayID)\nclient_nonce=\(clientNonce)\nserver_nonce=\(serverNonce)\nversion=1" + let mac = HMAC.authenticationCode(for: Data(message.utf8), using: SymmetricKey(data: token)) + return Data(mac).map { String(format: "%02x", $0) }.joined() + } + + /// Mirrors the CLI: authenticate, then send the command on the same connection. + private static func sendCommand(through relay: FakeRelay) throws { + let handshake = RemoteRelayClientHandshake(relayID: relayID, relayToken: token) + try handshake.perform(readLine: relay.readLine, writeLine: relay.writeLine) + try relay.writeLine(Data((request + "\n").utf8)) + } + + @Test("a listener that answers ok without relay_mac never receives the command") + func okWithoutRelayProofIsRefused() { + let relay = FakeRelay(relayID: Self.relayID) { _ in #"{"ok":true}"# } + + #expect(throws: (any Error).self) { try Self.sendCommand(through: relay) } + #expect(relay.receivedLines.count == 1, "Only the auth line may reach an unproven relay") + #expect(!relay.receivedLines.contains { $0.contains("notification.create") }) + } + + @Test("a relay_mac made with another token is refused") + func proofWithWrongTokenIsRefused() { + let relay = FakeRelay(relayID: Self.relayID) { auth in + let clientNonce = auth["client_nonce"] as? String ?? "" + let proof = Self.proofHex( + clientNonce: clientNonce, + serverNonce: "0123456789abcdef0123456789abcdef", + token: Data(repeating: 0xEE, count: 32) + ) + return #"{"ok":true,"relay_mac":"\#(proof)"}"# + } + + #expect(throws: (any Error).self) { try Self.sendCommand(through: relay) } + #expect(!relay.receivedLines.contains { $0.contains("notification.create") }) + } + + @Test("a relay_mac replayed from another client nonce is refused") + func replayedProofIsRefused() { + let relay = FakeRelay(relayID: Self.relayID) { _ in + let replayed = Self.proofHex( + clientNonce: String(repeating: "ab", count: 32), + serverNonce: "0123456789abcdef0123456789abcdef" + ) + return #"{"ok":true,"relay_mac":"\#(replayed)"}"# + } + + #expect(throws: (any Error).self) { try Self.sendCommand(through: relay) } + #expect(!relay.receivedLines.contains { $0.contains("notification.create") }) + } + + @Test("each auth line carries a fresh 32-byte client nonce") + func authLineCarriesFreshClientNonce() throws { + var nonces: [String] = [] + for _ in 0..<2 { + let relay = FakeRelay(relayID: Self.relayID) { _ in #"{"ok":false}"# } + _ = try? Self.sendCommand(through: relay) + let authLine = try #require(relay.receivedLines.first) + let auth = try #require( + JSONSerialization.jsonObject(with: Data(authLine.utf8)) as? [String: Any] + ) + let nonce = try #require(auth["client_nonce"] as? String) + #expect(nonce.count == 64) + #expect(nonce.allSatisfy { $0.isHexDigit && !$0.isUppercase }) + nonces.append(nonce) + } + #expect(nonces[0] != nonces[1]) + } + + @Test("a relay that proves the token receives the command") + func provenRelayReceivesCommand() throws { + let relay = FakeRelay(relayID: Self.relayID) { auth in + guard let clientNonce = auth["client_nonce"] as? String else { return #"{"ok":true}"# } + let proof = Self.proofHex( + clientNonce: clientNonce, + serverNonce: "0123456789abcdef0123456789abcdef" + ) + return #"{"ok":true,"relay_mac":"\#(proof)"}"# + } + + try Self.sendCommand(through: relay) + #expect(relay.receivedLines.last == Self.request) + } +} diff --git a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift index 31922c49bbb9..02cdaf2d09e6 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemoteCLIRelayServerTests.swift @@ -215,6 +215,16 @@ private final class RelayTestClient: @unchecked Sendable { } } + /// Complete newline-terminated lines received so far. + func receivedLines() -> [String] { + lock.lock() + let snapshot = received + lock.unlock() + guard let lastNewline = snapshot.lastIndex(of: 0x0A) else { return [] } + return snapshot[.. index }) else { + throw POSIXError(.ETIMEDOUT) + } + consumedLines += 1 + return client.receivedLines()[index] + }, + writeLine: { client.send($0) } + ) + + client.send(Data((#"{"id":"relay-test","method":"system.ping","params":{}}"# + "\n").utf8)) + #expect(client.wait { data, closed in + String(decoding: data, as: UTF8.self).contains("\"result\":42") && closed + }) + } + @Test("an older client without a nonce still authenticates") func olderClientWithoutNonceAuthenticates() throws { let server = try RemoteCLIRelayServer( From 6a925f0c1ccea5221f995a3f3ce572b36ca2f08c Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:55:03 -0700 Subject: [PATCH 17/48] cmux ssh relay: make the macOS CLI require the relay to prove the token The macOS CLI's relay handshake now matches the Go remote CLI: it sends a fresh 32-byte client_nonce with its MAC and writes nothing else until the success line carries relay_mac, which it checks in constant time against the HMAC over "cmux-relay-server-proof", the relay ID, both nonces and the version. The relay and the CLI build that message with the same RemoteRelayAuthentication helper. A listener another user bound on the forwarded port while it was down now gets only the auth line, which carries no secret, and the CLI fails with "Relay did not prove it holds the relay token; reconnect this SSH workspace". The CLI already refuses a TCP relay address without credentials: it loads them before opening the socket and fails with "Missing relay auth metadata". An empty token is now refused as well. Version skew matches the Go CLI: an older app's relay ignores the nonce and answers without relay_mac, so a new CLI fails closed against it. The CLI and fixture relays in cmuxTests and cmuxCLITests now answer with the proof, computed independently with CryptoKit. Co-Authored-By: Claude Opus 5.5 --- CLI/cmux.swift | 4 ++ .../RemoteRelayClientHandshake.swift | 47 +++++++++++++++++-- .../CLIRelayQueuedHookRegressionTests.swift | 20 +++++++- .../CMUXCLIErrorOutputRegressionTests.swift | 21 ++++++++- 4 files changed, 85 insertions(+), 7 deletions(-) diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 1c050dc0ef27..245530b9be41 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -3745,6 +3745,10 @@ final class SocketClient { return "Invalid relay authentication challenge" case .rejected: return "Relay authentication failed" + case .relayNotProven: + return "Relay did not prove it holds the relay token; reconnect this SSH workspace" + case .nonceUnavailable: + return "Failed to create relay authentication nonce" } } diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift index 8f888330f59c..8067b0ce20d3 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift @@ -1,11 +1,16 @@ public import Foundation +internal import Security /// The client side of the `cmux ssh` CLI relay handshake. /// /// The macOS cmux CLI runs it on a remote Mac, where `CMUX_SOCKET_PATH` names -/// a forwarded TCP port on the remote loopback. The caller owns the socket and -/// its deadlines and passes line I/O in; nothing but the handshake is written -/// until ``perform(readLine:writeLine:)`` returns. +/// a forwarded TCP port on the remote loopback. While that forward is down, +/// another user on the remote Mac can bind the port, so the handshake is +/// mutual: the client proves the token with its MAC over the relay's nonce, +/// and the relay proves it with `relay_mac` over a fresh client nonce. The +/// caller owns the socket and its deadlines and passes line I/O in; nothing +/// but the auth line is written until ``perform(readLine:writeLine:)`` +/// returns, and it returns only after the relay's proof checks out. public struct RemoteRelayClientHandshake: Sendable { /// Why the handshake refused to continue. public enum Failure: Error, Equatable, Sendable { @@ -13,8 +18,15 @@ public struct RemoteRelayClientHandshake: Sendable { case invalidChallenge /// The relay rejected the client's MAC. case rejected + /// The listener did not prove it holds the relay token. + case relayNotProven + /// No random client nonce could be generated. + case nonceUnavailable } + /// Size of the client nonce, matching the Go remote CLI. + public static let clientNonceByteCount = 32 + private let relayID: String private let relayToken: Data @@ -33,6 +45,8 @@ public struct RemoteRelayClientHandshake: Sendable { /// - Parameters: /// - readLine: Returns the next line from the relay, without its newline. /// - writeLine: Writes one line to the relay; the data ends with a newline. + /// - Throws: ``Failure`` when the relay is not authenticated, or any error + /// from `readLine` and `writeLine`. public func perform( readLine: () throws -> String, writeLine: (Data) throws -> Void @@ -47,6 +61,10 @@ public struct RemoteRelayClientHandshake: Sendable { !nonce.isEmpty else { throw Failure.invalidChallenge } + guard !relayToken.isEmpty else { throw Failure.rejected } + guard let clientNonce = Self.randomClientNonce() else { + throw Failure.nonceUnavailable + } let mac = RemoteRelayAuthentication.clientMAC( token: relayToken, @@ -57,6 +75,7 @@ public struct RemoteRelayClientHandshake: Sendable { let payload = try JSONSerialization.data(withJSONObject: [ "relay_id": relayID, "mac": RemoteRelayAuthentication.hexString(from: mac), + "client_nonce": clientNonce, ]) try writeLine(payload + Data([0x0A])) @@ -64,6 +83,28 @@ public struct RemoteRelayClientHandshake: Sendable { (result["ok"] as? Bool) == true else { throw Failure.rejected } + // Anyone who connected once has seen the relay ID, so only a proof + // over this client's nonce shows the listener holds the relay token. + let expectedProof = RemoteRelayAuthentication.relayProofMAC( + token: relayToken, + relayID: relayID, + clientNonce: clientNonce, + serverNonce: nonce, + version: version + ) + guard let proofHex = result["relay_mac"] as? String, + let receivedProof = RemoteRelayAuthentication.hexData(from: proofHex), + RemoteRelayAuthentication.constantTimeEqual(receivedProof, expectedProof) else { + throw Failure.relayNotProven + } + } + + private static func randomClientNonce() -> String? { + var bytes = [UInt8](repeating: 0, count: clientNonceByteCount) + guard SecRandomCopyBytes(kSecRandomDefault, bytes.count, &bytes) == errSecSuccess else { + return nil + } + return RemoteRelayAuthentication.hexString(from: Data(bytes)) } private static func jsonObject(_ line: String) -> [String: Any]? { diff --git a/cmuxCLITests/CLIRelayQueuedHookRegressionTests.swift b/cmuxCLITests/CLIRelayQueuedHookRegressionTests.swift index 49bd0828b13f..f90f2f6a6ba3 100644 --- a/cmuxCLITests/CLIRelayQueuedHookRegressionTests.swift +++ b/cmuxCLITests/CLIRelayQueuedHookRegressionTests.swift @@ -1,3 +1,4 @@ +import CryptoKit import Darwin import Foundation import Testing @@ -690,6 +691,21 @@ struct CLIRelayQueuedHookRegressionTests { private final class RelayQueuedHookMockServer: @unchecked Sendable { static let relayID = "relay-hook-regression" + /// Token the tests pass in `CMUX_RELAY_TOKEN` (64 hex `a`s). + static let relayToken = Data(repeating: 0xAA, count: 32) + + /// Success line proving the token over the client's nonce, as the app's + /// relay does; the CLI sends nothing further without it. + static func authResult(authLine: String) -> String { + guard let object = try? JSONSerialization.jsonObject(with: Data(authLine.utf8)) as? [String: Any], + let clientNonce = object["client_nonce"] as? String else { + return #"{"ok":true}"# + } + let message = "cmux-relay-server-proof\nrelay_id=\(relayID)\nclient_nonce=\(clientNonce)\nserver_nonce=relay-nonce\nversion=1" + let proof = HMAC.authenticationCode(for: Data(message.utf8), using: SymmetricKey(data: relayToken)) + let proofHex = Data(proof).map { String(format: "%02x", $0) }.joined() + return #"{"ok":true,"relay_mac":"\#(proofHex)"}"# + } private let listenerFD: Int32 private let port: UInt16 @@ -776,8 +792,8 @@ private final class RelayQueuedHookMockServer: @unchecked Sendable { #"{"protocol":"cmux-relay-auth","version":1,"relay_id":"\#(Self.relayID)","nonce":"relay-nonce"}"#, to: clientFD ) - guard readLine(from: clientFD) != nil else { return } - writeLine(#"{"ok":true}"#, to: clientFD) + guard let authLine = readLine(from: clientFD) else { return } + writeLine(Self.authResult(authLine: authLine), to: clientFD) while let line = readLine(from: clientFD) { captured.append(line) diff --git a/cmuxTests/CMUXCLIErrorOutputRegressionTests.swift b/cmuxTests/CMUXCLIErrorOutputRegressionTests.swift index 01dbaf63db77..d6134aac1a85 100644 --- a/cmuxTests/CMUXCLIErrorOutputRegressionTests.swift +++ b/cmuxTests/CMUXCLIErrorOutputRegressionTests.swift @@ -1,6 +1,7 @@ import CMUXAgentLaunch import CmuxControlSocket import CmuxSettings +import CryptoKit import Darwin import Foundation import SQLite3 @@ -5146,8 +5147,8 @@ final class RelaySocketResponder { socklen_t(MemoryLayout.size) ) let challenge = #"{"protocol":"cmux-relay-auth","version":1,"relay_id":"\#(relayID)","nonce":"test-nonce"}"# - guard writeLine(challenge, to: clientFD), readLine(from: clientFD) != nil else { return } - guard writeLine(#"{"ok":true}"#, to: clientFD), + guard writeLine(challenge, to: clientFD), let authLine = readLine(from: clientFD) else { return } + guard writeLine(Self.authResult(authLine: authLine, relayID: relayID), to: clientFD), let request = readLine(from: clientFD) else { return } lock.lock() @@ -5185,6 +5186,22 @@ final class RelaySocketResponder { } } + /// Token the relay tests pass in `CMUX_RELAY_TOKEN`. + static let relayToken = Data(repeating: 0x11, count: 32) + + /// Success line proving the token over the client's nonce, as the app's + /// relay does; the CLI sends nothing further without it. + private static func authResult(authLine: String, relayID: String) -> String { + guard let object = try? JSONSerialization.jsonObject(with: Data(authLine.utf8)) as? [String: Any], + let clientNonce = object["client_nonce"] as? String else { + return #"{"ok":true}"# + } + let message = "cmux-relay-server-proof\nrelay_id=\(relayID)\nclient_nonce=\(clientNonce)\nserver_nonce=test-nonce\nversion=1" + let proof = HMAC.authenticationCode(for: Data(message.utf8), using: SymmetricKey(data: relayToken)) + let proofHex = Data(proof).map { String(format: "%02x", $0) }.joined() + return #"{"ok":true,"relay_mac":"\#(proofHex)"}"# + } + private static func posixError(_ operation: String) -> NSError { NSError( domain: NSPOSIXErrorDomain, From 632e8fc9aefabc731220fd00fbb397981adb2cd2 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:56:01 -0700 Subject: [PATCH 18/48] cmux ssh: compare the PTY bridge token in constant time The PTY bridge checked its handshake token with String ==, which can stop at the first differing byte. Use the constant-time comparison the relay already uses, now shared as RemoteRelayAuthentication.constantTimeEqual. Timing is not observable in a unit test, so there is no red commit for this change. The new tests pin the behavior that must not change: a token that differs in its last byte, a prefix and an extension of the token are all refused without attaching, and the shared helper compares strings and bytes correctly, including unequal lengths. Co-Authored-By: Claude Opus 5.5 --- .../RemoteRelayClientHandshakeTests.swift | 16 ++++++++++ .../PTYBridge/RemotePTYBridgeSession.swift | 3 +- .../RemotePTYBridgeServerTests.swift | 29 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift index e5cdad3a7ece..68c1e6fc8c56 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift @@ -37,6 +37,22 @@ private final class FakeRelay { } } +@Suite +struct RemoteRelayConstantTimeEqualTests { + @Test(arguments: [ + ("token-abc", "token-abc", true), + ("token-abc", "token-abd", false), + ("token-abc", "token-ab", false), + ("token-abc", "token-abcd", false), + ("", "", true), + ("", "x", false), + ]) + func comparesStrings(lhs: String, rhs: String, expected: Bool) { + #expect(RemoteRelayAuthentication.constantTimeEqual(lhs, rhs) == expected) + #expect(RemoteRelayAuthentication.constantTimeEqual(Data(lhs.utf8), Data(rhs.utf8)) == expected) + } +} + @Suite struct RemoteRelayClientHandshakeTests { private static let relayID = "relay-handshake-test" diff --git a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/PTYBridge/RemotePTYBridgeSession.swift b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/PTYBridge/RemotePTYBridgeSession.swift index 9f6e14a50870..254e3b97423b 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/PTYBridge/RemotePTYBridgeSession.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/PTYBridge/RemotePTYBridgeSession.swift @@ -1,3 +1,4 @@ +internal import CmuxFoundation internal import CmuxRemoteDaemon internal import Foundation internal import Network @@ -173,7 +174,7 @@ extension RemotePTYBridgeServer { } guard let payload = try? JSONSerialization.jsonObject(with: lineData, options: []) as? [String: Any], let receivedToken = payload["token"] as? String, - receivedToken == token else { + RemoteRelayAuthentication.constantTimeEqual(receivedToken, token) else { close(detach: false) return } diff --git a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemotePTYBridgeServerTests.swift b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemotePTYBridgeServerTests.swift index 1718a2cde6a6..317c3eee743b 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemotePTYBridgeServerTests.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Tests/CmuxRemoteWorkspaceTests/RemotePTYBridgeServerTests.swift @@ -396,6 +396,35 @@ struct RemotePTYBridgeServerTests { }) } + @Test( + "a near-miss handshake token closes the connection without attaching", + arguments: ["lastByte", "prefix", "extended"] + ) + func nearMissTokenCloses(variant: String) throws { + let rpc = RecordingPTYBridgeRPCClient() + let server = makeServer(client: rpc) + defer { server.stop() } + let endpoint = try server.start() + let token = endpoint.token + let offered: String + switch variant { + case "lastByte": + offered = String(token.dropLast()) + (token.hasSuffix("0") ? "1" : "0") + case "prefix": + offered = String(token.dropLast()) + default: + offered = token + "0" + } + + let client = BridgeTestClient(endpoint: endpoint) + defer { client.cancel() } + client.send(Data("{\"token\":\"\(offered)\",\"cols\":120,\"rows\":40}\n".utf8)) + + #expect(client.waitForReceived { data, closed in + closed && data.isEmpty + }) + } + @Test("a failed attach reports the mapped error line before closing") func failedAttachReportsErrorLine() throws { let rpc = RecordingPTYBridgeRPCClient() From c7801d3e5412366172eecaf633f87bb7a2a41967 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 18:10:26 -0700 Subject: [PATCH 19/48] cmux ssh relay: make relay authentication an instantiated value The package-conventions lint rejects an all-static namespace enum. The MAC builder is now a value created with the relay token, and the hex and constant-time helpers are extensions on Data and String. Co-Authored-By: Claude Opus 5.5 --- CLI/cmux.swift | 4 +- .../RemoteRelayAuthentication.swift | 88 +++++++++++-------- .../RemoteRelayClientHandshake.swift | 15 ++-- .../RemoteRelayClientHandshakeTests.swift | 4 +- .../PTYBridge/RemotePTYBridgeSession.swift | 2 +- .../Relay/RemoteCLIRelaySession.swift | 13 ++- 6 files changed, 71 insertions(+), 55 deletions(-) diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 245530b9be41..99306c87d86c 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -3573,7 +3573,7 @@ final class SocketClient { let environment = ProcessInfo.processInfo.environment if let relayID = trimmedEnvValue(environment["CMUX_RELAY_ID"]), let relayTokenHex = trimmedEnvValue(environment["CMUX_RELAY_TOKEN"]), - let relayToken = RemoteRelayAuthentication.hexData(from: relayTokenHex) { + let relayToken = Data(relayHex: relayTokenHex) { return RelayCredentials(relayID: relayID, relayToken: relayToken) } @@ -3583,7 +3583,7 @@ final class SocketClient { let authObject = try? JSONSerialization.jsonObject(with: authData) as? [String: Any], let relayID = trimmedEnvValue(authObject["relay_id"] as? String), let relayTokenHex = trimmedEnvValue(authObject["relay_token"] as? String), - let relayToken = RemoteRelayAuthentication.hexData(from: relayTokenHex) else { + let relayToken = Data(relayHex: relayTokenHex) else { throw CLIError(message: "Missing relay auth metadata for \(endpoint.host):\(endpoint.port)") } diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayAuthentication.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayAuthentication.swift index 9ecf06aa563c..3eecceaaf4eb 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayAuthentication.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayAuthentication.swift @@ -6,19 +6,28 @@ public import Foundation /// The relay in the app and the macOS cmux CLI both build their MACs here, so /// the two sides of the handshake cannot drift. The Go remote CLI /// (`daemon/remote/cmd/cmuxd-remote/cli.go`) builds the same byte strings. -public enum RemoteRelayAuthentication { +public struct RemoteRelayAuthentication: Sendable { /// Value of the challenge line's `protocol` field. public static let protocolName = "cmux-relay-auth" + /// Relay token shared by the relay and its clients. + public let token: Data + + /// Creates the MAC builder for one relay token. + /// + /// - Parameter token: Relay token shared by the relay and its clients. + public init(token: Data) { + self.token = token + } + /// The client's MAC over the relay's challenge. /// /// - Parameters: - /// - token: Relay token shared by the relay and its clients. /// - relayID: Relay ID from the challenge. /// - nonce: Relay nonce from the challenge. /// - version: Protocol version from the challenge. - public static func clientMAC(token: Data, relayID: String, nonce: String, version: Int) -> Data { - hmac(token: token, message: "relay_id=\(relayID)\nnonce=\(nonce)\nversion=\(version)") + public func clientMAC(relayID: String, nonce: String, version: Int) -> Data { + hmac("relay_id=\(relayID)\nnonce=\(nonce)\nversion=\(version)") } /// The relay's proof that it holds the token, answering a client nonce. @@ -28,45 +37,34 @@ public enum RemoteRelayAuthentication { /// other. /// /// - Parameters: - /// - token: Relay token shared by the relay and its clients. /// - relayID: Relay ID from the challenge. /// - clientNonce: Hex nonce the client sent with its MAC. /// - serverNonce: Relay nonce from the challenge. /// - version: Protocol version from the challenge. - public static func relayProofMAC( - token: Data, + public func relayProofMAC( relayID: String, clientNonce: String, serverNonce: String, version: Int ) -> Data { hmac( - token: token, - message: "cmux-relay-server-proof\nrelay_id=\(relayID)\nclient_nonce=\(clientNonce)\nserver_nonce=\(serverNonce)\nversion=\(version)" + "cmux-relay-server-proof\nrelay_id=\(relayID)\nclient_nonce=\(clientNonce)\nserver_nonce=\(serverNonce)\nversion=\(version)" ) } - /// Constant-time equality for secrets and MACs. - /// - /// Inputs of different lengths are unequal; the length itself is not - /// treated as secret. - public static func constantTimeEqual(_ lhs: Data, _ rhs: Data) -> Bool { - guard lhs.count == rhs.count else { return false } - var difference: UInt8 = 0 - for (left, right) in zip(lhs, rhs) { - difference |= left ^ right - } - return difference == 0 - } - - /// Constant-time equality for UTF-8 strings such as bridge tokens. - public static func constantTimeEqual(_ lhs: String, _ rhs: String) -> Bool { - constantTimeEqual(Data(lhs.utf8), Data(rhs.utf8)) + private func hmac(_ message: String) -> Data { + let key = SymmetricKey(data: token) + return Data(HMAC.authenticationCode(for: Data(message.utf8), using: key)) } +} - /// Decodes an even-length hex string, or returns `nil`. - public static func hexData(from string: String) -> Data? { - let normalized = string.trimmingCharacters(in: .whitespacesAndNewlines) +extension Data { + /// Decodes an even-length hex string such as a relay token or MAC. + /// + /// - Parameter relayHex: Hex text; surrounding whitespace is ignored. + /// - Returns: `nil` for empty, odd-length or non-hex text. + public init?(relayHex: String) { + let normalized = relayHex.trimmingCharacters(in: .whitespacesAndNewlines) guard !normalized.isEmpty, normalized.count.isMultiple(of: 2) else { return nil } var data = Data(capacity: normalized.count / 2) var cursor = normalized.startIndex @@ -76,16 +74,36 @@ public enum RemoteRelayAuthentication { data.append(byte) cursor = next } - return data + self = data } - /// Lowercase hex encoding. - public static func hexString(from data: Data) -> String { - data.map { String(format: "%02x", $0) }.joined() + /// Lowercase hex encoding, as the relay handshake sends MACs and nonces. + public var relayHexString: String { + map { String(format: "%02x", $0) }.joined() } - static func hmac(token: Data, message: String) -> Data { - let key = SymmetricKey(data: token) - return Data(HMAC.authenticationCode(for: Data(message.utf8), using: key)) + /// Constant-time equality for secrets and MACs. + /// + /// Inputs of different lengths are unequal; the length itself is not + /// treated as secret. + /// + /// - Parameter other: The value to compare with. + public func constantTimeEquals(_ other: Data) -> Bool { + guard count == other.count else { return false } + var difference: UInt8 = 0 + for (left, right) in zip(self, other) { + difference |= left ^ right + } + return difference == 0 + } +} + +extension String { + /// Constant-time equality of the UTF-8 bytes, for tokens such as the + /// PTY bridge token. + /// + /// - Parameter other: The string to compare with. + public func constantTimeEquals(_ other: String) -> Bool { + Data(utf8).constantTimeEquals(Data(other.utf8)) } } diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift index 8067b0ce20d3..0932bd3659fc 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/RemoteRelay/RemoteRelayClientHandshake.swift @@ -66,15 +66,15 @@ public struct RemoteRelayClientHandshake: Sendable { throw Failure.nonceUnavailable } - let mac = RemoteRelayAuthentication.clientMAC( - token: relayToken, + let authentication = RemoteRelayAuthentication(token: relayToken) + let mac = authentication.clientMAC( relayID: relayID, nonce: nonce, version: version ) let payload = try JSONSerialization.data(withJSONObject: [ "relay_id": relayID, - "mac": RemoteRelayAuthentication.hexString(from: mac), + "mac": mac.relayHexString, "client_nonce": clientNonce, ]) try writeLine(payload + Data([0x0A])) @@ -85,16 +85,15 @@ public struct RemoteRelayClientHandshake: Sendable { } // Anyone who connected once has seen the relay ID, so only a proof // over this client's nonce shows the listener holds the relay token. - let expectedProof = RemoteRelayAuthentication.relayProofMAC( - token: relayToken, + let expectedProof = authentication.relayProofMAC( relayID: relayID, clientNonce: clientNonce, serverNonce: nonce, version: version ) guard let proofHex = result["relay_mac"] as? String, - let receivedProof = RemoteRelayAuthentication.hexData(from: proofHex), - RemoteRelayAuthentication.constantTimeEqual(receivedProof, expectedProof) else { + let receivedProof = Data(relayHex: proofHex), + receivedProof.constantTimeEquals(expectedProof) else { throw Failure.relayNotProven } } @@ -104,7 +103,7 @@ public struct RemoteRelayClientHandshake: Sendable { guard SecRandomCopyBytes(kSecRandomDefault, bytes.count, &bytes) == errSecSuccess else { return nil } - return RemoteRelayAuthentication.hexString(from: Data(bytes)) + return Data(bytes).relayHexString } private static func jsonObject(_ line: String) -> [String: Any]? { diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift index 68c1e6fc8c56..ce703afc8e8d 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/RemoteRelayClientHandshakeTests.swift @@ -48,8 +48,8 @@ struct RemoteRelayConstantTimeEqualTests { ("", "x", false), ]) func comparesStrings(lhs: String, rhs: String, expected: Bool) { - #expect(RemoteRelayAuthentication.constantTimeEqual(lhs, rhs) == expected) - #expect(RemoteRelayAuthentication.constantTimeEqual(Data(lhs.utf8), Data(rhs.utf8)) == expected) + #expect(lhs.constantTimeEquals(rhs) == expected) + #expect(Data(lhs.utf8).constantTimeEquals(Data(rhs.utf8)) == expected) } } diff --git a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/PTYBridge/RemotePTYBridgeSession.swift b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/PTYBridge/RemotePTYBridgeSession.swift index 254e3b97423b..d3d142bc1ea7 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/PTYBridge/RemotePTYBridgeSession.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/PTYBridge/RemotePTYBridgeSession.swift @@ -174,7 +174,7 @@ extension RemotePTYBridgeServer { } guard let payload = try? JSONSerialization.jsonObject(with: lineData, options: []) as? [String: Any], let receivedToken = payload["token"] as? String, - RemoteRelayAuthentication.constantTimeEqual(receivedToken, token) else { + receivedToken.constantTimeEquals(token) else { close(detach: false) return } diff --git a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift index be09005f6d41..5e01e5235ad3 100644 --- a/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift +++ b/Packages/macOS/CmuxRemoteWorkspace/Sources/CmuxRemoteWorkspace/Relay/RemoteCLIRelaySession.swift @@ -227,13 +227,13 @@ extension RemoteCLIRelayServer { return } - let expectedMAC = RemoteRelayAuthentication.clientMAC( - token: relayToken, + let authentication = RemoteRelayAuthentication(token: relayToken) + let expectedMAC = authentication.clientMAC( relayID: relayID, nonce: challengeNonce, version: challengeVersion ) - guard RemoteRelayAuthentication.constantTimeEqual(receivedMAC, expectedMAC) else { + guard receivedMAC.constantTimeEquals(expectedMAC) else { sendFailureAndClose() return } @@ -249,14 +249,13 @@ extension RemoteCLIRelayServer { sendFailureAndClose() return } - let proof = RemoteRelayAuthentication.relayProofMAC( - token: relayToken, + let proof = authentication.relayProofMAC( relayID: relayID, clientNonce: clientNonce, serverNonce: challengeNonce, version: challengeVersion ) - success["relay_mac"] = RemoteRelayAuthentication.hexString(from: proof) + success["relay_mac"] = proof.relayHexString } guard admitAuthenticated() else { @@ -455,7 +454,7 @@ extension RemoteCLIRelayServer { } static func hexData(from string: String) -> Data? { - RemoteRelayAuthentication.hexData(from: string) + Data(relayHex: string) } private static func randomHex(byteCount: Int) -> String? { From ba50fb1fa67279c26a205dfd74501a4522395a47 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 18:01:48 -0700 Subject: [PATCH 20/48] cmux ssh: test that a stalled or oversized replay cannot swallow input or output The remote daemon declares its replay length and the attach waits for that many bytes before forwarding keystrokes. Cover a peer that declares more than it sends (the replay phase must end at an idle or total deadline) and a reconnect replay larger than the 1 MiB validation buffer (every byte must still reach the terminal). The new API surface is declared with no-op bodies so the regression fails on assertions, not on compilation. Co-Authored-By: Claude Opus 5.5 --- .../SSHPTYAttachOutputProgress.swift | 10 ++ .../SSHPTYAttachReplayDeadline.swift | 35 +++++ .../SSHPTYReplayOutputFilter.swift | 3 + .../SSHPTYAttachReplayBoundTests.swift | 122 ++++++++++++++++++ 4 files changed, 170 insertions(+) create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayDeadline.swift create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayBoundTests.swift diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachOutputProgress.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachOutputProgress.swift index 04b1175df882..ec2dd25be90b 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachOutputProgress.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachOutputProgress.swift @@ -173,6 +173,16 @@ public struct SSHPTYAttachOutputProgress: Sendable { return Data(data.dropFirst(suppressBytes)) } + /// Declared replay bytes that actually arrived from the bridge. + public var deliveredReplayBytes: Int { 0 } + + /// Ends the replay phase before the declared byte count arrived. + /// + /// - Returns: Buffered replay output that must still reach the terminal. + public mutating func endReplay() -> Data { + Data() + } + /// Finishes a buffered candidate when the bridge closes before replay ends. /// /// - Parameter discarding: When another managed attempt is guaranteed, drop diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayDeadline.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayDeadline.swift new file mode 100644 index 000000000000..65ad25988a7d --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayDeadline.swift @@ -0,0 +1,35 @@ +public import Foundation + +/// Bounds how long an SSH PTY attachment waits for its declared replay. +/// +/// The remote daemon declares the replay length in its ready status, and input +/// forwarding waits for that many bytes. A peer that declares more than it +/// sends must not hold keystrokes forever, so the replay phase ends when the +/// bridge goes quiet for `idleTimeout` or when `totalTimeout` has elapsed +/// since the ready status, whichever comes first. +public struct SSHPTYAttachReplayDeadline: Sendable, Equatable { + /// Quiet interval after which a still-incomplete replay is abandoned. + public static let defaultIdleTimeout: TimeInterval = 2 + /// Longest replay phase measured from the ready status. + public static let defaultTotalTimeout: TimeInterval = 15 + + /// Creates a deadline for a replay that began at `startedAt`. + /// + /// - Parameters: + /// - startedAt: Monotonic time of the bridge ready status. + /// - idleTimeout: Quiet interval that ends the replay phase. + /// - totalTimeout: Maximum replay phase length. + public init( + startedAt: TimeInterval, + idleTimeout: TimeInterval = Self.defaultIdleTimeout, + totalTimeout: TimeInterval = Self.defaultTotalTimeout + ) {} + + /// Records bridge output that arrived at `now`. + public mutating func recordOutput(at now: TimeInterval) {} + + /// Seconds left before the replay phase ends; zero once it has expired. + public func remainingWait(at now: TimeInterval) -> TimeInterval { + .infinity + } +} diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift index 5c688e705f4c..7008aa1eba3b 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift @@ -95,6 +95,9 @@ public struct SSHPTYReplayOutputFilter: Sendable { return output } + /// Treats every later byte as live output after the replay phase ended early. + public mutating func endReplay() {} + /// Flushes an unterminated candidate when the bridge closes. /// /// Unterminated bytes cannot produce a terminal response, so they are diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayBoundTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayBoundTests.swift new file mode 100644 index 000000000000..90799b09bce4 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayBoundTests.swift @@ -0,0 +1,122 @@ +import Foundation +import Testing +@testable import CmuxFoundation + +/// A remote daemon declares its replay length. These tests cover a peer that +/// declares more than it sends and a reconnect replay larger than the +/// validation buffer: input must resume and no output may be lost. +@Suite("SSH PTY attach replay bounds") +struct SSHPTYAttachReplayBoundTests { + private func text(_ data: Data) -> String { + String(decoding: data, as: UTF8.self) + } + + @Test("a replay that goes quiet before its declared length expires at the idle deadline") + func stalledReplayExpiresAtIdleDeadline() { + var deadline = SSHPTYAttachReplayDeadline(startedAt: 100, idleTimeout: 2, totalTimeout: 10) + #expect(deadline.remainingWait(at: 100) == 2) + + deadline.recordOutput(at: 101) + #expect(deadline.remainingWait(at: 102) == 1) + #expect(deadline.remainingWait(at: 103) == 0) + #expect(deadline.remainingWait(at: 104) == 0) + } + + @Test("a replay that keeps trickling still ends at the total deadline") + func tricklingReplayEndsAtTotalDeadline() { + var deadline = SSHPTYAttachReplayDeadline(startedAt: 0, idleTimeout: 2, totalTimeout: 10) + for second in 0..<9 { + deadline.recordOutput(at: TimeInterval(second)) + } + + #expect(deadline.remainingWait(at: 8.5) == 1.5) + #expect(deadline.remainingWait(at: 9.5) == 0.5) + #expect(deadline.remainingWait(at: 10) == 0) + } + + @Test("ending a stalled replay lets later bytes count as live output") + func endingStalledReplayResumesLiveOutput() { + var progress = SSHPTYAttachOutputProgress(replayBytes: 1_000) + let replay = progress.terminalOutput(from: Data("partial".utf8), suppressingReplay: false) + #expect(text(replay) == "partial") + #expect(progress.replayBytesRemaining == 993) + + let pending = progress.endReplay() + + #expect(pending.isEmpty) + #expect(progress.replayBytesRemaining == 0) + #expect(progress.deliveredReplayBytes == 7) + #expect( + progress.completedReplayFingerprint == + SSHPTYAttachOutputProgress.fingerprint(of: Data("partial".utf8)) + ) + let live = progress.terminalOutput(from: Data("typed".utf8), suppressingReplay: false) + #expect(text(live) == "typed") + #expect(progress.receivedLiveOutput) + } + + @Test("ending a stalled reconnect replay forwards the buffered output") + func endingStalledReconnectReplayFlushesBuffer() { + var progress = SSHPTYAttachOutputProgress( + replayBytes: 100, + suppressReplayBytes: 6, + expectedReplayFingerprint: SSHPTYAttachOutputProgress.fingerprint( + of: Data("oldold".utf8) + ) + ) + let buffered = progress.terminalOutput(from: Data("oldoldnew".utf8), suppressingReplay: true) + #expect(buffered.isEmpty) + + let pending = progress.endReplay() + + #expect(text(pending) == "new") + #expect(progress.replayBytesRemaining == 0) + #expect(progress.deliveredReplayBytes == 9) + let live = progress.terminalOutput(from: Data("typed".utf8), suppressingReplay: true) + #expect(text(live) == "typed") + } + + @Test("a reconnect replay larger than the validation buffer is forwarded, not dropped") + func oversizedValidatedReplayIsForwarded() { + let prefix = Data(repeating: 0x61, count: 6) + let appended = Data(repeating: 0x62, count: (1 << 20) + 100_000) + let tail = Data("tail!".utf8) + let live = Data("live".utf8) + var progress = SSHPTYAttachOutputProgress( + replayBytes: prefix.count + appended.count + tail.count, + suppressReplayBytes: prefix.count, + expectedReplayFingerprint: SSHPTYAttachOutputProgress.fingerprint(of: prefix) + ) + + let stream = prefix + appended + tail + live + var output = Data() + var offset = 0 + while offset < stream.count { + let end = min(offset + 32_768, stream.count) + output.append(progress.terminalOutput( + from: Data(stream[offset.. Date: Tue, 29 Sep 2026 18:03:40 -0700 Subject: [PATCH 21/48] cmux ssh: bound the attach replay phase and never drop buffered replay The daemon declares replay_bytes and the attach forwarded no input until that many bytes arrived, so a peer that declared more than it sent left the pane showing output while holding every keystroke. The replay phase now ends when the bridge is quiet for 2 s or 15 s after ready, whichever comes first; any buffered replay is forwarded, the query filter treats later bytes as live, the stored snapshot records only the bytes actually delivered, and input forwarding starts. On a reconnect with fingerprint validation, a replay larger than the 1 MiB buffer stopped buffering without flushing, losing up to 1 MiB of output. The overflow now forwards the buffered bytes and the rest of the chunk in stream order. Co-Authored-By: Claude Opus 5.5 --- CLI/cmux.swift | 43 ++++++++++++++---- .../SSHPTYAttachOutputProgress.swift | 45 ++++++++++++++----- .../SSHPTYAttachReplayDeadline.swift | 16 +++++-- .../SSHPTYReplayOutputFilter.swift | 7 ++- 4 files changed, 88 insertions(+), 23 deletions(-) diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 99306c87d86c..7584ef2efef2 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -15891,6 +15891,25 @@ struct CMUXCLI { throw CLIError(message: "ssh-pty-attach: bridge write failed") } } + func storeReplayStateIfComplete() { + guard !replayStateStored, outputProgress.replayBytesRemaining == 0 else { return } + replayState.storeSnapshot( + replayBytes: outputProgress.deliveredReplayBytes, + fingerprint: outputProgress.completedReplayFingerprint ?? + SSHPTYAttachOutputProgress.fingerprint(of: Data()) + ) + replayStateStored = true + } + // The replay length is declared by the remote peer. Bound the replay + // phase so a peer that declares more than it sends cannot hold input + // forwarding off indefinitely. + var replayDeadline = SSHPTYAttachReplayDeadline(startedAt: bridgeReadyUptime) + func endStalledReplay() throws { + try writeReplayFilteredOutput(outputProgress.endReplay()) + replayOutputFilter.endReplay() + storeReplayStateIfComplete() + try startInputForwardingAfterReplay() + } try startInputForwardingAfterReplay() func finishBridgeClosedNormally() throws { resizeMonitor.cancel() @@ -15908,9 +15927,24 @@ struct CMUXCLI { var outputBuffer = [UInt8](repeating: 0, count: 32768) while true { + if !inputPumpStarted, outputProgress.replayBytesRemaining > 0 { + let wait = replayDeadline.remainingWait(at: ProcessInfo.processInfo.systemUptime) + var pollFD = pollfd(fd: fd, events: Int16(POLLIN), revents: 0) + let ready = wait > 0 ? poll(&pollFD, 1, Int32(min(wait * 1000, 60_000).rounded(.up))) : 0 + try checkSSHPTYCancellation(signalMonitor) + if ready == 0 { + if replayDeadline.remainingWait(at: ProcessInfo.processInfo.systemUptime) == 0 { + try endStalledReplay() + } + continue + } + // Other poll failures fall through so read reports them. + if ready < 0, errno == EINTR { continue } + } let count = Darwin.read(fd, &outputBuffer, outputBuffer.count) try checkSSHPTYCancellation(signalMonitor) if count > 0 { + replayDeadline.recordOutput(at: ProcessInfo.processInfo.systemUptime) let output = outputProgress.terminalOutput( from: Data(outputBuffer.prefix(count)), suppressingReplay: suppressReplay @@ -15921,14 +15955,7 @@ struct CMUXCLI { } try writeReplayFilteredOutput(output) } - if !replayStateStored, outputProgress.replayBytesRemaining == 0 { - replayState.storeSnapshot( - replayBytes: bridgeReplayBytes, - fingerprint: outputProgress.completedReplayFingerprint ?? - SSHPTYAttachOutputProgress.fingerprint(of: Data()) - ) - replayStateStored = true - } + storeReplayStateIfComplete() try startInputForwardingAfterReplay() } else if count == 0 { try finishBridgeClosedNormally() diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachOutputProgress.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachOutputProgress.swift index ec2dd25be90b..dd6d44f96069 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachOutputProgress.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachOutputProgress.swift @@ -20,6 +20,9 @@ public struct SSHPTYAttachOutputProgress: Sendable { private var validatedReplayOutput = Data() private var replayFingerprintHash = Self.fingerprintOffset + /// Declared replay bytes that actually arrived from the bridge. + public private(set) var deliveredReplayBytes = 0 + /// Whether any output arrived after the initial replay boundary. public private(set) var receivedLiveOutput = false @@ -74,6 +77,7 @@ public struct SSHPTYAttachOutputProgress: Sendable { guard byteCount > 0 else { return } let replayBytes = min(byteCount, replayBytesRemaining) replayBytesRemaining -= replayBytes + deliveredReplayBytes += replayBytes if byteCount > replayBytes { receivedLiveOutput = true } @@ -131,20 +135,24 @@ public struct SSHPTYAttachOutputProgress: Sendable { data.dropFirst(candidateBytes) .prefix(max(0, replayChunkBytes - candidateBytes)) ) - appendValidatedReplayBytes(replayRemainder) if !replayRemainder.isEmpty { receivedLiveOutput = true } + if let overflow = bufferValidatedReplayBytes(replayRemainder) { + return overflow + data.dropFirst(replayChunkBytes) + } return flushValidatedReplayIfComplete(from: data, replayChunkBytes: replayChunkBytes) } if suppressingReplay, expectedReplayFingerprint != nil, bufferingValidatedReplay { - appendValidatedReplayBytes(Data(data.prefix(replayChunkBytes))) if replayChunkBytes > 0 { receivedLiveOutput = true } + if let overflow = bufferValidatedReplayBytes(Data(data.prefix(replayChunkBytes))) { + return overflow + data.dropFirst(replayChunkBytes) + } return flushValidatedReplayIfComplete( from: data, replayChunkBytes: replayChunkBytes @@ -173,14 +181,19 @@ public struct SSHPTYAttachOutputProgress: Sendable { return Data(data.dropFirst(suppressBytes)) } - /// Declared replay bytes that actually arrived from the bridge. - public var deliveredReplayBytes: Int { 0 } - /// Ends the replay phase before the declared byte count arrived. /// + /// The declared length comes from the remote peer. When it overstates the + /// bytes actually sent, the caller's replay deadline ends the phase so + /// input forwarding resumes; everything after this call is live output. + /// The completed fingerprint then covers only the delivered prefix. + /// /// - Returns: Buffered replay output that must still reach the terminal. public mutating func endReplay() -> Data { - Data() + guard replayBytesRemaining > 0 else { return Data() } + replayBytesRemaining = 0 + completedReplayFingerprint = replayFingerprintHash + return finishPendingReplay() } /// Finishes a buffered candidate when the bridge closes before replay ends. @@ -210,16 +223,26 @@ public struct SSHPTYAttachOutputProgress: Sendable { } } - private mutating func appendValidatedReplayBytes(_ data: Data) { - guard !data.isEmpty else { return } + /// Buffers validated replay bytes until the replay completes. + /// + /// - Returns: `nil` while buffering. Once the buffer would exceed its bound, + /// buffering stops and the buffered bytes plus `data` are returned for + /// immediate forwarding, so an overflow never drops output. + private mutating func bufferValidatedReplayBytes(_ data: Data) -> Data? { + guard !data.isEmpty else { return nil } guard validatedReplayOutput.count <= Self.maximumBufferedReplayBytes - data.count else { // The daemon's replay is bounded to the same order of magnitude; - // if an older peer violates that contract, stop buffering rather - // than allowing reconnect validation to grow without bound. + // if a peer violates that contract, stop buffering rather than + // letting reconnect validation grow without bound, and forward + // what was held in stream order. + let overflow = validatedReplayOutput + data + validatedReplayOutput.removeAll(keepingCapacity: false) bufferingValidatedReplay = false - return + receivedLiveOutput = true + return overflow } validatedReplayOutput.append(data) + return nil } private mutating func flushValidatedReplayIfComplete( diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayDeadline.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayDeadline.swift index 65ad25988a7d..dcbb69f48c65 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayDeadline.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayDeadline.swift @@ -23,13 +23,23 @@ public struct SSHPTYAttachReplayDeadline: Sendable, Equatable { startedAt: TimeInterval, idleTimeout: TimeInterval = Self.defaultIdleTimeout, totalTimeout: TimeInterval = Self.defaultTotalTimeout - ) {} + ) { + self.idleTimeout = max(0, idleTimeout) + totalDeadline = startedAt + max(0, totalTimeout) + idleDeadline = startedAt + self.idleTimeout + } + + private let idleTimeout: TimeInterval + private let totalDeadline: TimeInterval + private var idleDeadline: TimeInterval /// Records bridge output that arrived at `now`. - public mutating func recordOutput(at now: TimeInterval) {} + public mutating func recordOutput(at now: TimeInterval) { + idleDeadline = max(idleDeadline, now + idleTimeout) + } /// Seconds left before the replay phase ends; zero once it has expired. public func remainingWait(at now: TimeInterval) -> TimeInterval { - .infinity + max(0, min(idleDeadline, totalDeadline) - now) } } diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift index 7008aa1eba3b..37ea2f6c30ad 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift @@ -96,7 +96,12 @@ public struct SSHPTYReplayOutputFilter: Sendable { } /// Treats every later byte as live output after the replay phase ended early. - public mutating func endReplay() {} + /// + /// A held candidate that began in replay is emitted unchanged with the next + /// chunk, matching how an oversized candidate fails open. + public mutating func endReplay() { + replayBytesRemaining = 0 + } /// Flushes an unterminated candidate when the bridge closes. /// From 06b19096ab2a8db5d9d1c665896c5063b3b97f09 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 18:05:18 -0700 Subject: [PATCH 22/48] cmux ssh: test that relayed remote status withholds the local window workspace.remote.status and the workspace.remote.terminal_session_* methods already narrow `remote` for relay callers, but they still return the local window_id and window_ref. workspace.list withholds the window for relay callers; these responses should too, while local callers keep it. Co-Authored-By: Claude Opus 5.5 --- ...CoordinatorRemoteRelayNarrowingTests.swift | 39 +++++++++++++++++-- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorRemoteRelayNarrowingTests.swift b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorRemoteRelayNarrowingTests.swift index e45872ad2cd6..e37491216afe 100644 --- a/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorRemoteRelayNarrowingTests.swift +++ b/Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorRemoteRelayNarrowingTests.swift @@ -6,6 +6,8 @@ import Testing private final class RemoteRelayNarrowingContext: ControlCommandContext { let workspaceID = UUID() let surfaceID = UUID() + /// The local window hosting the workspace; relay callers never see it. + let windowID = UUID() let status: JSONValue = .object([ "enabled": .bool(true), "state": .string("connected"), @@ -20,7 +22,7 @@ private final class RemoteRelayNarrowingContext: ControlCommandContext { func controlRemoteRelayDispatchError(method: String, params: [String: JSONValue]) -> ControlCallResult? { nil } func controlWorkspaceRemoteStatus(workspaceID: UUID) -> ControlWorkspaceRemoteResolution { - .resolved(windowID: nil, workspaceID: workspaceID, remoteStatus: status) + .resolved(windowID: windowID, workspaceID: workspaceID, remoteStatus: status) } func controlWorkspaceRemoteTerminalSessionLaunching( @@ -29,7 +31,7 @@ private final class RemoteRelayNarrowingContext: ControlCommandContext { terminalLifecycleID: UUID, attemptID: UUID ) -> ControlWorkspaceRemoteTerminalSessionConnectedResolution { - .resolved(windowID: nil, workspaceID: workspaceID, remoteStatus: status) + .resolved(windowID: windowID, workspaceID: workspaceID, remoteStatus: status) } func controlWorkspaceRemoteTerminalSessionConnected( @@ -39,7 +41,7 @@ private final class RemoteRelayNarrowingContext: ControlCommandContext { attemptID: UUID, commitLease: (any ControlRemotePTYLifecycleCommitLease)? ) -> ControlWorkspaceRemoteTerminalSessionConnectedResolution { - .resolved(windowID: nil, workspaceID: workspaceID, remoteStatus: status) + .resolved(windowID: windowID, workspaceID: workspaceID, remoteStatus: status) } func controlWorkspaceRemoteTerminalSessionEnd( @@ -51,7 +53,7 @@ private final class RemoteRelayNarrowingContext: ControlCommandContext { lifecycleID: String?, lifecycleOnly: Bool ) -> ControlWorkspaceRemoteTerminalSessionEndResolution { - .resolved(windowID: nil, workspaceID: workspaceID, remoteStatus: status) + .resolved(windowID: windowID, workspaceID: workspaceID, remoteStatus: status) } func controlNotificationCreateForTarget( @@ -136,6 +138,35 @@ struct ControlCommandCoordinatorRemoteRelayNarrowingTests { } } + @Test func relayedRemoteStatusOmitsTheLocalWindow() throws { + let context = RemoteRelayNarrowingContext() + let coordinator = ControlCommandCoordinator(context: context) + for (method, params) in lifecycleRequests(context) { + let result = dispatch(coordinator, context, method, relayed(params, context)) + guard case .ok(.object(let payload))? = result else { + Issue.record("\(method) failed: \(String(describing: result))") + continue + } + #expect(payload["window_id"] == nil, "\(method) returned window_id \(String(describing: payload["window_id"]))") + #expect(payload["window_ref"] == nil, "\(method) returned window_ref \(String(describing: payload["window_ref"]))") + #expect(payload["workspace_id"] == .string(context.workspaceID.uuidString), "\(method)") + } + } + + @Test func localRemoteStatusKeepsTheLocalWindow() throws { + let context = RemoteRelayNarrowingContext() + let coordinator = ControlCommandCoordinator(context: context) + for (method, params) in lifecycleRequests(context) { + let result = dispatch(coordinator, context, method, params) + guard case .ok(.object(let payload))? = result else { + Issue.record("\(method) failed: \(String(describing: result))") + continue + } + #expect(payload["window_id"] == .string(context.windowID.uuidString), "\(method)") + #expect(payload["window_ref"] != nil, "\(method)") + } + } + @Test func relayedNotificationDropsTheReplyShape() throws { let context = RemoteRelayNarrowingContext() let coordinator = ControlCommandCoordinator(context: context) From 48e1128735a905b5c84a8bf2b21ea3c48837f2d3 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 18:06:50 -0700 Subject: [PATCH 23/48] cmux ssh: withhold the local window from relayed remote status workspace.remote.status and workspace.remote.terminal_session_{launching, connected,end} returned the local window_id and window_ref to relay callers. No remote flow reads them: the lifecycle wrappers discard the response and the Go daemon never calls these methods. Relay callers now get only the workspace/surface echo and the narrowed `remote` state, matching workspace.list. Local socket callers keep the full payload. Co-Authored-By: Claude Opus 5.5 --- .../ControlCommandCoordinator+Workspace.swift | 13 +++---- ...Coordinator+WorkspaceRemoteLifecycle.swift | 34 ++++++++++++------- daemon/remote/README.md | 2 +- .../references/remote-relay-authorization.md | 3 +- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+Workspace.swift b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+Workspace.swift index bae444f2e54e..f5c4c58f5a09 100644 --- a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+Workspace.swift +++ b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+Workspace.swift @@ -773,7 +773,10 @@ extension ControlCommandCoordinator { /// Shapes the shared remote-mutation result for disconnect / reconnect / /// foreground_auth_ready / status. - private func workspaceRemoteResult(_ resolution: ControlWorkspaceRemoteResolution?) -> ControlCallResult { + private func workspaceRemoteResult( + _ resolution: ControlWorkspaceRemoteResolution?, + params: [String: JSONValue] = [:] + ) -> ControlCallResult { switch resolution ?? .missingWorkspaceID { case .missingWorkspaceID: return .err(code: "invalid_params", message: "Missing workspace_id", data: nil) @@ -793,13 +796,11 @@ extension ControlCommandCoordinator { "workspace_ref": ref(.workspace, workspaceID), ])) case .resolved(let windowID, let workspaceID, let remoteStatus): - return .ok(.object([ - "window_id": orNull(windowID?.uuidString), - "window_ref": ref(.window, windowID), + return .ok(.object(addingLocalWindow(windowID, for: params, to: [ "workspace_id": .string(workspaceID.uuidString), "workspace_ref": ref(.workspace, workspaceID), "remote": remoteStatus, - ])) + ]))) } } @@ -908,7 +909,7 @@ extension ControlCommandCoordinator { remoteStatus: self.remoteStatus(remoteStatus, for: params) ) } - return workspaceRemoteResult(status) + return workspaceRemoteResult(status, params: params) } /// `workspace.remote.pty_attach_end` — record a remote PTY attach end. diff --git a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+WorkspaceRemoteLifecycle.swift b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+WorkspaceRemoteLifecycle.swift index db209b359c46..1f3270799a19 100644 --- a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+WorkspaceRemoteLifecycle.swift +++ b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+WorkspaceRemoteLifecycle.swift @@ -50,16 +50,14 @@ extension ControlCommandCoordinator { "attempt_id": .string(attemptID.uuidString), ])) case .resolved(let windowID, let resolvedWorkspaceID, let remoteStatus): - return .ok(.object([ - "window_id": self.orNull(windowID?.uuidString), - "window_ref": self.ref(.window, windowID), + return .ok(.object(self.addingLocalWindow(windowID, for: params, to: [ "workspace_id": self.orNull(resolvedWorkspaceID?.uuidString), "workspace_ref": self.ref(.workspace, resolvedWorkspaceID), "surface_id": .string(surfaceID.uuidString), "surface_ref": self.ref(.surface, surfaceID), "attempt_id": .string(attemptID.uuidString), "remote": self.remoteStatus(remoteStatus, for: params), - ])) + ]))) } } } @@ -218,16 +216,14 @@ extension ControlCommandCoordinator { "relay_port": relayPort.map { .int(Int64($0)) } ?? .null, ])) case .resolved(let windowID, let resolvedWorkspaceID, let remoteStatus): - return .ok(.object([ - "window_id": self.orNull(windowID?.uuidString), - "window_ref": self.ref(.window, windowID), + return .ok(.object(self.addingLocalWindow(windowID, for: params, to: [ "workspace_id": self.orNull(resolvedWorkspaceID?.uuidString), "workspace_ref": self.ref(.workspace, resolvedWorkspaceID), "surface_id": .string(surfaceID.uuidString), "surface_ref": self.ref(.surface, surfaceID), "relay_port": relayPort.map { .int(Int64($0)) } ?? .null, "remote": self.remoteStatus(remoteStatus, for: params), - ])) + ]))) } } if let persistentOwner { @@ -290,19 +286,33 @@ extension ControlCommandCoordinator { "relay_port": relayPort.map { .int(Int64($0)) } ?? .null, ])) case .resolved(let windowID, let resolvedWorkspaceID, let remoteStatus): - return .ok(.object([ - "window_id": orNull(windowID?.uuidString), - "window_ref": ref(.window, windowID), + return .ok(.object(addingLocalWindow(windowID, for: params, to: [ "workspace_id": orNull(resolvedWorkspaceID?.uuidString), "workspace_ref": ref(.workspace, resolvedWorkspaceID), "surface_id": .string(surfaceID.uuidString), "surface_ref": ref(.surface, surfaceID), "relay_port": relayPort.map { .int(Int64($0)) } ?? .null, "remote": self.remoteStatus(remoteStatus, for: params), - ])) + ]))) } } + /// Adds the local window to a remote-status payload, except for a relay + /// caller: the remote host needs its workspace's connection state, not + /// which Mac window shows it. Matches `workspace.list`, which withholds + /// the window from relay callers. + func addingLocalWindow( + _ windowID: UUID?, + for params: [String: JSONValue], + to payload: [String: JSONValue] + ) -> [String: JSONValue] { + guard params["_cmux_remote_workspace_id"] == nil else { return payload } + var payload = payload + payload["window_id"] = orNull(windowID?.uuidString) + payload["window_ref"] = ref(.window, windowID) + return payload + } + /// The `remote` payload for a relay caller carries only connection state; /// the destination, proxy, local ports and daemon details stay on the Mac. nonisolated func remoteStatus(_ status: JSONValue, for params: [String: JSONValue]) -> JSONValue { diff --git a/daemon/remote/README.md b/daemon/remote/README.md index 44db04aae111..97fc9dbc4c47 100644 --- a/daemon/remote/README.md +++ b/daemon/remote/README.md @@ -153,7 +153,7 @@ Authenticated relay details: 2. The app runs a local loopback relay server that requires an HMAC-SHA256 challenge-response before forwarding a command to the real local Unix socket. Authentication is mutual: the CLI sends its own nonce with its MAC and sends nothing further until the relay's success line carries `relay_mac`, an HMAC over a `cmux-relay-server-proof` label, the relay ID and both nonces. Another remote user who binds the forwarded port while it is down cannot produce it. The CLI also refuses a TCP relay address that has no relay credentials. The relay still answers clients that send no nonce with the plain v1 `{"ok":true}`. 3. The remote shell never gets direct access to the local app socket. It only gets the reverse-forwarded relay port plus `~/.cmux/relay/.auth`, which is written with `0600` permissions and removed when the relay stops. -4. Authentication is not authorization. `RemoteRelayCommandPolicy` rejects unlisted methods, command-bearing startup parameters, invalid selectors, and every parameter outside the selected method’s explicit schema. The app then verifies a request HMAC binding the originating workspace and active local SSH controller generation, and validates targets against its live remote terminal identities (`RemoteRelayAuthorizationPolicy`); aliases translate IDs but do not grant ownership. Local/browser panels in a remote workspace are excluded. Dispatch rechecks the controller generation and live ownership before acting; replacing or retiring the controller invalidates previously admitted requests. Input, close, scrollback, and selection reads also recheck the actual terminal target, and relay reads bypass cached topology responses. `surface.split` is withheld because its local fallback can spawn a Mac PTY. `surface.create`, `pane.create`, `surface.respawn`, `surface.send_key`, workspace/window/group creation, and global listing/navigation methods are denied. The `surface.resume.*` methods are not relay methods: a binding carries a command that would run on the Mac, and the app also refuses any relay-origin binding (manaflow-ai/cmux#14907). `notification.create_for_target` accepts no `reply_shape`; the app delivers a relayed notification with the relay origin, no reply, and the remote destination in its title. For a relay caller, `workspace.remote.status` and the `terminal_session_*` lifecycle methods return only `enabled`, `state`, and `connected` in `remote`. `agent.hook.enqueue` admits only Claude lifecycle events (`session-start`, `prompt-submit`, `stop`, `notification`, `session-end`, `pre-tool-use`) with `relay_backed: true` and an owned `workspace_id`/`surface_id`; the app rebuilds the hook environment from those selectors and drops host paths from the payload. `agent.hook.barrier` and decision hooks stay unavailable. Relay-side denials return `remote_relay_denied`; app-side ownership denials return `remote_relay_*_denied` without executing the requested operation. +4. Authentication is not authorization. `RemoteRelayCommandPolicy` rejects unlisted methods, command-bearing startup parameters, invalid selectors, and every parameter outside the selected method’s explicit schema. The app then verifies a request HMAC binding the originating workspace and active local SSH controller generation, and validates targets against its live remote terminal identities (`RemoteRelayAuthorizationPolicy`); aliases translate IDs but do not grant ownership. Local/browser panels in a remote workspace are excluded. Dispatch rechecks the controller generation and live ownership before acting; replacing or retiring the controller invalidates previously admitted requests. Input, close, scrollback, and selection reads also recheck the actual terminal target, and relay reads bypass cached topology responses. `surface.split` is withheld because its local fallback can spawn a Mac PTY. `surface.create`, `pane.create`, `surface.respawn`, `surface.send_key`, workspace/window/group creation, and global listing/navigation methods are denied. The `surface.resume.*` methods are not relay methods: a binding carries a command that would run on the Mac, and the app also refuses any relay-origin binding (manaflow-ai/cmux#14907). `notification.create_for_target` accepts no `reply_shape`; the app delivers a relayed notification with the relay origin, no reply, and the remote destination in its title. For a relay caller, `workspace.remote.status` and the `terminal_session_*` lifecycle methods return only `enabled`, `state`, and `connected` in `remote`, and omit the local `window_id` and `window_ref`. `agent.hook.enqueue` admits only Claude lifecycle events (`session-start`, `prompt-submit`, `stop`, `notification`, `session-end`, `pre-tool-use`) with `relay_backed: true` and an owned `workspace_id`/`surface_id`; the app rebuilds the hook environment from those selectors and drops host paths from the payload. `agent.hook.barrier` and decision hooks stay unavailable. Relay-side denials return `remote_relay_denied`; app-side ownership denials return `remote_relay_*_denied` without executing the requested operation. Integration additions for the relay path: diff --git a/skills/cmux-socket-policy/references/remote-relay-authorization.md b/skills/cmux-socket-policy/references/remote-relay-authorization.md index 2c83c00b38b6..cc222b104f69 100644 --- a/skills/cmux-socket-policy/references/remote-relay-authorization.md +++ b/skills/cmux-socket-policy/references/remote-relay-authorization.md @@ -22,7 +22,8 @@ Relay callers see only what they need. `notification.create_for_target` takes no `reply_shape`, and the app delivers it with the relay origin, no reply and the remote destination in the title. `workspace.remote.status` and the `terminal_session_*` lifecycle methods return only `enabled`, `state` and -`connected` in `remote` for a relay caller. +`connected` in `remote` for a relay caller, and omit `window_id` and +`window_ref`. ## Checklist From bc938645f909bc109ff8adb9cd287227c6002f66 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 17:53:13 -0700 Subject: [PATCH 24/48] cmux ssh: test that npm bootstrap verifies the pinned binary digest A published build installs the remote cmux-tui from npm and then trusts only the binary's self-reported remote-probe. Whoever can publish cmux@ or a platform package gets code on every bootstrapped host. These tests pin the remote platform's SHA-256 in the local npm package's cmux-tui-ssh/manifest.json and expect a package that differs to be refused and removed, while a matching one installs. Co-Authored-By: Claude Opus 5.5 --- .../tests/ssh_cross_platform_bootstrap.rs | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/cmux-tui/crates/cmux-remote/tests/ssh_cross_platform_bootstrap.rs b/cmux-tui/crates/cmux-remote/tests/ssh_cross_platform_bootstrap.rs index f687d8311873..7e0c3d8d4b2c 100644 --- a/cmux-tui/crates/cmux-remote/tests/ssh_cross_platform_bootstrap.rs +++ b/cmux-tui/crates/cmux-remote/tests/ssh_cross_platform_bootstrap.rs @@ -96,3 +96,102 @@ async fn ssh_cross_platform_bootstrap_rejects_tampering_before_remote_upload() { assert!(!fixture.staged.exists()); assert!(!fixture.installed.exists()); } + +/// A published build installs from npm. Whatever npm serves must match the +/// SHA-256 that the local npm package pinned for the remote platform before +/// the binary runs or moves into place. +struct NpmFixture { + _directory: tempfile::TempDir, + config: SshBootstrapConfig, + installed: std::path::PathBuf, + staged: std::path::PathBuf, +} + +impl NpmFixture { + fn new(served: &[u8]) -> Self { + let directory = tempfile::tempdir().unwrap(); + // Same layout as an installed npm platform package: + // bin/cmux-tui next to bin/cmux-tui-ssh/manifest.json. + let package_bin = directory.path().join("bin"); + let pins = package_bin.join("cmux-tui-ssh"); + fs::create_dir_all(&pins).unwrap(); + let source = package_bin.join("cmux-tui"); + fs::write(&source, b"local npm platform executable").unwrap(); + let pinned = b"published linux executable"; + fs::write( + pins.join("manifest.json"), + serde_json::to_vec(&serde_json::json!({ + "commit": BUILD_IDENTITY, + "binaries": { + "cmux-tui-aarch64-unknown-linux-musl": format!("{:x}", Sha256::digest(pinned)), + }, + })) + .unwrap(), + ) + .unwrap(); + let served_file = directory.path().join("served"); + fs::write(&served_file, served).unwrap(); + let installed = directory.path().join("installed"); + let staged = directory.path().join("staged"); + let probe = serde_json::json!({ + "app": "cmux-tui", "version": "9.9.9", "distribution_version": "9.9.9", + "npm_bootstrap_version": "9.9.9", "build_identity": "npm-build", + "remote_protocol": REMOTE_PROTOCOL_VERSION, "os": "linux", "arch": "aarch64", + }); + let script = directory.path().join("ssh"); + fs::write( + &script, + format!( + r#"#!/bin/sh +digest() {{ sha256sum "$1" 2>/dev/null || shasum -a 256 "$1"; }} +case "$*" in + *"uname -s -m"*) printf '%s\n' 'Linux aarch64' ;; + *"mkdir -p "*|*"mkdir -m 700 "*) exit 0 ;; + *".cmux-upload-"*"npm pack "*"cmux-tui-linux-arm64@9.9.9"*) cp '{served}' '{staged}'; digest '{staged}' ;; + *"npx --yes"*) cp '{served}' '{installed}' ;; + *".cmux-upload-"*" remote-probe --json"*) [ -f '{staged}' ] || exit 127; printf '%s' '{probe}' ;; + *"remote-probe --json"*) [ -f '{installed}' ] || exit 127; printf '%s' '{probe}' ;; + *"mv -f "*".cmux-upload-"*) mv '{staged}' '{installed}' ;; + *"rm -f "*".cmux-upload-"*) rm -f '{staged}' ;; + *"rmdir "*) exit 0 ;; + *) exit 2 ;; +esac +"#, + served = served_file.display(), + staged = staged.display(), + installed = installed.display(), + ), + ) + .unwrap(); + fs::set_permissions(&script, fs::Permissions::from_mode(0o755)).unwrap(); + let mut config = SshBootstrapConfig::defaults("host"); + config.ssh_binary = script.to_string_lossy().into_owned(); + config.package_version = "9.9.9".into(); + config.package_installable = true; + config.local_binary = Some(source); + Self { _directory: directory, config, installed, staged } + } +} + +#[tokio::test] +async fn ssh_npm_bootstrap_refuses_a_package_that_differs_from_the_pinned_digest() { + let fixture = NpmFixture::new(b"attacker-published executable"); + let result = SshBootstrapper::new(fixture.config).unwrap().ensure_installed().await; + assert!( + matches!(&result, Err(error) if error.to_string().contains("checksum")), + "an npm package that differs from the pinned SHA-256 was trusted: {result:?}" + ); + assert!(!fixture.installed.exists(), "the unverified npm binary was left installed"); + assert!(!fixture.staged.exists(), "the unverified npm binary was left staged"); +} + +#[tokio::test] +async fn ssh_npm_bootstrap_installs_a_package_that_matches_the_pinned_digest() { + let fixture = NpmFixture::new(b"published linux executable"); + assert_eq!( + SshBootstrapper::new(fixture.config).unwrap().ensure_installed().await.unwrap(), + BootstrapOutcome::Installed + ); + assert_eq!(fs::read(&fixture.installed).unwrap(), b"published linux executable"); + assert!(!fixture.staged.exists()); +} From e7ebb274a44790245308edc8fcc563c2690a2db6 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 18:06:19 -0700 Subject: [PATCH 25/48] cmux ssh: verify npm-installed remote binaries against pinned digests Published builds installed the remote cmux-tui with `npx --yes cmux@ install-self`, which runs npm-served code (and install scripts) and then trusts the binary's own remote-probe. Each npm platform package now ships bin/cmux-tui-ssh/manifest.json, generated at package time with the SHA-256 of every published remote binary and the build commit. When that manifest is present, the bootstrap fetches the remote platform's package with `npm pack --ignore-scripts` into its exclusive 0700 staging directory, extracts bin/cmux-tui without running anything, and compares the remote sha256sum/shasum/openssl digest with the pinned one. A mismatch removes the download and fails; a match is probed in staging and only then moved into place, sharing the upload path's promotion step. Builds stamped with an npm version but shipped without the manifest (a PyPI wheel from the same release run, custom builds) keep the npx path, now with --ignore-scripts. Dev and source builds are unchanged: they upload their own binary or the checksum-verified companion. The package contract and its tests require the manifest in every cmux-tui platform package and check each digest against the packaged binary. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/cmux-tui-build-package.yml | 4 + .../crates/cmux-remote/src/ssh_artifacts.rs | 118 ++++++++--- .../crates/cmux-remote/src/ssh_bootstrap.rs | 189 ++++++++++++++++-- cmux-tui/dist/scripts/package_contract.py | 72 ++++++- cmux-tui/dist/scripts/package_npm.py | 67 ++++++- .../dist/scripts/validate_package_contract.py | 2 + cmux-tui/docs/remote.md | 2 +- tests/test_tui_npm_package_artifact.py | 26 ++- tests/test_tui_package_contract.py | 97 ++++++++- 9 files changed, 520 insertions(+), 57 deletions(-) diff --git a/.github/workflows/cmux-tui-build-package.yml b/.github/workflows/cmux-tui-build-package.yml index af7980aa47b1..244f53187c8c 100644 --- a/.github/workflows/cmux-tui-build-package.yml +++ b/.github/workflows/cmux-tui-build-package.yml @@ -611,9 +611,13 @@ jobs: - name: Build npm package directories if: inputs.package_npm run: | + # The build jobs stamp this same checkout's HEAD as + # CMUX_TUI_BUILD_COMMIT; the SSH bootstrap accepts the pinned + # remote-binary digests only for that build identity. python3 cmux-tui/dist/scripts/package_npm.py \ --binaries-dir dist/binaries \ --version "$NPM_VERSION" \ + --build-commit "$(git rev-parse HEAD)" \ ${{ inputs.include_windows && '--include-windows' || '' }} \ --out dist/npm-packages diff --git a/cmux-tui/crates/cmux-remote/src/ssh_artifacts.rs b/cmux-tui/crates/cmux-remote/src/ssh_artifacts.rs index 2954f2a030cd..7e0a0187473b 100644 --- a/cmux-tui/crates/cmux-remote/src/ssh_artifacts.rs +++ b/cmux-tui/crates/cmux-remote/src/ssh_artifacts.rs @@ -2,6 +2,8 @@ //! //! The packager authenticates the release manifest before embedding it. Runtime //! accepts only that client's exact build and checks the payload before SSH sees it. +//! npm platform packages ship the same manifest without payloads; the bootstrap +//! then checks the npm-downloaded binary on the remote before running it. use std::collections::HashMap; use std::io::Read; @@ -18,45 +20,95 @@ struct ArtifactManifest { binaries: HashMap, } +/// The SHA-256 digests this client build pins for every remote platform, read +/// from `cmux-tui-ssh/manifest.json` next to the local executable. The native +/// app ships the payloads beside it; an npm platform package ships only the +/// manifest, and the remote then downloads the payload from npm. +pub(crate) struct PinnedArtifacts { + directory: PathBuf, + manifest: ArtifactManifest, +} + +/// A remote platform's pinned digest and the npm package that publishes it. +pub(crate) struct PinnedPlatform { + pub(crate) sha256: String, + pub(crate) npm_package: &'static str, +} + +impl PinnedArtifacts { + /// Returns `None` when no manifest ships with this build. Dev and source + /// builds lack one; any manifest that exists must name this exact build. + pub(crate) fn load( + executable: &Path, + build_identity: &str, + ) -> Result, BootstrapError> { + let Some(parent) = executable.parent() else { return Ok(None) }; + let directory = parent.join("cmux-tui-ssh"); + let bytes = match std::fs::read(directory.join("manifest.json")) { + Ok(bytes) => bytes, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(BootstrapError::Io(error)), + }; + let manifest: ArtifactManifest = serde_json::from_slice(&bytes) + .map_err(|_| BootstrapError::Configuration("invalid SSH artifact manifest".into()))?; + if manifest.commit != build_identity { + return Err(BootstrapError::Configuration( + "SSH artifact manifest belongs to a different client build".into(), + )); + } + Ok(Some(Self { directory, manifest })) + } + + /// Returns `None` for a platform no release publishes. A published + /// platform without a well-formed digest is a packaging failure and must + /// not fall back to an unverified install. + pub(crate) fn platform( + &self, + os: &str, + arch: &str, + ) -> Result, BootstrapError> { + let Some((target, npm_package)) = release_target(os, arch) else { return Ok(None) }; + let sha256 = self + .manifest + .binaries + .get(&format!("cmux-tui-{target}")) + .filter(|digest| { + digest.len() == 64 && digest.bytes().all(|byte| byte.is_ascii_hexdigit()) + }) + .ok_or_else(|| { + BootstrapError::Configuration( + "SSH artifact manifest lacks a checksum for this platform".into(), + ) + })? + .to_ascii_lowercase(); + Ok(Some(PinnedPlatform { sha256, npm_package })) + } +} + +/// The Rust target and npm platform package for a normalized remote platform. +fn release_target(os: &str, arch: &str) -> Option<(&'static str, &'static str)> { + match (os, arch) { + ("linux", "aarch64") => Some(("aarch64-unknown-linux-musl", "cmux-tui-linux-arm64")), + ("linux", "x86_64") => Some(("x86_64-unknown-linux-musl", "cmux-tui-linux-x64")), + ("macos", "aarch64") => Some(("aarch64-apple-darwin", "cmux-tui-darwin-arm64")), + ("macos", "x86_64") => Some(("x86_64-apple-darwin", "cmux-tui-darwin-x64")), + _ => None, + } +} + pub(crate) fn payload( executable: &Path, build_identity: &str, os: &str, arch: &str, ) -> Result, BootstrapError> { - let Some(parent) = executable.parent() else { return Ok(None) }; - let directory = parent.join("cmux-tui-ssh"); - let manifest_path = directory.join("manifest.json"); - let bytes = match std::fs::read(&manifest_path) { - Ok(bytes) => bytes, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), - Err(error) => return Err(BootstrapError::Io(error)), - }; - let manifest: ArtifactManifest = serde_json::from_slice(&bytes) - .map_err(|_| BootstrapError::Configuration("invalid SSH artifact manifest".into()))?; - if manifest.commit != build_identity { - return Err(BootstrapError::Configuration( - "SSH artifact manifest belongs to a different client build".into(), - )); - } - let target = match (os, arch) { - ("linux", "aarch64") => "aarch64-unknown-linux-musl", - ("linux", "x86_64") => "x86_64-unknown-linux-musl", - ("macos", "aarch64") => "aarch64-apple-darwin", - ("macos", "x86_64") => "x86_64-apple-darwin", - _ => return Ok(None), + let Some(pinned) = PinnedArtifacts::load(executable, build_identity)? else { + return Ok(None); }; - let name = format!("cmux-tui-{target}"); - let expected = manifest - .binaries - .get(&name) - .filter(|digest| digest.len() == 64 && digest.bytes().all(|byte| byte.is_ascii_hexdigit())) - .ok_or_else(|| { - BootstrapError::Configuration( - "SSH artifact manifest lacks a checksum for this platform".into(), - ) - })?; - let path = directory.join(name); + let Some(platform) = pinned.platform(os, arch)? else { return Ok(None) }; + let Some((target, _)) = release_target(os, arch) else { return Ok(None) }; + let expected = platform.sha256; + let path = pinned.directory.join(format!("cmux-tui-{target}")); let mut input = std::fs::File::open(&path).map_err(BootstrapError::Io)?; let mut digest = Sha256::new(); let mut buffer = [0_u8; 64 * 1024]; @@ -67,7 +119,7 @@ pub(crate) fn payload( } digest.update(&buffer[..count]); } - if format!("{:x}", digest.finalize()) != expected.to_ascii_lowercase() { + if format!("{:x}", digest.finalize()) != expected { return Err(BootstrapError::Configuration("SSH artifact checksum mismatch".into())); } Ok(Some(path)) diff --git a/cmux-tui/crates/cmux-remote/src/ssh_bootstrap.rs b/cmux-tui/crates/cmux-remote/src/ssh_bootstrap.rs index 900b5b77a2c8..b1b9efd7097a 100644 --- a/cmux-tui/crates/cmux-remote/src/ssh_bootstrap.rs +++ b/cmux-tui/crates/cmux-remote/src/ssh_bootstrap.rs @@ -202,6 +202,65 @@ impl SshBootstrapper { if !self.config.package_installable { return self.install_local_binary().await; } + let pinned = match self.config.local_binary.as_deref() { + Some(executable) => crate::ssh_artifacts::PinnedArtifacts::load( + executable, + &self.config.build_identity, + )?, + None => None, + }; + if let Some(pinned) = pinned { + let remote = self.remote_platform().await?; + if let Some(platform) = pinned.platform(&remote.os, &remote.arch)? { + return self.install_pinned_package(&platform).await; + } + } + self.install_unpinned_package().await + } + + /// Downloads the remote platform's npm package without running any of + /// its code, checks the binary against the digest this build pins, and + /// only then probes and installs it. A mismatch removes the download. + async fn install_pinned_package( + &self, + platform: &crate::ssh_artifacts::PinnedPlatform, + ) -> Result { + let deadline = Instant::now() + self.config.timeout; + let temporary_dir = self.temporary_upload_path(); + let temporary = format!("{temporary_dir}/payload"); + self.create_remote_staging(self.remote_parent(), &temporary_dir).await?; + let package = format!("{}@{}", platform.npm_package, self.config.package_version); + let command = pinned_package_command(&temporary_dir, &package); + let output = match self.run_remote([command.as_str()]).await { + Ok(output) => output, + Err(error) => { + self.cleanup_remote_staging(&temporary_dir, deadline).await; + return Err(error); + } + }; + if output.status != 0 { + self.cleanup_remote_staging(&temporary_dir, deadline).await; + return Err(BootstrapError::Install { + status: output.status, + stderr: sanitize(&String::from_utf8_lossy(&output.stderr)), + }); + } + let actual = String::from_utf8_lossy(&output.stdout) + .split_whitespace() + .next() + .unwrap_or_default() + .to_ascii_lowercase(); + if actual != platform.sha256 { + self.cleanup_remote_staging(&temporary_dir, deadline).await; + return Err(BootstrapError::ChecksumMismatch { package }); + } + self.promote_staged(&temporary, &temporary_dir, deadline).await + } + + /// Builds without a pinned manifest (for example, a PyPI wheel or a + /// custom build stamped with an npm version) still install through npx. + /// Only the probe vouches for that binary; install scripts never run. + async fn install_unpinned_package(&self) -> Result { let npm_package = &self.config.npm_package; let package_version = &self.config.package_version; let package = format!("{npm_package}@{package_version}"); @@ -209,6 +268,7 @@ impl SshBootstrapper { .run_remote([ "npx", "--yes", + "--ignore-scripts", package.as_str(), "install-self", "--destination", @@ -276,16 +336,11 @@ impl SshBootstrapper { let source = artifact.as_deref().unwrap_or(source); let temporary_dir = self.temporary_upload_path(); let temporary = format!("{temporary_dir}/payload"); - let parent = self - .config - .remote_binary - .rsplit_once('/') - .map_or(".", |(parent, _)| if parent.is_empty() { "/" } else { parent }); // Create the directory in a separate, exclusive command. Cleanup is // allowed only after this command reports success, which proves that // this upload owns the staging directory. A failed or timed-out mkdir // is intentionally left untouched because ownership is unknown. - let encoding = self.create_remote_staging(parent, &temporary_dir).await?; + let encoding = self.create_remote_staging(self.remote_parent(), &temporary_dir).await?; let command = upload_command(&temporary, encoding); let output = match self.run_remote_with_input(&command, source, encoding).await { Ok(output) => output, @@ -301,22 +356,40 @@ impl SshBootstrapper { stderr: sanitize(&String::from_utf8_lossy(&output.stderr)), }); } - let probe = match self.probe_binary(&temporary).await { + self.promote_staged(&temporary, &temporary_dir, deadline).await + } + + fn remote_parent(&self) -> &str { + self.config + .remote_binary + .rsplit_once('/') + .map_or(".", |(parent, _)| if parent.is_empty() { "/" } else { parent }) + } + + /// Probes a verified staged binary, then moves it over the installed one. + /// A staged binary that fails the probe is removed and never installed. + async fn promote_staged( + &self, + temporary: &str, + temporary_dir: &str, + deadline: Instant, + ) -> Result { + let probe = match self.probe_binary(temporary).await { Ok(Some(probe)) => probe, Ok(None) => { - self.cleanup_remote_staging(&temporary_dir, deadline).await; + self.cleanup_remote_staging(temporary_dir, deadline).await; return Err(BootstrapError::Install { status: 126, - stderr: "uploaded binary could not run remote-probe".into(), + stderr: "staged binary could not run remote-probe".into(), }); } Err(error) => { - self.cleanup_remote_staging(&temporary_dir, deadline).await; + self.cleanup_remote_staging(temporary_dir, deadline).await; return Err(error); } }; if !self.compatible(&probe) { - self.cleanup_remote_staging(&temporary_dir, deadline).await; + self.cleanup_remote_staging(temporary_dir, deadline).await; return Err(BootstrapError::Incompatible { version: probe.version, protocol: probe.remote_protocol, @@ -331,12 +404,12 @@ impl SshBootstrapper { let output = match self.run_remote([command.as_str()]).await { Ok(output) => output, Err(error) => { - self.cleanup_remote_staging(&temporary_dir, deadline).await; + self.cleanup_remote_staging(temporary_dir, deadline).await; return Err(error); } }; if output.status != 0 { - self.cleanup_remote_staging(&temporary_dir, deadline).await; + self.cleanup_remote_staging(temporary_dir, deadline).await; return Err(BootstrapError::Install { status: output.status, stderr: sanitize(&String::from_utf8_lossy(&output.stderr)), @@ -345,7 +418,7 @@ impl SshBootstrapper { let Some(probe) = self.probe().await? else { return Err(BootstrapError::Install { status: 0, - stderr: "upload completed but the remote binary is absent".into(), + stderr: "install completed but the remote binary is absent".into(), }); }; if !self.compatible(&probe) { @@ -662,6 +735,27 @@ fn upload_command(temporary: &str, encoding: UploadEncoding) -> String { format!("umask 077; (set -C; exec 3> {temporary} && {writer} >&3) && chmod 755 {temporary}") } +/// Build the remote command that downloads a published platform package into +/// the exclusive staging directory and prints the extracted binary's SHA-256. +/// `npm pack` only fetches the tarball: no package code or install script +/// runs before the caller compares the digest. The package name and version +/// are validated and unscoped, so the tarball name is known in advance and no +/// glob is needed. +fn pinned_package_command(temporary_dir: &str, package: &str) -> String { + let tarball = format!("{}.tgz", package.replacen('@', "-", 1)); + format!( + "umask 077; cd {temporary_dir} || exit 1; \ + npm pack --ignore-scripts --silent {package} >/dev/null && \ + tar -xzf {tarball} package/bin/cmux-tui && \ + mv package/bin/cmux-tui payload && chmod 755 payload; rc=$?; \ + rm -f {tarball} package/bin/cmux-tui; rmdir package/bin package 2>/dev/null; \ + [ \"$rc\" -eq 0 ] || exit \"$rc\"; \ + sha256sum payload 2>/dev/null || shasum -a 256 payload 2>/dev/null || \ + openssl dgst -sha256 -r payload 2>/dev/null || \ + {{ echo 'cannot verify the npm package: the remote host has no sha256sum, shasum or openssl' >&2; exit 1; }}" + ) +} + /// Compressed upload bytes, in order, or the read error that ended them. type UploadChunks = mpsc::Receiver>>; @@ -834,6 +928,7 @@ pub enum BootstrapError { LocalBinaryIncompatible { local: String, remote: String }, WindowsRequiresWsl, Incompatible { version: String, protocol: u8 }, + ChecksumMismatch { package: String }, } impl fmt::Display for BootstrapError { @@ -865,6 +960,10 @@ impl fmt::Display for BootstrapError { Self::WindowsRequiresWsl => formatter.write_str( "native Windows cannot host the cmux-tui remote daemon yet; install a WSL 2 Linux distro with `wsl --install -d Ubuntu`, then connect through that Linux environment" ), + Self::ChecksumMismatch { package } => write!( + formatter, + "npm package {package} does not match the SHA-256 checksum this cmux-tui build pins; the download was removed" + ), Self::Incompatible { version, protocol } => write!( formatter, "remote cmux-tui {version} uses remote protocol {protocol}, expected {REMOTE_PROTOCOL_VERSION}" @@ -920,6 +1019,68 @@ mod tests { fifo.to_string_lossy().into_owned() } + /// Runs the real staging command in `sh` against a stand-in `npm` that + /// only writes a tarball, as `npm pack` does. The command must extract + /// the binary without running it, report its SHA-256 and leave nothing + /// but the payload behind. + #[cfg(unix)] + #[test] + fn pinned_package_command_extracts_and_hashes_without_running_the_package() { + use sha2::{Digest, Sha256}; + use std::fs; + use std::os::unix::fs::PermissionsExt; + + let directory = tempfile::tempdir().unwrap(); + let bin = directory.path().join("fake-bin"); + let source = directory.path().join("source"); + let staging = directory.path().join("staging"); + fs::create_dir_all(source.join("package/bin")).unwrap(); + fs::create_dir(&bin).unwrap(); + fs::create_dir(&staging).unwrap(); + let marker = directory.path().join("package-ran"); + let binary = format!("#!/bin/sh\ntouch '{}'\n", marker.display()); + fs::write(source.join("package/bin/cmux-tui"), &binary).unwrap(); + fs::set_permissions(source.join("package/bin/cmux-tui"), fs::Permissions::from_mode(0o755)) + .unwrap(); + fs::write(source.join("package/package.json"), b"{}").unwrap(); + fs::write( + bin.join("npm"), + format!( + "#!/bin/sh\n[ \"$1 $2 $3 $4\" = 'pack --ignore-scripts --silent cmux-tui-linux-arm64@9.9.9' ] || exit 9\ntar -czf cmux-tui-linux-arm64-9.9.9.tgz -C '{}' package\n", + source.display() + ), + ) + .unwrap(); + fs::set_permissions(bin.join("npm"), fs::Permissions::from_mode(0o755)).unwrap(); + + let command = + pinned_package_command(&staging.to_string_lossy(), "cmux-tui-linux-arm64@9.9.9"); + let output = std::process::Command::new("sh") + .arg("-c") + .arg(&command) + .env("PATH", format!("{}:{}", bin.display(), std::env::var("PATH").unwrap_or_default())) + .output() + .unwrap(); + + assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr)); + let reported = String::from_utf8_lossy(&output.stdout); + assert_eq!( + reported.split_whitespace().next(), + Some(format!("{:x}", Sha256::digest(binary.as_bytes())).as_str()) + ); + let entries = fs::read_dir(&staging) + .unwrap() + .map(|entry| entry.unwrap().file_name().into_string().unwrap()) + .collect::>(); + assert_eq!(entries, ["payload"]); + assert_eq!(fs::read(staging.join("payload")).unwrap(), binary.as_bytes()); + assert_eq!( + fs::metadata(staging.join("payload")).unwrap().permissions().mode() & 0o777, + 0o755 + ); + assert!(!marker.exists(), "the downloaded package ran before verification"); + } + #[test] fn upload_command_writes_only_after_exclusive_directory_creation() { let payload = "~/.local/bin/.cmux-upload-test/payload"; diff --git a/cmux-tui/dist/scripts/package_contract.py b/cmux-tui/dist/scripts/package_contract.py index 6284d0331f80..63b5a9caa19a 100644 --- a/cmux-tui/dist/scripts/package_contract.py +++ b/cmux-tui/dist/scripts/package_contract.py @@ -8,6 +8,7 @@ import hashlib import io import json +import re import zipfile from dataclasses import dataclass from pathlib import Path @@ -52,7 +53,19 @@ class NpmTarget: *NPM_PLATFORM_NAMES_WITH_WINDOWS, *NPM_RELAY_PLATFORM_NAMES_WITH_WINDOWS, ) -NPM_PLATFORM_FILES = frozenset({"package.json", "bin/cmux-tui", "bin/cmux-tui-hook"}) +# Every cmux-tui platform package pins the SHA-256 of each remote binary the +# SSH bootstrap may download from npm. It sits next to bin/cmux-tui, where +# cmux-remote looks for it. +NPM_SSH_MANIFEST = "bin/cmux-tui-ssh/manifest.json" +NPM_SSH_MANIFEST_BINARIES = { + "cmux-tui-aarch64-unknown-linux-musl": "cmux-tui-linux-arm64", + "cmux-tui-x86_64-unknown-linux-musl": "cmux-tui-linux-x64", + "cmux-tui-aarch64-apple-darwin": "cmux-tui-darwin-arm64", + "cmux-tui-x86_64-apple-darwin": "cmux-tui-darwin-x64", +} +NPM_PLATFORM_FILES = frozenset( + {"package.json", "bin/cmux-tui", "bin/cmux-tui-hook", NPM_SSH_MANIFEST} +) NPM_LAUNCHER_FILES = frozenset({"package.json", "bin/cmux.js"}) NPM_RELAY_PLATFORM_FILES = frozenset( {"package.json", "bin/chatmux-relay", "bin/cmux-tui"} @@ -190,6 +203,50 @@ def _validate_version(actual: object, expected: str | None, label: str) -> str: return actual +def _sha256_file(path: Path) -> str: + digest = hashlib.sha256() + with path.open("rb") as file: + for chunk in iter(lambda: file.read(1024 * 1024), b""): + digest.update(chunk) + return digest.hexdigest() + + +def _validate_ssh_manifests(packages_dir: Path, targets: tuple[NpmTarget, ...]) -> None: + """Require one identical SSH digest manifest that matches the packages. + + The SSH bootstrap trusts a remote binary downloaded from npm only when it + matches this manifest, so each digest must be the packaged binary's own. + """ + + manifests = { + target.name: _read_json(packages_dir / target.name / NPM_SSH_MANIFEST) + for target in targets + } + first = next(iter(manifests.values())) + for name, manifest in manifests.items(): + if manifest != first: + raise _error(f"{name}: SSH manifest differs from the other platform packages") + if set(first) != {"commit", "binaries"}: + raise _error("SSH manifest must contain only commit and binaries") + commit = first.get("commit") + if not isinstance(commit, str) or not re.fullmatch( + r"[0-9a-f]{40}(?:[0-9a-f]{24})?", commit + ): + raise _error("SSH manifest commit must be a full lowercase Git commit ID") + binaries = first.get("binaries") + if not isinstance(binaries, dict) or set(binaries) != set(NPM_SSH_MANIFEST_BINARIES): + raise _error( + "SSH manifest must pin exactly " + f"{sorted(NPM_SSH_MANIFEST_BINARIES)}, found {sorted(binaries or {})}" + ) + for artifact, package in NPM_SSH_MANIFEST_BINARIES.items(): + actual = _sha256_file(packages_dir / package / "bin" / "cmux-tui") + if binaries[artifact] != actual: + raise _error( + f"SSH manifest digest for {artifact} does not match {package}/bin/cmux-tui" + ) + + def _validate_package_files( package_dir: Path, target: NpmTarget, @@ -198,6 +255,7 @@ def _validate_package_files( expected_files: frozenset[str], label: str, version: str, + data_files: tuple[str, ...] = (), ) -> None: metadata = _read_json(package_dir / "package.json") if metadata.get("name") != target.name: @@ -206,12 +264,17 @@ def _validate_package_files( if metadata.get("os") != [target.os] or metadata.get("cpu") != [target.cpu]: raise _error(f"{label}: os/cpu selectors are incorrect") extension = ".exe" if target.os == "win32" else "" - files = [f"bin/{name}{extension}" for name in binary_names] + files = [f"bin/{name}{extension}" for name in binary_names] + list(data_files) if metadata.get("files") != files: raise _error(f"{label}: files must be {files}") files_on_disk = _files_below(package_dir) entries = _entries_below(package_dir) - expected_entries = expected_files | {"bin"} + expected_entries = expected_files | { + str(parent.as_posix()) + for path in expected_files + for parent in Path(path).parents + if parent != Path(".") + } if entries != expected_entries: raise _error( f"{label}: directory tree mismatch: expected " @@ -339,11 +402,14 @@ def validate_npm_tree( "package.json", f"bin/cmux-tui{'.exe' if target.os == 'win32' else ''}", f"bin/cmux-tui-hook{'.exe' if target.os == 'win32' else ''}", + NPM_SSH_MANIFEST, } ), label=target.name, version=package_version, + data_files=(NPM_SSH_MANIFEST,), ) + _validate_ssh_manifests(packages_dir, targets) for target in relay_targets: _validate_package_files( packages_dir / target.name, diff --git a/cmux-tui/dist/scripts/package_npm.py b/cmux-tui/dist/scripts/package_npm.py index 0d4098f70714..6727554b9e01 100644 --- a/cmux-tui/dist/scripts/package_npm.py +++ b/cmux-tui/dist/scripts/package_npm.py @@ -4,6 +4,7 @@ from __future__ import annotations import argparse +import hashlib import json import re import shutil @@ -50,6 +51,18 @@ for target in TARGETS ] +# Rust targets that the SSH bootstrap can install on a remote host, matching +# cmux-remote's ssh_artifacts target table. +SSH_TARGETS = ( + "aarch64-unknown-linux-musl", + "x86_64-unknown-linux-musl", + "aarch64-apple-darwin", + "x86_64-apple-darwin", +) +# Next to bin/cmux-tui, where cmux-remote looks for pinned SSH digests. +SSH_MANIFEST = "bin/cmux-tui-ssh/manifest.json" +BUILD_COMMIT_RE = re.compile(r"^[0-9a-f]{40}(?:[0-9a-f]{24})?$") + VERSION_RE = re.compile( r"^(?:[0-9]+\.[0-9]+\.[0-9]+(?:-rc\.[0-9]+)?|[0-9]+\.[0-9]+\.[0-9]+-nightly\.[0-9]{8}\.[0-9]+)$" ) @@ -79,6 +92,14 @@ def parse_args() -> argparse.Namespace: type=Path, help="Output directory for generated npm package directories.", ) + parser.add_argument( + "--build-commit", + required=True, + help=( + "Commit stamped into the binaries as CMUX_TUI_BUILD_COMMIT. The SSH " + "bootstrap accepts the pinned digests only for this build." + ), + ) parser.add_argument("--include-windows", action="store_true") return parser.parse_args() @@ -102,7 +123,39 @@ def recreate_dir(path: Path) -> None: path.mkdir(parents=True) -def package_platforms(binaries_dir: Path, version: str, out_dir: Path, include_windows: bool) -> None: +def sha256_file(path: Path) -> str: + digest = hashlib.sha256() + with path.open("rb") as file: + for chunk in iter(lambda: file.read(1024 * 1024), b""): + digest.update(chunk) + return digest.hexdigest() + + +def ssh_manifest(binaries_dir: Path, build_commit: str) -> dict: + """Pin the SHA-256 of every remote cmux-tui this release publishes. + + The SSH bootstrap downloads a remote host's binary from npm and refuses it + unless it matches the digest pinned here, so the package a user already + runs locally decides which remote bytes are trusted. + """ + + binaries = {} + for target in SSH_TARGETS: + path = binaries_dir / f"cmux-tui-{target}" + if not path.is_file(): + raise SystemExit(f"missing binary: {path}") + binaries[f"cmux-tui-{target}"] = sha256_file(path) + return {"commit": build_commit, "binaries": binaries} + + +def package_platforms( + binaries_dir: Path, + version: str, + out_dir: Path, + include_windows: bool, + build_commit: str, +) -> None: + manifest = ssh_manifest(binaries_dir, build_commit) targets = TARGETS if include_windows else [t for t in TARGETS if t["os"] != "win32"] relay_targets = RELAY_TARGETS if include_windows else [t for t in RELAY_TARGETS if t["os"] != "win32"] for target in targets: @@ -118,6 +171,9 @@ def package_platforms(binaries_dir: Path, version: str, out_dir: Path, include_w recreate_dir(package_dir) copy_executable(src, package_dir / "bin" / f"cmux-tui{ext}") copy_executable(hook_src, package_dir / "bin" / f"cmux-tui-hook{ext}") + manifest_path = package_dir / SSH_MANIFEST + manifest_path.parent.mkdir(parents=True) + write_json(manifest_path, manifest) write_json( package_dir / "package.json", @@ -136,7 +192,7 @@ def package_platforms(binaries_dir: Path, version: str, out_dir: Path, include_w "license": "MIT", "os": [target["os"]], "cpu": [target["cpu"]], - "files": [f"bin/cmux-tui{ext}", f"bin/cmux-tui-hook{ext}"], + "files": [f"bin/cmux-tui{ext}", f"bin/cmux-tui-hook{ext}", SSH_MANIFEST], }, ) @@ -228,13 +284,18 @@ def main() -> None: "X.Y.Z-nightly.YYYYMMDD.N" ) + if not BUILD_COMMIT_RE.fullmatch(args.build_commit): + raise SystemExit("--build-commit must be a full lowercase Git commit ID") + binaries_dir = args.binaries_dir.resolve() out_dir = args.out.resolve() if not binaries_dir.is_dir(): raise SystemExit(f"--binaries-dir is not a directory: {binaries_dir}") out_dir.mkdir(parents=True, exist_ok=True) - package_platforms(binaries_dir, args.version, out_dir, args.include_windows) + package_platforms( + binaries_dir, args.version, out_dir, args.include_windows, args.build_commit + ) package_launcher(args.version, out_dir, args.include_windows) diff --git a/cmux-tui/dist/scripts/validate_package_contract.py b/cmux-tui/dist/scripts/validate_package_contract.py index b525a45aa11a..b1279707fa4c 100644 --- a/cmux-tui/dist/scripts/validate_package_contract.py +++ b/cmux-tui/dist/scripts/validate_package_contract.py @@ -300,6 +300,7 @@ def _validate_npm_archive(archive: Path, package_name: str) -> None: from package_contract import ( NPM_LAUNCHER_FILES, NPM_RELAY_LAUNCHER_FILES, + NPM_SSH_MANIFEST, ) if package_name == "cmux": @@ -322,6 +323,7 @@ def _validate_npm_archive(archive: Path, package_name: str) -> None: "package.json", f"bin/cmux-tui{extension}", f"bin/cmux-tui-hook{extension}", + NPM_SSH_MANIFEST, } ) expected_names = {f"package/{path}" for path in expected} diff --git a/cmux-tui/docs/remote.md b/cmux-tui/docs/remote.md index b9b10d675833..924108823aff 100644 --- a/cmux-tui/docs/remote.md +++ b/cmux-tui/docs/remote.md @@ -108,7 +108,7 @@ cmux-tui remote ssh user@linux-server --session dev The SSH path probes for `~/.local/bin/cmux-tui`, then carries framed data over SSH stdio. It keeps a durable headless mux owner and a replaceable remote sidecar as separate processes. Existing terminal panes therefore survive sidecar replacement. If the named tmux-style session already has a live mux socket, the sidecar attaches without creating a second session. Otherwise cmux starts the user-scoped mux owner on demand. -If the binary is absent or reports an understood incompatible probe, an npm-backed release or nightly binary runs its exact embedded npm distribution version and installs it under the remote user's home directory. Source builds, raw commit-addressed artifacts, PyPI-only builds, and other binaries without an npm bootstrap stamp fail with an instruction to preinstall the same binary; they never substitute an older npm release. An unrecognized legacy probe fails closed; use explicit `--upgrade` from an npm-backed binary to force the pinned install and replace an SSH-managed sidecar. npm verifies the pinned package integrity, and a post-install probe checks the distribution, npm bootstrap, and protocol versions. Release CI verifies that the packaged binary reports the same distribution and npm bootstrap version. Automatic install requires Node.js 18 or newer, `npx`, registry access, and write access to `--remote-binary`; the published Linux packages use static musl binaries and do not require a glibc runtime. Use `--no-install` to require a preinstalled binary. A running sidecar is never silently replaced or restarted. +If the binary is absent or reports an understood incompatible probe, an npm-backed release or nightly binary runs its exact embedded npm distribution version and installs it under the remote user's home directory. Source builds, raw commit-addressed artifacts, PyPI-only builds, and other binaries without an npm bootstrap stamp fail with an instruction to preinstall the same binary; they never substitute an older npm release. An unrecognized legacy probe fails closed; use explicit `--upgrade` from an npm-backed binary to force the pinned install and replace an SSH-managed sidecar. Each npm platform package ships `bin/cmux-tui-ssh/manifest.json`, which pins the SHA-256 of every published remote binary for its exact build. The bootstrap downloads the remote platform's package with `npm pack --ignore-scripts` into a private staging directory, runs none of its code, and compares the extracted binary's SHA-256 (`sha256sum`, `shasum` or `openssl`) with the pinned digest; a mismatch removes the download and fails. Only then does a probe of the staged binary check the distribution, npm bootstrap, and protocol versions before it replaces `--remote-binary`. Release CI verifies that the packaged binary reports the same distribution and npm bootstrap version and that every pinned digest matches its packaged binary. A build stamped with an npm version but shipped without that manifest (for example, a PyPI wheel built in the same release run) falls back to `npx --yes --ignore-scripts cmux@VERSION install-self`, which trusts the npm registry and the post-install probe. Automatic install requires Node.js 18 or newer, `npm`, registry access, and write access to `--remote-binary`; the published Linux packages use static musl binaries and do not require a glibc runtime. Use `--no-install` to require a preinstalled binary. A running sidecar is never silently replaced or restarted. `--upgrade` explicitly installs the pinned distribution, stops an SSH-managed sidecar through its owner-only admin socket, waits for complete cleanup, and reconnects through the new binary. Terminal panes survive. Remote clients and forwards disconnect, sidecar-owned RPC processes terminate, and RPC workspace handles and route IDs reset. The durable mux owner continues running its previous binary until the user explicitly restarts the full session. Manually started embedded `cmux-tui server start` processes are refused because stopping one would terminate its mux workspaces. Use `--remote-state-dir` on every SSH command when the remote daemon uses a non-default state directory. diff --git a/tests/test_tui_npm_package_artifact.py b/tests/test_tui_npm_package_artifact.py index 8249e3ec9474..0ae77013c7f5 100644 --- a/tests/test_tui_npm_package_artifact.py +++ b/tests/test_tui_npm_package_artifact.py @@ -1,5 +1,6 @@ from __future__ import annotations +import hashlib import shutil import stat import subprocess @@ -94,7 +95,11 @@ def make_package_fixture(packages: Path) -> None: "version": VERSION, "os": [os_name], "cpu": [cpu], - "files": ["bin/cmux-tui", "bin/cmux-tui-hook"], + "files": [ + "bin/cmux-tui", + "bin/cmux-tui-hook", + "bin/cmux-tui-ssh/manifest.json", + ], } ) + "\n" @@ -104,6 +109,25 @@ def make_package_fixture(packages: Path) -> None: executable.parent.mkdir(parents=True, exist_ok=True) executable.write_text("#!/bin/sh\nexit 0\n") executable.chmod(0o755) + # Every platform package pins each remote binary's SHA-256; the fixture + # binaries are identical, so one digest serves them all. + digest = hashlib.sha256(b"#!/bin/sh\nexit 0\n").hexdigest() + manifest = { + "commit": "0123456789abcdef0123456789abcdef01234567", + "binaries": { + f"cmux-tui-{target}": digest + for target in ( + "aarch64-unknown-linux-musl", + "x86_64-unknown-linux-musl", + "aarch64-apple-darwin", + "x86_64-apple-darwin", + ) + }, + } + for name in TARGETS: + path = packages / name / "bin/cmux-tui-ssh/manifest.json" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(manifest) + "\n") launcher = packages / "cmux" launcher.mkdir(parents=True, exist_ok=True) diff --git a/tests/test_tui_package_contract.py b/tests/test_tui_package_contract.py index 314a65555645..ace4b0a4b6ef 100644 --- a/tests/test_tui_package_contract.py +++ b/tests/test_tui_package_contract.py @@ -1,5 +1,6 @@ from __future__ import annotations +import hashlib import json import os import platform @@ -31,6 +32,15 @@ } RELAY_LAUNCHER = ROOT / "cmux-tui/dist/npm/cmux-relay/bin/cmux-relay.js" +NPM_BUILDER = ROOT / "cmux-tui/dist/scripts/package_npm.py" +SSH_MANIFEST = "bin/cmux-tui-ssh/manifest.json" +BUILD_COMMIT = "0123456789abcdef0123456789abcdef01234567" +SSH_ARTIFACTS = { + "cmux-tui-aarch64-unknown-linux-musl": "cmux-tui-linux-arm64", + "cmux-tui-x86_64-unknown-linux-musl": "cmux-tui-linux-x64", + "cmux-tui-aarch64-apple-darwin": "cmux-tui-darwin-arm64", + "cmux-tui-x86_64-apple-darwin": "cmux-tui-darwin-x64", +} def host_npm_target() -> str: @@ -148,6 +158,22 @@ def write_relay_binary_fixture(path: Path) -> None: path.chmod(0o755) +def write_ssh_manifests(root: Path) -> None: + """Pin every platform's packaged cmux-tui in every platform package.""" + + manifest = { + "commit": BUILD_COMMIT, + "binaries": { + artifact: hashlib.sha256((root / package / "bin/cmux-tui").read_bytes()).hexdigest() + for artifact, package in SSH_ARTIFACTS.items() + }, + } + for name in NPM_TARGETS: + path = root / name / SSH_MANIFEST + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(manifest) + "\n") + + def make_npm_packages(root: Path) -> None: root.mkdir() for name, (os_name, cpu) in NPM_TARGETS.items(): @@ -160,13 +186,14 @@ def make_npm_packages(root: Path) -> None: "version": VERSION, "os": [os_name], "cpu": [cpu], - "files": ["bin/cmux-tui", "bin/cmux-tui-hook"], + "files": ["bin/cmux-tui", "bin/cmux-tui-hook", SSH_MANIFEST], } ) + "\n" ) - write_executable(package / "bin/cmux-tui") + write_executable(package / "bin/cmux-tui", f"cmux-tui 1.2.3 {name}") write_executable(package / "bin/cmux-tui-hook", "cmux-tui-hook 1.2.3") + write_ssh_manifests(root) for name, (os_name, cpu) in RELAY_TARGETS.items(): package = root / name @@ -347,6 +374,70 @@ def test_npm_contract_rejects_extra_file(tmp_path: Path) -> None: assert "mismatch" in result.stderr +def test_npm_contract_rejects_ssh_digest_that_differs_from_the_packaged_binary( + tmp_path: Path, +) -> None: + packages = tmp_path / "npm-packages" + make_npm_packages(packages) + write_executable(packages / "cmux-tui-linux-arm64/bin/cmux-tui", "rebuilt after pinning") + + result = run_validator("--npm-packages", str(packages), "--version", VERSION) + + assert result.returncode != 0 + assert "cmux-tui-aarch64-unknown-linux-musl" in result.stderr, result.stderr + + +def test_npm_builder_pins_every_remote_binary_for_ssh_bootstrap(tmp_path: Path) -> None: + binaries = tmp_path / "binaries" + for target in ( + "aarch64-apple-darwin", + "x86_64-apple-darwin", + "x86_64-unknown-linux-musl", + "aarch64-unknown-linux-musl", + ): + write_executable(binaries / f"cmux-tui-{target}", f"cmux-tui {target}") + write_executable(binaries / f"cmux-tui-hook-{target}", "hook") + write_relay_binary_fixture(binaries / f"chatmux-relay-{target}") + packages = tmp_path / "npm-packages" + built = subprocess.run( + [ + sys.executable, + str(NPM_BUILDER), + "--binaries-dir", + str(binaries), + "--version", + VERSION, + "--build-commit", + BUILD_COMMIT, + "--out", + str(packages), + ], + cwd=ROOT, + check=False, + capture_output=True, + text=True, + ) + assert built.returncode == 0, built.stderr + + manifest = json.loads((packages / "cmux-tui-darwin-arm64" / SSH_MANIFEST).read_text()) + assert manifest == { + "commit": BUILD_COMMIT, + "binaries": { + f"cmux-tui-{target}": hashlib.sha256( + (binaries / f"cmux-tui-{target}").read_bytes() + ).hexdigest() + for target in ( + "aarch64-unknown-linux-musl", + "x86_64-unknown-linux-musl", + "aarch64-apple-darwin", + "x86_64-apple-darwin", + ) + }, + } + result = run_validator("--npm-packages", str(packages), "--version", VERSION) + assert result.returncode == 0, result.stderr + + def test_relay_launcher_preserves_native_signal_exit_status(tmp_path: Path) -> None: """The npm shim must die by the same signal as the native relay.""" @@ -450,6 +541,8 @@ def main() -> None: lambda _directory: test_host_npm_target_rejects_unknown_architecture(), test_npm_contract_rejects_missing_hook, test_npm_contract_rejects_extra_file, + test_npm_contract_rejects_ssh_digest_that_differs_from_the_packaged_binary, + test_npm_builder_pins_every_remote_binary_for_ssh_bootstrap, test_relay_launcher_preserves_native_signal_exit_status, test_pypi_contract_requires_all_six_wheels_and_metadata, test_pypi_contract_rejects_non_executable_hook, From a399afc6afe71407e614e05c1e54499b055bf1cf Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 18:07:05 -0700 Subject: [PATCH 26/48] cmux ssh: test that squatted /tmp socket directories do not block startup The remote runtime puts long-path sockets in the fixed /tmp/cmux-r- (client) and /tmp/cmux-rd- (daemon). Another local user who creates either first is correctly rejected by the ownership and symlink checks, but nothing falls back, so the client or daemon cannot start. These tests squat both directories with a symlink and expect each side to start in a fresh private 0700 directory that a second resolution for the same session finds again. The shared /tmp root becomes a parameter so the tests never touch the real /tmp paths; behavior is unchanged. Co-Authored-By: Claude Opus 5.5 --- .../crates/cmux-tui/src/remote_runtime.rs | 133 +++++++++++++++--- 1 file changed, 114 insertions(+), 19 deletions(-) diff --git a/cmux-tui/crates/cmux-tui/src/remote_runtime.rs b/cmux-tui/crates/cmux-tui/src/remote_runtime.rs index 6882c9b99343..1478e780a6d7 100644 --- a/cmux-tui/crates/cmux-tui/src/remote_runtime.rs +++ b/cmux-tui/crates/cmux-tui/src/remote_runtime.rs @@ -787,10 +787,10 @@ async fn run_client( let _ = connection.close().await; return Err(anyhow!("remote client startup was cancelled")); } - let local_socket = options - .local_socket - .clone() - .unwrap_or_else(|| default_client_socket(&options.state_dir, options.session)); + let local_socket = match options.local_socket.clone() { + Some(path) => path, + None => default_client_socket(&options.state_dir, options.session)?, + }; let socket_preparation = prepare_client_socket_with_shutdown(&local_socket, Some(shutdown.clone())).await?; if *shutdown.borrow() { @@ -1672,23 +1672,32 @@ fn unix_socket_path_fits(path: &Path) -> bool { path.as_os_str().as_bytes().len() < capacity } -fn default_client_socket(state_dir: &Path, session: SessionId) -> PathBuf { +fn default_client_socket(state_dir: &Path, session: SessionId) -> anyhow::Result { + let runtime = std::env::var_os("XDG_RUNTIME_DIR").map(PathBuf::from); + client_socket_path_in(state_dir, session, runtime.as_deref(), Path::new("/tmp")) +} + +/// Resolves the client socket for `session`. `runtime` is `XDG_RUNTIME_DIR` +/// and `shared_tmp` is the world-writable directory used when the state path +/// is too long for a Unix socket. +fn client_socket_path_in( + state_dir: &Path, + session: SessionId, + runtime: Option<&Path>, + shared_tmp: &Path, +) -> anyhow::Result { let candidate = state_dir.join("connections").join(format!("{session:?}")).join("mux.sock"); - #[cfg(unix)] if !unix_socket_path_fits(&candidate) { let uid = unsafe { libc::geteuid() }; let name = format!("{}.sock", base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(session.0)); - let runtime = std::env::var_os("XDG_RUNTIME_DIR") - .map(PathBuf::from) - .unwrap_or_else(|| PathBuf::from("/tmp")); - let fallback = runtime.join(format!("cmux-r-{uid}")).join(&name); + let fallback = runtime.unwrap_or(shared_tmp).join(format!("cmux-r-{uid}")).join(&name); if unix_socket_path_fits(&fallback) { - return fallback; + return Ok(fallback); } - return PathBuf::from(format!("/tmp/cmux-r-{uid}/{name}")); + return Ok(shared_tmp.join(format!("cmux-r-{uid}")).join(name)); } - candidate + Ok(candidate) } pub fn daemon_paths( @@ -1720,16 +1729,27 @@ pub fn daemon_paths( #[cfg(unix)] fn daemon_runtime_socket_paths(state: &Path) -> anyhow::Result<(PathBuf, PathBuf)> { + let runtime = std::env::var_os("XDG_RUNTIME_DIR").map(PathBuf::from); + daemon_runtime_socket_paths_in(state, runtime.as_deref(), Path::new("/tmp")) +} + +/// Resolves the daemon's link and admin sockets for a state path that is too +/// long for a Unix socket. `runtime` is `XDG_RUNTIME_DIR` and `shared_tmp` is +/// the world-writable directory used when no runtime directory is usable. +#[cfg(unix)] +fn daemon_runtime_socket_paths_in( + state: &Path, + runtime: Option<&Path>, + shared_tmp: &Path, +) -> anyhow::Result<(PathBuf, PathBuf)> { use std::os::unix::ffi::OsStrExt; let digest = format!("{:x}", Sha256::digest(state.as_os_str().as_bytes())); let socket_names = |runtime: &Path| { (runtime.join(format!("{digest}-l.sock")), runtime.join(format!("{digest}-a.sock"))) }; - if let Some(runtime) = std::env::var_os("XDG_RUNTIME_DIR") - .map(PathBuf::from) - .filter(|path| path.is_absolute()) - .map(|path| path.join("cmux-rd")) + if let Some(runtime) = + runtime.filter(|path| path.is_absolute()).map(|path| path.join("cmux-rd")) { let (link, admin) = socket_names(&runtime); if unix_socket_path_fits(&link) @@ -1740,7 +1760,7 @@ fn daemon_runtime_socket_paths(state: &Path) -> anyhow::Result<(PathBuf, PathBuf } } - let runtime = PathBuf::from(format!("/tmp/cmux-rd-{}", unsafe { libc::geteuid() })); + let runtime = shared_tmp.join(format!("cmux-rd-{}", unsafe { libc::geteuid() })); ensure_secure_directory(&runtime, DirectoryAccess::ManagedOwnerOnly).with_context(|| { format!("could not create private remote daemon runtime directory {}", runtime.display()) })?; @@ -6261,8 +6281,83 @@ mod tests { #[test] fn long_state_path_uses_a_short_runtime_socket() { let state = PathBuf::from("/tmp").join("x".repeat(256)); - let socket = default_client_socket(&state, SessionId([4; 16])); + let socket = default_client_socket(&state, SessionId([4; 16])).unwrap(); assert!(unix_socket_path_fits(&socket)); assert!(!socket.starts_with(state)); } + + /// Another local user can create `/tmp/cmux-r-` first. The ownership + /// checks must still reject it, and the client must still start in a + /// fresh private directory that a second resolution finds again. + #[cfg(unix)] + #[tokio::test] + async fn squatted_shared_client_socket_directory_falls_back_to_a_private_directory() { + use std::os::unix::fs::{MetadataExt, PermissionsExt, symlink}; + + // A short root keeps the fallback socket paths inside sun_path, as + // they are under the real /tmp. + let root = tempfile::Builder::new().prefix("r").rand_bytes(2).tempdir_in("/tmp").unwrap(); + let shared_tmp = root.path().to_path_buf(); + let decoy = root.path().join("decoy"); + fs::create_dir(&decoy).unwrap(); + let squatted = shared_tmp.join(format!("cmux-r-{}", unsafe { libc::geteuid() })); + symlink(&decoy, &squatted).unwrap(); + let state = root.path().join("x".repeat(160)); + let session = SessionId([7; 16]); + + let socket = client_socket_path_in(&state, session, None, &shared_tmp).unwrap(); + let prepared = prepare_client_socket(&socket).await; + assert!( + prepared.is_ok(), + "a squatted shared directory blocked the client socket {}: {:?}", + socket.display(), + prepared.err() + ); + let directory = socket.parent().unwrap(); + assert_ne!(directory, squatted); + assert!(directory.starts_with(&shared_tmp)); + let metadata = fs::symlink_metadata(directory).unwrap(); + assert!(metadata.is_dir()); + assert_eq!(metadata.uid(), unsafe { libc::geteuid() }); + assert_eq!(metadata.permissions().mode() & 0o777, 0o700); + assert_eq!(client_socket_path_in(&state, session, None, &shared_tmp).unwrap(), socket); + assert!(fs::read_dir(&decoy).unwrap().next().is_none()); + } + + /// The daemon's link and admin sockets fall back the same way, and every + /// later `remote-link`, `remote-stop` or status call resolves the same + /// directory from the session state. + #[cfg(unix)] + #[test] + fn squatted_shared_daemon_runtime_directory_falls_back_to_a_private_directory() { + use std::os::unix::fs::{MetadataExt, PermissionsExt, symlink}; + + // A short root keeps the fallback socket paths inside sun_path, as + // they are under the real /tmp. + let root = tempfile::Builder::new().prefix("r").rand_bytes(2).tempdir_in("/tmp").unwrap(); + let shared_tmp = root.path().to_path_buf(); + let decoy = root.path().join("decoy"); + fs::create_dir(&decoy).unwrap(); + let squatted = shared_tmp.join(format!("cmux-rd-{}", unsafe { libc::geteuid() })); + symlink(&decoy, &squatted).unwrap(); + let state = root.path().join("state").join("sessions").join("session"); + + let resolved = daemon_runtime_socket_paths_in(&state, None, &shared_tmp); + assert!(resolved.is_ok(), "a squatted shared directory blocked the daemon: {resolved:?}"); + let (link, admin) = resolved.unwrap(); + let directory = link.parent().unwrap(); + assert_eq!(admin.parent().unwrap(), directory); + assert_ne!(directory, squatted); + assert!(directory.starts_with(&shared_tmp)); + assert!(unix_socket_path_fits(&link) && unix_socket_path_fits(&admin)); + let metadata = fs::symlink_metadata(directory).unwrap(); + assert!(metadata.is_dir()); + assert_eq!(metadata.uid(), unsafe { libc::geteuid() }); + assert_eq!(metadata.permissions().mode() & 0o777, 0o700); + assert_eq!( + daemon_runtime_socket_paths_in(&state, None, &shared_tmp).unwrap(), + (link, admin) + ); + assert!(fs::read_dir(&decoy).unwrap().next().is_none()); + } } From 8f5799405ba28a4951cff772b749503053dade2f Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 18:14:48 -0700 Subject: [PATCH 27/48] cmux ssh: fall back to a private socket directory when /tmp is squatted When a state path is too long for a Unix socket, the client used /tmp/cmux-r- and the daemon /tmp/cmux-rd-. Another local user could create either name first; the ownership and symlink checks rejected it, and startup failed with no alternative. Both now keep preferring the fixed directory but fall back, as the Go daemon does, to a fresh random 0700 sibling created exclusively and validated by the same checks. The fallback path is published with a no-replace hard link into a record file in the session's private state (`socket-dir` in the daemon session state, and next to the long client socket path), so remote-link, remote-stop and every other process that derives the sockets from daemon_paths finds the same directory. A record is honored only for a `-*` child of the same base that still passes validation; a stale one is replaced. Co-Authored-By: Claude Opus 5.5 --- .../crates/cmux-tui/src/remote_runtime.rs | 162 ++++++++++++++++-- 1 file changed, 149 insertions(+), 13 deletions(-) diff --git a/cmux-tui/crates/cmux-tui/src/remote_runtime.rs b/cmux-tui/crates/cmux-tui/src/remote_runtime.rs index 1478e780a6d7..048f418bb08e 100644 --- a/cmux-tui/crates/cmux-tui/src/remote_runtime.rs +++ b/cmux-tui/crates/cmux-tui/src/remote_runtime.rs @@ -1687,17 +1687,147 @@ fn client_socket_path_in( shared_tmp: &Path, ) -> anyhow::Result { let candidate = state_dir.join("connections").join(format!("{session:?}")).join("mux.sock"); - if !unix_socket_path_fits(&candidate) { - let uid = unsafe { libc::geteuid() }; - let name = - format!("{}.sock", base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(session.0)); - let fallback = runtime.unwrap_or(shared_tmp).join(format!("cmux-r-{uid}")).join(&name); - if unix_socket_path_fits(&fallback) { - return Ok(fallback); + if unix_socket_path_fits(&candidate) { + return Ok(candidate); + } + let prefix = format!("cmux-r-{}", unsafe { libc::geteuid() }); + let name = + format!("{}.sock", base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(session.0)); + let preferred_base = runtime.unwrap_or(shared_tmp); + let base = if unix_socket_path_fits(&preferred_base.join(&prefix).join(&name)) { + preferred_base + } else { + shared_tmp + }; + let record = candidate.with_file_name(SOCKET_DIRECTORY_RECORD); + let directory = + private_socket_directory(base, &prefix, &record, prepare_client_socket_directory)?; + let socket = directory.join(name); + if !unix_socket_path_fits(&socket) { + return Err(anyhow!( + "client socket path is too long for this platform: {}", + socket.display() + )); + } + Ok(socket) +} + +/// File in a private state directory that records the socket directory chosen +/// after the fixed shared one was unusable. +#[cfg(unix)] +const SOCKET_DIRECTORY_RECORD: &str = "socket-dir"; + +/// Chooses a private directory for Unix sockets under the world-writable +/// `base`. `/` is preferred, but any local user can create that +/// name first; `validate` rejects such a directory and a fresh random `0700` +/// sibling is used instead, like the Go daemon's fallback. The fallback is +/// recorded in `record`, inside the caller's private state, so every later +/// process for the same session resolves the same sockets. +#[cfg(unix)] +fn private_socket_directory( + base: &Path, + prefix: &str, + record: &Path, + validate: impl Fn(&Path) -> anyhow::Result<()>, +) -> anyhow::Result { + use std::os::unix::fs::DirBuilderExt; + + let preferred = base.join(prefix); + let mut preferred_error = None; + for _ in 0..8 { + if let Some(recorded) = recorded_socket_directory(record, base, prefix) + && validate(&recorded).is_ok() + { + return Ok(recorded); + } + match validate(&preferred) { + Ok(()) => return Ok(preferred), + Err(error) => preferred_error = Some(error), + } + let parent = record.parent().ok_or_else(|| anyhow!("socket record has no parent"))?; + ensure_secure_directory(parent, DirectoryAccess::OwnerControlled) + .with_context(|| format!("could not prepare {}", parent.display()))?; + let suffix = uuid::Uuid::new_v4().simple().to_string(); + // Keep the name short: socket paths must fit in sun_path. + let candidate = base.join(format!("{prefix}-{}", &suffix[..8])); + match fs::DirBuilder::new().mode(0o700).create(&candidate) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(error) => { + return Err(error) + .with_context(|| format!("could not create {}", candidate.display())); + } + } + if let Err(error) = validate(&candidate) { + let _ = fs::remove_dir(&candidate); + return Err(error); + } + match publish_socket_directory_record(record, &candidate) { + Ok(()) => return Ok(candidate), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { + // Another process for this session recorded its directory + // first. Use it if it is still private, otherwise replace + // the stale record on the next attempt. + let _ = fs::remove_dir(&candidate); + if let Some(recorded) = recorded_socket_directory(record, base, prefix) + && validate(&recorded).is_ok() + { + return Ok(recorded); + } + let _ = fs::remove_file(record); + } + Err(error) => { + let _ = fs::remove_dir(&candidate); + return Err(error) + .with_context(|| format!("could not record {}", candidate.display())); + } } - return Ok(shared_tmp.join(format!("cmux-r-{uid}")).join(name)); } - Ok(candidate) + Err(preferred_error.unwrap_or_else(|| anyhow!("no private socket directory was available"))) + .with_context(|| { + format!("could not create a private socket directory under {}", base.display()) + }) +} + +/// Reads a recorded fallback directory. Only a `-*` child of `base` +/// is accepted, so a damaged record cannot point sockets anywhere else. +#[cfg(unix)] +fn recorded_socket_directory(record: &Path, base: &Path, prefix: &str) -> Option { + use std::os::unix::ffi::{OsStrExt, OsStringExt}; + + let metadata = fs::symlink_metadata(record).ok()?; + if !metadata.is_file() || metadata.len() > 4096 { + return None; + } + let path = PathBuf::from(std::ffi::OsString::from_vec(fs::read(record).ok()?)); + let name = path.file_name()?.as_bytes(); + (path.parent() == Some(base) && name.starts_with(format!("{prefix}-").as_bytes())) + .then_some(path) +} + +/// Publishes `directory` as the session's socket directory without replacing +/// a record another process already published. +#[cfg(unix)] +fn publish_socket_directory_record(record: &Path, directory: &Path) -> std::io::Result<()> { + use std::io::Write as _; + use std::os::unix::ffi::OsStrExt; + use std::os::unix::fs::OpenOptionsExt; + + let staged = record + .with_file_name(format!(".{SOCKET_DIRECTORY_RECORD}.{}", uuid::Uuid::new_v4().simple())); + let result = (|| { + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .custom_flags(libc::O_NOFOLLOW | libc::O_CLOEXEC) + .open(&staged)?; + file.write_all(directory.as_os_str().as_bytes())?; + file.sync_all()?; + fs::hard_link(&staged, record) + })(); + let _ = fs::remove_file(&staged); + result } pub fn daemon_paths( @@ -1760,10 +1890,16 @@ fn daemon_runtime_socket_paths_in( } } - let runtime = shared_tmp.join(format!("cmux-rd-{}", unsafe { libc::geteuid() })); - ensure_secure_directory(&runtime, DirectoryAccess::ManagedOwnerOnly).with_context(|| { - format!("could not create private remote daemon runtime directory {}", runtime.display()) - })?; + // Every client of this session computes the same paths from `state`, so + // a fallback directory is recorded there for them to find. + let prefix = format!("cmux-rd-{}", unsafe { libc::geteuid() }); + let runtime = private_socket_directory( + shared_tmp, + &prefix, + &state.join(SOCKET_DIRECTORY_RECORD), + |path| Ok(ensure_secure_directory(path, DirectoryAccess::ManagedOwnerOnly)?), + ) + .context("could not create private remote daemon runtime directory")?; let (link, admin) = socket_names(&runtime); if !unix_socket_path_fits(&link) || !unix_socket_path_fits(&admin) { return Err(anyhow!("remote daemon runtime socket path is too long for this platform")); From ecac15199a5f604a391bf7e1ff965d08c5e1e808 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 21:04:53 -0700 Subject: [PATCH 28/48] test(hermes): prime fixture executables before the timed wrapper launch macOS assesses a newly written executable on its first exec, one file at a time across the whole machine. Every run_wrapper call writes a fresh wrapper copy, fake cmux CLI and fake Hermes, so on a runner where the parallel CLI lanes keep writing fixtures, the queue alone could push the first `bare` launch past the 5 s hang guard ("wrapper execution deadline exceeded"). Run each fresh fixture once, outside the deadline, before the timed launch. Installed Hermes and the bundled wrapper and CLI were executed before in real use, so the timed launch now measures only the wrapper's own work. The hang guard and installer deadlines are unchanged. Co-Authored-By: Claude Opus 5.5 --- tests/test_hermes_wrapper_hooks.py | 45 +++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/test_hermes_wrapper_hooks.py b/tests/test_hermes_wrapper_hooks.py index b42b4e54bc10..c562b83f8417 100644 --- a/tests/test_hermes_wrapper_hooks.py +++ b/tests/test_hermes_wrapper_hooks.py @@ -48,11 +48,42 @@ class WrapperResult: profile_homes: dict[str, str] +# Set only while priming a fixture's first exec; every fixture exits at once. +PRIME_ENVIRONMENT_KEY = "CMUX_HERMES_TEST_PRIME_EXEC" + + def make_executable(path: Path, content: str) -> None: - path.write_text(content, encoding="utf-8") + shebang, body = content.split("\n", 1) + path.write_text( + f'{shebang}\nif [ -n "${{{PRIME_ENVIRONMENT_KEY}:-}}" ]; then exit 0; fi\n{body}', + encoding="utf-8", + ) path.chmod(0o755) +def prime_first_exec(paths: list[Path]) -> None: + """Run each fresh executable once before a timed launch. + + macOS assesses a newly written executable on its first exec, one file at a + time across the whole machine. On a runner where parallel tests keep + writing fixtures, the queue alone can outlast the hang guard. Installed + Hermes and the bundled wrapper and CLI were run before, so priming keeps + the timed launch to the wrapper's own work. + """ + # Without CMUX_SURFACE_ID or a Hermes on PATH, the wrapper exits 127 at once. + environment = {"PATH": "/usr/bin:/bin", PRIME_ENVIRONMENT_KEY: "1"} + for path in paths: + subprocess.run( + [str(path)], + env=environment, + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + timeout=120, + check=False, + ) + + def read_nul_values(path: Path) -> list[str]: if not path.exists(): return [] @@ -430,6 +461,18 @@ def run_wrapper( else: env.pop("CMUX_HERMES_AGENT_HOOKS_DISABLED", None) + prime_first_exec( + [ + wrapper, + *( + path + for path in sorted(tmp.rglob("*")) + if path.is_file() + and not path.is_symlink() + and PRIME_ENVIRONMENT_KEY in path.read_text(encoding="utf-8", errors="ignore") + ), + ] + ) proc = subprocess.Popen( [str(wrapper), *argv], cwd=tmp, From 191b525d850ed500a607297910f4f311c3483b3f Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 21:09:05 -0700 Subject: [PATCH 29/48] cmux ssh: count the auth cleanup fork budget by real uid The deadline test lowered RLIMIT_NPROC to the user's process count plus 16, but counted by effective uid. The kernel charges the limit to the real uid, and setuid-root processes such as the /usr/bin/login behind every terminal tab report uid 0 while still counting against the user. On a host with 136 such processes the ceiling landed below the live count, so the harness could not fork the helper at all and exited 128 with the tree untouched. Count by ruid, name the helper's three-slot peak and the slack for other activity, fail loudly when the count cannot be taken, and report a non-zero helper status instead of waiting on a root it never signalled. Co-Authored-By: Claude Opus 5.5 --- ...groundAuthenticationRetryPolicyTests.swift | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHForegroundAuthenticationRetryPolicyTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHForegroundAuthenticationRetryPolicyTests.swift index fe1bad398ee6..620464c9099e 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHForegroundAuthenticationRetryPolicyTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHForegroundAuthenticationRetryPolicyTests.swift @@ -467,28 +467,48 @@ struct SSHForegroundAuthenticationRetryPolicyTests { done test -f "$CMUX_TEST_READY_MARKER" || exit 98 # Lower the helper shell's process ceiling only after the full fixture - # exists. Keep the limit just above the live per-user count so the - # fixture remains runnable while the old recursive cleanup receives - # EAGAIN on its short-lived scans. + # exists, so the cleanup runs under process pressure. + # + # The kernel charges RLIMIT_NPROC to the real user ID. Count by + # `ruid`, not `uid`: setuid-root processes the user started, such as + # the /usr/bin/login that every Terminal or cmux tab runs, report + # uid 0 but still count against this user's ceiling. Counting by + # effective uid put the ceiling below the live count on hosts with + # open terminals, so the harness could not fork the helper at all. + # + # The budget is explicit. The helper forks one command at a time and + # uses no pipelines or background jobs, so it needs at most three + # process slots at once: its subshell, a command-substitution child + # and the command that child runs. The remaining slots absorb other + # processes this user starts while cleanup runs. The count also + # includes the command substitution, ps and awk that take it; they + # exit before cleanup starts. + cmux_test_helper_process_slots=3 + cmux_test_concurrent_activity_slots=13 cmux_test_user_id=$(/usr/bin/id -u 2>/dev/null || true) cmux_test_process_count=$( - /bin/ps -axo uid= 2>/dev/null | + /bin/ps -axo ruid= 2>/dev/null | /usr/bin/awk -v uid="$cmux_test_user_id" '$1 == uid { count += 1 } END { print count + 0 }' ) || cmux_test_process_count= case "$cmux_test_process_count" in - ''|*[!0-9]*) cmux_test_process_count= ;; + ''|0|*[!0-9]*) + printf '%s\n' "could not count processes for uid $cmux_test_user_id" >&2 + exit 97 + ;; esac - if [ -n "$cmux_test_process_count" ]; then - ulimit -u "$((cmux_test_process_count + 16))" 2>/dev/null || \ - ulimit -u 100 2>/dev/null || true - else - ulimit -u 100 2>/dev/null || true - fi + ulimit -u "$((cmux_test_process_count + cmux_test_helper_process_slots + cmux_test_concurrent_activity_slots))" || exit 96 : > "$CMUX_TEST_CLEANUP_STARTED_MARKER" # Exercise the event-enabled path without publishing an event. The # helper must reserve a force pass instead of rolling back after the # bounded FIFO wait. cmux_ssh_terminate_auth_process_tree "$cmux_test_auth_root" "$$" 1 + cmux_test_cleanup_status=$? + if [ "$cmux_test_cleanup_status" -ne 0 ]; then + # Report the helper failure instead of waiting on a root it never + # signalled. + printf '%s\n' "cleanup helper exited with status $cmux_test_cleanup_status" >&2 + exit "$cmux_test_cleanup_status" + fi wait "$cmux_test_auth_root" 2>/dev/null || true """ From b88013b82cc7a1c31f0643860f3e2dd2e178175b Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 19:13:59 -0700 Subject: [PATCH 30/48] cmux ssh: test that a stalled-replay deadline keeps the replay query filter Move the attach loop's replay accounting and replay query filter into SSHPTYAttachReplayOutputStream, unchanged, so the deadline decision is testable. The new tests fail: ending a stalled replay also ends query filtering, so later replay bytes deliver OSC 52 reads and DA queries to the local terminal, and a huge declared replay strips live queries. Co-Authored-By: Claude Opus 5.5 --- CLI/cmux.swift | 61 ++++++++--------- .../SSHPTYAttachReplayOutputStream.swift | 54 +++++++++++++++ .../SSHPTYAttachReplayOutputStreamTests.swift | 68 +++++++++++++++++++ 3 files changed, 150 insertions(+), 33 deletions(-) create mode 100644 Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayOutputStream.swift create mode 100644 Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayOutputStreamTests.swift diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 7584ef2efef2..fe0f691cce97 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -15834,32 +15834,30 @@ struct CMUXCLI { suppressReplayBytes = 0 expectedReplayFingerprint = nil } - var outputProgress = SSHPTYAttachOutputProgress( - replayBytes: bridgeReplayBytes, - suppressReplayBytes: suppressReplayBytes, - expectedReplayFingerprint: expectedReplayFingerprint + // A persistent reattach's initial bytes are historical remote PTY + // output. Strip terminal queries before Ghostty parses them; otherwise + // its replies can arrive after the reconnect stdin filter hands off to + // live input and land in the remote shell. + let filtersReplayOutput = requireExisting && command == nil + var replayOutput = SSHPTYAttachReplayOutputStream( + progress: SSHPTYAttachOutputProgress( + replayBytes: bridgeReplayBytes, + suppressReplayBytes: suppressReplayBytes, + expectedReplayFingerprint: expectedReplayFingerprint + ), + queryFilterReplayBytes: filtersReplayOutput ? bridgeReplayBytes : 0 ) var replayStateStored = bridgeReplayBytes == 0 if replayStateStored { replayState.storeSnapshot( replayBytes: bridgeReplayBytes, - fingerprint: outputProgress.completedReplayFingerprint ?? + fingerprint: replayOutput.progress.completedReplayFingerprint ?? SSHPTYAttachOutputProgress.fingerprint(of: Data()) ) } - // A persistent reattach's initial bytes are historical remote PTY - // output. Strip terminal queries before Ghostty parses them; otherwise - // its replies can arrive after the reconnect stdin filter hands off to - // live input and land in the remote shell. - let filtersReplayOutput = requireExisting && command == nil - var replayOutputFilter = SSHPTYReplayOutputFilter( - replayBytes: filtersReplayOutput ? bridgeReplayBytes : 0 - ) - func writeReplayFilteredOutput(_ data: Data) throws { - guard !data.isEmpty else { return } - let filtered = replayOutputFilter.filter(data) - if !filtered.isEmpty, - !outputWriter.write(filtered, cancellation: signalMonitor!) { + func writeTerminalOutput(_ output: Data) throws { + if !output.isEmpty, + !outputWriter.write(output, cancellation: signalMonitor!) { // Local output failure unwinds termios while preserving the remote PTY. preserveLifecycleForRecovery = true try checkSSHPTYCancellation(signalMonitor) @@ -15867,14 +15865,12 @@ struct CMUXCLI { } } defer { - let pendingReplay = outputProgress.finishPendingReplay( - discarding: sshPTYAttachWrapperRetryPending() - ) - try? writeReplayFilteredOutput(pendingReplay) - _ = outputWriter.write(replayOutputFilter.finish(), cancellation: signalMonitor!) + try? writeTerminalOutput(replayOutput.finish( + discardingPendingReplay: sshPTYAttachWrapperRetryPending() + )) } func startInputForwardingAfterReplay() throws { - guard !inputPumpStarted, outputProgress.replayBytesRemaining == 0 else { return } + guard !inputPumpStarted, replayOutput.progress.replayBytesRemaining == 0 else { return } if filtersReconnectInput, terminalInputMode?.beginForwarding() != true { throw sshPTYTerminalModeError() } @@ -15892,10 +15888,10 @@ struct CMUXCLI { } } func storeReplayStateIfComplete() { - guard !replayStateStored, outputProgress.replayBytesRemaining == 0 else { return } + guard !replayStateStored, replayOutput.progress.replayBytesRemaining == 0 else { return } replayState.storeSnapshot( - replayBytes: outputProgress.deliveredReplayBytes, - fingerprint: outputProgress.completedReplayFingerprint ?? + replayBytes: replayOutput.progress.deliveredReplayBytes, + fingerprint: replayOutput.progress.completedReplayFingerprint ?? SSHPTYAttachOutputProgress.fingerprint(of: Data()) ) replayStateStored = true @@ -15905,8 +15901,7 @@ struct CMUXCLI { // forwarding off indefinitely. var replayDeadline = SSHPTYAttachReplayDeadline(startedAt: bridgeReadyUptime) func endStalledReplay() throws { - try writeReplayFilteredOutput(outputProgress.endReplay()) - replayOutputFilter.endReplay() + try writeTerminalOutput(replayOutput.endStalledReplay()) storeReplayStateIfComplete() try startInputForwardingAfterReplay() } @@ -15917,7 +15912,7 @@ struct CMUXCLI { _ = try reconcileBridgeEnd( intentionalOnly: false, sessionRunningExitCode: sshPTYAttachBridgeClosedExitCode( - receivedLiveOutput: outputProgress.receivedLiveOutput, + receivedLiveOutput: replayOutput.progress.receivedLiveOutput, readyUptime: bridgeReadyUptime ), reconciliationUnavailableExitCode: .bridgeClosedSessionRunning @@ -15927,7 +15922,7 @@ struct CMUXCLI { var outputBuffer = [UInt8](repeating: 0, count: 32768) while true { - if !inputPumpStarted, outputProgress.replayBytesRemaining > 0 { + if !inputPumpStarted, replayOutput.progress.replayBytesRemaining > 0 { let wait = replayDeadline.remainingWait(at: ProcessInfo.processInfo.systemUptime) var pollFD = pollfd(fd: fd, events: Int16(POLLIN), revents: 0) let ready = wait > 0 ? poll(&pollFD, 1, Int32(min(wait * 1000, 60_000).rounded(.up))) : 0 @@ -15945,7 +15940,7 @@ struct CMUXCLI { try checkSSHPTYCancellation(signalMonitor) if count > 0 { replayDeadline.recordOutput(at: ProcessInfo.processInfo.systemUptime) - let output = outputProgress.terminalOutput( + let output = replayOutput.terminalOutput( from: Data(outputBuffer.prefix(count)), suppressingReplay: suppressReplay ) @@ -15953,7 +15948,7 @@ struct CMUXCLI { if inputPumpStarted { reconnectInputFilterControl?.stopFilteringBeforeFirstOutput(unlessAlreadyRequested: &reconnectInputFilterStopRequested) } - try writeReplayFilteredOutput(output) + try writeTerminalOutput(output) } storeReplayStateIfComplete() try startInputForwardingAfterReplay() diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayOutputStream.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayOutputStream.swift new file mode 100644 index 000000000000..0b257d874981 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayOutputStream.swift @@ -0,0 +1,54 @@ +public import Foundation + +/// Turns an SSH PTY attachment's bridge output into terminal output. +/// +/// Pairs the replay progress accounting, which decides when input forwarding +/// may start, with the replay query filter, which keeps historical terminal +/// queries away from the local emulator. Both see the same ordered stream. +public struct SSHPTYAttachReplayOutputStream: Sendable { + /// Replay accounting for the attachment. + public private(set) var progress: SSHPTYAttachOutputProgress + private var queryFilter: SSHPTYReplayOutputFilter + + /// Creates the output stream for one attachment. + /// + /// - Parameters: + /// - progress: Replay accounting for the attachment's declared replay. + /// - queryFilterReplayBytes: Leading bytes whose terminal queries are + /// removed; zero forwards every query. + public init(progress: SSHPTYAttachOutputProgress, queryFilterReplayBytes: Int) { + self.progress = progress + queryFilter = SSHPTYReplayOutputFilter(replayBytes: queryFilterReplayBytes) + } + + /// Returns the bytes of one bridge chunk that belong in the terminal. + /// + /// - Parameters: + /// - data: Ordered bytes read from the bridge. + /// - suppressingReplay: Whether this managed attempt hides the replay + /// prefix an earlier attempt already rendered. + public mutating func terminalOutput(from data: Data, suppressingReplay: Bool) -> Data { + queryFilter.filter(progress.terminalOutput(from: data, suppressingReplay: suppressingReplay)) + } + + /// Ends the replay phase after the caller's replay deadline expired. + /// + /// - Returns: Buffered replay output that must still reach the terminal. + public mutating func endStalledReplay() -> Data { + let output = queryFilter.filter(progress.endReplay()) + queryFilter.endReplay() + return output + } + + /// Flushes everything still held when the bridge closes. + /// + /// - Parameter discardingPendingReplay: Drop an unvalidated replay + /// candidate because another managed attempt will render it. + public mutating func finish(discardingPendingReplay: Bool) -> Data { + var output = queryFilter.filter( + progress.finishPendingReplay(discarding: discardingPendingReplay) + ) + output.append(queryFilter.finish()) + return output + } +} diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayOutputStreamTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayOutputStreamTests.swift new file mode 100644 index 000000000000..702d8ee4b057 --- /dev/null +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayOutputStreamTests.swift @@ -0,0 +1,68 @@ +import Foundation +import Testing +@testable import CmuxFoundation + +/// The replay deadline only stops holding keystrokes. Queries in replay bytes +/// that arrive after it fires are still historical and must not reach the +/// local terminal, or its replies are typed into the now-live remote shell. +@Suite("SSH PTY attach replay output stream") +struct SSHPTYAttachReplayOutputStreamTests { + private static let osc52Read = "\u{1B}]52;c;?\u{07}" + private static let primaryDA = "\u{1B}[c" + + private func text(_ data: Data) -> String { + String(decoding: data, as: UTF8.self) + } + + private func stream(replayBytes: Int) -> SSHPTYAttachReplayOutputStream { + SSHPTYAttachReplayOutputStream( + progress: SSHPTYAttachOutputProgress(replayBytes: replayBytes), + queryFilterReplayBytes: replayBytes + ) + } + + @Test("replay bytes after a stalled-replay deadline still have their queries stripped") + func replayAfterDeadlineKeepsQueryFilter() { + let head = "old prompt$ " + let tail = "a\(Self.osc52Read)b\(Self.primaryDA)c" + var output = stream(replayBytes: head.utf8.count + tail.utf8.count) + + #expect(text(output.terminalOutput(from: Data(head.utf8), suppressingReplay: false)) == head) + #expect(output.progress.replayBytesRemaining > 0) + + // The link stalls past the deadline: input forwarding may start. + #expect(output.endStalledReplay().isEmpty) + #expect(output.progress.replayBytesRemaining == 0) + + // The rest of the declared replay then arrives. + let late = output.terminalOutput(from: Data(tail.utf8), suppressingReplay: false) + #expect(text(late) == "abc") + + // Output past the declared replay is live and keeps its queries. + let live = output.terminalOutput(from: Data(Self.primaryDA.utf8), suppressingReplay: false) + #expect(text(live) == Self.primaryDA) + } + + @Test("a query split across the deadline and the next replay chunk is stripped") + func splitQueryAcrossDeadlineIsStripped() { + var output = stream(replayBytes: 64) + #expect(text(output.terminalOutput(from: Data("x\u{1B}]52;c".utf8), suppressingReplay: false)) == "x") + + #expect(output.endStalledReplay().isEmpty) + + #expect(text(output.terminalOutput(from: Data(";?\u{07}y".utf8), suppressingReplay: false)) == "y") + } + + @Test("a declared replay beyond the daemon's scrollback cap stops filtering at the cap") + func oversizedDeclaredReplayIsCapped() { + let cap = 1 << 20 + var output = stream(replayBytes: Int.max) + let filler = Data(repeating: 0x61, count: cap) + let forwarded = output.terminalOutput(from: filler, suppressingReplay: false) + #expect(forwarded.count == cap) + + // A peer cannot make every later query look historical. + let live = output.terminalOutput(from: Data(Self.primaryDA.utf8), suppressingReplay: false) + #expect(text(live) == Self.primaryDA) + } +} From ce0874cce9bc5e7c49ad301a173441ee85feab01 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 19:25:16 -0700 Subject: [PATCH 31/48] cmux ssh: keep stripping replay queries after the replay deadline The replay deadline only stops holding keystrokes. Ending it also disabled the replay query filter, so on a slow link the rest of the declared replay reached Ghostty unfiltered: DA/DSR/XTVERSION replies were typed into the now-live shell and OSC 52 reads leaked the local clipboard. The deadline now ends only the output-progress hold; the filter keeps counting down the declared replay bytes. A peer that declares a huge replay could then strip queries from live output forever, so the filter caps its replay window at 1 MiB, the cmuxd-remote scrollback cap that bounds every replay snapshot. Co-Authored-By: Claude Opus 5.5 --- .../SSHPTYAttachReplayOutputStream.swift | 15 +++++++++---- .../SSHPTYReplayOutputFilter.swift | 21 ++++++++++--------- .../SSHPTYAttachReplayBoundTests.swift | 10 --------- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayOutputStream.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayOutputStream.swift index 0b257d874981..e6875dcac0bc 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayOutputStream.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYAttachReplayOutputStream.swift @@ -31,13 +31,20 @@ public struct SSHPTYAttachReplayOutputStream: Sendable { queryFilter.filter(progress.terminalOutput(from: data, suppressingReplay: suppressingReplay)) } - /// Ends the replay phase after the caller's replay deadline expired. + /// Ends the replay hold after the caller's replay deadline expired. + /// + /// The deadline exists so a slow or lying peer cannot hold keystrokes + /// forever; afterwards ``progress`` reports the replay as complete and + /// input forwarding may start. The declared replay is still historical + /// output, so the query filter keeps counting it down: a query in replay + /// bytes that arrive late is stripped rather than answered into the live + /// remote shell. ``SSHPTYReplayOutputFilter`` caps how many bytes it + /// treats as replay, so an inflated declaration cannot hide live queries + /// indefinitely. /// /// - Returns: Buffered replay output that must still reach the terminal. public mutating func endStalledReplay() -> Data { - let output = queryFilter.filter(progress.endReplay()) - queryFilter.endReplay() - return output + queryFilter.filter(progress.endReplay()) } /// Flushes everything still held when the bridge closes. diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift index 37ea2f6c30ad..a7fdc3a6b0ce 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReplayOutputFilter.swift @@ -18,6 +18,14 @@ public struct SSHPTYReplayOutputFilter: Sendable { private static let semicolon: UInt8 = 0x3B private static let questionMark: UInt8 = 0x3F private static let maxPendingBytes = 4 * 1024 + /// Most leading bytes treated as replay, whatever the peer declares. + /// + /// `cmuxd-remote` replays its session scrollback snapshot, which it bounds + /// to `defaultWebSocketScrollbackCap` (1 MiB); production never raises + /// that limit, and ``SSHPTYAttachOutputProgress`` validates reconnect + /// replays against the same bound. A larger declaration violates the + /// protocol, so it cannot keep query stripping on for live output. + public static let maximumReplayBytes = 1 << 20 private enum SequenceMatch { case strip(length: Int) @@ -31,9 +39,10 @@ public struct SSHPTYReplayOutputFilter: Sendable { /// Creates a filter for one ordered PTY attachment output stream. /// /// - Parameter replayBytes: Number of leading output bytes belonging to - /// the daemon's scrollback replay. Negative values are treated as zero. + /// the daemon's scrollback replay. Negative values are treated as zero + /// and larger values are capped at ``maximumReplayBytes``. public init(replayBytes: Int) { - replayBytesRemaining = max(0, replayBytes) + replayBytesRemaining = min(max(0, replayBytes), Self.maximumReplayBytes) } /// Filters one output chunk, stripping only query sequences that begin in replay. @@ -95,14 +104,6 @@ public struct SSHPTYReplayOutputFilter: Sendable { return output } - /// Treats every later byte as live output after the replay phase ended early. - /// - /// A held candidate that began in replay is emitted unchanged with the next - /// chunk, matching how an oversized candidate fails open. - public mutating func endReplay() { - replayBytesRemaining = 0 - } - /// Flushes an unterminated candidate when the bridge closes. /// /// Unterminated bytes cannot produce a terminal response, so they are diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayBoundTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayBoundTests.swift index 90799b09bce4..f276d7d8c578 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayBoundTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYAttachReplayBoundTests.swift @@ -109,14 +109,4 @@ struct SSHPTYAttachReplayBoundTests { #expect(forwardedEveryByteInOrder) #expect(progress.replayBytesRemaining == 0) } - - @Test("a replayed query filter passes live queries after the replay ends early") - func replayFilterStopsAfterEarlyReplayEnd() { - var filter = SSHPTYReplayOutputFilter(replayBytes: 1_000) - #expect(text(filter.filter(Data("old\u{1B}[c".utf8))) == "old") - - filter.endReplay() - - #expect(text(filter.filter(Data("\u{1B}[c".utf8))) == "\u{1B}[c") - } } From 29aa234be727bd37deaf6afc2d661bf923719527 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 19:36:19 -0700 Subject: [PATCH 32/48] cmux ssh: test that a stop request mid clipboard reply drops the rest Simulates the stdin pump's stop path: when the output side asks it to stop filtering while an OSC 52 reply is being discarded, the rest of the reply's base64 and its terminator are currently forwarded to the remote PTY as typed input. Co-Authored-By: Claude Opus 5.5 --- ...connectInputByteFilterClipboardTests.swift | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift index 50f2885272f8..0f4ba9ea2a60 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift @@ -74,6 +74,27 @@ struct SSHPTYReconnectInputByteFilterClipboardTests { #expect(filter.filter(normalInput) == normalInput) } + @Test( + "a stop request mid clipboard reply still discards the rest of the reply", + arguments: terminators + ) + func stopRequestMidReplyDiscardsRestOfReply(terminator: String) { + var filter = SSHPTYReconnectInputByteFilter(enabled: true) + #expect(filter.filter(Data("\u{1B}]52;c;c2Vj".utf8)) == Data()) + + let normalInput = Data("ls\n".utf8) + let forwarded = stopAndForward(&filter, later: [ + Data("cmV0".utf8), + Data("LXRva2Vu\(terminator)".utf8) + normalInput, + Data("\u{1B}]52;c;bGl2ZQ==\u{07}".utf8), + ]) + + // Only the reply in flight is dropped; later input is live, including + // a later clipboard reply for a query the live shell made. + #expect(forwarded == normalInput + Data("\u{1B}]52;c;bGl2ZQ==\u{07}".utf8)) + #expect(!filter.isFilteringActive) + } + @Test("OSC 52 bytes after filtering ends are forwarded unchanged") func forwardsClipboardSequenceAfterFilteringEnds() { var filter = SSHPTYReconnectInputByteFilter(enabled: true) @@ -88,6 +109,21 @@ struct SSHPTYReconnectInputByteFilterClipboardTests { /// does when no byte arrives within its 25 ms continuation timeout: a /// filter reporting `hasPendingInput` is ended with `stopFiltering()` and /// the returned bytes are forwarded to the remote PTY. + /// Applies what the CLI stdin pump does when the output side asks it to + /// stop filtering: it forwards what `stopFiltering()` returns, then routes + /// each later read through the filter only while `isFilteringActive`, + /// forwarding reads unchanged once the filter reports it is done. + private func stopAndForward( + _ filter: inout SSHPTYReconnectInputByteFilter, + later reads: [Data] + ) -> Data { + var forwarded = filter.stopFiltering() + for read in reads { + forwarded.append(filter.isFilteringActive ? filter.filter(read) : read) + } + return forwarded + } + private func expireContinuationTimeout(_ filter: inout SSHPTYReconnectInputByteFilter) -> Data { filter.hasPendingInput ? filter.stopFiltering() : Data() } From 1ac877da566418d9291945283fb5ac8613d57ea5 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 21:45:18 -0700 Subject: [PATCH 33/48] cmux ssh: keep discarding a clipboard reply after a reconnect stop A stop request from the output side ended reconnect filtering at once, so the rest of an OSC 52 reply that was mid-discard reached the remote PTY as typed input. stopFiltering() now keeps discarding that reply through its BEL/ST, and the stdin pump keeps routing input through the filter until it finishes. Only the reconnect deadline (stopFilteringAtDeadline) abandons a reply whose terminator never arrives. Co-Authored-By: Claude Opus 5.5 --- CLI/SSHPTYAttachReconnectInputFilter.swift | 21 ++++++- .../SSHPTYReconnectInputByteFilter.swift | 55 +++++++++++++++---- ...connectInputByteFilterClipboardTests.swift | 6 +- 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/CLI/SSHPTYAttachReconnectInputFilter.swift b/CLI/SSHPTYAttachReconnectInputFilter.swift index 4c998cd3a25d..4da6da04569a 100644 --- a/CLI/SSHPTYAttachReconnectInputFilter.swift +++ b/CLI/SSHPTYAttachReconnectInputFilter.swift @@ -104,6 +104,7 @@ final class SSHPTYAttachReconnectInputFilter { var reconnectInputFilter = reconnectInputFilterState.map(SSHPTYAttachReconnectInputFilter.init(state:)) var stopSignalFD = initialStopSignalFD var stopAcknowledgementFD = initialStopAcknowledgementFD + var reconnectStopAcknowledged = false var buffer = [UInt8](repeating: 0, count: 8192) defer { if let stopSignalFD { @@ -164,6 +165,15 @@ final class SSHPTYAttachReconnectInputFilter { defer { acknowledgeStopFiltering() closeStopSignal() + reconnectStopAcknowledged = true + } + // Callers flush pending probe bytes before stopping, so only a + // clipboard reply that is mid-discard can remain. Keep routing + // input through the filter until that reply's terminator (or the + // reconnect deadline) so its tail never reaches the remote PTY. + if let filter = reconnectInputFilter { + _ = filter.stopFiltering() + if filter.isFilteringActive { return true } } reconnectInputFilter = nil return true @@ -172,13 +182,14 @@ final class SSHPTYAttachReconnectInputFilter { func finishStdin() { // Reconnect input can disappear with the old bridge during wake. // It is not an intentional EOF for the newly attached remote PTY. - guard reconnectInputFilter == nil else { return } + guard reconnectInputFilter == nil || reconnectStopAcknowledged else { return } _ = shutdown(fd, SHUT_WR) } func stopReconnectFilteringAtDeadline() async -> Bool { guard let filter = reconnectInputFilter else { return true } - guard await writeOrShutdown(filter.stopFiltering()) else { return false } + guard await writeOrShutdown(filter.stopFilteringAtDeadline()) else { return false } + reconnectInputFilter = nil return stopReconnectFiltering() } @@ -273,7 +284,7 @@ final class SSHPTYAttachReconnectInputFilter { func filter(_ data: Data) -> Data { if isDeadlineReached { - var output = stopFiltering() + var output = stopFilteringAtDeadline() output.append(data) return output } @@ -288,6 +299,10 @@ final class SSHPTYAttachReconnectInputFilter { byteFilter.stopFiltering() } + func stopFilteringAtDeadline() -> Data { + byteFilter.stopFilteringAtDeadline() + } + var hasPendingInput: Bool { byteFilter.hasPendingInput } diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift index 6afcc9c83713..69edd2ff3912 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/SSHPTYReconnectInputByteFilter.swift @@ -5,7 +5,10 @@ public import Foundation /// /// Filtering remains active only while input consists entirely of recognized /// terminal replies or EOT bytes. The first ordinary key byte ends filtering, -/// and that byte plus all later input passes through unchanged. +/// and that byte plus all later input passes through unchanged. An OSC 52 +/// reply that has started is always discarded through its terminator, even +/// when ``stopFiltering()`` arrives mid-reply; only +/// ``stopFilteringAtDeadline()`` abandons it. public struct SSHPTYReconnectInputByteFilter: Sendable { private static let escape: UInt8 = 0x1B private static let endOfTransmission: UInt8 = 0x04 @@ -33,8 +36,9 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { /// /// Clipboard replies carry the user's clipboard and can exceed the /// pending-probe bound, so their bytes are dropped as they stream in - /// instead of being buffered and later flushed to the remote PTY. See - /// ``hasPendingInput`` for how long discarding may last. + /// instead of being buffered and later flushed to the remote PTY. It + /// survives ``stopFiltering()`` and ends at BEL/ST or at + /// ``stopFilteringAtDeadline()``. private var discardingClipboardReply = false /// Creates a reconnect-input filter. @@ -54,7 +58,7 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { /// - Parameter data: Raw bytes read from the reconnecting terminal. /// - Returns: Bytes that should be forwarded to the remote PTY. public mutating func filter(_ data: Data) -> Data { - guard isFiltering, !data.isEmpty else { + guard isFiltering || discardingClipboardReply, !data.isEmpty else { return data } @@ -71,6 +75,11 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { } discardingClipboardReply = false index = end + guard isFiltering else { + // Filtering stopped mid-reply: only the reply was discarded. + output.append(contentsOf: bytes[end...]) + return output + } } while index < bytes.count { if bytes[index] == Self.endOfTransmission { @@ -118,6 +127,8 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { /// Returns any incomplete escape sequence retained by the filter. /// + /// A clipboard reply being discarded is dropped, never returned. + /// /// - Returns: Pending bytes in their original order. public mutating func finish() -> Data { if discardingClipboardReply { @@ -134,10 +145,29 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { return data } - /// Ends filtering and returns any retained incomplete sequence. + /// Ends probe filtering and returns any retained incomplete sequence. + /// + /// If an OSC 52 clipboard reply is being discarded, the rest of that reply + /// is still discarded through its BEL/ST, so it never reaches the remote + /// PTY as typed input; ``isFilteringActive`` stays true until then and + /// the caller must keep routing input through ``filter(_:)``. Input after + /// the terminator passes through unchanged. /// /// - Returns: Pending bytes that must be forwarded before live input. public mutating func stopFiltering() -> Data { + isFiltering = false + guard !discardingClipboardReply else { return Data() } + return finish() + } + + /// Ends filtering unconditionally when the reconnect deadline expires. + /// + /// Unlike ``stopFiltering()``, a clipboard reply whose terminator never + /// arrived is abandoned (its retained bytes are dropped), so a lost + /// terminator cannot swallow input past the deadline. + /// + /// - Returns: Retained probe bytes that must be forwarded before live input. + public mutating func stopFilteringAtDeadline() -> Data { let input = finish() isFiltering = false return input @@ -150,10 +180,10 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { /// A clipboard reply being discarded is deliberately excluded: a pause in /// the middle of one must not end filtering, or the rest of the clipboard /// would reach the remote PTY. Discarding therefore lasts until BEL/ST - /// arrives or filtering stops (the caller's reconnect deadline, - /// ``finish()`` or ``stopFiltering()``), and nothing retained is forwarded. - /// The trade-off: if the terminator is lost, input typed before that - /// deadline is discarded with the reply. + /// arrives, ``finish()`` or ``stopFilteringAtDeadline()`` (the caller's + /// reconnect deadline); ``stopFiltering()`` does not end it, and nothing + /// retained is forwarded. The trade-off: if the terminator is lost, input + /// typed before that deadline is discarded with the reply. public var hasPendingInput: Bool { isFiltering && !pending.isEmpty && !discardingClipboardReply } @@ -163,9 +193,12 @@ public struct SSHPTYReconnectInputByteFilter: Sendable { isFiltering && pending.isEmpty && !discardingClipboardReply } - /// Whether recognized reconnect-time replies are still being removed. + /// Whether input must still be routed through ``filter(_:)``. + /// + /// True while probe filtering is on, and after ``stopFiltering()`` until + /// a clipboard reply that was mid-discard reaches its terminator. public var isFilteringActive: Bool { - isFiltering + isFiltering || discardingClipboardReply } private static func reconnectProbeReplySequence( diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift index 0f4ba9ea2a60..bfa5d96e952d 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/SSHPTYReconnectInputByteFilterClipboardTests.swift @@ -64,12 +64,12 @@ struct SSHPTYReconnectInputByteFilterClipboardTests { #expect(output == normalInput) } - @Test("stopping mid-reply does not forward the partial clipboard reply") - func stopFilteringDropsPartialClipboardReply() { + @Test("the reconnect deadline mid-reply drops the partial clipboard reply") + func deadlineDropsPartialClipboardReply() { var filter = SSHPTYReconnectInputByteFilter(enabled: true) #expect(filter.filter(Data("\u{1B}]52;c;c2VjcmV0".utf8)) == Data()) - #expect(filter.stopFiltering() == Data()) + #expect(filter.stopFilteringAtDeadline() == Data()) let normalInput = Data("ls\n".utf8) #expect(filter.filter(normalInput) == normalInput) } From 5fbb7ed597cbdcfe92019f05f49ed5019940cea1 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 29 Sep 2026 21:44:46 -0700 Subject: [PATCH 34/48] cmux ssh: test that bootstrap works when the remote login shell is not POSIX Co-Authored-By: Claude Opus 5.5 --- .../tests/ssh_cross_platform_bootstrap.rs | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) diff --git a/cmux-tui/crates/cmux-remote/tests/ssh_cross_platform_bootstrap.rs b/cmux-tui/crates/cmux-remote/tests/ssh_cross_platform_bootstrap.rs index 7e0c3d8d4b2c..9efcaf064959 100644 --- a/cmux-tui/crates/cmux-remote/tests/ssh_cross_platform_bootstrap.rs +++ b/cmux-tui/crates/cmux-remote/tests/ssh_cross_platform_bootstrap.rs @@ -195,3 +195,163 @@ async fn ssh_npm_bootstrap_installs_a_package_that_matches_the_pinned_digest() { assert_eq!(fs::read(&fixture.installed).unwrap(), b"published linux executable"); assert!(!fixture.staged.exists()); } + +/// A remote whose login shell is fish or tcsh. OpenSSH joins the remote +/// argv with spaces and hands the string to the login shell, which parses +/// only plain words the way `sh` does. Anything else (`$?`, `{ ...; }`, +/// `[ ... ]`, `( ... )`, redirections) must arrive as one +/// `sh -c '