Merge branch 'codex/psec-4394-git-03-path-containment' into codex/psec-4394-git-05a-apply-filters

# Conflicts:
#	codex-rs/git-utils/src/apply.rs
This commit is contained in:
Chris Bookholt
2026-07-01 15:25:38 -07:00
5 changed files with 410 additions and 84 deletions

View File

@@ -375,6 +375,13 @@ mod tests {
code == 0
}
#[cfg(unix)]
fn trusted_git_directory() -> PathBuf {
std::env::split_paths(&std::env::var_os("PATH").expect("PATH"))
.find(|directory| directory.is_absolute() && directory.join("git").is_file())
.expect("trusted Git directory")
}
#[test]
fn parse_output_unescapes_quoted_paths() {
let stderr = "error: patch failed: \"hello\\tworld.txt\":1\n";
@@ -457,6 +464,77 @@ mod tests {
assert_eq!(read_file_normalized(&root.join("file.txt")), "new\n");
}
#[cfg(unix)]
#[test]
fn apply_uses_logical_process_cwd_to_reject_enclosing_git() {
use std::os::unix::fs::PermissionsExt;
let _g = env_lock().lock().unwrap();
if std::env::var_os("CODEX_GIT_UTILS_APPLY_LOGICAL_CWD_CHILD").is_none() {
let fixture = tempfile::tempdir().expect("fixture");
let outer = fixture.path().join("outer");
let physical_nested = fixture.path().join("physical-nested");
let lexical_nested = outer.join("nested");
let outer_bin = outer.join("bin");
let outer_git = outer_bin.join("git");
let marker = outer_bin.join("git.ran");
std::fs::create_dir_all(&outer_bin).expect("outer Git directory");
std::fs::create_dir_all(&physical_nested).expect("physical nested repository");
let (outer_init, _, outer_err) = run(&outer, &["git", "init", "-q"]);
assert_eq!(outer_init, 0, "init outer repository: {outer_err}");
let (nested_init, _, nested_err) = run(&physical_nested, &["git", "init", "-q"]);
assert_eq!(nested_init, 0, "init nested repository: {nested_err}");
std::os::unix::fs::symlink(&physical_nested, &lexical_nested)
.expect("symlink nested repository");
std::fs::write(&outer_git, "#!/bin/sh\nprintf ran >\"$0.ran\"\nexit 1\n")
.expect("outer Git shim");
let mut permissions = std::fs::metadata(&outer_git)
.expect("outer Git metadata")
.permissions();
permissions.set_mode(0o755);
std::fs::set_permissions(&outer_git, permissions).expect("executable outer Git");
let path = std::env::join_paths([outer_bin, trusted_git_directory()]).expect("PATH");
let mut command =
std::process::Command::new(std::env::current_exe().expect("test binary"));
isolate_git_command_environment(&mut command);
let output = command
.arg("apply::tests::apply_uses_logical_process_cwd_to_reject_enclosing_git")
.arg("--exact")
.arg("--nocapture")
.current_dir(&lexical_nested)
.env("CODEX_GIT_UTILS_APPLY_LOGICAL_CWD_CHILD", "1")
.env("CODEX_GIT_UTILS_APPLY_LOGICAL_CWD_MARKER", &marker)
.env("PWD", &lexical_nested)
.env("PATH", path)
.env("RUST_TEST_THREADS", "1")
.output()
.expect("run isolated logical-cwd test");
assert!(
output.status.success(),
"isolated logical-cwd test failed:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(!marker.exists(), "enclosing Git shim must not run");
return;
}
let cwd = std::env::current_dir().expect("physical process cwd");
let marker = PathBuf::from(
std::env::var_os("CODEX_GIT_UTILS_APPLY_LOGICAL_CWD_MARKER").expect("marker path"),
);
let result = apply_git_patch(&ApplyGitRequest {
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: true,
})
.expect("preflight through trusted Git");
assert_eq!(result.exit_code, 0, "preflight should succeed");
assert!(!marker.exists(), "enclosing Git shim must not run");
}
#[test]
fn apply_modify_conflict() {
let _g = env_lock().lock().unwrap();

View File

@@ -16,7 +16,7 @@ pub(crate) struct GitRunner {
struct UntrustedGitLocations {
roots: Vec<PathBuf>,
common_dir: Option<PathBuf>,
common_dirs: Vec<PathBuf>,
}
impl GitRunner {
@@ -76,25 +76,110 @@ impl GitRunner {
}
fn untrusted_git_locations_for_cwd(cwd: &Path) -> Result<UntrustedGitLocations, GitReadError> {
let lexical_cwd = if cwd.is_absolute() {
cwd.to_path_buf()
} else {
std::env::current_dir()
.map_err(|_| GitReadError::NotRepository {
path: cwd.to_path_buf(),
})?
.join(cwd)
};
let canonical_cwd = std::fs::canonicalize(cwd).map_err(|_| GitReadError::NotRepository {
path: cwd.to_path_buf(),
})?;
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 mut roots = vec![worktree_root.clone()];
let mut locations = UntrustedGitLocations {
roots: Vec::new(),
common_dirs: Vec::new(),
};
record_repository_ancestry(&worktree_root, &mut locations)?;
// Canonicalization can erase a repository-controlled symlink prefix. Walk
// the requested spelling too, deliberately retaining symlink and `..`
// components so every lexical enclosing checkout remains untrusted.
let lexical_base = if lexical_cwd.is_dir() {
lexical_cwd
} else {
lexical_cwd
.parent()
.ok_or_else(|| GitReadError::NotRepository {
path: cwd.to_path_buf(),
})?
.to_path_buf()
};
record_repository_ancestry(&lexical_base, &mut locations)?;
// Callers commonly obtain their default cwd from `current_dir()`, which
// can already have erased a symlink spelling. Recover the standard logical
// process cwd only when it is absolute and resolves to both the requested
// cwd and the process cwd. Treating extra roots as untrusted cannot widen
// executable selection.
if let Some(logical_cwd) = validated_logical_process_cwd(&canonical_cwd) {
record_repository_ancestry(&logical_cwd, &mut locations)?;
}
Ok(locations)
}
fn validated_logical_process_cwd(canonical_cwd: &Path) -> Option<PathBuf> {
let process_cwd = std::fs::canonicalize(std::env::current_dir().ok()?).ok()?;
if !paths_equal(&process_cwd, canonical_cwd) {
return None;
}
let logical_cwd = PathBuf::from(std::env::var_os("PWD")?);
if !logical_cwd.is_absolute() {
return None;
}
let canonical_logical_cwd = std::fs::canonicalize(&logical_cwd).ok()?;
paths_equal(&canonical_logical_cwd, canonical_cwd).then_some(logical_cwd)
}
fn record_repository_ancestry(
start: &Path,
locations: &mut UntrustedGitLocations,
) -> Result<(), GitReadError> {
push_unique(&mut locations.roots, start.to_path_buf());
record_repository_marker(start, locations)?;
for ancestor in start.parent().into_iter().flat_map(Path::ancestors) {
let dot_git = ancestor.join(".git");
match std::fs::symlink_metadata(&dot_git) {
Ok(_) => {
push_unique(&mut locations.roots, ancestor.to_path_buf());
let canonical_root =
std::fs::canonicalize(ancestor).map_err(|_| GitReadError::NoTrustedGit)?;
push_unique(&mut locations.roots, canonical_root.clone());
record_repository_marker(ancestor, locations)?;
}
Err(error) if error.kind() == io::ErrorKind::NotFound => {}
Err(_) => return Err(GitReadError::NoTrustedGit),
}
}
Ok(())
}
fn record_repository_marker(
worktree_root: &Path,
locations: &mut UntrustedGitLocations,
) -> Result<(), GitReadError> {
let dot_git = worktree_root.join(".git");
let common_dir = match std::fs::symlink_metadata(&dot_git) {
Ok(_) => Some(resolve_common_git_dir(&dot_git).map_err(|()| GitReadError::NoTrustedGit)?),
Err(error) if error.kind() == io::ErrorKind::NotFound => None,
Ok(_) => resolve_common_git_dir(&dot_git).map_err(|()| GitReadError::NoTrustedGit)?,
Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()),
Err(_) => return Err(GitReadError::NoTrustedGit),
};
if let Some(common_dir) = &common_dir
&& !path_is_within(common_dir, &worktree_root)
{
roots.push(common_dir.clone());
if !path_is_within(&common_dir, worktree_root) {
push_unique(&mut locations.roots, common_dir.clone());
}
push_unique(&mut locations.common_dirs, common_dir);
Ok(())
}
fn push_unique(paths: &mut Vec<PathBuf>, path: PathBuf) {
if !paths.iter().any(|existing| paths_equal(existing, &path)) {
paths.push(path);
}
Ok(UntrustedGitLocations { roots, common_dir })
}
fn path_is_untrusted(path: &Path, locations: &UntrustedGitLocations) -> bool {
@@ -106,9 +191,9 @@ fn path_is_untrusted(path: &Path, locations: &UntrustedGitLocations) -> bool {
return true;
}
locations
.common_dir
.as_deref()
.is_some_and(|common_dir| path_is_in_worktree_for_common_dir(path, common_dir))
.common_dirs
.iter()
.any(|common_dir| path_is_in_worktree_for_common_dir(path, common_dir))
}
fn path_is_in_worktree_for_common_dir(path: &Path, expected_common_dir: &Path) -> bool {

View File

@@ -2,6 +2,8 @@ use super::*;
use pretty_assertions::assert_eq;
use std::process::Command;
use crate::safe_git::DISABLED_HOOKS_PATH;
#[cfg(unix)]
fn write_executable(path: &Path, body: &str) {
use std::os::unix::fs::PermissionsExt;
@@ -18,6 +20,12 @@ fn run_git(cwd: &Path, args: &[&str]) {
let mut command = Command::new("git");
isolate_git_command_environment(&mut command);
let status = command
.args([
"-c",
&format!("core.hooksPath={DISABLED_HOOKS_PATH}"),
"-c",
"core.fsmonitor=false",
])
.args(args)
.current_dir(cwd)
.status()
@@ -25,6 +33,23 @@ fn run_git(cwd: &Path, args: &[&str]) {
assert!(status.success(), "git {args:?} failed");
}
fn commit_all(cwd: &Path, message: &str) {
run_git(
cwd,
&[
"-c",
"user.name=Codex Test",
"-c",
"user.email=codex@example.com",
"-c",
"commit.gpgSign=false",
"commit",
"-qam",
message,
],
);
}
fn write_git_candidate(directory: &Path) {
std::fs::create_dir_all(directory).expect("create candidate directory");
let candidate = directory.join(git_executable_name());
@@ -39,7 +64,7 @@ fn write_git_candidate(directory: &Path) {
fn locations_for_root(root: &Path) -> UntrustedGitLocations {
UntrustedGitLocations {
roots: vec![std::fs::canonicalize(root).expect("canonical root")],
common_dir: None,
common_dirs: Vec::new(),
}
}
@@ -107,6 +132,130 @@ fn linked_worktree_rejects_git_from_main_and_linked_worktrees() {
));
}
#[test]
fn nested_repository_rejects_git_from_enclosing_repository() {
let fixture = tempfile::tempdir().expect("fixture");
let outer = fixture.path().join("outer");
let nested = outer.join("nested");
let outer_bin = outer.join("bin");
let trusted_bin = fixture.path().join("trusted-bin");
std::fs::create_dir_all(&nested).expect("create nested repository");
run_git(&outer, &["init", "-q"]);
run_git(&nested, &["init", "-q"]);
write_git_candidate(&outer_bin);
write_git_candidate(&trusted_bin);
let locations = untrusted_git_locations_for_cwd(&nested).expect("untrusted locations");
assert!(
path_is_untrusted(&outer_bin.join(git_executable_name()), &locations),
"Git from an enclosing repository must remain repository-controlled"
);
assert_eq!(
selected_git(&locations, &[&outer_bin, &trusted_bin]),
trusted_bin.join(git_executable_name())
);
}
#[cfg(unix)]
#[test]
fn symlinked_nested_repository_rejects_git_from_lexical_enclosing_repository() {
let fixture = tempfile::tempdir().expect("fixture");
let outer = fixture.path().join("outer");
let physical_nested = fixture.path().join("physical-nested");
let lexical_nested = outer.join("nested");
let outer_bin = outer.join("bin");
let trusted_bin = fixture.path().join("trusted-bin");
std::fs::create_dir_all(&outer).expect("create outer repository");
std::fs::create_dir_all(&physical_nested).expect("create physical nested repository");
run_git(&outer, &["init", "-q"]);
run_git(&physical_nested, &["init", "-q"]);
std::os::unix::fs::symlink(&physical_nested, &lexical_nested)
.expect("symlink nested repository");
write_git_candidate(&outer_bin);
write_git_candidate(&trusted_bin);
let locations = untrusted_git_locations_for_cwd(&lexical_nested).expect("untrusted locations");
assert!(
path_is_untrusted(&outer_bin.join(git_executable_name()), &locations),
"Git from the lexical enclosing repository must remain repository-controlled"
);
assert_eq!(
selected_git(&locations, &[&outer_bin, &trusted_bin]),
trusted_bin.join(git_executable_name())
);
}
#[test]
fn nested_repository_rejects_git_from_enclosing_repository_main_worktree() {
let fixture = tempfile::tempdir().expect("fixture");
let main = fixture.path().join("main");
let linked = fixture.path().join("linked");
let nested = linked.join("nested");
let main_bin = main.join("bin");
let trusted_bin = fixture.path().join("trusted-bin");
std::fs::create_dir_all(&main).expect("create main worktree");
run_git(&main, &["init", "-q"]);
run_git(&main, &["worktree", "add", "--orphan", path_text(&linked)]);
std::fs::create_dir_all(&nested).expect("create nested repository");
run_git(&nested, &["init", "-q"]);
write_git_candidate(&main_bin);
write_git_candidate(&trusted_bin);
let locations = untrusted_git_locations_for_cwd(&nested).expect("untrusted locations");
assert!(
path_is_untrusted(&main_bin.join(git_executable_name()), &locations),
"all worktrees of an enclosing repository must remain repository-controlled"
);
assert_eq!(
selected_git(&locations, &[&main_bin, &trusted_bin]),
trusted_bin.join(git_executable_name())
);
}
#[test]
fn submodule_rejects_git_from_enclosing_superproject() {
let fixture = tempfile::tempdir().expect("fixture");
let source = fixture.path().join("source");
let outer = fixture.path().join("outer");
let submodule = outer.join("nested");
let outer_bin = outer.join("bin");
let trusted_bin = fixture.path().join("trusted-bin");
std::fs::create_dir_all(&source).expect("create source repository");
std::fs::create_dir_all(&outer).expect("create superproject");
run_git(&source, &["init", "-q"]);
std::fs::write(source.join("source.txt"), "source\n").expect("write source file");
run_git(&source, &["add", "source.txt"]);
commit_all(&source, "source");
run_git(&outer, &["init", "-q"]);
std::fs::write(outer.join("outer.txt"), "outer\n").expect("write outer file");
run_git(&outer, &["add", "outer.txt"]);
commit_all(&outer, "outer");
run_git(
&outer,
&[
"-c",
"protocol.file.allow=always",
"submodule",
"add",
"-q",
path_text(&source),
"nested",
],
);
write_git_candidate(&outer_bin);
write_git_candidate(&trusted_bin);
let locations = untrusted_git_locations_for_cwd(&submodule).expect("untrusted locations");
assert!(
path_is_untrusted(&outer_bin.join(git_executable_name()), &locations),
"Git from a superproject must remain repository-controlled"
);
assert_eq!(
selected_git(&locations, &[&outer_bin, &trusted_bin]),
trusted_bin.join(git_executable_name())
);
}
#[test]
fn bare_backed_linked_worktree_allows_external_git_in_sibling_directory() {
let fixture = tempfile::tempdir().expect("fixture");
@@ -171,6 +320,23 @@ fn separate_dot_git_dir_rejects_main_candidate_and_allows_unrelated_repo_candida
);
}
#[test]
fn resolver_rejects_parent_traversal_spelled_through_repository() {
let fixture = tempfile::tempdir().expect("fixture");
let repo = fixture.path().join("repo");
let trusted_bin = fixture.path().join("trusted-bin");
std::fs::create_dir_all(&repo).expect("create repository");
write_git_candidate(&trusted_bin);
let locations = locations_for_root(&repo);
let traversing_path = locations.roots[0].join("../trusted-bin");
let search_path = std::env::join_paths([traversing_path]).expect("PATH");
assert!(matches!(
GitRunner::from_search_path(&locations, &search_path),
Err(GitReadError::NoTrustedGit)
));
}
#[cfg(windows)]
#[test]
fn resolver_selects_native_git_exe_only() {

View File

@@ -2,14 +2,10 @@ use std::ffi::OsStr;
use std::ffi::OsString;
use std::path::Path;
use std::path::PathBuf;
#[cfg(test)]
use std::process::Command;
use crate::GitToolingError;
use crate::git_command::GitRunner;
use crate::safe_git::DISABLED_HOOKS_PATH;
#[cfg(test)]
use crate::safe_git::isolate_git_command_environment;
pub(crate) fn ensure_git_repository(path: &Path) -> Result<(), GitToolingError> {
match run_git_for_stdout(
@@ -155,70 +151,5 @@ struct GitRun {
}
#[cfg(test)]
mod tests {
use super::*;
fn init_repo() -> tempfile::TempDir {
let repo = tempfile::tempdir().expect("tempdir");
let mut command = Command::new("git");
isolate_git_command_environment(&mut command);
let status = command
.args(["init", "-q"])
.current_dir(repo.path())
.status()
.expect("initialize repository");
assert!(status.success());
repo
}
#[test]
fn caller_env_cannot_restore_repository_or_pathspec_selectors() {
let target = init_repo();
let alternate = init_repo();
std::fs::write(target.path().join("target.txt"), "target\n").expect("target file");
std::fs::write(alternate.path().join("alternate.txt"), "alternate\n")
.expect("alternate file");
for (repo, path) in [(&target, "target.txt"), (&alternate, "alternate.txt")] {
let mut command = Command::new("git");
isolate_git_command_environment(&mut command);
let status = command
.args(["add", path])
.current_dir(repo.path())
.status()
.expect("add file");
assert!(status.success());
}
let alternate_git_dir = alternate.path().join(".git");
let env = [
(
OsString::from("GIT_DIR"),
alternate_git_dir.as_os_str().into(),
),
(
OsString::from("GIT_WORK_TREE"),
alternate.path().as_os_str().into(),
),
(
OsString::from("GIT_COMMON_DIR"),
alternate_git_dir.as_os_str().into(),
),
(
OsString::from("GIT_INDEX_FILE"),
alternate_git_dir.join("index").into_os_string(),
),
(OsString::from("GIT_PREFIX"), OsString::from("elsewhere/")),
(OsString::from("GIT_LITERAL_PATHSPECS"), OsString::from("1")),
(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");
assert_eq!(output, "target.txt");
}
}
#[path = "operations_tests.rs"]
mod tests;

View File

@@ -0,0 +1,66 @@
use super::*;
use crate::safe_git::isolate_git_command_environment;
use std::process::Command;
fn init_repo() -> tempfile::TempDir {
let repo = tempfile::tempdir().expect("tempdir");
let mut command = Command::new("git");
isolate_git_command_environment(&mut command);
let status = command
.args(["init", "-q"])
.current_dir(repo.path())
.status()
.expect("initialize repository");
assert!(status.success());
repo
}
#[test]
fn caller_env_cannot_restore_repository_or_pathspec_selectors() {
let target = init_repo();
let alternate = init_repo();
std::fs::write(target.path().join("target.txt"), "target\n").expect("target file");
std::fs::write(alternate.path().join("alternate.txt"), "alternate\n").expect("alternate file");
for (repo, path) in [(&target, "target.txt"), (&alternate, "alternate.txt")] {
let mut command = Command::new("git");
isolate_git_command_environment(&mut command);
let status = command
.args(["add", path])
.current_dir(repo.path())
.status()
.expect("add file");
assert!(status.success());
}
let alternate_git_dir = alternate.path().join(".git");
let env = [
(
OsString::from("GIT_DIR"),
alternate_git_dir.as_os_str().into(),
),
(
OsString::from("GIT_WORK_TREE"),
alternate.path().as_os_str().into(),
),
(
OsString::from("GIT_COMMON_DIR"),
alternate_git_dir.as_os_str().into(),
),
(
OsString::from("GIT_INDEX_FILE"),
alternate_git_dir.join("index").into_os_string(),
),
(OsString::from("GIT_PREFIX"), OsString::from("elsewhere/")),
(OsString::from("GIT_LITERAL_PATHSPECS"), OsString::from("1")),
(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");
assert_eq!(output, "target.txt");
}