git-utils: neutralize off-path filter helpers

This commit is contained in:
Chris Bookholt
2026-07-01 15:58:52 -07:00
parent 4bf391509e
commit bed78fad06
5 changed files with 395 additions and 12 deletions
+4 -3
View File
@@ -55,7 +55,10 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
// Keep tmpdir alive until function end to ensure the file exists
let _guard = tmpdir;
let patch_paths = extract_effective_paths_from_patch(&git, &patch_path, req.revert)?;
ensure_no_selected_executable_git_filters(&git, &git_root, &patch_paths, &cfg_parts)?;
let filter_guard =
ensure_no_selected_executable_git_filters(&git, &git_root, &patch_paths, &cfg_parts)?;
cfg_parts.extend(safe_git_config_parts());
cfg_parts.extend_from_slice(filter_guard.git_config_args());
if req.revert && !req.preflight {
// Stage WT paths first to avoid index mismatch on revert.
@@ -68,8 +71,6 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
args.push("-R".into());
}
cfg_parts.extend(safe_git_config_parts());
args.push(patch_path.to_string_lossy().to_string());
// Optional preflight: dry-run only; do not modify working tree
+253 -1
View File
@@ -1,11 +1,24 @@
use super::*;
use pretty_assertions::assert_eq;
use std::ffi::OsStr;
use std::fs::File;
use std::fs::FileTimes;
use std::path::Path;
use std::time::UNIX_EPOCH;
const PATCH: &str =
"diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-orig\n+next\n";
const FILTER_COMMAND: &str = "git config codex.filterran true && git hash-object --stdin";
const PROCESS_FILTER_COMMAND: &str =
"git config codex.filterran true && git rev-parse --verify refs/codex-filter-must-not-run";
const COMMAND_SCOPED_FILTER_CONFIG: &str =
"filter.demo.clean=git config codex.filterran true && git hash-object --stdin";
#[derive(Clone, Copy, Debug)]
enum PatchKind {
Direct,
ThreeWay,
}
fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) {
let mut command = std::process::Command::new("git");
@@ -40,9 +53,116 @@ fn init_repo() -> tempfile::TempDir {
}
fn configure_filter(root: &Path, driver: &str) {
configure_filter_command(root, driver, "clean", FILTER_COMMAND);
}
fn configure_filter_command(root: &Path, driver: &str, name: &str, command: &str) {
run_success(
root,
&["config", &format!("filter.{driver}.clean"), FILTER_COMMAND],
&["config", &format!("filter.{driver}.{name}"), command],
);
}
fn make_index_racy(root: &Path) {
let index = File::options()
.write(true)
.open(root.join(".git/index"))
.expect("open index");
index
.set_times(FileTimes::new().set_modified(UNIX_EPOCH))
.expect("make index entries racy");
}
fn build_patch(root: &Path, kind: PatchKind, paths: &[&str]) -> String {
if matches!(kind, PatchKind::Direct) && paths == ["file.txt"] {
return PATCH.to_string();
}
std::fs::write(root.join("file.txt"), "next\n").expect("write target postimage");
let mut args = vec!["diff"];
if matches!(kind, PatchKind::ThreeWay) {
args.extend(["--full-index", "--binary"]);
}
args.push("--");
args.extend_from_slice(paths);
let (code, stdout, stderr) = run(root, &args);
assert_eq!(code, 0, "generate patch: {stderr}");
let mut checkout = vec!["checkout", "-q", "--"];
checkout.extend_from_slice(paths);
run_success(root, &checkout);
stdout
}
fn index_contents(root: &Path, path: &str) -> String {
let (code, stdout, stderr) = run(root, &["show", &format!(":{path}")]);
assert_eq!(code, 0, "read index {path}: {stderr}");
stdout
}
fn setup_racy_offpath_filter(
driver: &str,
name: &str,
command: &str,
kind: PatchKind,
) -> (tempfile::TempDir, String, String) {
let repo = init_repo();
let root = repo.path();
std::fs::write(root.join("outside.txt"), "outside\n").expect("write off-path file");
std::fs::write(
root.join(".gitattributes"),
format!("outside.txt filter={driver}\n"),
)
.expect("write off-path attributes");
run_success(root, &["add", ".gitattributes", "outside.txt"]);
run_success(root, &["commit", "-m", "off-path filter target"]);
let patch = build_patch(root, kind, &["file.txt"]);
configure_filter_command(root, driver, name, command);
run_success(
root,
&["config", &format!("filter.{driver}.required"), "true"],
);
let outside_index = index_contents(root, "outside.txt");
make_index_racy(root);
(repo, patch, outside_index)
}
fn assert_guarded_round_trip(
repo: &tempfile::TempDir,
patch: String,
outside_index: &str,
context: &str,
) {
let root = repo.path();
let forward = apply_git_patch(&ApplyGitRequest {
cwd: root.to_path_buf(),
diff: patch.clone(),
revert: false,
preflight: false,
})
.expect("guard forward apply");
assert_eq!(forward.exit_code, 0, "{context}: {}", forward.stderr);
assert!(!configured_filter_ran(root), "{context}: forward helper");
assert_eq!(index_contents(root, "file.txt"), "next\n", "{context}");
assert_eq!(
index_contents(root, "outside.txt"),
outside_index,
"{context}"
);
make_index_racy(root);
let reverse = apply_git_patch(&ApplyGitRequest {
cwd: root.to_path_buf(),
diff: patch,
revert: true,
preflight: false,
})
.expect("guard reverse apply");
assert_eq!(reverse.exit_code, 0, "{context}: {}", reverse.stderr);
assert!(!configured_filter_ran(root), "{context}: reverse helper");
assert_eq!(index_contents(root, "file.txt"), "orig\n", "{context}");
assert_eq!(
index_contents(root, "outside.txt"),
outside_index,
"{context}"
);
}
@@ -225,3 +345,135 @@ fn apply_allows_effectively_empty_empty_name_filter() {
assert_eq!(result.exit_code, 0);
assert!(!configured_filter_ran(root));
}
#[test]
fn apply_neutralizes_racy_offpath_clean_and_process_filters() {
for (name, command) in [
("clean", FILTER_COMMAND),
("process", PROCESS_FILTER_COMMAND),
] {
for kind in [PatchKind::Direct, PatchKind::ThreeWay] {
let (repo, patch, outside_index) =
setup_racy_offpath_filter("demo", name, command, kind);
assert_guarded_round_trip(&repo, patch, &outside_index, &format!("{name} {kind:?}"));
}
}
}
#[test]
fn apply_neutralizes_empty_and_equals_filter_driver_names() {
for (driver, name, command, kind) in [
("", "clean", FILTER_COMMAND, PatchKind::Direct),
(
"x=y",
"process",
PROCESS_FILTER_COMMAND,
PatchKind::ThreeWay,
),
] {
let (repo, patch, outside_index) = setup_racy_offpath_filter(driver, name, command, kind);
assert_guarded_round_trip(
&repo,
patch,
&outside_index,
&format!("driver {driver:?} {name}"),
);
}
}
#[test]
fn command_scoped_offpath_filter_is_neutralized_after_local_config() {
if std::env::var_os("CODEX_GIT_UTILS_APPLY_FILTER_ENV_CHILD").is_none() {
run_isolated_test(
"apply::filter_tests::command_scoped_offpath_filter_is_neutralized_after_local_config",
&[(
"CODEX_APPLY_GIT_CFG",
OsStr::new(COMMAND_SCOPED_FILTER_CONFIG),
)],
);
return;
}
let repo = init_repo();
let root = repo.path();
std::fs::write(root.join("outside.txt"), "outside\n").expect("write off-path file");
std::fs::write(root.join(".gitattributes"), "outside.txt filter=demo\n")
.expect("write off-path attributes");
run_success(root, &["add", ".gitattributes", "outside.txt"]);
run_success(root, &["commit", "-m", "off-path filter target"]);
run_success(root, &["config", "filter.demo.clean", ""]);
run_success(root, &["config", "filter.demo.required", "true"]);
let outside_index = index_contents(root, "outside.txt");
make_index_racy(root);
assert_guarded_round_trip(
&repo,
PATCH.to_string(),
&outside_index,
"command-scoped clean",
);
}
#[test]
fn apply_neutralizes_filter_activated_for_racy_offpath_by_same_patch() {
for (name, command) in [
("clean", FILTER_COMMAND),
("process", PROCESS_FILTER_COMMAND),
] {
let repo = init_repo();
let root = repo.path();
std::fs::write(root.join("outside.txt"), "outside\n").expect("write off-path file");
std::fs::write(root.join(".gitattributes"), "outside.txt -filter\n")
.expect("write preimage attributes");
run_success(root, &["add", ".gitattributes", "outside.txt"]);
run_success(root, &["commit", "-m", "off-path filter preimage"]);
std::fs::write(root.join(".gitattributes"), "outside.txt filter=demo\n")
.expect("write postimage attributes");
let patch = build_patch(root, PatchKind::ThreeWay, &[".gitattributes", "file.txt"]);
configure_filter_command(root, "demo", name, command);
run_success(root, &["config", "filter.demo.required", "true"]);
let outside_index = index_contents(root, "outside.txt");
make_index_racy(root);
assert_guarded_round_trip(
&repo,
patch,
&outside_index,
&format!("dynamic off-path {name}"),
);
assert_eq!(
std::fs::read_to_string(root.join(".gitattributes")).expect("read restored attributes"),
"outside.txt -filter\n"
);
}
}
#[test]
fn apply_still_refuses_clean_and_process_filters_selected_for_target() {
for (name, command) in [
("clean", FILTER_COMMAND),
("process", PROCESS_FILTER_COMMAND),
] {
let repo = init_repo();
let root = repo.path();
std::fs::write(root.join(".gitattributes"), "file.txt filter=demo\n")
.expect("write target attributes");
run_success(root, &["add", ".gitattributes"]);
run_success(root, &["commit", "-m", "target filter"]);
configure_filter_command(root, "demo", name, command);
run_success(root, &["config", "filter.demo.required", "true"]);
let error = apply_git_patch(&request(
root, /*revert*/ false, /*preflight*/ false,
))
.expect_err("refuse selected target filter");
assert_eq!(error.kind(), io::ErrorKind::Unsupported, "{name}");
assert!(!configured_filter_ran(root), "{name}");
assert_eq!(index_contents(root, "file.txt"), "orig\n", "{name}");
assert_eq!(
std::fs::read_to_string(root.join("file.txt")).expect("read target"),
"orig\n",
"{name}"
);
}
}
+2 -4
View File
@@ -230,7 +230,7 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> {
let (tmpdir, patch_path) = write_temp_patch(diff)?;
let paths = extract_effective_paths_from_patch(&git, &patch_path, /*revert*/ true)?;
let _guard = tmpdir;
stage_effective_paths(&git, git_root, &paths, &[])
stage_effective_paths(&git, git_root, &paths, &safe_git_config_parts())
}
pub(crate) fn stage_effective_paths(
@@ -261,9 +261,7 @@ pub(crate) fn stage_effective_paths(
"--".to_string(),
];
args.extend(existing);
let mut config_parts = git_config_args.to_vec();
config_parts.extend(safe_git_config_parts());
let (_code, _, _) = run_git(git, git_root, &config_parts, &args)?;
let (_code, _, _) = run_git(git, git_root, git_config_args, &args)?;
// We do not hard fail staging; best-effort is OK. Return Ok even on non-zero.
Ok(())
}
+94 -4
View File
@@ -13,7 +13,8 @@ use crate::git_config::parse_effective_config;
use crate::git_config::parse_effective_config_with_origins;
pub(crate) const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
pub(crate) const EXECUTABLE_FILTER_CONFIG_PATTERN: &str = r"^filter\..*\.(clean|smudge|process)$";
pub(crate) const EXECUTABLE_FILTER_CONFIG_PATTERN: &str =
r"^filter\..*\.(clean|smudge|process|required)$";
#[derive(Debug, Clone, PartialEq, Eq)]
enum FilterAttributeValue {
@@ -21,6 +22,24 @@ enum FilterAttributeValue {
AmbiguousSentinel(String),
}
pub(crate) struct GitFilterNeutralization {
git_config_args: Vec<String>,
_config_dir: Option<tempfile::TempDir>,
filter_config: BTreeMap<String, GitConfigEntry>,
}
impl GitFilterNeutralization {
pub(crate) fn git_config_args(&self) -> &[String] {
&self.git_config_args
}
pub(crate) fn filter_value(&self, driver: &str, name: &str) -> Option<&str> {
self.filter_config
.get(&format!("filter.{driver}.{name}"))
.map(|entry| entry.value.as_str())
}
}
const ISOLATED_GIT_ENVIRONMENT: [&str; 11] = [
"GIT_DIR",
"GIT_WORK_TREE",
@@ -53,11 +72,15 @@ pub(crate) fn ensure_no_selected_executable_git_filters(
cwd: &Path,
paths: &[String],
git_config_args: &[String],
) -> io::Result<()> {
) -> io::Result<GitFilterNeutralization> {
let entries = read_filter_config(git, cwd, git_config_args)?;
let executable_drivers = executable_filter_drivers(&entries)?;
if executable_drivers.is_empty() {
return Ok(());
return Ok(GitFilterNeutralization {
git_config_args: Vec::new(),
_config_dir: None,
filter_config: entries,
});
}
let paths = paths
.iter()
@@ -74,7 +97,71 @@ pub(crate) fn ensure_no_selected_executable_git_filters(
),
));
}
Ok(())
executable_filter_guard(git, cwd, entries, &executable_drivers)
}
fn executable_filter_guard(
git: &GitRunner,
cwd: &Path,
filter_config: BTreeMap<String, GitConfigEntry>,
executable_drivers: &BTreeSet<String>,
) -> io::Result<GitFilterNeutralization> {
let config_dir = tempfile::tempdir()?;
let config_path = config_dir.path().join("filter-neutralization.gitconfig");
std::fs::write(&config_path, [])?;
let guard = GitFilterNeutralization {
git_config_args: vec![
"-c".to_string(),
format!(
"include.path={}",
config_path
.to_str()
.ok_or_else(|| invalid_filter_output("non-UTF-8 filter guard path"))?
),
],
_config_dir: Some(config_dir),
filter_config,
};
for driver in executable_drivers {
debug_assert!(["clean", "smudge", "process"].into_iter().any(|name| {
guard
.filter_value(driver, name)
.is_some_and(|value| !value.is_empty())
}));
for command in ["clean", "smudge", "process"] {
guard.write_config_value(git, cwd, &config_path, driver, command, "")?;
}
guard.write_config_value(git, cwd, &config_path, driver, "required", "false")?;
}
Ok(guard)
}
impl GitFilterNeutralization {
fn write_config_value(
&self,
git: &GitRunner,
cwd: &Path,
config_path: &Path,
driver: &str,
name: &str,
value: &str,
) -> io::Result<()> {
let mut command = git.command();
command
.args(["config", "--file"])
.arg(config_path)
.args(["--add", &format!("filter.{driver}.{name}"), value])
.current_dir(cwd);
let output = git.output(command)?;
if !output.status.success() {
return Err(io::Error::other(format!(
"failed to write Git filter neutralization for {driver:?} (status {}): {}",
output.status,
String::from_utf8_lossy(&output.stderr).trim()
)));
}
Ok(())
}
}
fn read_filter_config(
@@ -333,6 +420,9 @@ fn executable_filter_drivers(
) -> io::Result<BTreeSet<String>> {
let mut executable_drivers = BTreeSet::new();
for entry in entries.values() {
if entry.key.ends_with(".required") {
continue;
}
let driver = filter_driver_name(&entry.key)?;
if !entry.value.is_empty() {
executable_drivers.insert(driver);
+42
View File
@@ -2,6 +2,7 @@ use super::*;
use crate::git_config::GitConfigScope;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::path::Path;
#[test]
@@ -57,6 +58,47 @@ fn selected_filter_policy_allows_effective_empty_value() {
);
}
#[test]
fn filter_snapshot_retains_required_without_treating_it_as_executable() {
let mut entries = filter_entries(
GitConfigScope::Local,
Path::new("config"),
"filter.demo.smudge",
"smudge-command",
);
entries.extend(filter_entries(
GitConfigScope::Command,
Path::new("command line:"),
"filter.demo.required",
"true",
));
assert_eq!(
executable_filter_drivers(&entries).expect("executable drivers"),
BTreeSet::from(["demo".to_string()])
);
let neutralization = GitFilterNeutralization {
git_config_args: Vec::new(),
_config_dir: None,
filter_config: entries,
};
assert_eq!(
neutralization.filter_value("demo", "required"),
Some("true")
);
let required_only = filter_entries(
GitConfigScope::Local,
Path::new("config"),
"filter.demo.required",
"true",
);
assert!(
executable_filter_drivers(&required_only)
.expect("required-only config")
.is_empty()
);
}
#[test]
fn filter_driver_parser_accepts_empty_name() {
assert_eq!(