Use ShellKind::try_quote whenever we need to quote shell args (#40912)

Jakub Konka and Lukas Wirth created

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 <me@lukaswirth.dev>

Change summary

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 
crates/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/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(-)

Detailed changes

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",

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.

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

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(

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

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();

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

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 |_, _| {

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())
                                 }),
                         );
                     }

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,
     ))

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

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

@@ -203,17 +203,6 @@ impl AsMut<Child> 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<str>]) -> 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<String> {
+    async fn run_command(&self, program: &str, args: &[impl AsRef<str>]) -> Result<String> {
         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<CommandTemplate> {
     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();

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 {

crates/remote_server/src/headless_project.rs 🔗

@@ -774,7 +774,7 @@ impl HeadlessProject {
         envelope: TypedEnvelope<proto::GetDirectoryEnvironment>,
         mut cx: AsyncApp,
     ) -> Result<proto::DirectoryEnvironment> {
-        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| {

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<String>,
-        /// An optional string to override the title of the terminal tab
-        title_override: Option<SharedString>,
-    },
+pub fn shell_from_proto(proto: proto::Shell) -> anyhow::Result<Shell> {
+    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<Self> {
-        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),
     }
 }
 

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<PathBuf>,
     pub program: Option<String>,
     pub args: Option<Vec<String>>,
-    pub title_override: Option<SharedString>,
+    pub title_override: Option<String>,
     pub source: std::io::Error,
 }
 
@@ -445,14 +445,14 @@ impl TerminalBuilder {
             struct ShellParams {
                 program: String,
                 args: Option<Vec<String>>,
-                title_override: Option<SharedString>,
+                title_override: Option<String>,
             }
 
             impl ShellParams {
                 fn new(
                     program: String,
                     args: Option<Vec<String>>,
-                    title_override: Option<SharedString>,
+                    title_override: Option<String>,
                 ) -> 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<AlacPoint>,
     pub breadcrumb_text: String,
-    title_override: Option<SharedString>,
+    title_override: Option<String>,
     scroll_px: Pixels,
     next_link_id: usize,
     selection_phase: SelectionPhase,

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),
         },
     }
 }

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<PathBuf> = OnceLock::new();
 
@@ -84,9 +84,7 @@ pub trait PathExt {
     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>;
+    fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result<String>;
 }
 
 impl<T: AsRef<Path>> PathExt for T {
@@ -164,24 +162,16 @@ 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())
-        }
+    fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result<String> {
+        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")
     }
 }
 

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<String>,
+        /// An optional string to override the title of the terminal tab
+        title_override: Option<String>,
+    },
+}
+
+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<Cow<'a, str>> {
+        // 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<Vec<String>> {
+        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()
+        );
+    }
+}

crates/task/src/shell_builder.rs → 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.

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);

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<String> {
+pub fn get_shell_safe_zed_path(shell_kind: shell::ShellKind) -> anyhow::Result<String> {
     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.")
 }