openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-03 14:59:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(git): bind guarded patch operations

Browse Source
This commit is contained in:
Chris Bookholt
2026-07-03 18:36:13 -07:00
parent 9c2e1342a4
commit 463eed4400
8 changed files with 499 additions and 179 deletions
Download Patch File Download Diff File Expand all files Collapse all files

190
codex-rs/git-utils/src/apply.rs
View File

@@ -10,13 +10,11 @@ use std::io;
use std::path::Path;
use std::path::PathBuf;
use crate::FsmonitorOverride;
use crate::apply_output::parse_git_apply_output;
use crate::git_command::GitRunner;
use crate::git_config_sources::ensure_no_worktree_config_sources;
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::guarded_config::GuardedGitConfig;
use crate::patch_paths::extract_effective_paths_from_patch_guarded;
use crate::patch_paths::stage_effective_paths_guarded;
#[cfg(test)]
use crate::safe_git::isolate_git_command_environment;
@@ -46,26 +44,26 @@ pub struct ApplyGitResult {
/// When [`ApplyGitRequest::preflight`] is `true`, this behaves like `git apply --check` and
/// leaves the working tree untouched while still parsing the command output for diagnostics.
pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
let cfg_parts = configured_git_config_parts();
// Construct from the caller's exact route before canonicalization so
// repository authority retains lexical enclosing worktrees.
let git = GitRunner::for_cwd_io(&req.cwd)?;
let mut cfg_parts = configured_git_config_parts();
ensure_no_worktree_config_sources(&git, &req.cwd, &cfg_parts)?;
let requested_cwd = std::fs::canonicalize(&req.cwd)?;
let git_root = resolve_git_root(&git, &req.cwd, &cfg_parts)?;
if git_root != requested_cwd {
ensure_no_worktree_config_sources(&git, &git_root, &cfg_parts)?;
}
let expected_root = crate::get_git_repo_root(&requested_cwd)
.ok_or_else(|| io::Error::other("not a Git repository"))
.and_then(std::fs::canonicalize)?;
let config = GuardedGitConfig::authorize(&git, &expected_root, cfg_parts)?;
resolve_git_root(&config, &requested_cwd)?;
// 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(&git, &git_root, &patch_path, req.revert, &cfg_parts)?;
cfg_parts.extend(safe_git_config_parts());
let patch_paths = extract_effective_paths_from_patch_guarded(&config, &patch_path, req.revert)?;
if req.revert && !req.preflight {
// Stage WT paths first to avoid index mismatch on revert.
stage_effective_paths(&git, &git_root, &patch_paths, &cfg_parts)?;
stage_effective_paths_guarded(&config, &patch_paths)?;
}
// Build git args
@@ -83,8 +81,8 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
check_args.push("-R".to_string());
}
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, &git_root, &cfg_parts, &check_args)?;
let rendered = config.render_command_for_log(&check_args)?;
let (c_code, c_out, c_err) = run_guarded_apply(&config, &check_args)?;
let (mut applied_paths, mut skipped_paths, mut conflicted_paths) =
parse_git_apply_output(&c_out, &c_err);
applied_paths.sort();
@@ -104,8 +102,8 @@ 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, &git_root, &cfg_parts, &args)?;
let cmd_for_log = config.render_command_for_log(&args)?;
let (code, stdout, stderr) = run_guarded_apply(&config, &args)?;
let (mut applied_paths, mut skipped_paths, mut conflicted_paths) =
parse_git_apply_output(&stdout, &stderr);
@@ -127,18 +125,10 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
})
}
fn resolve_git_root(
git: &GitRunner,
cwd: &Path,
git_config_args: &[String],
) -> io::Result<PathBuf> {
let requested_cwd = std::fs::canonicalize(cwd)?;
let mut command = git.command_for_cwd(&requested_cwd)?;
command
.args(git_config_args)
.arg("rev-parse")
.arg("--show-toplevel");
let out = git.output(command)?;
fn resolve_git_root(config: &GuardedGitConfig<'_>, requested_cwd: &Path) -> io::Result<PathBuf> {
let mut command = config.rev_parse_command()?;
command.arg("--show-toplevel");
let out = command.output()?;
let code = out.status.code().unwrap_or(-1);
if code != 0 {
return Err(io::Error::other(format!(
@@ -149,18 +139,7 @@ fn resolve_git_root(
}
let reported_root = PathBuf::from(String::from_utf8_lossy(&out.stdout).trim());
let root = std::fs::canonicalize(&reported_root)?;
let expected_root = crate::get_git_repo_root(&requested_cwd)
.ok_or_else(|| {
io::Error::new(
io::ErrorKind::PermissionDenied,
format!(
"refusing to apply a patch because Git resolved worktree {} without a .git marker above requested cwd {}",
root.display(),
requested_cwd.display()
),
)
})
.and_then(std::fs::canonicalize)?;
let expected_root = config.canonical_root();
if root != expected_root {
return Err(io::Error::new(
io::ErrorKind::PermissionDenied,
@@ -197,60 +176,30 @@ pub(crate) fn write_temp_patch(diff: &str) -> io::Result<(tempfile::TempDir, Pat
Ok((dir, path))
}
pub(crate) fn run_git(
git: &GitRunner,
cwd: &Path,
git_cfg: &[String],
fn run_guarded_apply(
config: &GuardedGitConfig<'_>,
args: &[String],
) -> io::Result<(i32, String, String)> {
let mut cmd = git.command_for_cwd(cwd)?;
for p in git_cfg {
cmd.arg(p);
let Some((subcommand, args)) = args.split_first() else {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"missing guarded Git subcommand",
));
};
if subcommand != "apply" {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"unexpected guarded Git subcommand",
));
}
for a in args {
cmd.arg(a);
}
let out = git.output(cmd)?;
let code = out.status.code().unwrap_or(-1);
let stdout = String::from_utf8_lossy(&out.stdout).into_owned();
let stderr = String::from_utf8_lossy(&out.stderr).into_owned();
Ok((code, stdout, stderr))
}
pub(crate) 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 quote_shell(s: &str) -> String {
let simple = s
.chars()
.all(|c| c.is_ascii_alphanumeric() || "-_.:/@%+".contains(c));
if simple {
s.to_string()
} else {
format!("'{}'", s.replace('\'', "'\\''"))
}
}
fn render_command_for_log(cwd: &Path, git_cfg: &[String], args: &[String]) -> String {
let mut parts: Vec<String> = Vec::new();
parts.push("git".to_string());
for a in git_cfg {
parts.push(quote_shell(a));
}
for a in args {
parts.push(quote_shell(a));
}
format!(
"(cd {} && {})",
quote_shell(&cwd.display().to_string()),
parts.join(" ")
)
let mut command = config.apply_command()?;
command.args(args);
let output = command.output()?;
Ok((
output.status.code().unwrap_or(-1),
String::from_utf8_lossy(&output.stdout).into_owned(),
String::from_utf8_lossy(&output.stderr).into_owned(),
))
}
#[cfg(all(test, unix))]
@@ -260,6 +209,8 @@ mod transport_tests;
#[cfg(test)]
mod tests {
use super::*;
use crate::guarded_config::config_source_authorization_count;
use crate::guarded_config::reset_config_source_authorization_count;
use std::ffi::OsStr;
use std::path::Path;
use std::sync::Mutex;
@@ -353,7 +304,9 @@ mod tests {
revert: false,
preflight: false,
};
reset_config_source_authorization_count();
let r = apply_git_patch(&req).expect("run apply");
assert_eq!(config_source_authorization_count(), 1);
assert_eq!(r.exit_code, 0, "exit code 0");
// File exists now
assert!(root.join("hello.txt").exists());
@@ -482,6 +435,41 @@ mod tests {
assert!(!marker.exists(), "enclosing Git shim must not run");
}
#[cfg(unix)]
#[test]
fn apply_uses_physical_repository_for_symlinked_nested_cwd() {
let _g = env_lock().lock().unwrap();
let fixture = tempfile::tempdir().expect("fixture");
let outer = fixture.path().join("outer");
let target = fixture.path().join("target");
let nested = target.join("nested");
std::fs::create_dir_all(&outer).expect("outer");
std::fs::create_dir_all(&nested).expect("nested");
let (outer_init, _, outer_error) = run(&outer, &["git", "init", "-q"]);
assert_eq!(outer_init, 0, "outer init: {outer_error}");
let (target_init, _, target_error) = run(&target, &["git", "init", "-q"]);
assert_eq!(target_init, 0, "target init: {target_error}");
let lexical_cwd = outer.join("linked-nested");
std::os::unix::fs::symlink(&nested, &lexical_cwd).expect("nested cwd symlink");
reset_config_source_authorization_count();
let result = apply_git_patch(&ApplyGitRequest {
cwd: lexical_cwd,
diff: "diff --git a/physical.txt b/physical.txt\nnew file mode 100644\n--- /dev/null\n+++ b/physical.txt\n@@ -0,0 +1 @@\n+physical\n".to_string(),
revert: false,
preflight: false,
})
.expect("apply through symlinked nested cwd");
assert_eq!(result.exit_code, 0);
assert_eq!(config_source_authorization_count(), 1);
assert_eq!(
read_file_normalized(&target.join("physical.txt")),
"physical\n"
);
assert!(!outer.join("physical.txt").exists());
}
#[test]
fn apply_modify_conflict() {
let _g = env_lock().lock().unwrap();
@@ -540,7 +528,9 @@ mod tests {
revert: false,
preflight: false,
};
reset_config_source_authorization_count();
let res_apply = apply_git_patch(&apply_req).expect("apply ok");
assert_eq!(config_source_authorization_count(), 1);
assert_eq!(res_apply.exit_code, 0, "forward apply succeeded");
let after_apply = read_file_normalized(&root.join("file.txt"));
assert_eq!(after_apply, "ORIG\n");
@@ -552,7 +542,9 @@ mod tests {
revert: true,
preflight: false,
};
reset_config_source_authorization_count();
let res_revert = apply_git_patch(&revert_req).expect("revert ok");
assert_eq!(config_source_authorization_count(), 1);
assert_eq!(res_revert.exit_code, 0, "revert apply succeeded");
let after_revert = read_file_normalized(&root.join("file.txt"));
assert_eq!(after_revert, "orig\n");
@@ -589,7 +581,9 @@ mod tests {
revert: true,
preflight: true,
};
reset_config_source_authorization_count();
let res_preflight = apply_git_patch(&preflight_req).expect("preflight ok");
assert_eq!(config_source_authorization_count(), 1);
assert_eq!(res_preflight.exit_code, 0, "revert preflight succeeded");
let (_code_after, staged_after, _stderr_after) =
run(root, &["git", "diff", "--cached", "--name-only"]);
@@ -619,7 +613,9 @@ diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1
revert: false,
preflight: true,
};
reset_config_source_authorization_count();
let r1 = apply_git_patch(&req1).expect("preflight apply");
assert_eq!(config_source_authorization_count(), 1);
assert_ne!(r1.exit_code, 0, "preflight reports failure");
assert!(
!root.join("ok.txt").exists(),
@@ -637,7 +633,9 @@ diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1
revert: false,
preflight: false,
};
reset_config_source_authorization_count();
let r2 = apply_git_patch(&req2).expect("direct apply");
assert_eq!(config_source_authorization_count(), 1);
assert_ne!(r2.exit_code, 0, "apply is expected to fail overall");
assert!(
!r2.cmd_for_log.contains("--check"),
@@ -664,8 +662,10 @@ diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1
assert_eq!(config_code, 0, "configure core.worktree: {config_err}");
let git = GitRunner::for_cwd_io(&attacker).expect("trusted Git");
let guarded = GuardedGitConfig::authorize(&git, &attacker, Vec::new())
.expect("authorize attacker repository config");
let error =
resolve_git_root(&git, &attacker, &[]).expect_err("reject redirected worktree");
resolve_git_root(&guarded, &attacker).expect_err("reject redirected worktree");
assert_eq!(error.kind(), io::ErrorKind::PermissionDenied);
assert!(error.to_string().contains("instead of expected worktree"));
}

4
codex-rs/git-utils/src/git_command.rs
View File

@@ -116,6 +116,10 @@ impl GitRunner {
.ensure_config_source_is_not_worktree_controlled(path, description)
}
pub(crate) fn ensure_active_worktree_root(&self, root: &Path) -> io::Result<()> {
self.authority.ensure_active_worktree_root(root)
}
pub(crate) fn config_environment_value(&self, name: &str) -> Option<&OsStr> {
self.config_environment.value(name)
}

228
codex-rs/git-utils/src/guarded_config.rs Normal file
View File

@@ -0,0 +1,228 @@
use std::ffi::OsStr;
use std::io;
use std::path::Path;
use std::path::PathBuf;
use crate::FsmonitorOverride;
use crate::git_command::GitCommand;
use crate::git_command::GitRunner;
use crate::git_config_sources::ensure_no_worktree_config_sources;
use crate::safe_git::DISABLED_HOOKS_PATH;
/// Proof that one exact Git config invocation has no worktree-controlled
/// source routes for one runner and canonical repository root.
///
/// The capability deliberately owns the ordered base arguments and cannot be
/// cloned or rebound to another runner, root, or command environment.
pub(crate) struct ValidatedConfigSources<'git> {
git: &'git GitRunner,
canonical_root: PathBuf,
base_config_args: Box<[String]>,
}
impl<'git> ValidatedConfigSources<'git> {
fn authorize(
git: &'git GitRunner,
canonical_root: &Path,
base_config_args: Vec<String>,
) -> io::Result<Self> {
#[cfg(test)]
CONFIG_SOURCE_AUTHORIZATION_COUNT.with(|count| count.set(count.get() + 1));
validate_base_config_args(&base_config_args)?;
let canonical_root = std::fs::canonicalize(canonical_root)?;
git.ensure_active_worktree_root(&canonical_root)?;
ensure_no_worktree_config_sources(git, &canonical_root, &base_config_args)?;
Ok(Self {
git,
canonical_root,
base_config_args: base_config_args.into_boxed_slice(),
})
}
}
fn validate_base_config_args(args: &[String]) -> io::Result<()> {
let mut pairs = args.chunks_exact(2);
for pair in &mut pairs {
let Some((key, _value)) = pair[1].split_once('=') else {
return Err(invalid_base_config_args());
};
if pair[0] != "-c" || key.is_empty() {
return Err(invalid_base_config_args());
}
}
if !pairs.remainder().is_empty() {
return Err(invalid_base_config_args());
}
Ok(())
}
fn invalid_base_config_args() -> io::Error {
io::Error::new(
io::ErrorKind::InvalidInput,
"guarded Git base config must contain only ordered -c key=value pairs",
)
}
/// Operation-owned Git configuration capability.
///
/// All operation children are rooted at the authorized repository, inherit
/// the exact frozen base invocation, and receive fixed library safety scalars.
pub(crate) struct GuardedGitConfig<'git> {
sources: ValidatedConfigSources<'git>,
}
#[derive(Clone, Copy)]
enum BoundSubcommand {
AddLiteralPathspecs,
Apply,
RevParse,
}
impl BoundSubcommand {
fn as_str(self) -> &'static str {
match self {
Self::AddLiteralPathspecs => "add",
Self::Apply => "apply",
Self::RevParse => "rev-parse",
}
}
fn append_to(self, command: &mut GitCommand) {
if matches!(self, Self::AddLiteralPathspecs) {
command.arg("--literal-pathspecs");
}
command.arg(self.as_str());
}
}
/// A command whose runner, root, config invocation, and fixed subcommand are
/// inseparably bound to one operation capability.
pub(crate) struct GuardedGitCommand<'operation, 'git> {
operation: &'operation GuardedGitConfig<'git>,
inner: GitCommand,
}
impl GuardedGitCommand<'_, '_> {
pub(crate) fn arg(&mut self, arg: impl AsRef<OsStr>) -> &mut Self {
self.inner.arg(arg);
self
}
pub(crate) fn args<I, S>(&mut self, args: I) -> &mut Self
where
I: IntoIterator<Item = S>,
S: AsRef<OsStr>,
{
self.inner.args(args);
self
}
pub(crate) fn output(self) -> io::Result<std::process::Output> {
self.operation.sources.git.output(self.inner)
}
}
impl<'git> GuardedGitConfig<'git> {
pub(crate) fn authorize(
git: &'git GitRunner,
canonical_root: &Path,
base_config_args: Vec<String>,
) -> io::Result<Self> {
Ok(Self {
sources: ValidatedConfigSources::authorize(git, canonical_root, base_config_args)?,
})
}
pub(crate) fn canonical_root(&self) -> &Path {
&self.sources.canonical_root
}
pub(crate) fn apply_command(&self) -> io::Result<GuardedGitCommand<'_, 'git>> {
self.guarded_command(BoundSubcommand::Apply)
}
pub(crate) fn literal_add_command(&self) -> io::Result<GuardedGitCommand<'_, 'git>> {
self.guarded_command(BoundSubcommand::AddLiteralPathspecs)
}
pub(crate) fn rev_parse_command(&self) -> io::Result<GuardedGitCommand<'_, 'git>> {
self.guarded_command(BoundSubcommand::RevParse)
}
fn guarded_command(
&self,
subcommand: BoundSubcommand,
) -> io::Result<GuardedGitCommand<'_, 'git>> {
let mut command = self
.sources
.git
.command_for_cwd(&self.sources.canonical_root)?;
command.args(&self.sources.base_config_args);
append_safe_scalar_overrides(&mut command);
subcommand.append_to(&mut command);
Ok(GuardedGitCommand {
operation: self,
inner: command,
})
}
pub(crate) fn render_command_for_log(&self, args: &[String]) -> io::Result<String> {
let mut parts = vec!["git".to_string()];
parts.extend(self.sources.base_config_args.iter().cloned());
parts.extend(safe_scalar_override_args());
parts.extend(args.iter().cloned());
Ok(format!(
"(cd {} && {})",
quote_shell(&self.sources.canonical_root.display().to_string()),
parts
.into_iter()
.map(|part| quote_shell(&part))
.collect::<Vec<_>>()
.join(" ")
))
}
}
fn append_safe_scalar_overrides(command: &mut GitCommand) {
command.args(safe_scalar_override_args());
}
fn safe_scalar_override_args() -> [String; 4] {
[
"-c".to_string(),
format!("core.hooksPath={DISABLED_HOOKS_PATH}"),
"-c".to_string(),
FsmonitorOverride::Disabled.git_config_arg().to_string(),
]
}
fn quote_shell(value: &str) -> String {
let simple = value
.chars()
.all(|character| character.is_ascii_alphanumeric() || "-_.:/@%+".contains(character));
if simple {
value.to_string()
} else {
format!("'{}'", value.replace('\'', "'\\''"))
}
}
#[cfg(test)]
thread_local! {
static CONFIG_SOURCE_AUTHORIZATION_COUNT: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
#[cfg(test)]
pub(crate) fn reset_config_source_authorization_count() {
CONFIG_SOURCE_AUTHORIZATION_COUNT.with(|count| count.set(0));
}
#[cfg(test)]
pub(crate) fn config_source_authorization_count() -> usize {
CONFIG_SOURCE_AUTHORIZATION_COUNT.with(std::cell::Cell::get)
}
#[cfg(test)]
#[path = "guarded_config_tests.rs"]
mod tests;

56
codex-rs/git-utils/src/guarded_config_tests.rs Normal file
View File

@@ -0,0 +1,56 @@
use super::*;
use crate::safe_git::isolate_git_command_environment;
use pretty_assertions::assert_eq;
fn run_git(cwd: &Path, args: &[&str]) {
let mut command = std::process::Command::new("git");
isolate_git_command_environment(&mut command);
let output = command.current_dir(cwd).args(args).output().expect("Git");
assert!(
output.status.success(),
"git {args:?}: {}",
String::from_utf8_lossy(&output.stderr)
);
}
#[test]
fn authorization_rejects_untyped_global_config_arguments() {
let repo = tempfile::tempdir().expect("repo");
run_git(repo.path(), &["init", "-q"]);
let git = GitRunner::for_cwd_io(repo.path()).expect("runner");
for args in [
vec!["--config-env=include.path=UNSNAPSHOTTED".to_string()],
vec!["--git-dir=elsewhere".to_string()],
vec!["-c".to_string(), "missing-value".to_string()],
vec!["-c".to_string()],
] {
let error = match GuardedGitConfig::authorize(&git, repo.path(), args) {
Ok(_) => panic!("accepted untyped base config arguments"),
Err(error) => error,
};
assert_eq!(error.kind(), io::ErrorKind::InvalidInput);
}
}
#[test]
fn authorization_rejects_another_runner_repository_and_nested_or_prefix_roots() {
let fixture = tempfile::tempdir().expect("fixture");
let outer = fixture.path().join("repo");
let nested = outer.join("nested");
let prefix = fixture.path().join("repo-evil");
std::fs::create_dir_all(&nested).expect("nested");
std::fs::create_dir_all(&prefix).expect("prefix");
run_git(&outer, &["init", "-q"]);
run_git(&nested, &["init", "-q"]);
run_git(&prefix, &["init", "-q"]);
let git = GitRunner::for_cwd_io(&outer).expect("outer runner");
for wrong_root in [&nested, &prefix] {
let error = match GuardedGitConfig::authorize(&git, wrong_root, Vec::new()) {
Ok(_) => panic!("accepted another repository root"),
Err(error) => error,
};
assert_eq!(error.kind(), io::ErrorKind::PermissionDenied);
}
}

1
codex-rs/git-utils/src/lib.rs
View File

@@ -9,6 +9,7 @@ mod git_config;
mod git_config_environment;
mod git_config_sources;
mod git_executable;
mod guarded_config;
mod info;
mod local_only;
mod operations;

174
codex-rs/git-utils/src/patch_paths.rs
View File

@@ -4,28 +4,26 @@ use std::io;
use std::path::Path;
use std::path::PathBuf;
use crate::apply::run_git;
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::git_config_sources::ensure_no_worktree_config_sources;
use crate::guarded_config::GuardedGitConfig;
/// Extract paths with Git from a cwd whose config sources have already been
/// authorized for `git_config_args`.
pub(crate) fn extract_effective_paths_from_patch(
git: &GitRunner,
authorized_cwd: &Path,
/// Extract effective patch paths through a bound operation configuration.
pub(crate) fn extract_effective_paths_from_patch_guarded(
config: &GuardedGitConfig<'_>,
patch_path: &Path,
revert: bool,
git_config_args: &[String],
) -> io::Result<Vec<String>> {
let forward_paths =
git_apply_numstat_paths(git, authorized_cwd, patch_path, revert, git_config_args)?;
// `git apply --numstat` reports only the destination of a rename. Parse the
// opposite orientation too so both endpoints are included in the result.
let reverse_paths =
git_apply_numstat_paths(git, authorized_cwd, patch_path, !revert, git_config_args)?;
let forward_paths = git_apply_numstat_paths_guarded(config, patch_path, revert)?;
let reverse_paths = git_apply_numstat_paths_guarded(config, patch_path, !revert)?;
normalize_effective_patch_paths(forward_paths, reverse_paths)
}
fn normalize_effective_patch_paths(
forward_paths: Vec<String>,
reverse_paths: Vec<String>,
) -> io::Result<Vec<String>> {
if forward_paths.len() != reverse_paths.len() {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
@@ -46,6 +44,30 @@ pub(crate) fn extract_effective_paths_from_patch(
.collect()
}
fn git_apply_numstat_paths_guarded(
config: &GuardedGitConfig<'_>,
patch_path: &Path,
revert: bool,
) -> io::Result<Vec<String>> {
let mut command = config.apply_command()?;
command.args(["--numstat", "-z"]);
if revert {
command.arg("-R");
}
command.arg("--").arg(patch_path);
let output = command.output()?;
if !output.status.success() {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!(
"failed to parse patch paths: {}",
String::from_utf8_lossy(&output.stderr).trim()
),
));
}
parse_numstat_paths(&output.stdout)
}
/// Best-effort extraction of the paths Git would apply.
///
/// Security-sensitive callers must use the fallible internal extractor so an
@@ -66,42 +88,14 @@ fn extract_paths_from_patch_from_cwd(diff_text: &str, cwd: &Path) -> Vec<String>
let git_root = crate::get_git_repo_root(cwd)
.ok_or_else(|| io::Error::other("not a Git repository"))?;
let git_root = std::fs::canonicalize(git_root)?;
ensure_no_worktree_config_sources(&git, &git_root, &[])?;
extract_effective_paths_from_patch(&git, &git_root, &patch_path, /*revert*/ false, &[])
let config = GuardedGitConfig::authorize(&git, &git_root, Vec::new())?;
extract_effective_paths_from_patch_guarded(&config, &patch_path, /*revert*/ false)
})()
.unwrap_or_default();
drop(tmpdir);
paths
}
fn git_apply_numstat_paths(
git: &GitRunner,
authorized_cwd: &Path,
patch_path: &Path,
revert: bool,
git_config_args: &[String],
) -> io::Result<Vec<String>> {
let mut cmd = git.command_for_cwd(authorized_cwd)?;
cmd.args(git_config_args);
cmd.args(["apply", "--numstat", "-z"]);
if revert {
cmd.arg("-R");
}
cmd.arg("--").arg(patch_path);
let out = git.output(cmd)?;
if !out.status.success() {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!(
"failed to parse patch paths: {}",
String::from_utf8_lossy(&out.stderr).trim()
),
));
}
parse_numstat_paths(&out.stdout)
}
fn parse_numstat_paths(output: &[u8]) -> io::Result<Vec<String>> {
if !output.is_empty() && !output.ends_with(&[0]) {
return Err(io::Error::new(
@@ -244,30 +238,28 @@ fn invalid_windows_patch_component(component: &str) -> bool {
/// Stage only the files that actually exist on disk for the given diff.
pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> {
let git = GitRunner::for_cwd_io(git_root)?;
let git_config_args = safe_git_config_parts();
ensure_no_worktree_config_sources(&git, git_root, &git_config_args)?;
let requested_cwd = std::fs::canonicalize(git_root)?;
let canonical_root = crate::get_git_repo_root(&requested_cwd)
.ok_or_else(|| io::Error::other("not a Git repository"))
.and_then(std::fs::canonicalize)?;
let config = GuardedGitConfig::authorize(&git, &canonical_root, Vec::new())?;
let (tmpdir, patch_path) = write_temp_patch(diff)?;
let paths = extract_effective_paths_from_patch(
&git,
git_root,
&patch_path,
/*revert*/ true,
&git_config_args,
)?;
let paths =
extract_effective_paths_from_patch_guarded(&config, &patch_path, /*revert*/ true)?;
let _guard = tmpdir;
stage_effective_paths(&git, git_root, &paths, &git_config_args)
stage_effective_paths_guarded(&config, &paths)
}
pub(crate) fn stage_effective_paths(
git: &GitRunner,
git_root: &Path,
/// Preserve the existing reverse staging behavior while constructing every
/// Git child from the operation-owned configuration capability.
pub(crate) fn stage_effective_paths_guarded(
config: &GuardedGitConfig<'_>,
paths: &[String],
git_config_args: &[String],
) -> io::Result<()> {
let confined = confine_patch_paths(git, git_root, paths)?;
let confined = confine_patch_paths_guarded(config, paths)?;
let mut existing = Vec::new();
for path in confined.into_exact_leaves()? {
let joined = git_root.join(&path);
let joined = config.canonical_root().join(&path);
if let Ok(metadata) = std::fs::symlink_metadata(&joined) {
if leaf_is_traversable_directory(metadata.file_type()) {
return Err(containment_error(
@@ -280,14 +272,12 @@ pub(crate) fn stage_effective_paths(
if existing.is_empty() {
return Ok(());
}
let mut args = vec![
"--literal-pathspecs".to_string(),
"add".to_string(),
"--".to_string(),
];
let mut args = vec!["--".to_string()];
args.extend(existing);
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.
let mut command = config.literal_add_command()?;
command.args(&args);
let _output = command.output()?;
// Preserve the existing best-effort staging contract on non-zero status.
Ok(())
}
@@ -375,10 +365,29 @@ impl ConfinedPatchPaths {
}
}
#[cfg(test)]
pub(crate) fn confine_patch_paths(
git: &GitRunner,
git_root: &Path,
paths: &[String],
) -> io::Result<ConfinedPatchPaths> {
let config = GuardedGitConfig::authorize(git, git_root, Vec::new())?;
confine_patch_paths_guarded(&config, paths)
}
fn confine_patch_paths_guarded(
config: &GuardedGitConfig<'_>,
paths: &[String],
) -> io::Result<ConfinedPatchPaths> {
let canonical_root = std::fs::canonicalize(config.canonical_root())?;
let metadata_dirs = canonical_git_metadata_dirs_guarded(config)?;
confine_patch_paths_with_metadata(&canonical_root, paths, &metadata_dirs)
}
fn confine_patch_paths_with_metadata(
canonical_root: &Path,
paths: &[String],
metadata_dirs: &[PathBuf],
) -> io::Result<ConfinedPatchPaths> {
if paths.is_empty() {
return Ok(ConfinedPatchPaths {
@@ -401,8 +410,6 @@ pub(crate) fn confine_patch_paths(
});
}
let canonical_root = std::fs::canonicalize(git_root)?;
let metadata_dirs = canonical_git_metadata_dirs(git, &canonical_root)?;
let mut entries = Vec::with_capacity(paths.len());
let mut prefix_cache = std::collections::BTreeMap::new();
@@ -416,9 +423,9 @@ pub(crate) fn confine_patch_paths(
);
let (existing_len, mut projected) = longest_existing_strict_prefix(
&canonical_root,
canonical_root,
&components,
&metadata_dirs,
metadata_dirs,
&mut prefix_cache,
)?;
projected.extend(
@@ -513,22 +520,25 @@ fn containment_error(message: &'static str) -> io::Error {
io::Error::new(io::ErrorKind::PermissionDenied, message)
}
fn canonical_git_metadata_dirs(git: &GitRunner, git_root: &Path) -> io::Result<Vec<PathBuf>> {
let config_parts = safe_git_config_parts();
fn canonical_git_metadata_dirs_guarded(config: &GuardedGitConfig<'_>) -> io::Result<Vec<PathBuf>> {
let queries = [
vec!["rev-parse".to_string(), "--absolute-git-dir".to_string()],
vec!["rev-parse".to_string(), "--git-common-dir".to_string()],
["rev-parse", "--absolute-git-dir"],
["rev-parse", "--git-common-dir"],
];
let mut metadata_dirs = std::collections::BTreeSet::new();
for args in queries {
let (code, stdout, stderr) = run_git(git, git_root, &config_parts, &args)?;
if code != 0 {
let mut command = config.rev_parse_command()?;
command.args(&args[1..]);
let output = command.output()?;
if !output.status.success() {
return Err(io::Error::other(format!(
"failed to resolve Git repository metadata (exit {code}): {}",
stderr.trim()
"failed to resolve Git repository metadata (status {}): {}",
output.status,
String::from_utf8_lossy(&output.stderr).trim()
)));
}
let path = stdout.trim_end_matches(['\r', '\n']);
let path = String::from_utf8_lossy(&output.stdout);
let path = path.trim_end_matches(['\r', '\n']);
if path.is_empty() {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
@@ -539,7 +549,7 @@ fn canonical_git_metadata_dirs(git: &GitRunner, git_root: &Path) -> io::Result<V
let absolute = if path.is_absolute() {
path
} else {
git_root.join(path)
config.canonical_root().join(path)
};
metadata_dirs.insert(std::fs::canonicalize(absolute)?);
}

4
codex-rs/git-utils/src/patch_paths_tests.rs
View File

@@ -68,8 +68,8 @@ fn effective_paths(diff: &str, revert: bool) -> io::Result<Vec<String>> {
let git_root =
crate::get_git_repo_root(cwd).ok_or_else(|| io::Error::other("not a Git repository"))?;
let git_root = std::fs::canonicalize(git_root)?;
ensure_no_worktree_config_sources(&git, &git_root, &[])?;
let paths = extract_effective_paths_from_patch(&git, &git_root, &patch_path, revert, &[])?;
let config = GuardedGitConfig::authorize(&git, &git_root, Vec::new())?;
let paths = extract_effective_paths_from_patch_guarded(&config, &patch_path, revert)?;
drop(tmpdir);
Ok(paths)
}

21
codex-rs/git-utils/src/repository_authority/authority.rs
View File

@@ -155,6 +155,27 @@ impl RepositoryAuthority {
})
}
pub(crate) fn ensure_active_worktree_root(&self, root: &Path) -> io::Result<()> {
let canonical_root = std::fs::canonicalize(root)?;
let same_root = if let Some(expected) = &self.active_worktree_identity {
Handle::from_path(&canonical_root)? == *expected
} else {
canonical_root == self.active_worktree_root
|| same_file::is_same_file(&canonical_root, &self.active_worktree_root)?
};
if !same_root {
return Err(io::Error::new(
io::ErrorKind::PermissionDenied,
format!(
"guarded Git repository identity {} does not match runner repository {}",
canonical_root.display(),
self.active_worktree_root.display()
),
));
}
Ok(())
}
fn revalidate_active_worktree_identity(&self) -> io::Result<()> {
let Some(expected) = &self.active_worktree_identity else {
return Ok(());