Fix SSH transport dedupe and loopback review issues
This commit is contained in:
parent
5e7458b920
commit
902ee03019
5 changed files with 230 additions and 47 deletions
154
CLI/cmux.swift
154
CLI/cmux.swift
|
|
@ -927,6 +927,102 @@ final class SocketClient {
|
|||
}
|
||||
}
|
||||
|
||||
struct CLIProcessResult {
|
||||
let status: Int32
|
||||
let stdout: String
|
||||
let stderr: String
|
||||
let timedOut: Bool
|
||||
}
|
||||
|
||||
enum CLIProcessRunner {
|
||||
static func runProcess(
|
||||
executablePath: String,
|
||||
arguments: [String],
|
||||
stdinText: String? = nil,
|
||||
timeout: TimeInterval? = nil
|
||||
) -> CLIProcessResult {
|
||||
let process = Process()
|
||||
process.executableURL = URL(fileURLWithPath: executablePath)
|
||||
process.arguments = arguments
|
||||
|
||||
let stdoutPipe = Pipe()
|
||||
let stderrPipe = Pipe()
|
||||
process.standardOutput = stdoutPipe
|
||||
process.standardError = stderrPipe
|
||||
|
||||
let stdinPipe: Pipe?
|
||||
if stdinText != nil {
|
||||
let pipe = Pipe()
|
||||
process.standardInput = pipe
|
||||
stdinPipe = pipe
|
||||
} else {
|
||||
stdinPipe = nil
|
||||
}
|
||||
|
||||
let finished = DispatchSemaphore(value: 0)
|
||||
process.terminationHandler = { _ in
|
||||
finished.signal()
|
||||
}
|
||||
|
||||
do {
|
||||
try process.run()
|
||||
} catch {
|
||||
return CLIProcessResult(status: 1, stdout: "", stderr: String(describing: error), timedOut: false)
|
||||
}
|
||||
|
||||
if let stdinText, let stdinPipe {
|
||||
if let data = stdinText.data(using: .utf8) {
|
||||
stdinPipe.fileHandleForWriting.write(data)
|
||||
}
|
||||
stdinPipe.fileHandleForWriting.closeFile()
|
||||
}
|
||||
|
||||
let timedOut: Bool
|
||||
if let timeout {
|
||||
switch finished.wait(timeout: .now() + timeout) {
|
||||
case .success:
|
||||
timedOut = false
|
||||
case .timedOut:
|
||||
timedOut = true
|
||||
terminate(process: process, finished: finished)
|
||||
}
|
||||
} else {
|
||||
finished.wait()
|
||||
timedOut = false
|
||||
}
|
||||
|
||||
let stdout = String(data: stdoutPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? ""
|
||||
var stderr = String(data: stderrPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? ""
|
||||
if timedOut {
|
||||
let timeoutMessage = "process timed out"
|
||||
if stderr.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty {
|
||||
stderr = timeoutMessage
|
||||
} else if !stderr.contains(timeoutMessage) {
|
||||
stderr += "\n\(timeoutMessage)"
|
||||
}
|
||||
}
|
||||
|
||||
return CLIProcessResult(
|
||||
status: timedOut ? 124 : process.terminationStatus,
|
||||
stdout: stdout,
|
||||
stderr: stderr,
|
||||
timedOut: timedOut
|
||||
)
|
||||
}
|
||||
|
||||
private static func terminate(process: Process, finished: DispatchSemaphore) {
|
||||
guard process.isRunning else { return }
|
||||
process.terminate()
|
||||
if finished.wait(timeout: .now() + 0.5) == .success {
|
||||
return
|
||||
}
|
||||
if process.isRunning {
|
||||
kill(process.processIdentifier, SIGKILL)
|
||||
}
|
||||
_ = finished.wait(timeout: .now() + 0.5)
|
||||
}
|
||||
}
|
||||
|
||||
struct CMUXCLI {
|
||||
let args: [String]
|
||||
|
||||
|
|
@ -3430,7 +3526,17 @@ struct CMUXCLI {
|
|||
private func prepareSSHTerminfoIfNeeded(_ options: SSHCommandOptions) {
|
||||
guard let terminfoSource = localXtermGhosttyTerminfoSource(), !terminfoSource.isEmpty else { return }
|
||||
|
||||
let effectiveSSHOptions = effectiveSSHOptions(
|
||||
options.sshOptions,
|
||||
remoteRelayPort: options.remoteRelayPort
|
||||
)
|
||||
var args = baseSSHArguments(options)
|
||||
if !hasSSHOptionKey(effectiveSSHOptions, key: "ConnectTimeout") {
|
||||
args += ["-o", "ConnectTimeout=3"]
|
||||
}
|
||||
if !hasSSHOptionKey(effectiveSSHOptions, key: "ConnectionAttempts") {
|
||||
args += ["-o", "ConnectionAttempts=1"]
|
||||
}
|
||||
args += ["-o", "BatchMode=yes", "-o", "ControlMaster=no", options.destination]
|
||||
let installScript = """
|
||||
infocmp xterm-ghostty >/dev/null 2>&1 && exit 0
|
||||
|
|
@ -3443,7 +3549,8 @@ struct CMUXCLI {
|
|||
_ = runProcess(
|
||||
executablePath: "/usr/bin/ssh",
|
||||
arguments: Array(args.dropFirst()),
|
||||
stdinText: terminfoSource
|
||||
stdinText: terminfoSource,
|
||||
timeout: 4.0
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -3818,43 +3925,16 @@ struct CMUXCLI {
|
|||
private func runProcess(
|
||||
executablePath: String,
|
||||
arguments: [String],
|
||||
stdinText: String? = nil
|
||||
stdinText: String? = nil,
|
||||
timeout: TimeInterval? = nil
|
||||
) -> (status: Int32, stdout: String, stderr: String) {
|
||||
let process = Process()
|
||||
process.executableURL = URL(fileURLWithPath: executablePath)
|
||||
process.arguments = arguments
|
||||
|
||||
let stdoutPipe = Pipe()
|
||||
let stderrPipe = Pipe()
|
||||
process.standardOutput = stdoutPipe
|
||||
process.standardError = stderrPipe
|
||||
|
||||
let stdinPipe: Pipe?
|
||||
if stdinText != nil {
|
||||
let pipe = Pipe()
|
||||
process.standardInput = pipe
|
||||
stdinPipe = pipe
|
||||
} else {
|
||||
stdinPipe = nil
|
||||
}
|
||||
|
||||
do {
|
||||
try process.run()
|
||||
} catch {
|
||||
return (1, "", String(describing: error))
|
||||
}
|
||||
|
||||
if let stdinText, let stdinPipe {
|
||||
if let data = stdinText.data(using: .utf8) {
|
||||
stdinPipe.fileHandleForWriting.write(data)
|
||||
}
|
||||
stdinPipe.fileHandleForWriting.closeFile()
|
||||
}
|
||||
|
||||
process.waitUntilExit()
|
||||
let stdout = String(data: stdoutPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? ""
|
||||
let stderr = String(data: stderrPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? ""
|
||||
return (process.terminationStatus, stdout, stderr)
|
||||
let result = CLIProcessRunner.runProcess(
|
||||
executablePath: executablePath,
|
||||
arguments: arguments,
|
||||
stdinText: stdinText,
|
||||
timeout: timeout
|
||||
)
|
||||
return (result.status, result.stdout, result.stderr)
|
||||
}
|
||||
|
||||
private func runBrowserCommand(
|
||||
|
|
|
|||
|
|
@ -2895,7 +2895,7 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
}
|
||||
|
||||
private static func remoteProxyLoopbackAliasURL(for url: URL) -> URL? {
|
||||
guard let scheme = url.scheme?.lowercased(), scheme == "http" || scheme == "https" else { return nil }
|
||||
guard let scheme = url.scheme?.lowercased(), scheme == "http" else { return nil }
|
||||
guard let host = BrowserInsecureHTTPSettings.normalizeHost(url.host ?? "") else { return nil }
|
||||
guard remoteLoopbackHosts.contains(host) else { return nil }
|
||||
|
||||
|
|
|
|||
|
|
@ -2209,15 +2209,7 @@ private final class WorkspaceRemoteProxyBroker {
|
|||
}
|
||||
|
||||
private static func transportKey(for configuration: WorkspaceRemoteConfiguration) -> String {
|
||||
let destination = configuration.destination.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
let port = configuration.port.map(String.init) ?? ""
|
||||
let identity = configuration.identityFile?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
|
||||
let localProxyPort = configuration.localProxyPort.map(String.init) ?? ""
|
||||
let options = configuration.sshOptions
|
||||
.map { $0.trimmingCharacters(in: .whitespacesAndNewlines) }
|
||||
.filter { !$0.isEmpty }
|
||||
.joined(separator: "\u{1f}")
|
||||
return [destination, port, identity, options, localProxyPort].joined(separator: "\u{1e}")
|
||||
configuration.proxyBrokerTransportKey
|
||||
}
|
||||
|
||||
private static func allocateLoopbackPort() -> Int? {
|
||||
|
|
@ -4230,6 +4222,36 @@ struct WorkspaceRemoteConfiguration: Equatable {
|
|||
guard let port else { return destination }
|
||||
return "\(destination):\(port)"
|
||||
}
|
||||
|
||||
var proxyBrokerTransportKey: String {
|
||||
let normalizedDestination = destination.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
let normalizedPort = port.map(String.init) ?? ""
|
||||
let normalizedIdentity = identityFile?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
|
||||
let normalizedLocalProxyPort = localProxyPort.map(String.init) ?? ""
|
||||
let normalizedOptions = Self.proxyBrokerSSHOptions(sshOptions).joined(separator: "\u{1f}")
|
||||
return [normalizedDestination, normalizedPort, normalizedIdentity, normalizedOptions, normalizedLocalProxyPort]
|
||||
.joined(separator: "\u{1e}")
|
||||
}
|
||||
|
||||
private static func proxyBrokerSSHOptions(_ options: [String]) -> [String] {
|
||||
options.compactMap { option in
|
||||
let trimmed = option.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
guard !trimmed.isEmpty else { return nil }
|
||||
return trimmed
|
||||
}.filter { option in
|
||||
proxyBrokerSSHOptionKey(option) != "controlpath"
|
||||
}
|
||||
}
|
||||
|
||||
private static func proxyBrokerSSHOptionKey(_ option: String) -> String? {
|
||||
let trimmed = option.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
guard !trimmed.isEmpty else { return nil }
|
||||
return trimmed
|
||||
.split(whereSeparator: { $0 == "=" || $0.isWhitespace })
|
||||
.first
|
||||
.map(String.init)?
|
||||
.lowercased()
|
||||
}
|
||||
}
|
||||
|
||||
enum SidebarPullRequestStatus: String {
|
||||
|
|
|
|||
20
cmuxTests/CLIProcessRunnerTests.swift
Normal file
20
cmuxTests/CLIProcessRunnerTests.swift
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
import XCTest
|
||||
|
||||
#if canImport(cmux)
|
||||
@testable import cmux
|
||||
|
||||
final class CLIProcessRunnerTests: XCTestCase {
|
||||
func testRunProcessTimesOutHungChild() {
|
||||
let startedAt = Date()
|
||||
let result = CLIProcessRunner.runProcess(
|
||||
executablePath: "/bin/sh",
|
||||
arguments: ["-c", "sleep 5"],
|
||||
timeout: 0.2
|
||||
)
|
||||
|
||||
XCTAssertTrue(result.timedOut)
|
||||
XCTAssertEqual(result.status, 124)
|
||||
XCTAssertLessThan(Date().timeIntervalSince(startedAt), 2.0)
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
|
@ -909,6 +909,28 @@ final class BrowserPanelRemoteStoreTests: XCTestCase {
|
|||
XCTAssertEqual(panel.webView.url?.host, "cmux-loopback.localtest.me")
|
||||
}
|
||||
|
||||
func testRemoteWorkspaceKeepsHTTPSLoopbackUnaliased() {
|
||||
let remoteWorkspaceId = UUID()
|
||||
let url = URL(string: "https://localhost:3443/demo")!
|
||||
let panel = BrowserPanel(
|
||||
workspaceId: remoteWorkspaceId,
|
||||
initialURL: url,
|
||||
isRemoteWorkspace: true,
|
||||
remoteWebsiteDataStoreIdentifier: remoteWorkspaceId
|
||||
)
|
||||
|
||||
XCTAssertEqual(panel.preferredURLStringForOmnibar(), url.absoluteString)
|
||||
XCTAssertNil(panel.webView.url)
|
||||
|
||||
panel.setRemoteProxyEndpoint(BrowserProxyEndpoint(host: "127.0.0.1", port: 9876))
|
||||
|
||||
let deadline = Date().addingTimeInterval(1.0)
|
||||
while panel.webView.url == nil, RunLoop.main.run(mode: .default, before: deadline), Date() < deadline {}
|
||||
|
||||
XCTAssertEqual(panel.preferredURLStringForOmnibar(), url.absoluteString)
|
||||
XCTAssertEqual(panel.webView.url?.host, "localhost")
|
||||
}
|
||||
|
||||
func testNewTerminalSurfaceStaysRemoteWhileBrowserPanelsKeepWorkspaceRemote() throws {
|
||||
let workspace = Workspace()
|
||||
let paneId = try XCTUnwrap(workspace.bonsplitController.allPaneIds.first)
|
||||
|
|
@ -941,6 +963,45 @@ final class BrowserPanelRemoteStoreTests: XCTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
final class WorkspaceRemoteConfigurationTransportKeyTests: XCTestCase {
|
||||
func testProxyBrokerTransportKeyIgnoresControlPath() {
|
||||
let first = WorkspaceRemoteConfiguration(
|
||||
destination: "cmux-macmini",
|
||||
port: 22,
|
||||
identityFile: "~/.ssh/id_ed25519",
|
||||
sshOptions: [
|
||||
"Compression=yes",
|
||||
"ControlMaster=auto",
|
||||
"ControlPath=/tmp/cmux-ssh-501-64000-%C",
|
||||
],
|
||||
localProxyPort: 9000,
|
||||
relayPort: 64000,
|
||||
relayID: "relay-a",
|
||||
relayToken: "token-a",
|
||||
localSocketPath: "/tmp/cmux-a.sock",
|
||||
terminalStartupCommand: "ssh cmux-macmini"
|
||||
)
|
||||
let second = WorkspaceRemoteConfiguration(
|
||||
destination: "cmux-macmini",
|
||||
port: 22,
|
||||
identityFile: "~/.ssh/id_ed25519",
|
||||
sshOptions: [
|
||||
"Compression=yes",
|
||||
"ControlMaster=auto",
|
||||
"ControlPath=/tmp/cmux-ssh-501-64001-%C",
|
||||
],
|
||||
localProxyPort: 9000,
|
||||
relayPort: 64001,
|
||||
relayID: "relay-b",
|
||||
relayToken: "token-b",
|
||||
localSocketPath: "/tmp/cmux-b.sock",
|
||||
terminalStartupCommand: "ssh cmux-macmini"
|
||||
)
|
||||
|
||||
XCTAssertEqual(first.proxyBrokerTransportKey, second.proxyBrokerTransportKey)
|
||||
}
|
||||
}
|
||||
|
||||
final class WorkspaceRemoteDaemonPendingCallRegistryTests: XCTestCase {
|
||||
func testSupportsMultiplePendingCallsResolvedOutOfOrder() {
|
||||
let registry = WorkspaceRemoteDaemonPendingCallRegistry()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue