Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
214 changes: 212 additions & 2 deletions Sources/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4006,20 +4006,230 @@ final class WorkspaceRemoteSessionController {
}
}

/// Resolved SSH transport options with metadata about what was set.
private struct ResolvedTransportOptions {
let args: [String]
let hasStrictHostKeyChecking: Bool
}

private func reverseRelayArguments(relayPort: Int, localRelayPort: Int) -> [String] {
// Fallback standalone transport when dynamic forwarding through an existing
// control master is unavailable.
var args: [String] = ["-N", "-T", "-S", "none"]
args += sshCommonArguments(batchMode: true)
//
// Uses -F /dev/null to bypass ~/.ssh/config entirely, preventing config-file
// LocalForward/DynamicForward/RemoteForward directives from conflicting with
// ports already bound by the primary SSH connection. Transport-critical options
// (ProxyJump, ProxyCommand, HostKeyAlgorithms, etc.) are resolved via ssh -G
// and passed explicitly so connectivity is preserved without config-file forwards.
let resolved = resolvedRelayTransportOptions()
var args: [String] = ["-N", "-T", "-S", "none", "-F", "/dev/null"]
args += resolved.args
args += [
"-o", "ConnectTimeout=6",
"-o", "ServerAliveInterval=20",
"-o", "ServerAliveCountMax=2",
"-o", "BatchMode=yes",
"-o", "ExitOnForwardFailure=yes",
"-o", "RequestTTY=no",
]
// Only add StrictHostKeyChecking default if not resolved from ssh -G and not
// explicitly set in configuration.sshOptions.
if !resolved.hasStrictHostKeyChecking
&& !hasSSHOptionKey(configuration.sshOptions, key: "StrictHostKeyChecking") {
args += ["-o", "StrictHostKeyChecking=accept-new"]
}
args += [
"-R", "127.0.0.1:\(relayPort):127.0.0.1:\(localRelayPort)",
configuration.destination,
]
return args
}

/// Resolve transport-critical SSH options for the relay by running `ssh -G`.
/// This extracts options like ProxyJump, ProxyCommand, IdentityFile, HostKeyAlgorithms,
/// etc. from the user's SSH config without carrying over forwarding directives.
private func resolvedRelayTransportOptions() -> ResolvedTransportOptions {
// Build the ssh -G probe command with the same port/identity/options as the
// primary connection so Host/Match blocks resolve identically.
var probeArgs: [String] = ["-G"]
if let port = configuration.port {
probeArgs += ["-p", String(port)]
}
if let identityFile = configuration.identityFile,
!identityFile.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty {
probeArgs += ["-i", identityFile]
}
for option in backgroundSSHOptions(configuration.sshOptions) {
probeArgs += ["-o", option]
}
probeArgs.append(configuration.destination)

guard let result = try? sshExec(arguments: probeArgs, timeout: 5),
result.status == 0 else {
debugLog("remote.relay.sshG.failed falling back to explicit config only")
// Fallback: use just the explicitly configured options without ssh -G resolution.
return ResolvedTransportOptions(
args: explicitRelayTransportOptions(),
hasStrictHostKeyChecking: false
)
}

return parseRelayTransportOptions(from: result.stdout)
}

/// Keys from `ssh -G` output that are needed for transport but NOT forwarding.
private static let relayTransportKeys: Set<String> = [
"hostname",
"user",
"port",
"proxyjump",
"proxycommand",
"identityfile",
"identitiesonly",
"certificatefile",
"identityagent",
"hostkeyalgorithms",
"kexalgorithms",
"ciphers",
"macs",
"hostkeyalias",
"userknownhostsfile",
"globalknownhostsfile",
"pubkeyacceptedalgorithms",
"preferredauthentications",
"stricthostkeychecking",
]

/// Parse `ssh -G` output and extract transport-critical options as `-o Key=Value` pairs.
private func parseRelayTransportOptions(from sshGOutput: String) -> ResolvedTransportOptions {
var args: [String] = []
var hasStrictHostKeyChecking = false

for line in sshGOutput.split(separator: "\n") {
let parts = line.split(maxSplits: 1, whereSeparator: \.isWhitespace)
guard parts.count == 2 else { continue }
let key = parts[0].lowercased()
let value = parts[1].trimmingCharacters(in: .whitespacesAndNewlines)

guard Self.relayTransportKeys.contains(key), !value.isEmpty else { continue }

switch key {
case "hostname":
args += ["-o", "Hostname=\(value)"]
case "user":
args += ["-l", value]
case "port":
// Prefer explicitly configured port; fall back to ssh -G resolved value.
let effectivePort = configuration.port.map(String.init) ?? value
if let portNum = Int(effectivePort), portNum != 22 {
args += ["-p", effectivePort]
}
case "identityfile":
// ssh -G may emit multiple identity files; include all that exist on disk.
let expanded = (value as NSString).expandingTildeInPath
if FileManager.default.fileExists(atPath: expanded) {
args += ["-i", value]
}
case "proxyjump":
if value.lowercased() != "none" {
// Resolve the ProxyJump chain so jump hosts don't need config-file
// resolution (since -F /dev/null is propagated to jump connections).
let resolved = resolveProxyJumpChain(value)
args += ["-J", resolved]
Comment on lines +4133 to +4138
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

tmp="$(mktemp)"
trap 'rm -f "$tmp"' EXIT

cat >"$tmp" <<'EOF'
Host jumpalias
  HostName bastion.example.com

Host target
  ProxyJump bob@jumpalias:2222
EOF

echo 'proxyjump emitted for target:'
ssh -F "$tmp" -G target | awk '/^proxyjump /{print}'

echo
echo 'resolved jumpalias host:'
ssh -F "$tmp" -G jumpalias | awk '/^(hostname|user|port) /{print}'

Repository: manaflow-ai/cmux

Length of output: 355


🏁 Script executed:

grep -n "resolveProxyJumpChain" Sources/Workspace.swift | head -20

Repository: manaflow-ai/cmux

Length of output: 201


🏁 Script executed:

sed -n '4100,4200p' Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 5003


🏁 Script executed:

ast-grep --pattern 'func resolveProxyJumpChain($$$) {
  $$$
}'

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

sed -n '4171,4220p' Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 2262


🏁 Script executed:

sed -n '4176,4180p' Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 392


🏁 Script executed:

rg "proxyjump|resolveProxyJumpChain" -n Sources/Workspace.swift

Repository: manaflow-ai/cmux

Length of output: 262


The fast-path skips resolving ProxyJump hops that contain SSH config aliases.

The pattern check at lines 4176–4180 returns hop unchanged if it matches user@host:port, assuming it's already fully resolved. However, the host portion can still be an SSH config alias (e.g., bob@jumpalias:2222 where jumpalias is a Host alias). This bypasses the resolution logic below and passes the unresolved alias to -J, causing the spawned jump SSH to fail since it inherits -F /dev/null.

The shell test demonstrates this: ssh -G target reports proxyjump bob@jumpalias:2222, but jumpalias resolves to bastion.example.com when checked separately. Parse and resolve the host portion of the hop, or only skip literal IP addresses (e.g., user@192.0.2.1:2222 or user@[::1]:2222).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Workspace.swift` around lines 4133 - 4138, The fast-path in
resolveProxyJumpChain is incorrectly skipping hops that look like user@host:port
even when host is an SSH config alias; update resolveProxyJumpChain (the code
that currently returns hop unchanged for matches) to parse each hop into user,
host, and port, then only skip resolution when the host is a literal IP (match
IPv4 like \d+\.\d+\.\d+\.\d+ or bracketed IPv6 like [::1]); otherwise pass the
extracted host through the existing SSH-config resolution logic and rebuild the
hop (user@resolvedHost:port) before returning/adding to args (see the
"proxyjump" case and args += ["-J", resolved] for where this resolved value is
used).

}
case "proxycommand":
if value.lowercased() != "none" {
args += ["-o", "ProxyCommand=\(value)"]
}
case "stricthostkeychecking":
hasStrictHostKeyChecking = true
args += ["-o", "StrictHostKeyChecking=\(value)"]
default:
args += ["-o", "\(parts[0])=\(value)"]
}
}

return ResolvedTransportOptions(args: args, hasStrictHostKeyChecking: hasStrictHostKeyChecking)
}

/// Resolve a ProxyJump chain by running `ssh -G` on each hop to get its
/// actual hostname, user, and port. This is needed because `-F /dev/null`
/// is propagated to jump host SSH processes, preventing config-file Host
/// alias resolution.
///
/// Input: "jumphost,bastion" (comma-separated hops, may be config aliases)
/// Output: "admin@jump.example.com:22,user@bastion.example.com:22" (resolved)
///
/// Limitation: per-hop config beyond hostname/user/port (e.g. hop-specific
/// IdentityFile, CertificateFile, HostKeyAlias) cannot be represented in
/// the `-J` string and is lost when `-F /dev/null` propagates to jump
/// processes. This affects bastion chains where the jump host requires a
/// dedicated key or host-key alias. The preferred ControlMaster path does
/// not have this limitation since it reuses the primary connection. The SSH
/// agent will handle authentication for most cases; users with non-agent
/// hop-specific keys should ensure the ControlMaster path succeeds.
private func resolveProxyJumpChain(_ chain: String) -> String {
let hops = chain.split(separator: ",").map {
$0.trimmingCharacters(in: .whitespacesAndNewlines)
}

let resolved = hops.map { hop -> String in
// If the hop already looks fully qualified (user@host:port), pass through.
if hop.contains("@") && (hop.contains("]:") || (!hop.contains("[") && hop.last?.isNumber == true && hop.split(separator: ":").count == 2)) {
return hop
}
// Try to resolve via ssh -G
guard let result = try? sshExec(arguments: ["-G", hop], timeout: 5),
result.status == 0 else {
return hop // Can't resolve; pass through as-is
}
var hostname: String?
var user: String?
var port: String?
for line in result.stdout.split(separator: "\n") {
let parts = line.split(maxSplits: 1, whereSeparator: \.isWhitespace)
guard parts.count == 2 else { continue }
let key = parts[0].lowercased()
let val = parts[1].trimmingCharacters(in: .whitespacesAndNewlines)
switch key {
case "hostname": hostname = val
case "user": user = val
case "port": port = val
default: break
}
}
guard let resolvedHost = hostname else { return hop }
var resolved = ""
if let user { resolved += "\(user)@" }
// Bracket IPv6 literals when a non-default port is appended.
let needsPort = port != nil && port != "22"
let isIPv6 = resolvedHost.contains(":")
if isIPv6 && needsPort {
resolved += "[\(resolvedHost)]:\(port!)"
} else {
resolved += resolvedHost
if needsPort { resolved += ":\(port!)" }
}
return resolved
}

return resolved.joined(separator: ",")
}

/// Fallback transport options when ssh -G is unavailable. Uses only the
/// explicitly configured port and identity file.
private func explicitRelayTransportOptions() -> [String] {
var args: [String] = []
if let port = configuration.port {
args += ["-p", String(port)]
}
if let identityFile = configuration.identityFile,
!identityFile.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty {
args += ["-i", identityFile]
}
return args
}

private func startReverseRelayViaControlMasterLocked(forwardSpec: String) -> Bool {
guard let arguments = WorkspaceRemoteSSHBatchCommandBuilder.reverseRelayControlMasterArguments(
configuration: configuration,
Expand Down
2 changes: 1 addition & 1 deletion docs/remote-daemon-spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ This is a **living implementation spec** (also called an **execution spec**): a
- `DONE` `cmuxd-remote` includes a table-driven CLI relay (`cli` subcommand) that maps CLI args to v1 text or v2 JSON-RPC messages.
- `DONE` busybox-style argv[0] detection: when invoked as `cmux` via wrapper/symlink, auto-dispatches to CLI relay.
- `DONE` background `ssh -N -R 127.0.0.1:PORT:127.0.0.1:LOCAL_RELAY_PORT` process reverse-forwards a TCP port to a dedicated authenticated local relay server. Uses TCP instead of Unix socket forwarding because many servers have `AllowStreamLocalForwarding` disabled.
- `DONE` relay process uses `-S none` / standalone SSH transport (avoids ControlMaster multiplexing and inherited `RemoteForward` directives) and `ExitOnForwardFailure=yes` so dead reverse binds fail fast instead of publishing bad relay metadata.
- `DONE` relay process uses `-S none -F /dev/null` / standalone SSH transport with no config file (avoids ControlMaster multiplexing and prevents config-file `LocalForward`/`DynamicForward`/`RemoteForward` directives from conflicting with the primary connection). Transport-critical options (ProxyJump, IdentityFile, etc.) are resolved via `ssh -G` and passed explicitly. `ExitOnForwardFailure=yes` ensures dead reverse binds fail fast instead of publishing bad relay metadata.
- `DONE` relay address written to `~/.cmux/socket_addr` on the remote only after the reverse forward survives startup validation.
- `DONE` Go CLI no longer polls for relay readiness. It dials the published relay once and only refreshes `~/.cmux/socket_addr` a single time to recover from a stale shared address rewrite.
- `DONE` `cmux ssh` startup exports session-local `CMUX_SOCKET_PATH=127.0.0.1:<relay_port>` so parallel sessions pin to their own relay instead of racing on shared socket_addr.
Expand Down