diff --git a/src-tauri/src/commands/git.rs b/src-tauri/src/commands/git.rs index c906837c4..3dc30c196 100644 --- a/src-tauri/src/commands/git.rs +++ b/src-tauri/src/commands/git.rs @@ -551,14 +551,25 @@ async fn run_git_output_with_env_source_async( } } +fn build_git_command(git: &Path, path: &Path, args: &[&str]) -> TokioCommand { + let mut command = TokioCommand::new(git); + command.args(args).current_dir(path).kill_on_drop(true); + command +} + async fn run_git_once_async( path: &Path, args: &[&str], command_timeout: Duration, env_source: EnvSource, ) -> Result { - let mut command = TokioCommand::new("git"); - command.args(args).current_dir(path).kill_on_drop(true); + let git = dir_env::resolve_control_executable("git").ok_or_else(|| { + GitRunError::Spawn(io::Error::new( + io::ErrorKind::NotFound, + "trusted Git executable was not found", + )) + })?; + let mut command = build_git_command(&git, path, args); apply_git_environment( &mut command, @@ -1391,6 +1402,31 @@ exit \"$?\" ); } + #[cfg(windows)] + #[test] + fn git_command_program_is_not_resolved_from_captured_path() { + let trusted_git = PathBuf::from(r"C:\Program Files\Git\cmd\git.exe"); + let project_bin = PathBuf::from(r"C:\repo\.hermit\bin"); + let mut command = build_git_command(&trusted_git, Path::new(r"C:\repo"), &["status"]); + let env = HashMap::from([( + "Path".to_string(), + std::env::join_paths([project_bin.clone(), PathBuf::from(r"C:\Windows\System32")]) + .expect("captured PATH") + .to_string_lossy() + .into_owned(), + )]); + + apply_captured_git_env(&mut command, &env); + + assert_eq!(command.as_std().get_program(), trusted_git.as_os_str()); + assert_eq!( + std::env::split_paths(&env_value(&command, "PATH").expect("command PATH")) + .next() + .as_deref(), + Some(project_bin.as_path()) + ); + } + #[test] fn env_source_policy_uses_captured_for_hook_sensitive_mutations() { assert_eq!( diff --git a/src-tauri/src/commands/pull_requests.rs b/src-tauri/src/commands/pull_requests.rs index a567a8c31..b55e8fba4 100644 --- a/src-tauri/src/commands/pull_requests.rs +++ b/src-tauri/src/commands/pull_requests.rs @@ -123,12 +123,8 @@ fn fallback_summary(reference: &PullRequestRef) -> PullRequestSummary { } } -async fn fetch_summary( - reference: PullRequestRef, - cwd: &Path, - env: Option<&HashMap>, -) -> PullRequestSummary { - let mut command = Command::new("gh"); +fn build_gh_command(gh: &Path, reference: &PullRequestRef, cwd: &Path) -> Command { + let mut command = Command::new(gh); command .args([ "pr", @@ -140,6 +136,20 @@ async fn fetch_summary( .current_dir(cwd) .env("GH_PROMPT_DISABLED", "1") .kill_on_drop(true); + command +} + +async fn fetch_summary( + reference: PullRequestRef, + cwd: &Path, + env: Option<&HashMap>, + gh: Option<&Path>, +) -> PullRequestSummary { + let Some(gh) = gh else { + log::debug!("Trusted GitHub CLI executable was not found"); + return fallback_summary(&reference); + }; + let mut command = build_gh_command(gh, &reference, cwd); if let Some(env) = env { command.env_clear().envs(env).env("GH_PROMPT_DISABLED", "1"); } @@ -204,12 +214,16 @@ pub async fn get_pull_request_summaries( .filter(|path| path.is_dir()) .or_else(dirs::home_dir) .ok_or_else(|| "Could not resolve a directory for GitHub CLI".to_string())?; + // Resolve before capturing the project environment, which can prepend the + // repository's Hermit bin on Windows. + let gh = dir_env::resolve_control_executable("gh"); let env = dir_env::capture_dir_env(&cwd, ENV_CAPTURE_TIMEOUT).await; Ok(stream::iter(references.into_iter().map(|reference| { let cwd = cwd.clone(); let env = env.clone(); - async move { fetch_summary(reference, &cwd, env.as_ref()).await } + let gh = gh.clone(); + async move { fetch_summary(reference, &cwd, env.as_ref(), gh.as_deref()).await } })) .buffered(GH_CONCURRENCY) .collect() @@ -220,6 +234,30 @@ pub async fn get_pull_request_summaries( mod tests { use super::*; + #[cfg(windows)] + #[test] + fn gh_command_program_is_not_resolved_from_captured_path() { + let reference = parse_github_pull_request_url("https://github.com/block/berd/pull/1") + .expect("pull request"); + let trusted_gh = PathBuf::from(r"C:\Program Files\GitHub CLI\gh.exe"); + let project_bin = PathBuf::from(r"C:\repo\.hermit\bin"); + let path = std::env::join_paths([project_bin.clone(), PathBuf::from(r"C:\Windows")]) + .expect("captured PATH"); + let mut command = build_gh_command(&trusted_gh, &reference, Path::new(r"C:\repo")); + command.env_clear().env("Path", path); + + assert_eq!(command.as_std().get_program(), trusted_gh.as_os_str()); + let command_path = command + .as_std() + .get_envs() + .find_map(|(key, value)| key.eq_ignore_ascii_case("PATH").then_some(value).flatten()) + .expect("command PATH"); + assert_eq!( + std::env::split_paths(command_path).next().as_deref(), + Some(project_bin.as_path()) + ); + } + fn check(conclusion: Option<&str>, status: Option<&str>, state: Option<&str>) -> GhStatusCheck { GhStatusCheck { conclusion: conclusion.map(str::to_string), diff --git a/src-tauri/src/services/dir_env.rs b/src-tauri/src/services/dir_env.rs index 2fa3bd8f0..ab1c04053 100644 --- a/src-tauri/src/services/dir_env.rs +++ b/src-tauri/src/services/dir_env.rs @@ -167,6 +167,13 @@ pub async fn capture_terminal_env(dir: &Path) -> HashMap { platform::capture_terminal_env(dir, HOME_ENV_CAPTURE_TIMEOUT).await } +/// Resolve a control executable before any repository-local PATH is applied. +/// Windows returns a canonical absolute path; other platforms preserve the +/// existing PATH lookup behavior. +pub(crate) fn resolve_control_executable(name: &str) -> Option { + platform::resolve_control_executable(name) +} + pub async fn capture_home_interactive_env_with_timeout( timeout_duration: Duration, ) -> HashMap { @@ -385,7 +392,45 @@ mod tests { let git = platform::find_file_on_windows_path("git.exe", env_key::get(&env, "PATH")) .expect("trusted git"); - assert_eq!(git, trusted_bin.join("git.exe")); + assert_eq!( + git, + trusted_bin + .join("git.exe") + .canonicalize() + .expect("canonical trusted Git") + ); + } + + #[cfg(windows)] + #[test] + fn windows_control_executables_ignore_repository_hermit_bins() { + let temp = tempfile::tempdir().expect("temp dir"); + let repository_bin = temp.path().join("repo").join(".hermit").join("bin"); + let trusted_bin = temp.path().join("trusted").join("bin"); + std::fs::create_dir_all(&repository_bin).expect("repository Hermit bin"); + std::fs::create_dir_all(&trusted_bin).expect("trusted bin"); + for executable in ["git.exe", "gh.exe"] { + std::fs::write(repository_bin.join(executable), b"sentinel") + .expect("repository sentinel"); + std::fs::write(trusted_bin.join(executable), b"trusted").expect("trusted executable"); + } + let path = std::env::join_paths([repository_bin.clone(), trusted_bin.clone()]) + .expect("fixture PATH") + .to_string_lossy() + .into_owned(); + let env = HashMap::from([("Path".to_string(), path)]); + + for executable in ["git", "gh"] { + assert_eq!( + platform::resolve_control_executable_in_env(executable, env.clone()), + Some( + trusted_bin + .join(format!("{executable}.exe")) + .canonicalize() + .expect("canonical trusted executable") + ) + ); + } } #[test] diff --git a/src-tauri/src/services/dir_env/unix.rs b/src-tauri/src/services/dir_env/unix.rs index 4b5a9ad90..be500f36d 100644 --- a/src-tauri/src/services/dir_env/unix.rs +++ b/src-tauri/src/services/dir_env/unix.rs @@ -7,6 +7,10 @@ use std::{ }; use tokio::{io::AsyncWriteExt, process::Command, time::timeout}; +pub(crate) fn resolve_control_executable(name: &str) -> Option { + Some(PathBuf::from(name)) +} + pub(super) async fn capture_dir_env_uncached( dir: &Path, timeout_duration: Duration, diff --git a/src-tauri/src/services/dir_env/windows.rs b/src-tauri/src/services/dir_env/windows.rs index 592665415..ad736f88a 100644 --- a/src-tauri/src/services/dir_env/windows.rs +++ b/src-tauri/src/services/dir_env/windows.rs @@ -77,10 +77,29 @@ pub(crate) fn find_project_hermit_bin(dir: &Path) -> Option { find_project_hermit_bin_within(start, &repo_root) } +pub(crate) fn resolve_control_executable(name: &str) -> Option { + resolve_control_executable_in_env(name, dedupe_env_case_insensitive(std::env::vars())) +} + +pub(crate) fn resolve_control_executable_in_env( + name: &str, + mut env: HashMap, +) -> Option { + strip_untrusted_windows_tool_state(&mut env); + let file_name = if name.to_ascii_lowercase().ends_with(".exe") { + name.to_string() + } else { + format!("{name}.exe") + }; + find_file_on_windows_path(&file_name, env_key::get(&env, "PATH")) +} + pub(crate) fn find_file_on_windows_path(file_name: &str, path: Option<&str>) -> Option { std::env::split_paths(path?) .map(|dir| dir.join(file_name)) - .find(|candidate| candidate.is_file()) + .find(|candidate| candidate.is_file())? + .canonicalize() + .ok() } fn prepend_dir_to_windows_path(env: &mut HashMap, dir: &Path) {