openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-12 21:20:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

[git-utils] harden internal git helpers

Browse Source
This commit is contained in:
Eva Wong
2026-06-10 12:31:43 -07:00
parent 2ef007dc1a
commit 6e89d6342d
5 changed files with 415 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/git-utils/src/apply.rs
+126 -12
View File
@@ -8,11 +8,15 @@
use once_cell::sync::Lazy;
use regex::Regex;
use std::ffi::OsStr;
use std::io;
use std::path::Path;
use std::path::PathBuf;
use crate::FsmonitorOverride;
use crate::safe_git::DISABLED_HOOKS_PATH;
use crate::safe_git::GitConfigOverride;
use crate::safe_git::configured_executable_git_config_overrides;
/// Parameters for invoking [`apply_git_patch`].
#[derive(Debug, Clone)]
pub struct ApplyGitRequest {
@@ -50,6 +54,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
// Stage WT paths first to avoid index mismatch on revert.
stage_paths(&git_root, &req.diff)?;
}
let executable_git_config_overrides = configured_executable_git_config_overrides(&git_root)?;
// Build git args
let mut args: Vec<String> = vec!["apply".into(), "--3way".into()];
@@ -69,6 +74,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
cfg_parts.push(p.to_string());
}
}
cfg_parts.extend(safe_git_config_parts());
args.push(patch_path.to_string_lossy().to_string());
@@ -80,7 +86,12 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
}
check_args.push(patch_path.to_string_lossy().to_string());
let rendered = render_command_for_log(&git_root, &cfg_parts, &check_args);
let (c_code, c_out, c_err) = run_git(&git_root, &cfg_parts, &check_args)?;
let (c_code, c_out, c_err) = run_git(
&git_root,
&cfg_parts,
&executable_git_config_overrides,
&check_args,
)?;
let (mut applied_paths, mut skipped_paths, mut conflicted_paths) =
parse_git_apply_output(&c_out, &c_err);
applied_paths.sort();
@@ -101,7 +112,12 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
}
let cmd_for_log = render_command_for_log(&git_root, &cfg_parts, &args);
let (code, stdout, stderr) = run_git(&git_root, &cfg_parts, &args)?;
let (code, stdout, stderr) = run_git(
&git_root,
&cfg_parts,
&executable_git_config_overrides,
&args,
)?;
let (mut applied_paths, mut skipped_paths, mut conflicted_paths) =
parse_git_apply_output(&stdout, &stderr);
@@ -148,7 +164,12 @@ fn write_temp_patch(diff: &str) -> io::Result<(tempfile::TempDir, PathBuf)> {
Ok((dir, path))
}
fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32, String, String)> {
fn run_git(
cwd: &Path,
git_cfg: &[String],
executable_git_config_overrides: &[GitConfigOverride],
args: &[String],
) -> io::Result<(i32, String, String)> {
let mut cmd = std::process::Command::new("git");
for p in git_cfg {
cmd.arg(p);
@@ -156,6 +177,7 @@ fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32,
for a in args {
cmd.arg(a);
}
add_executable_git_config_overrides(&mut cmd, executable_git_config_overrides);
let out = cmd.current_dir(cwd).output()?;
let code = out.status.code().unwrap_or(-1);
let stdout = String::from_utf8_lossy(&out.stdout).into_owned();
@@ -163,6 +185,30 @@ fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32,
Ok((code, stdout, stderr))
}
fn safe_git_config_parts() -> Vec<String> {
vec![
"-c".to_string(),
format!("core.hooksPath={DISABLED_HOOKS_PATH}"),
"-c".to_string(),
FsmonitorOverride::Disabled.git_config_arg().to_string(),
]
}
fn add_executable_git_config_overrides(
command: &mut std::process::Command,
config_overrides: &[GitConfigOverride],
) {
if config_overrides.is_empty() {
return;
}
command.env("GIT_CONFIG_COUNT", config_overrides.len().to_string());
for (index, (key, value)) in config_overrides.iter().enumerate() {
command
.env(format!("GIT_CONFIG_KEY_{index}"), key)
.env(format!("GIT_CONFIG_VALUE_{index}"), value);
}
}
fn quote_shell(s: &str) -> String {
let simple = s
.chars()
@@ -329,14 +375,16 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> {
if existing.is_empty() {
return Ok(());
}
let mut cmd = std::process::Command::new("git");
cmd.arg("add");
cmd.arg("--");
for p in &existing {
cmd.arg(OsStr::new(p));
}
let out = cmd.current_dir(git_root).output()?;
let _code = out.status.code().unwrap_or(-1);
let mut args = vec!["add".to_string(), "--".to_string()];
args.extend(existing);
let config_parts = safe_git_config_parts();
let executable_git_config_overrides = configured_executable_git_config_overrides(git_root)?;
let (_code, _, _) = run_git(
git_root,
&config_parts,
&executable_git_config_overrides,
&args,
)?;
// We do not hard fail staging; best-effort is OK. Return Ok even on non-zero.
Ok(())
}
@@ -633,6 +681,33 @@ mod tests {
.replace("\r\n", "\n")
}
fn configure_clean_filter(root: &Path, tracked_path: &str) {
std::fs::write(
root.join(".gitattributes"),
format!("{tracked_path} filter=x=y\n"),
)
.expect("write attributes");
let (add_code, _, add_err) = run(root, &["git", "add", ".gitattributes"]);
assert_eq!(add_code, 0, "add attributes: {add_err}");
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "attributes"]);
assert_eq!(commit_code, 0, "commit attributes: {commit_err}");
let (config_code, _, config_err) = run(
root,
&[
"git",
"config",
"filter.x=y.clean",
"git config codex.filterran true && git hash-object --stdin",
],
);
assert_eq!(config_code, 0, "configure filter: {config_err}");
}
fn configured_filter_ran(root: &Path) -> bool {
let (code, _, _) = run(root, &["git", "config", "--get", "codex.filterran"]);
code == 0
}
#[test]
fn extract_paths_handles_quoted_headers() {
let diff = "diff --git \"a/hello world.txt\" \"b/hello world.txt\"\nnew file mode 100644\n--- /dev/null\n+++ b/hello world.txt\n@@ -0,0 +1 @@\n+hi\n";
@@ -844,4 +919,43 @@ diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1
"non-preflight path should not use --check"
);
}
#[test]
fn apply_ignores_configured_clean_filter() {
let _g = env_lock().lock().unwrap();
let repo = init_repo();
let root = repo.path();
std::fs::write(root.join("file.txt"), "orig\n").expect("write file");
let (add_code, _, add_err) = run(root, &["git", "add", "file.txt"]);
assert_eq!(add_code, 0, "add file: {add_err}");
let (commit_code, _, commit_err) = run(root, &["git", "commit", "-m", "seed"]);
assert_eq!(commit_code, 0, "commit file: {commit_err}");
configure_clean_filter(root, "file.txt");
let diff = "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,1 +1,1 @@\n-orig\n+next\n";
let preflight_req = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: false,
preflight: true,
};
let preflight = apply_git_patch(&preflight_req).expect("preflight apply");
assert_eq!(preflight.exit_code, 0, "preflight apply succeeded");
assert!(!configured_filter_ran(root));
let apply_req = ApplyGitRequest {
cwd: root.to_path_buf(),
diff: diff.to_string(),
revert: false,
preflight: false,
};
let applied = apply_git_patch(&apply_req).expect("apply");
assert_eq!(
applied.exit_code, 0,
"apply succeeded\nstdout:\n{}\nstderr:\n{}",
applied.stdout, applied.stderr
);
assert!(!configured_filter_ran(root));
assert_eq!(read_file_normalized(&root.join("file.txt")), "next\n");
}
}
codex-rs/git-utils/src/info.rs
+170 -4
View File
@@ -16,6 +16,10 @@ use tokio::time::timeout;
use ts_rs::TS;
use crate::GitSha;
use crate::safe_git::DISABLED_HOOKS_PATH;
use crate::safe_git::EXECUTABLE_GIT_CONFIG_PATTERN;
use crate::safe_git::GitConfigOverride;
use crate::safe_git::executable_git_config_overrides_from_output;
/// Return `true` if the project folder specified by the `Config` is inside a
/// Git repository.
@@ -57,7 +61,6 @@ pub async fn get_git_repo_root_with_fs(
/// Timeout for git commands to prevent freezing on large repositories
const GIT_COMMAND_TIMEOUT: TokioDuration = TokioDuration::from_secs(5);
const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
#[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, TS)]
pub struct GitInfo {
@@ -433,16 +436,20 @@ async fn run_git_command_with_timeout_from(
cwd: &Path,
fsmonitor: crate::FsmonitorOverride,
) -> Option<std::process::Output> {
let executable_git_config_overrides =
configured_executable_git_config_overrides_from(git, cwd).await?;
let disabled_hooks = format!("core.hooksPath={DISABLED_HOOKS_PATH}");
let mut command = Command::new(git);
command
.env("GIT_OPTIONAL_LOCKS", "0")
// Keep internal Git commands independent of repository-selected hooks
// and fsmonitor helpers while preserving built-in fsmonitor acceleration.
.args(["-c", &format!("core.hooksPath={DISABLED_HOOKS_PATH}")])
// and executable Git helpers while preserving built-in fsmonitor acceleration.
.args(["-c", &disabled_hooks])
.args(["-c", fsmonitor.git_config_arg()])
.args(args)
.current_dir(cwd)
.kill_on_drop(true);
add_executable_git_config_overrides(&mut command, &executable_git_config_overrides);
command.args(args);
let result = timeout(GIT_COMMAND_TIMEOUT, command.output()).await;
match result {
@@ -451,6 +458,51 @@ async fn run_git_command_with_timeout_from(
}
}
async fn configured_executable_git_config_overrides_from(
git: &Path,
cwd: &Path,
) -> Option<Vec<GitConfigOverride>> {
let mut command = Command::new(git);
command
.args([
"config",
"--null",
"--name-only",
"--get-regexp",
EXECUTABLE_GIT_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;
}
Some(executable_git_config_overrides_from_output(&output.stdout))
}
fn add_executable_git_config_overrides(
command: &mut Command,
config_overrides: &[GitConfigOverride],
) {
if config_overrides.is_empty() {
return;
}
command.env("GIT_CONFIG_COUNT", config_overrides.len().to_string());
for (index, (key, value)) in config_overrides.iter().enumerate() {
command
.env(format!("GIT_CONFIG_KEY_{index}"), key)
.env(format!("GIT_CONFIG_VALUE_{index}"), value);
}
}
async fn get_git_remotes(cwd: &Path) -> Option<Vec<String>> {
let output = run_git_command_with_timeout(&["remote"], cwd).await?;
if !output.status.success() {
@@ -906,6 +958,68 @@ mod tests {
use pretty_assertions::assert_eq;
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
use std::path::Path;
async fn run_git(repo_path: &Path, args: &[&str]) {
let output = Command::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
}
async fn configure_clean_filter(repo_path: &Path, tracked_path: &str) {
std::fs::write(
repo_path.join(".gitattributes"),
format!("{tracked_path} 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;
let tracked_file = repo_path.join(tracked_path);
let contents = std::fs::read_to_string(&tracked_file).expect("read tracked file");
std::thread::sleep(std::time::Duration::from_secs(1));
std::fs::write(tracked_file, contents).expect("refresh tracked file");
}
async fn configured_filter_ran(repo_path: &Path) -> bool {
let output = Command::new("git")
.args(["config", "--get", "codex.filterran"])
.current_dir(repo_path)
.output()
.await
.expect("read filter marker");
output.status.success()
}
#[test]
fn canonicalize_git_remote_url_normalizes_github_variants() {
@@ -944,6 +1058,54 @@ mod tests {
}
}
#[tokio::test]
async fn get_has_changes_ignores_configured_clean_filter() {
let temp_dir = tempfile::tempdir().expect("create temp dir");
let repo_path = create_test_git_repo(&temp_dir).await;
configure_clean_filter(&repo_path, "test.txt").await;
assert_eq!(get_has_changes(&repo_path).await, Some(false));
assert!(!configured_filter_ran(&repo_path).await);
}
#[tokio::test]
async fn git_diff_to_remote_ignores_configured_clean_filter() {
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");
run_git(
temp_dir.path(),
&["init", "--bare", remote_path.to_str().unwrap()],
)
.await;
run_git(
&repo_path,
&["remote", "add", "origin", remote_path.to_str().unwrap()],
)
.await;
let branch_output = Command::new("git")
.args(["rev-parse", "--abbrev-ref", "HEAD"])
.current_dir(&repo_path)
.output()
.await
.expect("read branch");
assert!(branch_output.status.success(), "read branch");
let branch = String::from_utf8(branch_output.stdout)
.expect("branch utf8")
.trim()
.to_string();
run_git(&repo_path, &["push", "-u", "origin", &branch]).await;
configure_clean_filter(&repo_path, "test.txt").await;
run_git(&repo_path, &["push", "origin", &branch]).await;
let state = git_diff_to_remote(&repo_path)
.await
.expect("collect working tree state");
assert!(state.diff.is_empty());
assert!(!configured_filter_ran(&repo_path).await);
}
#[cfg(unix)]
#[tokio::test]
async fn fsmonitor_override_rejects_configured_helper() {
@@ -994,6 +1156,9 @@ mod tests {
"config --null --get core.fsmonitor".to_string(),
"config --null --type=bool --fixed-value --get core.fsmonitor /tmp/fsmonitor-helper"
.to_string(),
format!(
"config --null --name-only --get-regexp {EXECUTABLE_GIT_CONFIG_PATTERN}"
),
format!("-c {disabled_hooks} -c core.fsmonitor=false status --porcelain"),
]
);
@@ -1081,6 +1246,7 @@ mod tests {
vec![
"config --null --get core.fsmonitor".to_string(),
"version --build-options".to_string(),
format!("config --null --name-only --get-regexp {EXECUTABLE_GIT_CONFIG_PATTERN}"),
format!("-c {disabled_hooks} -c core.fsmonitor=true status --porcelain"),
]
);
codex-rs/git-utils/src/lib.rs
+1
View File
@@ -6,6 +6,7 @@ mod fsmonitor;
mod info;
mod operations;
mod platform;
mod safe_git;
pub use apply::ApplyGitRequest;
pub use apply::ApplyGitResult;
codex-rs/git-utils/src/operations.rs
+20 -2
View File
@@ -5,8 +5,9 @@ use std::path::PathBuf;
use std::process::Command;
use crate::GitToolingError;
const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
use crate::safe_git::DISABLED_HOOKS_PATH;
use crate::safe_git::GitConfigOverride;
use crate::safe_git::configured_executable_git_config_overrides;
pub(crate) fn ensure_git_repository(path: &Path) -> Result<(), GitToolingError> {
match run_git_for_stdout(
@@ -109,6 +110,7 @@ where
for arg in iterator {
args_vec.push(OsString::from(arg.as_ref()));
}
let executable_git_config_overrides = configured_executable_git_config_overrides(dir)?;
let command_string = build_command_string(&args_vec);
let mut command = Command::new("git");
command.current_dir(dir);
@@ -117,6 +119,7 @@ where
command.env(key, value);
}
}
add_executable_git_config_overrides(&mut command, &executable_git_config_overrides);
command.args(&args_vec);
let output = command.output()?;
if !output.status.success() {
@@ -133,6 +136,21 @@ where
})
}
fn add_executable_git_config_overrides(
command: &mut Command,
config_overrides: &[GitConfigOverride],
) {
if config_overrides.is_empty() {
return;
}
command.env("GIT_CONFIG_COUNT", config_overrides.len().to_string());
for (index, (key, value)) in config_overrides.iter().enumerate() {
command
.env(format!("GIT_CONFIG_KEY_{index}"), key)
.env(format!("GIT_CONFIG_VALUE_{index}"), value);
}
}
fn build_command_string(args: &[OsString]) -> String {
if args.is_empty() {
return "git".to_string();
codex-rs/git-utils/src/safe_git.rs
+98
View File
@@ -0,0 +1,98 @@
use std::collections::BTreeSet;
use std::io;
use std::path::Path;
use std::process::Command;
pub(crate) const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
pub(crate) const EXECUTABLE_GIT_CONFIG_PATTERN: &str =
r"^(filter\..*\.(clean|smudge|process|required)|merge\..*\.driver)$";
pub(crate) type GitConfigOverride = (String, String);
pub(crate) fn configured_executable_git_config_overrides(
cwd: &Path,
) -> io::Result<Vec<GitConfigOverride>> {
let output = Command::new("git")
.args([
"config",
"--null",
"--name-only",
"--get-regexp",
EXECUTABLE_GIT_CONFIG_PATTERN,
])
.current_dir(cwd)
.output()?;
if output
.status
.code()
.is_some_and(|code| code == 0 || code == 1)
{
return Ok(executable_git_config_overrides_from_output(&output.stdout));
}
Err(io::Error::other(format!(
"git config probe failed with status {}: {}",
output.status,
String::from_utf8_lossy(&output.stderr).trim()
)))
}
pub(crate) fn executable_git_config_overrides_from_output(stdout: &[u8]) -> Vec<GitConfigOverride> {
let mut filter_drivers = BTreeSet::new();
let mut merge_drivers = BTreeSet::new();
for key in stdout
.split(|byte| *byte == 0)
.filter(|key| !key.is_empty())
.filter_map(|key| std::str::from_utf8(key).ok())
{
if let Some(driver) = key
.strip_suffix(".clean")
.or_else(|| key.strip_suffix(".smudge"))
.or_else(|| key.strip_suffix(".process"))
.or_else(|| key.strip_suffix(".required"))
{
filter_drivers.insert(driver.to_string());
} else if key.starts_with("merge.") && key.ends_with(".driver") {
merge_drivers.insert(key.to_string());
}
}
filter_drivers
.into_iter()
.flat_map(|driver| {
[
(format!("{driver}.clean"), String::new()),
(format!("{driver}.smudge"), String::new()),
(format!("{driver}.process"), String::new()),
(format!("{driver}.required"), "false".to_string()),
]
})
.chain(
merge_drivers
.into_iter()
.map(|driver| (driver, String::new())),
)
.collect()
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn executable_git_config_overrides_clear_filters_and_merge_drivers() {
let output = b"filter.x=y.clean\0filter.x=y.required\0merge.pwn.driver\0";
assert_eq!(
executable_git_config_overrides_from_output(output),
vec![
("filter.x=y.clean".to_string(), String::new()),
("filter.x=y.smudge".to_string(), String::new()),
("filter.x=y.process".to_string(), String::new()),
("filter.x=y.required".to_string(), "false".to_string()),
("merge.pwn.driver".to_string(), String::new()),
]
);
}
}