Reject duplicate file entries in apply_patch

codex-rs/apply-patch/src/invocation.rs

@@ -15,6 +15,7 @@ use crate::ApplyPatchFileChange;
 use crate::ApplyPatchFileUpdate;
 use crate::IoError;
 use crate::MaybeApplyPatchVerified;
+use crate::ensure_unique_hunk_paths;
 use crate::parser::Hunk;
 use crate::parser::ParseError;
 use crate::parser::parse_patch;
@@ -194,6 +195,7 @@ async fn try_verify_apply_patch_args(
         .map(|dir| cwd.join(dir))
         .transpose()?
         .unwrap_or_else(|| cwd.clone());
+    ensure_unique_hunk_paths(&hunks, &effective_cwd)?;
     let mut changes = HashMap::new();
     for hunk in hunks {
         let path = hunk.resolve_path(&effective_cwd)?;
@@ -526,6 +528,39 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn test_verified_rejects_duplicate_hunk_paths() {
+        let dir = tempdir().unwrap();
+        let path = dir.path().join("source.txt");
+        fs::write(&path, "first\nsecond\n").unwrap();
+        let argv = vec![
+            "apply_patch".to_string(),
+            r#"*** Begin Patch
+*** Update File: source.txt
+@@
+-first
++FIRST
+*** Update File: source.txt
+@@
+-second
++SECOND
+*** End Patch"#
+                .to_string(),
+        ];
+
+        let cwd = PathUri::from_host_native_path(dir.path()).expect("absolute test path");
+        let result =
+            maybe_parse_apply_patch_verified(&argv, &cwd, LOCAL_FS.as_ref(), /*sandbox*/ None)
+                .await;
+
+        assert_eq!(
+            result,
+            MaybeApplyPatchVerified::CorrectnessError(ApplyPatchError::DuplicateFilePath(
+                path.display().to_string()
+            ))
+        );
+    }
+
     #[tokio::test]
     async fn test_literal() {
         let args = strs_to_strings(&[

codex-rs/apply-patch/src/lib.rs

@@ -5,6 +5,7 @@ mod standalone_executable;
 mod streaming_parser;
 
 use std::collections::HashMap;
+use std::collections::HashSet;
 use std::io;
 use std::path::PathBuf;
 
@@ -57,6 +58,10 @@ pub enum ApplyPatchError {
         "patch detected without explicit call to apply_patch. Rerun as [\"apply_patch\", \"<patch>\"]"
     )]
     ImplicitInvocation,
+    /// A patch had multiple hunks for a single source path, which cannot be
+    /// represented faithfully in the approval payload.
+    #[error("patch contains multiple hunks for the same file: {0}")]
+    DuplicateFilePath(String),
 }
 
 impl From<std::io::Error> for ApplyPatchError {
@@ -320,6 +325,13 @@ pub async fn apply_hunks(
     fs: &dyn ExecutorFileSystem,
     sandbox: Option<&FileSystemSandboxContext>,
 ) -> Result<AppliedPatchDelta, ApplyPatchFailure> {
+    if let Err(error) = ensure_unique_hunk_paths(hunks, cwd) {
+        writeln!(stderr, "{error}")
+            .map_err(ApplyPatchError::from)
+            .map_err(ApplyPatchFailure::without_delta)?;
+        return Err(ApplyPatchFailure::without_delta(error));
+    }
+
     let mut delta = AppliedPatchDelta::empty();
     match apply_hunks_to_files(hunks, cwd, fs, sandbox, &mut delta).await {
         Ok(affected_paths) => {
@@ -346,6 +358,22 @@ pub async fn apply_hunks(
     }
 }
 
+pub(crate) fn ensure_unique_hunk_paths(
+    hunks: &[Hunk],
+    cwd: &PathUri,
+) -> Result<(), ApplyPatchError> {
+    let mut seen = HashSet::new();
+    for hunk in hunks {
+        let path = hunk.resolve_path(cwd)?;
+        if !seen.insert(path.clone()) {
+            return Err(ApplyPatchError::DuplicateFilePath(
+                path.inferred_native_path_string(),
+            ));
+        }
+    }
+    Ok(())
+}
+
 /// Applies each parsed patch hunk to the filesystem.
 /// Returns an error if any of the changes could not be applied.
 /// Tracks file paths affected by applying a patch, preserving the path spelling
@@ -934,6 +962,52 @@ mod tests {
         assert_eq!(contents, "ab\ncd\n");
     }
 
+    #[tokio::test]
+    async fn test_apply_patch_rejects_duplicate_hunk_paths_before_writing() {
+        let dir = tempdir().unwrap();
+        let path = dir.path().join("source.txt");
+        fs::write(&path, "first\nsecond\n").unwrap();
+        let patch = wrap_patch(
+            r#"*** Update File: source.txt
+@@
+-first
++FIRST
+*** Update File: source.txt
+@@
+-second
++SECOND"#,
+        );
+        let mut stdout = Vec::new();
+        let mut stderr = Vec::new();
+
+        let result = apply_patch(
+            &patch,
+            &PathUri::from_host_native_path(dir.path()).expect("absolute test path"),
+            &mut stdout,
+            &mut stderr,
+            LOCAL_FS.as_ref(),
+            /*sandbox*/ None,
+        )
+        .await;
+
+        let expected_error = ApplyPatchError::DuplicateFilePath(path.display().to_string());
+        assert_eq!(
+            result
+                .expect_err("duplicate paths should be rejected")
+                .into_parts(),
+            (expected_error, AppliedPatchDelta::empty())
+        );
+        assert_eq!(String::from_utf8(stdout).unwrap(), "");
+        assert_eq!(
+            String::from_utf8(stderr).unwrap(),
+            format!(
+                "patch contains multiple hunks for the same file: {}\n",
+                path.display()
+            )
+        );
+        assert_eq!(fs::read_to_string(path).unwrap(), "first\nsecond\n");
+    }
+
     #[tokio::test]
     async fn test_apply_patch_hunks_accept_relative_and_absolute_paths() {
         let dir = tempdir().unwrap();