mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
fix(git): reject process-relative procfs config
This commit is contained in:
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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::<String>::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";
|
||||
|
||||
@@ -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<bool> {
|
||||
route_contains_process_relative_procfs_path(route)
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn repository_route_boundaries(
|
||||
|
||||
@@ -107,6 +107,82 @@ fn symlink_route_hops(path: &Path) -> io::Result<Vec<SymlinkRouteHop>> {
|
||||
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<bool> {
|
||||
#[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<bool> {
|
||||
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<bool> {
|
||||
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<bool> {
|
||||
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,
|
||||
|
||||
@@ -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()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user