git-utils: harden reverse patch staging

codex-rs/git-utils/src/apply.rs

@@ -14,8 +14,8 @@ use crate::FsmonitorOverride;
 use crate::apply_output::parse_git_apply_output;
 use crate::git_command::GitRunner;
 use crate::merge_driver::ensure_no_selected_merge_drivers;
-use crate::patch_paths::extract_effective_paths_from_patch;
-use crate::patch_paths::stage_effective_paths_for_reverse;
+use crate::patch_paths::extract_patch_path_inventory;
+use crate::reverse_staging::stage_effective_paths_for_reverse;
 use crate::safe_git::DISABLED_HOOKS_PATH;
 use crate::safe_git::ensure_no_selected_executable_git_filters;
 #[cfg(test)]
@@ -55,9 +55,10 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
     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, &patch_path, req.revert)?;
+    let patch_path_inventory = extract_patch_path_inventory(&git, &patch_path, req.revert)?;
+    let patch_paths = &patch_path_inventory.effective_paths;
     let filter_guard =
-        ensure_no_selected_executable_git_filters(&git, &git_root, &patch_paths, &cfg_parts)?;
+        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());
     let patch_arg = patch_path.to_string_lossy().to_string();
@@ -106,7 +107,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
 
     let mut args = vec!["apply".to_string()];
     let merge_guard = if plain_check_code != 0 {
-        let guard = ensure_no_selected_merge_drivers(&git, &git_root, &patch_paths, &cfg_parts)?;
+        let guard = ensure_no_selected_merge_drivers(&git, &git_root, patch_paths, &cfg_parts)?;
         if let Some(guard) = &guard {
             cfg_parts.extend_from_slice(guard.git_config_args());
         }
@@ -114,17 +115,12 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
         guard
     } else {
         args.push("--index".to_string());
-        // Direct applications otherwise succeed silently, which leaves the
-        // caller unable to report which files were changed. `--verbose`
-        // emits the same parser-compatible "Applied patch ... cleanly"
-        // records that the three-way path already provides.
-        args.push("--verbose".to_string());
         None
     };
     if req.revert {
         // Stage only after the worktree-only applicability decision. The
         // guarded config is shared with the final Git command.
-        stage_effective_paths_for_reverse(&git, &git_root, &patch_paths, &cfg_parts)?;
+        stage_effective_paths_for_reverse(&git, &git_root, patch_paths, &cfg_parts)?;
     }
     if req.revert {
         args.push("-R".to_string());
@@ -135,8 +131,11 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
     let (code, stdout, stderr) = run_git(&git, &git_root, &cfg_parts, &args)?;
     drop(merge_guard);
 
-    let (mut applied_paths, mut skipped_paths, mut conflicted_paths) =
-        parse_git_apply_output(&stdout, &stderr);
+    let (mut applied_paths, mut skipped_paths, mut conflicted_paths) = if code == 0 {
+        (patch_path_inventory.primary_paths, Vec::new(), Vec::new())
+    } else {
+        parse_git_apply_output(&stdout, &stderr)
+    };
     applied_paths.sort();
     applied_paths.dedup();
     skipped_paths.sort();
@@ -291,6 +290,10 @@ mod transport_tests;
 #[path = "apply_filter_tests.rs"]
 mod filter_tests;
 
+#[cfg(test)]
+#[path = "reverse_apply_tests.rs"]
+mod reverse_apply_tests;
+
 #[cfg(test)]
 mod tests {
     use super::*;

codex-rs/git-utils/src/lib.rs

@@ -15,6 +15,7 @@ mod merge_driver;
 mod operations;
 mod patch_paths;
 mod platform;
+mod reverse_staging;
 mod safe_git;
 
 pub use apply::ApplyGitRequest;

codex-rs/git-utils/src/merge_driver_race_tests.rs

@@ -241,7 +241,7 @@ fn clean_reverse_stages_missing_delete_and_rename_endpoints() {
             .expect("clean topology reverse");
 
         assert_eq!(result.exit_code, 0, "{topology:?}: {}", result.stderr);
-        assert_eq!(result.applied_paths.len(), 1, "{topology:?}");
+        assert_eq!(result.applied_paths, vec!["old.txt"], "{topology:?}");
         assert!(result.cmd_for_log.contains("--index"), "{topology:?}");
         assert!(!result.cmd_for_log.contains("--3way"), "{topology:?}");
         assert_eq!(

codex-rs/git-utils/src/patch_paths.rs

@@ -1,5 +1,6 @@
 //! Effective patch-path discovery and safe staging guards.
 
+use std::collections::BTreeSet;
 use std::io;
 use std::path::Path;
 use std::path::PathBuf;
@@ -11,33 +12,58 @@ use crate::exact_staging::update_index_exact_paths;
 use crate::git_command::GitRunner;
 use crate::git_config::path_is_within;
 
-pub(crate) fn extract_effective_paths_from_patch(
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(crate) struct PatchPathInventory {
+    pub(crate) primary_paths: Vec<String>,
+    pub(crate) effective_paths: Vec<String>,
+}
+
+pub(crate) fn extract_patch_path_inventory(
     git: &GitRunner,
     patch_path: &Path,
     revert: bool,
-) -> io::Result<Vec<String>> {
-    let forward_paths = git_apply_numstat_paths(git, patch_path, revert)?;
+) -> io::Result<PatchPathInventory> {
+    let primary_paths = git_apply_numstat_paths(git, patch_path, revert)?;
     // `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, patch_path, !revert)?;
-    if forward_paths.len() != reverse_paths.len() {
+    let opposite_paths = git_apply_numstat_paths(git, patch_path, !revert)?;
+    if primary_paths.len() != opposite_paths.len() {
         return Err(io::Error::new(
             io::ErrorKind::InvalidData,
             "forward and reverse patch parsing returned different path counts",
         ));
     }
-    let effective_paths: std::collections::BTreeSet<String> =
-        forward_paths.into_iter().chain(reverse_paths).collect();
+    let primary_paths = primary_paths
+        .into_iter()
+        .map(validate_patch_path)
+        .collect::<io::Result<BTreeSet<_>>>()?;
+    let opposite_paths = opposite_paths
+        .into_iter()
+        .map(validate_patch_path)
+        .collect::<io::Result<BTreeSet<_>>>()?;
+    let effective_paths = primary_paths
+        .iter()
+        .cloned()
+        .chain(opposite_paths)
+        .collect::<BTreeSet<_>>();
     if effective_paths.is_empty() {
         return Err(io::Error::new(
             io::ErrorKind::InvalidInput,
             "patch does not identify any paths",
         ));
     }
-    effective_paths
-        .into_iter()
-        .map(validate_patch_path)
-        .collect()
+    Ok(PatchPathInventory {
+        primary_paths: primary_paths.into_iter().collect(),
+        effective_paths: effective_paths.into_iter().collect(),
+    })
+}
+
+pub(crate) fn extract_effective_paths_from_patch(
+    git: &GitRunner,
+    patch_path: &Path,
+    revert: bool,
+) -> io::Result<Vec<String>> {
+    Ok(extract_patch_path_inventory(git, patch_path, revert)?.effective_paths)
 }
 
 /// Best-effort extraction of the paths Git would apply.
@@ -119,7 +145,7 @@ fn parse_numstat_paths(output: &[u8]) -> io::Result<Vec<String>> {
             )
         })?;
         if path.is_empty() {
-            let old = records
+            let _old = records
                 .next()
                 .filter(|path| !path.is_empty())
                 .ok_or_else(|| {
@@ -137,7 +163,6 @@ fn parse_numstat_paths(output: &[u8]) -> io::Result<Vec<String>> {
                         "git apply returned an incomplete rename path record",
                     )
                 })?;
-            insert_numstat_path(&mut paths, old)?;
             insert_numstat_path(&mut paths, new)?;
         } else {
             insert_numstat_path(&mut paths, path)?;
@@ -274,12 +299,12 @@ pub(crate) fn stage_effective_paths(
 }
 
 #[cfg(not(windows))]
-fn leaf_is_traversable_directory(file_type: std::fs::FileType) -> bool {
+pub(crate) fn leaf_is_traversable_directory(file_type: std::fs::FileType) -> bool {
     file_type.is_dir()
 }
 
 #[cfg(unix)]
-fn leaf_may_run_git_content_filter(file_type: std::fs::FileType) -> bool {
+pub(crate) fn leaf_may_run_git_content_filter(file_type: std::fs::FileType) -> bool {
     // Git stages an exact Unix symlink as a mode-120000 blob containing the
     // link target. The whole-command neutralizer covers unrelated racy index
     // entries while this target is omitted from the selected-filter refusal.
@@ -287,14 +312,14 @@ fn leaf_may_run_git_content_filter(file_type: std::fs::FileType) -> bool {
 }
 
 #[cfg(not(unix))]
-fn leaf_may_run_git_content_filter(_file_type: std::fs::FileType) -> bool {
+pub(crate) fn leaf_may_run_git_content_filter(_file_type: std::fs::FileType) -> bool {
     // Keep the conservative policy on platforms whose symlink staging
     // behavior can depend on repository and host configuration.
     true
 }
 
 #[cfg(windows)]
-fn leaf_is_traversable_directory(file_type: std::fs::FileType) -> bool {
+pub(crate) fn leaf_is_traversable_directory(file_type: std::fs::FileType) -> bool {
     use std::os::windows::fs::FileTypeExt;
 
     // Git traverses junctions and container-mapped directory symlinks. Refuse
@@ -346,7 +371,7 @@ pub(crate) struct ConfinedPatchPaths {
 }
 
 impl ConfinedPatchPaths {
-    fn into_exact_leaves(self) -> io::Result<Vec<String>> {
+    pub(crate) fn into_exact_leaves(self) -> io::Result<Vec<String>> {
         self.entries
             .into_iter()
             .map(|entry| {

codex-rs/git-utils/src/patch_paths_tests.rs

@@ -134,10 +134,7 @@ fn effective_paths_cover_supported_patch_headers() {
 
     let nul_rename_paths = parse_numstat_paths(b"0\t0\t\0old name.txt\0new name.txt\0")
         .expect("parse NUL-delimited rename paths");
-    assert_eq!(
-        nul_rename_paths,
-        vec!["old name.txt".to_string(), "new name.txt".to_string()]
-    );
+    assert_eq!(nul_rename_paths, vec!["new name.txt".to_string()]);
 }
 
 #[test]

codex-rs/git-utils/src/reverse_apply_tests.rs

@@ -0,0 +1,561 @@
+use super::*;
+use crate::safe_git::isolate_git_command_environment;
+use pretty_assertions::assert_eq;
+use std::ffi::OsStr;
+use std::path::Path;
+use std::process::Stdio;
+
+#[derive(Clone, Copy, Debug)]
+enum Topology {
+    Delete,
+    Rename,
+}
+
+fn run(cwd: &Path, args: &[&str], input: Option<&[u8]>) -> std::process::Output {
+    let mut command = std::process::Command::new("git");
+    isolate_git_command_environment(&mut command);
+    command
+        .args(args)
+        .current_dir(cwd)
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped());
+    if input.is_some() {
+        command.stdin(Stdio::piped());
+    }
+    let mut child = command.spawn().expect("run Git");
+    if let Some(input) = input {
+        use std::io::Write;
+        child
+            .stdin
+            .take()
+            .expect("Git stdin")
+            .write_all(input)
+            .expect("write Git stdin");
+    }
+    child.wait_with_output().expect("read Git output")
+}
+
+fn run_success(cwd: &Path, args: &[&str]) -> Vec<u8> {
+    let output = run(cwd, args, None);
+    assert!(
+        output.status.success(),
+        "git {args:?}: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+    output.stdout
+}
+
+fn init_repo() -> tempfile::TempDir {
+    let repo = tempfile::tempdir().expect("repository");
+    let root = repo.path();
+    run_success(root, &["init", "-q"]);
+    run_success(root, &["config", "user.email", "codex@example.com"]);
+    run_success(root, &["config", "user.name", "Codex"]);
+    run_success(root, &["config", "core.autocrlf", "false"]);
+    run_success(root, &["config", "core.filemode", "true"]);
+    repo
+}
+
+fn request(root: &Path, patch: &str) -> ApplyGitRequest {
+    ApplyGitRequest {
+        cwd: root.to_path_buf(),
+        diff: patch.to_string(),
+        revert: true,
+        preflight: false,
+    }
+}
+
+fn topology_fixture(topology: Topology) -> (tempfile::TempDir, String) {
+    let repo = init_repo();
+    let root = repo.path();
+    std::fs::write(root.join("old.txt"), b"old\n").expect("base file");
+    run_success(root, &["add", "old.txt"]);
+    run_success(root, &["commit", "-qm", "base"]);
+    match topology {
+        Topology::Delete => {
+            run_success(root, &["rm", "-q", "old.txt"]);
+        }
+        Topology::Rename => {
+            run_success(root, &["mv", "old.txt", "new.txt"]);
+        }
+    }
+    let patch = String::from_utf8(run_success(
+        root,
+        &["diff", "--cached", "--full-index", "--binary", "-M"],
+    ))
+    .expect("UTF-8 patch");
+    run_success(root, &["reset", "--hard", "-q", "HEAD"]);
+    (repo, patch)
+}
+
+fn index_snapshot(root: &Path) -> Vec<u8> {
+    let mut snapshot = run_success(root, &["ls-files", "-v", "--stage", "-z"]);
+    snapshot.extend(run_success(root, &["ls-files", "--debug"]));
+    snapshot
+}
+
+#[derive(Debug, PartialEq, Eq)]
+enum LeafSnapshot {
+    Missing,
+    File(Vec<u8>, u32),
+    Symlink(std::path::PathBuf),
+}
+
+#[cfg(unix)]
+fn file_mode(metadata: &std::fs::Metadata) -> u32 {
+    use std::os::unix::fs::PermissionsExt;
+    metadata.permissions().mode()
+}
+
+#[cfg(not(unix))]
+fn file_mode(metadata: &std::fs::Metadata) -> u32 {
+    u32::from(metadata.permissions().readonly())
+}
+
+fn leaf_snapshot(root: &Path, path: &str) -> LeafSnapshot {
+    match std::fs::symlink_metadata(root.join(path)) {
+        Ok(metadata) if metadata.file_type().is_symlink() => {
+            LeafSnapshot::Symlink(std::fs::read_link(root.join(path)).expect("symlink target"))
+        }
+        Ok(metadata) => LeafSnapshot::File(
+            std::fs::read(root.join(path)).expect("file bytes"),
+            file_mode(&metadata),
+        ),
+        Err(error) if error.kind() == io::ErrorKind::NotFound => LeafSnapshot::Missing,
+        Err(error) => panic!("snapshot {path}: {error}"),
+    }
+}
+
+fn assert_refused_without_mutation(root: &Path, patch: &str) {
+    let before_index = index_snapshot(root);
+    let before_leaves = [
+        leaf_snapshot(root, "old.txt"),
+        leaf_snapshot(root, "new.txt"),
+    ];
+    let error = apply_git_patch(&request(root, patch)).expect_err("unsafe reverse must fail");
+    assert_eq!(error.kind(), io::ErrorKind::PermissionDenied, "{error}");
+    assert_eq!(index_snapshot(root), before_index);
+    assert_eq!(
+        [
+            leaf_snapshot(root, "old.txt"),
+            leaf_snapshot(root, "new.txt")
+        ],
+        before_leaves
+    );
+}
+
+#[test]
+fn reverse_delete_and_rename_preserve_divergent_staged_blobs() {
+    for (topology, endpoint) in [
+        (Topology::Delete, "old.txt"),
+        (Topology::Rename, "old.txt"),
+        (Topology::Rename, "new.txt"),
+    ] {
+        let (repo, patch) = topology_fixture(topology);
+        let root = repo.path();
+        if matches!(topology, Topology::Rename) {
+            std::fs::remove_file(root.join("old.txt")).expect("remove old endpoint");
+            std::fs::write(root.join("new.txt"), b"old\n").expect("new endpoint");
+        }
+        std::fs::write(root.join(endpoint), b"user-staged\n").expect("divergent staged blob");
+        run_success(root, &["add", endpoint]);
+        if endpoint == "old.txt" {
+            std::fs::remove_file(root.join(endpoint)).expect("missing old endpoint");
+        } else {
+            std::fs::write(root.join(endpoint), b"old\n").expect("restore worktree endpoint");
+        }
+        assert_refused_without_mutation(root, &patch);
+    }
+}
+
+#[test]
+fn reverse_rename_accepts_each_partially_staged_form() {
+    for staged_endpoints in ["old", "new", "both"] {
+        let (repo, patch) = topology_fixture(Topology::Rename);
+        let root = repo.path();
+        std::fs::remove_file(root.join("old.txt")).expect("remove old endpoint");
+        std::fs::write(root.join("new.txt"), b"old\n").expect("new endpoint");
+        match staged_endpoints {
+            "old" => {
+                run_success(root, &["add", "old.txt"]);
+            }
+            "new" => {
+                run_success(root, &["add", "new.txt"]);
+            }
+            "both" => {
+                run_success(root, &["add", "old.txt", "new.txt"]);
+            }
+            _ => unreachable!(),
+        }
+
+        let result = apply_git_patch(&request(root, &patch)).expect("safe partial rename reverse");
+        assert_eq!(result.exit_code, 0, "{}", result.stderr);
+        assert_eq!(result.applied_paths, vec!["old.txt"]);
+        assert_eq!(std::fs::read(root.join("old.txt")).unwrap(), b"old\n");
+        assert!(!root.join("new.txt").exists());
+        assert!(run_success(root, &["status", "--porcelain"]).is_empty());
+    }
+}
+
+#[test]
+fn reverse_staged_delete_succeeds_but_recreated_worktree_is_refused() {
+    let (repo, patch) = topology_fixture(Topology::Delete);
+    let root = repo.path();
+    run_success(root, &["rm", "-q", "old.txt"]);
+    let result = apply_git_patch(&request(root, &patch)).expect("already-staged reverse delete");
+    assert_eq!(result.exit_code, 0, "{}", result.stderr);
+    assert_eq!(result.applied_paths, vec!["old.txt"]);
+    assert!(run_success(root, &["status", "--porcelain"]).is_empty());
+
+    run_success(root, &["rm", "-q", "old.txt"]);
+    std::fs::write(root.join("old.txt"), b"recreated\n").unwrap();
+    assert_refused_without_mutation(root, &patch);
+}
+
+#[test]
+fn reverse_refuses_intent_to_add_conflicts_and_index_flags() {
+    for condition in [
+        "ita-old",
+        "ita-new",
+        "conflict-old",
+        "conflict-new",
+        "assume",
+        "skip",
+    ] {
+        let topology = if matches!(condition, "ita-new" | "conflict-new") {
+            Topology::Rename
+        } else {
+            Topology::Delete
+        };
+        let (repo, patch) = topology_fixture(topology);
+        let root = repo.path();
+        if matches!(topology, Topology::Rename) {
+            std::fs::remove_file(root.join("old.txt")).unwrap();
+            std::fs::write(root.join("new.txt"), b"old\n").unwrap();
+        }
+        match condition {
+            "ita-old" => {
+                run_success(root, &["rm", "--cached", "-q", "old.txt"]);
+                run_success(root, &["add", "-N", "old.txt"]);
+                std::fs::remove_file(root.join("old.txt")).unwrap();
+            }
+            "ita-new" => {
+                run_success(root, &["add", "-N", "new.txt"]);
+            }
+            "conflict-old" | "conflict-new" => {
+                let path = condition.strip_prefix("conflict-").unwrap();
+                let path = format!("{path}.txt");
+                if path == "old.txt" {
+                    std::fs::remove_file(root.join(&path)).unwrap();
+                }
+                let oid =
+                    String::from_utf8(run_success(root, &["rev-parse", "HEAD:old.txt"])).unwrap();
+                let input = format!(
+                    "0 0000000000000000000000000000000000000000\t{path}\n100644 {} 1\t{path}\n100644 {} 2\t{path}\n100644 {} 3\t{path}\n",
+                    oid.trim(),
+                    oid.trim(),
+                    oid.trim()
+                );
+                let output = run(
+                    root,
+                    &["update-index", "--index-info"],
+                    Some(input.as_bytes()),
+                );
+                assert!(
+                    output.status.success(),
+                    "{}",
+                    String::from_utf8_lossy(&output.stderr)
+                );
+            }
+            "assume" => {
+                run_success(root, &["update-index", "--assume-unchanged", "old.txt"]);
+                std::fs::remove_file(root.join("old.txt")).unwrap();
+            }
+            "skip" => {
+                run_success(root, &["update-index", "--skip-worktree", "old.txt"]);
+                std::fs::remove_file(root.join("old.txt")).unwrap();
+            }
+            _ => unreachable!(),
+        }
+        assert_refused_without_mutation(root, &patch);
+    }
+}
+
+#[cfg(unix)]
+#[test]
+fn reverse_refuses_divergent_staged_modes_and_symlinks_at_every_endpoint() {
+    use std::os::unix::fs::PermissionsExt;
+
+    for (topology, endpoint, symlink) in [
+        (Topology::Delete, "old.txt", false),
+        (Topology::Delete, "old.txt", true),
+        (Topology::Rename, "old.txt", false),
+        (Topology::Rename, "old.txt", true),
+        (Topology::Rename, "new.txt", false),
+        (Topology::Rename, "new.txt", true),
+    ] {
+        let (repo, patch) = topology_fixture(topology);
+        let root = repo.path();
+        if matches!(topology, Topology::Rename) {
+            std::fs::remove_file(root.join("old.txt")).unwrap();
+            std::fs::write(root.join("new.txt"), b"old\n").unwrap();
+        }
+        let endpoint_path = root.join(endpoint);
+        if symlink {
+            let _ = std::fs::remove_file(&endpoint_path);
+            std::os::unix::fs::symlink("staged-target", &endpoint_path).unwrap();
+        } else {
+            std::fs::write(&endpoint_path, b"old\n").unwrap();
+            let mut permissions = std::fs::metadata(&endpoint_path).unwrap().permissions();
+            permissions.set_mode(0o755);
+            std::fs::set_permissions(&endpoint_path, permissions).unwrap();
+        }
+        run_success(root, &["add", endpoint]);
+        std::fs::remove_file(&endpoint_path).unwrap();
+        if endpoint == "new.txt" {
+            std::fs::write(&endpoint_path, b"old\n").unwrap();
+        }
+        if !symlink {
+            run_success(root, &["config", "core.filemode", "false"]);
+        }
+        assert_refused_without_mutation(root, &patch);
+    }
+}
+
+#[cfg(unix)]
+#[test]
+fn reverse_mode_only_change_succeeds_when_repository_ignores_filemode() {
+    use std::os::unix::fs::PermissionsExt;
+
+    for (base_mode, forward_mode, expected_index_mode) in
+        [(0o644, 0o755, "100644 "), (0o755, 0o644, "100755 ")]
+    {
+        let repo = init_repo();
+        let root = repo.path();
+        std::fs::write(root.join("mode.txt"), b"unchanged\n").unwrap();
+        let mut permissions = std::fs::metadata(root.join("mode.txt"))
+            .unwrap()
+            .permissions();
+        permissions.set_mode(base_mode);
+        std::fs::set_permissions(root.join("mode.txt"), permissions).unwrap();
+        run_success(root, &["add", "mode.txt"]);
+        run_success(root, &["commit", "-qm", "base"]);
+
+        let mut permissions = std::fs::metadata(root.join("mode.txt"))
+            .unwrap()
+            .permissions();
+        permissions.set_mode(forward_mode);
+        std::fs::set_permissions(root.join("mode.txt"), permissions).unwrap();
+        let patch = String::from_utf8(run_success(root, &["diff", "--full-index"])).unwrap();
+        assert!(patch.contains("old mode "), "{patch}");
+        assert!(patch.contains("new mode "), "{patch}");
+
+        run_success(root, &["reset", "--hard", "-q", "HEAD"]);
+        run_success(root, &["config", "core.filemode", "false"]);
+        let mut permissions = std::fs::metadata(root.join("mode.txt"))
+            .unwrap()
+            .permissions();
+        permissions.set_mode(forward_mode);
+        std::fs::set_permissions(root.join("mode.txt"), permissions).unwrap();
+
+        let result =
+            apply_git_patch(&request(root, &patch)).expect("reverse executable-bit change");
+
+        assert_eq!(result.exit_code, 0, "{}", result.stderr);
+        assert_eq!(result.applied_paths, vec!["mode.txt"]);
+        let index =
+            String::from_utf8(run_success(root, &["ls-files", "--stage", "mode.txt"])).unwrap();
+        assert!(index.starts_with(expected_index_mode), "{index}");
+        assert_eq!(
+            std::fs::metadata(root.join("mode.txt"))
+                .unwrap()
+                .permissions()
+                .mode()
+                & 0o777,
+            base_mode
+        );
+        assert!(run_success(root, &["status", "--porcelain"]).is_empty());
+    }
+}
+
+#[cfg(unix)]
+#[test]
+fn reverse_content_only_change_preserves_executable_index_mode_when_filemode_is_ignored() {
+    use std::os::unix::fs::PermissionsExt;
+
+    let repo = init_repo();
+    let root = repo.path();
+    std::fs::write(root.join("script.sh"), b"old\n").unwrap();
+    let mut permissions = std::fs::metadata(root.join("script.sh"))
+        .unwrap()
+        .permissions();
+    permissions.set_mode(0o755);
+    std::fs::set_permissions(root.join("script.sh"), permissions).unwrap();
+    run_success(root, &["add", "script.sh"]);
+    run_success(root, &["commit", "-qm", "base"]);
+
+    std::fs::write(root.join("script.sh"), b"new\n").unwrap();
+    let patch = String::from_utf8(run_success(root, &["diff", "--full-index"])).unwrap();
+    assert!(!patch.contains("old mode "), "{patch}");
+    assert!(!patch.contains("new mode "), "{patch}");
+
+    run_success(root, &["reset", "--hard", "-q", "HEAD"]);
+    run_success(root, &["config", "core.filemode", "false"]);
+    let mut permissions = std::fs::metadata(root.join("script.sh"))
+        .unwrap()
+        .permissions();
+    permissions.set_mode(0o644);
+    std::fs::set_permissions(root.join("script.sh"), permissions).unwrap();
+    std::fs::write(root.join("script.sh"), b"new\n").unwrap();
+
+    let result = apply_git_patch(&request(root, &patch)).expect("reverse content-only change");
+
+    assert_eq!(result.exit_code, 0, "{}", result.stderr);
+    assert_eq!(result.applied_paths, vec!["script.sh"]);
+    assert_eq!(std::fs::read(root.join("script.sh")).unwrap(), b"old\n");
+    let index =
+        String::from_utf8(run_success(root, &["ls-files", "--stage", "script.sh"])).unwrap();
+    assert!(index.starts_with("100755 "), "{index}");
+    assert!(run_success(root, &["status", "--porcelain"]).is_empty());
+}
+
+#[test]
+fn reverse_refuses_ignored_untracked_rename_endpoint_without_mutation() {
+    let (repo, patch) = topology_fixture(Topology::Rename);
+    let root = repo.path();
+    std::fs::write(root.join(".gitignore"), b"new.txt\n").unwrap();
+    std::fs::rename(root.join("old.txt"), root.join("new.txt")).unwrap();
+    let before_index = index_snapshot(root);
+    let before_leaves = [
+        leaf_snapshot(root, "old.txt"),
+        leaf_snapshot(root, "new.txt"),
+    ];
+
+    let error = apply_git_patch(&request(root, &patch)).expect_err("ignored endpoint");
+    assert!(error.to_string().contains("ignored"), "{error}");
+    assert_eq!(index_snapshot(root), before_index);
+    assert_eq!(
+        [
+            leaf_snapshot(root, "old.txt"),
+            leaf_snapshot(root, "new.txt")
+        ],
+        before_leaves
+    );
+}
+
+#[test]
+fn reverse_handles_unborn_empty_index_and_refuses_unborn_index_state() {
+    const DELETE_PATCH: &str = "diff --git a/old.txt b/old.txt\ndeleted file mode 100644\n--- a/old.txt\n+++ /dev/null\n@@ -1 +0,0 @@\n-old\n";
+
+    let clean = init_repo();
+    let result = apply_git_patch(&request(clean.path(), DELETE_PATCH)).expect("unborn reverse add");
+    assert_eq!(result.exit_code, 0, "{}", result.stderr);
+    assert_eq!(result.applied_paths, vec!["old.txt"]);
+    assert_eq!(
+        std::fs::read(clean.path().join("old.txt")).unwrap(),
+        b"old\n"
+    );
+
+    for intent_to_add in [false, true] {
+        let repo = init_repo();
+        let root = repo.path();
+        std::fs::write(root.join("old.txt"), b"staged\n").unwrap();
+        run_success(
+            root,
+            if intent_to_add {
+                &["add", "-N", "old.txt"]
+            } else {
+                &["add", "old.txt"]
+            },
+        );
+        std::fs::remove_file(root.join("old.txt")).unwrap();
+        assert_refused_without_mutation(root, DELETE_PATCH);
+    }
+}
+
+#[test]
+fn reverse_propagates_index_staging_failure_before_final_apply() {
+    let (repo, patch) = topology_fixture(Topology::Delete);
+    let root = repo.path();
+    std::fs::remove_file(root.join("old.txt")).unwrap();
+    let before = index_snapshot(root);
+    std::fs::write(root.join(".git/index.lock"), b"locked").unwrap();
+
+    let error = apply_git_patch(&request(root, &patch)).expect_err("index staging failure");
+    assert!(
+        error
+            .to_string()
+            .contains("failed to stage reverse patch paths")
+    );
+    std::fs::remove_file(root.join(".git/index.lock")).unwrap();
+    assert_eq!(index_snapshot(root), before);
+    assert!(
+        !root.join("old.txt").exists(),
+        "final reverse apply must not run"
+    );
+}
+
+#[test]
+fn direct_success_paths_are_machine_readable_in_each_orientation() {
+    if std::env::var_os("CODEX_GIT_UTILS_REPORTING_LOCALE_CHILD").is_none() {
+        let mut command = std::process::Command::new(std::env::current_exe().unwrap());
+        isolate_git_command_environment(&mut command);
+        let output = command
+            .arg("apply::reverse_apply_tests::direct_success_paths_are_machine_readable_in_each_orientation")
+            .arg("--exact")
+            .arg("--nocapture")
+            .env("CODEX_GIT_UTILS_REPORTING_LOCALE_CHILD", "1")
+            .env("LC_ALL", OsStr::new("fr_FR.UTF-8"))
+            .env("RUST_TEST_THREADS", "1")
+            .output()
+            .expect("localized test child");
+        assert!(output.status.success(), "{output:?}");
+        return;
+    }
+
+    let repo = init_repo();
+    let root = repo.path();
+    for path in ["first.txt", "second.txt"] {
+        std::fs::write(root.join(path), b"old\n").unwrap();
+    }
+    run_success(root, &["add", "."]);
+    run_success(root, &["commit", "-qm", "base"]);
+    for path in ["first.txt", "second.txt"] {
+        std::fs::write(root.join(path), b"new\n").unwrap();
+    }
+    run_success(root, &["add", "."]);
+    let patch =
+        String::from_utf8(run_success(root, &["diff", "--cached", "--full-index"])).unwrap();
+    run_success(root, &["reset", "--hard", "-q", "HEAD"]);
+    for revert in [false, true] {
+        let result = apply_git_patch(&ApplyGitRequest {
+            cwd: root.to_path_buf(),
+            diff: patch.clone(),
+            revert,
+            preflight: false,
+        })
+        .unwrap();
+        assert_eq!(result.exit_code, 0, "{}", result.stderr);
+        assert_eq!(result.applied_paths, vec!["first.txt", "second.txt"]);
+        assert!(!result.cmd_for_log.contains("--verbose"));
+    }
+
+    run_success(root, &["mv", "first.txt", "renamed.txt"]);
+    let rename = String::from_utf8(run_success(
+        root,
+        &["diff", "--cached", "--full-index", "-M"],
+    ))
+    .unwrap();
+    run_success(root, &["reset", "--hard", "-q", "HEAD"]);
+    for (revert, expected) in [(false, "renamed.txt"), (true, "first.txt")] {
+        let result = apply_git_patch(&ApplyGitRequest {
+            cwd: root.to_path_buf(),
+            diff: rename.clone(),
+            revert,
+            preflight: false,
+        })
+        .unwrap();
+        assert_eq!(result.exit_code, 0, "{}", result.stderr);
+        assert_eq!(result.applied_paths, vec![expected]);
+    }
+}

codex-rs/git-utils/src/reverse_staging.rs

@@ -0,0 +1,431 @@
+//! Safe preparatory index staging for direct reverse patch application.
+
+use std::collections::BTreeMap;
+use std::collections::BTreeSet;
+use std::io;
+use std::path::Path;
+
+use crate::exact_staging::update_index_exact_paths;
+use crate::git_command::GitRunner;
+use crate::patch_paths::confine_patch_paths;
+use crate::patch_paths::leaf_is_traversable_directory;
+use crate::patch_paths::leaf_may_run_git_content_filter;
+
+#[cfg(unix)]
+const WORKTREE_FILEMODE_CONFIG_ARGS: &[&str] = &["-c", "core.filemode=true"];
+#[cfg(not(unix))]
+const WORKTREE_FILEMODE_CONFIG_ARGS: &[&str] = &[];
+
+pub(crate) fn stage_effective_paths_for_reverse(
+    git: &GitRunner,
+    git_root: &Path,
+    paths: &[String],
+    git_config_args: &[String],
+) -> io::Result<()> {
+    let confined = confine_patch_paths(git, git_root, paths)?;
+    let mut worktree_paths = BTreeMap::new();
+    for path in confined.into_exact_leaves()? {
+        let joined = git_root.join(&path);
+        let state = match std::fs::symlink_metadata(&joined) {
+            Ok(metadata) => {
+                let file_type = metadata.file_type();
+                if leaf_is_traversable_directory(file_type) {
+                    return Err(io::Error::new(
+                        io::ErrorKind::PermissionDenied,
+                        "refusing to recursively stage a directory patch path",
+                    ));
+                }
+                ReverseWorktreePath {
+                    exists: true,
+                    may_run_content_filter: leaf_may_run_git_content_filter(file_type),
+                }
+            }
+            Err(error) if error.kind() == io::ErrorKind::NotFound => ReverseWorktreePath {
+                exists: false,
+                may_run_content_filter: false,
+            },
+            Err(error) => {
+                return Err(io::Error::new(
+                    error.kind(),
+                    format!(
+                        "failed to inspect reverse patch path {}: {error}",
+                        joined.display()
+                    ),
+                ));
+            }
+        };
+        worktree_paths.insert(path, state);
+    }
+
+    let exact_paths = worktree_paths.keys().cloned().collect::<Vec<_>>();
+    let index_entries = read_reverse_index_entries(git, git_root, &exact_paths, git_config_args)?;
+    for path in &exact_paths {
+        let entries = index_entries.get(path).map(Vec::as_slice).unwrap_or(&[]);
+        if !matches!(
+            entries,
+            [] | [ReverseIndexEntry {
+                tag: b'H',
+                stage: 0
+            }]
+        ) {
+            return Err(reverse_staging_error(format!(
+                "refusing to stage reverse patch path {path:?} because its index entry is conflicted or carries assume-unchanged/skip-worktree state"
+            )));
+        }
+    }
+
+    let cached_invisible_status = read_name_status(
+        git,
+        git_root,
+        &exact_paths,
+        git_config_args,
+        &[
+            "--literal-pathspecs",
+            "diff",
+            "--cached",
+            "--name-status",
+            "-z",
+            "--no-renames",
+            "--ita-invisible-in-index",
+            "--no-ext-diff",
+            "--no-textconv",
+            "--ignore-submodules=none",
+        ],
+        "inspect reverse paths with intent-to-add entries hidden",
+    )?;
+    let cached_visible_status = read_name_status(
+        git,
+        git_root,
+        &exact_paths,
+        git_config_args,
+        &[
+            "--literal-pathspecs",
+            "diff",
+            "--cached",
+            "--name-status",
+            "-z",
+            "--no-renames",
+            "--ita-visible-in-index",
+            "--no-ext-diff",
+            "--no-textconv",
+            "--ignore-submodules=none",
+        ],
+        "inspect reverse paths with intent-to-add entries visible",
+    )?;
+    if cached_invisible_status != cached_visible_status {
+        let path = exact_paths
+            .iter()
+            .find(|path| cached_invisible_status.get(*path) != cached_visible_status.get(*path))
+            .ok_or_else(|| invalid_reverse_index_output("missing intent-to-add path"))?;
+        return Err(reverse_staging_error(format!(
+            "refusing to stage reverse patch path {path:?} because it has intent-to-add index state"
+        )));
+    }
+    let worktree_changed_paths = read_changed_paths(
+        git,
+        git_root,
+        &exact_paths,
+        git_config_args,
+        WORKTREE_FILEMODE_CONFIG_ARGS,
+        &[
+            "--literal-pathspecs",
+            "diff-files",
+            "--name-only",
+            "-z",
+            "--no-renames",
+            "--no-ext-diff",
+            "--no-textconv",
+            "--ignore-submodules=none",
+        ],
+        "compare reverse path index entries with the worktree",
+    )?;
+
+    let mut staging_candidates = Vec::new();
+    for path in &exact_paths {
+        let worktree = worktree_paths
+            .get(path)
+            .ok_or_else(|| invalid_reverse_index_output("missing worktree path state"))?;
+        let entries = index_entries.get(path).map(Vec::as_slice).unwrap_or(&[]);
+        match entries {
+            [] if !worktree.exists => continue,
+            [] => staging_candidates.push(path.clone()),
+            [_] if worktree_changed_paths.contains(path) => {
+                staging_candidates.push(path.clone());
+            }
+            [_] => continue,
+            _ => unreachable!("index entries were validated above"),
+        }
+    }
+    if staging_candidates.is_empty() {
+        return Ok(());
+    }
+
+    if let Some(path) = staging_candidates
+        .iter()
+        .find(|path| cached_visible_status.contains_key(*path))
+    {
+        return Err(reverse_staging_error(format!(
+            "refusing to prepare a reverse patch because staging {path:?} would replace existing staged data"
+        )));
+    }
+
+    let content_filter_paths = staging_candidates
+        .iter()
+        .filter(|path| {
+            worktree_paths
+                .get(*path)
+                .is_some_and(|state| state.may_run_content_filter)
+        })
+        .cloned()
+        .collect::<Vec<_>>();
+    let result = update_index_exact_paths(
+        git,
+        git_root,
+        &staging_candidates,
+        &content_filter_paths,
+        git_config_args,
+    )?;
+    if result.exit_code == 0 {
+        Ok(())
+    } else {
+        Err(io::Error::other(format!(
+            "failed to stage reverse patch paths (exit {}): {}",
+            result.exit_code,
+            result.stderr.trim()
+        )))
+    }
+}
+
+#[derive(Clone, Copy)]
+struct ReverseWorktreePath {
+    exists: bool,
+    may_run_content_filter: bool,
+}
+
+#[derive(Clone, Copy)]
+struct ReverseIndexEntry {
+    tag: u8,
+    stage: u8,
+}
+
+fn read_reverse_index_entries(
+    git: &GitRunner,
+    git_root: &Path,
+    paths: &[String],
+    git_config_args: &[String],
+) -> io::Result<BTreeMap<String, Vec<ReverseIndexEntry>>> {
+    let mut command = git.command();
+    command
+        .args(git_config_args)
+        .args([
+            "--literal-pathspecs",
+            "ls-files",
+            "-v",
+            "--stage",
+            "-z",
+            "--",
+        ])
+        .args(paths)
+        .current_dir(git_root);
+    let output = git.output(command)?;
+    if !output.status.success() {
+        return Err(io::Error::other(format!(
+            "failed to inspect reverse patch index entries: {}",
+            String::from_utf8_lossy(&output.stderr).trim()
+        )));
+    }
+    let records = if output.stdout.is_empty() {
+        &[][..]
+    } else {
+        output
+            .stdout
+            .strip_suffix(&[0])
+            .ok_or_else(|| invalid_reverse_index_output("unterminated git ls-files output"))?
+    };
+
+    let expected = paths.iter().map(String::as_str).collect::<BTreeSet<_>>();
+    let mut entries = BTreeMap::<String, Vec<ReverseIndexEntry>>::new();
+    if records.is_empty() {
+        return Ok(entries);
+    }
+    for record in records.split(|byte| *byte == 0) {
+        let Some(separator) = record.iter().position(|byte| *byte == b'\t') else {
+            return Err(invalid_reverse_index_output(
+                "git ls-files returned a malformed stage record",
+            ));
+        };
+        let (header, path_with_separator) = record.split_at(separator);
+        let path = &path_with_separator[1..];
+        let path = std::str::from_utf8(path)
+            .map_err(|_| invalid_reverse_index_output("git ls-files returned a non-UTF-8 path"))?;
+        if !expected.contains(path) {
+            return Err(invalid_reverse_index_output(
+                "git ls-files returned an unexpected path",
+            ));
+        }
+        let header = std::str::from_utf8(header).map_err(|_| {
+            invalid_reverse_index_output("git ls-files returned a non-UTF-8 stage header")
+        })?;
+        let mut fields = header.split_ascii_whitespace();
+        let tag = fields
+            .next()
+            .filter(|tag| tag.len() == 1)
+            .map(|tag| tag.as_bytes()[0])
+            .ok_or_else(|| invalid_reverse_index_output("missing git ls-files status tag"))?;
+        let _mode = fields
+            .next()
+            .ok_or_else(|| invalid_reverse_index_output("missing git ls-files mode"))?;
+        let _object_id = fields
+            .next()
+            .ok_or_else(|| invalid_reverse_index_output("missing git ls-files object ID"))?;
+        let stage = fields
+            .next()
+            .ok_or_else(|| invalid_reverse_index_output("missing git ls-files stage"))?
+            .parse::<u8>()
+            .map_err(|_| invalid_reverse_index_output("invalid git ls-files stage"))?;
+        if fields.next().is_some() {
+            return Err(invalid_reverse_index_output(
+                "git ls-files returned an ambiguous stage header",
+            ));
+        }
+        entries
+            .entry(path.to_string())
+            .or_default()
+            .push(ReverseIndexEntry { tag, stage });
+    }
+    Ok(entries)
+}
+
+fn read_name_status(
+    git: &GitRunner,
+    git_root: &Path,
+    paths: &[String],
+    git_config_args: &[String],
+    args: &[&str],
+    operation: &str,
+) -> io::Result<BTreeMap<String, u8>> {
+    let output =
+        run_path_list_command(git, git_root, paths, git_config_args, &[], args, operation)?;
+    let fields = nul_fields(&output, operation)?;
+    if fields.len() % 2 != 0 {
+        return Err(invalid_reverse_index_output(
+            "git diff returned an incomplete name-status record",
+        ));
+    }
+    let expected = paths.iter().map(String::as_str).collect::<BTreeSet<_>>();
+    let mut statuses = BTreeMap::new();
+    for record in fields.chunks_exact(2) {
+        let status = record[0];
+        if status.len() != 1 || !status[0].is_ascii_alphabetic() {
+            return Err(invalid_reverse_index_output(
+                "git diff returned an invalid name-status value",
+            ));
+        }
+        let path = parse_expected_path(record[1], &expected, "git diff")?;
+        if statuses.insert(path, status[0]).is_some() {
+            return Err(invalid_reverse_index_output(
+                "git diff returned a duplicate path",
+            ));
+        }
+    }
+    Ok(statuses)
+}
+
+fn read_changed_paths(
+    git: &GitRunner,
+    git_root: &Path,
+    paths: &[String],
+    git_config_args: &[String],
+    extra_git_args: &[&str],
+    args: &[&str],
+    operation: &str,
+) -> io::Result<BTreeSet<String>> {
+    let output = run_path_list_command(
+        git,
+        git_root,
+        paths,
+        git_config_args,
+        extra_git_args,
+        args,
+        operation,
+    )?;
+    let expected = paths.iter().map(String::as_str).collect::<BTreeSet<_>>();
+    let mut changed = BTreeSet::new();
+    for path in nul_fields(&output, operation)? {
+        let path = parse_expected_path(path, &expected, operation)?;
+        if !changed.insert(path) {
+            return Err(invalid_reverse_index_output(
+                "Git returned a duplicate changed path",
+            ));
+        }
+    }
+    Ok(changed)
+}
+
+fn run_path_list_command(
+    git: &GitRunner,
+    git_root: &Path,
+    paths: &[String],
+    git_config_args: &[String],
+    extra_git_args: &[&str],
+    args: &[&str],
+    operation: &str,
+) -> io::Result<Vec<u8>> {
+    let mut command = git.command();
+    command
+        .args(git_config_args)
+        .args(extra_git_args)
+        .args(args)
+        .arg("--")
+        .args(paths)
+        .current_dir(git_root);
+    let output = git.output(command)?;
+    if !output.status.success() {
+        return Err(io::Error::other(format!(
+            "failed to {operation} (status {}): {}",
+            output.status,
+            String::from_utf8_lossy(&output.stderr).trim()
+        )));
+    }
+    Ok(output.stdout)
+}
+
+fn reverse_staging_error(message: impl Into<String>) -> io::Error {
+    io::Error::new(io::ErrorKind::PermissionDenied, message.into())
+}
+
+fn nul_fields<'a>(output: &'a [u8], operation: &str) -> io::Result<Vec<&'a [u8]>> {
+    if output.is_empty() {
+        return Ok(Vec::new());
+    }
+    let body = output.strip_suffix(&[0]).ok_or_else(|| {
+        invalid_reverse_index_output(&format!("{operation} returned unterminated output"))
+    })?;
+    let fields = body.split(|byte| *byte == 0).collect::<Vec<_>>();
+    if fields.iter().any(|field| field.is_empty()) {
+        return Err(invalid_reverse_index_output(&format!(
+            "{operation} returned an empty output field"
+        )));
+    }
+    Ok(fields)
+}
+
+fn parse_expected_path(
+    path: &[u8],
+    expected: &BTreeSet<&str>,
+    operation: &str,
+) -> io::Result<String> {
+    let path = std::str::from_utf8(path).map_err(|_| {
+        invalid_reverse_index_output(&format!("{operation} returned a non-UTF-8 path"))
+    })?;
+    if !expected.contains(path) {
+        return Err(invalid_reverse_index_output(&format!(
+            "{operation} returned an unexpected path"
+        )));
+    }
+    Ok(path.to_string())
+}
+
+fn invalid_reverse_index_output(message: &str) -> io::Error {
+    io::Error::new(io::ErrorKind::InvalidData, message)
+}