diff --git a/codex-rs/git-utils/src/patch_paths.rs b/codex-rs/git-utils/src/patch_paths.rs index d6889fce89..5ec66152df 100644 --- a/codex-rs/git-utils/src/patch_paths.rs +++ b/codex-rs/git-utils/src/patch_paths.rs @@ -9,6 +9,7 @@ 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::safe_git::ensure_no_selected_executable_git_filters; pub(crate) fn extract_effective_paths_from_patch( git: &GitRunner, @@ -238,6 +239,7 @@ pub(crate) fn stage_effective_paths( git_root: &Path, paths: &[String], ) -> io::Result<()> { + ensure_no_selected_executable_git_filters(git, git_root, paths, &[])?; let confined = confine_patch_paths(git, git_root, paths)?; let mut existing = Vec::new(); for path in confined.into_exact_leaves()? { diff --git a/codex-rs/git-utils/src/patch_paths_tests.rs b/codex-rs/git-utils/src/patch_paths_tests.rs index e805c2d75b..7f0e72ac72 100644 --- a/codex-rs/git-utils/src/patch_paths_tests.rs +++ b/codex-rs/git-utils/src/patch_paths_tests.rs @@ -717,3 +717,73 @@ fn containment_metadata_queries_ignore_inherited_git_selection_environment() { ); } } + +#[cfg(unix)] +#[test] +fn staging_rejects_global_lfs_filter_without_running_it() { + let _g = env_lock().lock().unwrap(); + if std::env::var_os("CODEX_GIT_UTILS_PATH_ENV_CHILD").is_some() { + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_UTILS_TARGET_REPO").expect("target repository"), + ); + let git = GitRunner::for_cwd_io(&root).expect("trusted Git"); + let error = stage_effective_paths(&git, &root, &["file.txt".to_string()]) + .expect_err("reject global Git LFS filter"); + assert_eq!(error.kind(), io::ErrorKind::Unsupported); + return; + } + + let repo = init_repo(); + let root = repo.path(); + std::fs::write(root.join(".gitattributes"), "file.txt filter=lfs\n").expect("write attributes"); + std::fs::write(root.join("file.txt"), "old\n").expect("write file"); + let (add_code, _, add_err) = run(root, &["git", "add", "."]); + assert_eq!(add_code, 0, "add base files: {add_err}"); + let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "base"]); + assert_eq!(commit_code, 0, "commit base files: {commit_err}"); + std::fs::write(root.join("file.txt"), "new\n").expect("modify file"); + + let config_dir = tempfile::tempdir().expect("config tempdir"); + let global_config = config_dir.path().join("global.gitconfig"); + let system_config = config_dir.path().join("system.gitconfig"); + let filter_marker = config_dir.path().join("repo-lfs-ran"); + let repo_git_lfs = root.join("git-lfs"); + std::fs::write( + &repo_git_lfs, + "#!/bin/sh\n: > \"$CODEX_GIT_UTILS_LFS_MARKER\"\ncat\n", + ) + .expect("write repository git-lfs"); + { + use std::os::unix::fs::PermissionsExt; + + let mut permissions = std::fs::metadata(&repo_git_lfs) + .expect("repository git-lfs metadata") + .permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&repo_git_lfs, permissions) + .expect("make repository git-lfs executable"); + } + std::fs::write( + &global_config, + "[filter \"lfs\"]\n\tclean = git-lfs clean -- %f\n\trequired = true\n", + ) + .expect("write global config"); + std::fs::write(&system_config, "").expect("write system config"); + run_isolated_test( + "patch_paths::tests::staging_rejects_global_lfs_filter_without_running_it", + &[ + ("CODEX_GIT_UTILS_TARGET_REPO", root.as_os_str()), + ("CODEX_GIT_UTILS_LFS_MARKER", filter_marker.as_os_str()), + ("GIT_CONFIG_GLOBAL", global_config.as_os_str()), + ("GIT_CONFIG_SYSTEM", system_config.as_os_str()), + ("GIT_EXEC_PATH", root.as_os_str()), + ("GIT_GLOB_PATHSPECS", OsStr::new("1")), + ("GIT_ICASE_PATHSPECS", OsStr::new("1")), + ], + ); + + assert!(!filter_marker.exists(), "Git LFS filter must not run"); + let (diff_code, staged, diff_err) = run(root, &["git", "diff", "--cached", "--name-only"]); + assert_eq!(diff_code, 0, "read staged paths: {diff_err}"); + assert!(staged.is_empty(), "staging changed the index: {staged}"); +} diff --git a/codex-rs/git-utils/src/safe_git_tests.rs b/codex-rs/git-utils/src/safe_git_tests.rs index 7f5dca8a83..5d6efe0147 100644 --- a/codex-rs/git-utils/src/safe_git_tests.rs +++ b/codex-rs/git-utils/src/safe_git_tests.rs @@ -1,8 +1,17 @@ use super::*; +use crate::apply::ApplyGitRequest; +use crate::apply::apply_git_patch; use crate::git_config::GitConfigScope; +#[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 selected_filter_policy_allows_unused_and_rejects_selected_at_every_scope() { @@ -106,3 +115,267 @@ 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); + command + .arg(test_name) + .arg("--exact") + .arg("--nocapture") + .env("CODEX_GIT_UTILS_SAFE_GIT_ENV_CHILD", "1") + .env("RUST_TEST_THREADS", "1"); + for (name, value) in env { + command.env(name, value); + } + let output = command.output().expect("run isolated test process"); + assert!( + output.status.success(), + "isolated test {test_name} failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + +async fn run_git(repo_path: &Path, args: &[&str]) { + let output = TokioCommand::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 +} + +#[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); + let contents = std::fs::read_to_string(repo_path.join("test.txt")).expect("read result"); + assert!( + matches!(contents.as_str(), "new\n" | "new\r\n"), + "expected the patched contents with a platform line ending, got {contents:?}" + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn apply_and_stage_reject_global_relative_filter_without_running_it() { + if std::env::var_os("CODEX_GIT_UTILS_SAFE_GIT_ENV_CHILD").is_none() { + let config_dir = tempfile::tempdir().expect("config tempdir"); + let global_config = config_dir.path().join("global.gitconfig"); + let system_config = config_dir.path().join("system.gitconfig"); + std::fs::write(&global_config, "[filter \"evil\"]\n\tclean = ./clean.sh\n") + .expect("write global config"); + std::fs::write(&system_config, "").expect("write system config"); + run_isolated_test( + "safe_git::tests::apply_and_stage_reject_global_relative_filter_without_running_it", + &[ + ("GIT_CONFIG_GLOBAL", global_config.as_os_str()), + ("GIT_CONFIG_SYSTEM", system_config.as_os_str()), + ], + ); + return; + } + + 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; + let marker = repo_path.join("filter-ran"); + std::fs::write(repo_path.join("test.txt"), "old\n").expect("tracked file"); + std::fs::write(repo_path.join(".gitattributes"), "test.txt filter=evil\n").expect("attributes"); + std::fs::write( + repo_path.join("clean.sh"), + format!("#!/bin/sh\ntouch '{}'\ncat\n", marker.display()), + ) + .expect("relative filter"); + let mut permissions = std::fs::metadata(repo_path.join("clean.sh")) + .expect("filter metadata") + .permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(repo_path.join("clean.sh"), permissions).expect("filter executable"); + run_git( + &repo_path, + &[ + "-c", + "filter.evil.clean=", + "add", + "test.txt", + ".gitattributes", + ], + ) + .await; + run_git( + &repo_path, + &["-c", "filter.evil.clean=", "commit", "-m", "fixture"], + ) + .await; + assert!(!marker.exists(), "setup must not run filter"); + + let 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"; + let error = apply_git_patch(&ApplyGitRequest { + cwd: repo_path.clone(), + diff: diff.to_string(), + revert: false, + preflight: false, + }) + .expect_err("reject relative global filter"); + assert_eq!(error.kind(), std::io::ErrorKind::Unsupported); + assert!(!marker.exists(), "apply must not run filter"); + assert_eq!( + std::fs::read_to_string(repo_path.join("test.txt")).expect("read tracked file"), + "old\n" + ); + + let error = stage_paths(&repo_path, diff).expect_err("reject filter during staging"); + assert_eq!(error.kind(), std::io::ErrorKind::Unsupported); + assert!(!marker.exists(), "staging must not run filter"); +} + +#[cfg(unix)] +#[tokio::test] +async fn nested_cwd_rejects_global_lfs_filter_without_running_it() { + 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; + let nested = repo_path.join("nested"); + let repo_bin = repo_path.join("bin"); + std::fs::create_dir(&nested).expect("nested cwd"); + std::fs::create_dir(&repo_bin).expect("repository bin"); + std::fs::write(repo_path.join(".gitattributes"), "test.txt filter=lfs\n") + .expect("attributes"); + run_git(&repo_path, &["add", ".gitattributes"]).await; + run_git(&repo_path, &["commit", "-m", "attributes"]).await; + std::fs::write(repo_path.join("test.txt"), "changed\n").expect("modify tracked file"); + + let config_dir = tempfile::tempdir().expect("config tempdir"); + let global_config = config_dir.path().join("global.gitconfig"); + let system_config = config_dir.path().join("system.gitconfig"); + std::fs::write( + &global_config, + "[filter \"lfs\"]\n\tclean = git-lfs clean -- %f\n", + ) + .expect("write global config"); + std::fs::write(&system_config, "").expect("write system config"); + let marker = config_dir.path().join("repo-lfs-ran"); + let primary_git_marker = config_dir.path().join("repo-primary-git-ran"); + let repo_git_lfs = repo_bin.join("git-lfs"); + std::fs::write( + &repo_git_lfs, + "#!/bin/sh\n: > \"$CODEX_GIT_UTILS_UNSAFE_LFS_MARKER\"\nwhile IFS= read -r line\ndo\n printf '%s\\n' \"$line\"\ndone\n", + ) + .expect("repository git-lfs"); + let mut permissions = std::fs::metadata(&repo_git_lfs) + .expect("repository git-lfs metadata") + .permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&repo_git_lfs, permissions) + .expect("make repository git-lfs executable"); + + let output = std::process::Command::new("/bin/sh") + .args(["-c", "command -v git"]) + .output() + .expect("resolve git executable"); + assert!(output.status.success(), "resolve git executable"); + let git_path = PathBuf::from( + String::from_utf8(output.stdout) + .expect("Git path UTF-8") + .trim(), + ); + let repo_git = repo_bin.join("git"); + std::fs::write( + &repo_git, + "#!/bin/sh\nprintf ran > \"$CODEX_GIT_UTILS_PRIMARY_GIT_MARKER\"\nexec \"$CODEX_GIT_UTILS_REAL_GIT\" \"$@\"\n", + ) + .expect("repository Git"); + let mut permissions = std::fs::metadata(&repo_git) + .expect("repository Git metadata") + .permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&repo_git, permissions).expect("make repository Git executable"); + let search_path = std::env::join_paths([ + repo_bin.as_path(), + git_path.parent().expect("Git executable directory"), + ]) + .expect("construct controlled PATH"); + run_isolated_test( + "safe_git::tests::nested_cwd_rejects_global_lfs_filter_without_running_it", + &[ + ("CODEX_GIT_UTILS_TARGET_REPO", repo_path.as_os_str()), + ("CODEX_GIT_UTILS_UNSAFE_LFS_MARKER", marker.as_os_str()), + ( + "CODEX_GIT_UTILS_PRIMARY_GIT_MARKER", + primary_git_marker.as_os_str(), + ), + ("CODEX_GIT_UTILS_REAL_GIT", git_path.as_os_str()), + ("GIT_CONFIG_GLOBAL", global_config.as_os_str()), + ("GIT_CONFIG_SYSTEM", system_config.as_os_str()), + ("PATH", search_path.as_os_str()), + ], + ); + assert!(!marker.exists(), "repository git-lfs must not run"); + assert!( + !primary_git_marker.exists(), + "repository-controlled primary Git must not run" + ); + return; + } + + let repo_path = + PathBuf::from(std::env::var_os("CODEX_GIT_UTILS_TARGET_REPO").expect("target repository")); + let 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"; + let error = apply_git_patch(&ApplyGitRequest { + cwd: repo_path.join("nested"), + diff: diff.to_string(), + revert: false, + preflight: false, + }) + .expect_err("reject global Git LFS filter from nested cwd"); + assert_eq!(error.kind(), std::io::ErrorKind::Unsupported); + + let error = stage_paths(&repo_path, diff).expect_err("reject global Git LFS during staging"); + assert_eq!(error.kind(), std::io::ErrorKind::Unsupported); +}