diff --git a/codex-rs/git-utils/src/apply.rs b/codex-rs/git-utils/src/apply.rs index 92237ad797..12c999e9c8 100644 --- a/codex-rs/git-utils/src/apply.rs +++ b/codex-rs/git-utils/src/apply.rs @@ -13,6 +13,12 @@ use std::io; use std::path::Path; use std::path::PathBuf; +use crate::FsmonitorOverride; +use crate::git_command::GitRunner; +use crate::safe_git::DISABLED_HOOKS_PATH; +#[cfg(test)] +use crate::safe_git::isolate_git_command_environment; + /// Parameters for invoking [`apply_git_patch`]. #[derive(Debug, Clone)] pub struct ApplyGitRequest { @@ -39,7 +45,9 @@ pub struct ApplyGitResult { /// When [`ApplyGitRequest::preflight`] is `true`, this behaves like `git apply --check` and /// leaves the working tree untouched while still parsing the command output for diagnostics. pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { - let git_root = resolve_git_root(&req.cwd)?; + let git = GitRunner::for_cwd_io(&req.cwd)?; + let mut cfg_parts = configured_git_config_parts(); + let git_root = resolve_git_root(&git, &req.cwd, &cfg_parts)?; // Write unified diff into a temporary file let (tmpdir, patch_path) = write_temp_patch(&req.diff)?; @@ -57,18 +65,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { args.push("-R".into()); } - // Optional: additional git config via env knob (defaults OFF) - let mut cfg_parts: Vec = Vec::new(); - if let Ok(cfg) = std::env::var("CODEX_APPLY_GIT_CFG") { - for pair in cfg.split(',') { - let p = pair.trim(); - if p.is_empty() || !p.contains('=') { - continue; - } - cfg_parts.push("-c".into()); - cfg_parts.push(p.to_string()); - } - } + cfg_parts.extend(safe_git_config_parts()); args.push(patch_path.to_string_lossy().to_string()); @@ -80,7 +77,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { } check_args.push(patch_path.to_string_lossy().to_string()); let rendered = render_command_for_log(&git_root, &cfg_parts, &check_args); - let (c_code, c_out, c_err) = run_git(&git_root, &cfg_parts, &check_args)?; + let (c_code, c_out, c_err) = run_git(&git, &git_root, &cfg_parts, &check_args)?; let (mut applied_paths, mut skipped_paths, mut conflicted_paths) = parse_git_apply_output(&c_out, &c_err); applied_paths.sort(); @@ -101,7 +98,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { } let cmd_for_log = render_command_for_log(&git_root, &cfg_parts, &args); - let (code, stdout, stderr) = run_git(&git_root, &cfg_parts, &args)?; + let (code, stdout, stderr) = run_git(&git, &git_root, &cfg_parts, &args)?; let (mut applied_paths, mut skipped_paths, mut conflicted_paths) = parse_git_apply_output(&stdout, &stderr); @@ -123,12 +120,19 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { }) } -fn resolve_git_root(cwd: &Path) -> io::Result { - let out = local_git_command() +fn resolve_git_root( + git: &GitRunner, + cwd: &Path, + git_config_args: &[String], +) -> io::Result { + let requested_cwd = std::fs::canonicalize(cwd)?; + let mut command = git.command(); + command + .args(git_config_args) .arg("rev-parse") .arg("--show-toplevel") - .current_dir(cwd) - .output()?; + .current_dir(&requested_cwd); + let out = git.output(command)?; let code = out.status.code().unwrap_or(-1); if code != 0 { return Err(io::Error::other(format!( @@ -137,8 +141,47 @@ fn resolve_git_root(cwd: &Path) -> io::Result { String::from_utf8_lossy(&out.stderr) ))); } - let root = String::from_utf8_lossy(&out.stdout).trim().to_string(); - Ok(PathBuf::from(root)) + let reported_root = PathBuf::from(String::from_utf8_lossy(&out.stdout).trim()); + let root = std::fs::canonicalize(&reported_root)?; + let expected_root = crate::get_git_repo_root(&requested_cwd) + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::PermissionDenied, + format!( + "refusing to apply a patch because Git resolved worktree {} without a .git marker above requested cwd {}", + root.display(), + requested_cwd.display() + ), + ) + }) + .and_then(std::fs::canonicalize)?; + if root != expected_root { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + format!( + "refusing to apply a patch because Git resolved worktree {} instead of expected worktree {} for requested cwd {}", + root.display(), + expected_root.display(), + requested_cwd.display() + ), + )); + } + Ok(root) +} + +fn configured_git_config_parts() -> Vec { + let mut cfg_parts = Vec::new(); + if let Ok(cfg) = std::env::var("CODEX_APPLY_GIT_CFG") { + for pair in cfg.split(',') { + let pair = pair.trim(); + if pair.is_empty() || !pair.contains('=') { + continue; + } + cfg_parts.push("-c".to_string()); + cfg_parts.push(pair.to_string()); + } + } + cfg_parts } fn write_temp_patch(diff: &str) -> io::Result<(tempfile::TempDir, PathBuf)> { @@ -148,25 +191,34 @@ fn write_temp_patch(diff: &str) -> io::Result<(tempfile::TempDir, PathBuf)> { Ok((dir, path)) } -fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32, String, String)> { - let mut cmd = local_git_command(); +fn run_git( + git: &GitRunner, + cwd: &Path, + git_cfg: &[String], + args: &[String], +) -> io::Result<(i32, String, String)> { + let mut cmd = git.command(); for p in git_cfg { cmd.arg(p); } for a in args { cmd.arg(a); } - let out = cmd.current_dir(cwd).output()?; + cmd.current_dir(cwd); + let out = git.output(cmd)?; let code = out.status.code().unwrap_or(-1); let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); let stderr = String::from_utf8_lossy(&out.stderr).into_owned(); Ok((code, stdout, stderr)) } -fn local_git_command() -> std::process::Command { - let mut command = std::process::Command::new("git"); - command.envs(crate::local_only_git_env()); - command +fn safe_git_config_parts() -> Vec { + vec![ + "-c".to_string(), + format!("core.hooksPath={DISABLED_HOOKS_PATH}"), + "-c".to_string(), + FsmonitorOverride::Disabled.git_config_arg().to_string(), + ] } fn quote_shell(s: &str) -> String { @@ -324,6 +376,7 @@ fn unescape_c_string(input: &str) -> String { /// Stage only the files that actually exist on disk for the given diff. pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> { + let git = GitRunner::for_cwd_io(git_root)?; let paths = extract_paths_from_patch(diff); let mut existing: Vec = Vec::new(); for p in paths { @@ -335,13 +388,15 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> { if existing.is_empty() { return Ok(()); } - let mut cmd = local_git_command(); + let mut cmd = git.command(); + cmd.args(safe_git_config_parts()); cmd.arg("add"); cmd.arg("--"); for p in &existing { cmd.arg(OsStr::new(p)); } - let out = cmd.current_dir(git_root).output()?; + cmd.current_dir(git_root); + let out = git.output(cmd)?; let _code = out.status.code().unwrap_or(-1); // We do not hard fail staging; best-effort is OK. Return Ok even on non-zero. Ok(()) @@ -605,6 +660,7 @@ mod transport_tests; #[cfg(test)] mod tests { use super::*; + use std::ffi::OsStr; use std::path::Path; use std::sync::Mutex; use std::sync::OnceLock; @@ -615,7 +671,9 @@ mod tests { } fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) { - let out = std::process::Command::new(args[0]) + let mut command = std::process::Command::new(args[0]); + isolate_git_command_environment(&mut command); + let out = command .args(&args[1..]) .current_dir(cwd) .output() @@ -627,6 +685,27 @@ mod tests { ) } + fn run_isolated_test(test_name: &str, env: &[(&str, &OsStr)]) { + let mut command = std::process::Command::new(std::env::current_exe().expect("test binary")); + isolate_git_command_environment(&mut command); + command + .arg(test_name) + .arg("--exact") + .arg("--nocapture") + .env("CODEX_GIT_UTILS_APPLY_ENV_CHILD", "1") + .env("RUST_TEST_THREADS", "1"); + for (name, value) in env { + command.env(name, value); + } + let output = command.output().expect("run isolated test process"); + assert!( + output.status.success(), + "isolated test {test_name} failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + fn init_repo() -> tempfile::TempDir { let dir = tempfile::tempdir().expect("tempdir"); let root = dir.path(); @@ -678,10 +757,12 @@ mod tests { let _g = env_lock().lock().unwrap(); let repo = init_repo(); let root = repo.path(); + let nested_cwd = root.join("nested"); + std::fs::create_dir(&nested_cwd).expect("nested cwd"); let diff = "diff --git a/hello.txt b/hello.txt\nnew file mode 100644\n--- /dev/null\n+++ b/hello.txt\n@@ -0,0 +1,2 @@\n+hello\n+world\n"; let req = ApplyGitRequest { - cwd: root.to_path_buf(), + cwd: nested_cwd, diff: diff.to_string(), revert: false, preflight: false, @@ -692,6 +773,58 @@ mod tests { assert!(root.join("hello.txt").exists()); } + #[test] + fn apply_uses_cwd_repo_despite_inherited_repository_selectors() { + let _g = env_lock().lock().unwrap(); + if std::env::var_os("CODEX_GIT_UTILS_APPLY_ENV_CHILD").is_none() { + let alternate = init_repo(); + let alternate_root = alternate.path(); + std::fs::write(alternate_root.join("sentinel.txt"), "alternate\n") + .expect("write alternate sentinel"); + let (add_code, _, add_err) = run(alternate_root, &["git", "add", "sentinel.txt"]); + assert_eq!(add_code, 0, "add alternate sentinel: {add_err}"); + let (commit_code, _, commit_err) = + run(alternate_root, &["git", "commit", "-m", "alternate"]); + assert_eq!(commit_code, 0, "commit alternate sentinel: {commit_err}"); + + let alternate_git_dir = alternate_root.join(".git"); + let alternate_index = alternate_git_dir.join("index"); + run_isolated_test( + "apply::tests::apply_uses_cwd_repo_despite_inherited_repository_selectors", + &[ + ("GIT_DIR", alternate_git_dir.as_os_str()), + ("GIT_WORK_TREE", alternate_root.as_os_str()), + ("GIT_COMMON_DIR", alternate_git_dir.as_os_str()), + ("GIT_INDEX_FILE", alternate_index.as_os_str()), + ("GIT_PREFIX", OsStr::new("elsewhere/")), + ], + ); + assert_eq!( + read_file_normalized(&alternate_root.join("sentinel.txt")), + "alternate\n" + ); + return; + } + + let repo = init_repo(); + let root = repo.path(); + std::fs::write(root.join("file.txt"), "old\n").expect("write target file"); + let (add_code, _, add_err) = run(root, &["git", "add", "file.txt"]); + assert_eq!(add_code, 0, "add target file: {add_err}"); + let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "target"]); + assert_eq!(commit_code, 0, "commit target file: {commit_err}"); + + let result = apply_git_patch(&ApplyGitRequest { + cwd: root.to_path_buf(), + diff: "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-old\n+new\n".to_string(), + revert: false, + preflight: false, + }) + .expect("apply in cwd-selected repository"); + assert_eq!(result.exit_code, 0); + assert_eq!(read_file_normalized(&root.join("file.txt")), "new\n"); + } + #[test] fn apply_modify_conflict() { let _g = env_lock().lock().unwrap(); @@ -854,4 +987,30 @@ diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1 "non-preflight path should not use --check" ); } + + #[test] + fn resolve_git_root_rejects_core_worktree_redirection() { + let temp = tempfile::tempdir().expect("tempdir"); + let attacker = temp.path().join("attacker"); + let victim = temp.path().join("victim"); + std::fs::create_dir_all(&attacker).expect("attacker"); + std::fs::create_dir_all(&victim).expect("victim"); + let (init_code, _, init_err) = run(&attacker, &["git", "init"]); + assert_eq!(init_code, 0, "init attacker repo: {init_err}"); + + for redirected_worktree in [&victim, temp.path()] { + let redirected_worktree = redirected_worktree.to_string_lossy(); + let (config_code, _, config_err) = run( + &attacker, + &["git", "config", "core.worktree", &redirected_worktree], + ); + assert_eq!(config_code, 0, "configure core.worktree: {config_err}"); + + let git = GitRunner::for_cwd_io(&attacker).expect("trusted Git"); + let error = + resolve_git_root(&git, &attacker, &[]).expect_err("reject redirected worktree"); + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied); + assert!(error.to_string().contains("instead of expected worktree")); + } + } } diff --git a/codex-rs/git-utils/src/errors.rs b/codex-rs/git-utils/src/errors.rs index 41f7b6df49..0615a39421 100644 --- a/codex-rs/git-utils/src/errors.rs +++ b/codex-rs/git-utils/src/errors.rs @@ -33,3 +33,11 @@ pub enum GitToolingError { #[error(transparent)] Io(#[from] std::io::Error), } + +#[derive(Clone, Debug, Error, PartialEq, Eq)] +pub(crate) enum GitReadError { + #[error("no trusted Git executable is available")] + NoTrustedGit, + #[error("{path:?} is not a Git repository")] + NotRepository { path: PathBuf }, +} diff --git a/codex-rs/git-utils/src/git_command.rs b/codex-rs/git-utils/src/git_command.rs new file mode 100644 index 0000000000..997578b839 --- /dev/null +++ b/codex-rs/git-utils/src/git_command.rs @@ -0,0 +1,207 @@ +use std::ffi::OsStr; +use std::io; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; + +use crate::errors::GitReadError; +use crate::git_config::path_is_within; +use crate::safe_git::isolate_git_command_environment; + +/// A Git executable outside the repository-controlled roots for one operation. +#[derive(Clone, Debug)] +pub(crate) struct GitRunner { + executable: PathBuf, +} + +struct UntrustedGitLocations { + roots: Vec, + common_dir: Option, +} + +impl GitRunner { + pub(crate) fn for_cwd(cwd: &Path) -> Result { + let locations = untrusted_git_locations_for_cwd(cwd)?; + let search_path = std::env::var_os("PATH").ok_or(GitReadError::NoTrustedGit)?; + Self::from_search_path(&locations, &search_path) + } + + pub(crate) fn for_cwd_io(cwd: &Path) -> io::Result { + Self::for_cwd(cwd).map_err(|error| io::Error::new(io::ErrorKind::NotFound, error)) + } + + pub(crate) fn command(&self) -> Command { + Command::new(&self.executable) + } + + pub(crate) fn output(&self, mut command: Command) -> io::Result { + isolate_git_command_environment(&mut command); + command.envs(crate::local_only_git_env()); + command.output() + } + + fn from_search_path( + untrusted: &UntrustedGitLocations, + search_path: &OsStr, + ) -> Result { + for directory in std::env::split_paths(search_path) { + if !directory.is_absolute() { + continue; + } + let candidate = directory.join(git_executable_name()); + if path_is_untrusted(&candidate, untrusted) { + continue; + } + let Ok(canonical_parent) = std::fs::canonicalize(&directory) else { + continue; + }; + if path_is_untrusted(&canonical_parent, untrusted) { + continue; + } + let Ok(canonical_candidate) = std::fs::canonicalize(&candidate) else { + continue; + }; + if path_is_untrusted(&canonical_candidate, untrusted) + || !is_native_executable_file(&canonical_candidate) + { + continue; + } + return Ok(Self { + // Preserve multicall spelling because argv[0] may select mode. + executable: candidate, + }); + } + Err(GitReadError::NoTrustedGit) + } +} + +fn untrusted_git_locations_for_cwd(cwd: &Path) -> Result { + let canonical_cwd = std::fs::canonicalize(cwd).map_err(|_| GitReadError::NotRepository { + path: cwd.to_path_buf(), + })?; + let worktree_root = crate::get_git_repo_root(&canonical_cwd) + .and_then(|root| std::fs::canonicalize(root).ok()) + .unwrap_or_else(|| canonical_cwd.clone()); + let mut roots = vec![worktree_root.clone()]; + let dot_git = worktree_root.join(".git"); + let common_dir = match std::fs::symlink_metadata(&dot_git) { + Ok(_) => Some(resolve_common_git_dir(&dot_git).map_err(|()| GitReadError::NoTrustedGit)?), + Err(error) if error.kind() == io::ErrorKind::NotFound => None, + Err(_) => return Err(GitReadError::NoTrustedGit), + }; + if let Some(common_dir) = &common_dir + && !path_is_within(common_dir, &worktree_root) + { + roots.push(common_dir.clone()); + } + Ok(UntrustedGitLocations { roots, common_dir }) +} + +fn path_is_untrusted(path: &Path, locations: &UntrustedGitLocations) -> bool { + if locations + .roots + .iter() + .any(|root| path_is_within(path, root)) + { + return true; + } + locations + .common_dir + .as_deref() + .is_some_and(|common_dir| path_is_in_worktree_for_common_dir(path, common_dir)) +} + +fn path_is_in_worktree_for_common_dir(path: &Path, expected_common_dir: &Path) -> bool { + let path = if path.is_dir() { + path + } else { + path.parent().unwrap_or(path) + }; + for ancestor in path.ancestors() { + let dot_git = ancestor.join(".git"); + match std::fs::symlink_metadata(&dot_git) { + Ok(_) => match resolve_common_git_dir(&dot_git) { + Ok(common_dir) if paths_equal(&common_dir, expected_common_dir) => return true, + Ok(_) => {} + Err(()) => return true, + }, + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(_) => return true, + } + } + false +} + +fn paths_equal(left: &Path, right: &Path) -> bool { + path_is_within(left, right) && path_is_within(right, left) +} + +fn resolve_common_git_dir(dot_git: &Path) -> Result { + if dot_git.is_dir() { + return std::fs::canonicalize(dot_git).map_err(|_| ()); + } + let contents = std::fs::read_to_string(dot_git).map_err(|_| ())?; + let git_dir = contents + .trim() + .strip_prefix("gitdir:") + .map(str::trim) + .filter(|path| !path.is_empty()) + .ok_or(())?; + let git_dir = canonicalize_from(dot_git.parent().ok_or(())?, git_dir)?; + let commondir = git_dir.join("commondir"); + if commondir.is_file() { + let common_dir = std::fs::read_to_string(commondir).map_err(|_| ())?; + let common_dir = common_dir.trim(); + if common_dir.is_empty() { + return Err(()); + } + return canonicalize_from(&git_dir, common_dir); + } + if git_dir + .parent() + .is_some_and(|parent| parent.file_name() == Some(OsStr::new("worktrees"))) + { + return std::fs::canonicalize(git_dir.parent().and_then(Path::parent).ok_or(())?) + .map_err(|_| ()); + } + Ok(git_dir) +} + +fn canonicalize_from(base: &Path, path: &str) -> Result { + std::fs::canonicalize(base.join(path)).map_err(|_| ()) +} + +#[cfg(windows)] +fn git_executable_name() -> &'static str { + "git.exe" +} + +#[cfg(not(windows))] +fn git_executable_name() -> &'static str { + "git" +} + +#[cfg(unix)] +fn is_native_executable_file(path: &Path) -> bool { + use std::os::unix::fs::PermissionsExt; + + std::fs::metadata(path) + .is_ok_and(|metadata| metadata.is_file() && metadata.permissions().mode() & 0o111 != 0) +} + +#[cfg(windows)] +fn is_native_executable_file(path: &Path) -> bool { + path.extension() + .and_then(OsStr::to_str) + .is_some_and(|extension| extension.eq_ignore_ascii_case("exe")) + && std::fs::metadata(path).is_ok_and(|metadata| metadata.is_file()) +} + +#[cfg(not(any(unix, windows)))] +fn is_native_executable_file(path: &Path) -> bool { + std::fs::metadata(path).is_ok_and(|metadata| metadata.is_file()) +} + +#[cfg(test)] +#[path = "git_command_tests.rs"] +mod tests; diff --git a/codex-rs/git-utils/src/git_command_tests.rs b/codex-rs/git-utils/src/git_command_tests.rs new file mode 100644 index 0000000000..29debcd749 --- /dev/null +++ b/codex-rs/git-utils/src/git_command_tests.rs @@ -0,0 +1,194 @@ +use super::*; +use pretty_assertions::assert_eq; +use std::process::Command; + +#[cfg(unix)] +fn write_executable(path: &Path, body: &str) { + use std::os::unix::fs::PermissionsExt; + + std::fs::write(path, body).expect("write executable"); + let mut permissions = std::fs::metadata(path) + .expect("executable metadata") + .permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(path, permissions).expect("set executable permissions"); +} + +fn run_git(cwd: &Path, args: &[&str]) { + let mut command = Command::new("git"); + isolate_git_command_environment(&mut command); + let status = command + .args(args) + .current_dir(cwd) + .status() + .expect("run real Git"); + assert!(status.success(), "git {args:?} failed"); +} + +fn write_git_candidate(directory: &Path) { + std::fs::create_dir_all(directory).expect("create candidate directory"); + let candidate = directory.join(git_executable_name()); + #[cfg(unix)] + write_executable(&candidate, "#!/bin/sh\nexit 0\n"); + #[cfg(windows)] + std::fs::write(candidate, b"MZ").expect("write native executable fixture"); + #[cfg(not(any(unix, windows)))] + std::fs::write(candidate, b"git fixture").expect("write executable fixture"); +} + +fn locations_for_root(root: &Path) -> UntrustedGitLocations { + UntrustedGitLocations { + roots: vec![std::fs::canonicalize(root).expect("canonical root")], + common_dir: None, + } +} + +fn path_text(path: &Path) -> &str { + path.to_str().expect("UTF-8 fixture path") +} + +fn selected_git(locations: &UntrustedGitLocations, directories: &[&Path]) -> PathBuf { + let search_path = std::env::join_paths(directories).expect("PATH"); + GitRunner::from_search_path(locations, &search_path) + .expect("trusted Git") + .executable +} + +#[cfg(unix)] +#[test] +fn resolver_skips_untrusted_path_entries_and_runs_external_candidate() { + let fixture = tempfile::tempdir().expect("fixture"); + let repo = fixture.path().join("repo"); + let repo_bin = repo.join("bin"); + let outside = fixture.path().join("outside"); + let trusted_bin = fixture.path().join("trusted-bin"); + std::fs::create_dir_all(&repo_bin).expect("repo bin"); + std::fs::create_dir_all(&outside).expect("outside bin"); + std::fs::create_dir_all(&trusted_bin).expect("trusted bin"); + write_executable(&repo_bin.join("git"), "#!/bin/sh\nexit 1\n"); + std::os::unix::fs::symlink(repo_bin.join("git"), outside.join("git")) + .expect("outside symlink into repository"); + write_executable(&trusted_bin.join("git"), "#!/bin/sh\nprintf 'trusted\\n'\n"); + + let path = std::env::join_paths([ + PathBuf::from("relative"), + repo_bin, + outside, + trusted_bin.clone(), + ]) + .expect("PATH"); + let locations = locations_for_root(&repo); + let runner = GitRunner::from_search_path(&locations, &path).expect("trusted Git"); + assert_eq!(runner.executable, trusted_bin.join("git")); + let output = runner.output(runner.command()).expect("run trusted Git"); + assert_eq!(output.stdout, b"trusted\n"); +} + +#[test] +fn linked_worktree_rejects_git_from_main_and_linked_worktrees() { + let fixture = tempfile::tempdir().expect("fixture"); + let main = fixture.path().join("main"); + let linked = fixture.path().join("linked"); + let git_dir = main.join(".git/worktrees/linked"); + let main_bin = main.join("bin"); + std::fs::create_dir_all(&git_dir).expect("linked Git directory"); + std::fs::create_dir_all(&linked).expect("linked worktree"); + std::fs::write( + linked.join(".git"), + format!("gitdir: {}\n", git_dir.display()), + ) + .expect("linked .git file"); + write_git_candidate(&main_bin); + + let locations = untrusted_git_locations_for_cwd(&linked).expect("untrusted locations"); + assert!(path_is_untrusted( + &main_bin.join(git_executable_name()), + &locations + )); +} + +#[test] +fn bare_backed_linked_worktree_allows_external_git_in_sibling_directory() { + let fixture = tempfile::tempdir().expect("fixture"); + let bare = fixture.path().join("repository.git"); + let linked = fixture.path().join("linked"); + let trusted_bin = fixture.path().join("trusted-bin"); + run_git(fixture.path(), &["init", "--bare", path_text(&bare)]); + run_git( + fixture.path(), + &[ + "--git-dir", + path_text(&bare), + "worktree", + "add", + "--orphan", + path_text(&linked), + ], + ); + write_git_candidate(&trusted_bin); + + let locations = untrusted_git_locations_for_cwd(&linked).expect("untrusted locations"); + assert_eq!( + selected_git(&locations, &[&trusted_bin]), + trusted_bin.join(git_executable_name()) + ); +} + +#[test] +fn separate_dot_git_dir_rejects_main_candidate_and_allows_unrelated_repo_candidate() { + let fixture = tempfile::tempdir().expect("fixture"); + let main = fixture.path().join("main"); + let common_dir = fixture.path().join("git-storage/.git"); + let linked = fixture.path().join("linked"); + let main_bin = main.join("bin"); + let unrelated = fixture.path().join("unrelated"); + let unrelated_bin = unrelated.join("bin"); + let malformed = fixture.path().join("malformed"); + let malformed_bin = malformed.join("bin"); + std::fs::create_dir_all(&main).expect("create main worktree"); + std::fs::create_dir_all(common_dir.parent().expect("common-dir parent")) + .expect("create common-dir parent"); + run_git( + fixture.path(), + &[ + "init", + "--separate-git-dir", + path_text(&common_dir), + path_text(&main), + ], + ); + run_git(&main, &["worktree", "add", "--orphan", path_text(&linked)]); + run_git(fixture.path(), &["init", path_text(&unrelated)]); + write_git_candidate(&main_bin); + write_git_candidate(&unrelated_bin); + write_git_candidate(&malformed_bin); + std::fs::write(malformed.join(".git"), "not a gitdir").expect("malformed marker"); + + let locations = untrusted_git_locations_for_cwd(&linked).expect("untrusted locations"); + assert_eq!( + selected_git(&locations, &[&main_bin, &malformed_bin, &unrelated_bin]), + unrelated_bin.join(git_executable_name()) + ); +} + +#[cfg(windows)] +#[test] +fn resolver_selects_native_git_exe_only() { + assert!(paths_equal( + Path::new(r"C:\Repo\.git"), + Path::new(r"c:\repo\.GIT") + )); + let fixture = tempfile::tempdir().expect("fixture"); + let repo = fixture.path().join("repo"); + let scripts = fixture.path().join("scripts"); + let native = fixture.path().join("native"); + std::fs::create_dir_all(&repo).expect("repo"); + std::fs::create_dir_all(&scripts).expect("scripts"); + std::fs::create_dir_all(&native).expect("native"); + std::fs::write(scripts.join("git.cmd"), "@exit /b 0\r\n").expect("script"); + std::fs::write(native.join("git.exe"), b"MZ").expect("native executable fixture"); + let locations = locations_for_root(&repo); + let path = std::env::join_paths([scripts, native.clone()]).expect("PATH"); + let runner = GitRunner::from_search_path(&locations, &path).expect("native Git"); + assert_eq!(runner.executable, native.join("git.exe")); +} diff --git a/codex-rs/git-utils/src/git_config.rs b/codex-rs/git-utils/src/git_config.rs new file mode 100644 index 0000000000..236ab89ef7 --- /dev/null +++ b/codex-rs/git-utils/src/git_config.rs @@ -0,0 +1,31 @@ +use std::path::Component; +use std::path::Path; + +pub(crate) fn path_is_within(path: &Path, root: &Path) -> bool { + let mut path_components = path.components(); + for root_component in root.components() { + let Some(path_component) = path_components.next() else { + return false; + }; + if !components_equal(path_component, root_component) { + return false; + } + } + true +} + +#[cfg(windows)] +fn components_equal(left: Component<'_>, right: Component<'_>) -> bool { + left.as_os_str() + .to_string_lossy() + .eq_ignore_ascii_case(&right.as_os_str().to_string_lossy()) +} + +#[cfg(not(windows))] +fn components_equal(left: Component<'_>, right: Component<'_>) -> bool { + left == right +} + +#[cfg(test)] +#[path = "git_config_tests.rs"] +mod tests; diff --git a/codex-rs/git-utils/src/git_config_tests.rs b/codex-rs/git-utils/src/git_config_tests.rs new file mode 100644 index 0000000000..3795caf2f8 --- /dev/null +++ b/codex-rs/git-utils/src/git_config_tests.rs @@ -0,0 +1,10 @@ +use super::*; + +#[test] +fn path_containment_uses_component_boundaries() { + let root = Path::new("/repo/root"); + assert!(path_is_within(Path::new("/repo/root"), root)); + assert!(path_is_within(Path::new("/repo/root/config"), root)); + assert!(!path_is_within(Path::new("/repo/rooted/config"), root)); + assert!(!path_is_within(Path::new("/repo"), root)); +} diff --git a/codex-rs/git-utils/src/lib.rs b/codex-rs/git-utils/src/lib.rs index d4817d87c8..550461074a 100644 --- a/codex-rs/git-utils/src/lib.rs +++ b/codex-rs/git-utils/src/lib.rs @@ -3,10 +3,13 @@ mod baseline; mod branch; mod errors; mod fsmonitor; +mod git_command; +mod git_config; mod info; mod local_only; mod operations; mod platform; +mod safe_git; pub use apply::ApplyGitRequest; pub use apply::ApplyGitResult; diff --git a/codex-rs/git-utils/src/operations.rs b/codex-rs/git-utils/src/operations.rs index 00f44a6233..d58ff0beed 100644 --- a/codex-rs/git-utils/src/operations.rs +++ b/codex-rs/git-utils/src/operations.rs @@ -2,11 +2,14 @@ use std::ffi::OsStr; use std::ffi::OsString; use std::path::Path; use std::path::PathBuf; +#[cfg(test)] use std::process::Command; use crate::GitToolingError; - -const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" }; +use crate::git_command::GitRunner; +use crate::safe_git::DISABLED_HOOKS_PATH; +#[cfg(test)] +use crate::safe_git::isolate_git_command_environment; pub(crate) fn ensure_git_repository(path: &Path) -> Result<(), GitToolingError> { match run_git_for_stdout( @@ -110,16 +113,16 @@ where args_vec.push(OsString::from(arg.as_ref())); } let command_string = build_command_string(&args_vec); - let mut command = Command::new("git"); + let git = GitRunner::for_cwd_io(dir)?; + let mut command = git.command(); command.current_dir(dir); if let Some(envs) = env { for (key, value) in envs { command.env(key, value); } } - command.envs(crate::local_only_git_env()); command.args(&args_vec); - let output = command.output()?; + let output = git.output(command)?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); return Err(GitToolingError::GitCommand { @@ -150,3 +153,72 @@ struct GitRun { command: String, output: std::process::Output, } + +#[cfg(test)] +mod tests { + use super::*; + + fn init_repo() -> tempfile::TempDir { + let repo = tempfile::tempdir().expect("tempdir"); + let mut command = Command::new("git"); + isolate_git_command_environment(&mut command); + let status = command + .args(["init", "-q"]) + .current_dir(repo.path()) + .status() + .expect("initialize repository"); + assert!(status.success()); + repo + } + + #[test] + fn caller_env_cannot_restore_repository_or_pathspec_selectors() { + let target = init_repo(); + let alternate = init_repo(); + std::fs::write(target.path().join("target.txt"), "target\n").expect("target file"); + std::fs::write(alternate.path().join("alternate.txt"), "alternate\n") + .expect("alternate file"); + for (repo, path) in [(&target, "target.txt"), (&alternate, "alternate.txt")] { + let mut command = Command::new("git"); + isolate_git_command_environment(&mut command); + let status = command + .args(["add", path]) + .current_dir(repo.path()) + .status() + .expect("add file"); + assert!(status.success()); + } + + let alternate_git_dir = alternate.path().join(".git"); + let env = [ + ( + OsString::from("GIT_DIR"), + alternate_git_dir.as_os_str().into(), + ), + ( + OsString::from("GIT_WORK_TREE"), + alternate.path().as_os_str().into(), + ), + ( + OsString::from("GIT_COMMON_DIR"), + alternate_git_dir.as_os_str().into(), + ), + ( + OsString::from("GIT_INDEX_FILE"), + alternate_git_dir.join("index").into_os_string(), + ), + (OsString::from("GIT_PREFIX"), OsString::from("elsewhere/")), + (OsString::from("GIT_LITERAL_PATHSPECS"), OsString::from("1")), + (OsString::from("GIT_GLOB_PATHSPECS"), OsString::from("1")), + (OsString::from("GIT_NOGLOB_PATHSPECS"), OsString::from("1")), + (OsString::from("GIT_ICASE_PATHSPECS"), OsString::from("1")), + ( + OsString::from("GIT_CONFIG"), + alternate_git_dir.join("config").into_os_string(), + ), + ]; + let output = run_git_for_stdout(target.path(), ["ls-files"], Some(&env)) + .expect("query cwd-selected index"); + assert_eq!(output, "target.txt"); + } +} diff --git a/codex-rs/git-utils/src/safe_git.rs b/codex-rs/git-utils/src/safe_git.rs new file mode 100644 index 0000000000..8d45a33552 --- /dev/null +++ b/codex-rs/git-utils/src/safe_git.rs @@ -0,0 +1,30 @@ +use std::process::Command; + +pub(crate) const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" }; + +const ISOLATED_GIT_ENVIRONMENT: [&str; 11] = [ + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_COMMON_DIR", + "GIT_INDEX_FILE", + "GIT_PREFIX", + "GIT_LITERAL_PATHSPECS", + "GIT_GLOB_PATHSPECS", + "GIT_NOGLOB_PATHSPECS", + "GIT_ICASE_PATHSPECS", + "GIT_EXEC_PATH", + // Legacy `GIT_CONFIG` affects `git config` but not ordinary worktree + // commands, so inheriting it can make a safety probe inspect different + // configuration than the command it guards. + "GIT_CONFIG", +]; + +/// Keep internal worktree operations bound to their explicit cwd and pathspec +/// semantics instead of inheriting repository, index, or pathspec selectors. +/// Deliberately leave Git config channels intact: callers may rely on normal +/// system/global configuration, and executable helpers are probed separately. +pub(crate) fn isolate_git_command_environment(command: &mut Command) { + for name in ISOLATED_GIT_ENVIRONMENT { + command.env_remove(name); + } +}