From 3e1688f2cb8eff2efabb92f7e17ea46909cb7bf1 Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Fri, 3 Jul 2026 18:32:25 -0700 Subject: [PATCH] fix(git): authorize primary config sources --- codex-rs/git-utils/src/apply.rs | 14 +- codex-rs/git-utils/src/git_command.rs | 18 + codex-rs/git-utils/src/git_command_tests.rs | 109 ++++ codex-rs/git-utils/src/git_config.rs | 15 +- codex-rs/git-utils/src/git_config_sources.rs | 59 ++ .../src/git_config_sources/path_safety.rs | 38 ++ .../src/git_config_sources/primary_sources.rs | 205 ++++++ .../git-utils/src/git_config_sources_tests.rs | 611 ++++++++++++++++++ codex-rs/git-utils/src/git_config_tests.rs | 35 + codex-rs/git-utils/src/lib.rs | 1 + codex-rs/git-utils/src/patch_paths.rs | 56 +- codex-rs/git-utils/src/patch_paths_tests.rs | 20 +- codex-rs/git-utils/src/path_authority.rs | 2 +- .../repository_authority/authority/policy.rs | 33 + 14 files changed, 1190 insertions(+), 26 deletions(-) create mode 100644 codex-rs/git-utils/src/git_config_sources.rs create mode 100644 codex-rs/git-utils/src/git_config_sources/path_safety.rs create mode 100644 codex-rs/git-utils/src/git_config_sources/primary_sources.rs create mode 100644 codex-rs/git-utils/src/git_config_sources_tests.rs diff --git a/codex-rs/git-utils/src/apply.rs b/codex-rs/git-utils/src/apply.rs index 2b9d76e959..5ba64abde2 100644 --- a/codex-rs/git-utils/src/apply.rs +++ b/codex-rs/git-utils/src/apply.rs @@ -13,6 +13,7 @@ use std::path::PathBuf; use crate::FsmonitorOverride; use crate::apply_output::parse_git_apply_output; use crate::git_command::GitRunner; +use crate::git_config_sources::ensure_no_worktree_primary_config_sources; use crate::patch_paths::extract_effective_paths_from_patch; use crate::patch_paths::stage_effective_paths; use crate::safe_git::DISABLED_HOOKS_PATH; @@ -47,17 +48,24 @@ pub struct ApplyGitResult { pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { let git = GitRunner::for_cwd_io(&req.cwd)?; let mut cfg_parts = configured_git_config_parts(); + ensure_no_worktree_primary_config_sources(&git, &req.cwd)?; + let requested_cwd = std::fs::canonicalize(&req.cwd)?; let git_root = resolve_git_root(&git, &req.cwd, &cfg_parts)?; + if git_root != requested_cwd { + ensure_no_worktree_primary_config_sources(&git, &git_root)?; + } // Write unified diff into a temporary file let (tmpdir, patch_path) = write_temp_patch(&req.diff)?; // Keep tmpdir alive until function end to ensure the file exists let _guard = tmpdir; - let patch_paths = extract_effective_paths_from_patch(&git, &patch_path, req.revert)?; + let patch_paths = + extract_effective_paths_from_patch(&git, &git_root, &patch_path, req.revert, &cfg_parts)?; + cfg_parts.extend(safe_git_config_parts()); if req.revert && !req.preflight { // Stage WT paths first to avoid index mismatch on revert. - stage_effective_paths(&git, &git_root, &patch_paths)?; + stage_effective_paths(&git, &git_root, &patch_paths, &cfg_parts)?; } // Build git args @@ -66,8 +74,6 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { args.push("-R".into()); } - cfg_parts.extend(safe_git_config_parts()); - args.push(patch_path.to_string_lossy().to_string()); // Optional preflight: dry-run only; do not modify working tree diff --git a/codex-rs/git-utils/src/git_command.rs b/codex-rs/git-utils/src/git_command.rs index 3cac863d21..9d3c6af66e 100644 --- a/codex-rs/git-utils/src/git_command.rs +++ b/codex-rs/git-utils/src/git_command.rs @@ -59,6 +59,11 @@ impl GitCommand { self.inner.env(key, value); self } + + pub(crate) fn env_remove(&mut self, key: impl AsRef) -> &mut Self { + self.inner.env_remove(key); + self + } } impl GitRunner { @@ -102,6 +107,19 @@ impl GitRunner { Ok(command) } + pub(crate) fn ensure_config_source_is_not_worktree_controlled( + &self, + path: &Path, + description: &str, + ) -> io::Result<()> { + self.authority + .ensure_config_source_is_not_worktree_controlled(path, description) + } + + pub(crate) fn config_environment_value(&self, name: &str) -> Option<&OsStr> { + self.config_environment.value(name) + } + pub(crate) fn output(&self, mut command: GitCommand) -> io::Result { self.revalidate_active_repository_metadata()?; isolate_git_command_environment(&mut command.inner); diff --git a/codex-rs/git-utils/src/git_command_tests.rs b/codex-rs/git-utils/src/git_command_tests.rs index 868ad6c097..3d191735b3 100644 --- a/codex-rs/git-utils/src/git_command_tests.rs +++ b/codex-rs/git-utils/src/git_command_tests.rs @@ -242,6 +242,16 @@ fn locations_for_root(root: &Path) -> RepositoryAuthority { .expect("test repository authority") } +fn config_authority_for_root(root: &Path) -> RepositoryAuthority { + let mut roots = vec![root.to_path_buf()]; + let canonical = std::fs::canonicalize(root).expect("canonical root"); + if !roots.contains(&canonical) { + roots.push(canonical); + } + RepositoryAuthority::from_test_locations(roots.clone(), roots, vec![root.join(".git")]) + .expect("config repository authority") +} + fn raw_parent_traversal(root: &Path, sibling: &str) -> PathBuf { let separator = std::path::MAIN_SEPARATOR.to_string(); let mut path = root.as_os_str().to_os_string(); @@ -381,6 +391,105 @@ fn git_metadata_marker_parser_preserves_leading_path_space() { assert!(parse_git_marker_path(b"gitdir:/missing-space\n", b"gitdir: ").is_err()); } +#[test] +fn config_source_authority_requires_an_absolute_raw_path() { + let fixture = tempdir_for_native_git(); + let root = fixture.path().join("repo"); + std::fs::create_dir_all(&root).expect("create repository"); + run_git(&root, &["init", "-q"]); + let authority = config_authority_for_root(&root); + let path = Path::new("relative/config"); + + let error = authority + .ensure_config_source_is_not_worktree_controlled(path, "test config") + .expect_err("relative config path"); + + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + assert!(error.to_string().contains("test config"), "{error}"); + assert!(error.to_string().contains("relative/config"), "{error}"); +} + +#[test] +fn config_source_authority_preserves_and_rejects_a_worktree_crossing_spelling() { + let fixture = tempdir_for_native_git(); + let root = fixture.path().join("repo"); + let safe = fixture.path().join("safe"); + std::fs::create_dir_all(root.join("nested")).expect("create repository descendant"); + std::fs::create_dir_all(&safe).expect("create safe directory"); + run_git(&root, &["init", "-q"]); + let authority = config_authority_for_root(&root); + let raw = root + .join("nested") + .join("..") + .join("..") + .join("safe/config"); + + let error = authority + .ensure_config_source_is_not_worktree_controlled(&raw, "crossing config") + .expect_err("worktree-crossing config path"); + + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + assert!(error.to_string().contains("crossing config"), "{error}"); + assert!( + error.to_string().contains(&raw.display().to_string()), + "{error}" + ); +} + +#[test] +fn config_source_authority_allows_protected_metadata_and_unrelated_external_paths() { + let fixture = tempdir_for_native_git(); + let root = fixture.path().join("repo"); + let external = fixture.path().join("external/config"); + std::fs::create_dir_all(&root).expect("create repository"); + std::fs::create_dir_all(external.parent().expect("external parent")) + .expect("create external directory"); + run_git(&root, &["init", "-q"]); + let authority = config_authority_for_root(&root); + + authority + .ensure_config_source_is_not_worktree_controlled( + &root.join(".git/config"), + "protected metadata config", + ) + .expect("protected metadata config"); + let canonical_config = + std::fs::canonicalize(root.join(".git/config")).expect("canonical metadata config"); + authority + .ensure_config_source_is_not_worktree_controlled( + &canonical_config, + "canonical protected metadata config", + ) + .expect("canonical protected metadata config"); + authority + .ensure_config_source_is_not_worktree_controlled(&external, "external config") + .expect("unrelated external config"); +} + +#[test] +fn config_source_authority_rejects_an_unregistered_related_repository_root() { + let fixture = tempdir_for_native_git(); + let root = fixture.path().join("repo"); + let alias = fixture.path().join("unregistered-related-root"); + std::fs::create_dir_all(&root).expect("create repository"); + std::fs::create_dir_all(&alias).expect("create related root"); + run_git(&root, &["init", "-q"]); + std::fs::write( + alias.join(".git"), + format!("gitdir: {}\n", root.join(".git").display()), + ) + .expect("write related metadata marker"); + let authority = config_authority_for_root(&root); + let path = alias.join("config"); + + let error = authority + .ensure_config_source_is_not_worktree_controlled(&path, "related config") + .expect_err("unregistered related repository config"); + + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + assert!(error.to_string().contains("related config"), "{error}"); +} + fn selected_git(locations: &RepositoryAuthority, directories: &[&Path]) -> PathBuf { let search_path = std::env::join_paths(directories).expect("PATH"); select_git_executable(locations, &search_path) diff --git a/codex-rs/git-utils/src/git_config.rs b/codex-rs/git-utils/src/git_config.rs index 92496132a1..737d3c433b 100644 --- a/codex-rs/git-utils/src/git_config.rs +++ b/codex-rs/git-utils/src/git_config.rs @@ -2,6 +2,19 @@ use std::path::Component; use std::path::Path; pub(crate) fn parse_git_boolean(value: &[u8]) -> Option { + parse_git_boolean_with_minimum(value, i128::from(i32::MIN)) +} + +/// Parse the Git boolean grammar while excluding numeric `INT_MIN`. +/// +/// Older supported Git releases reject that one signed endpoint after unit +/// expansion, so environment gates that must succeed across versions use this +/// conservative variant. The accepted spellings otherwise share one parser. +pub(crate) fn parse_git_boolean_symmetric_i32(value: &[u8]) -> Option { + parse_git_boolean_with_minimum(value, -i128::from(i32::MAX)) +} + +fn parse_git_boolean_with_minimum(value: &[u8], minimum: i128) -> Option { if value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"yes") || value.eq_ignore_ascii_case(b"on") @@ -61,7 +74,7 @@ pub(crate) fn parse_git_boolean(value: &[u8]) -> Option { let magnitude = i128::from_str_radix(digits, base).ok()?; let signed = if negative { -magnitude } else { magnitude }; let value = signed.checked_mul(factor)?; - (i128::from(i32::MIN)..=i128::from(i32::MAX)) + (minimum..=i128::from(i32::MAX)) .contains(&value) .then_some(value != 0) } diff --git a/codex-rs/git-utils/src/git_config_sources.rs b/codex-rs/git-utils/src/git_config_sources.rs new file mode 100644 index 0000000000..b539ef266c --- /dev/null +++ b/codex-rs/git-utils/src/git_config_sources.rs @@ -0,0 +1,59 @@ +use std::io; +use std::path::Path; + +use crate::git_command::GitRunner; + +mod path_safety; +mod primary_sources; + +use path_safety::normalize_absolute_path; +use path_safety::resolve_literal_path; +use primary_sources::is_disabled_primary_config_path; +use primary_sources::legacy_primary_config_source_candidates; +use primary_sources::selected_git_home_config_candidates; +use primary_sources::selected_git_prefix_system_candidate; + +/// Reject primary Git configuration paths selected from worktree-controlled +/// routes before any ordinary Git command can open them. +/// +/// This layer intentionally does not authorize includes. The complete include +/// graph is added by the next stack layer before this is promoted to a full +/// config-source capability. +pub(crate) fn ensure_no_worktree_primary_config_sources( + git: &GitRunner, + git_root: &Path, +) -> io::Result<()> { + let git_root = normalize_absolute_path(std::fs::canonicalize(git_root)?)?; + + for (description, candidate) in legacy_primary_config_source_candidates(git)? { + if !is_disabled_primary_config_path(&candidate) { + reject_source(git, &git_root, &candidate, description)?; + } + } + for candidate in selected_git_home_config_candidates(git, &git_root)? { + reject_source(git, &git_root, &candidate, "selected Git HOME config")?; + } + if let Some(candidate) = selected_git_prefix_system_candidate(git, &git_root)? { + reject_source( + git, + &git_root, + &candidate, + "selected Git prefix system config", + )?; + } + Ok(()) +} + +fn reject_source( + git: &GitRunner, + cwd: &Path, + candidate: &Path, + description: &str, +) -> io::Result<()> { + let absolute = resolve_literal_path(candidate, cwd); + git.ensure_config_source_is_not_worktree_controlled(&absolute, description) +} + +#[cfg(test)] +#[path = "git_config_sources_tests.rs"] +mod tests; diff --git a/codex-rs/git-utils/src/git_config_sources/path_safety.rs b/codex-rs/git-utils/src/git_config_sources/path_safety.rs new file mode 100644 index 0000000000..a2d86fd062 --- /dev/null +++ b/codex-rs/git-utils/src/git_config_sources/path_safety.rs @@ -0,0 +1,38 @@ +use std::io; +use std::path::Path; +use std::path::PathBuf; + +use codex_utils_absolute_path::AbsolutePathBuf; + +pub(super) const CONFIG_PATH_KEY: &str = "codex.config-source.path"; + +#[cfg(unix)] +pub(super) fn git_var_path_from_bytes(path: &[u8]) -> io::Result { + use std::os::unix::ffi::OsStringExt; + + Ok(PathBuf::from(std::ffi::OsString::from_vec(path.to_vec()))) +} + +#[cfg(not(unix))] +pub(super) fn git_var_path_from_bytes(path: &[u8]) -> io::Result { + Ok(PathBuf::from(std::str::from_utf8(path).map_err(|_| { + invalid_config_source("non-UTF-8 Git config source path") + })?)) +} + +pub(super) fn resolve_literal_path(path: impl AsRef, base: &Path) -> PathBuf { + let path = path.as_ref(); + if path.is_absolute() { + path.to_path_buf() + } else { + base.join(path) + } +} + +pub(super) fn normalize_absolute_path(path: impl AsRef) -> io::Result { + Ok(AbsolutePathBuf::from_absolute_path(path)?.into_path_buf()) +} + +pub(super) fn invalid_config_source(message: &str) -> io::Error { + io::Error::new(io::ErrorKind::InvalidData, message) +} diff --git a/codex-rs/git-utils/src/git_config_sources/primary_sources.rs b/codex-rs/git-utils/src/git_config_sources/primary_sources.rs new file mode 100644 index 0000000000..1c5e835935 --- /dev/null +++ b/codex-rs/git-utils/src/git_config_sources/primary_sources.rs @@ -0,0 +1,205 @@ +use std::ffi::OsStr; +use std::io; +use std::path::Path; +use std::path::PathBuf; + +use super::path_safety::CONFIG_PATH_KEY; +use super::path_safety::git_var_path_from_bytes; +use super::path_safety::invalid_config_source; +use crate::git_command::GitRunner; +use crate::git_config::parse_git_boolean_symmetric_i32; + +pub(super) fn selected_git_home_config_candidates( + git: &GitRunner, + cwd: &Path, +) -> io::Result> { + if git.config_environment_value("GIT_CONFIG_GLOBAL").is_some() { + return Ok(Vec::new()); + } + #[cfg(not(windows))] + if git.config_environment_value("HOME").is_none() { + return Ok(Vec::new()); + } + let dot_gitconfig = selected_git_path_candidate(git, cwd, "~/.gitconfig")?; + let mut candidates = vec![dot_gitconfig.clone()]; + if git + .config_environment_value("XDG_CONFIG_HOME") + .is_none_or(OsStr::is_empty) + { + let home = dot_gitconfig + .parent() + .ok_or_else(|| invalid_config_source("selected Git HOME has no parent"))?; + candidates.push(home.join(".config/git/config")); + } + Ok(candidates) +} + +pub(super) fn selected_git_prefix_system_candidate( + git: &GitRunner, + cwd: &Path, +) -> io::Result> { + if git_env_bool(git, "GIT_CONFIG_NOSYSTEM")? + || git.config_environment_value("GIT_CONFIG_SYSTEM").is_some() + { + return Ok(None); + } + selected_git_path_candidate(git, cwd, "%(prefix)/etc/gitconfig").map(Some) +} + +fn selected_git_path_candidate(git: &GitRunner, cwd: &Path, raw: &str) -> io::Result { + let tempdir = tempfile::tempdir()?; + let nonexistent_git_dir = tempdir.path().join("nonexistent-git-dir"); + let disabled_config = if cfg!(windows) { "NUL" } else { "/dev/null" }; + let mut command = git.command_for_cwd(cwd)?; + command + .env("GIT_CONFIG_GLOBAL", disabled_config) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_COUNT", "0") + .env_remove("GIT_CONFIG_PARAMETERS") + .arg("--git-dir") + .arg(&nonexistent_git_dir) + .arg("-c") + .arg(format!("{CONFIG_PATH_KEY}={raw}")) + .args([ + "config", + "--null", + "--no-includes", + "--path", + "--get", + CONFIG_PATH_KEY, + ]); + let output = git.output(command)?; + if !output.status.success() { + return Err(io::Error::other(format!( + "isolated selected Git path expansion failed with status {}: {}", + output.status, + String::from_utf8_lossy(&output.stderr).trim() + ))); + } + let value = output + .stdout + .strip_suffix(&[0]) + .ok_or_else(|| invalid_config_source("unterminated selected Git path"))?; + if value.is_empty() || value.contains(&0) { + return Err(invalid_config_source("ambiguous selected Git path")); + } + git_var_path_from_bytes(value) +} + +pub(super) fn legacy_primary_config_source_candidates( + git: &GitRunner, +) -> io::Result> { + let mut candidates = Vec::new(); + match git.config_environment_value("GIT_CONFIG_GLOBAL") { + Some(path) if !path.is_empty() => { + candidates.push(("GIT_CONFIG_GLOBAL", PathBuf::from(path))); + } + Some(_) => {} + None => { + let homes = git_home_directories(git); + match git.config_environment_value("XDG_CONFIG_HOME") { + Some(xdg) if !xdg.is_empty() => candidates.push(( + "XDG_CONFIG_HOME Git config", + PathBuf::from(xdg).join("git/config"), + )), + _ => { + #[cfg(windows)] + if let Some(app_data) = git.config_environment_value("APPDATA") + && !app_data.is_empty() + { + candidates.push(( + "APPDATA Git config", + PathBuf::from(app_data).join("Git/config"), + )); + } + for home in &homes { + candidates.push(( + "HOME XDG Git config", + home_config_path(home, ".config/git/config"), + )); + } + } + } + for home in &homes { + candidates.push(("HOME Git config", home_config_path(home, ".gitconfig"))); + } + } + } + if !git_env_bool(git, "GIT_CONFIG_NOSYSTEM")? { + match git.config_environment_value("GIT_CONFIG_SYSTEM") { + Some(path) if !path.is_empty() => { + candidates.push(("GIT_CONFIG_SYSTEM", PathBuf::from(path))); + } + Some(_) => {} + None => { + #[cfg(windows)] + if let Some(program_data) = git.config_environment_value("PROGRAMDATA") + && !program_data.is_empty() + { + candidates.push(( + "PROGRAMDATA Git config", + PathBuf::from(program_data).join("Git/config"), + )); + } + } + } + } + Ok(candidates) +} + +fn git_home_directories(git: &GitRunner) -> Vec { + if let Some(home) = git.config_environment_value("HOME") { + return vec![home.to_owned()]; + } + #[cfg(windows)] + { + let mut homes = Vec::new(); + if let (Some(drive), Some(path)) = ( + git.config_environment_value("HOMEDRIVE"), + git.config_environment_value("HOMEPATH"), + ) { + let mut home = drive.to_owned(); + home.push(path); + homes.push(home); + } + if let Some(profile) = git.config_environment_value("USERPROFILE") + && !homes.iter().any(|home| home == profile) + { + homes.push(profile.to_owned()); + } + homes + } + #[cfg(not(windows))] + Vec::new() +} + +fn home_config_path(home: &std::ffi::OsStr, suffix: &str) -> PathBuf { + if home.is_empty() { + PathBuf::from(std::path::MAIN_SEPARATOR.to_string()).join(suffix) + } else { + PathBuf::from(home).join(suffix) + } +} + +fn git_env_bool(git: &GitRunner, name: &str) -> io::Result { + let Some(value) = git.config_environment_value(name) else { + return Ok(false); + }; + let value = value + .to_str() + .ok_or_else(|| invalid_config_source("non-UTF-8 Git boolean environment value"))?; + parse_git_boolean_symmetric_i32(value.as_bytes()) + .ok_or_else(|| invalid_config_source("invalid Git boolean environment value")) +} + +#[cfg(windows)] +pub(super) fn is_disabled_primary_config_path(path: &Path) -> bool { + path.as_os_str() + .to_str() + .is_some_and(|path| path.eq_ignore_ascii_case("NUL")) +} + +#[cfg(not(windows))] +pub(super) fn is_disabled_primary_config_path(_path: &Path) -> bool { + false +} diff --git a/codex-rs/git-utils/src/git_config_sources_tests.rs b/codex-rs/git-utils/src/git_config_sources_tests.rs new file mode 100644 index 0000000000..222e9a614f --- /dev/null +++ b/codex-rs/git-utils/src/git_config_sources_tests.rs @@ -0,0 +1,611 @@ +use super::*; +use pretty_assertions::assert_eq; +use std::ffi::OsStr; +use std::path::PathBuf; + +fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) { + let mut command = std::process::Command::new("git"); + crate::safe_git::isolate_git_command_environment(&mut command); + let output = command + .args(args) + .current_dir(cwd) + .output() + .expect("run Git"); + ( + output.status.code().unwrap_or(-1), + String::from_utf8_lossy(&output.stdout).into_owned(), + String::from_utf8_lossy(&output.stderr).into_owned(), + ) +} + +fn run_success(cwd: &Path, args: &[&str]) -> String { + let (code, stdout, stderr) = run(cwd, args); + assert_eq!(code, 0, "git {args:?}: {stderr}"); + stdout.trim().to_string() +} + +fn init_repo() -> tempfile::TempDir { + let repo = tempfile::tempdir().expect("tempdir"); + init_repo_at(repo.path()); + repo +} + +fn init_repo_at(root: &Path) { + std::fs::create_dir_all(root).expect("create repository directory"); + run_success(root, &["init"]); + run_success(root, &["config", "user.email", "codex@example.com"]); + run_success(root, &["config", "user.name", "Codex"]); + std::fs::write(root.join("file.txt"), "orig\n").expect("write file"); + run_success(root, &["add", "file.txt"]); + run_success(root, &["commit", "-m", "seed"]); +} + +#[cfg(windows)] +fn create_junction(path: &Path, target: &Path) { + // Bazel's GNU Windows runner can surface temporary paths with `/` + // separators. `mklink` treats those separators as option prefixes, so + // pass native separators to the cmd.exe built-in. + let path = path.as_os_str().to_string_lossy().replace('/', "\\"); + let target = target.as_os_str().to_string_lossy().replace('/', "\\"); + let output = std::process::Command::new("cmd.exe") + .args(["/D", "/C", "mklink", "/J"]) + .arg(path) + .arg(target) + .output() + .expect("create junction"); + assert!( + output.status.success(), + "mklink failed: stdout={} stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + +fn guard(root: &Path) -> io::Result<()> { + let git = GitRunner::for_cwd_io(root)?; + ensure_no_worktree_primary_config_sources(&git, root) +} + +fn assert_worktree_rejection(error: io::Error) { + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + assert!(error.to_string().contains("worktree-controlled"), "{error}"); +} + +fn run_isolated_source_test(test_name: &str, env: &[(&str, &OsStr)], removed: &[&str]) { + let mut command = std::process::Command::new(std::env::current_exe().expect("test binary")); + crate::safe_git::isolate_git_command_environment(&mut command); + command + .arg(test_name) + .arg("--exact") + .arg("--nocapture") + .env("CODEX_GIT_CONFIG_SOURCE_CHILD", "1") + .env("RUST_TEST_THREADS", "1"); + for (name, value) in env { + command.env(name, value); + } + for name in removed { + command.env_remove(name); + } + let output = command.output().expect("run isolated source test"); + 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) + ); +} + +#[test] +fn rejects_empty_primary_global_and_system_sources_in_the_worktree() { + const TEST_NAME: &str = "git_config_sources::tests::rejects_empty_primary_global_and_system_sources_in_the_worktree"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let repo = init_repo(); + let root = repo.path(); + let unsafe_config = root.join("empty-primary.gitconfig"); + std::fs::write(&unsafe_config, "").expect("write empty primary config"); + let safe = tempfile::NamedTempFile::new().expect("safe external config"); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", root.as_os_str()), + ("GIT_CONFIG_GLOBAL", unsafe_config.as_os_str()), + ("GIT_CONFIG_SYSTEM", safe.path().as_os_str()), + ], + &[], + ); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", root.as_os_str()), + ("GIT_CONFIG_GLOBAL", safe.path().as_os_str()), + ("GIT_CONFIG_SYSTEM", unsafe_config.as_os_str()), + ], + &[], + ); + return; + } + + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + assert_worktree_rejection(guard(&root).expect_err("unsafe primary config source")); +} + +#[test] +fn rejects_missing_home_and_xdg_sources_in_the_worktree() { + const TEST_NAME: &str = + "git_config_sources::tests::rejects_missing_home_and_xdg_sources_in_the_worktree"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let repo = init_repo(); + let root = repo.path(); + let xdg = root.join("xdg"); + std::fs::create_dir(&xdg).expect("create XDG root"); + let safe_system = tempfile::NamedTempFile::new().expect("safe system config"); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", root.as_os_str()), + ("HOME", root.as_os_str()), + ("XDG_CONFIG_HOME", xdg.as_os_str()), + ("GIT_CONFIG_SYSTEM", safe_system.path().as_os_str()), + ], + &["GIT_CONFIG_GLOBAL"], + ); + return; + } + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + assert_worktree_rejection(guard(&root).expect_err("unsafe HOME/XDG source")); +} + +#[test] +fn allows_explicitly_disabled_global_and_system_sources() { + const TEST_NAME: &str = + "git_config_sources::tests::allows_explicitly_disabled_global_and_system_sources"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let repo = init_repo(); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ("GIT_CONFIG_GLOBAL", OsStr::new("")), + ("GIT_CONFIG_SYSTEM", OsStr::new("")), + ], + &[], + ); + return; + } + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + guard(&root).expect("disabled primary config sources"); +} + +#[cfg(unix)] +#[test] +fn allows_unset_home_when_git_has_no_global_source() { + const TEST_NAME: &str = + "git_config_sources::tests::allows_unset_home_when_git_has_no_global_source"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let repo = init_repo(); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ], + &[ + "HOME", + "XDG_CONFIG_HOME", + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_SYSTEM", + ], + ); + return; + } + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + guard(&root).expect("Git without HOME or global config"); +} + +#[cfg(unix)] +#[test] +fn rejects_newline_bearing_raw_global_source_before_git_normalizes_it() { + const TEST_NAME: &str = "git_config_sources::tests::rejects_newline_bearing_raw_global_source_before_git_normalizes_it"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let parent = tempfile::tempdir().expect("fixture parent"); + let root = parent.path().join("repo"); + init_repo_at(&root); + let newline_dir = parent.path().join("external\n"); + std::fs::create_dir(&newline_dir).expect("create newline path component"); + let unsafe_config = root.join("global.gitconfig"); + std::fs::write(&unsafe_config, "").expect("write unsafe global config"); + let raw = newline_dir.join("../repo/global.gitconfig"); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", root.as_os_str()), + ("GIT_CONFIG_GLOBAL", raw.as_os_str()), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ], + &["GIT_CONFIG_SYSTEM"], + ); + return; + } + + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + assert_worktree_rejection(guard(&root).expect_err("newline raw global source")); +} + +#[cfg(unix)] +#[test] +fn ignores_non_utf8_values_for_unrelated_global_keys() { + const TEST_NAME: &str = + "git_config_sources::tests::ignores_non_utf8_values_for_unrelated_global_keys"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let repo = init_repo(); + let global = tempfile::NamedTempFile::new().expect("external global config"); + std::fs::write(global.path(), b"[user]\nname = \xff\n") + .expect("write non-UTF-8 unrelated config value"); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ("GIT_CONFIG_GLOBAL", global.path().as_os_str()), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ], + &["GIT_CONFIG_SYSTEM"], + ); + return; + } + + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + guard(&root).expect("unrelated non-UTF-8 global value"); +} + +#[cfg(unix)] +#[test] +fn rejects_worktree_fifo_primary_source_without_opening_it() { + const TEST_NAME: &str = + "git_config_sources::tests::rejects_worktree_fifo_primary_source_without_opening_it"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let repo = init_repo(); + let fifo = repo.path().join("global.fifo"); + let status = std::process::Command::new("mkfifo") + .arg(&fifo) + .status() + .expect("run mkfifo"); + assert!(status.success(), "mkfifo failed: {status}"); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ("GIT_CONFIG_GLOBAL", fifo.as_os_str()), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ], + &["GIT_CONFIG_SYSTEM"], + ); + return; + } + + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + assert_worktree_rejection(guard(&root).expect_err("worktree FIFO config source")); +} + +#[test] +fn git_config_nosystem_uses_the_shared_parser_with_a_cross_version_numeric_subset() { + const TEST_NAME: &str = "git_config_sources::tests::git_config_nosystem_uses_the_shared_parser_with_a_cross_version_numeric_subset"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let repo = init_repo(); + let unsafe_system = repo.path().join("system.gitconfig"); + std::fs::write(&unsafe_system, "").expect("write unsafe system config"); + for (value, expected) in [ + ("1", "ignored"), + ("2", "ignored"), + ("-1", "ignored"), + ("01", "ignored"), + ("+1", "ignored"), + ("0x1", "ignored"), + ("010", "ignored"), + ("1k", "ignored"), + ("-1g", "ignored"), + (" 1", "ignored"), + ("2147483647", "ignored"), + ("-2147483647", "ignored"), + ("true", "ignored"), + ("yes", "ignored"), + ("on", "ignored"), + ("", "rejected"), + ("0", "rejected"), + ("-0", "rejected"), + ("false", "rejected"), + ("no", "rejected"), + ("off", "rejected"), + ("not-a-bool", "invalid"), + ("08", "invalid"), + ("2147483648", "invalid"), + ("-2147483648", "invalid"), + ("-0x80000000", "invalid"), + ("-020000000000", "invalid"), + ("-2097152k", "invalid"), + ("-2048m", "invalid"), + ("-2g", "invalid"), + (" -2G", "invalid"), + ("-2147483649", "invalid"), + ("2g", "invalid"), + ] { + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ("CODEX_GIT_CONFIG_SOURCE_EXPECTED", OsStr::new(expected)), + ("GIT_CONFIG_GLOBAL", OsStr::new("")), + ("GIT_CONFIG_SYSTEM", unsafe_system.as_os_str()), + ("GIT_CONFIG_NOSYSTEM", OsStr::new(value)), + ], + &[], + ); + } + return; + } + + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + match std::env::var("CODEX_GIT_CONFIG_SOURCE_EXPECTED") + .expect("expected child outcome") + .as_str() + { + "ignored" => guard(&root).expect("system source disabled"), + "rejected" => assert_worktree_rejection(guard(&root).expect_err("system source enabled")), + "invalid" => assert_eq!( + guard(&root).expect_err("invalid NOSYSTEM value").kind(), + io::ErrorKind::InvalidData + ), + expected => panic!("unexpected child outcome {expected}"), + } +} + +#[cfg(windows)] +#[test] +fn rejects_both_windows_home_candidates_when_home_is_absent() { + const TEST_NAME: &str = + "git_config_sources::tests::rejects_both_windows_home_candidates_when_home_is_absent"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let repo = init_repo(); + let external_profile = tempfile::tempdir().expect("external USERPROFILE"); + for home in [repo.path().to_path_buf(), repo.path().join("future-home")] { + let home = home.to_str().expect("UTF-8 Windows test path"); + assert!(home.len() >= 3 && home.as_bytes()[1] == b':', "{home}"); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ("HOMEDRIVE", OsStr::new(&home[..2])), + ("HOMEPATH", OsStr::new(&home[2..])), + ("USERPROFILE", external_profile.path().as_os_str()), + ], + &[ + "HOME", + "XDG_CONFIG_HOME", + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_SYSTEM", + ], + ); + } + let external_home = external_profile + .path() + .to_str() + .expect("UTF-8 external Windows home"); + assert!( + external_home.len() >= 3 && external_home.as_bytes()[1] == b':', + "{external_home}" + ); + for profile in [ + repo.path().to_path_buf(), + repo.path().join("future-profile"), + ] { + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ("HOMEDRIVE", OsStr::new(&external_home[..2])), + ("HOMEPATH", OsStr::new(&external_home[2..])), + ("USERPROFILE", profile.as_os_str()), + ], + &[ + "HOME", + "XDG_CONFIG_HOME", + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_SYSTEM", + ], + ); + } + return; + } + + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + assert_worktree_rejection(guard(&root).expect_err("Windows synthesized HOME source")); +} + +#[cfg(windows)] +#[test] +fn allows_exact_windows_nul_for_both_primary_config_channels() { + const TEST_NAME: &str = + "git_config_sources::tests::allows_exact_windows_nul_for_both_primary_config_channels"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let repo = init_repo(); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ("GIT_CONFIG_GLOBAL", OsStr::new("NUL")), + ("GIT_CONFIG_SYSTEM", OsStr::new("NUL")), + ], + &["GIT_CONFIG_NOSYSTEM"], + ); + return; + } + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + guard(&root).expect("exact NUL disables primary config channels"); +} + +#[cfg(windows)] +#[test] +fn windows_appdata_global_config_matches_native_git_selection_and_precedence() { + const TEST_NAME: &str = "git_config_sources::tests::windows_appdata_global_config_matches_native_git_selection_and_precedence"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + for xdg in [None, Some(OsStr::new(""))] { + for existing in [true, false] { + let repo = init_repo(); + let appdata = repo.path().join("appdata"); + if existing { + let config = appdata.join("Git/config"); + std::fs::create_dir_all(config.parent().expect("APPDATA config parent")) + .expect("create APPDATA config parent"); + std::fs::write(&config, "[codex]\nappdata-marker = selected\n") + .expect("write APPDATA config"); + } + let external_home = tempfile::tempdir().expect("external HOME"); + let mut env = vec![ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ( + "CODEX_GIT_CONFIG_SOURCE_EXPECTED", + OsStr::new(if existing { + "reject-existing-appdata" + } else { + "reject-missing-appdata" + }), + ), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ("HOME", external_home.path().as_os_str()), + ("APPDATA", appdata.as_os_str()), + ]; + if let Some(xdg) = xdg { + env.push(("XDG_CONFIG_HOME", xdg)); + } + let mut removed = vec![ + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_SYSTEM", + "HOMEDRIVE", + "HOMEPATH", + "USERPROFILE", + ]; + if xdg.is_none() { + removed.push("XDG_CONFIG_HOME"); + } + run_isolated_source_test(TEST_NAME, &env, &removed); + } + } + + let repo = init_repo(); + let appdata = repo.path().join("appdata"); + let appdata_config = appdata.join("Git/config"); + std::fs::create_dir_all(appdata_config.parent().expect("APPDATA config parent")) + .expect("create APPDATA config parent"); + std::fs::write(&appdata_config, "[codex]\nappdata-marker = appdata\n") + .expect("write APPDATA config"); + let external_home = tempfile::tempdir().expect("external HOME"); + let external_xdg = tempfile::tempdir().expect("external XDG_CONFIG_HOME"); + let xdg_config = external_xdg.path().join("git/config"); + std::fs::create_dir_all(xdg_config.parent().expect("XDG config parent")) + .expect("create XDG config parent"); + std::fs::write(&xdg_config, "[codex]\nappdata-marker = xdg\n").expect("write XDG config"); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ("CODEX_GIT_CONFIG_SOURCE_EXPECTED", OsStr::new("allow-xdg")), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ("HOME", external_home.path().as_os_str()), + ("APPDATA", appdata.as_os_str()), + ("XDG_CONFIG_HOME", external_xdg.path().as_os_str()), + ], + &[ + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_SYSTEM", + "HOMEDRIVE", + "HOMEPATH", + "USERPROFILE", + ], + ); + + let external_global = tempfile::NamedTempFile::new().expect("external global config"); + std::fs::write( + external_global.path(), + "[codex]\nappdata-marker = explicit\n", + ) + .expect("write explicit global config"); + run_isolated_source_test( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", repo.path().as_os_str()), + ( + "CODEX_GIT_CONFIG_SOURCE_EXPECTED", + OsStr::new("allow-explicit-global"), + ), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ("HOME", repo.path().as_os_str()), + ("APPDATA", appdata.as_os_str()), + ("XDG_CONFIG_HOME", repo.path().as_os_str()), + ("GIT_CONFIG_GLOBAL", external_global.path().as_os_str()), + ], + &["GIT_CONFIG_SYSTEM", "HOMEDRIVE", "HOMEPATH", "USERPROFILE"], + ); + return; + } + + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + let expected = + std::env::var("CODEX_GIT_CONFIG_SOURCE_EXPECTED").expect("expected child outcome"); + if expected != "reject-missing-appdata" { + let output = std::process::Command::new("git") + .args(["config", "--global", "--get", "codex.appdata-marker"]) + .current_dir(&root) + .output() + .expect("run native Git global config query"); + assert!( + output.status.success(), + "native Git global query failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let actual = String::from_utf8_lossy(&output.stdout); + let native_expected = match expected.as_str() { + "reject-existing-appdata" => "selected", + "allow-xdg" => "xdg", + "allow-explicit-global" => "explicit", + expected => panic!("unexpected child outcome {expected}"), + }; + assert_eq!( + actual.trim(), + native_expected, + "native Git global selection" + ); + } + match expected.as_str() { + "reject-existing-appdata" | "reject-missing-appdata" => { + assert_worktree_rejection(guard(&root).expect_err("worktree APPDATA config")); + } + "allow-xdg" => guard(&root).expect("nonempty XDG suppresses APPDATA"), + "allow-explicit-global" => { + guard(&root).expect("explicit global config suppresses HOME, XDG, and APPDATA") + } + expected => panic!("unexpected child outcome {expected}"), + } +} diff --git a/codex-rs/git-utils/src/git_config_tests.rs b/codex-rs/git-utils/src/git_config_tests.rs index 825d9a597f..8f49871c03 100644 --- a/codex-rs/git-utils/src/git_config_tests.rs +++ b/codex-rs/git-utils/src/git_config_tests.rs @@ -64,6 +64,41 @@ fn git_boolean_rejects_values_native_git_rejects() { } } +#[test] +fn symmetric_git_boolean_parser_excludes_only_int_min_spellings() { + for value in [ + b"-2147483648".as_slice(), + b"-0x80000000".as_slice(), + b"-020000000000".as_slice(), + b"-2097152k".as_slice(), + b"-2048m".as_slice(), + b"-2g".as_slice(), + b" -2G".as_slice(), + ] { + assert_eq!(parse_git_boolean(value), Some(true), "value {value:?}"); + assert_eq!( + parse_git_boolean_symmetric_i32(value), + None, + "value {value:?}" + ); + } + + for value in [ + b"0x1".as_slice(), + b"010".as_slice(), + b"1k".as_slice(), + b"-1g".as_slice(), + b" 1".as_slice(), + b"-2147483647".as_slice(), + ] { + assert_eq!( + parse_git_boolean_symmetric_i32(value), + parse_git_boolean(value), + "value {value:?}" + ); + } +} + #[test] fn path_containment_uses_component_boundaries() { let root = Path::new("/repo/root"); diff --git a/codex-rs/git-utils/src/lib.rs b/codex-rs/git-utils/src/lib.rs index e4dca00166..b52bf1ef19 100644 --- a/codex-rs/git-utils/src/lib.rs +++ b/codex-rs/git-utils/src/lib.rs @@ -7,6 +7,7 @@ mod fsmonitor; mod git_command; mod git_config; mod git_config_environment; +mod git_config_sources; mod git_executable; mod info; mod local_only; diff --git a/codex-rs/git-utils/src/patch_paths.rs b/codex-rs/git-utils/src/patch_paths.rs index 168d9b448f..378a245859 100644 --- a/codex-rs/git-utils/src/patch_paths.rs +++ b/codex-rs/git-utils/src/patch_paths.rs @@ -9,16 +9,23 @@ use crate::apply::safe_git_config_parts; use crate::apply::write_temp_patch; use crate::git_command::GitRunner; use crate::git_config::path_is_within; +use crate::git_config_sources::ensure_no_worktree_primary_config_sources; +/// Extract paths with Git from a cwd whose config sources have already been +/// authorized for `git_config_args`. pub(crate) fn extract_effective_paths_from_patch( git: &GitRunner, + authorized_cwd: &Path, patch_path: &Path, revert: bool, + git_config_args: &[String], ) -> io::Result> { - let forward_paths = git_apply_numstat_paths(git, patch_path, revert)?; + let forward_paths = + git_apply_numstat_paths(git, authorized_cwd, patch_path, revert, git_config_args)?; // `git apply --numstat` reports only the destination of a rename. Parse the // opposite orientation too so both endpoints are included in the result. - let reverse_paths = git_apply_numstat_paths(git, patch_path, !revert)?; + let reverse_paths = + git_apply_numstat_paths(git, authorized_cwd, patch_path, !revert, git_config_args)?; if forward_paths.len() != reverse_paths.len() { return Err(io::Error::new( io::ErrorKind::InvalidData, @@ -44,27 +51,38 @@ pub(crate) fn extract_effective_paths_from_patch( /// Security-sensitive callers must use the fallible internal extractor so an /// invalid or ambiguous patch is rejected instead of becoming an empty list. pub fn extract_paths_from_patch(diff_text: &str) -> Vec { + let Ok(cwd) = std::env::current_dir() else { + return Vec::new(); + }; + extract_paths_from_patch_from_cwd(diff_text, &cwd) +} + +fn extract_paths_from_patch_from_cwd(diff_text: &str, cwd: &Path) -> Vec { let Ok((tmpdir, patch_path)) = write_temp_patch(diff_text) else { return Vec::new(); }; - let paths = std::env::current_dir() - .ok() - .and_then(|cwd| GitRunner::for_cwd(&cwd).ok()) - .and_then(|git| { - extract_effective_paths_from_patch(&git, &patch_path, /*revert*/ false).ok() - }) - .unwrap_or_default(); + let paths = (|| -> io::Result> { + let git = GitRunner::for_cwd_io(cwd)?; + let git_root = crate::get_git_repo_root(cwd) + .ok_or_else(|| io::Error::other("not a Git repository"))?; + let git_root = std::fs::canonicalize(git_root)?; + ensure_no_worktree_primary_config_sources(&git, &git_root)?; + extract_effective_paths_from_patch(&git, &git_root, &patch_path, /*revert*/ false, &[]) + })() + .unwrap_or_default(); drop(tmpdir); paths } fn git_apply_numstat_paths( git: &GitRunner, + authorized_cwd: &Path, patch_path: &Path, revert: bool, + git_config_args: &[String], ) -> io::Result> { - let command_cwd = patch_path.parent().unwrap_or_else(|| Path::new(".")); - let mut cmd = git.command_for_cwd(command_cwd)?; + let mut cmd = git.command_for_cwd(authorized_cwd)?; + cmd.args(git_config_args); cmd.args(["apply", "--numstat", "-z"]); if revert { cmd.arg("-R"); @@ -226,16 +244,25 @@ fn invalid_windows_patch_component(component: &str) -> bool { /// 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 git_config_args = safe_git_config_parts(); + ensure_no_worktree_primary_config_sources(&git, git_root)?; let (tmpdir, patch_path) = write_temp_patch(diff)?; - let paths = extract_effective_paths_from_patch(&git, &patch_path, /*revert*/ true)?; + let paths = extract_effective_paths_from_patch( + &git, + git_root, + &patch_path, + /*revert*/ true, + &git_config_args, + )?; let _guard = tmpdir; - stage_effective_paths(&git, git_root, &paths) + stage_effective_paths(&git, git_root, &paths, &git_config_args) } pub(crate) fn stage_effective_paths( git: &GitRunner, git_root: &Path, paths: &[String], + git_config_args: &[String], ) -> io::Result<()> { let confined = confine_patch_paths(git, git_root, paths)?; let mut existing = Vec::new(); @@ -259,8 +286,7 @@ pub(crate) fn stage_effective_paths( "--".to_string(), ]; args.extend(existing); - let config_parts = safe_git_config_parts(); - let (_code, _, _) = run_git(git, git_root, &config_parts, &args)?; + let (_code, _, _) = run_git(git, git_root, git_config_args, &args)?; // We do not hard fail staging; best-effort is OK. Return Ok even on non-zero. Ok(()) } diff --git a/codex-rs/git-utils/src/patch_paths_tests.rs b/codex-rs/git-utils/src/patch_paths_tests.rs index e805c2d75b..4d6643a97a 100644 --- a/codex-rs/git-utils/src/patch_paths_tests.rs +++ b/codex-rs/git-utils/src/patch_paths_tests.rs @@ -62,13 +62,23 @@ fn read_file_normalized(path: &Path) -> String { fn effective_paths(diff: &str, revert: bool) -> io::Result> { let (tmpdir, patch_path) = write_temp_patch(diff)?; - let cwd = std::env::current_dir()?; - let git = GitRunner::for_cwd_io(&cwd)?; - let paths = extract_effective_paths_from_patch(&git, &patch_path, revert)?; + let repo = init_repo(); + let cwd = repo.path(); + let git = GitRunner::for_cwd_io(cwd)?; + let git_root = + crate::get_git_repo_root(cwd).ok_or_else(|| io::Error::other("not a Git repository"))?; + let git_root = std::fs::canonicalize(git_root)?; + ensure_no_worktree_primary_config_sources(&git, &git_root)?; + let paths = extract_effective_paths_from_patch(&git, &git_root, &patch_path, revert, &[])?; drop(tmpdir); Ok(paths) } +fn best_effort_paths(diff: &str) -> Vec { + let repo = init_repo(); + extract_paths_from_patch_from_cwd(diff, repo.path()) +} + #[test] fn effective_paths_cover_supported_patch_headers() { let cases = [ @@ -127,7 +137,7 @@ fn effective_paths_cover_supported_patch_headers() { "{name}, revert={revert}" ); } - assert_eq!(extract_paths_from_patch(diff), expected, "{name}"); + assert_eq!(best_effort_paths(diff), expected, "{name}"); } let nul_rename_paths = parse_numstat_paths(b"0\t0\t\0old name.txt\0new name.txt\0") @@ -150,7 +160,7 @@ fn effective_paths_follow_git_for_mismatched_headers() { effective_paths(mismatch, /*revert*/ true).unwrap(), expected ); - assert_eq!(extract_paths_from_patch(mismatch), expected); + assert_eq!(best_effort_paths(mismatch), expected); } #[test] diff --git a/codex-rs/git-utils/src/path_authority.rs b/codex-rs/git-utils/src/path_authority.rs index b567df0368..1e2157c378 100644 --- a/codex-rs/git-utils/src/path_authority.rs +++ b/codex-rs/git-utils/src/path_authority.rs @@ -88,7 +88,7 @@ pub(crate) fn repository_route_boundaries( let canonical_root = std::fs::canonicalize(root)?; let canonical_marker = std::fs::canonicalize(&marker)?; if canonical_marker == canonical_root.join(".git") { - push_unique_location(&mut metadata_dirs, canonical_marker)?; + push_unique_location(&mut metadata_dirs, marker)?; } } for common in common_dirs { diff --git a/codex-rs/git-utils/src/repository_authority/authority/policy.rs b/codex-rs/git-utils/src/repository_authority/authority/policy.rs index e77afde247..e5f2379460 100644 --- a/codex-rs/git-utils/src/repository_authority/authority/policy.rs +++ b/codex-rs/git-utils/src/repository_authority/authority/policy.rs @@ -33,6 +33,29 @@ impl RepositoryAuthority { Ok(canonical) } + pub(crate) fn ensure_config_source_is_not_worktree_controlled( + &self, + path: &Path, + description: &str, + ) -> io::Result<()> { + if !path.is_absolute() { + return Err(worktree_controlled_config_source(path, description)); + } + let inspection = self.route_boundaries.inspect_route(path)?; + if inspection.crosses_worktree { + return Err(worktree_controlled_config_source(path, description)); + } + for observed in inspection.observed_paths { + if self.route_boundaries.contains_known_boundary(&observed)? { + continue; + } + if self.has_related_repository_ancestor(&observed)? { + return Err(worktree_controlled_config_source(path, description)); + } + } + Ok(()) + } + pub(super) fn path_is_untrusted_for_executable_result(&self, path: &Path) -> io::Result { let inspection = self.route_boundaries.inspect_route(path)?; if inspection.touches_worktree || inspection.crosses_metadata { @@ -145,3 +168,13 @@ impl RepositoryAuthority { Ok(()) } } + +fn worktree_controlled_config_source(path: &Path, description: &str) -> io::Error { + io::Error::new( + io::ErrorKind::PermissionDenied, + format!( + "refusing to use worktree-controlled {description}: {}", + path.display() + ), + ) +}