From 8f4646d6c357409cfc286104088b4d21f2bea851 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Thu, 23 Oct 2025 06:44:42 +0200 Subject: [PATCH] Use ShellKind::try_quote whenever we need to quote shell args (#40912) Using `shlex` unconditionally is dangerous as it assumes the underlying shell is POSIX which is not the case for PowerShell, CMD, or Nushell. Therefore, whenever we want to quote the args we should utilise our helper `util::shell::ShellKind::try_quote` which takes into account which shell is being used to actually exec/spawn the invocation. Release Notes: - N/A --------- Co-authored-by: Lukas Wirth --- Cargo.lock | 4 - crates/askpass/src/askpass.rs | 10 +- crates/dap_adapters/Cargo.toml | 1 - crates/dap_adapters/src/javascript.rs | 4 +- crates/debugger_ui/Cargo.toml | 1 - crates/debugger_ui/src/new_process_modal.rs | 14 +- crates/project/Cargo.toml | 1 - crates/project/src/environment.rs | 4 +- .../project/src/lsp_store/lsp_ext_command.rs | 3 +- crates/project/src/terminals.rs | 25 ++- crates/remote/Cargo.toml | 1 - crates/remote/src/transport/ssh.rs | 87 ++++---- crates/remote/src/transport/wsl.rs | 24 ++- crates/remote_server/src/headless_project.rs | 2 +- crates/task/src/task.rs | 103 +++------ crates/terminal/src/terminal.rs | 14 +- crates/terminal/src/terminal_settings.rs | 2 +- crates/util/src/paths.rs | 34 ++- crates/util/src/shell.rs | 201 ++++++++++++++---- crates/{task => util}/src/shell_builder.rs | 7 +- crates/util/src/shell_env.rs | 2 +- crates/util/src/util.rs | 5 +- 22 files changed, 317 insertions(+), 232 deletions(-) rename crates/{task => util}/src/shell_builder.rs (98%) diff --git a/Cargo.lock b/Cargo.lock index 0cc10bde43..48db1977ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4531,7 +4531,6 @@ dependencies = [ "paths", "serde", "serde_json", - "shlex", "smol", "task", "util", @@ -4757,7 +4756,6 @@ dependencies = [ "serde_json", "serde_json_lenient", "settings", - "shlex", "sysinfo 0.37.2", "task", "tasks_ui", @@ -12925,7 +12923,6 @@ dependencies = [ "settings", "sha2", "shellexpand 2.1.2", - "shlex", "smallvec", "smol", "snippet", @@ -13838,7 +13835,6 @@ dependencies = [ "serde", "serde_json", "settings", - "shlex", "smol", "tempfile", "thiserror 2.0.17", diff --git a/crates/askpass/src/askpass.rs b/crates/askpass/src/askpass.rs index dfe8a96ee6..9b5d584827 100644 --- a/crates/askpass/src/askpass.rs +++ b/crates/askpass/src/askpass.rs @@ -20,7 +20,7 @@ use futures::{ }; use gpui::{AsyncApp, BackgroundExecutor, Task}; use smol::fs; -use util::{ResultExt as _, debug_panic, maybe, paths::PathExt}; +use util::{ResultExt as _, debug_panic, maybe, paths::PathExt, shell::ShellKind}; /// Path to the program used for askpass /// @@ -199,9 +199,15 @@ impl PasswordProxy { let current_exec = std::env::current_exe().context("Failed to determine current zed executable path.")?; + // TODO: Inferred from the use of powershell.exe in askpass_helper_script + let shell_kind = if cfg!(windows) { + ShellKind::PowerShell + } else { + ShellKind::Posix + }; let askpass_program = ASKPASS_PROGRAM .get_or_init(|| current_exec) - .try_shell_safe() + .try_shell_safe(shell_kind) .context("Failed to shell-escape Askpass program path.")? .to_string(); // Create an askpass script that communicates back to this process. diff --git a/crates/dap_adapters/Cargo.toml b/crates/dap_adapters/Cargo.toml index 1593f51cf3..253674c0f3 100644 --- a/crates/dap_adapters/Cargo.toml +++ b/crates/dap_adapters/Cargo.toml @@ -35,7 +35,6 @@ log.workspace = true paths.workspace = true serde.workspace = true serde_json.workspace = true -shlex.workspace = true smol.workspace = true task.workspace = true util.workspace = true diff --git a/crates/dap_adapters/src/javascript.rs b/crates/dap_adapters/src/javascript.rs index 8c90bfc7c0..68f5ca7e79 100644 --- a/crates/dap_adapters/src/javascript.rs +++ b/crates/dap_adapters/src/javascript.rs @@ -6,7 +6,7 @@ use gpui::AsyncApp; use serde_json::Value; use std::{path::PathBuf, sync::OnceLock}; use task::DebugRequest; -use util::{ResultExt, maybe}; +use util::{ResultExt, maybe, shell::ShellKind}; use crate::*; @@ -67,7 +67,7 @@ impl JsDebugAdapter { .get("type") .filter(|value| value == &"node-terminal")?; let command = configuration.get("command")?.as_str()?.to_owned(); - let mut args = shlex::split(&command)?.into_iter(); + let mut args = ShellKind::Posix.split(&command)?.into_iter(); let program = args.next()?; configuration.insert("runtimeExecutable".to_owned(), program.into()); configuration.insert( diff --git a/crates/debugger_ui/Cargo.toml b/crates/debugger_ui/Cargo.toml index 28866f0d27..c1a0657c0e 100644 --- a/crates/debugger_ui/Cargo.toml +++ b/crates/debugger_ui/Cargo.toml @@ -60,7 +60,6 @@ serde.workspace = true serde_json.workspace = true serde_json_lenient.workspace = true settings.workspace = true -shlex.workspace = true sysinfo.workspace = true task.workspace = true tasks_ui.workspace = true diff --git a/crates/debugger_ui/src/new_process_modal.rs b/crates/debugger_ui/src/new_process_modal.rs index b56c0a5d3b..cf3c779aba 100644 --- a/crates/debugger_ui/src/new_process_modal.rs +++ b/crates/debugger_ui/src/new_process_modal.rs @@ -32,7 +32,7 @@ use ui::{ SharedString, Styled, StyledExt, ToggleButton, ToggleState, Toggleable, Tooltip, Window, div, h_flex, relative, rems, v_flex, }; -use util::{ResultExt, rel_path::RelPath}; +use util::{ResultExt, rel_path::RelPath, shell::ShellKind}; use workspace::{ModalView, Workspace, notifications::DetachAndPromptErr, pane}; use crate::{attach_modal::AttachModal, debugger_panel::DebugPanel}; @@ -837,7 +837,11 @@ impl ConfigureMode { }; } let command = self.program.read(cx).text(cx); - let mut args = shlex::split(&command).into_iter().flatten().peekable(); + let mut args = ShellKind::Posix + .split(&command) + .into_iter() + .flatten() + .peekable(); let mut env = FxHashMap::default(); while args.peek().is_some_and(|arg| arg.contains('=')) { let arg = args.next().unwrap(); @@ -1263,7 +1267,11 @@ impl PickerDelegate for DebugDelegate { }) .unwrap_or_default(); - let mut args = shlex::split(&text).into_iter().flatten().peekable(); + let mut args = ShellKind::Posix + .split(&text) + .into_iter() + .flatten() + .peekable(); let mut env = HashMap::default(); while args.peek().is_some_and(|arg| arg.contains('=')) { let arg = args.next().unwrap(); diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index 0297611d10..d9285a8c24 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -72,7 +72,6 @@ serde_json.workspace = true settings.workspace = true sha2.workspace = true shellexpand.workspace = true -shlex.workspace = true smallvec.workspace = true smol.workspace = true snippet.workspace = true diff --git a/crates/project/src/environment.rs b/crates/project/src/environment.rs index 519000f6a6..da2933d317 100644 --- a/crates/project/src/environment.rs +++ b/crates/project/src/environment.rs @@ -4,7 +4,7 @@ use language::Buffer; use remote::RemoteClient; use rpc::proto::{self, REMOTE_SERVER_PROJECT_ID}; use std::{collections::VecDeque, path::Path, sync::Arc}; -use task::Shell; +use task::{Shell, shell_to_proto}; use util::ResultExt; use worktree::Worktree; @@ -198,7 +198,7 @@ impl ProjectEnvironment { .proto_client() .request(proto::GetDirectoryEnvironment { project_id: REMOTE_SERVER_PROJECT_ID, - shell: Some(shell.clone().to_proto()), + shell: Some(shell_to_proto(shell.clone())), directory: abs_path.to_string_lossy().to_string(), }); cx.spawn(async move |_, _| { diff --git a/crates/project/src/lsp_store/lsp_ext_command.rs b/crates/project/src/lsp_store/lsp_ext_command.rs index c79e3df178..5066143244 100644 --- a/crates/project/src/lsp_store/lsp_ext_command.rs +++ b/crates/project/src/lsp_store/lsp_ext_command.rs @@ -657,6 +657,7 @@ impl LspCommand for GetLspRunnables { ); task_template.args.extend(cargo.cargo_args); if !cargo.executable_args.is_empty() { + let shell_kind = task_template.shell.shell_kind(cfg!(windows)); task_template.args.push("--".to_string()); task_template.args.extend( cargo @@ -682,7 +683,7 @@ impl LspCommand for GetLspRunnables { // That bit is not auto-expanded when using single quotes. // Escape extra cargo args unconditionally as those are unlikely to contain `~`. .flat_map(|extra_arg| { - shlex::try_quote(&extra_arg).ok().map(|s| s.to_string()) + shell_kind.try_quote(&extra_arg).map(|s| s.to_string()) }), ); } diff --git a/crates/project/src/terminals.rs b/crates/project/src/terminals.rs index 4a0a1790b4..cf25735e4b 100644 --- a/crates/project/src/terminals.rs +++ b/crates/project/src/terminals.rs @@ -168,20 +168,19 @@ impl Project { match remote_client { Some(remote_client) => match activation_script.clone() { activation_script if !activation_script.is_empty() => { - let activation_script = activation_script.join("; "); + let separator = shell_kind.sequential_commands_separator(); + let activation_script = + activation_script.join(&format!("{separator} ")); let to_run = format_to_run(); - let args = vec![ - "-c".to_owned(), - format!("{activation_script}; {to_run}"), - ]; + let shell = remote_client + .read(cx) + .shell() + .unwrap_or_else(get_default_system_shell); + let arg = format!("{activation_script}{separator} {to_run}"); + let args = shell_kind.args_for_shell(false, arg); + create_remote_shell( - Some(( - &remote_client - .read(cx) - .shell() - .unwrap_or_else(get_default_system_shell), - &args, - )), + Some((&shell, &args)), env, path, remote_client, @@ -562,7 +561,7 @@ fn create_remote_shell( Shell::WithArguments { program: command.program, args: command.args, - title_override: Some(format!("{} — Terminal", host).into()), + title_override: Some(format!("{} — Terminal", host)), }, command.env, )) diff --git a/crates/remote/Cargo.toml b/crates/remote/Cargo.toml index 0256048492..d1a91af9a5 100644 --- a/crates/remote/Cargo.toml +++ b/crates/remote/Cargo.toml @@ -34,7 +34,6 @@ rpc = { workspace = true, features = ["gpui"] } serde.workspace = true serde_json.workspace = true settings.workspace = true -shlex.workspace = true smol.workspace = true tempfile.workspace = true thiserror.workspace = true diff --git a/crates/remote/src/transport/ssh.rs b/crates/remote/src/transport/ssh.rs index a1337c2d65..745391e17e 100644 --- a/crates/remote/src/transport/ssh.rs +++ b/crates/remote/src/transport/ssh.rs @@ -203,17 +203,6 @@ impl AsMut for MasterProcess { } } -macro_rules! shell_script { - ($fmt:expr, $($name:ident = $arg:expr),+ $(,)?) => {{ - format!( - $fmt, - $( - $name = shlex::try_quote($arg).unwrap() - ),+ - ) - }}; -} - #[async_trait(?Send)] impl RemoteConnection for SshRemoteConnection { async fn kill(&self) -> Result<()> { @@ -738,21 +727,24 @@ impl SshRemoteConnection { delegate.set_status(Some("Extracting remote development server"), cx); let server_mode = 0o755; + let shell_kind = ShellKind::Posix; let orig_tmp_path = tmp_path.display(self.path_style()); + let server_mode = format!("{:o}", server_mode); + let server_mode = shell_kind + .try_quote(&server_mode) + .context("shell quoting")?; + let dst_path = dst_path.display(self.path_style()); + let dst_path = shell_kind.try_quote(&dst_path).context("shell quoting")?; let script = if let Some(tmp_path) = orig_tmp_path.strip_suffix(".gz") { - shell_script!( - "gunzip -f {orig_tmp_path} && chmod {server_mode} {tmp_path} && mv {tmp_path} {dst_path}", - server_mode = &format!("{:o}", server_mode), - dst_path = &dst_path.display(self.path_style()), + format!( + "gunzip -f {orig_tmp_path} && chmod {server_mode} {tmp_path} && mv {tmp_path} {dst_path}" ) } else { - shell_script!( - "chmod {server_mode} {orig_tmp_path} && mv {orig_tmp_path} {dst_path}", - server_mode = &format!("{:o}", server_mode), - dst_path = &dst_path.display(self.path_style()) - ) + format!("chmod {server_mode} {orig_tmp_path} && mv {orig_tmp_path} {dst_path}") }; - self.socket.run_command("sh", &["-c", &script]).await?; + let script = shell_kind.try_quote(&script).context("shell quoting")?; + let args = shell_kind.args_for_shell(false, script.to_string()); + self.socket.run_command("sh", &args).await?; Ok(()) } @@ -886,8 +878,12 @@ impl SshSocket { // into a machine. You must use `cd` to get back to $HOME. // You need to do it like this: $ ssh host "cd; sh -c 'ls -l /tmp'" fn ssh_command(&self, program: &str, args: &[impl AsRef]) -> process::Command { + let shell_kind = ShellKind::Posix; let mut command = util::command::new_smol_command("ssh"); - let mut to_run = shlex::try_quote(program).unwrap().into_owned(); + let mut to_run = shell_kind + .try_quote(program) + .expect("shell quoting") + .into_owned(); for arg in args { // We're trying to work with: sh, bash, zsh, fish, tcsh, ...? debug_assert!( @@ -895,9 +891,10 @@ impl SshSocket { "multiline arguments do not work in all shells" ); to_run.push(' '); - to_run.push_str(&shlex::try_quote(arg.as_ref()).unwrap()); + to_run.push_str(&shell_kind.try_quote(arg.as_ref()).expect("shell quoting")); } - let to_run = format!("cd; {to_run}"); + let separator = shell_kind.sequential_commands_separator(); + let to_run = format!("cd{separator} {to_run}"); self.ssh_options(&mut command, true) .arg(self.connection_options.ssh_url()) .arg("-T") @@ -906,7 +903,7 @@ impl SshSocket { command } - async fn run_command(&self, program: &str, args: &[&str]) -> Result { + async fn run_command(&self, program: &str, args: &[impl AsRef]) -> Result { let output = self.ssh_command(program, args).output().await?; anyhow::ensure!( output.status.success(), @@ -1080,7 +1077,10 @@ impl SshConnectionOptions { "-w", ]; - let mut tokens = shlex::split(input).context("invalid input")?.into_iter(); + let mut tokens = ShellKind::Posix + .split(input) + .context("invalid input")? + .into_iter(); 'outer: while let Some(arg) = tokens.next() { if ALLOWED_OPTS.contains(&(&arg as &str)) { @@ -1243,6 +1243,7 @@ fn build_command( ) -> Result { use std::fmt::Write as _; + let shell_kind = ShellKind::new(ssh_shell, false); let mut exec = String::new(); if let Some(working_dir) = working_dir { let working_dir = RemotePathBuf::new(working_dir, ssh_path_style).to_string(); @@ -1252,29 +1253,41 @@ fn build_command( const TILDE_PREFIX: &'static str = "~/"; if working_dir.starts_with(TILDE_PREFIX) { let working_dir = working_dir.trim_start_matches("~").trim_start_matches("/"); - write!(exec, "cd \"$HOME/{working_dir}\" && ",).unwrap(); + write!(exec, "cd \"$HOME/{working_dir}\" && ",)?; } else { - write!(exec, "cd \"{working_dir}\" && ",).unwrap(); + write!(exec, "cd \"{working_dir}\" && ",)?; } } else { - write!(exec, "cd && ").unwrap(); + write!(exec, "cd && ")?; }; - write!(exec, "exec env ").unwrap(); + write!(exec, "exec env ")?; for (k, v) in input_env.iter() { - if let Some((k, v)) = shlex::try_quote(k).ok().zip(shlex::try_quote(v).ok()) { - write!(exec, "{}={} ", k, v).unwrap(); - } + write!( + exec, + "{}={} ", + k, + shell_kind.try_quote(v).context("shell quoting")? + )?; } if let Some(input_program) = input_program { - write!(exec, "{}", shlex::try_quote(&input_program).unwrap()).unwrap(); + write!( + exec, + "{}", + shell_kind + .try_quote(&input_program) + .context("shell quoting")? + )?; for arg in input_args { - let arg = shlex::try_quote(&arg)?; - write!(exec, " {}", &arg).unwrap(); + write!( + exec, + " {}", + shell_kind.try_quote(&arg).context("shell quoting")? + )?; } } else { - write!(exec, "{ssh_shell} -l").unwrap(); + write!(exec, "{ssh_shell} -l")?; }; let mut args = Vec::new(); diff --git a/crates/remote/src/transport/wsl.rs b/crates/remote/src/transport/wsl.rs index 4eca7c4d52..4ccadee73f 100644 --- a/crates/remote/src/transport/wsl.rs +++ b/crates/remote/src/transport/wsl.rs @@ -2,7 +2,7 @@ use crate::{ RemoteClientDelegate, RemotePlatform, remote_client::{CommandTemplate, RemoteConnection, RemoteConnectionOptions}, }; -use anyhow::{Result, anyhow, bail}; +use anyhow::{Context, Result, anyhow, bail}; use async_trait::async_trait; use collections::HashMap; use futures::channel::mpsc::{Sender, UnboundedReceiver, UnboundedSender}; @@ -441,6 +441,7 @@ impl RemoteConnection for WslRemoteConnection { bail!("WSL shares the network interface with the host system"); } + let shell = ShellKind::new(&self.shell, false); let working_dir = working_dir .map(|working_dir| RemotePathBuf::new(working_dir, PathStyle::Posix).to_string()) .unwrap_or("~".to_string()); @@ -448,19 +449,26 @@ impl RemoteConnection for WslRemoteConnection { let mut exec = String::from("exec env "); for (k, v) in env.iter() { - if let Some((k, v)) = shlex::try_quote(k).ok().zip(shlex::try_quote(v).ok()) { - write!(exec, "{}={} ", k, v).unwrap(); - } + write!( + exec, + "{}={} ", + k, + shell.try_quote(&v).context("shell quoting")? + )?; } if let Some(program) = program { - write!(exec, "{}", shlex::try_quote(&program)?).unwrap(); + write!( + exec, + "{}", + shell.try_quote(&program).context("shell quoting")? + )?; for arg in args { - let arg = shlex::try_quote(&arg)?; - write!(exec, " {}", &arg).unwrap(); + let arg = shell.try_quote(&arg).context("shell quoting")?; + write!(exec, " {}", &arg)?; } } else { - write!(&mut exec, "{} -l", self.shell).unwrap(); + write!(&mut exec, "{} -l", self.shell)?; } let wsl_args = if let Some(user) = &self.connection_options.user { diff --git a/crates/remote_server/src/headless_project.rs b/crates/remote_server/src/headless_project.rs index 83000c8bac..588ee83608 100644 --- a/crates/remote_server/src/headless_project.rs +++ b/crates/remote_server/src/headless_project.rs @@ -774,7 +774,7 @@ impl HeadlessProject { envelope: TypedEnvelope, mut cx: AsyncApp, ) -> Result { - let shell = task::Shell::from_proto(envelope.payload.shell.context("missing shell")?)?; + let shell = task::shell_from_proto(envelope.payload.shell.context("missing shell")?)?; let directory = PathBuf::from(envelope.payload.directory); let environment = this .update(&mut cx, |this, cx| { diff --git a/crates/task/src/task.rs b/crates/task/src/task.rs index bfb84ced94..280bf5ccdb 100644 --- a/crates/task/src/task.rs +++ b/crates/task/src/task.rs @@ -3,7 +3,6 @@ mod adapter_schema; mod debug_format; mod serde_helpers; -mod shell_builder; pub mod static_source; mod task_template; mod vscode_debug_format; @@ -12,23 +11,22 @@ mod vscode_format; use anyhow::Context as _; use collections::{HashMap, HashSet, hash_map}; use gpui::SharedString; -use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::borrow::Cow; use std::path::PathBuf; use std::str::FromStr; -use util::get_system_shell; pub use adapter_schema::{AdapterSchema, AdapterSchemas}; pub use debug_format::{ AttachRequest, BuildTaskDefinition, DebugRequest, DebugScenario, DebugTaskFile, LaunchRequest, Request, TcpArgumentsTemplate, ZedDebugConfig, }; -pub use shell_builder::{ShellBuilder, ShellKind}; pub use task_template::{ DebugArgsRequest, HideStrategy, RevealStrategy, TaskTemplate, TaskTemplates, substitute_variables_in_map, substitute_variables_in_str, }; +pub use util::shell::{Shell, ShellKind}; +pub use util::shell_builder::ShellBuilder; pub use vscode_debug_format::VsCodeDebugTaskFile; pub use vscode_format::VsCodeTaskFile; pub use zed_actions::RevealTarget; @@ -318,81 +316,32 @@ pub struct TaskContext { #[derive(Clone, Debug)] pub struct RunnableTag(pub SharedString); -/// Shell configuration to open the terminal with. -#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, Hash)] -#[serde(rename_all = "snake_case")] -pub enum Shell { - /// Use the system's default terminal configuration in /etc/passwd - #[default] - System, - /// Use a specific program with no arguments. - Program(String), - /// Use a specific program with arguments. - WithArguments { - /// The program to run. - program: String, - /// The arguments to pass to the program. - args: Vec, - /// An optional string to override the title of the terminal tab - title_override: Option, - }, +pub fn shell_from_proto(proto: proto::Shell) -> anyhow::Result { + let shell_type = proto.shell_type.context("invalid shell type")?; + let shell = match shell_type { + proto::shell::ShellType::System(_) => Shell::System, + proto::shell::ShellType::Program(program) => Shell::Program(program), + proto::shell::ShellType::WithArguments(program) => Shell::WithArguments { + program: program.program, + args: program.args, + title_override: None, + }, + }; + Ok(shell) } -impl Shell { - pub fn program(&self) -> String { - match self { - Shell::Program(program) => program.clone(), - Shell::WithArguments { program, .. } => program.clone(), - Shell::System => get_system_shell(), - } - } - - pub fn program_and_args(&self) -> (String, &[String]) { - match self { - Shell::Program(program) => (program.clone(), &[]), - Shell::WithArguments { program, args, .. } => (program.clone(), args), - Shell::System => (get_system_shell(), &[]), - } - } - - pub fn shell_kind(&self, is_windows: bool) -> ShellKind { - match self { - Shell::Program(program) => ShellKind::new(program, is_windows), - Shell::WithArguments { program, .. } => ShellKind::new(program, is_windows), - Shell::System => ShellKind::system(), - } - } - - pub fn from_proto(proto: proto::Shell) -> anyhow::Result { - let shell_type = proto.shell_type.context("invalid shell type")?; - let shell = match shell_type { - proto::shell::ShellType::System(_) => Self::System, - proto::shell::ShellType::Program(program) => Self::Program(program), - proto::shell::ShellType::WithArguments(program) => Self::WithArguments { - program: program.program, - args: program.args, - title_override: None, - }, - }; - Ok(shell) - } - - pub fn to_proto(self) -> proto::Shell { - let shell_type = match self { - Shell::System => proto::shell::ShellType::System(proto::System {}), - Shell::Program(program) => proto::shell::ShellType::Program(program), - Shell::WithArguments { - program, - args, - title_override: _, - } => proto::shell::ShellType::WithArguments(proto::shell::WithArguments { - program, - args, - }), - }; - proto::Shell { - shell_type: Some(shell_type), - } +pub fn shell_to_proto(shell: Shell) -> proto::Shell { + let shell_type = match shell { + Shell::System => proto::shell::ShellType::System(proto::System {}), + Shell::Program(program) => proto::shell::ShellType::Program(program), + Shell::WithArguments { + program, + args, + title_override: _, + } => proto::shell::ShellType::WithArguments(proto::shell::WithArguments { program, args }), + }; + proto::Shell { + shell_type: Some(shell_type), } } diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index fa42a94e93..68ac3d3e29 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -67,7 +67,7 @@ use thiserror::Error; use gpui::{ App, AppContext as _, Bounds, ClipboardItem, Context, EventEmitter, Hsla, Keystroke, Modifiers, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, Point, Rgba, - ScrollWheelEvent, SharedString, Size, Task, TouchPhase, Window, actions, black, px, + ScrollWheelEvent, Size, Task, TouchPhase, Window, actions, black, px, }; use crate::mappings::{colors::to_alac_rgb, keys::to_esc_str}; @@ -277,7 +277,7 @@ pub struct TerminalError { pub directory: Option, pub program: Option, pub args: Option>, - pub title_override: Option, + pub title_override: Option, pub source: std::io::Error, } @@ -445,14 +445,14 @@ impl TerminalBuilder { struct ShellParams { program: String, args: Option>, - title_override: Option, + title_override: Option, } impl ShellParams { fn new( program: String, args: Option>, - title_override: Option, + title_override: Option, ) -> Self { log::debug!("Using {program} as shell"); Self { @@ -514,10 +514,8 @@ impl TerminalBuilder { working_directory: working_directory.clone(), drain_on_exit: true, env: env.clone().into_iter().collect(), - // We do not want to escape arguments if we are using CMD as our shell. - // If we do we end up with too many quotes/escaped quotes for CMD to handle. #[cfg(windows)] - escape_args: shell_kind != util::shell::ShellKind::Cmd, + escape_args: shell_kind.tty_escape_args(), } }; @@ -824,7 +822,7 @@ pub struct Terminal { pub last_content: TerminalContent, pub selection_head: Option, pub breadcrumb_text: String, - title_override: Option, + title_override: Option, scroll_px: Pixels, next_link_id: usize, selection_phase: SelectionPhase, diff --git a/crates/terminal/src/terminal_settings.rs b/crates/terminal/src/terminal_settings.rs index 9bb5ffb517..b8576a1de3 100644 --- a/crates/terminal/src/terminal_settings.rs +++ b/crates/terminal/src/terminal_settings.rs @@ -66,7 +66,7 @@ fn settings_shell_to_task_shell(shell: settings::Shell) -> Shell { } => Shell::WithArguments { program, args, - title_override, + title_override: title_override.map(Into::into), }, } } diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index 0743601839..20187bf737 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -15,7 +15,7 @@ use std::{ sync::LazyLock, }; -use crate::rel_path::RelPath; +use crate::{rel_path::RelPath, shell::ShellKind}; static HOME_DIR: OnceLock = OnceLock::new(); @@ -84,9 +84,7 @@ pub trait PathExt { fn multiple_extensions(&self) -> Option; /// 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; + fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result; } impl> PathExt for T { @@ -164,24 +162,16 @@ impl> PathExt for T { Some(parts.into_iter().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()) - } + fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result { + let path_str = self + .as_ref() + .to_str() + .with_context(|| "Path contains invalid UTF-8")?; + shell_kind + .try_quote(path_str) + .as_deref() + .map(ToOwned::to_owned) + .context("Failed to quote path") } } diff --git a/crates/util/src/shell.rs b/crates/util/src/shell.rs index 22e07acf25..d81946b8ad 100644 --- a/crates/util/src/shell.rs +++ b/crates/util/src/shell.rs @@ -1,6 +1,53 @@ +use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::{borrow::Cow, fmt, path::Path, sync::LazyLock}; +/// Shell configuration to open the terminal with. +#[derive(Clone, Debug, Default, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum Shell { + /// Use the system's default terminal configuration in /etc/passwd + #[default] + System, + /// Use a specific program with no arguments. + Program(String), + /// Use a specific program with arguments. + WithArguments { + /// The program to run. + program: String, + /// The arguments to pass to the program. + args: Vec, + /// An optional string to override the title of the terminal tab + title_override: Option, + }, +} + +impl Shell { + pub fn program(&self) -> String { + match self { + Shell::Program(program) => program.clone(), + Shell::WithArguments { program, .. } => program.clone(), + Shell::System => get_system_shell(), + } + } + + pub fn program_and_args(&self) -> (String, &[String]) { + match self { + Shell::Program(program) => (program.clone(), &[]), + Shell::WithArguments { program, args, .. } => (program.clone(), args), + Shell::System => (get_system_shell(), &[]), + } + } + + pub fn shell_kind(&self, is_windows: bool) -> ShellKind { + match self { + Shell::Program(program) => ShellKind::new(program, is_windows), + Shell::WithArguments { program, .. } => ShellKind::new(program, is_windows), + Shell::System => ShellKind::system(), + } + } +} + #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum ShellKind { #[default] @@ -185,32 +232,20 @@ impl ShellKind { .unwrap_or_else(|| program.as_os_str()) .to_string_lossy(); - if program == "powershell" || program == "pwsh" { - ShellKind::PowerShell - } else if program == "cmd" { - ShellKind::Cmd - } else if program == "nu" { - ShellKind::Nushell - } else if program == "fish" { - ShellKind::Fish - } else if program == "csh" { - ShellKind::Csh - } else if program == "tcsh" { - ShellKind::Tcsh - } else if program == "rc" { - ShellKind::Rc - } else if program == "xonsh" { - ShellKind::Xonsh - } else if program == "sh" || program == "bash" { - ShellKind::Posix - } else { - if is_windows { - ShellKind::PowerShell - } else { - // Some other shell detected, the user might install and use a - // unix-like shell. - ShellKind::Posix - } + match &*program { + "powershell" | "pwsh" => ShellKind::PowerShell, + "cmd" => ShellKind::Cmd, + "nu" => ShellKind::Nushell, + "fish" => ShellKind::Fish, + "csh" => ShellKind::Csh, + "tcsh" => ShellKind::Tcsh, + "rc" => ShellKind::Rc, + "xonsh" => ShellKind::Xonsh, + "sh" | "bash" => ShellKind::Posix, + _ if is_windows => ShellKind::PowerShell, + // Some other shell detected, the user might install and use a + // unix-like shell. + _ => ShellKind::Posix, } } @@ -363,44 +398,132 @@ impl ShellKind { match self { ShellKind::PowerShell => Some('&'), ShellKind::Nushell => Some('^'), - _ => None, + ShellKind::Posix + | ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Rc + | ShellKind::Fish + | ShellKind::Cmd + | ShellKind::Xonsh => None, } } pub const fn sequential_commands_separator(&self) -> char { match self { ShellKind::Cmd => '&', - _ => ';', + ShellKind::Posix + | ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Rc + | ShellKind::Fish + | ShellKind::PowerShell + | ShellKind::Nushell + | ShellKind::Xonsh => ';', } } pub fn try_quote<'a>(&self, arg: &'a str) -> Option> { + // 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 :( shlex::try_quote(arg).ok().map(|arg| match self { - // If we are running in PowerShell, we want to take extra care when escaping strings. - // In particular, we want to escape strings with a backtick (`) rather than a backslash (\). - // TODO double escaping backslashes is not necessary in PowerShell and probably CMD - ShellKind::PowerShell => Cow::Owned(arg.replace("\\\"", "`\"")), - _ => arg, + ShellKind::PowerShell => Cow::Owned(arg.replace("\\\"", "`\"").replace("\\\\", "\\")), + ShellKind::Cmd => Cow::Owned(arg.replace("\\\\", "\\")), + ShellKind::Posix + | ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Rc + | ShellKind::Fish + | ShellKind::Nushell + | ShellKind::Xonsh => arg, }) } + pub fn split(&self, input: &str) -> Option> { + shlex::split(input) + } + pub const fn activate_keyword(&self) -> &'static str { match self { ShellKind::Cmd => "", ShellKind::Nushell => "overlay use", ShellKind::PowerShell => ".", - ShellKind::Fish => "source", - ShellKind::Csh => "source", - ShellKind::Tcsh => "source", - ShellKind::Posix | ShellKind::Rc => "source", - ShellKind::Xonsh => "source", + ShellKind::Fish + | ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Posix + | ShellKind::Rc + | ShellKind::Xonsh => "source", } } pub const fn clear_screen_command(&self) -> &'static str { match self { ShellKind::Cmd => "cls", - _ => "clear", + ShellKind::Posix + | ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Rc + | ShellKind::Fish + | ShellKind::PowerShell + | ShellKind::Nushell + | ShellKind::Xonsh => "clear", + } + } + + #[cfg(windows)] + /// We do not want to escape arguments if we are using CMD as our shell. + /// If we do we end up with too many quotes/escaped quotes for CMD to handle. + pub const fn tty_escape_args(&self) -> bool { + match self { + ShellKind::Cmd => false, + ShellKind::Posix + | ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Rc + | ShellKind::Fish + | ShellKind::PowerShell + | ShellKind::Nushell + | ShellKind::Xonsh => true, } } } + +#[cfg(test)] +mod tests { + use super::*; + + // Examples + // WSL + // wsl.exe --distribution NixOS --cd /home/user -- /usr/bin/zsh -c "echo hello" + // wsl.exe --distribution NixOS --cd /home/user -- /usr/bin/zsh -c "\"echo hello\"" | grep hello" + // wsl.exe --distribution NixOS --cd ~ env RUST_LOG=info,remote=debug .zed_wsl_server/zed-remote-server-dev-build proxy --identifier dev-workspace-53 + // PowerShell from Nushell + // nu -c overlay use "C:\Users\kubko\dev\python\39007\tests\.venv\Scripts\activate.nu"; ^"C:\Program Files\PowerShell\7\pwsh.exe" -C "C:\Users\kubko\dev\python\39007\tests\.venv\Scripts\python.exe -m pytest \"test_foo.py::test_foo\"" + // PowerShell from CMD + // cmd /C \" \"C:\\\\Users\\\\kubko\\\\dev\\\\python\\\\39007\\\\tests\\\\.venv\\\\Scripts\\\\activate.bat\"& \"C:\\\\Program Files\\\\PowerShell\\\\7\\\\pwsh.exe\" -C \"C:\\\\Users\\\\kubko\\\\dev\\\\python\\\\39007\\\\tests\\\\.venv\\\\Scripts\\\\python.exe -m pytest \\\"test_foo.py::test_foo\\\"\"\" + + #[test] + fn test_try_quote_powershell() { + let shell_kind = ShellKind::PowerShell; + assert_eq!( + shell_kind + .try_quote("C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \"test_foo.py::test_foo\"") + .unwrap() + .into_owned(), + "\"C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest `\"test_foo.py::test_foo`\"\"".to_string() + ); + } + + #[test] + fn test_try_quote_cmd() { + let shell_kind = ShellKind::Cmd; + assert_eq!( + shell_kind + .try_quote("C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \"test_foo.py::test_foo\"") + .unwrap() + .into_owned(), + "\"C:\\Users\\johndoe\\dev\\python\\39007\\tests\\.venv\\Scripts\\python.exe -m pytest \\\"test_foo.py::test_foo\\\"\"".to_string() + ); + } +} diff --git a/crates/task/src/shell_builder.rs b/crates/util/src/shell_builder.rs similarity index 98% rename from crates/task/src/shell_builder.rs rename to crates/util/src/shell_builder.rs index a6504f4eb7..7e52b67b35 100644 --- a/crates/task/src/shell_builder.rs +++ b/crates/util/src/shell_builder.rs @@ -1,8 +1,5 @@ -use util::shell::get_system_shell; - -use crate::Shell; - -pub use util::shell::ShellKind; +use crate::shell::get_system_shell; +use crate::shell::{Shell, ShellKind}; /// ShellBuilder is used to turn a user-requested task into a /// program that can be executed by the shell. diff --git a/crates/util/src/shell_env.rs b/crates/util/src/shell_env.rs index a82bea154e..b3c9e3bef3 100644 --- a/crates/util/src/shell_env.rs +++ b/crates/util/src/shell_env.rs @@ -35,8 +35,8 @@ async fn capture_unix( use std::os::unix::process::CommandExt; use std::process::Stdio; - let zed_path = super::get_shell_safe_zed_path()?; let shell_kind = ShellKind::new(shell_path, false); + let zed_path = super::get_shell_safe_zed_path(shell_kind)?; let mut command_string = String::new(); let mut command = std::process::Command::new(shell_path); diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index f725167724..3a78ef3d41 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -9,6 +9,7 @@ pub mod rel_path; pub mod schemars; pub mod serde; pub mod shell; +pub mod shell_builder; pub mod shell_env; pub mod size; #[cfg(any(test, feature = "test-support"))] @@ -295,12 +296,12 @@ fn load_shell_from_passwd() -> Result<()> { } /// Returns a shell escaped path for the current zed executable -pub fn get_shell_safe_zed_path() -> anyhow::Result { +pub fn get_shell_safe_zed_path(shell_kind: shell::ShellKind) -> anyhow::Result { let zed_path = std::env::current_exe().context("Failed to determine current zed executable path.")?; zed_path - .try_shell_safe() + .try_shell_safe(shell_kind) .context("Failed to shell-escape Zed executable path.") }