diff --git a/Cargo.lock b/Cargo.lock index 1f04b08872..bf813e606b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6995,6 +6995,7 @@ dependencies = [ "futures", "futures-util", "git2", + "glob", "hex", "hkdf", "hostname", diff --git a/src/apps/desktop/src/api/ssh_api.rs b/src/apps/desktop/src/api/ssh_api.rs index 555c6ac74b..6cf11d3e9b 100644 --- a/src/apps/desktop/src/api/ssh_api.rs +++ b/src/apps/desktop/src/api/ssh_api.rs @@ -1506,6 +1506,7 @@ mod tests { }, default_workspace: Some("/workspace".to_string()), proxy_jump: None, + proxy_command: None, container: Some(ContainerWorkspaceConfig { name: "dev".to_string(), access: ContainerAccess::DockerExec, diff --git a/src/crates/assembly/core/src/agentic/tools/implementations/port_forward_tool.rs b/src/crates/assembly/core/src/agentic/tools/implementations/port_forward_tool.rs index f7d532a6a2..8658eeb465 100644 --- a/src/crates/assembly/core/src/agentic/tools/implementations/port_forward_tool.rs +++ b/src/crates/assembly/core/src/agentic/tools/implementations/port_forward_tool.rs @@ -234,7 +234,10 @@ impl PortForwardTool { username, auth, default_workspace: None, - proxy_jump: config_entry.and_then(|entry| entry.proxy_jump), + proxy_jump: config_entry + .as_ref() + .and_then(|entry| entry.proxy_jump.clone()), + proxy_command: config_entry.and_then(|entry| entry.proxy_command), container: None, wsl: None, options: SSHConnectionOptions::default(), diff --git a/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs b/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs index 13ccbaa156..9554b46cd6 100644 --- a/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs +++ b/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs @@ -1201,6 +1201,7 @@ mod tests { default_workspace: Some("/srv/project".to_string()), last_connected: Some(1), proxy_jump: Some("jump.example.invalid".to_string()), + proxy_command: None, container: None, options: owner::SshConnectionOptionsRecord::default(), } diff --git a/src/crates/services/services-integrations/Cargo.toml b/src/crates/services/services-integrations/Cargo.toml index b3e73a73cf..71f374d6b4 100644 --- a/src/crates/services/services-integrations/Cargo.toml +++ b/src/crates/services/services-integrations/Cargo.toml @@ -36,6 +36,7 @@ chrono = { workspace = true, optional = true } dunce = { workspace = true, optional = true } futures = { workspace = true, optional = true } futures-util = { workspace = true, optional = true } +glob = { workspace = true, optional = true } fs2 = { workspace = true, optional = true } git2 = { workspace = true, optional = true } hostname = { workspace = true, optional = true } @@ -510,7 +511,7 @@ product-full = [ "workspace-search", ] skillhub = ["openbitfun-services-core/tls-provider", "reqwest", "reqwest/json", "reqwest/query", "reqwest/rustls-no-provider", "reqwest/stream", "zip", "tempfile", "tokio/fs", "tokio/io-util"] -ssh_config = ["dep:ssh_config"] +ssh_config = ["dep:ssh_config", "dep:glob"] [dev-dependencies] tempfile = { workspace = true } diff --git a/src/crates/services/services-integrations/src/remote_persistence.rs b/src/crates/services/services-integrations/src/remote_persistence.rs index 42c2504e37..75c0e33b9d 100644 --- a/src/crates/services/services-integrations/src/remote_persistence.rs +++ b/src/crates/services/services-integrations/src/remote_persistence.rs @@ -615,6 +615,8 @@ pub struct SavedConnectionRecord { pub last_connected: Option, #[serde(default)] pub proxy_jump: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub proxy_command: Option, #[serde(default)] pub container: Option, #[serde(default)] diff --git a/src/crates/services/services-integrations/src/remote_ssh/manager.rs b/src/crates/services/services-integrations/src/remote_ssh/manager.rs index 898f5a0761..a426618564 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/manager.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/manager.rs @@ -2,7 +2,10 @@ //! //! This module manages SSH connections using the pure-Russ SSH implementation +use super::proxy_command::{expand_tokens, ProxyCommandChild}; use super::sftp_file::ManagedSftpSession; +#[cfg(feature = "ssh_config")] +use super::ssh_config_loader::load_merged_config; use crate::remote_ssh::password_vault::SSHPasswordVault; use crate::remote_ssh::types::{ ConnectionTestReport, ConnectionTestStage, ContainerAccess, ContainerWorkspaceConfig, @@ -455,12 +458,23 @@ fn parse_ssh_config_value(value: &str) -> Option<&str> { value.split_whitespace().next() } +/// Extract a whole-line command value such as `ProxyCommand`'s. +/// +/// Unlike [`parse_ssh_config_value`], the value spans every remaining token of +/// the line. A single pair of surrounding double quotes is stripped so +/// `ProxyCommand "coder ssh --stdio %h"` keeps its inner quoting intact. #[cfg(feature = "ssh_config")] -fn strip_utf8_bom(content: String) -> String { - content - .strip_prefix('\u{feff}') - .unwrap_or(&content) - .to_string() +fn parse_ssh_config_command_value(value: &str) -> Option { + let value = value.trim(); + if value.is_empty() { + return None; + } + if let Some(inner) = value.strip_prefix('"') { + if let Some(stripped) = inner.strip_suffix('"') { + return Some(stripped.to_string()); + } + } + Some(value.to_string()) } /// Manually parse `~/.ssh/config` content into Host blocks with their direct settings. @@ -484,6 +498,7 @@ fn parse_ssh_config_manually(content: &str) -> Vec { let mut block_identity_file: Option = None; let mut block_certificate_file: Option = None; let mut block_proxy_jump: Option = None; + let mut block_proxy_command: Option = None; for line in content.lines() { let line = line.trim(); @@ -508,6 +523,7 @@ fn parse_ssh_config_manually(content: &str) -> Vec { agent: Some(true), certificate_file: block_certificate_file.take(), proxy_jump: block_proxy_jump.take(), + proxy_command: block_proxy_command.take(), }); } @@ -519,6 +535,7 @@ fn parse_ssh_config_manually(content: &str) -> Vec { block_identity_file = None; block_certificate_file = None; block_proxy_jump = None; + block_proxy_command = None; } else if current_host.is_some() { // Track details within the current Host block if keyword.eq_ignore_ascii_case("HostName") { @@ -535,6 +552,12 @@ fn parse_ssh_config_manually(content: &str) -> Vec { parse_ssh_config_value(value).map(|s| shellexpand::tilde(s).to_string()); } else if keyword.eq_ignore_ascii_case("ProxyJump") { block_proxy_jump = parse_ssh_config_value(value).map(ToOwned::to_owned); + } else if keyword.eq_ignore_ascii_case("ProxyCommand") { + // Keep the raw command text: `%h`/`%p`/`%r` tokens are expanded + // at connect time, and shell quoting must survive intact — + // unlike single-token directives, the whole remainder of the + // line is the value. + block_proxy_command = parse_ssh_config_command_value(value); } } } @@ -550,6 +573,7 @@ fn parse_ssh_config_manually(content: &str) -> Vec { agent: Some(true), certificate_file: block_certificate_file, proxy_jump: block_proxy_jump, + proxy_command: block_proxy_command, }); } @@ -582,6 +606,9 @@ struct ActiveConnection { bounded_sftp_session: Arc, #[allow(dead_code)] server_key: Option, + /// ProxyCommand process carrying the SSH transport; dropping it kills the + /// proxy. Absent for every non-proxy transport. + proxy_child: Option, /// Liveness flag; flipped to false from `SSHHandler::disconnected`. /// Allows `is_connected` and SFTP/exec entry points to detect a dead session /// without waiting for the next failed I/O. @@ -662,6 +689,9 @@ struct EstablishedSession { server_info: Option, effective_config: SSHConnectionConfig, unpublished: UnpublishedSshSession, + /// ProxyCommand process carrying the SSH transport, kept alive exactly as + /// long as the session. `None` for every non-proxy transport. + proxy_child: Option, } /// Owns authenticated transports until the complete chain is published. A @@ -672,6 +702,9 @@ struct UnpublishedSshSession { // Creation order is first jump through final target. One guard owns the // whole chain so cancellation cannot race independent hop cleanups. handles: Vec>>, + // ProxyCommand children spawned for transports that have not been + // published yet; the guard's Drop tears them down with the handles. + proxy_children: Vec, } impl UnpublishedSshSession { @@ -681,6 +714,11 @@ impl UnpublishedSshSession { fn disarm(&mut self) { self.handles.clear(); + self.proxy_children.clear(); + } + + fn take_proxy_child(&mut self) -> Option { + self.proxy_children.pop() } } @@ -2410,18 +2448,13 @@ impl SSHConnectionManager { .map(|p| p.join(".ssh").join("config")) .unwrap_or_default(); - if !ssh_config_path.exists() { - log::debug!("SSH config not found at {:?}", ssh_config_path); - return SSHConfigLookupResult { - found: false, - config: None, - }; - } - - let config_content = match tokio::fs::read_to_string(&ssh_config_path).await { - Ok(c) => strip_utf8_bom(c), - Err(e) => { - log::warn!("Failed to read SSH config: {:?}", e); + // Load the user config merged with its `Include` tree; ecosystem + // tooling (Coder, AtomGit, devenv) writes Host blocks into included + // files, and `ssh_config` 0.1 cannot expand them itself. + let config_content = match load_merged_config(&ssh_config_path) { + Some(content) => content, + None => { + log::debug!("SSH config not found at {:?}", ssh_config_path); return SSHConfigLookupResult { found: false, config: None, @@ -2455,6 +2488,8 @@ impl SSHConnectionManager { .map(|f| shellexpand::tilde(f).to_string()); let has_proxy_command = ssh_cfg_has(&host_settings, "ProxyCommand"); let proxy_jump = ssh_cfg_get(&host_settings, "ProxyJump").map(ToOwned::to_owned); + let proxy_command = + ssh_cfg_get(&host_settings, "ProxyCommand").map(ToOwned::to_owned); return SSHConfigLookupResult { found: true, @@ -2467,6 +2502,7 @@ impl SSHConnectionManager { agent: if has_proxy_command { None } else { Some(true) }, certificate_file, proxy_jump, + proxy_command, }), }; } @@ -2504,15 +2540,11 @@ impl SSHConnectionManager { .map(|p| p.join(".ssh").join("config")) .unwrap_or_default(); - if !ssh_config_path.exists() { - log::debug!("SSH config not found at {:?}", ssh_config_path); - return Vec::new(); - } - - let config_content = match tokio::fs::read_to_string(&ssh_config_path).await { - Ok(c) => strip_utf8_bom(c), - Err(e) => { - log::warn!("Failed to read SSH config: {:?}", e); + // Merged view including the `Include` tree; see `get_ssh_config`. + let config_content = match load_merged_config(&ssh_config_path) { + Some(content) => content, + None => { + log::debug!("SSH config not found at {:?}", ssh_config_path); return Vec::new(); } }; @@ -2555,6 +2587,9 @@ impl SSHConnectionManager { if let Some(proxy_jump) = ssh_cfg_get(&settings, "ProxyJump") { entry.proxy_jump = Some(proxy_jump.to_string()); } + if let Some(proxy_command) = ssh_cfg_get(&settings, "ProxyCommand") { + entry.proxy_command = Some(proxy_command.to_string()); + } } } } @@ -2959,6 +2994,7 @@ impl SSHConnectionManager { default_workspace: config.default_workspace.clone(), last_connected: Some(chrono::Utc::now().timestamp() as u64), proxy_jump: config.proxy_jump.clone(), + proxy_command: config.proxy_command.clone(), container: config.container.clone(), wsl: config.wsl.clone(), options: config.options.clone(), @@ -3089,6 +3125,7 @@ impl SSHConnectionManager { sftp_session: Arc::new(SftpCache::new()), bounded_sftp_session: Arc::new(BoundedSftpCache::new()), server_key: None, + proxy_child: established.proxy_child, alive: established.alive, }, ) @@ -3120,8 +3157,14 @@ impl SSHConnectionManager { .proxy_jump .as_deref() .is_some_and(|jump| !jump.trim().is_empty()) + || config + .proxy_command + .as_deref() + .is_some_and(|command| !command.trim().is_empty()) { - anyhow::bail!("WSL cannot be combined with Docker or SSH jump hosts"); + anyhow::bail!( + "WSL cannot be combined with Docker, SSH jump hosts, or ProxyCommand" + ); } let server_info = super::wsl::probe(wsl, Duration::from_secs(timeout_secs)).await?; return Ok(EstablishedSession { @@ -3131,6 +3174,7 @@ impl SSHConnectionManager { server_info: Some(server_info), effective_config: config.clone(), unpublished: UnpublishedSshSession::default(), + proxy_child: None, }); } if config.uses_local_docker() { @@ -3158,9 +3202,36 @@ impl SSHConnectionManager { server_info: Some(server_info), effective_config: resolved_container_config(config, ContainerAccess::DockerExec), unpublished: UnpublishedSshSession::default(), + proxy_child: None, }); } + if let Some(proxy_command) = config + .proxy_command + .as_deref() + .map(str::trim) + .filter(|command| !command.is_empty()) + { + // Deliberately exclusive combinations for this release: a proxy + // command already owns the transport to the target, so layering a + // jump chain or a container target on top has no tested semantics. + if config + .proxy_jump + .as_deref() + .is_some_and(|jump| !jump.trim().is_empty()) + { + anyhow::bail!( + "ProxyCommand and ProxyJump cannot be combined; remove one of them from the connection" + ); + } + if config.container.is_some() { + anyhow::bail!("ProxyCommand cannot be combined with a Docker container target"); + } + return self + .establish_proxy_command_session(config, proxy_command, timeout_secs) + .await; + } + let mut unpublished = UnpublishedSshSession::default(); let jumps = self .resolve_proxy_jump_chain(config) @@ -3192,6 +3263,7 @@ impl SSHConnectionManager { server_info: container_info, effective_config, unpublished, + proxy_child: None, }); } } @@ -3217,6 +3289,7 @@ impl SSHConnectionManager { config.clone() }, unpublished, + proxy_child: None, }); } @@ -3323,6 +3396,7 @@ impl SSHConnectionManager { server_info: container_info, effective_config, unpublished, + proxy_child: None, }); } } @@ -3348,6 +3422,7 @@ impl SSHConnectionManager { config.clone() }, unpublished, + proxy_child: None, }) } @@ -3423,6 +3498,7 @@ impl SSHConnectionManager { server_info, effective_config, unpublished, + proxy_child: None, }) } Err(error) => { @@ -3589,6 +3665,7 @@ impl SSHConnectionManager { auth, default_workspace: None, proxy_jump: None, + proxy_command: None, container: None, wsl: None, options: config.options.clone(), @@ -3685,6 +3762,78 @@ impl SSHConnectionManager { .await } + /// Build a fresh SSH session whose transport is the stdin/stdout of a + /// local ProxyCommand process (OpenSSH semantics). + /// + /// Reusing `establish_stream_session` keeps handshake timeout, known-hosts + /// verification, and authentication identical to the direct path. The child + /// process is owned by `UnpublishedSshSession` until the session is fully + /// established, so every failure path kills it; afterwards the guard moves + /// into the returned `EstablishedSession` (and from there into the + /// published connection record), so disconnects and app shutdown kill it. + async fn establish_proxy_command_session( + &self, + config: &SSHConnectionConfig, + proxy_command: &str, + timeout_secs: u64, + ) -> anyhow::Result { + let stage = format!("proxy command {}", connection_label(config)); + let mut unpublished = UnpublishedSshSession::default(); + + let expanded = expand_tokens(proxy_command, &config.host, config.port, &config.username); + let mut child = ProxyCommandChild::spawn(&expanded, &stage) + .map_err(|error| tag_failed_stage("proxy command", error))?; + let stream = child + .take_stream() + .map_err(|error| tag_failed_stage("proxy command", error))?; + unpublished.proxy_children.push(child); + + let session = self + .establish_stream_session(config, stream, timeout_secs, &stage, &mut unpublished) + .await; + match session { + Ok((handle, alive, server_info)) => { + let proxy_child = unpublished + .take_proxy_child() + .expect("proxy child was pushed before the handshake"); + Ok(EstablishedSession { + handle: Some(handle), + jump_handles: Vec::new(), + alive, + server_info, + effective_config: config.clone(), + unpublished, + proxy_child: Some(proxy_child), + }) + } + Err(error) => { + let detail = match unpublished.take_proxy_child() { + Some(child) => { + let (exit_code, stderr_tail) = child.finish().await; + let mut parts = Vec::new(); + match exit_code { + Some(code) => { + parts.push(format!("proxy command exited with code {code}")) + } + None => parts + .push("proxy command was terminated before completing".to_string()), + } + if !stderr_tail.is_empty() { + parts.push(format!("stderr: {stderr_tail}")); + } + parts.join("; ") + } + None => String::new(), + }; + if detail.is_empty() { + Err(error) + } else { + Err(anyhow!("{error} ({detail})")) + } + } + } + } + async fn probe_remote_container( &self, handle: &Arc>, @@ -4409,6 +4558,7 @@ impl SSHConnectionManager { auth, default_workspace: saved.default_workspace, proxy_jump: saved.proxy_jump, + proxy_command: saved.proxy_command, container: saved.container, wsl: saved.wsl, options: saved.options, @@ -4556,23 +4706,25 @@ impl SSHConnectionManager { // session is usually dead, but "usually" covers neither config drift // (where the old session is perfectly healthy) nor a half-open link the // server still counts against its session limit. - let (stale_handle, stale_jump_handles) = { + let (stale_handle, stale_jump_handles, stale_proxy_child) = { let mut guard = self.connections.write().await; established.unpublished.disarm(); if let Some(conn) = guard.get_mut(connection_id) { let stale_handle = conn.handle.take(); let stale_jump_handles = std::mem::take(&mut conn.jump_handles); + let stale_proxy_child = conn.proxy_child.take(); conn.handle = established.handle; conn.jump_handles = established.jump_handles; conn.config = config; conn.effective_config = established.effective_config; conn.alive = established.alive; + conn.proxy_child = established.proxy_child; if let Some(si) = server_info.as_ref() { conn.server_info = Some(si.clone()); } conn.sftp_session = Arc::new(SftpCache::new()); conn.bounded_sftp_session = Arc::new(BoundedSftpCache::new()); - (stale_handle, stale_jump_handles) + (stale_handle, stale_jump_handles, stale_proxy_child) } else { let replaced = guard.insert( connection_id.to_string(), @@ -4585,12 +4737,16 @@ impl SSHConnectionManager { sftp_session: Arc::new(SftpCache::new()), bounded_sftp_session: Arc::new(BoundedSftpCache::new()), server_key: None, + proxy_child: established.proxy_child, alive: established.alive, }, ); match replaced { - Some(previous) => (previous.handle, previous.jump_handles), - None => (None, Vec::new()), + Some(previous) => { + let replaced_child = previous.proxy_child; + (previous.handle, previous.jump_handles, replaced_child) + } + None => (None, Vec::new(), None), } } }; @@ -4612,6 +4768,26 @@ impl SSHConnectionManager { ); Self::shutdown_handle_chain(stale_chain).await; + // The stale ProxyCommand process is the old transport's other half: + // close it only once that transport was actually unused, and keep it + // alive otherwise — the same lease the skipped goodbye grants the old + // handle while in-flight work drains. + if let Some(child) = stale_proxy_child { + if stale_handle + .as_ref() + .map_or(true, |handle| Arc::strong_count(handle) == 1) + { + drop(child); + } else { + log::info!( + "SSH session {} reconnect keeps the previous ProxyCommand process alive \ + while its old transport still serves in-flight work", + connection_id + ); + std::mem::forget(child); + } + } + log::info!("SSH session {} reconnected successfully", connection_id); Ok(()) } @@ -7003,6 +7179,7 @@ mod tests { }, default_workspace: None, proxy_jump: None, + proxy_command: None, container: None, wsl: None, options: Default::default(), @@ -7503,6 +7680,7 @@ mod tests { }, default_workspace: None, proxy_jump: None, + proxy_command: None, container: Some(ContainerWorkspaceConfig { name: "fixture".into(), access: ContainerAccess::DockerExec, @@ -7526,6 +7704,7 @@ mod tests { sftp_session: Arc::new(SftpCache::new()), bounded_sftp_session: Arc::new(BoundedSftpCache::new()), server_key: None, + proxy_child: None, alive: Arc::new(AtomicBool::new(true)), }, ); @@ -7751,6 +7930,7 @@ mod tests { }, default_workspace: None, proxy_jump: None, + proxy_command: None, container: Some(ContainerWorkspaceConfig { name: "fixture".into(), access: ContainerAccess::DockerExec, @@ -7775,6 +7955,7 @@ mod tests { sftp_session: Arc::new(SftpCache::new()), bounded_sftp_session: Arc::new(BoundedSftpCache::new()), server_key: None, + proxy_child: None, alive: Arc::new(AtomicBool::new(true)), }, ); @@ -8231,7 +8412,7 @@ mod tests { #[test] #[cfg(feature = "ssh_config")] fn ssh_config_fallback_accepts_utf8_bom_and_defaults_to_agent() { - let content = strip_utf8_bom( + let content = crate::remote_ssh::ssh_config_loader::strip_utf8_bom( "\u{feff}Host 跳板\n HostName jump.example.com\n User 构建\n".to_string(), ); let entries = parse_ssh_config_manually(&content); @@ -8271,6 +8452,7 @@ mod tests { }, default_workspace: Some("/srv/project".to_string()), proxy_jump: None, + proxy_command: None, container: None, wsl: None, options: Default::default(), @@ -8292,6 +8474,7 @@ mod tests { sftp_session: Arc::new(SftpCache::new()), bounded_sftp_session: Arc::new(BoundedSftpCache::new()), server_key: None, + proxy_child: None, alive: alive.clone(), }, ); @@ -8339,6 +8522,106 @@ mod tests { ); } + #[test] + fn parses_proxy_command_as_a_whole_line_value() { + let hosts = parse_ssh_config_manually( + "Host coder-x\n ProxyCommand \"C:\\tools\\coder.exe\" ssh --stdio %h\n IdentityFile ~/.ssh/id_ed25519\n", + ); + assert_eq!(hosts.len(), 1); + // The full remainder of the line survives, inner quotes included. + assert_eq!( + hosts[0].proxy_command.as_deref(), + Some("\"C:\\tools\\coder.exe\" ssh --stdio %h") + ); + + let hosts = + parse_ssh_config_manually("Host quoted\n ProxyCommand \"coder ssh --stdio %h\"\n"); + assert_eq!( + hosts[0].proxy_command.as_deref(), + Some("coder ssh --stdio %h") + ); + + let hosts = parse_ssh_config_manually("Host plain\n HostName plain.example\n"); + assert_eq!(hosts[0].proxy_command, None); + } + + fn proxy_command_fixture_config(command: &str) -> SSHConnectionConfig { + SSHConnectionConfig { + id: "proxy-fixture".to_string(), + name: "proxy fixture".to_string(), + host: "coder-vscode.coder.sage.org.ai--abc".to_string(), + port: 22, + username: "coder".to_string(), + auth: SSHAuthMethod::PrivateKey { + key_path: "/nonexistent/key".to_string(), + passphrase: None, + certificate_path: None, + }, + default_workspace: None, + proxy_jump: None, + proxy_command: Some(command.to_string()), + container: None, + wsl: None, + options: Default::default(), + } + } + + #[tokio::test] + async fn proxy_command_transport_reports_proxy_exit_code() { + let temp = tempfile::tempdir().unwrap(); + let manager = SSHConnectionManager::new(temp.path().into()); + let config = proxy_command_fixture_config("exit 7"); + + let error = manager + .establish_session(&config, 10) + .await + .err() + .expect("a proxy that exits immediately cannot complete a handshake"); + let message = error.to_string(); + assert!( + message.contains("proxy command exited with code 7"), + "unexpected error: {message}" + ); + } + + #[tokio::test] + async fn proxy_command_rejects_jump_and_container_combinations() { + let temp = tempfile::tempdir().unwrap(); + let manager = SSHConnectionManager::new(temp.path().into()); + + let mut config = proxy_command_fixture_config("exit 0"); + config.proxy_jump = Some("jump.example".to_string()); + let error = manager + .establish_session(&config, 10) + .await + .err() + .expect("ProxyCommand plus ProxyJump is rejected in this release"); + assert!( + error.to_string().contains("ProxyCommand and ProxyJump"), + "unexpected error: {error}" + ); + + let mut config = proxy_command_fixture_config("exit 0"); + config.container = Some(ContainerWorkspaceConfig { + name: "dev".to_string(), + access: ContainerAccess::DockerExec, + local: false, + docker_path: "docker".to_string(), + shell: "/bin/sh".to_string(), + user: None, + interactive: false, + }); + let error = manager + .establish_session(&config, 10) + .await + .err() + .expect("ProxyCommand plus a container target is rejected in this release"); + assert!( + error.to_string().contains("Docker container"), + "unexpected error: {error}" + ); + } + #[test] fn parses_docker_published_ipv4_and_ipv6_endpoints() { assert_eq!( @@ -8829,6 +9112,7 @@ mod tests { }, default_workspace: Some("/tmp/openbitfun-remote-workspace".to_string()), proxy_jump: None, + proxy_command: None, container: Some(ContainerWorkspaceConfig { name: container_name, access: ContainerAccess::DockerExec, @@ -9086,6 +9370,7 @@ mod tests { }, default_workspace: None, proxy_jump: None, + proxy_command: None, container: None, wsl: None, options: Default::default(), @@ -9115,6 +9400,7 @@ mod tests { }, default_workspace: Some("/root/project".to_string()), proxy_jump: Some("jump-a,jump-b".to_string()), + proxy_command: None, container: None, wsl: None, options: Default::default(), @@ -9205,6 +9491,7 @@ mod tests { }, default_workspace: Some("/workspace".to_string()), proxy_jump: Some("jump.example.com".to_string()), + proxy_command: None, container: None, wsl: None, options: Default::default(), @@ -9250,6 +9537,7 @@ mod tests { }, default_workspace: None, proxy_jump: None, + proxy_command: None, container: None, wsl: None, options: Default::default(), diff --git a/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs b/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs index 8b4cec0dca..2d9e75dd89 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/manager/tests/workspace_sftp.rs @@ -369,6 +369,7 @@ impl Fixture { }, default_workspace: None, proxy_jump: None, + proxy_command: None, container: None, wsl: None, options: Default::default(), @@ -384,6 +385,7 @@ impl Fixture { sftp_session: Arc::new(SftpCache::new()), bounded_sftp_session: Arc::new(BoundedSftpCache::new()), server_key: None, + proxy_child: None, alive: Arc::new(AtomicBool::new(true)), }, ); diff --git a/src/crates/services/services-integrations/src/remote_ssh/mod.rs b/src/crates/services/services-integrations/src/remote_ssh/mod.rs index 8281c36ef6..b84030d1f2 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/mod.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/mod.rs @@ -31,6 +31,8 @@ mod password_vault; #[cfg(feature = "remote-ssh-concrete")] mod port_forward; #[cfg(feature = "remote-ssh-concrete")] +mod proxy_command; +#[cfg(feature = "remote-ssh-concrete")] #[cfg(feature = "remote-ssh-concrete")] mod release_verify; #[cfg(feature = "remote-ssh-concrete")] @@ -41,6 +43,8 @@ mod remote_exec_runtime_port; pub mod remote_fs; #[cfg(feature = "remote-ssh-concrete")] pub mod remote_terminal; +#[cfg(all(feature = "remote-ssh-concrete", feature = "ssh_config"))] +mod ssh_config_loader; pub use file_name_search::{search_remote_file_names, RemoteFileNameSearch}; pub use paths::*; diff --git a/src/crates/services/services-integrations/src/remote_ssh/proxy_command.rs b/src/crates/services/services-integrations/src/remote_ssh/proxy_command.rs new file mode 100644 index 0000000000..e54a990732 --- /dev/null +++ b/src/crates/services/services-integrations/src/remote_ssh/proxy_command.rs @@ -0,0 +1,303 @@ +//! `ProxyCommand` transport support. +//! +//! OpenSSH runs `ProxyCommand` as a local shell process and carries the SSH +//! protocol over its stdin/stdout. This module does the same for OpenBitFun's +//! russh client: the manager spawns the command here and hands the joined +//! stdout/stdin to `establish_stream_session`, which keeps handshake timeouts, +//! known-hosts checks, and authentication identical to the direct path. +//! +//! Platform notes: +//! - Windows runs the command through `cmd.exe /C` with the raw string (no +//! extra MS quoting), matching how OpenSSH's Windows build passes +//! `ProxyCommand` along; Unix uses `sh -c`. +//! - `%h`, `%p`, `%r`, and `%%` are expanded before the shell sees the string. +//! - stderr is drained continuously into the log plus a small tail buffer: +//! proxies like `coder ssh --stdio` log there, and a full pipe would block +//! the child mid-handshake. + +use std::collections::VecDeque; +use std::process::Stdio; +use std::sync::{Arc, Mutex}; + +use anyhow::{anyhow, Context}; +use openbitfun_services_core::process_manager; +use tokio::io::{join, AsyncBufReadExt, BufReader, Join}; +use tokio::process::{Child, ChildStderr, ChildStdin, ChildStdout}; + +/// How many recent stderr lines are kept for error enrichment. +const STDERR_TAIL_LINES: usize = 16; +/// Cap a single buffered stderr line so a chatty proxy cannot grow the tail +/// without bound. +const STDERR_LINE_MAX_CHARS: usize = 400; +/// Grace period for a proxy to exit on its own before it is killed, so its +/// real exit code survives as the diagnostic. +const EXIT_DRAIN_GRACE: std::time::Duration = std::time::Duration::from_secs(2); + +/// Owns a spawned proxy process for the lifetime of the SSH session carried +/// over its stdio. +/// +/// The child is killed when the guard is dropped, whether that happens because +/// the connection was closed, the handshake failed, or the session was torn +/// down by [`crate::remote_ssh::manager::SSHConnectionManager`]. +pub(crate) struct ProxyCommandChild { + child: Option, + stream: Option<(ChildStdout, ChildStdin)>, + stderr_tail: Arc>>, + drain: Option>, + label: String, +} + +impl ProxyCommandChild { + /// Spawn `command` through the platform shell with piped stdio. + /// + /// `label` identifies the connection in logs; the command text is kept out + /// of error messages because ProxyCommand lines sometimes embed tokens. + pub(crate) fn spawn(command: &str, label: &str) -> anyhow::Result { + let mut cmd = process_manager::create_tokio_command(proxy_shell_program()); + configure_shell_args(&mut cmd, command); + cmd.stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + // Safety net for any path that drops the guard without an explicit + // `finish`; the guard's own Drop reaps as well. + .kill_on_drop(true); + let mut child = cmd + .spawn() + .with_context(|| format!("Failed to start proxy command for {label}"))?; + let stdout = child + .stdout + .take() + .ok_or_else(|| anyhow!("Proxy command for {label} has no stdout pipe"))?; + let stdin = child + .stdin + .take() + .ok_or_else(|| anyhow!("Proxy command for {label} has no stdin pipe"))?; + let stderr = child + .stderr + .take() + .ok_or_else(|| anyhow!("Proxy command for {label} has no stderr pipe"))?; + + let stderr_tail = Arc::new(Mutex::new(VecDeque::new())); + let drain = spawn_stderr_drain(stderr, stderr_tail.clone(), label.to_string()); + Ok(Self { + child: Some(child), + stream: Some((stdout, stdin)), + stderr_tail, + drain: Some(drain), + label: label.to_string(), + }) + } + + /// Take the joined stdout/stdin stream for the SSH handshake. + pub(crate) fn take_stream(&mut self) -> anyhow::Result> { + let (stdout, stdin) = self + .stream + .take() + .ok_or_else(|| anyhow!("Proxy command stream for {} was already taken", self.label))?; + Ok(join(stdout, stdin)) + } + + /// Kill the process (unless it already exited), reap it, and report the + /// exit code plus the buffered stderr tail for error messages. + pub(crate) async fn finish(mut self) -> (Option, String) { + let mut exit_code = None; + if let Some(child) = self.child.as_mut() { + // Give a fast-exiting proxy a moment to report its own exit code — + // that code is the actual diagnostic. Only a proxy that is still + // alive (wedged on a black hole, say) gets killed, and a killed + // proxy reports no code. + match tokio::time::timeout(EXIT_DRAIN_GRACE, child.wait()).await { + Ok(Ok(status)) => exit_code = status.code(), + _ => { + let _ = child.start_kill(); + if let Ok(status) = child.wait().await { + exit_code = status.code(); + } + } + } + } + // The drain task ends on stderr EOF, which follows process exit; wait + // for it so the tail reflects every line the proxy produced. + if let Some(drain) = self.drain.take() { + let _ = tokio::time::timeout(EXIT_DRAIN_GRACE, drain).await; + } + (exit_code, self.stderr_tail_text()) + } + + fn stderr_tail_text(&self) -> String { + match self.stderr_tail.lock() { + Ok(tail) => tail + .iter() + .map(String::as_str) + .collect::>() + .join("\n"), + Err(_) => String::new(), + } + } +} + +impl Drop for ProxyCommandChild { + fn drop(&mut self) { + let Some(mut child) = self.child.take() else { + return; + }; + let _ = child.start_kill(); + // Reap asynchronously so a killed child does not linger as a zombie on + // Unix. Without an active runtime there is nothing to await: the kill + // already happened, and `kill_on_drop` remains armed as a backstop. + if let Ok(runtime) = tokio::runtime::Handle::try_current() { + runtime.spawn(async move { + let _ = child.wait().await; + }); + } + } +} + +/// Expand OpenSSH ProxyCommand tokens: `%h` (target host), `%p` (port), `%r` +/// (user), and `%%` (literal `%`). Unknown escapes stay literal, matching the +/// conservative end of OpenSSH's behavior for this release. +pub(crate) fn expand_tokens(command: &str, host: &str, port: u16, user: &str) -> String { + let mut expanded = String::with_capacity(command.len()); + let mut chars = command.chars(); + while let Some(c) = chars.next() { + if c != '%' { + expanded.push(c); + continue; + } + match chars.next() { + Some('h') => expanded.push_str(host), + Some('p') => expanded.push_str(&port.to_string()), + Some('r') => expanded.push_str(user), + Some('%') => expanded.push('%'), + Some(other) => { + expanded.push('%'); + expanded.push(other); + } + None => expanded.push('%'), + } + } + expanded +} + +fn proxy_shell_program() -> &'static str { + #[cfg(windows)] + { + "cmd.exe" + } + #[cfg(not(windows))] + { + "sh" + } +} + +fn configure_shell_args(cmd: &mut tokio::process::Command, command: &str) { + #[cfg(windows)] + { + // cmd.exe parses everything after /C itself, so the command must reach + // it raw: the standard Windows argument quoting would wrap it in extra + // quotes and mangle the quoting conventions ProxyCommand authors + // already rely on. + cmd.raw_arg("/C"); + cmd.raw_arg(command); + } + #[cfg(not(windows))] + { + cmd.arg("-c").arg(command); + } +} + +/// Drain the proxy's stderr forever: one line at a time into the log, with the +/// most recent lines kept in a bounded tail for connection error messages. +/// Returns the task handle; the task ends when stderr hits EOF (process exit). +fn spawn_stderr_drain( + stderr: ChildStderr, + tail: Arc>>, + label: String, +) -> tokio::task::JoinHandle<()> { + tokio::spawn(async move { + let mut reader = BufReader::new(stderr); + let mut line = String::new(); + loop { + line.clear(); + match reader.read_line(&mut line).await { + Ok(0) | Err(_) => break, + Ok(_) => { + let trimmed = line.trim_end(); + if !trimmed.is_empty() { + log::debug!("proxy command {label}: {trimmed}"); + } + if let Ok(mut tail) = tail.lock() { + if tail.len() == STDERR_TAIL_LINES { + tail.pop_front(); + } + let mut owned = trimmed.to_string(); + if owned.chars().count() > STDERR_LINE_MAX_CHARS { + owned = owned.chars().take(STDERR_LINE_MAX_CHARS).collect(); + owned.push_str(" [truncated]"); + } + tail.push_back(owned); + } + } + } + } + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn expands_tokens_and_keeps_unknown_escapes_literal() { + assert_eq!( + expand_tokens("coder ssh --stdio %h", "coder-x", 22, "alice"), + "coder ssh --stdio coder-x" + ); + assert_eq!( + expand_tokens("connect %r@%h:%p", "target", 2222, "ops"), + "connect ops@target:2222" + ); + assert_eq!(expand_tokens("100%% sure", "h", 22, "u"), "100% sure"); + // Unrecognized escapes survive untouched. + assert_eq!(expand_tokens("tool %j %n", "h", 22, "u"), "tool %j %n"); + assert_eq!(expand_tokens("trailing %", "h", 22, "u"), "trailing %"); + } + + #[cfg(windows)] + #[tokio::test] + async fn finish_reports_exit_code_and_stderr_tail() { + let mut child = + ProxyCommandChild::spawn("echo proxy-err-line 1>&2 & exit 7", "test-connection") + .unwrap(); + let _stream = child.take_stream().unwrap(); + let (exit_code, tail) = child.finish().await; + assert_eq!(exit_code, Some(7)); + assert!(tail.contains("proxy-err-line"), "stderr tail was: {tail}"); + } + + #[cfg(not(windows))] + #[tokio::test] + async fn finish_reports_exit_code_and_stderr_tail() { + let mut child = + ProxyCommandChild::spawn("echo proxy-err-line >&2; exit 7", "test-connection").unwrap(); + let _stream = child.take_stream().unwrap(); + let (exit_code, tail) = child.finish().await; + assert_eq!(exit_code, Some(7)); + assert!(tail.contains("proxy-err-line"), "stderr tail was: {tail}"); + } + + #[tokio::test] + async fn drop_kills_a_still_running_proxy() { + let script = if cfg!(windows) { + "ping -n 30 127.0.0.1 >nul" + } else { + "sleep 30" + }; + let guard = ProxyCommandChild::spawn(script, "long-running").unwrap(); + drop(guard); + // `finish` was never called; the child must not outlive the guard. A + // leaked process would show up as a dangling `ping`/`sleep` beyond this + // test's lifetime, so no explicit assertion is needed beyond the test + // finishing while the child is still running. + } +} diff --git a/src/crates/services/services-integrations/src/remote_ssh/ssh_config_loader.rs b/src/crates/services/services-integrations/src/remote_ssh/ssh_config_loader.rs new file mode 100644 index 0000000000..9fe2e8cd7f --- /dev/null +++ b/src/crates/services/services-integrations/src/remote_ssh/ssh_config_loader.rs @@ -0,0 +1,348 @@ +//! Merged `~/.ssh/config` loading with `Include` expansion. +//! +//! The `ssh_config` crate (0.1) parses a single file and has no `Include` +//! support, but ecosystem tooling (Coder's `config-ssh`, AtomGit, devenv) writes +//! Host blocks into separate files pulled in through `Include`. Hosts that only +//! exist behind an `Include` were therefore invisible to both the host list and +//! `get_ssh_config`. This loader merges the main file with its include tree +//! in place, preserving OpenSSH's order semantics (first occurrence of a +//! setting wins). + +use std::collections::HashSet; +use std::path::{Path, PathBuf}; + +/// OpenSSH processes nested `Include` directives recursively. The cap only +/// exists to turn accidental self-reference into a bounded warning instead of a +/// hang; real-world configs stay a handful of levels deep. +const MAX_INCLUDE_DEPTH: usize = 8; + +/// Merge `config_path` with the files pulled in by its `Include` directives. +/// +/// Returns `None` when the main file is missing or unreadable, mirroring the +/// previous single-file behavior. Individual includes that fail to expand are +/// skipped with a warning so one broken include cannot hide every other host. +pub(crate) fn load_merged_config(config_path: &Path) -> Option { + let mut visited = HashSet::new(); + let mut merged = String::new(); + expand_file( + config_path, + 0, + config_path.parent(), + &mut visited, + &mut merged, + ); + if merged.trim().is_empty() { + None + } else { + Some(merged) + } +} + +/// Strip a leading UTF-8 BOM if present. Windows editors (and some SSH tooling) +/// still write one. +pub(crate) fn strip_utf8_bom(content: String) -> String { + content + .strip_prefix('\u{feff}') + .unwrap_or(&content) + .to_string() +} + +fn expand_file( + path: &Path, + depth: usize, + base_dir: Option<&Path>, + visited: &mut HashSet, + merged: &mut String, +) { + if depth > MAX_INCLUDE_DEPTH { + log::warn!( + "SSH config include chain deeper than {} levels at {}; skipping the rest", + MAX_INCLUDE_DEPTH, + path.display() + ); + return; + } + // Deduplicate on the canonical path so symlink or `..` cycles, plus plain + // repeated includes, cannot loop forever. Skipping a repeat is harmless: + // first-occurrence-wins parsing keeps the first copy authoritative anyway. + let canonical = match path.canonicalize() { + Ok(canonical) => canonical, + Err(error) => { + if depth == 0 { + log::debug!("SSH config not readable at {}: {}", path.display(), error); + } else { + log::warn!( + "SSH config include not readable at {}: {}", + path.display(), + error + ); + } + return; + } + }; + if !visited.insert(canonical) { + return; + } + let content = match std::fs::read_to_string(path) { + Ok(content) => strip_utf8_bom(content), + Err(error) => { + log::warn!( + "SSH config include not readable at {}: {}", + path.display(), + error + ); + return; + } + }; + + for line in content.lines() { + let trimmed = line.trim(); + if trimmed.is_empty() || trimmed.starts_with('#') { + merged.push_str(line); + merged.push('\n'); + continue; + } + match include_value(trimmed) { + Some(value) => { + for target in resolve_include_targets(value, base_dir) { + expand_file(&target, depth + 1, base_dir, visited, merged); + } + } + None => { + merged.push_str(line); + merged.push('\n'); + } + } + } +} + +/// Extract the value part of an `Include` directive, or `None` for any other +/// line. Accepts both `Include value` and `Include=value`; ssh_config keywords +/// are case-insensitive. +fn include_value(line: &str) -> Option<&str> { + let (keyword, value) = match line.split_once(char::is_whitespace) { + Some((keyword, value)) => (keyword, value), + None => line.split_once('=')?, + }; + if keyword.eq_ignore_ascii_case("include") { + Some(value.trim()) + } else { + None + } +} + +/// Resolve one `Include` value into concrete file paths, in order. +/// +/// Tokens may be double-quoted, may start with `~`, and may contain glob +/// patterns. Relative paths resolve against the user config directory +/// (`~/.ssh`), matching OpenSSH. Glob matches are yielded in sorted order by +/// the `glob` crate, which keeps merged output deterministic. +fn resolve_include_targets(value: &str, base_dir: Option<&Path>) -> Vec { + let mut targets = Vec::new(); + for token in split_quoted_tokens(value) { + let expanded = shellexpand::tilde(&token).into_owned(); + let candidate = PathBuf::from(&expanded); + let absolute = match base_dir { + Some(base_dir) if candidate.is_relative() => base_dir.join(candidate), + _ => candidate, + }; + // The glob crate wants forward slashes even on Windows. + let pattern = absolute.to_string_lossy().replace('\\', "/"); + if !pattern.contains(['*', '?', '[']) { + targets.push(absolute); + continue; + } + match glob::glob(&pattern) { + Ok(paths) => targets.extend(paths.filter_map(Result::ok)), + Err(error) => log::warn!( + "SSH config include pattern '{}' is invalid: {}", + token, + error + ), + } + } + targets +} + +/// Split on whitespace, keeping double-quoted spans (quote characters +/// themselves are dropped) as single tokens. +fn split_quoted_tokens(value: &str) -> Vec { + let mut tokens = Vec::new(); + let mut current = String::new(); + let mut in_quotes = false; + let mut has_token = false; + for ch in value.chars() { + match ch { + '"' => { + in_quotes = !in_quotes; + has_token = true; + } + c if c.is_whitespace() && !in_quotes => { + if has_token { + tokens.push(std::mem::take(&mut current)); + has_token = false; + } + } + c => { + current.push(c); + has_token = true; + } + } + } + if has_token { + tokens.push(current); + } + tokens +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn unique_temp_dir(label: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "openbitfun-ssh-config-loader-{label}-{}-{}", + std::process::id(), + chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default() + )); + fs::create_dir_all(&dir).unwrap(); + dir + } + + fn write(path: &Path, content: &str) { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).unwrap(); + } + fs::write(path, content).unwrap(); + } + + #[test] + fn merges_included_files_at_the_include_position() { + let dir = unique_temp_dir("position"); + let ssh_dir = dir.join(".ssh"); + write( + &ssh_dir.join("config"), + "Host main-only\n HostName main.example\n\nInclude coder.conf\n\nHost after-include\n HostName tail.example\n", + ); + write( + &ssh_dir.join("coder.conf"), + "Host coder-workspace\n ProxyCommand coder ssh --stdio %h\n", + ); + + let merged = load_merged_config(&ssh_dir.join("config")).unwrap(); + let main_pos = merged.find("Host main-only").unwrap(); + let coder_pos = merged.find("Host coder-workspace").unwrap(); + let tail_pos = merged.find("Host after-include").unwrap(); + assert!(main_pos < coder_pos && coder_pos < tail_pos); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn resolves_relative_includes_against_the_ssh_directory() { + let dir = unique_temp_dir("relative"); + let ssh_dir = dir.join(".ssh"); + write( + &ssh_dir.join("config"), + "Include extra/host.conf\nHost base\n HostName base.example\n", + ); + write( + &ssh_dir.join("extra").join("host.conf"), + "Host nested\n HostName nested.example\n", + ); + + let merged = load_merged_config(&ssh_dir.join("config")).unwrap(); + assert!(merged.contains("Host nested")); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn expands_globs_in_sorted_order() { + let dir = unique_temp_dir("glob"); + let ssh_dir = dir.join(".ssh"); + write(&ssh_dir.join("config"), "Include parts/*.conf\n"); + write(&ssh_dir.join("parts").join("b.conf"), "Host from-b\n"); + write(&ssh_dir.join("parts").join("a.conf"), "Host from-a\n"); + + let merged = load_merged_config(&ssh_dir.join("config")).unwrap(); + let a_pos = merged.find("Host from-a").unwrap(); + let b_pos = merged.find("Host from-b").unwrap(); + assert!(a_pos < b_pos, "glob matches must be merged in sorted order"); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn keeps_parsing_when_an_include_is_missing_or_loops() { + let dir = unique_temp_dir("cycle"); + let ssh_dir = dir.join(".ssh"); + write( + &ssh_dir.join("config"), + "Include missing.conf\nInclude cycle-a.conf\nHost base\n HostName base.example\n", + ); + write( + &ssh_dir.join("cycle-a.conf"), + "Include cycle-b.conf\nHost a\n", + ); + write( + &ssh_dir.join("cycle-b.conf"), + "Include cycle-a.conf\nHost b\n", + ); + + let merged = load_merged_config(&ssh_dir.join("config")).unwrap(); + assert!(merged.contains("Host a")); + assert!(merged.contains("Host b")); + assert_eq!(merged.matches("Host a").count(), 1); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn returns_none_when_the_main_config_is_missing() { + let dir = unique_temp_dir("missing-main"); + assert!(load_merged_config(&dir.join(".ssh").join("config")).is_none()); + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn strips_utf8_bom_from_the_main_config() { + let dir = unique_temp_dir("bom"); + let ssh_dir = dir.join(".ssh"); + write(&ssh_dir.join("config"), "\u{feff}Host bom-host\n"); + + let merged = load_merged_config(&ssh_dir.join("config")).unwrap(); + assert!(merged.starts_with("Host bom-host")); + + fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn include_value_accepts_keyword_forms_case_insensitively() { + assert_eq!(include_value("Include coder.conf"), Some("coder.conf")); + assert_eq!(include_value("include=parts/*.conf"), Some("parts/*.conf")); + assert_eq!( + include_value("INCLUDE \"path with spaces.conf\" "), + Some("\"path with spaces.conf\"") + ); + assert_eq!(include_value("HostName example"), None); + assert_eq!(include_value("Host=example"), None); + } + + #[test] + fn quoted_include_tokens_stay_whole() { + let dir = unique_temp_dir("quoted"); + let ssh_dir = dir.join(".ssh"); + write(&ssh_dir.join("my hosts.conf"), "Host quoted\n"); + write( + &ssh_dir.join("config"), + "Include \"my hosts.conf\"\nHost base\n", + ); + + let merged = load_merged_config(&ssh_dir.join("config")).unwrap(); + assert!(merged.contains("Host quoted")); + + fs::remove_dir_all(&dir).unwrap(); + } +} diff --git a/src/crates/services/services-integrations/src/remote_ssh/types.rs b/src/crates/services/services-integrations/src/remote_ssh/types.rs index db4a1b9c29..0a965352f5 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/types.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/types.rs @@ -52,6 +52,15 @@ pub struct SSHConnectionConfig { /// SSH config entry, so every hop may use a different user and identity. #[serde(default)] pub proxy_jump: Option, + /// OpenSSH-compatible `ProxyCommand`: a local shell command whose stdin/stdout + /// carries the SSH transport to the target (e.g. `coder ssh --stdio`). + /// + /// `%h` (target host), `%p` (port), `%r` (user), and `%%` are expanded before + /// the command runs. Mutually exclusive with `proxy_jump` and container + /// targets; the target host is typically a bare `~/.ssh/config` alias with no + /// `HostName`, because the proxy command resolves the real endpoint itself. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub proxy_command: Option, /// Optional Docker container that becomes the effective workspace target. #[serde(default)] pub container: Option, @@ -74,6 +83,7 @@ impl SSHConnectionConfig { && self.username == other.username && self.auth.connection_params_equal(&other.auth) && self.proxy_jump == other.proxy_jump + && self.proxy_command == other.proxy_command && self.container == other.container && self.wsl == other.wsl && self.options == other.options @@ -361,6 +371,8 @@ pub struct SavedConnection { pub last_connected: Option, #[serde(default)] pub proxy_jump: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub proxy_command: Option, #[serde(default)] pub container: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -608,6 +620,9 @@ pub struct SSHConfigEntry { /// OpenSSH ProxyJump chain, preserving aliases and order. #[serde(default)] pub proxy_jump: Option, + /// OpenSSH ProxyCommand resolved for this host, if any. + #[serde(default)] + pub proxy_command: Option, } /// Result of looking up SSH config for a host diff --git a/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_contracts.rs b/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_contracts.rs index afe5c7b17b..b9cf3083bf 100644 --- a/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_contracts.rs +++ b/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_contracts.rs @@ -85,6 +85,7 @@ fn remote_target_contract_uses_proxy_jump_and_kebab_case_container_access() { }, default_workspace: Some("/workspace".to_string()), proxy_jump: Some("jump1,jump2".to_string()), + proxy_command: None, container: Some(ContainerWorkspaceConfig { name: "trainer-dev".to_string(), access: ContainerAccess::DockerExec, @@ -109,6 +110,76 @@ fn remote_target_contract_uses_proxy_jump_and_kebab_case_container_access() { ); } +#[test] +fn remote_target_contract_carries_proxy_command_and_stays_back_compatible() { + // A payload that names a ProxyCommand must round-trip through the camelCase + // wire shape and mark the config as drift-changed when the command changes. + let config: SSHConnectionConfig = serde_json::from_value(serde_json::json!({ + "id": "conn-1", + "name": "coder", + "host": "coder-vscode.coder.sage.org.ai--abc", + "port": 22, + "username": "alice", + "auth": { "type": "PrivateKey", "keyPath": "~/.ssh/coder_deploy" }, + "defaultWorkspace": "/home/coder", + "proxyCommand": "\"coder\" --url https://coder.example:9443 ssh --stdio %h" + })) + .unwrap(); + assert_eq!( + config.proxy_command.as_deref(), + Some("\"coder\" --url https://coder.example:9443 ssh --stdio %h") + ); + + let json = serde_json::to_value(&config).unwrap(); + assert_eq!( + json["proxyCommand"], + "\"coder\" --url https://coder.example:9443 ssh --stdio %h" + ); + let round_trip: SSHConnectionConfig = serde_json::from_value(json).unwrap(); + assert_eq!( + round_trip.proxy_command.as_deref(), + config.proxy_command.as_deref() + ); + assert!(config.connection_params_equal(&round_trip)); + + // Older clients and persisted files predate the field: it must deserialize + // as absent, and two such configs must still compare equal. + let legacy: SSHConnectionConfig = serde_json::from_value(serde_json::json!({ + "id": "conn-1", + "name": "coder", + "host": "coder-vscode.coder.sage.org.ai--abc", + "port": 22, + "username": "alice", + "auth": { "type": "PrivateKey", "keyPath": "~/.ssh/coder_deploy" }, + "defaultWorkspace": "/home/coder" + })) + .unwrap(); + assert_eq!(legacy.proxy_command, None); + assert!(!config.connection_params_equal(&legacy)); +} + +#[test] +#[cfg(feature = "remote-persistence")] +fn saved_connection_record_tolerates_legacy_payloads_without_proxy_command() { + let saved: openbitfun_services_integrations::remote_persistence::SavedConnectionRecord = + serde_json::from_value(serde_json::json!({ + "id": "conn-1", + "name": "coder", + "host": "coder-vscode.coder.sage.org.ai--abc", + "port": 22, + "username": "alice", + "authType": { "type": "PrivateKey", "keyPath": "~/.ssh/coder_deploy" }, + "defaultWorkspace": "/home/coder", + "lastConnected": 1 + })) + .unwrap(); + assert_eq!(saved.proxy_command, None); + assert_eq!(saved.proxy_jump, None); + + let written = serde_json::to_value(&saved).unwrap(); + assert!(written.get("proxyCommand").is_none()); +} + #[test] fn remote_workspace_defaults_keep_older_files_loadable() { let workspace: RemoteWorkspace = serde_json::from_value(serde_json::json!({ diff --git a/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_disabled_contracts.rs b/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_disabled_contracts.rs index 80300b009f..e2d557bbb2 100644 --- a/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_disabled_contracts.rs +++ b/src/crates/services/services-integrations/tests/remote_ssh_contracts/remote_ssh_disabled_contracts.rs @@ -21,6 +21,7 @@ fn test_config() -> SSHConnectionConfig { }, default_workspace: Some("/repo".to_string()), proxy_jump: None, + proxy_command: None, container: None, wsl: None, options: Default::default(),