From a7c1b363eb0129d777d5dc9b8bf505a33afde559 Mon Sep 17 00:00:00 2001 From: Jelle Besseling Date: Wed, 3 May 2023 00:56:35 +0200 Subject: [PATCH] Don't run .sh files with /bin/sh (#8951) # Description The previous behaviour broke for me because I didn't have `sh` in my path for my nu script. I think we shouldn't assume that just because a file ends with `.sh` it should be executed with `sh`. `sh` might not be available or the script might contain a hashbang for a different shell. The idea with this PR is that nushell shouldn't assume anything about executable files and just execute them. Later on we can think about how non-executable files should be executed if we detect they are a script. # User-Facing Changes This may break some people's scripts or habits if they have wrong assumptions about `.sh` files. We can tell them to add a hashbang and +x bit to execute shell scripts, or prepend `bash`. If this a common assumption something like this should be added to the book # Tests + Formatting I only tested manually and that did work # After Submitting Co-authored-by: Jelle Besseling --- crates/nu-command/src/system/run_external.rs | 34 -------------------- 1 file changed, 34 deletions(-) diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index c3f4d5db70..46e9ab5b10 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -1,6 +1,4 @@ use crate::hook::eval_hook; -use fancy_regex::Regex; -use itertools::Itertools; use nu_engine::env_to_strings; use nu_engine::CallExt; use nu_protocol::{ @@ -651,8 +649,6 @@ impl ExternalCommand { } else { self.spawn_simple_command(cwd) } - } else if self.name.item.ends_with(".sh") { - Ok(self.spawn_sh_command()) } else { self.spawn_simple_command(cwd) } @@ -698,19 +694,6 @@ impl ExternalCommand { process } - - /// Spawn a sh command with `sh -c args...` - pub fn spawn_sh_command(&self) -> std::process::Command { - let joined_and_escaped_arguments = self - .args - .iter() - .map(|arg| shell_arg_escape(&arg.item)) - .join(" "); - let cmd_with_args = vec![self.name.item.clone(), joined_and_escaped_arguments].join(" "); - let mut process = std::process::Command::new("sh"); - process.arg("-c").arg(cmd_with_args); - process - } } fn trim_expand_and_apply_arg( @@ -796,23 +779,6 @@ fn suggest_command(attempted_command: &str, engine_state: &EngineState) -> Optio } } -fn has_unsafe_shell_characters(arg: &str) -> bool { - let re: Regex = Regex::new(r"[^\w@%+=:,./-]").expect("regex to be valid"); - - re.is_match(arg).unwrap_or(false) -} - -fn shell_arg_escape(arg: &str) -> String { - match arg { - "" => String::from("''"), - s if !has_unsafe_shell_characters(s) => String::from(s), - _ => { - let single_quotes_escaped = arg.split('\'').join("'\"'\"'"); - format!("'{single_quotes_escaped}'") - } - } -} - /// This function returns a tuple with 3 items: /// 1st item: trimmed string. /// 2nd item: a boolean value indicate if it's ok to run glob expansion.