From c279cbe85a86b01f3c1f71ada2763c9f0cf01fb5 Mon Sep 17 00:00:00 2001
From: damocles <damocles@hyperhive>
Date: Tue, 26 May 2026 19:16:51 +0200
Subject: [PATCH] hive-forge: parse git-quoted paths in diff parser (#222
 followup)

---
 hive-forge/src/verbs/diff.rs | 117 ++++++++++++++++++++++++++++++++++-
 1 file changed, 114 insertions(+), 3 deletions(-)

diff --git a/hive-forge/src/verbs/diff.rs b/hive-forge/src/verbs/diff.rs
index a9575ae3..e35793b9 100644
--- a/hive-forge/src/verbs/diff.rs
+++ b/hive-forge/src/verbs/diff.rs
@@ -130,10 +130,62 @@ impl CollapseState {
 /// `rest` is everything after `diff --git `, e.g. `a/foo b/foo`
 /// or `"a/path with space" "b/path with space"`. Return the
 /// post-rename (`b/`-side) path so renames report the new path.
+///
+/// Git uses C-style quoting (`\"`, `\\`, octal escapes) for paths
+/// with spaces or unusual bytes — we don't unescape because we
+/// only need the bytewise file name for the lockfile allowlist
+/// match. We do parse the quoted-vs-unquoted form correctly so a
+/// whitespace-containing path doesn't break `split_whitespace`.
 fn parse_diff_git_path(rest: &str) -> Option<String> {
-    let token = rest.split_whitespace().nth(1)?;
-    let unquoted = token.trim_matches('"');
-    Some(unquoted.strip_prefix("b/").unwrap_or(unquoted).to_owned())
+    if let Some(after_open) = rest.strip_prefix('"') {
+        // Quoted form: `"a/<path>" "b/<path>"`. Find the closing
+        // quote of the a-side (skipping `\"` escapes so a path
+        // containing `"` doesn't terminate early).
+        let mut iter = after_open.char_indices();
+        let mut a_close = None;
+        loop {
+            let Some((i, c)) = iter.next() else { break };
+            if c == '\\' {
+                // Skip the next char — it's part of the escape.
+                iter.next();
+                continue;
+            }
+            if c == '"' {
+                a_close = Some(i);
+                break;
+            }
+        }
+        let a_close = a_close?;
+        // After the a-side closing quote, we expect `" "b/...`.
+        // `after_open` skipped the leading `"`, so the b-side
+        // starts in `after_open[a_close + 1..]` — strip leading
+        // space + opening quote + `b/`, then strip trailing `"`.
+        let after_a = after_open.get(a_close + 1..)?;
+        let after_a = after_a.strip_prefix(' ')?;
+        let b_inside = after_a.strip_prefix('"')?;
+        // Find b-side's closing quote with the same escape rule.
+        let mut iter = b_inside.char_indices();
+        let mut b_close = None;
+        loop {
+            let Some((i, c)) = iter.next() else { break };
+            if c == '\\' {
+                iter.next();
+                continue;
+            }
+            if c == '"' {
+                b_close = Some(i);
+                break;
+            }
+        }
+        let b_close = b_close?;
+        let b_path = b_inside.get(..b_close)?;
+        Some(b_path.strip_prefix("b/").unwrap_or(b_path).to_owned())
+    } else {
+        // Unquoted: paths have no whitespace, so `split_whitespace`
+        // gives exactly two tokens.
+        let token = rest.split_whitespace().nth(1)?;
+        Some(token.strip_prefix("b/").unwrap_or(token).to_owned())
+    }
 }
 
 /// File-name match against a small whitelist of well-known
@@ -299,4 +351,63 @@ index 1111..2222 100644
             Some("dir/Cargo.lock".to_owned())
         );
     }
+
+    #[test]
+    fn parse_diff_git_path_handles_quoted_paths_with_spaces() {
+        // Git quotes paths with spaces / unusual chars per
+        // `core.quotePath`. The b-side path must survive intact.
+        assert_eq!(
+            parse_diff_git_path("\"a/foo bar\" \"b/foo bar\""),
+            Some("foo bar".to_owned())
+        );
+        // Rename with quoted sides.
+        assert_eq!(
+            parse_diff_git_path("\"a/old name\" \"b/new name\""),
+            Some("new name".to_owned())
+        );
+        // Lockfile inside a directory whose name has a space.
+        assert_eq!(
+            parse_diff_git_path("\"a/dir with space/Cargo.lock\" \"b/dir with space/Cargo.lock\""),
+            Some("dir with space/Cargo.lock".to_owned())
+        );
+    }
+
+    #[test]
+    fn parse_diff_git_path_handles_escaped_quote_inside_path() {
+        // Git escapes embedded `"` as `\"`. The closing-quote
+        // search must skip these so it doesn't terminate early.
+        // Path is literally `a/has"quote` / `b/has"quote`.
+        let rest = r#""a/has\"quote" "b/has\"quote""#;
+        assert_eq!(
+            parse_diff_git_path(rest),
+            // Bytewise: backslash + quote stay in the result
+            // because we don't unescape (allowlist match is by
+            // file name, never contains escapes).
+            Some(r#"has\"quote"#.to_owned())
+        );
+    }
+
+    #[test]
+    fn collapse_recognises_lockfile_in_quoted_path() {
+        // Path "a/odd dir/Cargo.lock" forces git to use the
+        // quoted form. We must still detect it as a lockfile.
+        let diff = "\
+diff --git \"a/odd dir/Cargo.lock\" \"b/odd dir/Cargo.lock\"
+index 1111..2222 100644
+--- \"a/odd dir/Cargo.lock\"
++++ \"b/odd dir/Cargo.lock\"
+@@ -1,1 +1,1 @@
+-old
++new
+";
+        let out = collapse_autogenerated(diff);
+        // Content must be suppressed (lockfile detected).
+        assert!(!out.contains("old\n"), "lock content leaked: {out}");
+        assert!(!out.contains("+new"), "lock content leaked: {out}");
+        // Placeholder uses the parsed b-side path (without quotes).
+        assert!(
+            out.contains("[odd dir/Cargo.lock: +1 -1"),
+            "expected stat placeholder for quoted-path lockfile, got: {out}"
+        );
+    }
 }