Harden the git argument handling flagged by a security review

CLAUDE.md

@@ -50,8 +50,8 @@ same way and test the argument vector, not a real repo.
 
 Tests are in-module `#[cfg(test)] mod tests`, pure functions only, no `tests/` directory.
 
-Numeric literals are named `const`s at the top of the module. `ponytail:` marks a deliberate
-simplification and names its ceiling (`term.rs:82`).
+Numeric literals are named `const`s at the top of the module. A comment on a deliberate shortcut
+names its ceiling and how to lift it (`term.rs:82`).
 
 `interactive.rs` imports crate-private items from the root; `main.rs` items stay non-`pub`.
 

src/interactive.rs

@@ -16,6 +16,7 @@ const ACTION_MENU: &str =
     "action: [p] push branches  [c] commit  [b] commit on new branch  [q] cancel > ";
 const FLAGS_HELP: &str =
     "push flags? (l = --force-with-lease, n = --no-verify, ln = both, empty = none) > ";
+const FLAGS_REJECTED: &str = "expected l, n, ln or nothing";
 const ROWS_RESERVED_FOR_CHROME: usize = 4;
 const FILES_PREVIEW_LIMIT: usize = 10;
 /// Cursor, checkbox, status letter and the gaps around them, before the file path.
@@ -719,8 +720,25 @@ fn ask(question: &str) -> String {
 }
 
 fn ask_push_options() -> PushOptions {
-    let answer = ask(FLAGS_HELP);
-    PushOptions { force_with_lease: answer.contains('l'), no_verify: answer.contains('n') }
+    loop {
+        if let Some(options) = parse_push_options(&ask(FLAGS_HELP)) {
+            return options;
+        }
+        println!("{FLAGS_REJECTED}");
+    }
+}
+
+/// The whole answer has to be a flag word: a substring match would turn "none" into
+/// --no-verify and skip the pre-push hooks without saying so.
+fn parse_push_options(answer: &str) -> Option<PushOptions> {
+    let (force_with_lease, no_verify) = match answer {
+        "" => (false, false),
+        "l" => (true, false),
+        "n" => (false, true),
+        "ln" | "nl" => (true, true),
+        _ => return None,
+    };
+    Some(PushOptions { force_with_lease, no_verify })
 }
 
 #[cfg(test)]
@@ -736,6 +754,17 @@ mod tests {
         Upstream { remote: "origin".to_string(), remote_ref: format!("refs/heads/{name}") }
     }
 
+    #[test]
+    fn takes_a_whole_flag_word_only() {
+        let none = parse_push_options("").unwrap();
+        assert!(!none.force_with_lease && !none.no_verify);
+        let both = parse_push_options("nl").unwrap();
+        assert!(both.force_with_lease && both.no_verify);
+        assert!(parse_push_options("n").unwrap().no_verify);
+        assert!(parse_push_options("none").is_none());
+        assert!(parse_push_options("lol").is_none());
+    }
+
     #[test]
     fn pushes_to_the_tracked_ref() {
         let options = PushOptions { force_with_lease: true, no_verify: false };

src/main.rs

@@ -19,6 +19,8 @@ const SKIP_DIRS: &[&str] = &[
     "build",
 ];
 const UNTRACKED_SIZE_LIMIT_BYTES: u64 = 1 << 20;
+/// Bounds the recursion: raise it if repos ever nest deeper than this.
+const MAX_SCAN_DEPTH: usize = 32;
 const SECONDS_PER_MINUTE: i64 = 60;
 const SECONDS_PER_HOUR: i64 = 60 * SECONDS_PER_MINUTE;
 const SECONDS_PER_DAY: i64 = 24 * SECONDS_PER_HOUR;
@@ -164,7 +166,7 @@ fn main() {
 
     let started = Instant::now();
     let mut repos = Vec::new();
-    collect_repos(&options.root, &mut repos);
+    collect_repos(&options.root, MAX_SCAN_DEPTH, &mut repos);
     let mut reports: Vec<RepoReport> = repos
         .par_iter()
         .map(|repo| scan_repo(repo, cutoff))
@@ -223,7 +225,10 @@ fn now_unix() -> u64 {
     SystemTime::now().duration_since(UNIX_EPOCH).map(|since| since.as_secs()).unwrap_or(0)
 }
 
-fn collect_repos(dir: &Path, found: &mut Vec<PathBuf>) {
+fn collect_repos(dir: &Path, depth: usize, found: &mut Vec<PathBuf>) {
+    if depth == 0 {
+        return;
+    }
     if dir.join(".git").exists() {
         found.push(dir.to_path_buf());
         return;
@@ -241,7 +246,7 @@ fn collect_repos(dir: &Path, found: &mut Vec<PathBuf>) {
         if name.starts_with('.') || SKIP_DIRS.contains(&name.as_ref()) {
             continue;
         }
-        collect_repos(&entry.path(), found);
+        collect_repos(&entry.path(), depth - 1, found);
     }
 }
 
@@ -272,15 +277,16 @@ fn scan_repo(repo: &Path, cutoff: u64) -> RepoReport {
 fn scan_working_tree(repo: &Path, cutoff: u64) -> WorkingTreeChanges {
     let mut changes = WorkingTreeChanges { files: Vec::new(), last_change: None };
 
-    let tracked = git(repo, &["diff", "--numstat", "--no-renames", "HEAD"]).unwrap_or_default();
-    for line in tracked.lines() {
-        let mut fields = line.splitn(3, '\t');
+    let tracked =
+        git(repo, &["diff", "--numstat", "--no-renames", "-z", "HEAD"]).unwrap_or_default();
+    for record in split_nul(&tracked) {
+        let mut fields = record.splitn(3, '\t');
         let (Some(added), Some(removed), Some(path)) = (fields.next(), fields.next(), fields.next())
         else {
             continue;
         };
         let full_path = repo.join(path);
-        let Some(changed_at) = change_time(&full_path, cutoff) else {
+        let Some(changed_at) = change_time(&full_path, repo, cutoff) else {
             continue;
         };
         changes.files.push(ChangedFile {
@@ -293,10 +299,11 @@ fn scan_working_tree(repo: &Path, cutoff: u64) -> WorkingTreeChanges {
         changes.last_change = changes.last_change.max(Some(changed_at));
     }
 
-    let untracked = git(repo, &["ls-files", "--others", "--exclude-standard"]).unwrap_or_default();
-    for path in untracked.lines() {
+    let untracked =
+        git(repo, &["ls-files", "--others", "--exclude-standard", "-z"]).unwrap_or_default();
+    for path in split_nul(&untracked) {
         let full_path = repo.join(path);
-        let Some(changed_at) = change_time(&full_path, cutoff) else {
+        let Some(changed_at) = change_time(&full_path, repo, cutoff) else {
             continue;
         };
         changes.files.push(ChangedFile {
@@ -312,18 +319,26 @@ fn scan_working_tree(repo: &Path, cutoff: u64) -> WorkingTreeChanges {
     changes
 }
 
-fn change_time(path: &Path, cutoff: u64) -> Option<u64> {
+fn change_time(path: &Path, repo: &Path, cutoff: u64) -> Option<u64> {
     // A deleted file has no mtime of its own, so fall back to the nearest surviving
-    // ancestor: removing an entry updates the mtime of the directory holding it.
+    // ancestor: removing an entry updates the mtime of the directory holding it. The walk
+    // stops at the repo, above which a directory says nothing about this repo.
     let modified = path
         .ancestors()
+        .take_while(|ancestor| ancestor.starts_with(repo))
         .find_map(|ancestor| std::fs::metadata(ancestor).and_then(|entry| entry.modified()).ok())?;
     let seconds = modified.duration_since(UNIX_EPOCH).ok()?.as_secs();
     (seconds >= cutoff).then_some(seconds)
 }
 
+/// Git leaves a NUL terminated path verbatim, where its line output quotes any path
+/// holding a quote, a backslash or a control character.
+fn split_nul(output: &str) -> impl Iterator<Item = &str> {
+    output.split('\0').filter(|record| !record.is_empty())
+}
+
 fn count_lines(path: &Path) -> u64 {
-    // ponytail: huge untracked blobs count as a file but not as lines, reading them would dominate runtime.
+    // Huge untracked blobs count as a file but not as lines: reading them would dominate runtime.
     let Ok(metadata) = std::fs::metadata(path) else {
         return 0;
     };
@@ -828,6 +843,17 @@ mod tests {
         assert_eq!(join_within_width(&single, 20), single[0]);
     }
 
+    #[test]
+    fn keeps_the_paths_git_would_have_quoted() {
+        let numstat = "1\t0\tplain.txt\u{0}2\t1\twe\"ird.txt\u{0}\n";
+        assert_eq!(
+            split_nul(numstat).collect::<Vec<_>>(),
+            ["1\t0\tplain.txt", "2\t1\twe\"ird.txt", "\n"]
+        );
+        assert_eq!(split_nul("new\"file.txt\u{0}").collect::<Vec<_>>(), ["new\"file.txt"]);
+        assert_eq!(split_nul("").next(), None);
+    }
+
     #[test]
     fn parses_periods() {
         assert_eq!(parse_period("24h"), Some(SECONDS_PER_DAY as u64));

src/term.rs

@@ -79,7 +79,7 @@ pub fn read_key() -> Option<Key> {
 }
 
 fn read_arrow_key() -> Option<Key> {
-    // ponytail: a bare Esc press swallows the next two bytes, q and ctrl-c are the documented exits.
+    // A bare Esc press swallows the next two bytes: q and ctrl-c are the documented exits.
     let mut sequence = [0u8; 2];
     std::io::stdin().read_exact(&mut sequence).ok()?;
     Some(match sequence {