diff --git a/src/remote/attach.rs b/src/remote/attach.rs index 8e0950b99c..447cf82a33 100644 --- a/src/remote/attach.rs +++ b/src/remote/attach.rs @@ -627,6 +627,7 @@ impl RemoteSsh { fn base_command(&self) -> Command { let mut command = Command::new("ssh"); + remove_inherited_herdr_runtime_environment(&mut command); apply_managed_ssh_options(&mut command, self.options()); command } @@ -1947,7 +1948,7 @@ fn probe_remote_endpoint( remote_herdr.clone(), path.clone(), ssh.session_name.clone(), - None, + ssh.options(), true, )?; let mut stream = crate::ipc::connect_local_stream(&path)?; @@ -2827,22 +2828,7 @@ fn bridge_connection( bridge_stop: &Arc, ) -> io::Result<()> { let upload_stop = Arc::new(BridgeUploadStop::new()?); - let mut command = Command::new("ssh"); - apply_managed_ssh_options(&mut command, ssh_options); - if noninteractive { - apply_noninteractive_ssh_options(&mut command); - } - command - .arg("-T") - .arg(target) - .arg(remote_command) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(if noninteractive { - Stdio::piped() - } else { - Stdio::inherit() - }); + let mut command = bridge_ssh_command(target, remote_command, ssh_options, noninteractive); let mut child = command .spawn() @@ -2991,6 +2977,44 @@ fn bridge_connection( } } +// A command launched in a Herdr pane inherits these variables. SSH +// configurations can forward them with SendEnv, which makes the remote +// `remote-client-bridge` process look like a nested Herdr. Every SSH command +// which launches Herdr remotely must establish an independent runtime. +fn remove_inherited_herdr_runtime_environment(command: &mut Command) { + command + .env_remove(crate::HERDR_ENV_VAR) + .env_remove(crate::session::SESSION_ENV_VAR) + .env_remove(crate::api::SOCKET_PATH_ENV_VAR) + .env_remove(crate::server::socket_paths::CLIENT_SOCKET_PATH_ENV_VAR); +} + +fn bridge_ssh_command( + target: &str, + remote_command: &str, + ssh_options: Option<&ManagedSshOptions>, + noninteractive: bool, +) -> Command { + let mut command = Command::new("ssh"); + remove_inherited_herdr_runtime_environment(&mut command); + apply_managed_ssh_options(&mut command, ssh_options); + if noninteractive { + apply_noninteractive_ssh_options(&mut command); + } + command + .arg("-T") + .arg(target) + .arg(remote_command) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(if noninteractive { + Stdio::piped() + } else { + Stdio::inherit() + }); + command +} + fn ssh_bridge_exit_error(status: std::process::ExitStatus, stderr: &[u8]) -> io::Error { let stderr = String::from_utf8_lossy(stderr); let stderr = stderr.trim(); @@ -3142,8 +3166,32 @@ fn run_client_process( reattach_command: &str, keybindings: RemoteKeybindings, ) -> io::Result<()> { - let exe = std::env::current_exe()?; - let status = Command::new(exe) + let status = client_process_command( + std::env::current_exe()?, + local_socket, + reattach_command, + keybindings, + ) + .status()?; + + if status.success() { + Ok(()) + } else { + Err(io::Error::new( + io::ErrorKind::Interrupted, + format!("remote client exited with {status}"), + )) + } +} + +fn client_process_command( + exe: PathBuf, + local_socket: &Path, + reattach_command: &str, + keybindings: RemoteKeybindings, +) -> Command { + let mut command = Command::new(exe); + command .arg("client") .env( crate::server::socket_paths::CLIENT_SOCKET_PATH_ENV_VAR, @@ -3151,20 +3199,12 @@ fn run_client_process( ) .env(REATTACH_COMMAND_ENV_VAR, reattach_command) .env(REMOTE_KEYBINDINGS_ENV_VAR, keybindings.as_str()) + .env_remove(crate::HERDR_ENV_VAR) .env_remove(crate::api::SOCKET_PATH_ENV_VAR) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .status()?; - - if status.success() { - Ok(()) - } else { - Err(io::Error::new( - io::ErrorKind::Interrupted, - format!("remote client exited with {status}"), - )) - } + .stderr(Stdio::inherit()); + command } fn local_forward_socket_path(target: &str, session_name: &str) -> PathBuf { @@ -3661,6 +3701,32 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn endpoint_probe_bridge_reuses_interactive_ssh_control_socket() { + let mut managed_config = write_managed_ssh_config().expect("write managed config"); + let control_path = PathBuf::from("/tmp/herdr-password-auth/control"); + managed_config.options.control_path = Some(control_path.clone()); + let ssh = RemoteSsh { + target: "example".to_string(), + session_name: crate::session::DEFAULT_SESSION_NAME.into(), + managed_config: Some(managed_config), + noninteractive: false, + }; + + let command = bridge_ssh_command(ssh.target(), "remote-client-bridge", ssh.options(), true); + let args = command + .get_args() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect::>(); + assert!(args + .windows(2) + .any(|args| args[0] == "-S" && args[1] == control_path.to_string_lossy())); + assert!(args + .windows(2) + .any(|args| args[0] == "-o" && args[1] == "BatchMode=yes")); + } + #[cfg(windows)] #[test] fn windows_managed_ssh_config_uses_keepalives_without_control_socket() { @@ -3871,6 +3937,62 @@ mod tests { assert_eq!(ssh.scp_command().get_args().collect::>(), vec!["-C"]); } + #[test] + fn remote_ssh_command_removes_inherited_herdr_runtime_environment() { + let ssh = RemoteSsh { + target: "example".to_string(), + session_name: crate::session::DEFAULT_SESSION_NAME.into(), + managed_config: None, + noninteractive: false, + }; + + let command = ssh.command(); + assert_command_removes_inherited_herdr_runtime_environment(&command); + } + + #[test] + fn bridge_ssh_command_removes_inherited_herdr_runtime_environment() { + let command = bridge_ssh_command("example", "remote-client-bridge", None, false); + assert_command_removes_inherited_herdr_runtime_environment(&command); + } + + #[test] + fn remote_client_command_removes_inherited_nested_marker() { + let local_socket = PathBuf::from("/tmp/herdr-remote.sock"); + let command = client_process_command( + PathBuf::from("herdr"), + &local_socket, + "herdr --remote example", + RemoteKeybindings::Local, + ); + let environments = command.get_envs().collect::>(); + assert!(environments + .iter() + .any(|(name, value)| { *name == crate::HERDR_ENV_VAR && value.is_none() })); + assert!(environments.iter().any(|(name, value)| { + *name == crate::server::socket_paths::CLIENT_SOCKET_PATH_ENV_VAR && value.is_some() + })); + } + + fn assert_command_removes_inherited_herdr_runtime_environment(command: &Command) { + let removed = command + .get_envs() + .filter(|(_, value)| value.is_none()) + .map(|(name, _)| name.to_string_lossy()) + .collect::>(); + + assert!(removed.iter().any(|name| name == crate::HERDR_ENV_VAR)); + assert!(removed + .iter() + .any(|name| name == crate::session::SESSION_ENV_VAR)); + assert!(removed + .iter() + .any(|name| name == crate::api::SOCKET_PATH_ENV_VAR)); + assert!(removed + .iter() + .any(|name| { name == crate::server::socket_paths::CLIENT_SOCKET_PATH_ENV_VAR })); + } + #[test] fn remote_install_stream_command_avoids_shell_c_wrapper() { let command = remote_install_stream_command("/home/a b/.local/bin/herdr.tmp.123"); diff --git a/src/server/client_transport.rs b/src/server/client_transport.rs index 0d297fb95b..c1fb908cb8 100644 --- a/src/server/client_transport.rs +++ b/src/server/client_transport.rs @@ -1688,12 +1688,15 @@ mod tests { .set_send_timeout(Some(Duration::from_millis(100))) .unwrap(); server.set_nonblocking(true).unwrap(); + let (start_tx, start_rx) = std::sync::mpsc::sync_channel(0); let worker = std::thread::spawn(move || { + start_rx.recv().expect("client is ready to receive"); assert!(write_framed_bytes(&mut server, &vec![b'x'; 1024 * 1024])); }); client .set_recv_timeout(Some(Duration::from_secs(3))) .unwrap(); + start_tx.send(()).expect("start writer after reader setup"); let mut received = 0; let mut buffer = [0; 16 * 1024]; while received < 1024 * 1024 { diff --git a/src/server/handoff.rs b/src/server/handoff.rs index aaa87f382b..6ab00190d9 100644 --- a/src/server/handoff.rs +++ b/src/server/handoff.rs @@ -390,7 +390,7 @@ fn send_fd_batch(stream: &UnixStream, fds: &[RawFd]) -> io::Result<()> { if fds.is_empty() { return Ok(()); } - let byte = [b'F']; + let byte = *b"F"; let iov = [libc::iovec { iov_base: byte.as_ptr() as *mut libc::c_void, iov_len: byte.len(), diff --git a/src/terminal_theme.rs b/src/terminal_theme.rs index 2460731e2f..c85698d94f 100644 --- a/src/terminal_theme.rs +++ b/src/terminal_theme.rs @@ -135,12 +135,12 @@ pub fn osc_reset_default_color_sequence(kind: DefaultColorKind) -> &'static str fn parse_rgb_color(value: &str) -> Option { if let Some(rgb) = value.strip_prefix("rgb:") { let mut parts = rgb.split('/'); - return Some(RgbColor { + let color = RgbColor { r: parse_hex_component(parts.next()?)?, g: parse_hex_component(parts.next()?)?, b: parse_hex_component(parts.next()?)?, - }) - .filter(|_| parts.next().is_none()); + }; + return parts.next().is_none().then_some(color); } if let Some(hex) = value.strip_prefix('#') {