From c56bd60054e19335ae0b7442278cc84e00f98971 Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Mon, 6 Jul 2026 15:27:36 -0700 Subject: [PATCH] fix(git): reject process-relative procfs config --- codex-rs/Cargo.lock | 1 + codex-rs/git-utils/Cargo.toml | 3 + codex-rs/git-utils/src/apply.rs | 44 +++++++++ codex-rs/git-utils/src/git_command_tests.rs | 33 +++++++ .../git-utils/src/git_config_sources_tests.rs | 97 ++++++++++++++++++- codex-rs/git-utils/src/patch_paths_tests.rs | 25 +++++ codex-rs/git-utils/src/path_authority.rs | 8 ++ .../src/path_authority/route_walker.rs | 76 +++++++++++++++ .../src/path_authority/route_walker_tests.rs | 62 ++++++++++++ .../repository_authority/authority/policy.rs | 16 +++ 10 files changed, 364 insertions(+), 1 deletion(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 3ba4ea8a25..aa27d32491 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", diff --git a/codex-rs/git-utils/Cargo.toml b/codex-rs/git-utils/Cargo.toml index 9f76e14424..9f35494902 100644 --- a/codex-rs/git-utils/Cargo.toml +++ b/codex-rs/git-utils/Cargo.toml @@ -33,6 +33,9 @@ ts-rs = { workspace = true, features = [ ] } walkdir = { workspace = true } +[target.'cfg(target_os = "linux")'.dependencies] +rustix = { version = "1.1.4", features = ["fs"] } + [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 8526a62682..5d389ee3aa 100644 --- a/codex-rs/git-utils/src/apply.rs +++ b/codex-rs/git-utils/src/apply.rs @@ -401,6 +401,50 @@ mod tests { 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(); diff --git a/codex-rs/git-utils/src/git_command_tests.rs b/codex-rs/git-utils/src/git_command_tests.rs index 3d191735b3..8d78a002d3 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_sources_tests.rs b/codex-rs/git-utils/src/git_config_sources_tests.rs index 5a0569d424..2cd7d91ed8 100644 --- a/codex-rs/git-utils/src/git_config_sources_tests.rs +++ b/codex-rs/git-utils/src/git_config_sources_tests.rs @@ -73,6 +73,16 @@ fn assert_worktree_rejection(error: io::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 @@ -80,7 +90,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); } @@ -96,6 +107,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 rejects_empty_primary_global_and_system_sources_in_the_worktree() { const TEST_NAME: &str = "git_config_sources::tests::rejects_empty_primary_global_and_system_sources_in_the_worktree"; diff --git a/codex-rs/git-utils/src/patch_paths_tests.rs b/codex-rs/git-utils/src/patch_paths_tests.rs index bf2396b22a..66b2319f78 100644 --- a/codex-rs/git-utils/src/patch_paths_tests.rs +++ b/codex-rs/git-utils/src/patch_paths_tests.rs @@ -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 1e2157c378..aa843838d8 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(windows)] @@ -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/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() + ), + ) +}