askpass: Fix cli path when executed in a remote server (#39475)

Marco Mihai Condrache created

Closes #39469
Closes #39438
Closes #39458

I'm not able to test it, i would appreciate if somebody could do it. I
think this bug was present also for SSH remote projects

Release Notes:

- Fixed an issue where zed bin was not found in remote servers for
askpass

---------

Signed-off-by: Marco Mihai Condrache <52580954+marcocondrache@users.noreply.github.com>

Change summary

Cargo.lock                    |  2 +
crates/askpass/Cargo.toml     |  4 ++
crates/askpass/src/askpass.rs | 39 ++++++++++++++++------
crates/util/src/paths.rs      | 61 +++++++++++++++++++++++++++---------
crates/util/src/util.rs       | 43 +++++++------------------
crates/zed/Cargo.toml         |  1 
crates/zed/src/main.rs        | 23 +++++++++++++
7 files changed, 115 insertions(+), 58 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -806,6 +806,7 @@ dependencies = [
  "anyhow",
  "futures 0.3.31",
  "gpui",
+ "log",
  "net",
  "proto",
  "smol",
@@ -19971,6 +19972,7 @@ dependencies = [
  "agent_ui",
  "anyhow",
  "ashpd 0.11.0",
+ "askpass",
  "assets",
  "assistant_tools",
  "audio",

crates/askpass/Cargo.toml 🔗

@@ -18,6 +18,7 @@ gpui.workspace = true
 net.workspace = true
 proto.workspace = true
 smol.workspace = true
+log.workspace = true
 tempfile.workspace = true
 util.workspace = true
 workspace-hack.workspace = true
@@ -25,3 +26,6 @@ zeroize.workspace = true
 
 [target.'cfg(target_os = "windows")'.dependencies]
 windows.workspace = true
+
+[package.metadata.cargo-machete]
+ignored = ["log"]

crates/askpass/src/askpass.rs 🔗

@@ -1,8 +1,8 @@
 mod encrypted_password;
 
 pub use encrypted_password::{EncryptedPassword, ProcessExt};
+use util::paths::PathExt;
 
-#[cfg(target_os = "windows")]
 use std::sync::OnceLock;
 use std::{ffi::OsStr, time::Duration};
 
@@ -14,10 +14,16 @@ use futures::{
 };
 use gpui::{AsyncApp, BackgroundExecutor, Task};
 use smol::fs;
-use util::ResultExt as _;
+use util::{ResultExt as _, debug_panic};
 
 use crate::encrypted_password::decrypt;
 
+/// Path to the program used for askpass
+///
+/// On Unix and remote servers, this defaults to the current executable
+/// On Windows, this is set to the CLI variant of zed
+static ASKPASS_PROGRAM: OnceLock<std::path::PathBuf> = OnceLock::new();
+
 #[derive(PartialEq, Eq)]
 pub enum AskPassResult {
     CancelledByUser,
@@ -85,8 +91,15 @@ impl AskPassSession {
         let askpass_script_path = temp_dir.path().join(ASKPASS_SCRIPT_NAME);
         let (askpass_opened_tx, askpass_opened_rx) = oneshot::channel::<()>();
         let listener = UnixListener::bind(&askpass_socket).context("creating askpass socket")?;
-        let zed_cli_path =
-            util::get_shell_safe_zed_cli_path().context("getting zed-cli path for askpass")?;
+
+        let current_exec =
+            std::env::current_exe().context("Failed to determine current zed executable path.")?;
+
+        let askpass_program = ASKPASS_PROGRAM
+            .get_or_init(|| current_exec)
+            .try_shell_safe()
+            .context("Failed to shell-escape Askpass program path.")?
+            .to_string();
 
         let (askpass_kill_master_tx, askpass_kill_master_rx) = oneshot::channel::<()>();
         let mut kill_tx = Some(askpass_kill_master_tx);
@@ -129,7 +142,7 @@ impl AskPassSession {
         });
 
         // Create an askpass script that communicates back to this process.
-        let askpass_script = generate_askpass_script(&zed_cli_path, &askpass_socket);
+        let askpass_script = generate_askpass_script(&askpass_program, &askpass_socket);
         fs::write(&askpass_script_path, askpass_script)
             .await
             .with_context(|| format!("creating askpass script at {askpass_script_path:?}"))?;
@@ -244,12 +257,17 @@ pub fn main(socket: &str) {
     }
 }
 
+pub fn set_askpass_program(path: std::path::PathBuf) {
+    if ASKPASS_PROGRAM.set(path).is_err() {
+        debug_panic!("askpass program has already been set");
+    }
+}
+
 #[inline]
 #[cfg(not(target_os = "windows"))]
-fn generate_askpass_script(zed_cli_path: &str, askpass_socket: &std::path::Path) -> String {
+fn generate_askpass_script(askpass_program: &str, askpass_socket: &std::path::Path) -> String {
     format!(
-        "{shebang}\n{print_args} | {zed_cli} --askpass={askpass_socket} 2> /dev/null \n",
-        zed_cli = zed_cli_path,
+        "{shebang}\n{print_args} | {askpass_program} --askpass={askpass_socket} 2> /dev/null \n",
         askpass_socket = askpass_socket.display(),
         print_args = "printf '%s\\0' \"$@\"",
         shebang = "#!/bin/sh",
@@ -258,13 +276,12 @@ fn generate_askpass_script(zed_cli_path: &str, askpass_socket: &std::path::Path)
 
 #[inline]
 #[cfg(target_os = "windows")]
-fn generate_askpass_script(zed_cli_path: &str, askpass_socket: &std::path::Path) -> String {
+fn generate_askpass_script(askpass_program: &str, askpass_socket: &std::path::Path) -> String {
     format!(
         r#"
         $ErrorActionPreference = 'Stop';
-        ($args -join [char]0) | & "{zed_cli}" --askpass={askpass_socket} 2> $null
+        ($args -join [char]0) | & "{askpass_program}" --askpass={askpass_socket} 2> $null
         "#,
-        zed_cli = zed_cli_path,
         askpass_socket = askpass_socket.display(),
     )
 }

crates/util/src/paths.rs 🔗

@@ -1,3 +1,4 @@
+use anyhow::Context;
 use globset::{Glob, GlobSet, GlobSetBuilder};
 use itertools::Itertools;
 use regex::Regex;
@@ -35,8 +36,19 @@ pub fn home_dir() -> &'static PathBuf {
 }
 
 pub trait PathExt {
+    /// Compacts a given file path by replacing the user's home directory
+    /// prefix with a tilde (`~`).
+    ///
+    /// # Returns
+    ///
+    /// * A `PathBuf` containing the compacted file path. If the input path
+    ///   does not have the user's home directory prefix, or if we are not on
+    ///   Linux or macOS, the original path is returned unchanged.
     fn compact(&self) -> PathBuf;
+
+    /// Returns a file's extension or, if the file is hidden, its name without the leading dot
     fn extension_or_hidden_file_name(&self) -> Option<&str>;
+
     fn try_from_bytes<'a>(bytes: &'a [u8]) -> anyhow::Result<Self>
     where
         Self: From<&'a Path>,
@@ -48,7 +60,6 @@ pub trait PathExt {
         }
         #[cfg(windows)]
         {
-            use anyhow::Context as _;
             use tendril::fmt::{Format, WTF8};
             WTF8::validate(bytes)
                 .then(|| {
@@ -60,19 +71,24 @@ pub trait PathExt {
                 .with_context(|| format!("Invalid WTF-8 sequence: {bytes:?}"))
         }
     }
+
+    /// Converts a local path to one that can be used inside of WSL.
+    /// Returns `None` if the path cannot be converted into a WSL one (network share).
     fn local_to_wsl(&self) -> Option<PathBuf>;
+
+    /// Returns a file's "full" joined collection of extensions, in the case where a file does not
+    /// just have a singular extension but instead has multiple (e.g File.tar.gz, Component.stories.tsx)
+    ///
+    /// Will provide back the extensions joined together such as tar.gz or stories.tsx
     fn multiple_extensions(&self) -> Option<String>;
+
+    /// Try to make a shell-safe representation of the path.
+    ///
+    /// For Unix, the path is escaped to be safe for POSIX shells
+    fn try_shell_safe(&self) -> anyhow::Result<String>;
 }
 
 impl<T: AsRef<Path>> PathExt for T {
-    /// Compacts a given file path by replacing the user's home directory
-    /// prefix with a tilde (`~`).
-    ///
-    /// # Returns
-    ///
-    /// * A `PathBuf` containing the compacted file path. If the input path
-    ///   does not have the user's home directory prefix, or if we are not on
-    ///   Linux or macOS, the original path is returned unchanged.
     fn compact(&self) -> PathBuf {
         if cfg!(any(target_os = "linux", target_os = "freebsd")) || cfg!(target_os = "macos") {
             match self.as_ref().strip_prefix(home_dir().as_path()) {
@@ -89,7 +105,6 @@ impl<T: AsRef<Path>> PathExt for T {
         }
     }
 
-    /// Returns a file's extension or, if the file is hidden, its name without the leading dot
     fn extension_or_hidden_file_name(&self) -> Option<&str> {
         let path = self.as_ref();
         let file_name = path.file_name()?.to_str()?;
@@ -102,8 +117,6 @@ impl<T: AsRef<Path>> PathExt for T {
             .or_else(|| path.file_stem()?.to_str())
     }
 
-    /// Converts a local path to one that can be used inside of WSL.
-    /// Returns `None` if the path cannot be converted into a WSL one (network share).
     fn local_to_wsl(&self) -> Option<PathBuf> {
         // quite sketchy to convert this back to path at the end, but a lot of functions only accept paths
         // todo: ideally rework them..?
@@ -133,10 +146,6 @@ impl<T: AsRef<Path>> PathExt for T {
         Some(new_path.into())
     }
 
-    /// Returns a file's "full" joined collection of extensions, in the case where a file does not
-    /// just have a singular extension but instead has multiple (e.g File.tar.gz, Component.stories.tsx)
-    ///
-    /// Will provide back the extensions joined together such as tar.gz or stories.tsx
     fn multiple_extensions(&self) -> Option<String> {
         let path = self.as_ref();
         let file_name = path.file_name()?.to_str()?;
@@ -153,6 +162,26 @@ impl<T: AsRef<Path>> PathExt for T {
 
         Some(parts.into_iter().join("."))
     }
+
+    fn try_shell_safe(&self) -> anyhow::Result<String> {
+        #[cfg(target_os = "windows")]
+        {
+            Ok(self.as_ref().to_string_lossy().to_string())
+        }
+
+        #[cfg(not(target_os = "windows"))]
+        {
+            let path_str = self
+                .as_ref()
+                .to_str()
+                .with_context(|| "Path contains invalid UTF-8")?;
+
+            // As of writing, this can only be fail if the path contains a null byte, which shouldn't be possible
+            // but shlex has annotated the error as #[non_exhaustive] so we can't make it a compile error if other
+            // errors are introduced in the future :(
+            Ok(shlex::try_quote(path_str)?.into_owned())
+        }
+    }
 }
 
 /// In memory, this is identical to `Path`. On non-Windows conversions to this type are no-ops. On

crates/util/src/util.rs 🔗

@@ -18,7 +18,9 @@ pub mod time;
 use anyhow::{Context as _, Result};
 use futures::Future;
 use itertools::Either;
+use paths::PathExt;
 use regex::Regex;
+use std::path::PathBuf;
 use std::sync::{LazyLock, OnceLock};
 use std::{
     borrow::Cow,
@@ -288,27 +290,19 @@ fn load_shell_from_passwd() -> Result<()> {
     Ok(())
 }
 
-#[cfg(unix)]
 /// Returns a shell escaped path for the current zed executable
 pub fn get_shell_safe_zed_path() -> anyhow::Result<String> {
-    let zed_path = std::env::current_exe()
-        .context("Failed to determine current zed executable path.")?
-        .to_string_lossy()
-        .trim_end_matches(" (deleted)") // see https://github.com/rust-lang/rust/issues/69343
-        .to_string();
-
-    // As of writing, this can only be fail if the path contains a null byte, which shouldn't be possible
-    // but shlex has annotated the error as #[non_exhaustive] so we can't make it a compile error if other
-    // errors are introduced in the future :(
-    let zed_path_escaped =
-        shlex::try_quote(&zed_path).context("Failed to shell-escape Zed executable path.")?;
-
-    Ok(zed_path_escaped.to_string())
+    let zed_path =
+        std::env::current_exe().context("Failed to determine current zed executable path.")?;
+
+    zed_path
+        .try_shell_safe()
+        .context("Failed to shell-escape Zed executable path.")
 }
 
-/// Returns a shell escaped path for the zed cli executable, this function
+/// Returns a path for the zed cli executable, this function
 /// should be called from the zed executable, not zed-cli.
-pub fn get_shell_safe_zed_cli_path() -> Result<String> {
+pub fn get_zed_cli_path() -> Result<PathBuf> {
     let zed_path =
         std::env::current_exe().context("Failed to determine current zed executable path.")?;
     let parent = zed_path
@@ -329,7 +323,7 @@ pub fn get_shell_safe_zed_cli_path() -> Result<String> {
         anyhow::bail!("unsupported platform for determining zed-cli path");
     };
 
-    let zed_cli_path = possible_locations
+    possible_locations
         .iter()
         .find_map(|p| {
             parent
@@ -343,20 +337,7 @@ pub fn get_shell_safe_zed_cli_path() -> Result<String> {
                 "could not find zed-cli from any of: {}",
                 possible_locations.join(", ")
             )
-        })?
-        .to_string_lossy()
-        .to_string();
-
-    #[cfg(target_os = "windows")]
-    {
-        Ok(zed_cli_path)
-    }
-    #[cfg(not(target_os = "windows"))]
-    {
-        Ok(shlex::try_quote(&zed_cli_path)
-            .context("Failed to shell-escape Zed executable path.")?
-            .to_string())
-    }
+        })
 }
 
 #[cfg(unix)]

crates/zed/Cargo.toml 🔗

@@ -25,6 +25,7 @@ agent.workspace = true
 agent_settings.workspace = true
 agent_ui.workspace = true
 anyhow.workspace = true
+askpass.workspace = true
 assets.workspace = true
 assistant_tools.workspace = true
 audio.workspace = true

crates/zed/src/main.rs 🔗

@@ -170,6 +170,13 @@ pub fn main() {
 
     let args = Args::parse();
 
+    // `zed --askpass` Makes zed operate in nc/netcat mode for use with askpass
+    #[cfg(not(target_os = "windows"))]
+    if let Some(socket) = &args.askpass {
+        askpass::main(socket);
+        return;
+    }
+
     // `zed --crash-handler` Makes zed operate in minidump crash handler mode
     if let Some(socket) = &args.crash_handler {
         crashes::crash_server(socket.as_path());
@@ -212,6 +219,15 @@ pub fn main() {
         }
     }
 
+    #[cfg(target_os = "windows")]
+    match util::get_zed_cli_path() {
+        Ok(path) => askpass::set_askpass_program(path),
+        Err(err) => {
+            eprintln!("Error: {}", err);
+            process::exit(1);
+        }
+    }
+
     let file_errors = init_paths();
     if !file_errors.is_empty() {
         files_not_created_on_launch(file_errors);
@@ -1266,6 +1282,13 @@ struct Args {
     #[arg(hide = true)]
     dock_action: Option<usize>,
 
+    /// Used for SSH/Git password authentication, to remove the need for netcat as a dependency,
+    /// by having Zed act like netcat communicating over a Unix socket.
+    #[arg(long)]
+    #[cfg(not(target_os = "windows"))]
+    #[arg(hide = true)]
+    askpass: Option<String>,
+
     #[arg(long, hide = true)]
     dump_all_actions: bool,