diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index d572f68f21..3356b66c14 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -804,7 +804,7 @@ "critical-section_1.2.0": "{\"dependencies\":[],\"features\":{\"restore-state-bool\":[],\"restore-state-none\":[],\"restore-state-u16\":[],\"restore-state-u32\":[],\"restore-state-u64\":[],\"restore-state-u8\":[],\"restore-state-usize\":[],\"std\":[\"restore-state-bool\"]}}", "crossbeam-channel_0.5.15": "{\"dependencies\":[{\"default_features\":false,\"name\":\"crossbeam-utils\",\"req\":\"^0.8.18\"},{\"kind\":\"dev\",\"name\":\"num_cpus\",\"req\":\"^1.13.0\"},{\"kind\":\"dev\",\"name\":\"rand\",\"req\":\"^0.8\"},{\"kind\":\"dev\",\"name\":\"signal-hook\",\"req\":\"^0.3\"}],\"features\":{\"default\":[\"std\"],\"std\":[\"crossbeam-utils/std\"]}}", "crossbeam-deque_0.8.6": "{\"dependencies\":[{\"default_features\":false,\"name\":\"crossbeam-epoch\",\"req\":\"^0.9.17\"},{\"default_features\":false,\"name\":\"crossbeam-utils\",\"req\":\"^0.8.18\"},{\"kind\":\"dev\",\"name\":\"rand\",\"req\":\"^0.8\"}],\"features\":{\"default\":[\"std\"],\"std\":[\"crossbeam-epoch/std\",\"crossbeam-utils/std\"]}}", - "crossbeam-epoch_0.9.18": "{\"dependencies\":[{\"default_features\":false,\"name\":\"crossbeam-utils\",\"req\":\"^0.8.18\"},{\"name\":\"loom-crate\",\"optional\":true,\"package\":\"loom\",\"req\":\"^0.7.1\",\"target\":\"cfg(crossbeam_loom)\"},{\"kind\":\"dev\",\"name\":\"rand\",\"req\":\"^0.8\"}],\"features\":{\"alloc\":[],\"default\":[\"std\"],\"loom\":[\"loom-crate\",\"crossbeam-utils/loom\"],\"nightly\":[\"crossbeam-utils/nightly\"],\"std\":[\"alloc\",\"crossbeam-utils/std\"]}}", + "crossbeam-epoch_0.9.20": "{\"dependencies\":[{\"default_features\":false,\"name\":\"crossbeam-utils\",\"req\":\"^0.8.18\"},{\"name\":\"loom-crate\",\"optional\":true,\"package\":\"loom\",\"req\":\"^0.7.1\",\"target\":\"cfg(crossbeam_loom)\"},{\"kind\":\"dev\",\"name\":\"rand\",\"req\":\"^0.8\"}],\"features\":{\"alloc\":[],\"default\":[\"std\"],\"loom\":[\"loom-crate\",\"crossbeam-utils/loom\"],\"nightly\":[\"crossbeam-utils/nightly\"],\"std\":[\"alloc\",\"crossbeam-utils/std\"]}}", "crossbeam-queue_0.3.12": "{\"dependencies\":[{\"default_features\":false,\"name\":\"crossbeam-utils\",\"req\":\"^0.8.18\"},{\"kind\":\"dev\",\"name\":\"rand\",\"req\":\"^0.8\"}],\"features\":{\"alloc\":[],\"default\":[\"std\"],\"nightly\":[\"crossbeam-utils/nightly\"],\"std\":[\"alloc\",\"crossbeam-utils/std\"]}}", "crossbeam-utils_0.8.21": "{\"dependencies\":[{\"name\":\"loom\",\"optional\":true,\"req\":\"^0.7.1\",\"target\":\"cfg(crossbeam_loom)\"},{\"kind\":\"dev\",\"name\":\"rand\",\"req\":\"^0.8\"}],\"features\":{\"default\":[\"std\"],\"nightly\":[],\"std\":[]}}", "crossterm_winapi_0.9.1": "{\"dependencies\":[{\"features\":[\"winbase\",\"consoleapi\",\"processenv\",\"handleapi\",\"synchapi\",\"impl-default\"],\"name\":\"winapi\",\"req\":\"^0.3.8\",\"target\":\"cfg(windows)\"}],\"features\":{}}", diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 3ba4ea8a25..33957e9fbe 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3137,6 +3137,7 @@ dependencies = [ "once_cell", "pretty_assertions", "regex", + "rustix 1.1.4", "same-file", "schemars 0.8.22", "serde", @@ -3146,6 +3147,7 @@ dependencies = [ "tokio", "ts-rs", "walkdir", + "windows-sys 0.52.0", ] [[package]] @@ -4821,9 +4823,9 @@ dependencies = [ [[package]] name = "crossbeam-epoch" -version = "0.9.18" +version = "0.9.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +checksum = "2d6914041f254d6e9176c01941b21115dcfb7089e55135a35411081bd106ef3f" dependencies = [ "crossbeam-utils", ] diff --git a/codex-rs/git-utils/Cargo.toml b/codex-rs/git-utils/Cargo.toml index 9f76e14424..ba370c5c19 100644 --- a/codex-rs/git-utils/Cargo.toml +++ b/codex-rs/git-utils/Cargo.toml @@ -33,6 +33,12 @@ ts-rs = { workspace = true, features = [ ] } walkdir = { workspace = true } +[target.'cfg(target_os = "linux")'.dependencies] +rustix = { version = "1.1.4", features = ["fs"] } + +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.52", features = ["Win32_Storage_FileSystem"] } + [dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/git-utils/src/apply.rs b/codex-rs/git-utils/src/apply.rs index d3a0787674..8726af7821 100644 --- a/codex-rs/git-utils/src/apply.rs +++ b/codex-rs/git-utils/src/apply.rs @@ -312,6 +312,116 @@ mod tests { assert!(root.join("hello.txt").exists()); } + #[test] + fn apply_resolves_relative_primary_config_from_repository_root() { + let _g = env_lock().lock().unwrap(); + if std::env::var_os("CODEX_GIT_UTILS_APPLY_ENV_CHILD").is_none() { + run_isolated_test( + "apply::tests::apply_resolves_relative_primary_config_from_repository_root", + &[("GIT_CONFIG_GLOBAL", OsStr::new("../external/config"))], + ); + return; + } + + let fixture = tempfile::tempdir().expect("fixture"); + let root = fixture.path().join("repo"); + let nested_cwd = root.join("nested"); + let external_config = fixture.path().join("external/config"); + let mismatched_nested_config = root.join("external/config"); + std::fs::create_dir_all(&nested_cwd).expect("nested cwd"); + std::fs::create_dir_all(external_config.parent().expect("config parent")) + .expect("external config directory"); + std::fs::create_dir_all( + mismatched_nested_config + .parent() + .expect("mismatched config parent"), + ) + .expect("mismatched config directory"); + std::fs::write(&external_config, "[codex]\n\tprobe = loaded\n").expect("external config"); + std::fs::write(&mismatched_nested_config, "[invalid\n") + .expect("mismatched nested-cwd config"); + let (init_code, _, init_err) = run(&root, &["git", "init"]); + assert_eq!(init_code, 0, "init repository: {init_err}"); + + let result = apply_git_patch(&ApplyGitRequest { + cwd: nested_cwd, + diff: "diff --git a/hello.txt b/hello.txt\nnew file mode 100644\n--- /dev/null\n+++ b/hello.txt\n@@ -0,0 +1 @@\n+hello\n".to_string(), + revert: false, + preflight: false, + }) + .expect("apply with root-relative primary config"); + assert_eq!(result.exit_code, 0, "apply result: {result:?}"); + assert_eq!(read_file_normalized(&root.join("hello.txt")), "hello\n"); + } + + #[cfg(target_os = "linux")] + #[test] + fn apply_refuses_process_relative_primary_config_before_mutating_the_repository() { + const TEST_NAME: &str = "apply::tests::apply_refuses_process_relative_primary_config_before_mutating_the_repository"; + if std::env::var_os("CODEX_GIT_UTILS_APPLY_ENV_CHILD").is_none() { + run_isolated_test( + TEST_NAME, + &[ + ( + "GIT_CONFIG_GLOBAL", + OsStr::new("/proc/self/cwd/codex-process-relative.gitconfig"), + ), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ], + ); + return; + } + + let repo = init_repo(); + let root = repo.path(); + std::fs::write( + root.join("codex-process-relative.gitconfig"), + "[filter \"unsafe\"]\nclean = false\n", + ) + .expect("worktree config"); + let before_index = run(root, &["git", "ls-files", "--stage", "-z"]).1; + let request = ApplyGitRequest { + cwd: root.to_path_buf(), + diff: "diff --git a/hello.txt b/hello.txt\nnew file mode 100644\n--- /dev/null\n+++ b/hello.txt\n@@ -0,0 +1 @@\n+hello\n".to_string(), + revert: false, + preflight: false, + }; + + let error = apply_git_patch(&request).expect_err("process-relative primary config"); + + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + assert!(error.to_string().contains("process-relative"), "{error}"); + assert!(!root.join("hello.txt").exists()); + assert_eq!( + run(root, &["git", "ls-files", "--stage", "-z"]).1, + before_index + ); + } + + #[test] + fn numstat_path_discovery_does_not_preempt_apply_whitespace_result() { + let _g = env_lock().lock().unwrap(); + let repo = init_repo(); + let root = repo.path(); + let (config_code, _, config_err) = + run(root, &["git", "config", "apply.whitespace", "error"]); + assert_eq!(config_code, 0, "configure whitespace policy: {config_err}"); + + let result = apply_git_patch(&ApplyGitRequest { + cwd: root.to_path_buf(), + diff: "diff --git a/trailing.txt b/trailing.txt\nnew file mode 100644\n--- /dev/null\n+++ b/trailing.txt\n@@ -0,0 +1 @@\n+trailing \n".to_string(), + revert: false, + preflight: false, + }) + .expect("path discovery must leave apply failure in the structured result"); + assert_ne!(result.exit_code, 0); + assert!( + result.stderr.contains("trailing whitespace"), + "apply result: {result:?}" + ); + assert!(!root.join("trailing.txt").exists()); + } + #[test] fn apply_uses_cwd_repo_despite_inherited_repository_selectors() { let _g = env_lock().lock().unwrap(); diff --git a/codex-rs/git-utils/src/git_command_tests.rs b/codex-rs/git-utils/src/git_command_tests.rs index 8113d2478e..ab563f94f9 100644 --- a/codex-rs/git-utils/src/git_command_tests.rs +++ b/codex-rs/git-utils/src/git_command_tests.rs @@ -436,6 +436,39 @@ fn config_source_authority_preserves_and_rejects_a_worktree_crossing_spelling() ); } +#[cfg(target_os = "linux")] +#[test] +fn config_source_authority_rejects_direct_and_aliased_procfs_symlinks() { + use std::os::unix::fs::symlink; + + let fixture = tempdir_for_native_git(); + let root = fixture.path().join("repo"); + let alias = fixture.path().join("config-alias"); + std::fs::create_dir_all(&root).expect("create repository"); + run_git(&root, &["init", "-q"]); + symlink("/proc/self/cwd", &alias).expect("create procfs alias"); + let authority = config_authority_for_root(&root); + + for path in [ + PathBuf::from("/proc/self/cwd/config"), + PathBuf::from("/proc/thread-self/cwd/config"), + PathBuf::from("/proc/self/fd/1048575/config"), + PathBuf::from("/proc/4294967295/cwd/config"), + alias.join("config"), + ] { + let error = authority + .ensure_config_source_is_not_worktree_controlled(&path, "test config") + .expect_err("process-relative config source"); + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + assert!(error.to_string().contains("process-relative"), "{error}"); + assert!(error.to_string().contains("test config"), "{error}"); + assert!( + error.to_string().contains(&path.display().to_string()), + "{error}" + ); + } +} + #[test] fn config_source_authority_allows_protected_metadata_and_unrelated_external_paths() { let fixture = tempdir_for_native_git(); diff --git a/codex-rs/git-utils/src/git_config_environment.rs b/codex-rs/git-utils/src/git_config_environment.rs index f386cfda39..1cd718c13e 100644 --- a/codex-rs/git-utils/src/git_config_environment.rs +++ b/codex-rs/git-utils/src/git_config_environment.rs @@ -61,11 +61,16 @@ impl GitConfigEnvironmentSnapshot { .collect::>(); if let Some(raw_count) = value_for(OsStr::new("GIT_CONFIG_COUNT")) { - let count = raw_count + let raw_count = raw_count .to_str() - .ok_or_else(|| invalid_environment("non-UTF-8 GIT_CONFIG_COUNT"))? - .parse::() - .map_err(|_| invalid_environment("invalid GIT_CONFIG_COUNT"))?; + .ok_or_else(|| invalid_environment("non-UTF-8 GIT_CONFIG_COUNT"))?; + let count = if raw_count.is_empty() { + 0 + } else { + raw_count + .parse::() + .map_err(|_| invalid_environment("invalid GIT_CONFIG_COUNT"))? + }; if count > MAX_CONFIG_ENVIRONMENT_ENTRIES { return Err(invalid_environment( "GIT_CONFIG_COUNT exceeds the supported safety bound", diff --git a/codex-rs/git-utils/src/git_config_environment_tests.rs b/codex-rs/git-utils/src/git_config_environment_tests.rs index 3cdec9a790..54e4cd5766 100644 --- a/codex-rs/git-utils/src/git_config_environment_tests.rs +++ b/codex-rs/git-utils/src/git_config_environment_tests.rs @@ -40,10 +40,15 @@ fn snapshot_binds_present_empty_and_absent_fixed_and_indexed_values() { snapshot.value("GIT_CONFIG_PARAMETERS"), Some(OsStr::new("'safe.parameter'='present'")) ); + assert_eq!(snapshot.value("GIT_CONFIG_COUNT"), Some(OsStr::new("2"))); assert_eq!( snapshot.value("GIT_CONFIG_KEY_0"), Some(OsStr::new("safe.one")) ); + assert_eq!( + snapshot.value("GIT_CONFIG_VALUE_0"), + Some(OsStr::new("present")) + ); assert_eq!(snapshot.value("GIT_CONFIG_VALUE_1"), None); let mut command = Command::new("git"); @@ -80,6 +85,39 @@ fn snapshot_rejects_malformed_or_unbounded_command_config_count() { } } +#[test] +fn snapshot_treats_empty_command_config_count_as_zero() { + let values = BTreeMap::from([ + (OsString::from("GIT_CONFIG_COUNT"), OsString::from("")), + ( + OsString::from("GIT_CONFIG_KEY_0"), + OsString::from("untrusted.key"), + ), + ( + OsString::from("GIT_CONFIG_VALUE_0"), + OsString::from("untrusted-value"), + ), + ]); + let snapshot = GitConfigEnvironmentSnapshot::capture_from(|name| values.get(name).cloned()) + .expect("capture empty count"); + + assert_eq!(snapshot.value("GIT_CONFIG_COUNT"), Some(OsStr::new(""))); + assert_eq!(snapshot.value("GIT_CONFIG_KEY_0"), None); + assert_eq!(snapshot.value("GIT_CONFIG_VALUE_0"), None); + + let mut command = Command::new("git"); + command.env("GIT_CONFIG_COUNT", "1"); + snapshot.apply_to(&mut command); + let child_environment = command + .get_envs() + .map(|(name, value)| (name.to_owned(), value.map(OsStr::to_owned))) + .collect::>(); + assert_eq!( + child_environment.get(OsStr::new("GIT_CONFIG_COUNT")), + Some(&Some(OsString::from(""))) + ); +} + #[cfg(unix)] #[test] fn snapshot_preserves_non_utf8_config_path_bytes() { diff --git a/codex-rs/git-utils/src/git_config_sources_tests.rs b/codex-rs/git-utils/src/git_config_sources_tests.rs index 6e4b626173..da07af9052 100644 --- a/codex-rs/git-utils/src/git_config_sources_tests.rs +++ b/codex-rs/git-utils/src/git_config_sources_tests.rs @@ -75,7 +75,28 @@ fn assert_worktree_rejection(error: io::Error) { assert!(error.to_string().contains("worktree-controlled"), "{error}"); } +#[cfg(target_os = "linux")] +fn assert_process_relative_include_rejection(error: io::Error) { + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + assert!( + error + .to_string() + .contains("process-relative Git config include"), + "{error}" + ); +} + fn run_isolated_source_test(test_name: &str, env: &[(&str, &OsStr)], removed: &[&str]) { + let cwd = std::env::current_dir().expect("current directory"); + run_isolated_source_test_from(test_name, env, removed, &cwd); +} + +fn run_isolated_source_test_from( + test_name: &str, + env: &[(&str, &OsStr)], + removed: &[&str], + cwd: &Path, +) { let mut command = std::process::Command::new(std::env::current_exe().expect("test binary")); crate::safe_git::isolate_git_command_environment(&mut command); command @@ -83,7 +104,8 @@ fn run_isolated_source_test(test_name: &str, env: &[(&str, &OsStr)], removed: &[ .arg("--exact") .arg("--nocapture") .env("CODEX_GIT_CONFIG_SOURCE_CHILD", "1") - .env("RUST_TEST_THREADS", "1"); + .env("RUST_TEST_THREADS", "1") + .current_dir(cwd); for (name, value) in env { command.env(name, value); } @@ -99,6 +121,90 @@ fn run_isolated_source_test(test_name: &str, env: &[(&str, &OsStr)], removed: &[ ); } +#[cfg(target_os = "linux")] +#[test] +fn rejects_process_relative_primary_config_sources_before_git_changes_cwd() { + use std::os::unix::fs::symlink; + + const TEST_NAME: &str = "git_config_sources::tests::rejects_process_relative_primary_config_sources_before_git_changes_cwd"; + if std::env::var_os("CODEX_GIT_CONFIG_SOURCE_CHILD").is_none() { + let fixture = tempfile::tempdir().expect("fixture"); + let root = fixture.path().join("repo"); + let external = fixture.path().join("external"); + let alias = fixture.path().join("config-alias"); + let chained_alias = fixture.path().join("chained-alias"); + init_repo_at(&root); + std::fs::create_dir(&external).expect("external directory"); + std::fs::write( + external.join("decoy.gitconfig"), + "[safe]\nvalue = external\n", + ) + .expect("external decoy"); + std::fs::write( + external.join(".gitconfig"), + "[safe]\nvalue = external-home\n", + ) + .expect("external HOME config"); + for leaf in ["decoy.gitconfig", "missing-parent.gitconfig"] { + std::fs::write(root.join(leaf), "[unsafe]\nhelper = worktree\n") + .expect("worktree config"); + } + symlink("/proc/self/cwd", &alias).expect("procfs alias"); + symlink("config-alias", &chained_alias).expect("chained procfs alias"); + + let candidates = [ + PathBuf::from("/proc/self/cwd/decoy.gitconfig"), + PathBuf::from("/proc/thread-self/cwd/decoy.gitconfig"), + PathBuf::from("/proc/self/cwd/missing-parent.gitconfig"), + alias.join("decoy.gitconfig"), + chained_alias.join("decoy.gitconfig"), + ]; + for candidate in &candidates { + run_isolated_source_test_from( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", root.as_os_str()), + ("GIT_CONFIG_GLOBAL", candidate.as_os_str()), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ], + &["GIT_CONFIG_SYSTEM"], + &external, + ); + } + run_isolated_source_test_from( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", root.as_os_str()), + ("GIT_CONFIG_GLOBAL", OsStr::new("")), + ( + "GIT_CONFIG_SYSTEM", + OsStr::new("/proc/self/cwd/decoy.gitconfig"), + ), + ], + &["GIT_CONFIG_NOSYSTEM"], + &external, + ); + run_isolated_source_test_from( + TEST_NAME, + &[ + ("CODEX_GIT_CONFIG_SOURCE_ROOT", root.as_os_str()), + ("HOME", OsStr::new("/proc/self/cwd")), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ], + &["GIT_CONFIG_GLOBAL", "GIT_CONFIG_SYSTEM", "XDG_CONFIG_HOME"], + &external, + ); + return; + } + + let root = PathBuf::from( + std::env::var_os("CODEX_GIT_CONFIG_SOURCE_ROOT").expect("fixture repository root"), + ); + let error = guard(&root).expect_err("process-relative primary config source"); + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}"); + assert!(error.to_string().contains("process-relative"), "{error}"); +} + #[test] fn allows_protected_metadata_and_external_config_sources() { let repo = init_repo(); @@ -824,6 +930,73 @@ fn command_scoped_include_paths_follow_the_same_boundary() { ); } +#[cfg(target_os = "linux")] +#[test] +fn rejects_process_relative_procfs_includes_across_the_include_graph() { + use std::os::unix::fs::symlink; + + for (key, value, leaf) in [ + ( + "include.path", + "/proc/self/cwd/direct.gitconfig", + "direct.gitconfig", + ), + ( + "includeIf.gitdir:/definitely/not/this/repository/**.path", + "/proc/thread-self/cwd/conditional.gitconfig", + "conditional.gitconfig", + ), + ] { + let repo = init_repo(); + let root = repo.path(); + std::fs::write(root.join(leaf), "[unsafe]\nhelper = worktree\n") + .expect("write process-relative include target"); + add_include(root, key, value); + assert_process_relative_include_rejection( + guard(root).expect_err("process-relative repository include"), + ); + } + + let repo = init_repo(); + let root = repo.path(); + std::fs::write( + root.join("command.gitconfig"), + "[unsafe]\nhelper = worktree\n", + ) + .expect("write command include target"); + let git = GitRunner::for_cwd_io(root).expect("Git runner"); + let args = vec![ + "-c".to_string(), + "include.path=/proc/self/cwd/command.gitconfig".to_string(), + ]; + assert_process_relative_include_rejection( + ensure_no_worktree_config_sources(&git, root, &args) + .expect_err("process-relative command-scoped include"), + ); + + let repo = init_repo(); + let root = repo.path(); + std::fs::write( + root.join("nested.gitconfig"), + "[unsafe]\nhelper = worktree\n", + ) + .expect("write nested include target"); + let external = tempfile::tempdir().expect("external include directory"); + let parent = external.path().join("parent.gitconfig"); + let alias = external.path().join("procfs-alias"); + symlink("/proc/self/cwd", &alias).expect("create procfs alias"); + std::fs::write(&parent, "[include]\npath = procfs-alias/nested.gitconfig\n") + .expect("write external parent config"); + add_include( + root, + "include.path", + parent.to_str().expect("UTF-8 parent config path"), + ); + assert_process_relative_include_rejection( + guard(root).expect_err("nested aliased process-relative include"), + ); +} + #[test] fn rejects_every_duplicate_and_nested_include_target() { for unsafe_first in [true, false] { diff --git a/codex-rs/git-utils/src/patch_paths.rs b/codex-rs/git-utils/src/patch_paths.rs index f09e027c75..213cb3e826 100644 --- a/codex-rs/git-utils/src/patch_paths.rs +++ b/codex-rs/git-utils/src/patch_paths.rs @@ -9,6 +9,10 @@ use crate::git_command::GitRunner; use crate::git_config::path_is_within; use crate::guarded_config::GuardedGitConfig; +// Path discovery must not turn a repository's whitespace policy into an +// API-level parser failure before the real apply can report its status. +const APPLY_NUMSTAT_ARGS: [&str; 3] = ["--numstat", "--whitespace=nowarn", "-z"]; + /// Extract effective patch paths through a bound operation configuration. pub(crate) fn extract_effective_paths_from_patch_guarded( config: &GuardedGitConfig<'_>, @@ -50,7 +54,7 @@ fn git_apply_numstat_paths_guarded( revert: bool, ) -> io::Result> { let mut command = config.apply_command()?; - command.args(["--numstat", "-z"]); + command.args(APPLY_NUMSTAT_ARGS); if revert { command.arg("-R"); } @@ -85,10 +89,11 @@ fn extract_paths_from_patch_from_cwd(diff_text: &str, cwd: &Path) -> Vec }; 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)?; - let config = GuardedGitConfig::authorize(&git, &git_root, Vec::new())?; + let authorized_cwd = crate::get_git_repo_root(cwd) + .map(std::fs::canonicalize) + .transpose()? + .unwrap_or(std::fs::canonicalize(cwd)?); + let config = GuardedGitConfig::authorize(&git, &authorized_cwd, Vec::new())?; extract_effective_paths_from_patch_guarded(&config, &patch_path, /*revert*/ false) })() .unwrap_or_default(); diff --git a/codex-rs/git-utils/src/patch_paths_tests.rs b/codex-rs/git-utils/src/patch_paths_tests.rs index acff91dce9..f6f9721e51 100644 --- a/codex-rs/git-utils/src/patch_paths_tests.rs +++ b/codex-rs/git-utils/src/patch_paths_tests.rs @@ -75,8 +75,8 @@ fn effective_paths(diff: &str, revert: bool) -> io::Result> { } fn best_effort_paths(diff: &str) -> Vec { - let repo = init_repo(); - extract_paths_from_patch_from_cwd(diff, repo.path()) + let cwd = tempfile::tempdir().expect("non-repository cwd"); + extract_paths_from_patch_from_cwd(diff, cwd.path()) } #[test] @@ -148,6 +148,31 @@ fn effective_paths_cover_supported_patch_headers() { ); } +#[cfg(target_os = "linux")] +#[test] +fn best_effort_parser_returns_empty_for_process_relative_primary_config() { + const TEST_NAME: &str = + "patch_paths::tests::best_effort_parser_returns_empty_for_process_relative_primary_config"; + if std::env::var_os("CODEX_GIT_UTILS_PATH_ENV_CHILD").is_none() { + run_isolated_test( + TEST_NAME, + &[ + ( + "GIT_CONFIG_GLOBAL", + OsStr::new("/proc/self/cwd/codex-process-relative.gitconfig"), + ), + ("GIT_CONFIG_NOSYSTEM", OsStr::new("1")), + ], + ); + return; + } + + assert_eq!( + extract_paths_from_patch(&new_file_diff("safe.txt")), + Vec::::new() + ); +} + #[test] fn effective_paths_follow_git_for_mismatched_headers() { let mismatch = "diff --git a/safe.txt b/safe.txt\n--- a/nested/file.txt\n+++ b/nested/file.txt\n@@ -1 +1 @@\n-old\n+new\n"; diff --git a/codex-rs/git-utils/src/path_authority.rs b/codex-rs/git-utils/src/path_authority.rs index c27f9e698c..39e9fe29db 100644 --- a/codex-rs/git-utils/src/path_authority.rs +++ b/codex-rs/git-utils/src/path_authority.rs @@ -16,6 +16,7 @@ use route_walker::RouteObservationSnapshot; #[cfg(windows)] use route_walker::invalid_data; use route_walker::observe_route; +use route_walker::route_contains_process_relative_procfs_path; #[cfg(any(windows, test))] pub(crate) use windows_path::windows_authority_path_is_ambiguous; #[cfg(any(windows, test))] @@ -64,6 +65,13 @@ impl RepositoryRouteBoundaries { &self.metadata_identities, )? != CandidateBoundary::Outside) } + + pub(crate) fn route_contains_process_relative_procfs_path( + &self, + route: &Path, + ) -> io::Result { + route_contains_process_relative_procfs_path(route) + } } pub(crate) fn repository_route_boundaries( diff --git a/codex-rs/git-utils/src/path_authority/route_walker.rs b/codex-rs/git-utils/src/path_authority/route_walker.rs index 12f14df273..b1d883f06e 100644 --- a/codex-rs/git-utils/src/path_authority/route_walker.rs +++ b/codex-rs/git-utils/src/path_authority/route_walker.rs @@ -107,6 +107,82 @@ fn symlink_route_hops(path: &Path) -> io::Result> { Ok(hops) } +/// Detect Linux procfs routes whose target can differ in a later Git process. +/// +/// Existing procfs-hosted symlinks are process-dependent, and missing procfs +/// descendants can materialize when the Git child's PID or descriptors exist. +pub(super) fn route_contains_process_relative_procfs_path(path: &Path) -> io::Result { + #[cfg(target_os = "linux")] + { + let hops = symlink_route_hops(path)?; + for hop in &hops { + if symlink_is_on_procfs(&hop.entry)? { + return Ok(true); + } + } + if missing_descendant_is_below_procfs(path)? { + return Ok(true); + } + for hop in hops { + if missing_descendant_is_below_procfs(&hop.target)? + || missing_descendant_is_below_procfs(&hop.projected)? + { + return Ok(true); + } + } + } + #[cfg(not(target_os = "linux"))] + let _ = path; + Ok(false) +} + +#[cfg(target_os = "linux")] +fn symlink_is_on_procfs(path: &Path) -> io::Result { + filesystem_is_procfs( + path, + rustix::fs::OFlags::PATH | rustix::fs::OFlags::NOFOLLOW | rustix::fs::OFlags::CLOEXEC, + ) +} + +#[cfg(target_os = "linux")] +fn missing_descendant_is_below_procfs(path: &Path) -> io::Result { + match std::fs::symlink_metadata(path) { + Ok(_) => return Ok(false), + Err(error) if is_missing_path_error(&error) => {} + Err(error) => return Err(error), + } + for ancestor in path.ancestors().skip(1) { + match std::fs::symlink_metadata(ancestor) { + Ok(_) => match filesystem_is_procfs( + ancestor, + rustix::fs::OFlags::PATH | rustix::fs::OFlags::CLOEXEC, + ) { + Ok(is_procfs) => return Ok(is_procfs), + Err(error) if is_missing_path_error(&error) => {} + Err(error) => return Err(error), + }, + Err(error) if is_missing_path_error(&error) => {} + Err(error) => return Err(error), + } + } + Err(invalid_data("authority path has no existing ancestor")) +} + +#[cfg(target_os = "linux")] +fn filesystem_is_procfs(path: &Path, flags: rustix::fs::OFlags) -> io::Result { + let fd = rustix::fs::open(path, flags, rustix::fs::Mode::empty()).map_err(io::Error::from)?; + let filesystem = rustix::fs::fstatfs(&fd).map_err(io::Error::from)?; + Ok(filesystem.f_type == rustix::fs::PROC_SUPER_MAGIC) +} + +#[cfg(target_os = "linux")] +fn is_missing_path_error(error: &io::Error) -> bool { + matches!( + error.kind(), + io::ErrorKind::NotFound | io::ErrorKind::NotADirectory + ) +} + fn collect_symlink_route_hops( path: &Path, depth: usize, diff --git a/codex-rs/git-utils/src/path_authority/route_walker_tests.rs b/codex-rs/git-utils/src/path_authority/route_walker_tests.rs index 1b7604d93a..6ff3e68b18 100644 --- a/codex-rs/git-utils/src/path_authority/route_walker_tests.rs +++ b/codex-rs/git-utils/src/path_authority/route_walker_tests.rs @@ -1,6 +1,12 @@ use pretty_assertions::assert_eq; +#[cfg(target_os = "linux")] +use std::path::Path; +#[cfg(target_os = "linux")] +use std::path::PathBuf; use super::read_link_if_symlink; +#[cfg(target_os = "linux")] +use super::route_contains_process_relative_procfs_path; #[test] fn ordinary_paths_are_not_treated_as_failed_symlink_reads() { @@ -16,3 +22,59 @@ fn ordinary_paths_are_not_treated_as_failed_symlink_reads() { ); assert_eq!(read_link_if_symlink(&file).expect("ordinary file"), None); } + +#[cfg(target_os = "linux")] +#[test] +fn process_relative_procfs_detection_follows_aliases_without_rejecting_ordinary_paths() { + use std::os::unix::fs::symlink; + + let fixture = tempfile::tempdir().expect("fixture"); + let external = fixture.path().join("external"); + let ordinary_alias = fixture.path().join("ordinary-alias"); + let procfs_alias = fixture.path().join("procfs-alias"); + let procfs_root_alias = fixture.path().join("procfs-root-alias"); + let chained_alias = fixture.path().join("chained-alias"); + let literal = fixture.path().join("proc/self/cwd/config"); + std::fs::create_dir_all(&external).expect("external directory"); + std::fs::create_dir_all(literal.parent().expect("literal parent")) + .expect("literal directories"); + std::fs::write(&literal, "[safe]\n").expect("literal config"); + symlink(&external, &ordinary_alias).expect("ordinary alias"); + symlink("/proc/self/cwd", &procfs_alias).expect("procfs alias"); + symlink("/proc", &procfs_root_alias).expect("procfs root alias"); + symlink("procfs-alias", &chained_alias).expect("chained alias"); + + let mut procfs_paths = vec![ + PathBuf::from("/proc/self/cwd/missing-config"), + PathBuf::from("/proc/thread-self/cwd/missing-config"), + PathBuf::from("/proc/self/fd/1048575/missing-config"), + PathBuf::from("/proc/self/exe"), + PathBuf::from(format!("/proc/{}/cwd/missing-config", std::process::id())), + PathBuf::from("/proc/4294967295/cwd/missing-config"), + procfs_alias.join("missing-config"), + procfs_root_alias.join("4294967295/cwd/missing-config"), + chained_alias.join("missing-config"), + ]; + if Path::new("/dev/fd").exists() { + procfs_paths.push(PathBuf::from("/dev/fd/1048575/missing-config")); + } + for path in procfs_paths { + assert!( + route_contains_process_relative_procfs_path(&path).expect("inspect procfs route"), + "expected process-relative procfs route in {}", + path.display() + ); + } + + for path in [ + PathBuf::from("/proc/version"), + ordinary_alias.join("missing-config"), + literal, + ] { + assert!( + !route_contains_process_relative_procfs_path(&path).expect("inspect ordinary route"), + "unexpected process-relative procfs route in {}", + path.display() + ); + } +} diff --git a/codex-rs/git-utils/src/repository_authority/authority.rs b/codex-rs/git-utils/src/repository_authority/authority.rs index db55a32a9e..836b6e040a 100644 --- a/codex-rs/git-utils/src/repository_authority/authority.rs +++ b/codex-rs/git-utils/src/repository_authority/authority.rs @@ -26,6 +26,31 @@ use super::resolve_repository_metadata; mod policy; +fn open_active_worktree_identity(path: &Path) -> io::Result { + #[cfg(windows)] + { + use std::fs::OpenOptions; + use std::os::windows::fs::OpenOptionsExt; + use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS; + use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; + use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; + + // Deliberately omit FILE_SHARE_DELETE so the repository root cannot + // be renamed or replaced between validation and the guarded child. + let directory = OpenOptions::new() + .read(true) + .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE) + .custom_flags(FILE_FLAG_BACKUP_SEMANTICS) + .open(path)?; + Handle::from_file(directory) + } + + #[cfg(not(windows))] + { + Handle::from_path(path) + } +} + /// Repository and filesystem authority retained for the lifetime of one /// trusted Git runner. #[derive(Debug)] @@ -60,7 +85,7 @@ impl RepositoryAuthority { let worktree_root = crate::get_git_repo_root(&canonical_cwd) .and_then(|root| std::fs::canonicalize(root).ok()) .unwrap_or_else(|| canonical_cwd.clone()); - let active_worktree_identity = Handle::from_path(&worktree_root) + let active_worktree_identity = open_active_worktree_identity(&worktree_root) .map_err(|error| invalid_metadata(&worktree_root, error))?; let mut authority = Self { active_worktree_root: worktree_root.clone(), @@ -208,7 +233,7 @@ impl RepositoryAuthority { .or_else(|| roots.first()) .cloned() .unwrap_or_default(); - let active_worktree_identity = Handle::from_path(&active_worktree_root).ok(); + let active_worktree_identity = open_active_worktree_identity(&active_worktree_root).ok(); Ok(Self { active_worktree_root, active_worktree_identity, 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 e5f2379460..b8c43f18f7 100644 --- a/codex-rs/git-utils/src/repository_authority/authority/policy.rs +++ b/codex-rs/git-utils/src/repository_authority/authority/policy.rs @@ -41,6 +41,12 @@ impl RepositoryAuthority { if !path.is_absolute() { return Err(worktree_controlled_config_source(path, description)); } + if self + .route_boundaries + .route_contains_process_relative_procfs_path(path)? + { + return Err(process_relative_config_source(path, description)); + } let inspection = self.route_boundaries.inspect_route(path)?; if inspection.crosses_worktree { return Err(worktree_controlled_config_source(path, description)); @@ -178,3 +184,13 @@ fn worktree_controlled_config_source(path: &Path, description: &str) -> io::Erro ), ) } + +fn process_relative_config_source(path: &Path, description: &str) -> io::Error { + io::Error::new( + io::ErrorKind::PermissionDenied, + format!( + "refusing to use process-relative {description}: {}", + path.display() + ), + ) +}