diff --git a/codex-rs/git-utils/src/apply.rs b/codex-rs/git-utils/src/apply.rs index b269e7beb3..0021c2c3c6 100644 --- a/codex-rs/git-utils/src/apply.rs +++ b/codex-rs/git-utils/src/apply.rs @@ -17,7 +17,7 @@ use crate::patch_paths::ensure_paths_do_not_enter_submodules; use crate::patch_paths::extract_effective_paths_from_patch; use crate::patch_paths::stage_effective_paths; use crate::safe_git::DISABLED_HOOKS_PATH; -use crate::safe_git::ensure_no_executable_git_filters; +use crate::safe_git::ensure_no_selected_executable_git_filters; use crate::safe_git::isolate_git_command_environment; /// Parameters for invoking [`apply_git_patch`]. @@ -48,13 +48,13 @@ pub struct ApplyGitResult { pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { let mut cfg_parts = configured_git_config_parts(); let git_root = resolve_git_root(&req.cwd, &cfg_parts)?; - ensure_no_executable_git_filters(&git_root, &cfg_parts)?; // 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(&patch_path, req.revert)?; + ensure_no_selected_executable_git_filters(&git_root, &patch_paths, &cfg_parts)?; ensure_paths_do_not_enter_submodules(&git_root, &patch_paths)?; if !req.preflight { @@ -750,8 +750,17 @@ diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1 "filter.codex-test.clean=git hash-object --stdin".to_string(), ]; - let error = ensure_no_executable_git_filters(repo.path(), &config_args) - .expect_err("reject command-scoped filter"); + std::fs::write( + repo.path().join(".gitattributes"), + "test.txt filter=codex-test\n", + ) + .expect("attributes"); + let error = ensure_no_selected_executable_git_filters( + repo.path(), + &["test.txt".to_string()], + &config_args, + ) + .expect_err("reject command-scoped filter"); assert_eq!(error.kind(), io::ErrorKind::Unsupported); } diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index 0a73626164..ac1addb290 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -20,8 +20,9 @@ use ts_rs::TS; use crate::GitSha; use crate::safe_git::DISABLED_HOOKS_PATH; use crate::safe_git::GIT_COMMAND_TIMEOUT; -use crate::safe_git::has_configured_executable_filters_from; +use crate::safe_git::has_selected_executable_filters_from; use crate::safe_git::isolate_tokio_git_command_environment; +use crate::safe_git::safe_untracked_paths_for_diff; /// Return `true` if the project folder specified by the `Config` is inside a /// Git repository. @@ -289,7 +290,7 @@ fn trim_git_suffix(value: &str) -> &str { pub async fn get_has_changes(cwd: &Path) -> Option { let git = Path::new("git"); - if has_configured_executable_filters_from(git, cwd).await? { + if has_selected_executable_filters_from(git, cwd).await? { return None; } let fsmonitor = detect_local_fsmonitor_override(git, cwd).await; @@ -749,9 +750,7 @@ async fn find_closest_sha(cwd: &Path, branches: &[String], remotes: &[String]) - async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option { let git = Path::new("git"); - if has_configured_executable_filters_from(git, cwd).await? { - return None; - } + let untracked = safe_untracked_paths_for_diff(git, cwd).await?; let fsmonitor = detect_local_fsmonitor_override(git, cwd).await; let output = run_git_command_with_timeout_from( git, @@ -775,47 +774,20 @@ async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option { } let mut diff = String::from_utf8(output.stdout).ok()?; - if let Some(untracked_output) = run_git_command_with_timeout_from( - git, - &["ls-files", "--others", "--exclude-standard"], - cwd, - fsmonitor, - ) - .await - && untracked_output.status.success() - { - let untracked: Vec = String::from_utf8(untracked_output.stdout) - .ok()? - .lines() - .map(str::to_string) - .filter(|s| !s.is_empty()) - .collect(); - - if !untracked.is_empty() { - // Use platform-appropriate null device and guard paths with `--`. - let null_device: &str = if cfg!(windows) { "NUL" } else { "/dev/null" }; - let futures_iter = untracked.into_iter().map(|file| async move { - let file_owned = file; - let args_vec: Vec<&str> = vec![ - "diff", - "--no-textconv", - "--no-ext-diff", - "--binary", - "--no-index", - // -- ensures that filenames that start with - are not treated as options. - "--", - null_device, - &file_owned, - ]; - run_git_command_with_timeout_from(git, &args_vec, cwd, fsmonitor).await - }); - let results = join_all(futures_iter).await; - for extra in results.into_iter().flatten() { - if extra.status.code().is_some_and(|c| c == 0 || c == 1) - && let Ok(s) = String::from_utf8(extra.stdout) - { - diff.push_str(&s); - } + if !untracked.is_empty() { + let untracked = untracked + .into_iter() + .map(git_path_bytes_to_os_string) + .collect::>>()?; + let futures_iter = untracked.into_iter().map(|file| async move { + run_git_no_index_diff_from(git, cwd, fsmonitor, &file).await + }); + let results = join_all(futures_iter).await; + for extra in results.into_iter().flatten() { + if extra.status.code().is_some_and(|c| c == 0 || c == 1) + && let Ok(s) = String::from_utf8(extra.stdout) + { + diff.push_str(&s); } } } @@ -823,6 +795,50 @@ async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option { Some(diff) } +fn git_path_bytes_to_os_string(path: Vec) -> Option { + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + Some(std::ffi::OsString::from_vec(path)) + } + #[cfg(windows)] + { + String::from_utf8(path).ok().map(std::ffi::OsString::from) + } +} + +async fn run_git_no_index_diff_from( + git: &Path, + cwd: &Path, + fsmonitor: crate::FsmonitorOverride, + path: &OsStr, +) -> Option { + let disabled_hooks = format!("core.hooksPath={DISABLED_HOOKS_PATH}"); + let null_device = if cfg!(windows) { "NUL" } else { "/dev/null" }; + let mut command = Command::new(git); + isolate_tokio_git_command_environment(&mut command); + command + .env("GIT_OPTIONAL_LOCKS", "0") + .args(["-c", &disabled_hooks]) + .args(["-c", fsmonitor.git_config_arg()]) + .args([ + "diff", + "--no-textconv", + "--no-ext-diff", + "--binary", + "--no-index", + "--", + null_device, + ]) + .arg(path) + .current_dir(cwd) + .kill_on_drop(true); + match timeout(GIT_COMMAND_TIMEOUT, command.output()).await { + Ok(Ok(output)) => Some(output), + _ => None, + } +} + /// Resolve the path that should be used for trust checks. Similar to /// `[get_git_repo_root]`, but resolves to the root of the main /// repository. Handles worktrees via filesystem inspection without invoking diff --git a/codex-rs/git-utils/src/operations.rs b/codex-rs/git-utils/src/operations.rs index 4f4a06a100..95a2c9b9bc 100644 --- a/codex-rs/git-utils/src/operations.rs +++ b/codex-rs/git-utils/src/operations.rs @@ -209,6 +209,10 @@ mod tests { (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"); diff --git a/codex-rs/git-utils/src/patch_paths.rs b/codex-rs/git-utils/src/patch_paths.rs index 903090267c..d8b799c7e2 100644 --- a/codex-rs/git-utils/src/patch_paths.rs +++ b/codex-rs/git-utils/src/patch_paths.rs @@ -8,7 +8,7 @@ use crate::apply::run_git; use crate::apply::safe_git_config_parts; use crate::apply::write_temp_patch; use crate::git_config::path_is_within; -use crate::safe_git::ensure_no_executable_git_filters; +use crate::safe_git::ensure_no_selected_executable_git_filters; use crate::safe_git::isolate_git_command_environment; pub(crate) fn extract_effective_paths_from_patch( @@ -175,7 +175,7 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> { } pub(crate) fn stage_effective_paths(git_root: &Path, paths: &[String]) -> io::Result<()> { - ensure_no_executable_git_filters(git_root, &[])?; + ensure_no_selected_executable_git_filters(git_root, paths, &[])?; let mut existing: Vec = Vec::new(); for p in paths { let joined = git_root.join(p); diff --git a/codex-rs/git-utils/src/safe_git.rs b/codex-rs/git-utils/src/safe_git.rs index a8339e26e8..6448330fc2 100644 --- a/codex-rs/git-utils/src/safe_git.rs +++ b/codex-rs/git-utils/src/safe_git.rs @@ -1,7 +1,12 @@ use std::collections::BTreeMap; +use std::collections::BTreeSet; use std::io; +use std::io::Seek; +use std::io::Write; use std::path::Path; +use std::path::PathBuf; use std::process::Command; +use std::process::Stdio; use tokio::process::Command as TokioCommand; use tokio::time::Duration; use tokio::time::timeout; @@ -14,7 +19,7 @@ pub(crate) const EXECUTABLE_FILTER_CONFIG_PATTERN: &str = r"^filter\..*\.(clean| /// Timeout for internal Git commands to prevent freezing on large repositories. pub(crate) const GIT_COMMAND_TIMEOUT: Duration = Duration::from_secs(5); -const ISOLATED_GIT_ENVIRONMENT: [&str; 10] = [ +const ISOLATED_GIT_ENVIRONMENT: [&str; 11] = [ "GIT_DIR", "GIT_WORK_TREE", "GIT_COMMON_DIR", @@ -25,6 +30,10 @@ const ISOLATED_GIT_ENVIRONMENT: [&str; 10] = [ "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 @@ -43,41 +52,146 @@ pub(crate) fn isolate_tokio_git_command_environment(command: &mut tokio::process } } -pub(crate) async fn has_configured_executable_filters_from(git: &Path, cwd: &Path) -> Option { +pub(crate) async fn has_selected_executable_filters_from(git: &Path, cwd: &Path) -> Option { + let git_root = resolve_git_root_async(git, cwd).await?; + let entries = read_filter_config_async(git, &git_root).await?; + if !entries.values().any(|entry| !entry.value.is_empty()) { + return Some(false); + } + let paths = read_paths_async(git, &git_root, PathSelection::Tracked).await?; + let attributes = read_filter_attributes_async(git, &git_root, &paths).await?; + selected_executable_filter(&entries, &attributes) + .ok() + .map(|selected| selected.is_some()) +} + +/// Validate every tracked path plus the exact untracked paths that the caller +/// will later feed to `git diff --no-index`. The returned raw paths must be +/// reused rather than reconstructed from Git's quoted line-oriented output. +pub(crate) async fn safe_untracked_paths_for_diff(git: &Path, cwd: &Path) -> Option>> { + let requested_cwd = std::fs::canonicalize(cwd).ok()?; + let git_root = resolve_git_root_async(git, &requested_cwd).await?; + let untracked = read_paths_async(git, &requested_cwd, PathSelection::Untracked).await?; + // An embedded untracked repository is reported by `ls-files` as a single + // directory entry. Passing that entry to `git diff --no-index` invokes + // file-vs-directory comparison semantics, which can derive and open a + // child path that was never returned by the path probe. Fail closed rather + // than let the sink operate on a different path vector than we validated. + for path in &untracked { + let path = git_path_bytes_to_path_buf(path)?; + if std::fs::symlink_metadata(requested_cwd.join(path)) + .ok()? + .file_type() + .is_dir() + { + return None; + } + } + let entries = read_filter_config_async(git, &git_root).await?; + if !entries.values().any(|entry| !entry.value.is_empty()) { + return Some(untracked); + } + + let tracked = read_paths_async(git, &git_root, PathSelection::Tracked).await?; + let tracked_attributes = read_filter_attributes_async(git, &git_root, &tracked).await?; + if selected_executable_filter(&entries, &tracked_attributes) + .ok()? + .is_some() + { + return None; + } + let untracked_attributes = + read_filter_attributes_async(git, &requested_cwd, &untracked).await?; + if selected_executable_filter(&entries, &untracked_attributes) + .ok()? + .is_some() + { + return None; + } + Some(untracked) +} + +fn git_path_bytes_to_path_buf(path: &[u8]) -> Option { + #[cfg(unix)] + { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + + Some(PathBuf::from(OsString::from_vec(path.to_vec()))) + } + #[cfg(windows)] + { + String::from_utf8(path.to_vec()).ok().map(PathBuf::from) + } +} + +async fn resolve_git_root_async(git: &Path, cwd: &Path) -> Option { + let requested_cwd = std::fs::canonicalize(cwd).ok()?; let mut command = TokioCommand::new(git); isolate_tokio_git_command_environment(&mut command); command + .env("GIT_OPTIONAL_LOCKS", "0") .args([ - "config", - "--null", - "--show-scope", - "--show-origin", - "--includes", - "--get-regexp", - EXECUTABLE_FILTER_CONFIG_PATTERN, + "-c", + &format!("core.hooksPath={DISABLED_HOOKS_PATH}"), + "-c", + "core.fsmonitor=false", + "rev-parse", + "--show-toplevel", ]) - .current_dir(cwd) + .current_dir(&requested_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) - { + if !output.status.success() { return None; } - - let entries = parse_effective_config(&output.stdout).ok()?; - Some(config_entries_have_untrusted_filters(&entries)) + let path = String::from_utf8(output.stdout).ok()?; + let path = path.trim_end_matches(['\r', '\n']); + if path.is_empty() { + return None; + } + let reported_root = std::fs::canonicalize(PathBuf::from(path)).ok()?; + let expected_root = crate::get_git_repo_root(&requested_cwd) + .and_then(|root| std::fs::canonicalize(root).ok())?; + if reported_root != expected_root { + return None; + } + Some(reported_root) } -pub(crate) fn ensure_no_executable_git_filters( +pub(crate) fn ensure_no_selected_executable_git_filters( cwd: &Path, + paths: &[String], git_config_args: &[String], ) -> io::Result<()> { + let entries = read_filter_config(cwd, git_config_args)?; + if !entries.values().any(|entry| !entry.value.is_empty()) { + return Ok(()); + } + let paths = paths + .iter() + .map(|path| path.as_bytes().to_vec()) + .collect::>(); + let attributes = read_filter_attributes(cwd, &paths, git_config_args)?; + if let Some((driver, path)) = selected_executable_filter(&entries, &attributes)? { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + format!( + "refusing to run an internal Git worktree operation with executable filter {driver:?} selected for {}", + String::from_utf8_lossy(&path) + ), + )); + } + Ok(()) +} + +fn read_filter_config( + cwd: &Path, + git_config_args: &[String], +) -> io::Result> { let mut command = Command::new("git"); isolate_git_command_environment(&mut command); let output = command @@ -105,18 +219,271 @@ pub(crate) fn ensure_no_executable_git_filters( String::from_utf8_lossy(&output.stderr).trim() ))); } - let entries = parse_effective_config(&output.stdout)?; - if config_entries_have_untrusted_filters(&entries) { - return Err(io::Error::new( - io::ErrorKind::Unsupported, - "refusing to run an internal Git worktree operation with an executable Git filter configured", - )); + parse_effective_config(&output.stdout) +} + +async fn read_filter_config_async( + git: &Path, + cwd: &Path, +) -> Option> { + let mut command = TokioCommand::new(git); + isolate_tokio_git_command_environment(&mut command); + command + .args([ + "config", + "--null", + "--show-scope", + "--show-origin", + "--includes", + "--get-regexp", + EXECUTABLE_FILTER_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; + } + parse_effective_config(&output.stdout).ok() +} + +#[derive(Clone, Copy)] +enum PathSelection { + Tracked, + Untracked, +} + +async fn read_paths_async( + git: &Path, + cwd: &Path, + selection: PathSelection, +) -> Option>> { + let mut command = TokioCommand::new(git); + isolate_tokio_git_command_environment(&mut command); + let hooks_config = format!("core.hooksPath={DISABLED_HOOKS_PATH}"); + let mut args = vec![ + "-c", + hooks_config.as_str(), + "-c", + "core.fsmonitor=false", + "ls-files", + "-z", + ]; + match selection { + PathSelection::Tracked => args.push("--cached"), + PathSelection::Untracked => args.extend(["--others", "--exclude-standard"]), + } + command + .env("GIT_OPTIONAL_LOCKS", "0") + .args(args) + .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.success() { + return None; + } + parse_nul_paths(&output.stdout).ok() +} + +async fn read_filter_attributes_async( + git: &Path, + cwd: &Path, + paths: &[Vec], +) -> Option, String>> { + if paths.is_empty() { + return Some(BTreeMap::new()); + } + let mut input = tempfile::tempfile().ok()?; + write_nul_paths(&mut input, paths).ok()?; + input.rewind().ok()?; + + let mut command = TokioCommand::new(git); + isolate_tokio_git_command_environment(&mut command); + command + .env("GIT_OPTIONAL_LOCKS", "0") + .args([ + "-c", + &format!("core.hooksPath={DISABLED_HOOKS_PATH}"), + "-c", + "core.fsmonitor=false", + "check-attr", + "--stdin", + "-z", + "filter", + ]) + .current_dir(cwd) + .stdin(Stdio::from(input)) + .kill_on_drop(true); + let output = match timeout(GIT_COMMAND_TIMEOUT, command.output()).await { + Ok(Ok(output)) => output, + _ => return None, + }; + if !output.status.success() { + return None; + } + parse_filter_attributes(&output.stdout, paths).ok() +} + +fn read_filter_attributes( + cwd: &Path, + paths: &[Vec], + git_config_args: &[String], +) -> io::Result, String>> { + if paths.is_empty() { + return Ok(BTreeMap::new()); + } + let mut input = tempfile::tempfile()?; + write_nul_paths(&mut input, paths)?; + input.rewind()?; + + let mut command = Command::new("git"); + isolate_git_command_environment(&mut command); + let output = command + .env("GIT_OPTIONAL_LOCKS", "0") + .args(git_config_args) + .args([ + "-c", + &format!("core.hooksPath={DISABLED_HOOKS_PATH}"), + "-c", + "core.fsmonitor=false", + "check-attr", + "--stdin", + "-z", + "filter", + ]) + .current_dir(cwd) + .stdin(Stdio::from(input)) + .output()?; + if !output.status.success() { + return Err(io::Error::other(format!( + "git filter attribute probe failed with status {}: {}", + output.status, + String::from_utf8_lossy(&output.stderr).trim() + ))); + } + parse_filter_attributes(&output.stdout, paths) +} + +fn selected_executable_filter( + entries: &BTreeMap, + attributes: &BTreeMap, String>, +) -> io::Result)>> { + let mut executable_drivers = BTreeSet::new(); + for entry in entries.values() { + let driver = filter_driver_name(&entry.key)?; + if !entry.value.is_empty() { + executable_drivers.insert(driver); + } + } + for (path, driver) in attributes { + if executable_drivers.contains(driver) { + return Ok(Some((driver.clone(), path.clone()))); + } + } + Ok(None) +} + +fn filter_driver_name(key: &str) -> io::Result { + let Some(remainder) = key.strip_prefix("filter.") else { + return Err(invalid_filter_output("malformed filter config key")); + }; + let driver = [".clean", ".smudge", ".process"] + .into_iter() + .find_map(|suffix| remainder.strip_suffix(suffix)) + .filter(|driver| !driver.is_empty()) + .ok_or_else(|| invalid_filter_output("malformed filter config key"))?; + Ok(driver.to_string()) +} + +fn parse_nul_paths(output: &[u8]) -> io::Result>> { + if output.is_empty() { + return Ok(Vec::new()); + } + let Some(body) = output.strip_suffix(&[0]) else { + return Err(invalid_filter_output("unterminated Git path output")); + }; + let mut paths = Vec::new(); + for path in body.split(|byte| *byte == 0) { + if path.is_empty() { + return Err(invalid_filter_output("empty Git path")); + } + paths.push(path.to_vec()); + } + Ok(paths) +} + +fn write_nul_paths(input: &mut std::fs::File, paths: &[Vec]) -> io::Result<()> { + let mut unique = BTreeSet::new(); + for path in paths { + if path.is_empty() || path.contains(&0) { + return Err(invalid_filter_output("invalid Git path")); + } + if unique.insert(path.as_slice()) { + input.write_all(path)?; + input.write_all(&[0])?; + } } Ok(()) } -fn config_entries_have_untrusted_filters(entries: &BTreeMap) -> bool { - entries.values().any(|entry| !entry.value.is_empty()) +fn parse_filter_attributes( + output: &[u8], + expected_paths: &[Vec], +) -> io::Result, String>> { + let expected = expected_paths + .iter() + .map(Vec::as_slice) + .collect::>(); + if expected.is_empty() && output.is_empty() { + return Ok(BTreeMap::new()); + } + let Some(body) = output.strip_suffix(&[0]) else { + return Err(invalid_filter_output( + "unterminated Git filter attribute output", + )); + }; + let fields = body.split(|byte| *byte == 0).collect::>(); + if fields.len() % 3 != 0 { + return Err(invalid_filter_output( + "incomplete Git filter attribute record", + )); + } + let mut attributes = BTreeMap::new(); + for record in fields.chunks_exact(3) { + if !expected.contains(record[0]) || record[1] != b"filter" { + return Err(invalid_filter_output( + "unexpected Git filter attribute record", + )); + } + let driver = std::str::from_utf8(record[2]) + .map_err(|_| invalid_filter_output("non-UTF-8 Git filter attribute value"))?; + if attributes + .insert(record[0].to_vec(), driver.to_string()) + .is_some() + { + return Err(invalid_filter_output( + "duplicate Git filter attribute record", + )); + } + } + if attributes.len() != expected.len() { + return Err(invalid_filter_output("missing Git filter attribute record")); + } + Ok(attributes) +} + +fn invalid_filter_output(message: &str) -> io::Error { + io::Error::new(io::ErrorKind::InvalidData, message) } #[cfg(test)] diff --git a/codex-rs/git-utils/src/safe_git_tests.rs b/codex-rs/git-utils/src/safe_git_tests.rs index f052cb145f..a9dfe7cdb4 100644 --- a/codex-rs/git-utils/src/safe_git_tests.rs +++ b/codex-rs/git-utils/src/safe_git_tests.rs @@ -4,16 +4,19 @@ use crate::apply::apply_git_patch; use crate::get_has_changes; use crate::git_config::GitConfigScope; use crate::git_diff_to_remote; +#[cfg(unix)] use crate::patch_paths::stage_paths; use pretty_assertions::assert_eq; use std::collections::BTreeMap; +#[cfg(unix)] use std::ffi::OsStr; use std::path::Path; +#[cfg(unix)] use std::path::PathBuf; use tokio::process::Command as TokioCommand; #[test] -fn rejects_nonempty_filters_at_every_scope_including_lfs() { +fn selected_filter_policy_allows_unused_and_rejects_selected_at_every_scope() { let config_dir = tempfile::tempdir().expect("config"); let config = config_dir.path().join("global.gitconfig"); std::fs::write(&config, "").expect("config file"); @@ -32,8 +35,18 @@ fn rejects_nonempty_filters_at_every_scope_including_lfs() { ("filter.lfs.process", "git-lfs filter-process"), ] { let entries = filter_entries(scope, &config, key, value); + let driver = filter_driver_name(key).expect("driver name"); + let selected = BTreeMap::from([(b"file.txt".to_vec(), driver.clone())]); assert!( - config_entries_have_untrusted_filters(&entries), + selected_executable_filter(&entries, &selected) + .expect("selected filter policy") + .is_some(), + "{scope:?} {key}" + ); + let unused = BTreeMap::from([(b"file.txt".to_vec(), "other".to_string())]); + assert_eq!( + selected_executable_filter(&entries, &unused).expect("unused filter policy"), + None, "{scope:?} {key}" ); } @@ -41,14 +54,46 @@ fn rejects_nonempty_filters_at_every_scope_including_lfs() { } #[test] -fn allows_only_effective_empty_filter_values() { +fn selected_filter_policy_allows_effective_empty_value() { let disabled = filter_entries( GitConfigScope::Command, Path::new("command line:"), "filter.demo.clean", "", ); - assert!(!config_entries_have_untrusted_filters(&disabled)); + let selected = BTreeMap::from([(b"file.txt".to_vec(), "demo".to_string())]); + assert_eq!( + selected_executable_filter(&disabled, &selected).expect("empty filter policy"), + None + ); +} + +#[test] +fn filter_attribute_parser_rejects_malformed_or_unexpected_records() { + let paths = vec![b"a.txt".to_vec(), b"b.txt".to_vec()]; + let parsed = + parse_filter_attributes(b"a.txt\0filter\0unspecified\0b.txt\0filter\0lfs\0", &paths) + .expect("parse attributes"); + assert_eq!( + parsed.get(b"a.txt".as_slice()).map(String::as_str), + Some("unspecified") + ); + assert_eq!( + parsed.get(b"b.txt".as_slice()).map(String::as_str), + Some("lfs") + ); + + for output in [ + b"a.txt\0filter\0unspecified".as_slice(), + b"a.txt\0merge\0unspecified\0b.txt\0filter\0lfs\0".as_slice(), + b"a.txt\0filter\0unspecified\0".as_slice(), + b"a.txt\0filter\0unspecified\0a.txt\0filter\0lfs\0".as_slice(), + ] { + assert!( + parse_filter_attributes(output, &paths).is_err(), + "{output:?}" + ); + } } fn filter_entries( @@ -73,6 +118,7 @@ fn filter_entries( )]) } +#[cfg(unix)] 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); @@ -121,6 +167,38 @@ async fn create_test_git_repo(temp_dir: &tempfile::TempDir) -> std::path::PathBu repo_path } +#[tokio::test] +async fn ordinary_apply_allows_an_unselected_executable_filter() { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + std::fs::write(repo_path.join("test.txt"), "old\n").expect("write fixture"); + run_git(&repo_path, &["add", "test.txt"]).await; + run_git(&repo_path, &["commit", "-m", "normalize fixture"]).await; + run_git( + &repo_path, + &[ + "config", + "filter.unused.clean", + "codex-definitely-missing-filter-command", + ], + ) + .await; + + let result = apply_git_patch(&ApplyGitRequest { + cwd: repo_path.clone(), + diff: "diff --git a/test.txt b/test.txt\n--- a/test.txt\n+++ b/test.txt\n@@ -1 +1 @@\n-old\n+new\n" + .to_string(), + revert: false, + preflight: false, + }) + .expect("unused filter must not block apply"); + assert_eq!(result.exit_code, 0); + assert_eq!( + std::fs::read_to_string(repo_path.join("test.txt")).expect("read result"), + "new\n" + ); +} + async fn configure_clean_filter(repo_path: &Path, tracked_path: &str) { std::fs::write( repo_path.join(".gitattributes"), @@ -237,6 +315,205 @@ async fn get_has_changes_rejects_configured_clean_filter_without_running_it() { assert!(!configured_filter_ran(&repo_path).await); } +#[tokio::test] +async fn get_has_changes_rejects_core_worktree_redirection_before_running_filter() { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + let redirected = temp_dir.path().join("redirected"); + std::fs::create_dir(&redirected).expect("redirected worktree"); + std::fs::write(redirected.join(".gitattributes"), "test.txt filter=x=y\n").expect("attributes"); + std::fs::write(redirected.join("test.txt"), "redirected content\n").expect("redirected file"); + run_git( + &repo_path, + &[ + "config", + "core.worktree", + redirected.to_str().expect("redirected path"), + ], + ) + .await; + run_git( + &repo_path, + &[ + "config", + "filter.x=y.clean", + "git config codex.filterran true && git hash-object --stdin", + ], + ) + .await; + + assert_eq!(get_has_changes(&repo_path).await, None); + assert!(!configured_filter_ran(&repo_path).await); +} + +#[cfg(unix)] +#[tokio::test] +async fn legacy_git_config_cannot_hide_a_selected_local_filter_from_probe() { + if std::env::var_os("CODEX_GIT_UTILS_SAFE_GIT_ENV_CHILD").is_none() { + use std::os::unix::fs::PermissionsExt; + + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + std::fs::write(repo_path.join(".gitattributes"), "test.txt filter=evil\n") + .expect("attributes"); + run_git(&repo_path, &["add", ".gitattributes"]).await; + run_git(&repo_path, &["commit", "-m", "attributes"]).await; + + let marker = temp_dir.path().join("filter-ran"); + let filter = repo_path.join("clean.sh"); + std::fs::write( + &filter, + format!("#!/bin/sh\n: > '{}'\ncat\n", marker.display()), + ) + .expect("filter script"); + let mut permissions = std::fs::metadata(&filter) + .expect("filter metadata") + .permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&filter, permissions).expect("filter executable"); + run_git( + &repo_path, + &[ + "config", + "--local", + "filter.evil.clean", + filter.to_str().expect("filter path"), + ], + ) + .await; + std::fs::write(repo_path.join("test.txt"), "changed\n").expect("modify tracked file"); + + let safe_config = temp_dir.path().join("safe.gitconfig"); + std::fs::write(&safe_config, "").expect("safe config"); + run_isolated_test( + "safe_git::tests::legacy_git_config_cannot_hide_a_selected_local_filter_from_probe", + &[ + ("CODEX_GIT_UTILS_TARGET_REPO", repo_path.as_os_str()), + ("GIT_CONFIG", safe_config.as_os_str()), + ], + ); + assert!(!marker.exists(), "selected local filter must not run"); + return; + } + + let repo_path = + PathBuf::from(std::env::var_os("CODEX_GIT_UTILS_TARGET_REPO").expect("target repository")); + assert_eq!(get_has_changes(&repo_path).await, None); +} + +#[tokio::test] +async fn diff_rejects_a_filter_selected_only_for_an_untracked_path() { + 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"); + add_origin_and_push(&repo_path, &remote_path).await; + std::fs::write( + repo_path.join(".gitattributes"), + "untracked.txt 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; + std::fs::write(repo_path.join("untracked.txt"), "untracked\n").expect("untracked file"); + + assert_eq!(get_has_changes(&repo_path).await, Some(true)); + assert!( + !configured_filter_ran(&repo_path).await, + "status ran filter" + ); + assert!(git_diff_to_remote(&repo_path).await.is_none()); + assert!(!configured_filter_ran(&repo_path).await, "diff ran filter"); +} + +#[cfg(unix)] +#[tokio::test] +async fn untracked_diff_reuses_raw_nul_paths_without_c_quoted_collision() { + 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"); + add_origin_and_push(&repo_path, &remote_path).await; + + // Git's line-oriented output C-quotes `dir/ab` as + // `"dir/a\\nb"`. Create an ignored file with that exact quoted spelling; + // a consumer that line-parses and reuses the display form will open the + // ignored collision instead of the nonignored raw pathname. + std::fs::write(repo_path.join(".gitignore"), "\"dir/\n").expect("ignore collision"); + std::fs::write( + repo_path.join(".gitattributes"), + "\\\"dir/** filter=collision\n", + ) + .expect("collision attributes"); + run_git(&repo_path, &["add", ".gitignore", ".gitattributes"]).await; + run_git(&repo_path, &["commit", "-m", "collision metadata"]).await; + + let raw_dir = repo_path.join("dir"); + let collision_dir = repo_path.join("\"dir"); + std::fs::create_dir(&raw_dir).expect("raw dir"); + std::fs::create_dir(&collision_dir).expect("collision dir"); + std::fs::write(raw_dir.join("a\nb"), "safe untracked\n").expect("raw newline path"); + std::fs::write(collision_dir.join("a\\nb\""), "ignored collision\n").expect("collision path"); + run_git( + &repo_path, + &[ + "config", + "filter.collision.clean", + "git config codex.filterran true && git hash-object --stdin", + ], + ) + .await; + run_git(&repo_path, &["check-ignore", "\"dir/a\\nb\""]).await; + + assert!(git_diff_to_remote(&repo_path).await.is_some()); + assert!(!configured_filter_ran(&repo_path).await); +} + +#[cfg(unix)] +#[tokio::test] +async fn untracked_diff_rejects_embedded_repo_before_derived_filter_path() { + 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"); + add_origin_and_push(&repo_path, &remote_path).await; + + // `ls-files --others` represents an embedded repository as `nested/`, but + // `git diff --no-index /dev/null nested/` compares `/dev/null` with + // `nested/null`. The latter path can select a filter that a probe of only + // the reported directory entry would miss. + std::fs::write( + repo_path.join(".gitattributes"), + "nested/null filter=evil\n", + ) + .expect("write attributes"); + run_git(&repo_path, &["add", ".gitattributes"]).await; + run_git(&repo_path, &["commit", "-m", "embedded path attributes"]).await; + run_git( + &repo_path, + &[ + "config", + "filter.evil.clean", + "git config codex.filterran true && git hash-object --stdin", + ], + ) + .await; + + let nested = repo_path.join("nested"); + std::fs::create_dir(&nested).expect("create embedded repo"); + run_git(&nested, &["init"]).await; + std::fs::write(nested.join("null"), "embedded contents\n").expect("write embedded file"); + + assert!(git_diff_to_remote(&repo_path).await.is_none()); + assert!(!configured_filter_ran(&repo_path).await); +} + #[tokio::test] async fn get_has_changes_does_not_enter_dirty_submodules() { let temp_dir = tempfile::tempdir().expect("create temp dir");