Fix issues with Windows SSH support (#47822)

John Tur created

- Remove the newlines in the unzip command string, which made it not
work.
- Fix spawning the terminal and tasks. Unfortunately, the Windows
OpenSSH server has limitations that require rather ugly workarounds.

Release Notes:

- N/A

Change summary

Cargo.lock                         |   1 
crates/remote/Cargo.toml           |   1 
crates/remote/src/transport.rs     |  27 ++--
crates/remote/src/transport/ssh.rs | 150 +++++++++++++++++++++++++++----
crates/util/src/shell.rs           |   9 -
5 files changed, 146 insertions(+), 42 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -13627,6 +13627,7 @@ dependencies = [
  "anyhow",
  "askpass",
  "async-trait",
+ "base64 0.22.1",
  "collections",
  "fs",
  "futures 0.3.31",

crates/remote/Cargo.toml 🔗

@@ -22,6 +22,7 @@ test-support = ["fs/test-support"]
 anyhow.workspace = true
 askpass.workspace = true
 async-trait.workspace = true
+base64.workspace = true
 collections.workspace = true
 fs.workspace = true
 futures.workspace = true

crates/remote/src/transport.rs 🔗

@@ -352,26 +352,25 @@ async fn build_remote_server_from_source(
 
         #[cfg(target_os = "windows")]
         {
-            // On Windows, we use 7z to compress the binary
-
-            let seven_zip = which("7z.exe",cx)
-                .await?
-                .context("7z.exe not found on $PATH, install it (e.g. with `winget install -e --id 7zip.7zip`) or, if you don't want this behaviour, set $env:ZED_BUILD_REMOTE_SERVER=\"nocompress\"")?;
-            let gz_path = format!("target/remote_server/{}/debug/remote_server.gz", triple);
-            if smol::fs::metadata(&gz_path).await.is_ok() {
-                smol::fs::remove_file(&gz_path).await?;
+            let zip_path = format!("target/remote_server/{}/debug/remote_server.zip", triple);
+            if smol::fs::metadata(&zip_path).await.is_ok() {
+                smol::fs::remove_file(&zip_path).await?;
             }
-            run_cmd(new_smol_command(seven_zip).args([
-                "a",
-                "-tgzip",
-                &gz_path,
-                &bin_path.to_string_lossy(),
+            let compress_command = format!(
+                "Compress-Archive -Path '{}' -DestinationPath '{}' -Force",
+                bin_path.to_string_lossy(),
+                zip_path
+            );
+            run_cmd(new_smol_command("powershell.exe").args([
+                "-NoProfile",
+                "-Command",
+                &compress_command,
             ]))
             .await?;
         }
 
         let mut archive_path = bin_path;
-        archive_path.set_extension("gz");
+        archive_path.set_extension("zip");
         std::env::current_dir()?.join(archive_path)
     } else {
         bin_path

crates/remote/src/transport/ssh.rs 🔗

@@ -302,19 +302,36 @@ impl RemoteConnection for SshRemoteConnection {
             ..
         } = self;
         let env = socket.envs.clone();
-        build_command(
-            input_program,
-            input_args,
-            input_env,
-            working_dir,
-            port_forward,
-            env,
-            *ssh_path_style,
-            ssh_shell,
-            *ssh_shell_kind,
-            socket.ssh_args(),
-            interactive,
-        )
+
+        if self.ssh_platform.os.is_windows() {
+            build_command_windows(
+                input_program,
+                input_args,
+                input_env,
+                working_dir,
+                port_forward,
+                env,
+                *ssh_path_style,
+                ssh_shell,
+                *ssh_shell_kind,
+                socket.ssh_args(),
+                interactive,
+            )
+        } else {
+            build_command_posix(
+                input_program,
+                input_args,
+                input_env,
+                working_dir,
+                port_forward,
+                env,
+                *ssh_path_style,
+                ssh_shell,
+                *ssh_shell_kind,
+                socket.ssh_args(),
+                interactive,
+            )
+        }
     }
 
     fn build_forward_ports_command(
@@ -950,10 +967,7 @@ impl SshRemoteConnection {
                 .try_quote(&tmp_exe_path)
                 .context("shell quoting")?;
             format!(
-                "Expand-Archive -Force -Path {orig_tmp_path} -DestinationPath {tmp_path} -ErrorAction Stop;
-                 Move-Item -Force {tmp_exe_path} {dst_path};
-                 Remove-Item -Force {tmp_path} -Recurse;
-                 Remove-Item -Force {orig_tmp_path}",
+                "Expand-Archive -Force -Path {orig_tmp_path} -DestinationPath {tmp_path} -ErrorAction Stop; Move-Item -Force {tmp_exe_path} {dst_path}; Remove-Item -Force {tmp_path} -Recurse; Remove-Item -Force {orig_tmp_path}",
             )
         } else {
             let orig_tmp_path = shell_kind
@@ -1564,7 +1578,7 @@ impl SshConnectionOptions {
     }
 }
 
-fn build_command(
+fn build_command_posix(
     input_program: Option<String>,
     input_args: &[String],
     input_env: &HashMap<String, String>,
@@ -1660,6 +1674,100 @@ fn build_command(
     })
 }
 
+fn build_command_windows(
+    input_program: Option<String>,
+    input_args: &[String],
+    _input_env: &HashMap<String, String>,
+    working_dir: Option<String>,
+    port_forward: Option<(u16, String, u16)>,
+    ssh_env: HashMap<String, String>,
+    ssh_path_style: PathStyle,
+    ssh_shell: &str,
+    _ssh_shell_kind: ShellKind,
+    ssh_args: Vec<String>,
+    interactive: Interactive,
+) -> Result<CommandTemplate> {
+    use base64::Engine as _;
+    use std::fmt::Write as _;
+
+    let mut exec = String::new();
+    let shell_kind = ShellKind::PowerShell;
+
+    if let Some(working_dir) = working_dir {
+        let working_dir = RemotePathBuf::new(working_dir, ssh_path_style).to_string();
+
+        write!(
+            exec,
+            "Set-Location -Path {} {} ",
+            shell_kind
+                .try_quote(&working_dir)
+                .context("shell quoting")?,
+            shell_kind.sequential_and_commands_separator()
+        )?;
+    }
+
+    // Windows OpenSSH has an 8K character limit for command lines. Sending a lot of environment variables easily puts us over the limit.
+    // Until we have a better solution for this, we just won't set environment variables for now.
+    // for (k, v) in input_env.iter() {
+    //     write!(
+    //         exec,
+    //         "$env:{}={} {} ",
+    //         k,
+    //         shell_kind.try_quote(v).context("shell quoting")?,
+    //         shell_kind.sequential_and_commands_separator()
+    //     )?;
+    // }
+
+    if let Some(input_program) = input_program {
+        write!(
+            exec,
+            "{}",
+            shell_kind
+                .try_quote_prefix_aware(&shell_kind.prepend_command_prefix(&input_program))
+                .context("shell quoting")?
+        )?;
+        for arg in input_args {
+            let arg = shell_kind.try_quote(arg).context("shell quoting")?;
+            write!(exec, " {}", &arg)?;
+        }
+    } else {
+        // Launch an interactive shell session
+        write!(exec, "{ssh_shell}")?;
+    };
+
+    let mut args = Vec::new();
+    args.extend(ssh_args);
+
+    if let Some((local_port, host, remote_port)) = port_forward {
+        args.push("-L".into());
+        args.push(format!("{local_port}:{host}:{remote_port}"));
+    }
+
+    // -q suppresses the "Connection to ... closed." message that SSH prints when
+    // the connection terminates with -t (pseudo-terminal allocation)
+    args.push("-q".into());
+    match interactive {
+        // -t forces pseudo-TTY allocation (for interactive use)
+        Interactive::Yes => args.push("-t".into()),
+        // -T disables pseudo-TTY allocation (for non-interactive piped stdio)
+        Interactive::No => args.push("-T".into()),
+    }
+
+    // Windows OpenSSH server incorrectly escapes the command string when the PTY is used.
+    // The simplest way to work around this is to use a base64 encoded command, which doesn't require escaping.
+    let utf16_bytes: Vec<u16> = exec.encode_utf16().collect();
+    let byte_slice: Vec<u8> = utf16_bytes.iter().flat_map(|&u| u.to_le_bytes()).collect();
+    let base64_encoded = base64::engine::general_purpose::STANDARD.encode(&byte_slice);
+
+    args.push(format!("powershell.exe -E {}", base64_encoded));
+
+    Ok(CommandTemplate {
+        program: "ssh".into(),
+        args,
+        env: ssh_env,
+    })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1672,7 +1780,7 @@ mod tests {
         env.insert("SSH_VAR".to_string(), "ssh-val".to_string());
 
         // Test non-interactive command (interactive=false should use -T)
-        let command = build_command(
+        let command = build_command_posix(
             Some("remote_program".to_string()),
             &["arg1".to_string(), "arg2".to_string()],
             &input_env,
@@ -1691,7 +1799,7 @@ mod tests {
         assert!(!command.args.iter().any(|arg| arg == "-t"));
 
         // Test interactive command (interactive=true should use -t)
-        let command = build_command(
+        let command = build_command_posix(
             Some("remote_program".to_string()),
             &["arg1".to_string(), "arg2".to_string()],
             &input_env,
@@ -1723,7 +1831,7 @@ mod tests {
         let mut env = HashMap::default();
         env.insert("SSH_VAR".to_string(), "ssh-val".to_string());
 
-        let command = build_command(
+        let command = build_command_posix(
             None,
             &[],
             &input_env,

crates/util/src/shell.rs 🔗

@@ -482,9 +482,8 @@ impl ShellKind {
             | ShellKind::Rc
             | ShellKind::Fish
             | ShellKind::Pwsh
-            | ShellKind::PowerShell
             | ShellKind::Xonsh => "&&",
-            ShellKind::Nushell | ShellKind::Elvish => ";",
+            ShellKind::PowerShell | ShellKind::Nushell | ShellKind::Elvish => ";",
         }
     }
 
@@ -643,11 +642,7 @@ impl ShellKind {
     pub fn quote_cmd(arg: &str) -> Cow<'_, str> {
         let crt_quoted = Self::quote_windows(arg, true);
 
-        let needs_cmd_escaping = crt_quoted.contains('"')
-            || crt_quoted.contains('%')
-            || crt_quoted
-                .chars()
-                .any(|c| matches!(c, '^' | '<' | '>' | '&' | '|' | '(' | ')'));
+        let needs_cmd_escaping = crt_quoted.contains(['"', '%', '^', '<', '>', '&', '|', '(', ')']);
 
         if !needs_cmd_escaping {
             return crt_quoted;