diff --git a/codex-rs/git-utils/src/apply.rs b/codex-rs/git-utils/src/apply.rs index b9adb82e6b..20ebd63456 100644 --- a/codex-rs/git-utils/src/apply.rs +++ b/codex-rs/git-utils/src/apply.rs @@ -8,11 +8,15 @@ use once_cell::sync::Lazy; use regex::Regex; -use std::ffi::OsStr; use std::io; use std::path::Path; use std::path::PathBuf; +use crate::FsmonitorOverride; +use crate::safe_git::DISABLED_HOOKS_PATH; +use crate::safe_git::GitConfigOverride; +use crate::safe_git::configured_executable_git_config_overrides; + /// Parameters for invoking [`apply_git_patch`]. #[derive(Debug, Clone)] pub struct ApplyGitRequest { @@ -50,6 +54,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { // Stage WT paths first to avoid index mismatch on revert. stage_paths(&git_root, &req.diff)?; } + let executable_git_config_overrides = configured_executable_git_config_overrides(&git_root)?; // Build git args let mut args: Vec = vec!["apply".into(), "--3way".into()]; @@ -69,6 +74,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { cfg_parts.push(p.to_string()); } } + cfg_parts.extend(safe_git_config_parts()); args.push(patch_path.to_string_lossy().to_string()); @@ -80,7 +86,12 @@ 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_root, + &cfg_parts, + &executable_git_config_overrides, + &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 +112,12 @@ 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_root, + &cfg_parts, + &executable_git_config_overrides, + &args, + )?; let (mut applied_paths, mut skipped_paths, mut conflicted_paths) = parse_git_apply_output(&stdout, &stderr); @@ -148,7 +164,12 @@ 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)> { +fn run_git( + cwd: &Path, + git_cfg: &[String], + executable_git_config_overrides: &[GitConfigOverride], + args: &[String], +) -> io::Result<(i32, String, String)> { let mut cmd = std::process::Command::new("git"); for p in git_cfg { cmd.arg(p); @@ -156,6 +177,7 @@ fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32, for a in args { cmd.arg(a); } + add_executable_git_config_overrides(&mut cmd, executable_git_config_overrides); let out = cmd.current_dir(cwd).output()?; let code = out.status.code().unwrap_or(-1); let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); @@ -163,6 +185,30 @@ fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32, Ok((code, stdout, stderr)) } +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 add_executable_git_config_overrides( + command: &mut std::process::Command, + config_overrides: &[GitConfigOverride], +) { + if config_overrides.is_empty() { + return; + } + command.env("GIT_CONFIG_COUNT", config_overrides.len().to_string()); + for (index, (key, value)) in config_overrides.iter().enumerate() { + command + .env(format!("GIT_CONFIG_KEY_{index}"), key) + .env(format!("GIT_CONFIG_VALUE_{index}"), value); + } +} + fn quote_shell(s: &str) -> String { let simple = s .chars() @@ -329,14 +375,16 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> { if existing.is_empty() { return Ok(()); } - let mut cmd = std::process::Command::new("git"); - cmd.arg("add"); - cmd.arg("--"); - for p in &existing { - cmd.arg(OsStr::new(p)); - } - let out = cmd.current_dir(git_root).output()?; - let _code = out.status.code().unwrap_or(-1); + let mut args = vec!["add".to_string(), "--".to_string()]; + args.extend(existing); + let config_parts = safe_git_config_parts(); + let executable_git_config_overrides = configured_executable_git_config_overrides(git_root)?; + let (_code, _, _) = run_git( + git_root, + &config_parts, + &executable_git_config_overrides, + &args, + )?; // We do not hard fail staging; best-effort is OK. Return Ok even on non-zero. Ok(()) } @@ -633,6 +681,33 @@ mod tests { .replace("\r\n", "\n") } + fn configure_clean_filter(root: &Path, tracked_path: &str) { + std::fs::write( + root.join(".gitattributes"), + format!("{tracked_path} filter=x=y\n"), + ) + .expect("write attributes"); + let (add_code, _, add_err) = run(root, &["git", "add", ".gitattributes"]); + assert_eq!(add_code, 0, "add attributes: {add_err}"); + let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "attributes"]); + assert_eq!(commit_code, 0, "commit attributes: {commit_err}"); + let (config_code, _, config_err) = run( + root, + &[ + "git", + "config", + "filter.x=y.clean", + "git config codex.filterran true && git hash-object --stdin", + ], + ); + assert_eq!(config_code, 0, "configure filter: {config_err}"); + } + + fn configured_filter_ran(root: &Path) -> bool { + let (code, _, _) = run(root, &["git", "config", "--get", "codex.filterran"]); + code == 0 + } + #[test] fn extract_paths_handles_quoted_headers() { let diff = "diff --git \"a/hello world.txt\" \"b/hello world.txt\"\nnew file mode 100644\n--- /dev/null\n+++ b/hello world.txt\n@@ -0,0 +1 @@\n+hi\n"; @@ -844,4 +919,43 @@ 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 apply_ignores_configured_clean_filter() { + let _g = env_lock().lock().unwrap(); + let repo = init_repo(); + let root = repo.path(); + std::fs::write(root.join("file.txt"), "orig\n").expect("write file"); + let (add_code, _, add_err) = run(root, &["git", "add", "file.txt"]); + assert_eq!(add_code, 0, "add file: {add_err}"); + let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "seed"]); + assert_eq!(commit_code, 0, "commit file: {commit_err}"); + configure_clean_filter(root, "file.txt"); + + let diff = "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,1 +1,1 @@\n-orig\n+next\n"; + let preflight_req = ApplyGitRequest { + cwd: root.to_path_buf(), + diff: diff.to_string(), + revert: false, + preflight: true, + }; + let preflight = apply_git_patch(&preflight_req).expect("preflight apply"); + assert_eq!(preflight.exit_code, 0, "preflight apply succeeded"); + assert!(!configured_filter_ran(root)); + + let apply_req = ApplyGitRequest { + cwd: root.to_path_buf(), + diff: diff.to_string(), + revert: false, + preflight: false, + }; + let applied = apply_git_patch(&apply_req).expect("apply"); + assert_eq!( + applied.exit_code, 0, + "apply succeeded\nstdout:\n{}\nstderr:\n{}", + applied.stdout, applied.stderr + ); + assert!(!configured_filter_ran(root)); + assert_eq!(read_file_normalized(&root.join("file.txt")), "next\n"); + } } diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index 7c0d59462c..4869d63116 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -16,6 +16,10 @@ use tokio::time::timeout; use ts_rs::TS; use crate::GitSha; +use crate::safe_git::DISABLED_HOOKS_PATH; +use crate::safe_git::EXECUTABLE_GIT_CONFIG_PATTERN; +use crate::safe_git::GitConfigOverride; +use crate::safe_git::executable_git_config_overrides_from_output; /// Return `true` if the project folder specified by the `Config` is inside a /// Git repository. @@ -57,7 +61,6 @@ pub async fn get_git_repo_root_with_fs( /// Timeout for git commands to prevent freezing on large repositories const GIT_COMMAND_TIMEOUT: TokioDuration = TokioDuration::from_secs(5); -const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" }; #[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, TS)] pub struct GitInfo { @@ -433,16 +436,20 @@ async fn run_git_command_with_timeout_from( cwd: &Path, fsmonitor: crate::FsmonitorOverride, ) -> Option { + let executable_git_config_overrides = + configured_executable_git_config_overrides_from(git, cwd).await?; + let disabled_hooks = format!("core.hooksPath={DISABLED_HOOKS_PATH}"); let mut command = Command::new(git); command .env("GIT_OPTIONAL_LOCKS", "0") // Keep internal Git commands independent of repository-selected hooks - // and fsmonitor helpers while preserving built-in fsmonitor acceleration. - .args(["-c", &format!("core.hooksPath={DISABLED_HOOKS_PATH}")]) + // and executable Git helpers while preserving built-in fsmonitor acceleration. + .args(["-c", &disabled_hooks]) .args(["-c", fsmonitor.git_config_arg()]) - .args(args) .current_dir(cwd) .kill_on_drop(true); + add_executable_git_config_overrides(&mut command, &executable_git_config_overrides); + command.args(args); let result = timeout(GIT_COMMAND_TIMEOUT, command.output()).await; match result { @@ -451,6 +458,51 @@ async fn run_git_command_with_timeout_from( } } +async fn configured_executable_git_config_overrides_from( + git: &Path, + cwd: &Path, +) -> Option> { + let mut command = Command::new(git); + command + .args([ + "config", + "--null", + "--name-only", + "--get-regexp", + EXECUTABLE_GIT_CONFIG_PATTERN, + ]) + .current_dir(cwd) + .kill_on_drop(true); + let output = match timeout(GIT_COMMAND_TIMEOUT, command.output()).await { + Ok(Ok(output)) => output, + _ => return None, + }; + if !output + .status + .code() + .is_some_and(|code| code == 0 || code == 1) + { + return None; + } + + Some(executable_git_config_overrides_from_output(&output.stdout)) +} + +fn add_executable_git_config_overrides( + command: &mut Command, + config_overrides: &[GitConfigOverride], +) { + if config_overrides.is_empty() { + return; + } + command.env("GIT_CONFIG_COUNT", config_overrides.len().to_string()); + for (index, (key, value)) in config_overrides.iter().enumerate() { + command + .env(format!("GIT_CONFIG_KEY_{index}"), key) + .env(format!("GIT_CONFIG_VALUE_{index}"), value); + } +} + async fn get_git_remotes(cwd: &Path) -> Option> { let output = run_git_command_with_timeout(&["remote"], cwd).await?; if !output.status.success() { @@ -906,6 +958,68 @@ mod tests { use pretty_assertions::assert_eq; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; + use std::path::Path; + + async fn run_git(repo_path: &Path, args: &[&str]) { + let output = Command::new("git") + .args(args) + .current_dir(repo_path) + .output() + .await + .expect("run git command"); + assert!( + output.status.success(), + "git command failed: {args:?}\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + + async fn create_test_git_repo(temp_dir: &tempfile::TempDir) -> std::path::PathBuf { + let repo_path = temp_dir.path().join("repo"); + std::fs::create_dir(&repo_path).expect("create repo dir"); + run_git(&repo_path, &["init"]).await; + run_git(&repo_path, &["config", "user.name", "Test User"]).await; + run_git(&repo_path, &["config", "user.email", "test@example.com"]).await; + std::fs::write(repo_path.join("test.txt"), "test content").expect("write test file"); + run_git(&repo_path, &["add", "."]).await; + run_git(&repo_path, &["commit", "-m", "initial"]).await; + repo_path + } + + async fn configure_clean_filter(repo_path: &Path, tracked_path: &str) { + std::fs::write( + repo_path.join(".gitattributes"), + format!("{tracked_path} filter=x=y\n"), + ) + .expect("write attributes"); + run_git(repo_path, &["add", ".gitattributes"]).await; + run_git(repo_path, &["commit", "-m", "attributes"]).await; + run_git( + repo_path, + &[ + "config", + "filter.x=y.clean", + "git config codex.filterran true && git hash-object --stdin", + ], + ) + .await; + + let tracked_file = repo_path.join(tracked_path); + let contents = std::fs::read_to_string(&tracked_file).expect("read tracked file"); + std::thread::sleep(std::time::Duration::from_secs(1)); + std::fs::write(tracked_file, contents).expect("refresh tracked file"); + } + + async fn configured_filter_ran(repo_path: &Path) -> bool { + let output = Command::new("git") + .args(["config", "--get", "codex.filterran"]) + .current_dir(repo_path) + .output() + .await + .expect("read filter marker"); + output.status.success() + } #[test] fn canonicalize_git_remote_url_normalizes_github_variants() { @@ -944,6 +1058,54 @@ mod tests { } } + #[tokio::test] + async fn get_has_changes_ignores_configured_clean_filter() { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + configure_clean_filter(&repo_path, "test.txt").await; + + assert_eq!(get_has_changes(&repo_path).await, Some(false)); + assert!(!configured_filter_ran(&repo_path).await); + } + + #[tokio::test] + async fn git_diff_to_remote_ignores_configured_clean_filter() { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + let remote_path = temp_dir.path().join("remote.git"); + run_git( + temp_dir.path(), + &["init", "--bare", remote_path.to_str().unwrap()], + ) + .await; + run_git( + &repo_path, + &["remote", "add", "origin", remote_path.to_str().unwrap()], + ) + .await; + let branch_output = Command::new("git") + .args(["rev-parse", "--abbrev-ref", "HEAD"]) + .current_dir(&repo_path) + .output() + .await + .expect("read branch"); + assert!(branch_output.status.success(), "read branch"); + let branch = String::from_utf8(branch_output.stdout) + .expect("branch utf8") + .trim() + .to_string(); + run_git(&repo_path, &["push", "-u", "origin", &branch]).await; + + configure_clean_filter(&repo_path, "test.txt").await; + run_git(&repo_path, &["push", "origin", &branch]).await; + + let state = git_diff_to_remote(&repo_path) + .await + .expect("collect working tree state"); + assert!(state.diff.is_empty()); + assert!(!configured_filter_ran(&repo_path).await); + } + #[cfg(unix)] #[tokio::test] async fn fsmonitor_override_rejects_configured_helper() { @@ -994,6 +1156,9 @@ mod tests { "config --null --get core.fsmonitor".to_string(), "config --null --type=bool --fixed-value --get core.fsmonitor /tmp/fsmonitor-helper" .to_string(), + format!( + "config --null --name-only --get-regexp {EXECUTABLE_GIT_CONFIG_PATTERN}" + ), format!("-c {disabled_hooks} -c core.fsmonitor=false status --porcelain"), ] ); @@ -1081,6 +1246,7 @@ mod tests { vec![ "config --null --get core.fsmonitor".to_string(), "version --build-options".to_string(), + format!("config --null --name-only --get-regexp {EXECUTABLE_GIT_CONFIG_PATTERN}"), format!("-c {disabled_hooks} -c core.fsmonitor=true status --porcelain"), ] ); diff --git a/codex-rs/git-utils/src/lib.rs b/codex-rs/git-utils/src/lib.rs index e04e604410..3141579d4c 100644 --- a/codex-rs/git-utils/src/lib.rs +++ b/codex-rs/git-utils/src/lib.rs @@ -6,6 +6,7 @@ mod fsmonitor; mod info; 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 f17ce209b0..d7aec12cf0 100644 --- a/codex-rs/git-utils/src/operations.rs +++ b/codex-rs/git-utils/src/operations.rs @@ -5,8 +5,9 @@ use std::path::PathBuf; use std::process::Command; use crate::GitToolingError; - -const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" }; +use crate::safe_git::DISABLED_HOOKS_PATH; +use crate::safe_git::GitConfigOverride; +use crate::safe_git::configured_executable_git_config_overrides; pub(crate) fn ensure_git_repository(path: &Path) -> Result<(), GitToolingError> { match run_git_for_stdout( @@ -109,6 +110,7 @@ where for arg in iterator { args_vec.push(OsString::from(arg.as_ref())); } + let executable_git_config_overrides = configured_executable_git_config_overrides(dir)?; let command_string = build_command_string(&args_vec); let mut command = Command::new("git"); command.current_dir(dir); @@ -117,6 +119,7 @@ where command.env(key, value); } } + add_executable_git_config_overrides(&mut command, &executable_git_config_overrides); command.args(&args_vec); let output = command.output()?; if !output.status.success() { @@ -133,6 +136,21 @@ where }) } +fn add_executable_git_config_overrides( + command: &mut Command, + config_overrides: &[GitConfigOverride], +) { + if config_overrides.is_empty() { + return; + } + command.env("GIT_CONFIG_COUNT", config_overrides.len().to_string()); + for (index, (key, value)) in config_overrides.iter().enumerate() { + command + .env(format!("GIT_CONFIG_KEY_{index}"), key) + .env(format!("GIT_CONFIG_VALUE_{index}"), value); + } +} + fn build_command_string(args: &[OsString]) -> String { if args.is_empty() { return "git".to_string(); 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..6b8453fd94 --- /dev/null +++ b/codex-rs/git-utils/src/safe_git.rs @@ -0,0 +1,98 @@ +use std::collections::BTreeSet; +use std::io; +use std::path::Path; +use std::process::Command; + +pub(crate) const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" }; +pub(crate) const EXECUTABLE_GIT_CONFIG_PATTERN: &str = + r"^(filter\..*\.(clean|smudge|process|required)|merge\..*\.driver)$"; +pub(crate) type GitConfigOverride = (String, String); + +pub(crate) fn configured_executable_git_config_overrides( + cwd: &Path, +) -> io::Result> { + let output = Command::new("git") + .args([ + "config", + "--null", + "--name-only", + "--get-regexp", + EXECUTABLE_GIT_CONFIG_PATTERN, + ]) + .current_dir(cwd) + .output()?; + if output + .status + .code() + .is_some_and(|code| code == 0 || code == 1) + { + return Ok(executable_git_config_overrides_from_output(&output.stdout)); + } + + Err(io::Error::other(format!( + "git config probe failed with status {}: {}", + output.status, + String::from_utf8_lossy(&output.stderr).trim() + ))) +} + +pub(crate) fn executable_git_config_overrides_from_output(stdout: &[u8]) -> Vec { + let mut filter_drivers = BTreeSet::new(); + let mut merge_drivers = BTreeSet::new(); + + for key in stdout + .split(|byte| *byte == 0) + .filter(|key| !key.is_empty()) + .filter_map(|key| std::str::from_utf8(key).ok()) + { + if let Some(driver) = key + .strip_suffix(".clean") + .or_else(|| key.strip_suffix(".smudge")) + .or_else(|| key.strip_suffix(".process")) + .or_else(|| key.strip_suffix(".required")) + { + filter_drivers.insert(driver.to_string()); + } else if key.starts_with("merge.") && key.ends_with(".driver") { + merge_drivers.insert(key.to_string()); + } + } + + filter_drivers + .into_iter() + .flat_map(|driver| { + [ + (format!("{driver}.clean"), String::new()), + (format!("{driver}.smudge"), String::new()), + (format!("{driver}.process"), String::new()), + (format!("{driver}.required"), "false".to_string()), + ] + }) + .chain( + merge_drivers + .into_iter() + .map(|driver| (driver, String::new())), + ) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn executable_git_config_overrides_clear_filters_and_merge_drivers() { + let output = b"filter.x=y.clean\0filter.x=y.required\0merge.pwn.driver\0"; + + assert_eq!( + executable_git_config_overrides_from_output(output), + vec![ + ("filter.x=y.clean".to_string(), String::new()), + ("filter.x=y.smudge".to_string(), String::new()), + ("filter.x=y.process".to_string(), String::new()), + ("filter.x=y.required".to_string(), "false".to_string()), + ("merge.pwn.driver".to_string(), String::new()), + ] + ); + } +}