mirror of
https://github.com/openai/codex.git
synced 2026-09-04 15:08:45 +00:00
git-utils: block selected filters before staging
This commit is contained in:
Chris Bookholt
@@ -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()? {
|
||||
|
||||
@@ -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}");
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user