From b7e11b38f4ce04a12b50022e86eac4d7aafff66e Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 29 Jan 2026 01:08:05 -0500 Subject: [PATCH] Allow always_allow patterns for Nushell, Elvish, and Rc shells (#47908) ## Summary This PR extends the `always_allow` tool permission patterns to work with Nushell, Elvish, and Rc shells. Previously, these shells were incorrectly excluded because they don't use `&&`/`||` operators for command chaining. However, brush-parser can safely parse their command syntax since they all use `;` for sequential execution. ## Changes - Add `ShellKind::Nushell`, `ShellKind::Elvish`, and `ShellKind::Rc` to `supports_posix_chaining()` - Split `ShellKind::Unknown` into `ShellKind::UnknownWindows` and `ShellKind::UnknownUnix` to preserve platform-specific fallback behavior while still denying `always_allow` patterns for unrecognized shells - Add comprehensive tests for the new shell support - Clarify documentation about shell compatibility ## Shell Notes - **Nushell**: Uses `;` for sequential execution. The `and`/`or` keywords are boolean operators on values, not command chaining. - **Elvish**: Uses `;` to separate pipelines. Does not have `&&` or `||` operators. Its `and`/`or` are special commands operating on values. - **Rc (Plan 9)**: Uses `;` for sequential execution and `|` for piping. Does not have `&&`/`||` operators. ## Security Unknown shells still return `false` from `supports_posix_chaining()`, so `always_allow` patterns are denied for safety when we can't verify the shell's syntax. (No release notes because granular tool permissions are still feature-flagged.) Release Notes: - N/A --------- Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com> --- crates/acp_thread/src/acp_thread.rs | 10 +- crates/acp_thread/src/terminal.rs | 3 +- crates/agent/src/thread.rs | 4 +- crates/agent/src/tool_permissions.rs | 193 +++++++--- crates/agent_servers/src/acp.rs | 2 +- crates/askpass/src/askpass.rs | 13 +- .../src/transport/stdio_transport.rs | 2 +- crates/dap_adapters/src/javascript.rs | 10 +- crates/debugger_ui/src/new_process_modal.rs | 12 +- crates/debugger_ui/src/session/running.rs | 3 +- crates/language/src/toolchain.rs | 2 +- crates/languages/src/python.rs | 68 ++-- crates/project/src/debugger/locators/cargo.rs | 2 +- .../project/src/lsp_store/lsp_ext_command.rs | 5 +- crates/project/src/terminals.rs | 34 +- .../tests/integration/project_tests.rs | 2 +- crates/prompt_store/src/prompts.rs | 8 +- crates/remote/src/transport/docker.rs | 5 +- crates/remote/src/transport/ssh.rs | 26 +- crates/remote/src/transport/wsl.rs | 10 +- crates/task/src/task.rs | 2 +- crates/terminal/src/terminal.rs | 13 +- crates/terminal_view/src/terminal_panel.rs | 15 +- crates/util/src/paths.rs | 14 +- crates/util/src/shell.rs | 334 +++++++++++++----- crates/util/src/shell_builder.rs | 174 +++++---- crates/util/src/shell_env.rs | 44 +-- crates/util/src/util.rs | 2 +- 28 files changed, 694 insertions(+), 318 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 840c443b11a679cae349a33ffe0a2323445dceea..b61d869243ac87e06becc0cf29fa88a6799c625b 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -2421,7 +2421,6 @@ impl AcpThread { let project = self.project.clone(); let language_registry = project.read(cx).languages().clone(); - let is_windows = project.read(cx).path_style(cx).is_windows(); let terminal_id = acp::TerminalId::new(Uuid::new_v4().to_string()); let terminal_task = cx.spawn({ @@ -2435,10 +2434,9 @@ impl AcpThread { .and_then(|r| r.read(cx).default_system_shell()) }) .unwrap_or_else(|| get_default_system_shell_preferring_bash()); - let (task_command, task_args) = - ShellBuilder::new(&Shell::Program(shell), is_windows) - .redirect_stdin_to_dev_null() - .build(Some(command.clone()), &args); + let (task_command, task_args) = ShellBuilder::new(&Shell::Program(shell)) + .redirect_stdin_to_dev_null() + .build(Some(command.clone()), &args); let terminal = project .update(cx, |project, cx| { project.create_terminal_task( @@ -2792,7 +2790,7 @@ mod tests { // Create a real PTY terminal that runs a command which prints output then sleeps // We use printf instead of echo and chain with && sleep to ensure proper execution let (completion_tx, _completion_rx) = smol::channel::unbounded(); - let (program, args) = ShellBuilder::new(&Shell::System, false).build( + let (program, args) = ShellBuilder::new(&Shell::System).build( Some("printf 'output_before_kill\\n' && sleep 60".to_owned()), &[], ); diff --git a/crates/acp_thread/src/terminal.rs b/crates/acp_thread/src/terminal.rs index 61da3c27b551cbdb156a86f120a8a7af4799af96..21b01bb665119333c4b1b65464d49210290ad906 100644 --- a/crates/acp_thread/src/terminal.rs +++ b/crates/acp_thread/src/terminal.rs @@ -227,8 +227,7 @@ pub async fn create_terminal_entity( .map(Shell::Program) }) .unwrap_or_else(|| Shell::Program(get_default_system_shell_preferring_bash())); - let is_windows = project.read_with(cx, |project, cx| project.path_style(cx).is_windows()); - let (task_command, task_args) = task::ShellBuilder::new(&shell, is_windows) + let (task_command, task_args) = task::ShellBuilder::new(&shell) .redirect_stdin_to_dev_null() .build(Some(command.clone()), &args); diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index f9c2a761b18f5f8aa68d0274fae5dec55da3ef63..ea684f8625f0b6b2daaa6f817807c967a1c7d54e 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -654,7 +654,9 @@ impl ToolPermissionContext { // Check if the user's shell supports POSIX-like command chaining. // See the doc comment above for the full explanation of why this is needed. let shell_supports_always_allow = if tool_name == TerminalTool::name() { - ShellKind::system().supports_posix_chaining() + ShellKind::system() + .map(|k| k.supports_posix_chaining()) + .unwrap_or(false) } else { true }; diff --git a/crates/agent/src/tool_permissions.rs b/crates/agent/src/tool_permissions.rs index f747aa56eb36f3b73ff8c8ba2450e7f6e843290a..bf30c750ed56ad740fc3010632ca4c8913545859 100644 --- a/crates/agent/src/tool_permissions.rs +++ b/crates/agent/src/tool_permissions.rs @@ -30,15 +30,22 @@ impl ToolPermissionDecision { /// /// # Shell Compatibility (Terminal Tool Only) /// - /// For the terminal tool, commands are parsed to extract sub-commands for security. - /// This parsing only works for shells with POSIX-like `&&` / `||` / `;` / `|` syntax: + /// For the terminal tool, commands are parsed using brush-parser to extract sub-commands + /// for security. This ensures that patterns like `^cargo\b` cannot be bypassed with + /// shell injection like `cargo && rm -rf /`. /// - /// **Compatible shells:** Posix (sh, bash, dash, zsh), Fish 3.0+, PowerShell 7+/Pwsh, - /// Cmd, Xonsh, Csh, Tcsh + /// The parser handles `;` (sequential execution), `|` (piping), `&&` and `||` + /// (conditional execution), `$()` and backticks (command substitution), and process + /// substitution. /// - /// **Incompatible shells:** Nushell, Elvish, Rc (Plan 9) + /// # Shell Notes /// - /// For incompatible shells, `always_allow` patterns are disabled for safety. + /// - **Nushell**: Uses `;` for sequential execution. The `and`/`or` keywords are + /// boolean operators on values (e.g., `$true and $false`), not command chaining. + /// - **Elvish**: Uses `;` to separate pipelines, which brush-parser handles. Elvish + /// does not have `&&` or `||` operators. Its `and`/`or` are special commands that + /// operate on values, not command chaining (e.g., `and $true $false`). + /// - **Rc (Plan 9)**: Uses `;` for sequential execution. Does not have `&&`/`||`. /// /// # Pattern Matching Tips /// @@ -57,7 +64,7 @@ impl ToolPermissionDecision { input: &str, permissions: &ToolPermissions, always_allow_tool_actions: bool, - shell_kind: ShellKind, + shell_kind: Option, ) -> ToolPermissionDecision { // If always_allow_tool_actions is enabled, bypass all permission checks. // This is intentionally placed first - it's a global override that the user @@ -86,22 +93,28 @@ impl ToolPermissionDecision { // If parsing fails or the shell syntax is unsupported, always_allow is // disabled for this command (we set allow_enabled to false to signal this). if tool_name == TerminalTool::name() { - // Our shell parser (brush-parser) only supports POSIX-like shell syntax. - // See the doc comment above for the list of compatible/incompatible shells. - if !shell_kind.supports_posix_chaining() { + // Check if this shell's syntax can be parsed by brush-parser. + // See the doc comment above for shell compatibility notes. + let supports_chaining = shell_kind + .map(|k| k.supports_posix_chaining()) + .unwrap_or(false); + if !supports_chaining { // For shells with incompatible syntax, we can't reliably parse // the command to extract sub-commands. if !rules.always_allow.is_empty() { // If the user has configured always_allow patterns, we must deny // because we can't safely verify the command doesn't contain // hidden sub-commands that bypass the allow patterns. - return ToolPermissionDecision::Deny(format!( - "The {} shell does not support \"always allow\" patterns for the terminal \ - tool because Zed cannot parse its command chaining syntax. Please remove \ - the always_allow patterns from your tool_permissions settings, or switch \ - to a POSIX-conforming shell.", - shell_kind - )); + const SUFFIX: &str = " does not support \"always allow\" patterns for the \ + terminal tool because Zed cannot parse its command chaining syntax. \ + Please remove the always_allow patterns from your tool_permissions \ + settings, or switch to a supported shell."; + let message = if let Some(name) = shell_kind.map(|k| k.name()) { + format!("The {} shell{}", name, SUFFIX) + } else { + format!("This shell is unrecognized, and{}", SUFFIX) + }; + return ToolPermissionDecision::Deny(message); } // No always_allow rules, so we can still check deny/confirm patterns. return check_commands(std::iter::once(input.to_string()), rules, tool_name, false); @@ -229,6 +242,7 @@ mod tests { use crate::pattern_extraction::extract_terminal_pattern; use agent_settings::{CompiledRegex, InvalidRegexPattern, ToolRules}; use std::sync::Arc; + use util::shell::PosixShell; fn pattern(command: &str) -> &'static str { Box::leak( @@ -246,7 +260,7 @@ mod tests { deny: Vec<(&'static str, bool)>, confirm: Vec<(&'static str, bool)>, global: bool, - shell: ShellKind, + shell: Option, } impl PermTest { @@ -259,7 +273,7 @@ mod tests { deny: vec![], confirm: vec![], global: false, - shell: ShellKind::Posix, + shell: Some(ShellKind::Posix(PosixShell::Sh)), } } @@ -295,7 +309,7 @@ mod tests { self.global = g; self } - fn shell(mut self, s: ShellKind) -> Self { + fn shell(mut self, s: Option) -> Self { self.shell = s; self } @@ -370,7 +384,7 @@ mod tests { tools: collections::HashMap::default(), }, global, - ShellKind::Posix, + Some(ShellKind::Posix(PosixShell::Sh)), ) } @@ -605,16 +619,34 @@ mod tests { let p = ToolPermissions { tools }; // With always_allow_tool_actions=true, even default_mode: Deny is overridden assert_eq!( - ToolPermissionDecision::from_input("terminal", "x", &p, true, ShellKind::Posix), + ToolPermissionDecision::from_input( + "terminal", + "x", + &p, + true, + Some(ShellKind::Posix(PosixShell::Sh)) + ), ToolPermissionDecision::Allow ); // With always_allow_tool_actions=false, default_mode: Deny is respected assert!(matches!( - ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + "terminal", + "x", + &p, + false, + Some(ShellKind::Posix(PosixShell::Sh)) + ), ToolPermissionDecision::Deny(_) )); assert_eq!( - ToolPermissionDecision::from_input("edit_file", "x", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + "edit_file", + "x", + &p, + false, + Some(ShellKind::Posix(PosixShell::Sh)) + ), ToolPermissionDecision::Allow ); } @@ -635,7 +667,13 @@ mod tests { let p = ToolPermissions { tools }; // "terminal" should not match "term" rules, so falls back to Confirm (no rules) assert_eq!( - ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + "terminal", + "x", + &p, + false, + Some(ShellKind::Posix(PosixShell::Sh)) + ), ToolPermissionDecision::Confirm ); } @@ -663,12 +701,24 @@ mod tests { }; // With global=true, all checks are bypassed including invalid pattern check assert!(matches!( - ToolPermissionDecision::from_input("terminal", "echo hi", &p, true, ShellKind::Posix), + ToolPermissionDecision::from_input( + "terminal", + "echo hi", + &p, + true, + Some(ShellKind::Posix(PosixShell::Sh)) + ), ToolPermissionDecision::Allow )); // With global=false, invalid patterns block the tool assert!(matches!( - ToolPermissionDecision::from_input("terminal", "echo hi", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + "terminal", + "echo hi", + &p, + false, + Some(ShellKind::Posix(PosixShell::Sh)) + ), ToolPermissionDecision::Deny(_) )); } @@ -857,7 +907,13 @@ mod tests { ); let p = ToolPermissions { tools }; assert!(matches!( - ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + "terminal", + "x", + &p, + false, + Some(ShellKind::Posix(PosixShell::Sh)) + ), ToolPermissionDecision::Deny(_) )); assert_eq!( @@ -866,7 +922,7 @@ mod tests { "x", &p, false, - ShellKind::Posix + Some(ShellKind::Posix(PosixShell::Sh)) ), ToolPermissionDecision::Allow ); @@ -900,38 +956,88 @@ mod tests { } #[test] - fn nushell_denies_when_always_allow_configured() { - t("ls").allow(&["^ls"]).shell(ShellKind::Nushell).is_deny(); + fn nushell_allows_with_allow_pattern() { + // Nushell uses `;` for sequential execution, which brush-parser handles. + // The `and`/`or` keywords are boolean operators, not command chaining. + t("ls") + .allow(&["^ls"]) + .shell(Some(ShellKind::Nushell)) + .is_allow(); } #[test] - fn nushell_allows_deny_patterns() { + fn nushell_denies_with_deny_pattern() { t("rm -rf /") .deny(&["rm\\s+-rf"]) - .shell(ShellKind::Nushell) + .shell(Some(ShellKind::Nushell)) .is_deny(); } #[test] - fn nushell_allows_confirm_patterns() { + fn nushell_confirms_with_confirm_pattern() { t("sudo reboot") .confirm(&["sudo"]) - .shell(ShellKind::Nushell) + .shell(Some(ShellKind::Nushell)) .is_confirm(); } #[test] - fn nushell_no_allow_patterns_uses_default() { + fn nushell_falls_through_to_default() { t("ls") .deny(&["rm"]) .mode(ToolPermissionMode::Allow) - .shell(ShellKind::Nushell) + .shell(Some(ShellKind::Nushell)) + .is_allow(); + } + + #[test] + fn elvish_allows_with_allow_pattern() { + // Elvish uses `;` to separate pipelines, which brush-parser handles. + // The `and`/`or` special commands require parentheses around expressions. + t("ls") + .allow(&["^ls"]) + .shell(Some(ShellKind::Elvish)) + .is_allow(); + } + + #[test] + fn rc_allows_with_allow_pattern() { + // Rc (Plan 9) uses `;` for sequential execution, which brush-parser handles. + // Rc does not have `&&`/`||` operators. + t("ls") + .allow(&["^ls"]) + .shell(Some(ShellKind::Rc)) .is_allow(); } #[test] - fn elvish_denies_when_always_allow_configured() { - t("ls").allow(&["^ls"]).shell(ShellKind::Elvish).is_deny(); + fn unknown_shell_denies_when_always_allow_configured() { + // Unknown shells have unrecognized syntax, so we cannot safely parse them. + // For security, always_allow patterns are disabled. + t("ls").allow(&["^ls"]).shell(None).is_deny(); + } + + #[test] + fn unknown_shell_allows_deny_patterns() { + // Deny patterns still work for unknown shells since they're checked + // against the raw input string. + t("rm -rf /").deny(&["rm\\s+-rf"]).shell(None).is_deny(); + } + + #[test] + fn unknown_shell_allows_confirm_patterns() { + // Confirm patterns still work for unknown shells. + t("sudo reboot").confirm(&["sudo"]).shell(None).is_confirm(); + } + + #[test] + fn unknown_shell_falls_through_to_default() { + // With no always_allow patterns, unknown shells use the default mode. + t("ls") + .deny(&["rm"]) + .mode(ToolPermissionMode::Allow) + .shell(None) + .is_allow(); } #[test] @@ -960,8 +1066,13 @@ mod tests { ); let p = ToolPermissions { tools }; - let result = - ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix); + let result = ToolPermissionDecision::from_input( + "terminal", + "x", + &p, + false, + Some(ShellKind::Posix(PosixShell::Sh)), + ); match result { ToolPermissionDecision::Deny(msg) => { assert!(msg.contains("2 regex patterns"), "Expected plural: {}", msg); diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index d4a30c539dc6e0879c2604cbdea1a670e58db36f..1d3adb0d846afa915cc003be8b3b1a76a53635a4 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -195,7 +195,7 @@ impl AcpConnection { cx: &mut AsyncApp, ) -> Result { let shell = cx.update(|cx| TerminalSettings::get(None, cx).shell.clone()); - let builder = ShellBuilder::new(&shell, cfg!(windows)).non_interactive(); + let builder = ShellBuilder::new(&shell).non_interactive(); let mut child = builder.build_std_command(Some(command.path.display().to_string()), &command.args); child.envs(command.env.iter().flatten()); diff --git a/crates/askpass/src/askpass.rs b/crates/askpass/src/askpass.rs index 917b0efc1599112e44d23199cd832d75c4ba30b1..97308dc3335e00b47b3258f35a75fda6c4918e66 100644 --- a/crates/askpass/src/askpass.rs +++ b/crates/askpass/src/askpass.rs @@ -20,7 +20,11 @@ use futures::{ }; use gpui::{AsyncApp, BackgroundExecutor, Task}; use smol::fs; -use util::{ResultExt as _, debug_panic, maybe, paths::PathExt, shell::ShellKind}; +use util::{ + ResultExt as _, debug_panic, maybe, + paths::PathExt, + shell::{PosixShell, ShellKind}, +}; /// Path to the program used for askpass /// @@ -208,7 +212,8 @@ impl PasswordProxy { let shell_kind = if cfg!(windows) { ShellKind::PowerShell } else { - ShellKind::Posix + // TODO: Consider using the user's actual shell instead of hardcoding "sh" + ShellKind::Posix(PosixShell::Sh) }; let askpass_program = ASKPASS_PROGRAM.get_or_init(|| current_exec); // Create an askpass script that communicates back to this process. @@ -354,7 +359,7 @@ fn generate_askpass_script( .try_quote_prefix_aware(&askpass_program) .context("Failed to shell-escape Askpass program path")?; let askpass_socket = askpass_socket - .try_shell_safe(shell_kind) + .try_shell_safe(Some(&shell_kind)) .context("Failed to shell-escape Askpass socket path")?; let print_args = "printf '%s\\0' \"$@\""; let shebang = "#!/bin/sh"; @@ -379,7 +384,7 @@ fn generate_askpass_script( .try_quote_prefix_aware(&askpass_program) .context("Failed to shell-escape Askpass program path")?; let askpass_socket = askpass_socket - .try_shell_safe(shell_kind) + .try_shell_safe(Some(&shell_kind)) .context("Failed to shell-escape Askpass socket path")?; Ok(format!( r#" diff --git a/crates/context_server/src/transport/stdio_transport.rs b/crates/context_server/src/transport/stdio_transport.rs index c3af1aa8745a074ad545cad0518d2ffea2822b65..c9f48d69e089c98356507423989aea633f1b4125 100644 --- a/crates/context_server/src/transport/stdio_transport.rs +++ b/crates/context_server/src/transport/stdio_transport.rs @@ -32,7 +32,7 @@ impl StdioTransport { cx: &AsyncApp, ) -> Result { let shell = cx.update(|cx| TerminalSettings::get(None, cx).shell.clone()); - let builder = ShellBuilder::new(&shell, cfg!(windows)).non_interactive(); + let builder = ShellBuilder::new(&shell).non_interactive(); let mut command = builder.build_smol_command(Some(binary.executable.display().to_string()), &binary.args); diff --git a/crates/dap_adapters/src/javascript.rs b/crates/dap_adapters/src/javascript.rs index 68f5ca7e7976640c5b3e44ec5e2e2b880a6c2407..b5a2e39ecd1fd9fcf24cd70af62914a3f11b9b15 100644 --- a/crates/dap_adapters/src/javascript.rs +++ b/crates/dap_adapters/src/javascript.rs @@ -6,7 +6,10 @@ use gpui::AsyncApp; use serde_json::Value; use std::{path::PathBuf, sync::OnceLock}; use task::DebugRequest; -use util::{ResultExt, maybe, shell::ShellKind}; +use util::{ + ResultExt, maybe, + shell::{PosixShell, ShellKind}, +}; use crate::*; @@ -67,7 +70,10 @@ impl JsDebugAdapter { .get("type") .filter(|value| value == &"node-terminal")?; let command = configuration.get("command")?.as_str()?.to_owned(); - let mut args = ShellKind::Posix.split(&command)?.into_iter(); + // TODO: Consider using the user's actual shell instead of hardcoding "sh" + let mut args = ShellKind::Posix(PosixShell::Sh) + .split(&command)? + .into_iter(); let program = args.next()?; configuration.insert("runtimeExecutable".to_owned(), program.into()); configuration.insert( diff --git a/crates/debugger_ui/src/new_process_modal.rs b/crates/debugger_ui/src/new_process_modal.rs index 982edd7feca720666cf90b3b771e272f628076bd..4ac49d5bb0cc382221f27d5e3b748a65f9603cbe 100644 --- a/crates/debugger_ui/src/new_process_modal.rs +++ b/crates/debugger_ui/src/new_process_modal.rs @@ -28,7 +28,11 @@ use ui::{ prelude::*, }; use ui_input::InputField; -use util::{ResultExt, debug_panic, rel_path::RelPath, shell::ShellKind}; +use util::{ + ResultExt, debug_panic, + rel_path::RelPath, + shell::{PosixShell, ShellKind}, +}; use workspace::{ModalView, Workspace, notifications::DetachAndPromptErr, pane}; use crate::{ @@ -870,7 +874,8 @@ impl ConfigureMode { }; } let command = self.program.read(cx).text(cx); - let mut args = ShellKind::Posix + // TODO: Consider using the user's actual shell instead of hardcoding "sh" + let mut args = ShellKind::Posix(PosixShell::Sh) .split(&command) .into_iter() .flatten() @@ -1298,7 +1303,8 @@ impl PickerDelegate for DebugDelegate { }) .unwrap_or_default(); - let mut args = ShellKind::Posix + // TODO: Consider using the user's actual shell instead of hardcoding "sh" + let mut args = ShellKind::Posix(PosixShell::Sh) .split(&text) .into_iter() .flatten() diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index 94878c3f0c0ffdc910b15297cb6573e5c8cd252e..2d7088870590736c3c24d1aa28321b947c70a40b 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -971,7 +971,6 @@ impl RunningState { let task_store = project.read(cx).task_store().downgrade(); let weak_project = project.downgrade(); let weak_workspace = workspace.downgrade(); - let is_windows = project.read(cx).path_style(cx).is_windows(); let remote_shell = project .read(cx) .remote_client() @@ -1089,7 +1088,7 @@ impl RunningState { task.resolved.shell = Shell::Program(remote_shell); } - let builder = ShellBuilder::new(&task.resolved.shell, is_windows); + let builder = ShellBuilder::new(&task.resolved.shell); let command_label = builder.command_label(task.resolved.command.as_deref().unwrap_or("")); let (command, args) = builder.build(task.resolved.command.clone(), &task.resolved.args); diff --git a/crates/language/src/toolchain.rs b/crates/language/src/toolchain.rs index 0d80f84e7ec1dc330db823a0938421a1f5ad85c9..33ac65caf342e59b484595ec5564427ccd67605b 100644 --- a/crates/language/src/toolchain.rs +++ b/crates/language/src/toolchain.rs @@ -117,7 +117,7 @@ pub trait ToolchainLister: Send + Sync + 'static { fn activation_script( &self, toolchain: &Toolchain, - shell: ShellKind, + shell: Option, cx: &App, ) -> BoxFuture<'static, Vec>; diff --git a/crates/languages/src/python.rs b/crates/languages/src/python.rs index 0efaa3f0964a28711ccb3e55a3c75825cd3a0b83..d0402f96d91b9a00791b0707d6e96b583dce7312 100644 --- a/crates/languages/src/python.rs +++ b/crates/languages/src/python.rs @@ -46,7 +46,7 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; -use task::{ShellKind, TaskTemplate, TaskTemplates, VariableName}; +use task::{PosixShell, ShellKind, TaskTemplate, TaskTemplates, VariableName}; use util::{ResultExt, maybe}; #[derive(Debug, Serialize, Deserialize)] @@ -1168,13 +1168,15 @@ fn wr_distance( } } -fn micromamba_shell_name(kind: ShellKind) -> &'static str { +fn micromamba_shell_name(kind: Option) -> &'static str { match kind { - ShellKind::Csh => "csh", - ShellKind::Fish => "fish", - ShellKind::Nushell => "nu", - ShellKind::PowerShell => "powershell", - ShellKind::Cmd => "cmd.exe", + Some(ShellKind::Csh) => "csh", + Some(ShellKind::Fish) => "fish", + Some(ShellKind::Nushell) => "nu", + Some(ShellKind::PowerShell) | Some(ShellKind::Pwsh) => "powershell", + Some(ShellKind::Cmd) => "cmd.exe", + #[cfg(windows)] + None => "powershell", // default / catch-all: _ => "posix", } @@ -1339,7 +1341,7 @@ impl ToolchainLister for PythonToolchainProvider { fn activation_script( &self, toolchain: &Toolchain, - shell: ShellKind, + shell: Option, cx: &App, ) -> BoxFuture<'static, Vec> { let settings = TerminalSettings::get_global(cx); @@ -1406,10 +1408,21 @@ impl ToolchainLister for PythonToolchainProvider { | PythonEnvironmentKind::Poetry, ) => { if let Some(activation_scripts) = &toolchain.activation_scripts { - if let Some(activate_script_path) = activation_scripts.get(&shell) { - let activate_keyword = shell.activate_keyword(); + // For unknown shells, fall back to platform-appropriate defaults: + // POSIX (sh) on Unix, PowerShell on Windows + #[cfg(unix)] + let fallback_shell = ShellKind::Posix(PosixShell::Sh); + #[cfg(windows)] + let fallback_shell = ShellKind::PowerShell; + + let shell_kind = shell.unwrap_or(fallback_shell); + // Use activation_script_key() to normalize POSIX shells (e.g., Bash, Zsh) + // to Posix(Sh) for lookup, since activation scripts are stored with Posix(Sh) key + let lookup_key = shell_kind.activation_script_key(); + if let Some(activate_script_path) = activation_scripts.get(&lookup_key) { + let activate_keyword = shell_kind.activate_keyword(); if let Some(quoted) = - shell.try_quote(&activate_script_path.to_string_lossy()) + shell_kind.try_quote(&activate_script_path.to_string_lossy()) { activation_script.push(format!("{activate_keyword} {quoted}")); } @@ -1423,17 +1436,27 @@ impl ToolchainLister for PythonToolchainProvider { let version = toolchain.environment.version.as_deref().unwrap_or("system"); let pyenv = &manager.executable; let pyenv = pyenv.display(); + let pyenv_sh_activation = format!("\"{pyenv}\" shell - sh {version}"); activation_script.extend(match shell { - ShellKind::Fish => Some(format!("\"{pyenv}\" shell - fish {version}")), - ShellKind::Posix => Some(format!("\"{pyenv}\" shell - sh {version}")), - ShellKind::Nushell => Some(format!("^\"{pyenv}\" shell - nu {version}")), - ShellKind::PowerShell | ShellKind::Pwsh => None, - ShellKind::Csh => None, - ShellKind::Tcsh => None, - ShellKind::Cmd => None, - ShellKind::Rc => None, - ShellKind::Xonsh => None, - ShellKind::Elvish => None, + Some(ShellKind::Fish) => { + Some(format!("\"{pyenv}\" shell - fish {version}")) + } + Some(ShellKind::Posix(_)) => Some(pyenv_sh_activation), + #[cfg(unix)] + None => Some(pyenv_sh_activation), + Some(ShellKind::Nushell) => { + Some(format!("^\"{pyenv}\" shell - nu {version}")) + } + Some(ShellKind::PowerShell) + | Some(ShellKind::Pwsh) + | Some(ShellKind::Csh) + | Some(ShellKind::Tcsh) + | Some(ShellKind::Cmd) + | Some(ShellKind::Rc) + | Some(ShellKind::Xonsh) + | Some(ShellKind::Elvish) => None, + #[cfg(windows)] + None => None, }) } _ => {} @@ -1497,8 +1520,9 @@ async fn resolve_venv_activation_scripts( ) { log::debug!("(Python) Resolving activation scripts for venv toolchain {venv:?}"); if let Some(prefix) = &venv.prefix { + // TODO: Consider using the user's actual shell instead of hardcoding "sh" for (shell_kind, script_name) in &[ - (ShellKind::Posix, "activate"), + (ShellKind::Posix(PosixShell::Sh), "activate"), (ShellKind::Rc, "activate"), (ShellKind::Csh, "activate.csh"), (ShellKind::Tcsh, "activate.csh"), diff --git a/crates/project/src/debugger/locators/cargo.rs b/crates/project/src/debugger/locators/cargo.rs index 66af7a2fe62fa06478680d23dd35575a64fd3c0f..063480120ccfd514344a59efbf67b6673384dad3 100644 --- a/crates/project/src/debugger/locators/cargo.rs +++ b/crates/project/src/debugger/locators/cargo.rs @@ -122,7 +122,7 @@ impl DapLocator for CargoLocator { .cwd .clone() .context("Couldn't get cwd from debug config which is needed for locators")?; - let builder = ShellBuilder::new(&build_config.shell, cfg!(windows)).non_interactive(); + let builder = ShellBuilder::new(&build_config.shell).non_interactive(); let mut child = builder .build_smol_command( Some("cargo".into()), diff --git a/crates/project/src/lsp_store/lsp_ext_command.rs b/crates/project/src/lsp_store/lsp_ext_command.rs index 270db67576f0a02155997757a01d489d44ef1766..9bd02c318a2f3742079ee759ee1a70f72460f571 100644 --- a/crates/project/src/lsp_store/lsp_ext_command.rs +++ b/crates/project/src/lsp_store/lsp_ext_command.rs @@ -657,7 +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)); + let shell_kind = task_template.shell.shell_kind(); task_template.args.push("--".to_string()); task_template.args.extend( cargo @@ -683,7 +683,8 @@ 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| { - shell_kind.try_quote(&extra_arg).map(|s| s.to_string()) + util::shell::try_quote_option(shell_kind, &extra_arg) + .map(|s| s.to_string()) }), ); } diff --git a/crates/project/src/terminals.rs b/crates/project/src/terminals.rs index 5ab3c4c36214d744aff7d24f3a3877ee91b1e7d4..0a1f6ed266ab4cbbca47da26ea744e869d93cfaf 100644 --- a/crates/project/src/terminals.rs +++ b/crates/project/src/terminals.rs @@ -12,11 +12,12 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; -use task::{Shell, ShellBuilder, ShellKind, SpawnInTerminal}; +use task::{Shell, ShellBuilder, SpawnInTerminal}; use terminal::{ TaskState, TaskStatus, Terminal, TerminalBuilder, insert_zed_terminal_env, terminal_settings::TerminalSettings, }; +use util::shell::ShellKind; use util::{command::new_std_command, get_default_system_shell, maybe, rel_path::RelPath}; use crate::{Project, ProjectPath}; @@ -92,7 +93,7 @@ impl Project { None => settings.shell.program(), }; let path_style = self.path_style(cx); - let shell_kind = ShellKind::new(&shell, path_style.is_windows()); + let shell_kind = ShellKind::new(&shell); // Prepare a task for resolving the environment let env_task = @@ -142,12 +143,14 @@ impl Project { .update(cx, move |_, cx| { let format_to_run = || { if let Some(command) = &spawn_task.command { - let command = shell_kind.prepend_command_prefix(command); - let command = shell_kind.try_quote_prefix_aware(&command); + let command = + util::shell::prepend_command_prefix_option(shell_kind, command); + let command = + util::shell::try_quote_prefix_aware_option(shell_kind, &command); let args = spawn_task .args .iter() - .filter_map(|arg| shell_kind.try_quote(&arg)); + .filter_map(|arg| util::shell::try_quote_option(shell_kind, arg)); command.into_iter().chain(args).join(" ") } else { @@ -161,13 +164,17 @@ impl Project { match remote_client { Some(remote_client) => match activation_script.clone() { activation_script if !activation_script.is_empty() => { - let separator = shell_kind.sequential_commands_separator(); + let separator = + util::shell::sequential_commands_separator_option( + shell_kind, + ); let activation_script = activation_script.join(&format!("{separator} ")); let to_run = format_to_run(); let arg = format!("{activation_script}{separator} {to_run}"); - let args = shell_kind.args_for_shell(false, arg); + let args = + util::shell::args_for_shell_option(shell_kind, false, arg); let shell = remote_client .read(cx) .shell() @@ -194,13 +201,17 @@ impl Project { }, None => match activation_script.clone() { activation_script if !activation_script.is_empty() => { - let separator = shell_kind.sequential_commands_separator(); + let separator = + util::shell::sequential_commands_separator_option( + shell_kind, + ); let activation_script = activation_script.join(&format!("{separator} ")); let to_run = format_to_run(); let arg = format!("{activation_script}{separator} {to_run}"); - let args = shell_kind.args_for_shell(false, arg); + let args = + util::shell::args_for_shell_option(shell_kind, false, arg); ( Shell::WithArguments { @@ -358,7 +369,7 @@ impl Project { let lang_registry = self.languages.clone(); cx.spawn(async move |project, cx| { - let shell_kind = ShellKind::new(&shell, path_style.is_windows()); + let shell_kind = ShellKind::new(&shell); let mut env = env_task.await.unwrap_or_default(); env.extend(settings.env); @@ -513,8 +524,7 @@ impl Project { .and_then(|remote_client| remote_client.read(cx).shell()) .map(Shell::Program) .unwrap_or_else(|| settings.shell.clone()); - let is_windows = self.path_style(cx).is_windows(); - let builder = ShellBuilder::new(&shell, is_windows).non_interactive(); + let builder = ShellBuilder::new(&shell).non_interactive(); let (command, args) = builder.build(Some(command), &Vec::new()); let env_task = self.resolve_directory_environment( diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index dabd4282dee96a5952bb3e903bdfb54bf54d36f8..dd6ca5cc046d491d69ea4b4296e3111677d2eb03 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -11266,7 +11266,7 @@ fn python_lang(fs: Arc) -> Arc { fn activation_script( &self, _: &Toolchain, - _: ShellKind, + _: Option, _: &gpui::App, ) -> futures::future::BoxFuture<'static, Vec> { Box::pin(async { vec![] }) diff --git a/crates/prompt_store/src/prompts.rs b/crates/prompt_store/src/prompts.rs index 6a845bb8dd394f8a1ff26a8a0e130156a2a158bd..b85b97bccbe00fb272619cd8b358445017ac40c4 100644 --- a/crates/prompt_store/src/prompts.rs +++ b/crates/prompt_store/src/prompts.rs @@ -57,8 +57,12 @@ impl ProjectContext { user_rules: default_user_rules, os: std::env::consts::OS.to_string(), arch: std::env::consts::ARCH.to_string(), - shell: ShellKind::new(&get_default_system_shell_preferring_bash(), cfg!(windows)) - .to_string(), + shell: ShellKind::new_with_fallback( + &get_default_system_shell_preferring_bash(), + cfg!(windows), + ) + .name() + .to_string(), } } } diff --git a/crates/remote/src/transport/docker.rs b/crates/remote/src/transport/docker.rs index 713e532704063a244b94a7f2fb0fbd5a47727f23..d709c619765b37894735d09812fa4c7e23d18a29 100644 --- a/crates/remote/src/transport/docker.rs +++ b/crates/remote/src/transport/docker.rs @@ -13,7 +13,7 @@ use std::{ sync::Arc, }; use util::ResultExt; -use util::shell::ShellKind; +use util::shell::{PosixShell, ShellKind}; use util::{ paths::{PathStyle, RemotePathBuf}, rel_path::RelPath, @@ -301,7 +301,8 @@ impl DockerExecConnection { delegate.set_status(Some("Extracting remote development server"), cx); let server_mode = 0o755; - let shell_kind = ShellKind::Posix; + // TODO: Consider using the remote's actual shell instead of hardcoding "sh" + let shell_kind = ShellKind::Posix(PosixShell::Sh); let orig_tmp_path = tmp_path.display(self.path_style()); let server_mode = format!("{:o}", server_mode); let server_mode = shell_kind diff --git a/crates/remote/src/transport/ssh.rs b/crates/remote/src/transport/ssh.rs index e93ed18892be6bb5b4515b67ffb902fb6244301d..8924f499a0257184287a24afb2d58f38405d023c 100644 --- a/crates/remote/src/transport/ssh.rs +++ b/crates/remote/src/transport/ssh.rs @@ -32,7 +32,7 @@ use tempfile::TempDir; use util::{ paths::{PathStyle, RemotePathBuf}, rel_path::RelPath, - shell::ShellKind, + shell::{PosixShell, ShellKind}, }; pub(crate) struct SshRemoteConnection { @@ -591,7 +591,7 @@ impl SshRemoteConnection { let ssh_shell = socket.shell(is_windows).await; log::info!("Remote shell discovered: {}", ssh_shell); - let ssh_shell_kind = ShellKind::new(&ssh_shell, is_windows); + let ssh_shell_kind = ShellKind::new_with_fallback(&ssh_shell, is_windows); let ssh_platform = socket.platform(ssh_shell_kind, is_windows).await?; log::info!("Remote platform discovered: {:?}", ssh_platform); @@ -918,7 +918,7 @@ impl SshRemoteConnection { dst_path: &RelPath, tmp_path: &RelPath, ) -> Result<()> { - let shell_kind = ShellKind::Posix; + let shell_kind = self.ssh_shell_kind; let server_mode = 0o755; let orig_tmp_path = tmp_path.display(self.path_style()); let server_mode = format!("{:o}", server_mode); @@ -1138,7 +1138,6 @@ impl SshSocket { .expect("shell quoting") .into_owned(); for arg in args { - // We're trying to work with: sh, bash, zsh, fish, tcsh, ...? debug_assert!( !arg.as_ref().contains('\n'), "multiline arguments do not work in all shells" @@ -1291,8 +1290,14 @@ impl SshSocket { async fn shell_posix(&self) -> String { const DEFAULT_SHELL: &str = "sh"; + // TODO: Consider using the user's actual shell instead of hardcoding "sh" match self - .run_command(ShellKind::Posix, "sh", &["-c", "echo $SHELL"], false) + .run_command( + ShellKind::Posix(PosixShell::Sh), + "sh", + &["-c", "echo $SHELL"], + false, + ) .await { Ok(output) => parse_shell(&output, DEFAULT_SHELL), @@ -1386,7 +1391,8 @@ impl SshConnectionOptions { "-w", ]; - let mut tokens = ShellKind::Posix + // TODO: Consider using the user's actual shell instead of hardcoding "sh" + let mut tokens = ShellKind::Posix(PosixShell::Sh) .split(input) .context("invalid input")? .into_iter(); @@ -1624,7 +1630,7 @@ fn build_command_posix( .context("shell quoting")? )?; for arg in input_args { - let arg = ssh_shell_kind.try_quote(&arg).context("shell quoting")?; + let arg = ssh_shell_kind.try_quote(arg).context("shell quoting")?; write!(exec, " {}", &arg)?; } } else { @@ -1666,7 +1672,7 @@ fn build_command_windows( ssh_env: HashMap, ssh_path_style: PathStyle, ssh_shell: &str, - _ssh_shell_kind: ShellKind, + ssh_shell_kind: ShellKind, ssh_args: Vec, interactive: Interactive, ) -> Result { @@ -1685,7 +1691,7 @@ fn build_command_windows( shell_kind .try_quote(&working_dir) .context("shell quoting")?, - shell_kind.sequential_and_commands_separator() + ssh_shell_kind.sequential_and_commands_separator() )?; } @@ -1772,7 +1778,7 @@ mod tests { env.clone(), PathStyle::Posix, "/bin/bash", - ShellKind::Posix, + ShellKind::Posix(PosixShell::Bash), vec!["-o".to_string(), "ControlMaster=auto".to_string()], Interactive::No, )?; diff --git a/crates/remote/src/transport/wsl.rs b/crates/remote/src/transport/wsl.rs index 2a1b5d5ce144421af8516154fe7869914041a743..cad8e43e0427698b7e5f1f41f2767bb9d05f771d 100644 --- a/crates/remote/src/transport/wsl.rs +++ b/crates/remote/src/transport/wsl.rs @@ -23,7 +23,7 @@ use std::{ use util::{ paths::{PathStyle, RemotePathBuf}, rel_path::RelPath, - shell::{Shell, ShellKind}, + shell::{PosixShell, Shell, ShellKind}, shell_builder::ShellBuilder, }; @@ -75,7 +75,8 @@ impl WslRemoteConnection { arch: RemoteArch::X86_64, }, shell: String::new(), - shell_kind: ShellKind::Posix, + // TODO: Consider using the user's actual shell instead of hardcoding "sh" + shell_kind: ShellKind::Posix(PosixShell::Sh), default_system_shell: String::from("/bin/sh"), has_wsl_interop: false, }; @@ -85,7 +86,8 @@ impl WslRemoteConnection { .await .context("failed detecting shell")?; log::info!("Remote shell discovered: {}", this.shell); - this.shell_kind = ShellKind::new(&this.shell, false); + // WSL is always Linux, so is_windows = false + this.shell_kind = ShellKind::new_with_fallback(&this.shell, false); this.has_wsl_interop = this.detect_has_wsl_interop().await.unwrap_or_default(); log::info!( "Remote has wsl interop {}", @@ -468,7 +470,7 @@ impl RemoteConnection for WslRemoteConnection { write!(&mut exec, "{} -l", self.shell)?; } let (command, args) = - ShellBuilder::new(&Shell::Program(self.shell.clone()), false).build(Some(exec), &[]); + ShellBuilder::new(&Shell::Program(self.shell.clone())).build(Some(exec), &[]); let mut wsl_args = if let Some(user) = &self.connection_options.user { vec![ diff --git a/crates/task/src/task.rs b/crates/task/src/task.rs index 92d59094190de9924327722bb659058569869269..a90cd2b8bdeec359ea2d6ced586e00c73833bdbd 100644 --- a/crates/task/src/task.rs +++ b/crates/task/src/task.rs @@ -25,7 +25,7 @@ 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::{PosixShell, Shell, ShellKind}; pub use util::shell_builder::ShellBuilder; pub use vscode_debug_format::VsCodeDebugTaskFile; pub use vscode_format::VsCodeTaskFile; diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 4026a047df80627fbc7c7d96d9da605c2464e259..1bcc912339e6b8cb361b249c21f3b42b02095924 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -516,7 +516,7 @@ impl TerminalBuilder { // the compilation target. This is fine right now due to the restricted // way we use the return value, but would become incorrect if we // supported remoting into windows. - let shell_kind = shell.shell_kind(cfg!(windows)); + let shell_kind = shell.shell_kind(); let pty_options = { let alac_shell = shell_params.as_ref().map(|params| { @@ -532,7 +532,7 @@ impl TerminalBuilder { drain_on_exit: true, env: env.clone().into_iter().collect(), #[cfg(windows)] - escape_args: shell_kind.tty_escape_args(), + escape_args: shell_kind.map(|k| k.tty_escape_args()).unwrap_or(true), } }; @@ -664,7 +664,10 @@ impl TerminalBuilder { // and while we have sent the activation script to the pty, it will be executed asynchronously. // Therefore, we somehow need to wait for the activation script to finish executing before we // can proceed with clearing the screen. - terminal.write_to_pty(shell_kind.clear_screen_command().as_bytes()); + let clear_cmd = shell_kind + .map(|k| k.clear_screen_command()) + .unwrap_or("clear"); + terminal.write_to_pty(clear_cmd.as_bytes()); // Simulate enter key press terminal.write_to_pty(b"\x0d"); } @@ -2558,7 +2561,7 @@ mod tests { let (completion_tx, completion_rx) = smol::channel::unbounded(); let args: Vec = args.iter().map(|s| s.to_string()).collect(); let (program, args) = - ShellBuilder::new(&Shell::System, false).build(Some(command.to_owned()), &args); + ShellBuilder::new(&Shell::System).build(Some(command.to_owned()), &args); let builder = cx .update(|cx| { TerminalBuilder::new( @@ -2779,7 +2782,7 @@ mod tests { cx.executor().allow_parking(); let (completion_tx, completion_rx) = smol::channel::unbounded(); - let (program, args) = ShellBuilder::new(&Shell::System, false) + let (program, args) = ShellBuilder::new(&Shell::System) .build(Some("asdasdasdasd".to_owned()), &["@@@@@".to_owned()]); let builder = cx .update(|cx| { diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index cafb7f7da81416f1b60162adc3d3b8adf39375d0..de0a4f712517406a834c2cdebffe452bfbc46f30 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -562,7 +562,6 @@ impl TerminalPanel { } let remote_client = project.remote_client(); - let is_windows = project.path_style(cx).is_windows(); let remote_shell = remote_client .as_ref() .and_then(|remote_client| remote_client.read(cx).shell()); @@ -575,7 +574,7 @@ impl TerminalPanel { task.shell.clone() }; - let task = prepare_task_for_spawn(task, &shell, is_windows); + let task = prepare_task_for_spawn(task, &shell); if task.allow_concurrent_runs && task.use_new_terminal { return self.spawn_in_new_terminal(task, window, cx); @@ -1136,12 +1135,8 @@ impl TerminalPanel { /// Prepares a `SpawnInTerminal` by computing the command, args, and command_label /// based on the shell configuration. This is a pure function that can be tested /// without spawning actual terminals. -pub fn prepare_task_for_spawn( - task: &SpawnInTerminal, - shell: &Shell, - is_windows: bool, -) -> SpawnInTerminal { - let builder = ShellBuilder::new(shell, is_windows); +pub fn prepare_task_for_spawn(task: &SpawnInTerminal, shell: &Shell) -> SpawnInTerminal { + let builder = ShellBuilder::new(shell); let command_label = builder.command_label(task.command.as_deref().unwrap_or("")); let (command, args) = builder.build_no_quote(task.command.clone(), &task.args); @@ -1833,7 +1828,7 @@ mod tests { let input = SpawnInTerminal::default(); let shell = Shell::System; - let result = prepare_task_for_spawn(&input, &shell, false); + let result = prepare_task_for_spawn(&input, &shell); let expected_shell = util::get_system_shell(); assert_eq!(result.env, HashMap::default()); @@ -1905,7 +1900,7 @@ mod tests { }; let shell = Shell::System; - let result = prepare_task_for_spawn(&input, &shell, false); + let result = prepare_task_for_spawn(&input, &shell); let system_shell = util::get_system_shell(); assert_eq!(result.env, HashMap::default()); diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index f010ff57c636f40de50a06baefd99c9ee43728f9..17be735cf7638d3130ce848976411084abde5f3a 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -86,7 +86,7 @@ pub trait PathExt { fn multiple_extensions(&self) -> Option; /// Try to make a shell-safe representation of the path. - fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result; + fn try_shell_safe(&self, shell_kind: Option<&ShellKind>) -> anyhow::Result; } impl> PathExt for T { @@ -164,13 +164,19 @@ impl> PathExt for T { Some(parts.into_iter().join(".")) } - fn try_shell_safe(&self, shell_kind: ShellKind) -> anyhow::Result { + fn try_shell_safe(&self, shell_kind: Option<&ShellKind>) -> anyhow::Result { let path_str = self .as_ref() .to_str() .with_context(|| "Path contains invalid UTF-8")?; - shell_kind - .try_quote(path_str) + let quoted = match shell_kind { + Some(kind) => kind.try_quote(path_str), + #[cfg(windows)] + None => Some(ShellKind::quote_powershell(path_str)), + #[cfg(unix)] + None => shlex::try_quote(path_str).ok(), + }; + quoted .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 ecac8eaf6aa7d13148d6a5c0770bcedc351c14cf..2d6156d6b0f5c072b115829bfa88ef6704ce192d 100644 --- a/crates/util/src/shell.rs +++ b/crates/util/src/shell.rs @@ -1,6 +1,6 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use std::{borrow::Cow, fmt, path::Path, sync::LazyLock}; +use std::{borrow::Cow, ffi::OsStr, path::Path, sync::LazyLock}; /// Shell configuration to open the terminal with. #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, Hash)] @@ -39,19 +39,45 @@ impl Shell { } } - pub fn shell_kind(&self, is_windows: bool) -> ShellKind { + pub fn shell_kind(&self) -> Option { match self { - Shell::Program(program) => ShellKind::new(program, is_windows), - Shell::WithArguments { program, .. } => ShellKind::new(program, is_windows), + Shell::Program(program) => ShellKind::new(program), + Shell::WithArguments { program, .. } => ShellKind::new(program), Shell::System => ShellKind::system(), } } } +/// Specific POSIX-compatible shell variants. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub enum ShellKind { +pub enum PosixShell { #[default] - Posix, + Sh, + Bash, + Zsh, + Dash, + Ksh, + Mksh, + Ash, +} + +impl PosixShell { + pub fn name(&self) -> &'static str { + match self { + PosixShell::Sh => "sh", + PosixShell::Bash => "bash", + PosixShell::Zsh => "zsh", + PosixShell::Dash => "dash", + PosixShell::Ksh => "ksh", + PosixShell::Mksh => "mksh", + PosixShell::Ash => "ash", + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub enum ShellKind { + Posix(PosixShell), Csh, Tcsh, Rc, @@ -233,84 +259,127 @@ pub fn get_windows_system_shell() -> String { (*SYSTEM_SHELL).clone() } -impl fmt::Display for ShellKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl ShellKind { + /// Returns the canonical shell kind for activation script lookups. + /// + /// This normalizes all POSIX shell variants to `Posix(Sh)` so that + /// activation scripts stored with a single POSIX key can be found + /// regardless of which specific POSIX shell the user has. + pub fn activation_script_key(&self) -> ShellKind { match self { - ShellKind::Posix => write!(f, "sh"), - ShellKind::Csh => write!(f, "csh"), - ShellKind::Tcsh => write!(f, "tcsh"), - ShellKind::Fish => write!(f, "fish"), - ShellKind::PowerShell => write!(f, "powershell"), - ShellKind::Pwsh => write!(f, "pwsh"), - ShellKind::Nushell => write!(f, "nu"), - ShellKind::Cmd => write!(f, "cmd"), - ShellKind::Rc => write!(f, "rc"), - ShellKind::Xonsh => write!(f, "xonsh"), - ShellKind::Elvish => write!(f, "elvish"), + ShellKind::Posix(_) => ShellKind::Posix(PosixShell::Sh), + other => *other, } } -} -impl ShellKind { - pub fn system() -> Self { - Self::new(&get_system_shell(), cfg!(windows)) + /// Creates a ShellKind from a program path, with a platform-aware fallback for unknown shells. + /// + /// Unlike `new()` which returns `None` for unrecognized shells, this method + /// falls back to `PowerShell` when `is_windows` is true, or `Posix(Sh)` otherwise. + /// This is useful for remote connections where we know the target platform but + /// may not recognize the specific shell. + pub fn new_with_fallback(program: impl AsRef, is_windows: bool) -> Self { + Self::new(program).unwrap_or_else(|| { + if is_windows { + ShellKind::PowerShell + } else { + ShellKind::Posix(PosixShell::Sh) + } + }) } - /// Returns whether this shell uses POSIX-like command chaining syntax (`&&`, `||`, `;`, `|`). + /// Returns the name of the shell. + pub fn name(&self) -> &'static str { + match self { + ShellKind::Posix(posix) => posix.name(), + ShellKind::Csh => "csh", + ShellKind::Tcsh => "tcsh", + ShellKind::Fish => "fish", + ShellKind::PowerShell => "powershell", + ShellKind::Pwsh => "pwsh", + ShellKind::Nushell => "nu", + ShellKind::Cmd => "cmd", + ShellKind::Rc => "rc", + ShellKind::Xonsh => "xonsh", + ShellKind::Elvish => "elvish", + } + } + + pub fn system() -> Option { + Self::new(&get_system_shell()) + } + + /// Returns whether this shell's command chaining syntax can be parsed by brush-parser. /// /// This is used to determine if we can safely parse shell commands to extract sub-commands /// for security purposes (e.g., preventing shell injection in "always allow" patterns). /// - /// **Compatible shells:** Posix (sh, bash, dash, zsh), Fish 3.0+, PowerShell 7+/Pwsh, - /// Cmd, Xonsh, Csh, Tcsh + /// The brush-parser handles `;` (sequential execution) and `|` (piping), which are + /// supported by all common shells. It also handles `&&` and `||` for conditional + /// execution, `$()` and backticks for command substitution, and process substitution. + /// + /// # Security Note + /// + /// Only explicitly recognized shells return `true`. Unknown shells (None) return `false` to + /// prevent security bypasses - if we don't know a shell's syntax, we can't safely + /// parse it for `always_allow` patterns. + /// + /// # Shell Notes /// - /// **Incompatible shells:** Nushell (uses `and`/`or` keywords), Elvish (uses `and`/`or` - /// keywords), Rc (Plan 9 shell - no `&&`/`||` operators) + /// - **Nushell**: Uses `;` for sequential execution. The `and`/`or` keywords are boolean + /// operators on values (e.g., `$true and $false`), not command chaining operators. + /// - **Elvish**: Uses `;` to separate pipelines, which brush-parser handles. Elvish does + /// not have `&&` or `||` operators. Its `and`/`or` are special commands that operate + /// on values, not command chaining (e.g., `and $true $false`). + /// - **Rc (Plan 9)**: Uses `;` for sequential execution and `|` for piping. Does not + /// have `&&`/`||` operators for conditional chaining. pub fn supports_posix_chaining(&self) -> bool { - matches!( - self, - ShellKind::Posix - | ShellKind::Fish - | ShellKind::PowerShell - | ShellKind::Pwsh - | ShellKind::Cmd - | ShellKind::Xonsh - | ShellKind::Csh - | ShellKind::Tcsh - ) - } - - pub fn new(program: impl AsRef, is_windows: bool) -> Self { - let program = program.as_ref(); - let program = program - .file_stem() - .unwrap_or_else(|| program.as_os_str()) - .to_string_lossy(); - - match &*program { - "powershell" => ShellKind::PowerShell, - "pwsh" => ShellKind::Pwsh, - "cmd" => ShellKind::Cmd, - "nu" => ShellKind::Nushell, - "fish" => ShellKind::Fish, - "csh" => ShellKind::Csh, - "tcsh" => ShellKind::Tcsh, - "rc" => ShellKind::Rc, - "xonsh" => ShellKind::Xonsh, - "elvish" => ShellKind::Elvish, - "sh" | "bash" | "zsh" => ShellKind::Posix, - _ if is_windows => ShellKind::PowerShell, - // Some other shell detected, the user might install and use a - // unix-like shell. - _ => ShellKind::Posix, - } - } - - pub fn to_shell_variable(self, input: &str) -> String { + match self { + ShellKind::Posix(_) + | ShellKind::Fish + | ShellKind::PowerShell + | ShellKind::Pwsh + | ShellKind::Cmd + | ShellKind::Xonsh + | ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Nushell + | ShellKind::Elvish + | ShellKind::Rc => true, + } + } + + pub fn new(program: impl AsRef) -> Option { + match program.as_ref().file_stem().and_then(OsStr::to_str)? { + "powershell" => Some(ShellKind::PowerShell), + "pwsh" => Some(ShellKind::Pwsh), + "cmd" => Some(ShellKind::Cmd), + "nu" => Some(ShellKind::Nushell), + "fish" => Some(ShellKind::Fish), + "csh" => Some(ShellKind::Csh), + "tcsh" => Some(ShellKind::Tcsh), + "rc" => Some(ShellKind::Rc), + "xonsh" => Some(ShellKind::Xonsh), + "elvish" => Some(ShellKind::Elvish), + "sh" => Some(ShellKind::Posix(PosixShell::Sh)), + "bash" => Some(ShellKind::Posix(PosixShell::Bash)), + "zsh" => Some(ShellKind::Posix(PosixShell::Zsh)), + "dash" => Some(ShellKind::Posix(PosixShell::Dash)), + "ksh" => Some(ShellKind::Posix(PosixShell::Ksh)), + "mksh" => Some(ShellKind::Posix(PosixShell::Mksh)), + "ash" => Some(ShellKind::Posix(PosixShell::Ash)), + // Unrecognized shell - we cannot safely parse its syntax for + // `always_allow` patterns, so they will be disabled. + // Fall back to platform-specific behavior for non-security purposes. + _ => None, + } + } + + pub fn to_shell_variable(&self, input: &str) -> String { match self { Self::PowerShell | Self::Pwsh => Self::to_powershell_variable(input), Self::Cmd => Self::to_cmd_variable(input), - Self::Posix => input.to_owned(), + Self::Posix(_) => input.to_owned(), Self::Fish => input.to_owned(), Self::Csh => input.to_owned(), Self::Tcsh => input.to_owned(), @@ -340,7 +409,7 @@ impl ShellKind { } } - fn to_powershell_variable(input: &str) -> String { + pub fn to_powershell_variable(input: &str) -> String { if let Some(var_str) = input.strip_prefix("${") { if var_str.find(':').is_none() { // If the input starts with "${", remove the trailing "}" @@ -436,13 +505,15 @@ impl ShellKind { pub fn args_for_shell(&self, interactive: bool, combined_command: String) -> Vec { match self { - ShellKind::PowerShell | ShellKind::Pwsh => vec!["-C".to_owned(), combined_command], + ShellKind::PowerShell | ShellKind::Pwsh => { + vec!["-C".to_owned(), combined_command] + } ShellKind::Cmd => vec![ "/S".to_owned(), "/C".to_owned(), format!("\"{combined_command}\""), ], - ShellKind::Posix + ShellKind::Posix(_) | ShellKind::Nushell | ShellKind::Fish | ShellKind::Csh @@ -461,7 +532,7 @@ impl ShellKind { match self { ShellKind::PowerShell | ShellKind::Pwsh => Some('&'), ShellKind::Nushell => Some('^'), - ShellKind::Posix + ShellKind::Posix(_) | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -484,7 +555,7 @@ impl ShellKind { pub const fn sequential_commands_separator(&self) -> char { match self { ShellKind::Cmd => '&', - ShellKind::Posix + ShellKind::Posix(_) | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -500,7 +571,7 @@ impl ShellKind { pub const fn sequential_and_commands_separator(&self) -> &'static str { match self { ShellKind::Cmd - | ShellKind::Posix + | ShellKind::Posix(_) | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -516,7 +587,7 @@ impl ShellKind { ShellKind::PowerShell => Some(Self::quote_powershell(arg)), ShellKind::Pwsh => Some(Self::quote_pwsh(arg)), ShellKind::Cmd => Some(Self::quote_cmd(arg)), - ShellKind::Posix + ShellKind::Posix(_) | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -527,7 +598,7 @@ impl ShellKind { } } - fn quote_windows(arg: &str, enclose: bool) -> Cow<'_, str> { + pub fn quote_windows(arg: &str, enclose: bool) -> Cow<'_, str> { if arg.is_empty() { return Cow::Borrowed("\"\""); } @@ -739,7 +810,7 @@ impl ShellKind { ShellKind::Fish | ShellKind::Csh | ShellKind::Tcsh - | ShellKind::Posix + | ShellKind::Posix(_) | ShellKind::Rc | ShellKind::Xonsh | ShellKind::Elvish => "source", @@ -749,7 +820,7 @@ impl ShellKind { pub const fn clear_screen_command(&self) -> &'static str { match self { ShellKind::Cmd => "cls", - ShellKind::Posix + ShellKind::Posix(_) | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -768,7 +839,7 @@ impl ShellKind { pub const fn tty_escape_args(&self) -> bool { match self { ShellKind::Cmd => false, - ShellKind::Posix + ShellKind::Posix(_) | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc @@ -782,6 +853,109 @@ impl ShellKind { } } +// Helper functions for `Option` that provide platform-specific defaults for unknown shells. +// These should be used when you have an `Option` and need to handle the `None` case +// with appropriate fallback behavior (POSIX-like on Unix, PowerShell-like on Windows). + +/// Quote an argument for the shell, with platform-specific fallback for unknown shells. +pub fn try_quote_option(shell_kind: Option, arg: &str) -> Option> { + match shell_kind { + Some(kind) => kind.try_quote(arg), + #[cfg(windows)] + None => Some(ShellKind::quote_powershell(arg)), + #[cfg(unix)] + None => shlex::try_quote(arg).ok(), + } +} + +/// Quote an argument for the shell (prefix-aware), with platform-specific fallback for unknown shells. +pub fn try_quote_prefix_aware_option( + shell_kind: Option, + arg: &str, +) -> Option> { + match shell_kind { + Some(kind) => kind.try_quote_prefix_aware(arg), + #[cfg(windows)] + None => Some(ShellKind::quote_powershell(arg)), + #[cfg(unix)] + None => shlex::try_quote(arg).ok(), + } +} + +/// Get the command prefix for the shell, with platform-specific fallback for unknown shells. +pub fn command_prefix_option(shell_kind: Option) -> Option { + match shell_kind { + Some(kind) => kind.command_prefix(), + #[cfg(windows)] + None => Some('&'), + #[cfg(unix)] + None => None, + } +} + +/// Prepend the command prefix if needed, with platform-specific fallback for unknown shells. +pub fn prepend_command_prefix_option<'a>( + shell_kind: Option, + command: &'a str, +) -> Cow<'a, str> { + match command_prefix_option(shell_kind) { + Some(prefix) if !command.starts_with(prefix) => Cow::Owned(format!("{prefix}{command}")), + _ => Cow::Borrowed(command), + } +} + +/// Get the sequential commands separator, with platform-specific fallback for unknown shells. +pub fn sequential_commands_separator_option(shell_kind: Option) -> char { + match shell_kind { + Some(kind) => kind.sequential_commands_separator(), + #[cfg(windows)] + None => ';', + #[cfg(unix)] + None => ';', + } +} + +/// Get the sequential-and commands separator, with platform-specific fallback for unknown shells. +pub fn sequential_and_commands_separator_option(shell_kind: Option) -> &'static str { + match shell_kind { + Some(kind) => kind.sequential_and_commands_separator(), + #[cfg(windows)] + None => ";", + #[cfg(unix)] + None => ";", + } +} + +/// Get shell arguments for running a command, with platform-specific fallback for unknown shells. +pub fn args_for_shell_option( + shell_kind: Option, + interactive: bool, + combined_command: String, +) -> Vec { + match shell_kind { + Some(kind) => kind.args_for_shell(interactive, combined_command), + #[cfg(windows)] + None => vec!["-C".to_owned(), combined_command], + #[cfg(unix)] + None => interactive + .then(|| "-i".to_owned()) + .into_iter() + .chain(["-c".to_owned(), combined_command]) + .collect(), + } +} + +/// Convert a variable to shell syntax, with platform-specific fallback for unknown shells. +pub fn to_shell_variable_option(shell_kind: Option, input: &str) -> String { + match shell_kind { + Some(kind) => kind.to_shell_variable(input), + #[cfg(windows)] + None => ShellKind::to_powershell_variable(input), + #[cfg(unix)] + None => input.to_owned(), + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/util/src/shell_builder.rs b/crates/util/src/shell_builder.rs index 86a588d44afa3bf4841a83f2700bd371dce4bd42..a54b93658bc42e6804d8cc65f363b177167a02ab 100644 --- a/crates/util/src/shell_builder.rs +++ b/crates/util/src/shell_builder.rs @@ -1,7 +1,15 @@ use std::borrow::Cow; use crate::shell::get_system_shell; -use crate::shell::{Shell, ShellKind}; +use crate::shell::{ + Shell, ShellKind, args_for_shell_option, to_shell_variable_option, try_quote_option, + try_quote_prefix_aware_option, +}; + +const POSIX_STDIN_REDIRECT_PREFIX: char = '('; +const POSIX_STDIN_REDIRECT_SUFFIX: &str = ") , } impl ShellBuilder { /// Create a new ShellBuilder as configured. - pub fn new(shell: &Shell, is_windows: bool) -> Self { + pub fn new(shell: &Shell) -> Self { let (program, args) = match shell { Shell::System => (get_system_shell(), Vec::new()), Shell::Program(shell) => (shell.clone(), Vec::new()), Shell::WithArguments { program, args, .. } => (program.clone(), args.clone()), }; - let kind = ShellKind::new(&program, is_windows); + let kind = ShellKind::new(&program); Self { program, args, @@ -44,20 +52,34 @@ impl ShellBuilder { self.program.clone() } else { match self.kind { - ShellKind::PowerShell | ShellKind::Pwsh => { + Some(ShellKind::PowerShell) | Some(ShellKind::Pwsh) => { + format!("{} -C '{}'", self.program, command_to_use_in_label) + } + #[cfg(windows)] + None => { format!("{} -C '{}'", self.program, command_to_use_in_label) } - ShellKind::Cmd => { + Some(ShellKind::Cmd) => { format!("{} /C \"{}\"", self.program, command_to_use_in_label) } - ShellKind::Posix - | ShellKind::Nushell - | ShellKind::Fish - | ShellKind::Csh - | ShellKind::Tcsh - | ShellKind::Rc - | ShellKind::Xonsh - | ShellKind::Elvish => { + Some( + ShellKind::Posix(_) + | ShellKind::Nushell + | ShellKind::Fish + | ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Rc + | ShellKind::Xonsh + | ShellKind::Elvish, + ) => { + let interactivity = self.interactive.then_some("-i ").unwrap_or_default(); + format!( + "{PROGRAM} {interactivity}-c '{command_to_use_in_label}'", + PROGRAM = self.program + ) + } + #[cfg(unix)] + None => { let interactivity = self.interactive.then_some("-i ").unwrap_or_default(); format!( "{PROGRAM} {interactivity}-c '{command_to_use_in_label}'", @@ -73,6 +95,44 @@ impl ShellBuilder { self } + fn apply_stdin_redirect(&self, combined_command: &mut String) { + match self.kind { + Some(ShellKind::Fish) => { + combined_command.insert_str(0, "begin; "); + combined_command.push_str("; end { + combined_command.insert(0, POSIX_STDIN_REDIRECT_PREFIX); + combined_command.push_str(POSIX_STDIN_REDIRECT_SUFFIX); + } + #[cfg(unix)] + None => { + combined_command.insert(0, POSIX_STDIN_REDIRECT_PREFIX); + combined_command.push_str(POSIX_STDIN_REDIRECT_SUFFIX); + } + Some(ShellKind::PowerShell) | Some(ShellKind::Pwsh) => { + combined_command.insert_str(0, POWERSHELL_STDIN_REDIRECT_PREFIX); + combined_command.push_str(POWERSHELL_STDIN_REDIRECT_SUFFIX); + } + #[cfg(windows)] + None => { + combined_command.insert_str(0, POWERSHELL_STDIN_REDIRECT_PREFIX); + combined_command.push_str(POWERSHELL_STDIN_REDIRECT_SUFFIX); + } + Some(ShellKind::Cmd) => { + combined_command.push_str("< NUL"); + } + } + } + /// Returns the program and arguments to run this task in a shell. pub fn build( mut self, @@ -81,7 +141,7 @@ impl ShellBuilder { ) -> (String, Vec) { if let Some(task_command) = task_command { let task_command = if !task_args.is_empty() { - match self.kind.try_quote_prefix_aware(&task_command) { + match try_quote_prefix_aware_option(self.kind, &task_command) { Some(task_command) => task_command.into_owned(), None => task_command, } @@ -90,41 +150,22 @@ impl ShellBuilder { }; let mut combined_command = task_args.iter().fold(task_command, |mut command, arg| { command.push(' '); - let shell_variable = self.kind.to_shell_variable(arg); - command.push_str(&match self.kind.try_quote(&shell_variable) { + let shell_variable = to_shell_variable_option(self.kind, arg); + command.push_str(&match try_quote_option(self.kind, &shell_variable) { Some(shell_variable) => shell_variable, None => Cow::Owned(shell_variable), }); command }); if self.redirect_stdin { - match self.kind { - ShellKind::Fish => { - combined_command.insert_str(0, "begin; "); - combined_command.push_str("; end { - combined_command.insert(0, '('); - combined_command.push_str(") { - combined_command.insert_str(0, "$null | & {"); - combined_command.push_str("}"); - } - ShellKind::Cmd => { - combined_command.push_str("< NUL"); - } - } + self.apply_stdin_redirect(&mut combined_command); } - self.args - .extend(self.kind.args_for_shell(self.interactive, combined_command)); + self.args.extend(args_for_shell_option( + self.kind, + self.interactive, + combined_command, + )); } (self.program, self.args) @@ -140,37 +181,18 @@ impl ShellBuilder { if let Some(task_command) = task_command { let mut combined_command = task_args.iter().fold(task_command, |mut command, arg| { command.push(' '); - command.push_str(&self.kind.to_shell_variable(arg)); + command.push_str(&to_shell_variable_option(self.kind, arg)); command }); if self.redirect_stdin { - match self.kind { - ShellKind::Fish => { - combined_command.insert_str(0, "begin; "); - combined_command.push_str("; end { - combined_command.insert(0, '('); - combined_command.push_str(") { - combined_command.insert_str(0, "$null | & {"); - combined_command.push_str("}"); - } - ShellKind::Cmd => { - combined_command.push_str("< NUL"); - } - } + self.apply_stdin_redirect(&mut combined_command); } - self.args - .extend(self.kind.args_for_shell(self.interactive, combined_command)); + self.args.extend(args_for_shell_option( + self.kind, + self.interactive, + combined_command, + )); } (self.program, self.args) @@ -202,7 +224,7 @@ impl ShellBuilder { if task_args.is_empty() { task_command = task_command .as_ref() - .map(|cmd| self.kind.try_quote_prefix_aware(&cmd).map(Cow::into_owned)) + .map(|cmd| try_quote_prefix_aware_option(self.kind, cmd).map(Cow::into_owned)) .unwrap_or(task_command); } let (program, args) = self.build(task_command, task_args); @@ -210,7 +232,7 @@ impl ShellBuilder { let mut child = crate::command::new_std_command(program); #[cfg(windows)] - if kind == ShellKind::Cmd { + if kind == Some(ShellKind::Cmd) { use std::os::windows::process::CommandExt; for arg in args { @@ -226,7 +248,7 @@ impl ShellBuilder { child } - pub fn kind(&self) -> ShellKind { + pub fn kind(&self) -> Option { self.kind } } @@ -238,7 +260,7 @@ mod test { #[test] fn test_nu_shell_variable_substitution() { let shell = Shell::Program("nu".to_owned()); - let shell_builder = ShellBuilder::new(&shell, false); + let shell_builder = ShellBuilder::new(&shell); let (program, args) = shell_builder.build( Some("echo".into()), @@ -266,7 +288,7 @@ mod test { #[test] fn redirect_stdin_to_dev_null_precedence() { let shell = Shell::Program("nu".to_owned()); - let shell_builder = ShellBuilder::new(&shell, false); + let shell_builder = ShellBuilder::new(&shell); let (program, args) = shell_builder .redirect_stdin_to_dev_null() @@ -279,7 +301,7 @@ mod test { #[test] fn redirect_stdin_to_dev_null_fish() { let shell = Shell::Program("fish".to_owned()); - let shell_builder = ShellBuilder::new(&shell, false); + let shell_builder = ShellBuilder::new(&shell); let (program, args) = shell_builder .redirect_stdin_to_dev_null() @@ -292,7 +314,7 @@ mod test { #[test] fn does_not_quote_sole_command_only() { let shell = Shell::Program("fish".to_owned()); - let shell_builder = ShellBuilder::new(&shell, false); + let shell_builder = ShellBuilder::new(&shell); let (program, args) = shell_builder.build(Some("echo".into()), &[]); @@ -300,7 +322,7 @@ mod test { assert_eq!(args, vec!["-i", "-c", "echo"]); let shell = Shell::Program("fish".to_owned()); - let shell_builder = ShellBuilder::new(&shell, false); + let shell_builder = ShellBuilder::new(&shell); let (program, args) = shell_builder.build(Some("echo oo".into()), &[]); diff --git a/crates/util/src/shell_env.rs b/crates/util/src/shell_env.rs index c41a28b46953bdb87f319bfbbb84b7ebd22be245..c8f9259467f443458002dc83e9f6fcdef5a2e57b 100644 --- a/crates/util/src/shell_env.rs +++ b/crates/util/src/shell_env.rs @@ -36,8 +36,8 @@ async fn capture_unix( use crate::command::new_std_command; - let shell_kind = ShellKind::new(shell_path, false); - let zed_path = super::get_shell_safe_zed_path(shell_kind)?; + let shell_kind = ShellKind::new(shell_path); + let zed_path = super::get_shell_safe_zed_path(shell_kind.as_ref())?; let mut command_string = String::new(); let mut command = new_std_command(shell_path); @@ -50,21 +50,21 @@ async fn capture_unix( const FD_STDERR: std::os::fd::RawFd = 2; let (fd_num, redir) = match shell_kind { - ShellKind::Rc => (FD_STDIN, format!(">[1={}]", FD_STDIN)), // `[1=0]` - ShellKind::Nushell | ShellKind::Tcsh => (FD_STDOUT, "".to_string()), + Some(ShellKind::Rc) => (FD_STDIN, format!(">[1={}]", FD_STDIN)), // `[1=0]` + Some(ShellKind::Nushell) | Some(ShellKind::Tcsh) => (FD_STDOUT, "".to_string()), // xonsh doesn't support redirecting to stdin, and control sequences are printed to // stdout on startup - ShellKind::Xonsh => (FD_STDERR, "o>e".to_string()), - ShellKind::PowerShell => (FD_STDIN, format!(">{}", FD_STDIN)), + Some(ShellKind::Xonsh) => (FD_STDERR, "o>e".to_string()), + Some(ShellKind::PowerShell) => (FD_STDIN, format!(">{}", FD_STDIN)), _ => (FD_STDIN, format!(">&{}", FD_STDIN)), // `>&0` }; match shell_kind { - ShellKind::Csh | ShellKind::Tcsh => { + Some(ShellKind::Csh) | Some(ShellKind::Tcsh) => { // For csh/tcsh, login shell requires passing `-` as 0th argument (instead of `-l`) command.arg0("-"); } - ShellKind::Fish => { + Some(ShellKind::Fish) => { // in fish, asdf, direnv attach to the `fish_prompt` event command_string.push_str("emit fish_prompt;"); command.arg("-l"); @@ -75,7 +75,7 @@ async fn capture_unix( } // cd into the directory, triggering directory specific side-effects (asdf, direnv, etc) command_string.push_str(&format!("cd '{}';", directory.display())); - if let Some(prefix) = shell_kind.command_prefix() { + if let Some(prefix) = shell_kind.and_then(|k| k.command_prefix()) { command_string.push(prefix); } command_string.push_str(&format!("{} --printenv {}", zed_path, redir)); @@ -140,16 +140,18 @@ async fn capture_windows( let zed_path = std::env::current_exe().context("Failed to determine current zed executable path.")?; - let shell_kind = ShellKind::new(shell_path, true); + let shell_kind = ShellKind::new(shell_path); let mut cmd = crate::command::new_smol_command(shell_path); cmd.args(args); let cmd = match shell_kind { - ShellKind::Csh - | ShellKind::Tcsh - | ShellKind::Rc - | ShellKind::Fish - | ShellKind::Xonsh - | ShellKind::Posix => cmd.args([ + Some( + ShellKind::Csh + | ShellKind::Tcsh + | ShellKind::Rc + | ShellKind::Fish + | ShellKind::Xonsh + | ShellKind::Posix(_), + ) => cmd.args([ "-l", "-i", "-c", @@ -159,7 +161,7 @@ async fn capture_windows( zed_path.display() ), ]), - ShellKind::PowerShell | ShellKind::Pwsh => cmd.args([ + Some(ShellKind::PowerShell) | Some(ShellKind::Pwsh) | None => cmd.args([ "-NonInteractive", "-NoProfile", "-Command", @@ -169,7 +171,7 @@ async fn capture_windows( zed_path.display() ), ]), - ShellKind::Elvish => cmd.args([ + Some(ShellKind::Elvish) => cmd.args([ "-c", &format!( "cd '{}'; '{}' --printenv", @@ -177,19 +179,19 @@ async fn capture_windows( zed_path.display() ), ]), - ShellKind::Nushell => cmd.args([ + Some(ShellKind::Nushell) => cmd.args([ "-c", &format!( "cd '{}'; {}'{}' --printenv", directory.display(), - shell_kind + ShellKind::Nushell .command_prefix() .map(|prefix| prefix.to_string()) .unwrap_or_default(), zed_path.display() ), ]), - ShellKind::Cmd => cmd.args([ + Some(ShellKind::Cmd) => cmd.args([ "/c", "cd", &directory.display().to_string(), diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 87b8a1615d86c74fa4b3b1cb7c89fc94939558a0..4d5e2dd073d1e59a3078fe090f7d6051a818189b 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -303,7 +303,7 @@ fn load_shell_from_passwd() -> Result<()> { } /// Returns a shell escaped path for the current zed executable -pub fn get_shell_safe_zed_path(shell_kind: shell::ShellKind) -> anyhow::Result { +pub fn get_shell_safe_zed_path(shell_kind: Option<&shell::ShellKind>) -> anyhow::Result { let mut zed_path = std::env::current_exe().context("Failed to determine current zed executable path.")?; if cfg!(target_os = "linux")