From 25d7cb4e0d0ff210bdea36e54fd7b34f0ddd80a8 Mon Sep 17 00:00:00 2001 From: Marco Mihai Condrache <52580954+marcocondrache@users.noreply.github.com> Date: Tue, 7 Oct 2025 00:49:06 +0200 Subject: [PATCH] askpass: Fix cli path when executed in a remote server (#39475) 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> --- Cargo.lock | 2 + crates/askpass/Cargo.toml | 4 ++ crates/askpass/src/askpass.rs | 39 +++++++++++++----- crates/util/src/paths.rs | 75 +++++++++++++++++++++++++++++------ crates/util/src/util.rs | 43 ++++++-------------- crates/zed/Cargo.toml | 1 + crates/zed/src/main.rs | 23 +++++++++++ 7 files changed, 133 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c290f08a7bff1e9827e3e2427c775f0e0d85f8bf..6c9842c6aa34d4bbf7c42e61bc61133c95f428de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -805,6 +805,7 @@ dependencies = [ "anyhow", "futures 0.3.31", "gpui", + "log", "net", "proto", "smol", @@ -20204,6 +20205,7 @@ dependencies = [ "agent_ui", "anyhow", "ashpd 0.11.0", + "askpass", "assets", "assistant_tools", "audio", diff --git a/crates/askpass/Cargo.toml b/crates/askpass/Cargo.toml index 297139545caeb1c23386cb9f5d8f97b84791dc10..5c7367b9dbf0fe72fb97b0904bd6570f93270299 100644 --- a/crates/askpass/Cargo.toml +++ b/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"] diff --git a/crates/askpass/src/askpass.rs b/crates/askpass/src/askpass.rs index df4ac19a93504cca4bb3d6234da33c3474ab5bba..4e08d64ea00d8601e469e1bb3f7648d1703d8259 100644 --- a/crates/askpass/src/askpass.rs +++ b/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 = 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); @@ -134,7 +147,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:?}"))?; @@ -249,12 +262,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", @@ -263,13 +281,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(), ) } diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index c2742036b8449d02ecd1b21f69fda46bdb9756eb..dab9a9de32f85103ccfb7fde402c10969be809de 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -1,3 +1,4 @@ +use anyhow::Context; use globset::{Glob, GlobSet, GlobSetBuilder}; use regex::Regex; use serde::{Deserialize, Serialize}; @@ -32,8 +33,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 where Self: From<&'a Path>, @@ -45,7 +57,6 @@ pub trait PathExt { } #[cfg(windows)] { - use anyhow::Context as _; use tendril::fmt::{Format, WTF8}; WTF8::validate(bytes) .then(|| { @@ -57,18 +68,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; -} -impl> PathExt for T { - /// Compacts a given file path by replacing the user's home directory - /// prefix with a tilde (`~`). + /// 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) /// - /// # Returns + /// Will provide back the extensions joined together such as tar.gz or stories.tsx + fn multiple_extensions(&self) -> Option; + + /// Try to make a shell-safe representation of the path. /// - /// * 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. + /// For Unix, the path is escaped to be safe for POSIX shells + fn try_shell_safe(&self) -> anyhow::Result; +} + +impl> PathExt for T { 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()) { @@ -85,7 +102,6 @@ impl> 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()?; @@ -98,8 +114,6 @@ impl> 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 { let mut new_path = PathBuf::new(); for component in self.as_ref().components() { @@ -117,6 +131,43 @@ impl> PathExt for T { Some(new_path) } + + fn multiple_extensions(&self) -> Option { + let path = self.as_ref(); + let file_name = path.file_name()?.to_str()?; + + let parts: Vec<&str> = file_name + .split('.') + // Skip the part with the file name extension + .skip(1) + .collect(); + + if parts.len() < 2 { + return None; + } + + Some(parts.join(".")) + } + + fn try_shell_safe(&self) -> anyhow::Result { + #[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 diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 732a07c982698d917dd80441c7766a769e31613b..e46fc2277a4f1a2a6996cc013b71f436645b8438 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -17,7 +17,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, @@ -287,27 +289,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 { - 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 { +pub fn get_zed_cli_path() -> Result { let zed_path = std::env::current_exe().context("Failed to determine current zed executable path.")?; let parent = zed_path @@ -328,7 +322,7 @@ pub fn get_shell_safe_zed_cli_path() -> Result { anyhow::bail!("unsupported platform for determining zed-cli path"); }; - let zed_cli_path = possible_locations + possible_locations .iter() .find_map(|p| { parent @@ -342,20 +336,7 @@ pub fn get_shell_safe_zed_cli_path() -> Result { "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)] diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 4651f8c2e2457c988295128e78d121d4d85005ae..5f55662f2945ac04818fd744791671e2a6a52a3a 100644 --- a/crates/zed/Cargo.toml +++ b/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 diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 29868be577bac136e97781a0082d77fa35c73e25..8c89f0c514c9ba86f07db299c101c6be163ceee7 100644 --- a/crates/zed/src/main.rs +++ b/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); @@ -1265,6 +1281,13 @@ struct Args { #[arg(hide = true)] dock_action: Option, + /// 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, + #[arg(long, hide = true)] dump_all_actions: bool,