mirror of
https://github.com/openai/codex.git
synced 2026-09-04 15:08:45 +00:00
[git-utils] guard selected filter sink paths
This commit is contained in:
@@ -17,7 +17,7 @@ use crate::patch_paths::ensure_paths_do_not_enter_submodules;
|
||||
use crate::patch_paths::extract_effective_paths_from_patch;
|
||||
use crate::patch_paths::stage_effective_paths;
|
||||
use crate::safe_git::DISABLED_HOOKS_PATH;
|
||||
use crate::safe_git::ensure_no_executable_git_filters;
|
||||
use crate::safe_git::ensure_no_selected_executable_git_filters;
|
||||
use crate::safe_git::isolate_git_command_environment;
|
||||
|
||||
/// Parameters for invoking [`apply_git_patch`].
|
||||
@@ -48,13 +48,13 @@ pub struct ApplyGitResult {
|
||||
pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
|
||||
let mut cfg_parts = configured_git_config_parts();
|
||||
let git_root = resolve_git_root(&req.cwd, &cfg_parts)?;
|
||||
ensure_no_executable_git_filters(&git_root, &cfg_parts)?;
|
||||
|
||||
// Write unified diff into a temporary file
|
||||
let (tmpdir, patch_path) = write_temp_patch(&req.diff)?;
|
||||
// Keep tmpdir alive until function end to ensure the file exists
|
||||
let _guard = tmpdir;
|
||||
let patch_paths = extract_effective_paths_from_patch(&patch_path, req.revert)?;
|
||||
ensure_no_selected_executable_git_filters(&git_root, &patch_paths, &cfg_parts)?;
|
||||
ensure_paths_do_not_enter_submodules(&git_root, &patch_paths)?;
|
||||
|
||||
if !req.preflight {
|
||||
@@ -750,8 +750,17 @@ diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1
|
||||
"filter.codex-test.clean=git hash-object --stdin".to_string(),
|
||||
];
|
||||
|
||||
let error = ensure_no_executable_git_filters(repo.path(), &config_args)
|
||||
.expect_err("reject command-scoped filter");
|
||||
std::fs::write(
|
||||
repo.path().join(".gitattributes"),
|
||||
"test.txt filter=codex-test\n",
|
||||
)
|
||||
.expect("attributes");
|
||||
let error = ensure_no_selected_executable_git_filters(
|
||||
repo.path(),
|
||||
&["test.txt".to_string()],
|
||||
&config_args,
|
||||
)
|
||||
.expect_err("reject command-scoped filter");
|
||||
assert_eq!(error.kind(), io::ErrorKind::Unsupported);
|
||||
}
|
||||
|
||||
|
||||
@@ -20,8 +20,9 @@ use ts_rs::TS;
|
||||
use crate::GitSha;
|
||||
use crate::safe_git::DISABLED_HOOKS_PATH;
|
||||
use crate::safe_git::GIT_COMMAND_TIMEOUT;
|
||||
use crate::safe_git::has_configured_executable_filters_from;
|
||||
use crate::safe_git::has_selected_executable_filters_from;
|
||||
use crate::safe_git::isolate_tokio_git_command_environment;
|
||||
use crate::safe_git::safe_untracked_paths_for_diff;
|
||||
|
||||
/// Return `true` if the project folder specified by the `Config` is inside a
|
||||
/// Git repository.
|
||||
@@ -289,7 +290,7 @@ fn trim_git_suffix(value: &str) -> &str {
|
||||
|
||||
pub async fn get_has_changes(cwd: &Path) -> Option<bool> {
|
||||
let git = Path::new("git");
|
||||
if has_configured_executable_filters_from(git, cwd).await? {
|
||||
if has_selected_executable_filters_from(git, cwd).await? {
|
||||
return None;
|
||||
}
|
||||
let fsmonitor = detect_local_fsmonitor_override(git, cwd).await;
|
||||
@@ -749,9 +750,7 @@ async fn find_closest_sha(cwd: &Path, branches: &[String], remotes: &[String]) -
|
||||
|
||||
async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option<String> {
|
||||
let git = Path::new("git");
|
||||
if has_configured_executable_filters_from(git, cwd).await? {
|
||||
return None;
|
||||
}
|
||||
let untracked = safe_untracked_paths_for_diff(git, cwd).await?;
|
||||
let fsmonitor = detect_local_fsmonitor_override(git, cwd).await;
|
||||
let output = run_git_command_with_timeout_from(
|
||||
git,
|
||||
@@ -775,47 +774,20 @@ async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option<String> {
|
||||
}
|
||||
let mut diff = String::from_utf8(output.stdout).ok()?;
|
||||
|
||||
if let Some(untracked_output) = run_git_command_with_timeout_from(
|
||||
git,
|
||||
&["ls-files", "--others", "--exclude-standard"],
|
||||
cwd,
|
||||
fsmonitor,
|
||||
)
|
||||
.await
|
||||
&& untracked_output.status.success()
|
||||
{
|
||||
let untracked: Vec<String> = String::from_utf8(untracked_output.stdout)
|
||||
.ok()?
|
||||
.lines()
|
||||
.map(str::to_string)
|
||||
.filter(|s| !s.is_empty())
|
||||
.collect();
|
||||
|
||||
if !untracked.is_empty() {
|
||||
// Use platform-appropriate null device and guard paths with `--`.
|
||||
let null_device: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
|
||||
let futures_iter = untracked.into_iter().map(|file| async move {
|
||||
let file_owned = file;
|
||||
let args_vec: Vec<&str> = vec![
|
||||
"diff",
|
||||
"--no-textconv",
|
||||
"--no-ext-diff",
|
||||
"--binary",
|
||||
"--no-index",
|
||||
// -- ensures that filenames that start with - are not treated as options.
|
||||
"--",
|
||||
null_device,
|
||||
&file_owned,
|
||||
];
|
||||
run_git_command_with_timeout_from(git, &args_vec, cwd, fsmonitor).await
|
||||
});
|
||||
let results = join_all(futures_iter).await;
|
||||
for extra in results.into_iter().flatten() {
|
||||
if extra.status.code().is_some_and(|c| c == 0 || c == 1)
|
||||
&& let Ok(s) = String::from_utf8(extra.stdout)
|
||||
{
|
||||
diff.push_str(&s);
|
||||
}
|
||||
if !untracked.is_empty() {
|
||||
let untracked = untracked
|
||||
.into_iter()
|
||||
.map(git_path_bytes_to_os_string)
|
||||
.collect::<Option<Vec<_>>>()?;
|
||||
let futures_iter = untracked.into_iter().map(|file| async move {
|
||||
run_git_no_index_diff_from(git, cwd, fsmonitor, &file).await
|
||||
});
|
||||
let results = join_all(futures_iter).await;
|
||||
for extra in results.into_iter().flatten() {
|
||||
if extra.status.code().is_some_and(|c| c == 0 || c == 1)
|
||||
&& let Ok(s) = String::from_utf8(extra.stdout)
|
||||
{
|
||||
diff.push_str(&s);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -823,6 +795,50 @@ async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option<String> {
|
||||
Some(diff)
|
||||
}
|
||||
|
||||
fn git_path_bytes_to_os_string(path: Vec<u8>) -> Option<std::ffi::OsString> {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::ffi::OsStringExt;
|
||||
Some(std::ffi::OsString::from_vec(path))
|
||||
}
|
||||
#[cfg(windows)]
|
||||
{
|
||||
String::from_utf8(path).ok().map(std::ffi::OsString::from)
|
||||
}
|
||||
}
|
||||
|
||||
async fn run_git_no_index_diff_from(
|
||||
git: &Path,
|
||||
cwd: &Path,
|
||||
fsmonitor: crate::FsmonitorOverride,
|
||||
path: &OsStr,
|
||||
) -> Option<std::process::Output> {
|
||||
let disabled_hooks = format!("core.hooksPath={DISABLED_HOOKS_PATH}");
|
||||
let null_device = if cfg!(windows) { "NUL" } else { "/dev/null" };
|
||||
let mut command = Command::new(git);
|
||||
isolate_tokio_git_command_environment(&mut command);
|
||||
command
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.args(["-c", &disabled_hooks])
|
||||
.args(["-c", fsmonitor.git_config_arg()])
|
||||
.args([
|
||||
"diff",
|
||||
"--no-textconv",
|
||||
"--no-ext-diff",
|
||||
"--binary",
|
||||
"--no-index",
|
||||
"--",
|
||||
null_device,
|
||||
])
|
||||
.arg(path)
|
||||
.current_dir(cwd)
|
||||
.kill_on_drop(true);
|
||||
match timeout(GIT_COMMAND_TIMEOUT, command.output()).await {
|
||||
Ok(Ok(output)) => Some(output),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve the path that should be used for trust checks. Similar to
|
||||
/// `[get_git_repo_root]`, but resolves to the root of the main
|
||||
/// repository. Handles worktrees via filesystem inspection without invoking
|
||||
|
||||
@@ -209,6 +209,10 @@ mod tests {
|
||||
(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");
|
||||
|
||||
@@ -8,7 +8,7 @@ use crate::apply::run_git;
|
||||
use crate::apply::safe_git_config_parts;
|
||||
use crate::apply::write_temp_patch;
|
||||
use crate::git_config::path_is_within;
|
||||
use crate::safe_git::ensure_no_executable_git_filters;
|
||||
use crate::safe_git::ensure_no_selected_executable_git_filters;
|
||||
use crate::safe_git::isolate_git_command_environment;
|
||||
|
||||
pub(crate) fn extract_effective_paths_from_patch(
|
||||
@@ -175,7 +175,7 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> {
|
||||
}
|
||||
|
||||
pub(crate) fn stage_effective_paths(git_root: &Path, paths: &[String]) -> io::Result<()> {
|
||||
ensure_no_executable_git_filters(git_root, &[])?;
|
||||
ensure_no_selected_executable_git_filters(git_root, paths, &[])?;
|
||||
let mut existing: Vec<String> = Vec::new();
|
||||
for p in paths {
|
||||
let joined = git_root.join(p);
|
||||
|
||||
@@ -1,7 +1,12 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::BTreeSet;
|
||||
use std::io;
|
||||
use std::io::Seek;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Command;
|
||||
use std::process::Stdio;
|
||||
use tokio::process::Command as TokioCommand;
|
||||
use tokio::time::Duration;
|
||||
use tokio::time::timeout;
|
||||
@@ -14,7 +19,7 @@ pub(crate) const EXECUTABLE_FILTER_CONFIG_PATTERN: &str = r"^filter\..*\.(clean|
|
||||
/// Timeout for internal Git commands to prevent freezing on large repositories.
|
||||
pub(crate) const GIT_COMMAND_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
|
||||
const ISOLATED_GIT_ENVIRONMENT: [&str; 10] = [
|
||||
const ISOLATED_GIT_ENVIRONMENT: [&str; 11] = [
|
||||
"GIT_DIR",
|
||||
"GIT_WORK_TREE",
|
||||
"GIT_COMMON_DIR",
|
||||
@@ -25,6 +30,10 @@ const ISOLATED_GIT_ENVIRONMENT: [&str; 10] = [
|
||||
"GIT_NOGLOB_PATHSPECS",
|
||||
"GIT_ICASE_PATHSPECS",
|
||||
"GIT_EXEC_PATH",
|
||||
// Legacy `GIT_CONFIG` affects `git config` but not ordinary worktree
|
||||
// commands, so inheriting it can make a safety probe inspect different
|
||||
// configuration than the command it guards.
|
||||
"GIT_CONFIG",
|
||||
];
|
||||
|
||||
/// Keep internal worktree operations bound to their explicit cwd and pathspec
|
||||
@@ -43,41 +52,146 @@ pub(crate) fn isolate_tokio_git_command_environment(command: &mut tokio::process
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn has_configured_executable_filters_from(git: &Path, cwd: &Path) -> Option<bool> {
|
||||
pub(crate) async fn has_selected_executable_filters_from(git: &Path, cwd: &Path) -> Option<bool> {
|
||||
let git_root = resolve_git_root_async(git, cwd).await?;
|
||||
let entries = read_filter_config_async(git, &git_root).await?;
|
||||
if !entries.values().any(|entry| !entry.value.is_empty()) {
|
||||
return Some(false);
|
||||
}
|
||||
let paths = read_paths_async(git, &git_root, PathSelection::Tracked).await?;
|
||||
let attributes = read_filter_attributes_async(git, &git_root, &paths).await?;
|
||||
selected_executable_filter(&entries, &attributes)
|
||||
.ok()
|
||||
.map(|selected| selected.is_some())
|
||||
}
|
||||
|
||||
/// Validate every tracked path plus the exact untracked paths that the caller
|
||||
/// will later feed to `git diff --no-index`. The returned raw paths must be
|
||||
/// reused rather than reconstructed from Git's quoted line-oriented output.
|
||||
pub(crate) async fn safe_untracked_paths_for_diff(git: &Path, cwd: &Path) -> Option<Vec<Vec<u8>>> {
|
||||
let requested_cwd = std::fs::canonicalize(cwd).ok()?;
|
||||
let git_root = resolve_git_root_async(git, &requested_cwd).await?;
|
||||
let untracked = read_paths_async(git, &requested_cwd, PathSelection::Untracked).await?;
|
||||
// An embedded untracked repository is reported by `ls-files` as a single
|
||||
// directory entry. Passing that entry to `git diff --no-index` invokes
|
||||
// file-vs-directory comparison semantics, which can derive and open a
|
||||
// child path that was never returned by the path probe. Fail closed rather
|
||||
// than let the sink operate on a different path vector than we validated.
|
||||
for path in &untracked {
|
||||
let path = git_path_bytes_to_path_buf(path)?;
|
||||
if std::fs::symlink_metadata(requested_cwd.join(path))
|
||||
.ok()?
|
||||
.file_type()
|
||||
.is_dir()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
}
|
||||
let entries = read_filter_config_async(git, &git_root).await?;
|
||||
if !entries.values().any(|entry| !entry.value.is_empty()) {
|
||||
return Some(untracked);
|
||||
}
|
||||
|
||||
let tracked = read_paths_async(git, &git_root, PathSelection::Tracked).await?;
|
||||
let tracked_attributes = read_filter_attributes_async(git, &git_root, &tracked).await?;
|
||||
if selected_executable_filter(&entries, &tracked_attributes)
|
||||
.ok()?
|
||||
.is_some()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let untracked_attributes =
|
||||
read_filter_attributes_async(git, &requested_cwd, &untracked).await?;
|
||||
if selected_executable_filter(&entries, &untracked_attributes)
|
||||
.ok()?
|
||||
.is_some()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
Some(untracked)
|
||||
}
|
||||
|
||||
fn git_path_bytes_to_path_buf(path: &[u8]) -> Option<PathBuf> {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::ffi::OsString;
|
||||
use std::os::unix::ffi::OsStringExt;
|
||||
|
||||
Some(PathBuf::from(OsString::from_vec(path.to_vec())))
|
||||
}
|
||||
#[cfg(windows)]
|
||||
{
|
||||
String::from_utf8(path.to_vec()).ok().map(PathBuf::from)
|
||||
}
|
||||
}
|
||||
|
||||
async fn resolve_git_root_async(git: &Path, cwd: &Path) -> Option<PathBuf> {
|
||||
let requested_cwd = std::fs::canonicalize(cwd).ok()?;
|
||||
let mut command = TokioCommand::new(git);
|
||||
isolate_tokio_git_command_environment(&mut command);
|
||||
command
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.args([
|
||||
"config",
|
||||
"--null",
|
||||
"--show-scope",
|
||||
"--show-origin",
|
||||
"--includes",
|
||||
"--get-regexp",
|
||||
EXECUTABLE_FILTER_CONFIG_PATTERN,
|
||||
"-c",
|
||||
&format!("core.hooksPath={DISABLED_HOOKS_PATH}"),
|
||||
"-c",
|
||||
"core.fsmonitor=false",
|
||||
"rev-parse",
|
||||
"--show-toplevel",
|
||||
])
|
||||
.current_dir(cwd)
|
||||
.current_dir(&requested_cwd)
|
||||
.kill_on_drop(true);
|
||||
let output = match timeout(GIT_COMMAND_TIMEOUT, command.output()).await {
|
||||
Ok(Ok(output)) => output,
|
||||
_ => return None,
|
||||
};
|
||||
if !output
|
||||
.status
|
||||
.code()
|
||||
.is_some_and(|code| code == 0 || code == 1)
|
||||
{
|
||||
if !output.status.success() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let entries = parse_effective_config(&output.stdout).ok()?;
|
||||
Some(config_entries_have_untrusted_filters(&entries))
|
||||
let path = String::from_utf8(output.stdout).ok()?;
|
||||
let path = path.trim_end_matches(['\r', '\n']);
|
||||
if path.is_empty() {
|
||||
return None;
|
||||
}
|
||||
let reported_root = std::fs::canonicalize(PathBuf::from(path)).ok()?;
|
||||
let expected_root = crate::get_git_repo_root(&requested_cwd)
|
||||
.and_then(|root| std::fs::canonicalize(root).ok())?;
|
||||
if reported_root != expected_root {
|
||||
return None;
|
||||
}
|
||||
Some(reported_root)
|
||||
}
|
||||
|
||||
pub(crate) fn ensure_no_executable_git_filters(
|
||||
pub(crate) fn ensure_no_selected_executable_git_filters(
|
||||
cwd: &Path,
|
||||
paths: &[String],
|
||||
git_config_args: &[String],
|
||||
) -> io::Result<()> {
|
||||
let entries = read_filter_config(cwd, git_config_args)?;
|
||||
if !entries.values().any(|entry| !entry.value.is_empty()) {
|
||||
return Ok(());
|
||||
}
|
||||
let paths = paths
|
||||
.iter()
|
||||
.map(|path| path.as_bytes().to_vec())
|
||||
.collect::<Vec<_>>();
|
||||
let attributes = read_filter_attributes(cwd, &paths, git_config_args)?;
|
||||
if let Some((driver, path)) = selected_executable_filter(&entries, &attributes)? {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::Unsupported,
|
||||
format!(
|
||||
"refusing to run an internal Git worktree operation with executable filter {driver:?} selected for {}",
|
||||
String::from_utf8_lossy(&path)
|
||||
),
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn read_filter_config(
|
||||
cwd: &Path,
|
||||
git_config_args: &[String],
|
||||
) -> io::Result<BTreeMap<String, GitConfigEntry>> {
|
||||
let mut command = Command::new("git");
|
||||
isolate_git_command_environment(&mut command);
|
||||
let output = command
|
||||
@@ -105,18 +219,271 @@ pub(crate) fn ensure_no_executable_git_filters(
|
||||
String::from_utf8_lossy(&output.stderr).trim()
|
||||
)));
|
||||
}
|
||||
let entries = parse_effective_config(&output.stdout)?;
|
||||
if config_entries_have_untrusted_filters(&entries) {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::Unsupported,
|
||||
"refusing to run an internal Git worktree operation with an executable Git filter configured",
|
||||
));
|
||||
parse_effective_config(&output.stdout)
|
||||
}
|
||||
|
||||
async fn read_filter_config_async(
|
||||
git: &Path,
|
||||
cwd: &Path,
|
||||
) -> Option<BTreeMap<String, GitConfigEntry>> {
|
||||
let mut command = TokioCommand::new(git);
|
||||
isolate_tokio_git_command_environment(&mut command);
|
||||
command
|
||||
.args([
|
||||
"config",
|
||||
"--null",
|
||||
"--show-scope",
|
||||
"--show-origin",
|
||||
"--includes",
|
||||
"--get-regexp",
|
||||
EXECUTABLE_FILTER_CONFIG_PATTERN,
|
||||
])
|
||||
.current_dir(cwd)
|
||||
.kill_on_drop(true);
|
||||
let output = match timeout(GIT_COMMAND_TIMEOUT, command.output()).await {
|
||||
Ok(Ok(output)) => output,
|
||||
_ => return None,
|
||||
};
|
||||
if !output
|
||||
.status
|
||||
.code()
|
||||
.is_some_and(|code| code == 0 || code == 1)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
parse_effective_config(&output.stdout).ok()
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum PathSelection {
|
||||
Tracked,
|
||||
Untracked,
|
||||
}
|
||||
|
||||
async fn read_paths_async(
|
||||
git: &Path,
|
||||
cwd: &Path,
|
||||
selection: PathSelection,
|
||||
) -> Option<Vec<Vec<u8>>> {
|
||||
let mut command = TokioCommand::new(git);
|
||||
isolate_tokio_git_command_environment(&mut command);
|
||||
let hooks_config = format!("core.hooksPath={DISABLED_HOOKS_PATH}");
|
||||
let mut args = vec![
|
||||
"-c",
|
||||
hooks_config.as_str(),
|
||||
"-c",
|
||||
"core.fsmonitor=false",
|
||||
"ls-files",
|
||||
"-z",
|
||||
];
|
||||
match selection {
|
||||
PathSelection::Tracked => args.push("--cached"),
|
||||
PathSelection::Untracked => args.extend(["--others", "--exclude-standard"]),
|
||||
}
|
||||
command
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.args(args)
|
||||
.current_dir(cwd)
|
||||
.kill_on_drop(true);
|
||||
let output = match timeout(GIT_COMMAND_TIMEOUT, command.output()).await {
|
||||
Ok(Ok(output)) => output,
|
||||
_ => return None,
|
||||
};
|
||||
if !output.status.success() {
|
||||
return None;
|
||||
}
|
||||
parse_nul_paths(&output.stdout).ok()
|
||||
}
|
||||
|
||||
async fn read_filter_attributes_async(
|
||||
git: &Path,
|
||||
cwd: &Path,
|
||||
paths: &[Vec<u8>],
|
||||
) -> Option<BTreeMap<Vec<u8>, String>> {
|
||||
if paths.is_empty() {
|
||||
return Some(BTreeMap::new());
|
||||
}
|
||||
let mut input = tempfile::tempfile().ok()?;
|
||||
write_nul_paths(&mut input, paths).ok()?;
|
||||
input.rewind().ok()?;
|
||||
|
||||
let mut command = TokioCommand::new(git);
|
||||
isolate_tokio_git_command_environment(&mut command);
|
||||
command
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.args([
|
||||
"-c",
|
||||
&format!("core.hooksPath={DISABLED_HOOKS_PATH}"),
|
||||
"-c",
|
||||
"core.fsmonitor=false",
|
||||
"check-attr",
|
||||
"--stdin",
|
||||
"-z",
|
||||
"filter",
|
||||
])
|
||||
.current_dir(cwd)
|
||||
.stdin(Stdio::from(input))
|
||||
.kill_on_drop(true);
|
||||
let output = match timeout(GIT_COMMAND_TIMEOUT, command.output()).await {
|
||||
Ok(Ok(output)) => output,
|
||||
_ => return None,
|
||||
};
|
||||
if !output.status.success() {
|
||||
return None;
|
||||
}
|
||||
parse_filter_attributes(&output.stdout, paths).ok()
|
||||
}
|
||||
|
||||
fn read_filter_attributes(
|
||||
cwd: &Path,
|
||||
paths: &[Vec<u8>],
|
||||
git_config_args: &[String],
|
||||
) -> io::Result<BTreeMap<Vec<u8>, String>> {
|
||||
if paths.is_empty() {
|
||||
return Ok(BTreeMap::new());
|
||||
}
|
||||
let mut input = tempfile::tempfile()?;
|
||||
write_nul_paths(&mut input, paths)?;
|
||||
input.rewind()?;
|
||||
|
||||
let mut command = Command::new("git");
|
||||
isolate_git_command_environment(&mut command);
|
||||
let output = command
|
||||
.env("GIT_OPTIONAL_LOCKS", "0")
|
||||
.args(git_config_args)
|
||||
.args([
|
||||
"-c",
|
||||
&format!("core.hooksPath={DISABLED_HOOKS_PATH}"),
|
||||
"-c",
|
||||
"core.fsmonitor=false",
|
||||
"check-attr",
|
||||
"--stdin",
|
||||
"-z",
|
||||
"filter",
|
||||
])
|
||||
.current_dir(cwd)
|
||||
.stdin(Stdio::from(input))
|
||||
.output()?;
|
||||
if !output.status.success() {
|
||||
return Err(io::Error::other(format!(
|
||||
"git filter attribute probe failed with status {}: {}",
|
||||
output.status,
|
||||
String::from_utf8_lossy(&output.stderr).trim()
|
||||
)));
|
||||
}
|
||||
parse_filter_attributes(&output.stdout, paths)
|
||||
}
|
||||
|
||||
fn selected_executable_filter(
|
||||
entries: &BTreeMap<String, GitConfigEntry>,
|
||||
attributes: &BTreeMap<Vec<u8>, String>,
|
||||
) -> io::Result<Option<(String, Vec<u8>)>> {
|
||||
let mut executable_drivers = BTreeSet::new();
|
||||
for entry in entries.values() {
|
||||
let driver = filter_driver_name(&entry.key)?;
|
||||
if !entry.value.is_empty() {
|
||||
executable_drivers.insert(driver);
|
||||
}
|
||||
}
|
||||
for (path, driver) in attributes {
|
||||
if executable_drivers.contains(driver) {
|
||||
return Ok(Some((driver.clone(), path.clone())));
|
||||
}
|
||||
}
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
fn filter_driver_name(key: &str) -> io::Result<String> {
|
||||
let Some(remainder) = key.strip_prefix("filter.") else {
|
||||
return Err(invalid_filter_output("malformed filter config key"));
|
||||
};
|
||||
let driver = [".clean", ".smudge", ".process"]
|
||||
.into_iter()
|
||||
.find_map(|suffix| remainder.strip_suffix(suffix))
|
||||
.filter(|driver| !driver.is_empty())
|
||||
.ok_or_else(|| invalid_filter_output("malformed filter config key"))?;
|
||||
Ok(driver.to_string())
|
||||
}
|
||||
|
||||
fn parse_nul_paths(output: &[u8]) -> io::Result<Vec<Vec<u8>>> {
|
||||
if output.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
let Some(body) = output.strip_suffix(&[0]) else {
|
||||
return Err(invalid_filter_output("unterminated Git path output"));
|
||||
};
|
||||
let mut paths = Vec::new();
|
||||
for path in body.split(|byte| *byte == 0) {
|
||||
if path.is_empty() {
|
||||
return Err(invalid_filter_output("empty Git path"));
|
||||
}
|
||||
paths.push(path.to_vec());
|
||||
}
|
||||
Ok(paths)
|
||||
}
|
||||
|
||||
fn write_nul_paths(input: &mut std::fs::File, paths: &[Vec<u8>]) -> io::Result<()> {
|
||||
let mut unique = BTreeSet::new();
|
||||
for path in paths {
|
||||
if path.is_empty() || path.contains(&0) {
|
||||
return Err(invalid_filter_output("invalid Git path"));
|
||||
}
|
||||
if unique.insert(path.as_slice()) {
|
||||
input.write_all(path)?;
|
||||
input.write_all(&[0])?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn config_entries_have_untrusted_filters(entries: &BTreeMap<String, GitConfigEntry>) -> bool {
|
||||
entries.values().any(|entry| !entry.value.is_empty())
|
||||
fn parse_filter_attributes(
|
||||
output: &[u8],
|
||||
expected_paths: &[Vec<u8>],
|
||||
) -> io::Result<BTreeMap<Vec<u8>, String>> {
|
||||
let expected = expected_paths
|
||||
.iter()
|
||||
.map(Vec::as_slice)
|
||||
.collect::<BTreeSet<_>>();
|
||||
if expected.is_empty() && output.is_empty() {
|
||||
return Ok(BTreeMap::new());
|
||||
}
|
||||
let Some(body) = output.strip_suffix(&[0]) else {
|
||||
return Err(invalid_filter_output(
|
||||
"unterminated Git filter attribute output",
|
||||
));
|
||||
};
|
||||
let fields = body.split(|byte| *byte == 0).collect::<Vec<_>>();
|
||||
if fields.len() % 3 != 0 {
|
||||
return Err(invalid_filter_output(
|
||||
"incomplete Git filter attribute record",
|
||||
));
|
||||
}
|
||||
let mut attributes = BTreeMap::new();
|
||||
for record in fields.chunks_exact(3) {
|
||||
if !expected.contains(record[0]) || record[1] != b"filter" {
|
||||
return Err(invalid_filter_output(
|
||||
"unexpected Git filter attribute record",
|
||||
));
|
||||
}
|
||||
let driver = std::str::from_utf8(record[2])
|
||||
.map_err(|_| invalid_filter_output("non-UTF-8 Git filter attribute value"))?;
|
||||
if attributes
|
||||
.insert(record[0].to_vec(), driver.to_string())
|
||||
.is_some()
|
||||
{
|
||||
return Err(invalid_filter_output(
|
||||
"duplicate Git filter attribute record",
|
||||
));
|
||||
}
|
||||
}
|
||||
if attributes.len() != expected.len() {
|
||||
return Err(invalid_filter_output("missing Git filter attribute record"));
|
||||
}
|
||||
Ok(attributes)
|
||||
}
|
||||
|
||||
fn invalid_filter_output(message: &str) -> io::Error {
|
||||
io::Error::new(io::ErrorKind::InvalidData, message)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -4,16 +4,19 @@ use crate::apply::apply_git_patch;
|
||||
use crate::get_has_changes;
|
||||
use crate::git_config::GitConfigScope;
|
||||
use crate::git_diff_to_remote;
|
||||
#[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 rejects_nonempty_filters_at_every_scope_including_lfs() {
|
||||
fn selected_filter_policy_allows_unused_and_rejects_selected_at_every_scope() {
|
||||
let config_dir = tempfile::tempdir().expect("config");
|
||||
let config = config_dir.path().join("global.gitconfig");
|
||||
std::fs::write(&config, "").expect("config file");
|
||||
@@ -32,8 +35,18 @@ fn rejects_nonempty_filters_at_every_scope_including_lfs() {
|
||||
("filter.lfs.process", "git-lfs filter-process"),
|
||||
] {
|
||||
let entries = filter_entries(scope, &config, key, value);
|
||||
let driver = filter_driver_name(key).expect("driver name");
|
||||
let selected = BTreeMap::from([(b"file.txt".to_vec(), driver.clone())]);
|
||||
assert!(
|
||||
config_entries_have_untrusted_filters(&entries),
|
||||
selected_executable_filter(&entries, &selected)
|
||||
.expect("selected filter policy")
|
||||
.is_some(),
|
||||
"{scope:?} {key}"
|
||||
);
|
||||
let unused = BTreeMap::from([(b"file.txt".to_vec(), "other".to_string())]);
|
||||
assert_eq!(
|
||||
selected_executable_filter(&entries, &unused).expect("unused filter policy"),
|
||||
None,
|
||||
"{scope:?} {key}"
|
||||
);
|
||||
}
|
||||
@@ -41,14 +54,46 @@ fn rejects_nonempty_filters_at_every_scope_including_lfs() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allows_only_effective_empty_filter_values() {
|
||||
fn selected_filter_policy_allows_effective_empty_value() {
|
||||
let disabled = filter_entries(
|
||||
GitConfigScope::Command,
|
||||
Path::new("command line:"),
|
||||
"filter.demo.clean",
|
||||
"",
|
||||
);
|
||||
assert!(!config_entries_have_untrusted_filters(&disabled));
|
||||
let selected = BTreeMap::from([(b"file.txt".to_vec(), "demo".to_string())]);
|
||||
assert_eq!(
|
||||
selected_executable_filter(&disabled, &selected).expect("empty filter policy"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filter_attribute_parser_rejects_malformed_or_unexpected_records() {
|
||||
let paths = vec![b"a.txt".to_vec(), b"b.txt".to_vec()];
|
||||
let parsed =
|
||||
parse_filter_attributes(b"a.txt\0filter\0unspecified\0b.txt\0filter\0lfs\0", &paths)
|
||||
.expect("parse attributes");
|
||||
assert_eq!(
|
||||
parsed.get(b"a.txt".as_slice()).map(String::as_str),
|
||||
Some("unspecified")
|
||||
);
|
||||
assert_eq!(
|
||||
parsed.get(b"b.txt".as_slice()).map(String::as_str),
|
||||
Some("lfs")
|
||||
);
|
||||
|
||||
for output in [
|
||||
b"a.txt\0filter\0unspecified".as_slice(),
|
||||
b"a.txt\0merge\0unspecified\0b.txt\0filter\0lfs\0".as_slice(),
|
||||
b"a.txt\0filter\0unspecified\0".as_slice(),
|
||||
b"a.txt\0filter\0unspecified\0a.txt\0filter\0lfs\0".as_slice(),
|
||||
] {
|
||||
assert!(
|
||||
parse_filter_attributes(output, &paths).is_err(),
|
||||
"{output:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn filter_entries(
|
||||
@@ -73,6 +118,7 @@ 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);
|
||||
@@ -121,6 +167,38 @@ async fn create_test_git_repo(temp_dir: &tempfile::TempDir) -> std::path::PathBu
|
||||
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);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(repo_path.join("test.txt")).expect("read result"),
|
||||
"new\n"
|
||||
);
|
||||
}
|
||||
|
||||
async fn configure_clean_filter(repo_path: &Path, tracked_path: &str) {
|
||||
std::fs::write(
|
||||
repo_path.join(".gitattributes"),
|
||||
@@ -237,6 +315,205 @@ async fn get_has_changes_rejects_configured_clean_filter_without_running_it() {
|
||||
assert!(!configured_filter_ran(&repo_path).await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn get_has_changes_rejects_core_worktree_redirection_before_running_filter() {
|
||||
let temp_dir = tempfile::tempdir().expect("create temp dir");
|
||||
let repo_path = create_test_git_repo(&temp_dir).await;
|
||||
let redirected = temp_dir.path().join("redirected");
|
||||
std::fs::create_dir(&redirected).expect("redirected worktree");
|
||||
std::fs::write(redirected.join(".gitattributes"), "test.txt filter=x=y\n").expect("attributes");
|
||||
std::fs::write(redirected.join("test.txt"), "redirected content\n").expect("redirected file");
|
||||
run_git(
|
||||
&repo_path,
|
||||
&[
|
||||
"config",
|
||||
"core.worktree",
|
||||
redirected.to_str().expect("redirected path"),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
run_git(
|
||||
&repo_path,
|
||||
&[
|
||||
"config",
|
||||
"filter.x=y.clean",
|
||||
"git config codex.filterran true && git hash-object --stdin",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(get_has_changes(&repo_path).await, None);
|
||||
assert!(!configured_filter_ran(&repo_path).await);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn legacy_git_config_cannot_hide_a_selected_local_filter_from_probe() {
|
||||
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;
|
||||
std::fs::write(repo_path.join(".gitattributes"), "test.txt filter=evil\n")
|
||||
.expect("attributes");
|
||||
run_git(&repo_path, &["add", ".gitattributes"]).await;
|
||||
run_git(&repo_path, &["commit", "-m", "attributes"]).await;
|
||||
|
||||
let marker = temp_dir.path().join("filter-ran");
|
||||
let filter = repo_path.join("clean.sh");
|
||||
std::fs::write(
|
||||
&filter,
|
||||
format!("#!/bin/sh\n: > '{}'\ncat\n", marker.display()),
|
||||
)
|
||||
.expect("filter script");
|
||||
let mut permissions = std::fs::metadata(&filter)
|
||||
.expect("filter metadata")
|
||||
.permissions();
|
||||
permissions.set_mode(0o755);
|
||||
std::fs::set_permissions(&filter, permissions).expect("filter executable");
|
||||
run_git(
|
||||
&repo_path,
|
||||
&[
|
||||
"config",
|
||||
"--local",
|
||||
"filter.evil.clean",
|
||||
filter.to_str().expect("filter path"),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
std::fs::write(repo_path.join("test.txt"), "changed\n").expect("modify tracked file");
|
||||
|
||||
let safe_config = temp_dir.path().join("safe.gitconfig");
|
||||
std::fs::write(&safe_config, "").expect("safe config");
|
||||
run_isolated_test(
|
||||
"safe_git::tests::legacy_git_config_cannot_hide_a_selected_local_filter_from_probe",
|
||||
&[
|
||||
("CODEX_GIT_UTILS_TARGET_REPO", repo_path.as_os_str()),
|
||||
("GIT_CONFIG", safe_config.as_os_str()),
|
||||
],
|
||||
);
|
||||
assert!(!marker.exists(), "selected local filter must not run");
|
||||
return;
|
||||
}
|
||||
|
||||
let repo_path =
|
||||
PathBuf::from(std::env::var_os("CODEX_GIT_UTILS_TARGET_REPO").expect("target repository"));
|
||||
assert_eq!(get_has_changes(&repo_path).await, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn diff_rejects_a_filter_selected_only_for_an_untracked_path() {
|
||||
let temp_dir = tempfile::tempdir().expect("create temp dir");
|
||||
let repo_path = create_test_git_repo(&temp_dir).await;
|
||||
let remote_path = temp_dir.path().join("remote.git");
|
||||
add_origin_and_push(&repo_path, &remote_path).await;
|
||||
std::fs::write(
|
||||
repo_path.join(".gitattributes"),
|
||||
"untracked.txt filter=x=y\n",
|
||||
)
|
||||
.expect("write attributes");
|
||||
run_git(&repo_path, &["add", ".gitattributes"]).await;
|
||||
run_git(&repo_path, &["commit", "-m", "attributes"]).await;
|
||||
run_git(
|
||||
&repo_path,
|
||||
&[
|
||||
"config",
|
||||
"filter.x=y.clean",
|
||||
"git config codex.filterran true && git hash-object --stdin",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
std::fs::write(repo_path.join("untracked.txt"), "untracked\n").expect("untracked file");
|
||||
|
||||
assert_eq!(get_has_changes(&repo_path).await, Some(true));
|
||||
assert!(
|
||||
!configured_filter_ran(&repo_path).await,
|
||||
"status ran filter"
|
||||
);
|
||||
assert!(git_diff_to_remote(&repo_path).await.is_none());
|
||||
assert!(!configured_filter_ran(&repo_path).await, "diff ran filter");
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn untracked_diff_reuses_raw_nul_paths_without_c_quoted_collision() {
|
||||
let temp_dir = tempfile::tempdir().expect("create temp dir");
|
||||
let repo_path = create_test_git_repo(&temp_dir).await;
|
||||
let remote_path = temp_dir.path().join("remote.git");
|
||||
add_origin_and_push(&repo_path, &remote_path).await;
|
||||
|
||||
// Git's line-oriented output C-quotes `dir/a<newline>b` as
|
||||
// `"dir/a\\nb"`. Create an ignored file with that exact quoted spelling;
|
||||
// a consumer that line-parses and reuses the display form will open the
|
||||
// ignored collision instead of the nonignored raw pathname.
|
||||
std::fs::write(repo_path.join(".gitignore"), "\"dir/\n").expect("ignore collision");
|
||||
std::fs::write(
|
||||
repo_path.join(".gitattributes"),
|
||||
"\\\"dir/** filter=collision\n",
|
||||
)
|
||||
.expect("collision attributes");
|
||||
run_git(&repo_path, &["add", ".gitignore", ".gitattributes"]).await;
|
||||
run_git(&repo_path, &["commit", "-m", "collision metadata"]).await;
|
||||
|
||||
let raw_dir = repo_path.join("dir");
|
||||
let collision_dir = repo_path.join("\"dir");
|
||||
std::fs::create_dir(&raw_dir).expect("raw dir");
|
||||
std::fs::create_dir(&collision_dir).expect("collision dir");
|
||||
std::fs::write(raw_dir.join("a\nb"), "safe untracked\n").expect("raw newline path");
|
||||
std::fs::write(collision_dir.join("a\\nb\""), "ignored collision\n").expect("collision path");
|
||||
run_git(
|
||||
&repo_path,
|
||||
&[
|
||||
"config",
|
||||
"filter.collision.clean",
|
||||
"git config codex.filterran true && git hash-object --stdin",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
run_git(&repo_path, &["check-ignore", "\"dir/a\\nb\""]).await;
|
||||
|
||||
assert!(git_diff_to_remote(&repo_path).await.is_some());
|
||||
assert!(!configured_filter_ran(&repo_path).await);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn untracked_diff_rejects_embedded_repo_before_derived_filter_path() {
|
||||
let temp_dir = tempfile::tempdir().expect("create temp dir");
|
||||
let repo_path = create_test_git_repo(&temp_dir).await;
|
||||
let remote_path = temp_dir.path().join("remote.git");
|
||||
add_origin_and_push(&repo_path, &remote_path).await;
|
||||
|
||||
// `ls-files --others` represents an embedded repository as `nested/`, but
|
||||
// `git diff --no-index /dev/null nested/` compares `/dev/null` with
|
||||
// `nested/null`. The latter path can select a filter that a probe of only
|
||||
// the reported directory entry would miss.
|
||||
std::fs::write(
|
||||
repo_path.join(".gitattributes"),
|
||||
"nested/null filter=evil\n",
|
||||
)
|
||||
.expect("write attributes");
|
||||
run_git(&repo_path, &["add", ".gitattributes"]).await;
|
||||
run_git(&repo_path, &["commit", "-m", "embedded path attributes"]).await;
|
||||
run_git(
|
||||
&repo_path,
|
||||
&[
|
||||
"config",
|
||||
"filter.evil.clean",
|
||||
"git config codex.filterran true && git hash-object --stdin",
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let nested = repo_path.join("nested");
|
||||
std::fs::create_dir(&nested).expect("create embedded repo");
|
||||
run_git(&nested, &["init"]).await;
|
||||
std::fs::write(nested.join("null"), "embedded contents\n").expect("write embedded file");
|
||||
|
||||
assert!(git_diff_to_remote(&repo_path).await.is_none());
|
||||
assert!(!configured_filter_ran(&repo_path).await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn get_has_changes_does_not_enter_dirty_submodules() {
|
||||
let temp_dir = tempfile::tempdir().expect("create temp dir");
|
||||
|
||||
Reference in New Issue
Block a user