From 1eaf996a3645704910ea30b0ee19a97ab4f1daf6 Mon Sep 17 00:00:00 2001 From: David Knaack Date: Fri, 16 Jul 2021 21:20:59 +0200 Subject: fix(windows): avoid inadvertly running exes from cwd (#2885) On Windows when running commands with their name instead of the path with Command::new, executable with that name from the current working directory will be executed. This PR replaces all instances of Command::new with a new create_command function which will first resolve any executable paths and avoid this issue. --- CONTRIBUTING.md | 2 ++ clippy.toml | 2 ++ src/bug_report.rs | 12 ------------ src/configure.rs | 5 ++--- src/init/mod.rs | 5 ++--- src/lib.rs | 2 ++ src/main.rs | 2 ++ src/modules/custom.rs | 15 ++++++++++----- src/modules/directory.rs | 4 ++-- src/modules/dotnet.rs | 4 ++-- src/modules/git_branch.rs | 20 ++++++++++---------- src/modules/git_commit.rs | 32 ++++++++++++++++---------------- src/modules/git_metrics.rs | 5 +++-- src/modules/git_state.rs | 5 +++-- src/modules/git_status.rs | 40 ++++++++++++++++++++-------------------- src/modules/hg_branch.rs | 4 ++-- src/modules/rust.rs | 13 +++++++------ src/test/mod.rs | 16 +++++++++------- src/utils.rs | 31 ++++++++++++++++++++++++++++++- 19 files changed, 126 insertions(+), 93 deletions(-) create mode 100644 clippy.toml diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0a0a23de0..fbc3dbe6a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,6 +68,8 @@ pub fn module<'a>(context: &'a Context) -> Option> { } ``` +If using `context.exec_cmd` isn't possible, please use `crate::utils::create_command` instead of `std::process::Command::new`. + ## Logging Debug logging in starship is done with our custom logger implementation. diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 000000000..3054dd619 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,2 @@ +# std::process::Command::new may inadvertly run executables from the current working directory +disallowed-methods = ["std::process::Command::new"] diff --git a/src/bug_report.rs b/src/bug_report.rs index 82a76b61e..28a9bb720 100644 --- a/src/bug_report.rs +++ b/src/bug_report.rs @@ -265,18 +265,6 @@ mod tests { assert!(link.contains("No+Starship+config")); } - #[test] - fn test_get_shell_info() { - env::remove_var("STARSHIP_SHELL"); - let unknown_shell = get_shell_info(); - assert_eq!(UNKNOWN_SHELL, &unknown_shell.name); - - env::set_var("STARSHIP_SHELL", "fish"); - - let fish_shell = get_shell_info(); - assert_eq!("fish", &fish_shell.name); - } - #[test] #[cfg(not(windows))] fn test_get_config_path() { diff --git a/src/configure.rs b/src/configure.rs index b524845ec..3a623c66a 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -2,7 +2,6 @@ use std::env; use std::ffi::OsString; use std::io::ErrorKind; use std::process; -use std::process::Command; use crate::config::RootModuleConfig; use crate::config::StarshipConfig; @@ -136,9 +135,9 @@ pub fn write_configuration(table: &mut Table) { pub fn edit_configuration() { let config_path = get_config_path(); let editor_cmd = shell_words::split(&get_editor()).expect("Unmatched quotes found in $EDITOR."); - let editor_path = which::which(&editor_cmd[0]).expect("Unable to locate editor in $PATH."); - let command = Command::new(editor_path) + let command = utils::create_command(&editor_cmd[0]) + .expect("Unable to locate editor in $PATH.") .args(&editor_cmd[1..]) .arg(config_path) .status(); diff --git a/src/init/mod.rs b/src/init/mod.rs index dfe360b43..893a063c7 100644 --- a/src/init/mod.rs +++ b/src/init/mod.rs @@ -1,3 +1,4 @@ +use crate::utils::create_command; use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::{env, io}; @@ -51,9 +52,7 @@ impl StarshipPath { return self.sprint(); } let str_path = self.str_path()?; - let res = std::process::Command::new("cygpath.exe") - .arg(str_path) - .output(); + let res = create_command("cygpath").and_then(|mut cmd| cmd.arg(str_path).output()); let output = match res { Ok(output) => output, Err(e) => { diff --git a/src/lib.rs b/src/lib.rs index 09beaceac..8329f389d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,5 @@ +#![warn(clippy::disallowed_method)] + #[macro_use] extern crate shadow_rs; diff --git a/src/main.rs b/src/main.rs index 20b9d4113..f1bce8fe6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,3 +1,5 @@ +#![warn(clippy::disallowed_method)] + use clap::crate_authors; use std::io; use std::time::SystemTime; diff --git a/src/modules/custom.rs b/src/modules/custom.rs index ee1e07389..d569190e8 100644 --- a/src/modules/custom.rs +++ b/src/modules/custom.rs @@ -5,7 +5,7 @@ use std::time::Instant; use super::{Context, Module, RootModuleConfig}; -use crate::{configs::custom::CustomConfig, formatter::StringFormatter}; +use crate::{configs::custom::CustomConfig, formatter::StringFormatter, utils::create_command}; /// Creates a custom module with some configuration /// @@ -100,7 +100,7 @@ fn get_shell<'a, 'b>(shell_args: &'b [&'a str]) -> (std::borrow::Cow<'a, str>, & #[cfg(not(windows))] fn shell_command(cmd: &str, shell_args: &[&str]) -> Option { let (shell, shell_args) = get_shell(shell_args); - let mut command = Command::new(shell.as_ref()); + let mut command = create_command(shell.as_ref()).ok()?; command .args(shell_args) @@ -118,6 +118,7 @@ fn shell_command(cmd: &str, shell_args: &[&str]) -> Option { "Could not launch command with given shell or STARSHIP_SHELL env variable, retrying with /usr/bin/env sh" ); + #[allow(clippy::disallowed_method)] Command::new("/usr/bin/env") .arg("sh") .stdin(Stdio::piped()) @@ -141,14 +142,17 @@ fn shell_command(cmd: &str, shell_args: &[&str]) -> Option { Some(std::borrow::Cow::Borrowed(shell_args[0])), &shell_args[1..], ) - } else if let Ok(env_shell) = std::env::var("STARSHIP_SHELL") { + } else if let Some(env_shell) = std::env::var("STARSHIP_SHELL") + .ok() + .filter(|s| !cfg!(test) && !s.is_empty()) + { (Some(std::borrow::Cow::Owned(env_shell)), &[] as &[&str]) } else { (None, &[] as &[&str]) }; if let Some(forced_shell) = shell { - let mut command = Command::new(forced_shell.as_ref()); + let mut command = create_command(forced_shell.as_ref()).ok()?; command .args(shell_args) @@ -169,7 +173,8 @@ fn shell_command(cmd: &str, shell_args: &[&str]) -> Option { ); } - let command = Command::new("cmd.exe") + let command = create_command("cmd") + .ok()? .arg("/C") .arg(cmd) .stdin(Stdio::piped()) diff --git a/src/modules/directory.rs b/src/modules/directory.rs index 879bc6631..27561f62b 100644 --- a/src/modules/directory.rs +++ b/src/modules/directory.rs @@ -298,6 +298,7 @@ fn to_fish_style(pwd_dir_length: usize, dir_string: String, truncated_dir_string mod tests { use super::*; use crate::test::ModuleRenderer; + use crate::utils::create_command; use crate::utils::home_dir; use ansi_term::Color; #[cfg(not(target_os = "windows"))] @@ -305,7 +306,6 @@ mod tests { #[cfg(target_os = "windows")] use std::os::windows::fs::symlink_dir as symlink; use std::path::Path; - use std::process::Command; use std::{fs, io}; use tempfile::TempDir; @@ -443,7 +443,7 @@ mod tests { } fn init_repo(path: &Path) -> io::Result<()> { - Command::new("git") + create_command("git")? .args(&["init"]) .current_dir(path) .output() diff --git a/src/modules/dotnet.rs b/src/modules/dotnet.rs index 68d03ecdc..6daa736bf 100644 --- a/src/modules/dotnet.rs +++ b/src/modules/dotnet.rs @@ -345,10 +345,10 @@ enum FileType { mod tests { use super::*; use crate::test::ModuleRenderer; + use crate::utils::create_command; use ansi_term::Color; use std::fs::{self, OpenOptions}; use std::io::{self, Write}; - use std::process::Command; use tempfile::{self, TempDir}; #[test] @@ -551,7 +551,7 @@ mod tests { let repo_dir = tempfile::tempdir()?; if is_repo { - Command::new("git") + create_command("git")? .args(&["init", "--quiet"]) .current_dir(repo_dir.path()) .output()?; diff --git a/src/modules/git_branch.rs b/src/modules/git_branch.rs index e6cadb810..9e767ec7d 100644 --- a/src/modules/git_branch.rs +++ b/src/modules/git_branch.rs @@ -121,9 +121,9 @@ fn get_first_grapheme(text: &str) -> &str { mod tests { use ansi_term::Color; use std::io; - use std::process::Command; use crate::test::{fixture_repo, FixtureProvider, ModuleRenderer}; + use crate::utils::create_command; #[test] fn show_nothing_on_empty_dir() -> io::Result<()> { @@ -270,12 +270,12 @@ mod tests { fn test_works_with_unborn_default_branch() -> io::Result<()> { let repo_dir = tempfile::tempdir()?; - Command::new("git") + create_command("git")? .args(&["init"]) .current_dir(&repo_dir) .output()?; - Command::new("git") + create_command("git")? .args(&["symbolic-ref", "HEAD", "refs/heads/main"]) .current_dir(&repo_dir) .output()?; @@ -297,7 +297,7 @@ mod tests { fn test_render_branch_only_attached_on_branch() -> io::Result<()> { let repo_dir = fixture_repo(FixtureProvider::Git)?; - Command::new("git") + create_command("git")? .args(&["checkout", "-b", "test_branch"]) .current_dir(repo_dir.path()) .output()?; @@ -325,7 +325,7 @@ mod tests { fn test_render_branch_only_attached_on_detached() -> io::Result<()> { let repo_dir = fixture_repo(FixtureProvider::Git)?; - Command::new("git") + create_command("git")? .args(&["checkout", "@~1"]) .current_dir(&repo_dir.path()) .output()?; @@ -348,12 +348,12 @@ mod tests { fn test_works_in_bare_repo() -> io::Result<()> { let repo_dir = tempfile::tempdir()?; - Command::new("git") + create_command("git")? .args(&["init", "--bare"]) .current_dir(&repo_dir) .output()?; - Command::new("git") + create_command("git")? .args(&["symbolic-ref", "HEAD", "refs/heads/main"]) .current_dir(&repo_dir) .output()?; @@ -379,7 +379,7 @@ mod tests { // fn test_git_dir_env_variable() -> io::Result<()> {let repo_dir = // tempfile::tempdir()?; - // Command::new("git") + // create_command("git")? // .args(&["init"]) // .current_dir(&repo_dir) // .output()?; @@ -424,7 +424,7 @@ mod tests { ) -> io::Result<()> { let repo_dir = fixture_repo(FixtureProvider::Git)?; - Command::new("git") + create_command("git")? .args(&["checkout", "-b", branch_name]) .current_dir(repo_dir.path()) .output()?; @@ -463,7 +463,7 @@ mod tests { ) -> io::Result<()> { let repo_dir = fixture_repo(FixtureProvider::Git)?; - Command::new("git") + create_command("git")? .args(&["checkout", "-b", branch_name]) .current_dir(repo_dir.path()) .output()?; diff --git a/src/modules/git_commit.rs b/src/modules/git_commit.rs index 7484e5031..1382c7ef0 100644 --- a/src/modules/git_commit.rs +++ b/src/modules/git_commit.rs @@ -105,10 +105,10 @@ fn id_to_hex_abbrev(bytes: &[u8], len: usize) -> String { #[cfg(test)] mod tests { use ansi_term::Color; - use std::process::Command; use std::{io, str}; use crate::test::{fixture_repo, FixtureProvider, ModuleRenderer}; + use crate::utils::create_command; #[test] fn show_nothing_on_empty_dir() -> io::Result<()> { @@ -128,7 +128,7 @@ mod tests { fn test_render_commit_hash() -> io::Result<()> { let repo_dir = fixture_repo(FixtureProvider::Git)?; - let mut git_output = Command::new("git") + let mut git_output = create_command("git")? .args(&["rev-parse", "HEAD"]) .current_dir(&repo_dir.path()) .output()? @@ -160,7 +160,7 @@ mod tests { fn test_render_commit_hash_len_override() -> io::Result<()> { let repo_dir = fixture_repo(FixtureProvider::Git)?; - let mut git_output = Command::new("git") + let mut git_output = create_command("git")? .args(&["rev-parse", "HEAD"]) .current_dir(&repo_dir.path()) .output()? @@ -207,12 +207,12 @@ mod tests { fn test_render_commit_hash_only_detached_on_detached() -> io::Result<()> { let repo_dir = fixture_repo(FixtureProvider::Git)?; - Command::new("git") + create_command("git")? .args(&["checkout", "@~1"]) .current_dir(&repo_dir.path()) .output()?; - let mut git_output = Command::new("git") + let mut git_output = create_command("git")? .args(&["rev-parse", "HEAD"]) .current_dir(&repo_dir.path()) .output()? @@ -240,7 +240,7 @@ mod tests { fn test_render_commit_hash_with_tag_enabled() -> io::Result<()> { let repo_dir = fixture_repo(FixtureProvider::Git)?; - let mut git_commit = Command::new("git") + let mut git_commit = create_command("git")? .args(&["rev-parse", "HEAD"]) .current_dir(&repo_dir.path()) .output()? @@ -248,7 +248,7 @@ mod tests { git_commit.truncate(7); let commit_output = str::from_utf8(&git_commit).unwrap().trim(); - let git_tag = Command::new("git") + let git_tag = create_command("git")? .args(&["describe", "--tags", "--exact-match", "HEAD"]) .current_dir(&repo_dir.path()) .output()? @@ -283,17 +283,17 @@ mod tests { fn test_render_commit_hash_only_detached_on_detached_with_tag_enabled() -> io::Result<()> { let repo_dir = fixture_repo(FixtureProvider::Git)?; - Command::new("git") + create_command("git")? .args(&["checkout", "@~1"]) .current_dir(&repo_dir.path()) .output()?; - Command::new("git") + create_command("git")? .args(&["tag", "tagOnDetached", "-m", "Testing tags on detached"]) .current_dir(&repo_dir.path()) .output()?; - let mut git_commit = Command::new("git") + let mut git_commit = create_command("git")? .args(&["rev-parse", "HEAD"]) .current_dir(&repo_dir.path()) .output()? @@ -301,7 +301,7 @@ mod tests { git_commit.truncate(7); let commit_output = str::from_utf8(&git_commit).unwrap().trim(); - let git_tag = Command::new("git") + let git_tag = create_command("git")? .args(&["describe", "--tags", "--exact-match", "HEAD"]) .current_dir(&repo_dir.path()) .output()? @@ -337,7 +337,7 @@ mod tests { let repo_dir = fixture_repo(FixtureProvider::Git)?; - let mut git_commit = Command::new("git") + let mut git_commit = create_command("git")? .args(&["rev-parse", "HEAD"]) .current_dir(&repo_dir.path()) .output()? @@ -345,7 +345,7 @@ mod tests { git_commit.truncate(7); let commit_output = str::from_utf8(&git_commit).unwrap().trim(); - Command::new("git") + create_command("git")? .args(&["tag", "v2", "-m", "Testing tags v2"]) .current_dir(&repo_dir.path()) .output()?; @@ -353,17 +353,17 @@ mod tests { // Wait one second between tags thread::sleep(time::Duration::from_millis(1000)); - Command::new("git") + create_command("git")? .args(&["tag", "v0", "-m", "Testing tags v0", "HEAD~1"]) .current_dir(&repo_dir.path()) .output()?; - Command::new("git") + create_command("git")? .args(&["tag", "v1", "-m", "Testing tags v1"]) .current_dir(&repo_dir.path()) .output()?; - let git_tag = Command::new("git") + let git_tag = create_command("git")? .args(&[ "for-each-ref", "--contains", diff --git a/src/modules/git_metrics.rs b/src/modules/git_metrics.rs index 833faec34..218971d2f 100644 --- a/src/modules/git_metrics.rs +++ b/src/modules/git_metrics.rs @@ -93,11 +93,12 @@ impl<'a> GitDiff<'a> { #[cfg(test)] mod tests { + use crate::utils::create_command; use std::ffi::OsStr; use std::fs::OpenOptions; use std::io::{self, Error, ErrorKind, Write}; use std::path::{Path, PathBuf}; - use std::process::{Command, Stdio}; + use std::process::Stdio; use ansi_term::Color; @@ -193,7 +194,7 @@ mod tests { A: IntoIterator, S: AsRef, { - let mut command = Command::new("git"); + let mut command = create_command("git")?; command .args(args) .stdout(Stdio::null()) diff --git a/src/modules/git_state.rs b/src/modules/git_state.rs index f2d1e1c9d..8c68e9d2b 100644 --- a/src/modules/git_state.rs +++ b/src/modules/git_state.rs @@ -177,9 +177,10 @@ mod tests { use std::fs::OpenOptions; use std::io::{self, Error, ErrorKind, Write}; use std::path::Path; - use std::process::{Command, Stdio}; + use std::process::Stdio; use crate::test::ModuleRenderer; + use crate::utils::create_command; #[test] fn show_nothing_on_empty_dir() -> io::Result<()> { @@ -278,7 +279,7 @@ mod tests { A: IntoIterator, S: AsRef, { - let mut command = Command::new("git"); + let mut command = create_command("git")?; command .args(args) .stdout(Stdio::null()) diff --git a/src/modules/git_status.rs b/src/modules/git_status.rs index 0c921c09c..b526ff284 100644 --- a/src/modules/git_status.rs +++ b/src/modules/git_status.rs @@ -309,9 +309,9 @@ mod tests { use std::fs::{self, File}; use std::io; use std::path::Path; - use std::process::Command; use crate::test::{fixture_repo, FixtureProvider, ModuleRenderer}; + use crate::utils::create_command; /// Right after the calls to git the filesystem state may not have finished /// updating yet causing some of the tests to fail. These barriers are placed @@ -524,7 +524,7 @@ mod tests { create_untracked(&repo_dir.path())?; - Command::new("git") + create_command("git")? .args(&["config", "status.showUntrackedFiles", "no"]) .current_dir(repo_dir.path()) .output()?; @@ -546,7 +546,7 @@ mod tests { create_stash(&repo_dir.path())?; - Command::new("git") + create_command("git")? .args(&["reset", "--hard", "HEAD"]) .current_dir(repo_dir.path()) .output()?; @@ -569,7 +569,7 @@ mod tests { create_stash(&repo_dir.path())?; barrier(); - Command::new("git") + create_command("git")? .args(&["reset", "--hard", "HEAD"]) .current_dir(repo_dir.path()) .output()?; @@ -751,7 +751,7 @@ mod tests { let worktree_dir = tempfile::tempdir()?; let repo_dir = fixture_repo(FixtureProvider::Git)?; - Command::new("git") + create_command("git")? .args(&[ "config", "core.worktree", @@ -781,11 +781,11 @@ mod tests { let repo_dir = fixture_repo(FixtureProvider::Git)?; File::create(repo_dir.path().join("a"))?.sync_all()?; File::create(repo_dir.path().join("b"))?.sync_all()?; - Command::new("git") + create_command("git")? .args(&["add", "--all"]) .current_dir(&repo_dir.path()) .output()?; - Command::new("git") + create_command("git")? .args(&["commit", "-m", "add new files", "--no-gpg-sign"]) .current_dir(&repo_dir.path()) .output()?; @@ -814,7 +814,7 @@ mod tests { fn ahead(repo_dir: &Path) -> io::Result<()> { File::create(repo_dir.join("readme.md"))?.sync_all()?; - Command::new("git") + create_command("git")? .args(&["commit", "-am", "Update readme", "--no-gpg-sign"]) .current_dir(&repo_dir) .output()?; @@ -824,7 +824,7 @@ mod tests { } fn behind(repo_dir: &Path) -> io::Result<()> { - Command::new("git") + create_command("git")? .args(&["reset", "--hard", "HEAD^"]) .current_dir(repo_dir) .output()?; @@ -834,7 +834,7 @@ mod tests { } fn diverge(repo_dir: &Path) -> io::Result<()> { - Command::new("git") + create_command("git")? .args(&["reset", "--hard", "HEAD^"]) .current_dir(repo_dir) .output()?; @@ -842,7 +842,7 @@ mod tests { fs::write(repo_dir.join("Cargo.toml"), " ")?; - Command::new("git") + create_command("git")? .args(&["commit", "-am", "Update readme", "--no-gpg-sign"]) .current_dir(repo_dir) .output()?; @@ -852,7 +852,7 @@ mod tests { } fn create_conflict(repo_dir: &Path) -> io::Result<()> { - Command::new("git") + create_command("git")? .args(&["reset", "--hard", "HEAD^"]) .current_dir(repo_dir) .output()?; @@ -860,19 +860,19 @@ mod tests { fs::write(repo_dir.join("readme.md"), "# goodbye")?; - Command::new("git") + create_command("git")? .args(&["add", "."]) .current_dir(repo_dir) .output()?; barrier(); - Command::new("git") + create_command("git")? .args(&["commit", "-m", "Change readme", "--no-gpg-sign"]) .current_dir(repo_dir) .output()?; barrier(); - Command::new("git") + create_command("git")? .args(&["pull", "--rebase"]) .current_dir(repo_dir) .output()?; @@ -885,7 +885,7 @@ mod tests { File::create(repo_dir.join("readme.md"))?.sync_all()?; barrier(); - Command::new("git") + create_command("git")? .args(&["stash", "--all"]) .current_dir(repo_dir) .output()?; @@ -903,7 +903,7 @@ mod tests { fn create_added(repo_dir: &Path) -> io::Result<()> { File::create(repo_dir.join("license"))?.sync_all()?; - Command::new("git") + create_command("git")? .args(&["add", "-A", "-N"]) .current_dir(repo_dir) .output()?; @@ -921,7 +921,7 @@ mod tests { fn create_staged(repo_dir: &Path) -> io::Result<()> { File::create(repo_dir.join("license"))?.sync_all()?; - Command::new("git") + create_command("git")? .args(&["add", "."]) .current_dir(repo_dir) .output()?; @@ -931,13 +931,13 @@ mod tests { } fn create_renamed(repo_dir: &Path) -> io::Result<()> { - Command::new("git") + create_command("git")? .args(&["mv", "readme.md", "readme.md.bak"]) .current_dir(repo_dir) .output()?; barrier(); - Command::new("git") + create_command("git")? .args(&["add", "-A"]) .current_dir(repo_dir) .output()?; diff --git a/src/modules/hg_branch.rs b/src/modules/hg_branch.rs index b0d19edbb..a30c2a538 100644 --- a/src/modules/hg_branch.rs +++ b/src/modules/hg_branch.rs @@ -103,9 +103,9 @@ mod tests { use std::fs; use std::io; use std::path::Path; - use std::process::Command; use crate::test::{fixture_repo, FixtureProvider, ModuleRenderer}; + use crate::utils::create_command; enum Expect<'a> { BranchName(&'a str), @@ -335,7 +335,7 @@ mod tests { } fn run_hg(args: &[&str], repo_dir: &Path) -> io::Result<()> { - Command::new("hg") + create_command("hg")? .args(args) .current_dir(&repo_dir) .output()?; diff --git a/src/modules/rust.rs b/src/modules/rust.rs index 2dcf73087..ad004cdbc 100644 --- a/src/modules/rust.rs +++ b/src/modules/rust.rs @@ -1,6 +1,6 @@ use std::fs; use std::path::Path; -use std::process::{Command, Output}; +use std::process::Output; use serde::Deserialize; @@ -8,6 +8,7 @@ use super::{Context, Module, RootModuleConfig}; use crate::configs::rust::RustConfig; use crate::formatter::{StringFormatter, VersionFormatter}; +use crate::utils::create_command; /// Creates a module with the current Rust version pub fn module<'a>(context: &'a Context) -> Option> { @@ -99,7 +100,8 @@ fn env_rustup_toolchain(context: &Context) -> Option { } fn execute_rustup_override_list(cwd: &Path) -> Option { - let Output { stdout, .. } = Command::new("rustup") + let Output { stdout, .. } = create_command("rustup") + .ok()? .args(&["override", "list"]) .output() .ok()?; @@ -188,9 +190,8 @@ fn find_rust_toolchain_file(context: &Context) -> Option { } fn execute_rustup_run_rustc_version(toolchain: &str) -> RustupRunRustcVersionOutcome { - Command::new("rustup") - .args(&["run", toolchain, "rustc", "--version"]) - .output() + create_command("rustup") + .and_then(|mut cmd| cmd.args(&["run", toolchain, "rustc", "--version"]).output()) .map(extract_toolchain_from_rustup_run_rustc_version) .unwrap_or(RustupRunRustcVersionOutcome::RustupNotWorking) } @@ -212,7 +213,7 @@ fn extract_toolchain_from_rustup_run_rustc_version(output: Output) -> RustupRunR } fn execute_rustc_version() -> Option { - match Command::new("rustc").arg("--version").output() { + match create_command("rustc").ok()?.arg("--version").output() { Ok(output) => Some(String::from_utf8(output.stdout).unwrap()), Err(_) => None, } diff --git a/src/test/mod.rs b/src/test/mod.rs index 299b6f309..890e46f85 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -1,11 +1,13 @@ use crate::context::{Context, Shell}; use crate::logger::StarshipLogger; -use crate::{config::StarshipConfig, utils::CommandOutput}; +use crate::{ + config::StarshipConfig, + utils::{create_command, CommandOutput}, +}; use log::{Level, LevelFilter}; use once_cell::sync::Lazy; use std::io; use std::path::PathBuf; -use std::process::Command; use tempfile::TempDir; static FIXTURE_DIR: Lazy = @@ -151,24 +153,24 @@ pub fn fixture_repo(provider: FixtureProvider) -> io::Result { FixtureProvider::Git => { let path = tempfile::tempdir()?; - Command::new("git") + create_command("git")? .current_dir(path.path()) .args(&["clone", "-b", "master"]) .arg(GIT_FIXTURE.as_os_str()) .arg(&path.path()) .output()?; - Command::new("git") + create_command("git")? .args(&["config", "--local", "user.email", "starship@example.com"]) .current_dir(&path.path()) .output()?; - Command::new("git") + create_command("git")? .args(&["config", "--local", "user.name", "starship"]) .current_dir(&path.path()) .output()?; - Command::new("git") + create_command("git")? .args(&["reset", "--hard", "HEAD"]) .current_dir(&path.path()) .output()?; @@ -178,7 +180,7 @@ pub fn fixture_repo(provider: FixtureProvider) -> io::Result { FixtureProvider::Hg => { let path = tempfile::tempdir()?; - Command::new("hg") + create_command("hg")? .current_dir(path.path()) .arg("clone") .arg(HG_FIXTURE.as_os_str()) diff --git a/src/utils.rs b/src/utils.rs index def0a5fa5..b9699af0b 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,7 +1,8 @@ use process_control::{ChildExt, Timeout}; +use std::ffi::OsStr; use std::fmt::Debug; use std::fs::read_to_string; -use std::io::Result; +use std::io::{Error, ErrorKind, Result}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::time::{Duration, Instant}; @@ -23,6 +24,33 @@ pub fn read_file + Debug>(file_name: P) -> Result { result } +/// Attempt to resolve `binary_name` from and creates a new `Command` pointing at it +/// This allows executing cmd files on Windows and prevents running executable from cwd on Windows +/// This function also initialises std{err,out,in} to protect against processes changing the console mode +pub fn create_command>(binary_name: T) -> Result { + let binary_name = binary_name.as_ref(); + log::trace!("Creating Command struct with binary name {:?}", binary_name); + + let full_path = match which::which(binary_name) { + Ok(full_path) => { + log::trace!("Using {:?} as {:?}", full_path, binary_name); + full_path + } + Err(error) => { + log::trace!("Unable to find {:?} in PATH, {:?}", binary_name, error); + return Err(Error::new(ErrorKind::NotFound, error)); + } + }; + + #[allow(clippy::disallowed_method)] + let mut cmd = Command::new(full_path); + cmd.stderr(Stdio::piped()) + .stdout(Stdio::piped()) + .stdin(Stdio::null()); + + Ok(cmd) +} + #[derive(Debug, Clone)] pub struct CommandOutput { pub stdout: String, @@ -324,6 +352,7 @@ fn internal_exec_cmd(cmd: &str, args: &[&str], time_limit: Duration) -> Option