diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 420c70881e..1ac2f3704f 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -174,6 +174,25 @@ mise run my-bash-task MISE_BASH_PATH = "C:/tools/msys64/usr/bin/bash.exe" ``` +mise honors an **explicit** bash path as-is. If you set `shell` (in a task) or +`windows_default_inline_shell_args` to an absolute path such as +`C:/msys64/usr/bin/bash.exe -c`, mise uses exactly that binary — the +`MISE_BASH_PATH` override and the Git Bash / MSYS2 auto-detection apply only +when the shell is the bare name `bash`. + +If your shell path contains spaces (e.g. `C:\Program Files\Git\bin\bash.exe`), +wrap the program in double quotes so the space is not treated as an argument +separator. On Windows, backslashes are treated literally, so they need no +escaping; forward slashes work too: + +```toml +[tasks.build] +run = "echo hi" +shell = '"C:\Program Files\Git\bin\bash.exe" -c' +``` + +(On macOS/Linux, `shell` follows POSIX quoting rules instead.) + ## mise isn't working when calling from tmux or another shell initialization script `mise activate` will not update PATH until the shell prompt is displayed. So if you need to access a diff --git a/src/config/settings.rs b/src/config/settings.rs index 7cedbc8f39..f12353317a 100644 --- a/src/config/settings.rs +++ b/src/config/settings.rs @@ -708,7 +708,7 @@ impl Settings { } else { &self.unix_default_inline_shell_args }; - Ok(shell_words::split(sa)?) + crate::path::split_shell_command(sa) } pub fn default_file_shell(&self) -> Result> { @@ -717,7 +717,7 @@ impl Settings { } else { &self.unix_default_file_shell_args }; - Ok(shell_words::split(sa)?) + crate::path::split_shell_command(sa) } pub fn os(&self) -> &str { diff --git a/src/hooks.rs b/src/hooks.rs index 5d9dd051e8..56aba83b9c 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -426,7 +426,7 @@ async fn execute( } let shell = shell .as_ref() - .map(|shell| shell_words::split(shell)) + .map(|shell| crate::path::split_shell_command(shell)) .transpose()? .unwrap_or(Settings::get().default_inline_shell()?); diff --git a/src/path.rs b/src/path.rs index a1a12e03c5..1abcbd6f7f 100644 --- a/src/path.rs +++ b/src/path.rs @@ -137,10 +137,85 @@ pub fn is_posix_shell_program(program: &Path) -> bool { POSIX_SHELLS.iter().any(|name| *name == stem) } +/// Split a configured shell *command string* (program + args) into argv, +/// honoring host conventions. +/// +/// On Windows, backslashes are ordinary path characters (NOT escapes) and only +/// double-quoted spans group whitespace — matching how a Windows user expects +/// `C:\path\bash.exe` or `"C:\Program Files\..\bash.exe" -c` to parse. A `""` +/// inside a quoted span is a literal `"`; single quotes are literal characters +/// (cmd does not use them, and they can occur in paths). On Unix, defer to +/// `shell_words::split` for POSIX quoting/escaping. +/// +/// Used for every configured shell string — a task's `shell`, hook and +/// `[[watch_files]]` shells, and the `*_default_*_shell_args` settings — so an +/// explicit shell path with spaces (when double-quoted) or with backslashes +/// reaches the spawn verbatim instead of being mangled. Returns `Err` only on +/// an unbalanced double quote (Windows) or a `shell_words` parse error (Unix). +pub fn split_shell_command(s: &str) -> eyre::Result> { + #[cfg(windows)] + { + split_shell_command_windows(s) + } + #[cfg(not(windows))] + { + Ok(shell_words::split(s)?) + } +} + +/// Windows `CommandLineToArgvW`-style splitter, narrowed to mise's needs: +/// double quotes group whitespace, `""` inside a quoted span is a literal `"`, +/// and backslash is a plain character (never an escape — so Windows paths +/// survive). Single quotes are literal. Errors only on an unterminated +/// double-quoted span. +#[cfg(windows)] +fn split_shell_command_windows(s: &str) -> eyre::Result> { + let mut args: Vec = Vec::new(); + let mut cur = String::new(); + let mut in_token = false; + let mut in_quotes = false; + let mut chars = s.chars().peekable(); + while let Some(c) = chars.next() { + if c == '"' { + in_token = true; + if in_quotes { + if chars.peek() == Some(&'"') { + // `""` inside a quoted span → a literal `"`. + cur.push('"'); + chars.next(); + } else { + in_quotes = false; + } + } else { + in_quotes = true; + } + } else if c.is_whitespace() && !in_quotes { + if in_token { + args.push(std::mem::take(&mut cur)); + in_token = false; + } + } else { + in_token = true; + cur.push(c); + } + } + if in_quotes { + return Err(eyre::eyre!("unbalanced quote in shell command: {s}")); + } + if in_token { + args.push(cur); + } + Ok(args) +} + #[cfg(test)] mod tests { use super::*; + fn sv(parts: &[&str]) -> Vec { + parts.iter().map(|s| s.to_string()).collect() + } + #[test] fn test_windows_path_list_to_unix_basic() { assert_eq!(windows_path_list_to_unix(r"C:\foo;D:\bar"), "/c/foo:/d/bar"); @@ -249,4 +324,80 @@ mod tests { assert!(!is_posix_shell_program(Path::new("rustc"))); assert!(!is_posix_shell_program(Path::new(""))); } + + #[test] + fn test_split_shell_command_bare_names() { + assert_eq!(split_shell_command("bash -c").unwrap(), sv(&["bash", "-c"])); + assert_eq!(split_shell_command("sh -c").unwrap(), sv(&["sh", "-c"])); + assert_eq!( + split_shell_command("sh -c -o errexit").unwrap(), + sv(&["sh", "-c", "-o", "errexit"]) + ); + } + + #[test] + fn test_split_shell_command_empty() { + assert_eq!(split_shell_command("").unwrap(), sv(&[])); + assert_eq!(split_shell_command(" ").unwrap(), sv(&[])); + } + + #[test] + fn test_split_shell_command_quoted_path_with_spaces() { + // A double-quoted path containing spaces is one token on both platforms. + assert_eq!( + split_shell_command("\"C:/Program Files/Git/bin/bash.exe\" -c").unwrap(), + sv(&["C:/Program Files/Git/bin/bash.exe", "-c"]) + ); + } + + #[cfg(windows)] + #[test] + fn test_split_shell_command_windows_backslash_is_literal() { + // Backslash is a plain path char on Windows, not an escape. + assert_eq!( + split_shell_command(r"C:\msys64\usr\bin\bash.exe -c").unwrap(), + sv(&[r"C:\msys64\usr\bin\bash.exe", "-c"]) + ); + assert_eq!( + split_shell_command("\"C:\\Program Files\\Git\\bin\\bash.exe\" -c").unwrap(), + sv(&[r"C:\Program Files\Git\bin\bash.exe", "-c"]) + ); + } + + #[cfg(windows)] + #[test] + fn test_split_shell_command_windows_unquoted_space_splits() { + // Documented ambiguity: an unquoted space splits even inside a path. + assert_eq!( + split_shell_command(r"C:/Program Files/Git/bin/bash.exe -c").unwrap(), + sv(&["C:/Program", "Files/Git/bin/bash.exe", "-c"]) + ); + } + + #[cfg(windows)] + #[test] + fn test_split_shell_command_windows_double_quote_is_literal() { + // `""` inside a quoted span → a literal `"`. + assert_eq!( + split_shell_command("\"a\"\"b\" c").unwrap(), + sv(&["a\"b", "c"]) + ); + } + + #[cfg(windows)] + #[test] + fn test_split_shell_command_windows_unbalanced_quote_errs() { + assert!(split_shell_command("\"unterminated").is_err()); + } + + #[cfg(not(windows))] + #[test] + fn test_split_shell_command_unix_posix_semantics() { + // Unix keeps shell_words (POSIX) behavior: backslash escapes, single quotes group. + assert_eq!( + split_shell_command(r"bash\ script -c").unwrap(), + sv(&["bash script", "-c"]) + ); + assert_eq!(split_shell_command("'a b' c").unwrap(), sv(&["a b", "c"])); + } } diff --git a/src/task/mod.rs b/src/task/mod.rs index c7b0b43181..52c343b3d2 100644 --- a/src/task/mod.rs +++ b/src/task/mod.rs @@ -1230,19 +1230,20 @@ impl Task { false } - pub fn shell(&self) -> Option> { - self.shell.as_ref().and_then(|shell| { - let shell_cmd = shell - .split_whitespace() - .map(|s| s.to_string()) - .collect::>(); - if shell_cmd.is_empty() || shell_cmd[0].trim().is_empty() { - warn!("invalid shell '{shell}', expected ' ' (e.g. sh -c)"); - None - } else { - Some(shell_cmd) - } - }) + pub fn shell(&self) -> eyre::Result>> { + let Some(shell) = self.shell.as_ref() else { + return Ok(None); + }; + // A malformed explicit shell (e.g. an unbalanced quote in a path with + // spaces) must fail loudly rather than silently falling back to the + // default shell and running the task under the wrong interpreter. + let shell_cmd = crate::path::split_shell_command(shell)?; + if shell_cmd.is_empty() || shell_cmd[0].trim().is_empty() { + warn!("invalid shell '{shell}', expected ' ' (e.g. sh -c)"); + Ok(None) + } else { + Ok(Some(shell_cmd)) + } } /// Overlay metadata from a `[tasks.]` TOML block onto this task. @@ -2353,6 +2354,68 @@ mod tests { } } + #[test] + fn test_shell_parses_and_validates() { + let mut task = Task { + shell: Some("bash -c".to_string()), + ..Default::default() + }; + assert_eq!( + task.shell().unwrap(), + Some(vec!["bash".to_string(), "-c".to_string()]) + ); + + // Whitespace-only is invalid → Ok(None). + task.shell = Some(" ".to_string()); + assert_eq!(task.shell().unwrap(), None); + + // No shell configured → Ok(None). + task.shell = None; + assert_eq!(task.shell().unwrap(), None); + } + + #[test] + fn test_shell_unbalanced_quote_fails_loudly() { + // A malformed explicit shell (unbalanced quote) must error, not silently + // fall back to the default shell and run under the wrong interpreter. + // Codex review of #9932. + let task = Task { + shell: Some("\"unterminated".to_string()), + ..Default::default() + }; + assert!(task.shell().is_err()); + } + + #[test] + #[cfg(windows)] + fn test_shell_parses_explicit_windows_path() { + // #9932: a backslash path stays one token, and a double-quoted path with + // spaces is one token. (POSIX parsing is covered in path.rs tests.) + let task = Task { + shell: Some(r"C:\msys64\usr\bin\bash.exe -c".to_string()), + ..Default::default() + }; + assert_eq!( + task.shell().unwrap(), + Some(vec![ + r"C:\msys64\usr\bin\bash.exe".to_string(), + "-c".to_string() + ]) + ); + + let task = Task { + shell: Some("\"C:\\Program Files\\Git\\bin\\bash.exe\" -c".to_string()), + ..Default::default() + }; + assert_eq!( + task.shell().unwrap(), + Some(vec![ + r"C:\Program Files\Git\bin\bash.exe".to_string(), + "-c".to_string() + ]) + ); + } + // This test verifies that resolve_depends correctly uses self.depends_post // instead of iterating through all tasks_to_run (which was the bug) #[tokio::test] diff --git a/src/task/task_executor.rs b/src/task/task_executor.rs index ce96a7f606..3c3f28a08f 100644 --- a/src/task/task_executor.rs +++ b/src/task/task_executor.rs @@ -859,7 +859,7 @@ impl TaskExecutor { return Ok((display, args.to_vec())); } let shell = task - .shell() + .shell()? .or_else(|| shell_from_shebang(file)) .or_else(|| shell_from_extension(file)) .unwrap_or(Settings::get().default_file_shell()?); @@ -878,7 +878,7 @@ impl TaskExecutor { task: &Task, args: &[String], ) -> Result<(String, Vec)> { - let shell = task.shell().unwrap_or(self.clone_default_inline_shell()?); + let shell = task.shell()?.unwrap_or(self.clone_default_inline_shell()?); trace!("using shell: {}", shell.join(" ")); let mut full_args = shell.clone(); @@ -901,7 +901,7 @@ impl TaskExecutor { fn clone_default_inline_shell(&self) -> Result> { if let Some(shell) = &self.shell { - Ok(shell_words::split(shell)?) + crate::path::split_shell_command(shell) } else { Settings::get().default_inline_shell() } @@ -1360,12 +1360,13 @@ fn shell_from_extension(path: &Path) -> Option> { /// entry that isn't the WSL launcher. This rescues setups where a real /// POSIX bash is on PATH but appears after `C:\Windows\System32`. /// -/// Returns `None` when the program is not a POSIX shell, the env has no PATH, -/// the PATH is already in Unix form (no `;` and no `\`, so no conversion will -/// fire), `which` finds nothing, or every PATH match for `bash` is the WSL -/// launcher — in those cases the caller keeps the original program string and -/// lets the stdlib spawn it (which will then fail loudly rather than silently -/// routing into WSL). +/// Returns `None` when the program is not a POSIX shell, the program is already +/// an explicit path (absolute, or relative with a directory component — that is +/// honored verbatim and never re-resolved), the env has no PATH, the PATH is +/// already in Unix form (no `;` and no `\`, so no conversion will fire), `which` +/// finds nothing, or every PATH match for `bash` is the WSL launcher — in those +/// cases the caller keeps the original program string and lets the stdlib spawn +/// it (which will then fail loudly rather than silently routing into WSL). #[cfg(windows)] fn resolve_posix_shell_program_path( program: &std::ffi::OsStr, @@ -1374,6 +1375,15 @@ fn resolve_posix_shell_program_path( if !crate::path::is_posix_shell_program(Path::new(program)) { return None; } + // An explicit path (absolute, or relative with a directory component) is a + // deliberate choice of *which* shell binary to run — honor it verbatim + // rather than re-resolving via the bash candidate list or a PATH search. + // Only a bare command name (`bash`, `bash.exe`) flows into the WSL-avoidance + // resolution below. Regression fix for discussion #9932: PR #9750 over- + // resolved and silently swapped an explicit Cygwin bash for Git Bash. + if program_has_directory_component(program) { + return None; + } let path_val = env.get(&*crate::env::PATH_KEY)?; if !path_val.contains(';') && !path_val.contains('\\') { return None; @@ -1433,6 +1443,17 @@ fn is_bash_basename(program: &std::ffi::OsStr) -> bool { crate::path::program_stem(Path::new(program)).as_deref() == Some("bash") } +/// Returns true if `program` carries an explicit directory component — an +/// absolute path (`C:\x\bash.exe`, `C:/x/bash.exe`) or a relative one with a +/// separator (`./bash`, `bin/bash`) — as opposed to a bare command name +/// (`bash`, `bash.exe`) that must be looked up on PATH. Uses `Path::components` +/// (allocation-free, and treats both `/` and `\` as separators on Windows): a +/// bare file name has exactly one component, anything with a directory has more. +#[cfg(windows)] +fn program_has_directory_component(program: &std::ffi::OsStr) -> bool { + Path::new(program).components().count() > 1 +} + /// Common real-POSIX-bash install locations on Windows (Git Bash + MSYS2), in /// preference order. Pure given `env` (no filesystem access), so the caller /// stats each candidate. `MISE_BASH_PATH` covers anything outside this list, @@ -1762,4 +1783,81 @@ mod tests { let env = env_with_path("/c/foo:/d/bar"); assert!(resolve_posix_shell_program_path(std::ffi::OsStr::new("bash"), &env).is_none()); } + + #[test] + #[cfg(windows)] + fn test_resolve_posix_shell_program_path_honors_explicit_forward_slash_path() { + // #9932: an explicit absolute bash path must be kept verbatim (None here, + // so the caller keeps the original), NOT re-resolved to Git Bash via the + // candidate list — even with a Windows-form PATH that would otherwise + // trigger resolution. + let env = env_with_path(r"C:\Windows\System32;C:\Program Files\Git\bin"); + assert!( + resolve_posix_shell_program_path( + std::ffi::OsStr::new("C:/msys64/usr/bin/bash.exe"), + &env + ) + .is_none() + ); + } + + #[test] + #[cfg(windows)] + fn test_resolve_posix_shell_program_path_honors_explicit_path_backslashes() { + let env = env_with_path(r"C:\Windows\System32;C:\Program Files\Git\bin"); + assert!( + resolve_posix_shell_program_path( + std::ffi::OsStr::new(r"C:\msys64\usr\bin\bash.exe"), + &env + ) + .is_none() + ); + } + + #[test] + #[cfg(windows)] + fn test_resolve_posix_shell_program_path_honors_explicit_relative_path() { + // A relative path with a separator is still an explicit choice, not a + // bare name to look up on PATH. + let env = env_with_path(r"C:\Windows\System32;C:\Program Files\Git\bin"); + assert!(resolve_posix_shell_program_path(std::ffi::OsStr::new("bin/bash"), &env).is_none()); + } + + #[test] + #[cfg(windows)] + fn test_resolve_posix_shell_program_path_honors_explicit_non_bash_shell_path() { + // An explicit path to a non-bash POSIX shell is honored verbatim too. + let env = env_with_path(r"C:\Windows\System32;C:\msys64\usr\bin"); + assert!( + resolve_posix_shell_program_path( + std::ffi::OsStr::new(r"C:\msys64\usr\bin\zsh.exe"), + &env + ) + .is_none() + ); + } + + #[test] + #[cfg(windows)] + fn test_program_has_directory_component_detects_explicit_paths() { + use std::ffi::OsStr; + assert!(program_has_directory_component(OsStr::new( + "C:/msys64/usr/bin/bash.exe" + ))); + assert!(program_has_directory_component(OsStr::new( + r"C:\msys64\usr\bin\bash.exe" + ))); + assert!(program_has_directory_component(OsStr::new("./bash"))); + assert!(program_has_directory_component(OsStr::new("bin/bash"))); + assert!(program_has_directory_component(OsStr::new("/usr/bin/bash"))); + } + + #[test] + #[cfg(windows)] + fn test_program_has_directory_component_rejects_bare_names() { + use std::ffi::OsStr; + assert!(!program_has_directory_component(OsStr::new("bash"))); + assert!(!program_has_directory_component(OsStr::new("bash.exe"))); + assert!(!program_has_directory_component(OsStr::new("BASH.EXE"))); + } } diff --git a/src/task/task_script_parser.rs b/src/task/task_script_parser.rs index 14414a546f..1ef72bf63b 100644 --- a/src/task/task_script_parser.rs +++ b/src/task/task_script_parser.rs @@ -772,7 +772,7 @@ impl TaskScriptParser { continue; } let shell_type = shell_from_shebang(script) - .or(task.shell()) + .or(task.shell()?) .unwrap_or(Settings::get().default_inline_shell()?)[0] .parse() .ok(); diff --git a/src/watch_files.rs b/src/watch_files.rs index bcf7aaddd3..b07a529b43 100644 --- a/src/watch_files.rs +++ b/src/watch_files.rs @@ -83,7 +83,7 @@ async fn execute( .map(|f| f.to_string_lossy().replace(':', "\\:")) .join(":"); let shell = match shell { - Some(shell) => shell_words::split(shell)?, + Some(shell) => crate::path::split_shell_command(shell)?, None => Settings::get().default_inline_shell()?, }; let (program, shell_args) = shell.split_first().ok_or_else(|| {