The staged secret scan refuses a token with the git shim on PATH: the shim stops warning on gitleaks's diff argv, and a gitleaks that logged an error is no longer a pass

deny.toml

@@ -45,6 +45,9 @@ allow = [
     "Unicode-3.0",
     "Unicode-DFS-2016",
     "Unlicense",
+    # foldhash, which serde_json depends on. Permissive and OSI
+    # approved: no attribution in the binary, no copyleft.
+    "Zlib",
 ]
 # A dual-licensed crate is taken under the first of its licences this list
 # admits, which is how `MIT OR Apache-2.0` resolves without a note per crate.

src/shim.rs

@@ -1859,9 +1859,28 @@ impl Shim {
         if bare.verb == valued.verb && bare.noun == valued.noun {
             return Reading::Absent;
         }
+        // And a disagreement about the noun alone is no doubt where no entry
+        // names this verb with any noun: whichever word it is, no `match` entry
+        // could have named the pair. `git diff -U0 --no-ext-diff --staged .` is
+        // `diff` under both readings and only `.` moves, and calling it unclear
+        // wrote a line to stderr under every `gitleaks git --staged`, which
+        // gitleaks reads as a failed diff and answers by scanning nothing.
+        if bare.verb == valued.verb && !self.names_a_noun_of(&bare.verb) {
+            return Reading::Absent;
+        }
         Reading::Unclear(flag)
     }
 
+    /// Whether any `match` entry names this verb, whatever noun it pairs it
+    /// with.
+    fn names_a_noun_of(&self, verb: &str) -> bool {
+        self.match_.iter().any(|entry| {
+            entry
+                .split_once(':')
+                .is_some_and(|(named, _)| named == verb)
+        })
+    }
+
     /// argv as the command itself will read it, with an alias in the verb
     /// position expanded once.
     ///
@@ -7039,12 +7058,26 @@ mod tests {
             "status --short --branch",
             "diff --stat --cached",
             "log --format=%H -5",
+            // The argv `gitleaks git --staged` runs, which moves the noun and
+            // not the verb: no entry names `diff` with anything.
+            "-C . diff -U0 --no-ext-diff --staged .",
+            "diff -U0 --no-ext-diff --staged .",
         ] {
             assert!(
                 matches!(git_push().reading(&argv(line)), Reading::Absent),
                 "{line}"
             );
         }
+        // A verb an entry DOES pair with a noun keeps the doubt, because there
+        // the word that moved is the one the match turns on.
+        let push_to_main = Shim {
+            match_: vec!["push:main".into()],
+            ..git_push()
+        };
+        assert!(matches!(
+            push_to_main.reading(&argv("push --fic-a --fic-b --fic-c x")),
+            Reading::Unclear(flag) if flag == "--fic-a"
+        ));
     }
 
     #[test]

src/supply.rs

@@ -1838,8 +1838,18 @@ fn gitleaks_version_mismatch() -> Option<String> {
 /// gitleaks' could-not-look, which its own exit code names once the finding
 /// has been moved off `1`: anything but `0` and [`GITLEAKS_FOUND`] is a scan
 /// that did not finish, the partial scan included.
+///
+/// And a `0` that logged an error. gitleaks 8.30.1 treats any line git writes
+/// to stderr as a failed diff, logs `ERR error="stderr is not empty"`, scans
+/// nothing, says `no leaks found` and exits 0 -- so a warning from whatever
+/// `git` is on PATH turned the staged scan into a pass over a diff nobody read.
 fn gitleaks_could_not_look(code: i32, _stdout: &str, stderr: &str) -> Option<String> {
-    if matches!(code, 0 | GITLEAKS_FOUND) {
+    if code == 0 {
+        return gitleaks_logged_an_error(stderr).map(|line| {
+            format!("gitleaks exited 0 but logged an error, so what it read is unknown: {line}")
+        });
+    }
+    if code == GITLEAKS_FOUND {
         return None;
     }
     Some(format!(
@@ -1848,6 +1858,15 @@ fn gitleaks_could_not_look(code: i32, _stdout: &str, stderr: &str) -> Option<Str
     ))
 }
 
+/// The first line gitleaks logged at `ERR`, which under `--no-color` is the
+/// second word of its line: `9:52PM ERR [git] ...`.
+fn gitleaks_logged_an_error(stderr: &str) -> Option<&str> {
+    stderr
+        .lines()
+        .map(str::trim)
+        .find(|line| line.split_whitespace().nth(1) == Some("ERR"))
+}
+
 /// A file that exists for one child process and is removed on the way out.
 ///
 /// Hand-rolled rather than a crate, because this is the only place the binary
@@ -2112,6 +2131,21 @@ mod tests {
         assert!(gitleaks_could_not_look(126, "", "unknown flag").is_some());
     }
 
+    #[test]
+    fn gitleaks_exit_0_that_logged_an_error_is_not_a_pass() {
+        // What gitleaks 8.30.1 printed, `--no-color`, for `git --staged` when
+        // the git on PATH wrote one line to stderr: it exited 0.
+        let stderr = "9:52PM ERR [git] noise on stderr\n\
+                      9:52PM ERR error=\"stderr is not empty\"\n\
+                      9:52PM INF 0 commits scanned.\n\
+                      9:52PM INF no leaks found\n";
+        let reason = gitleaks_could_not_look(0, "", stderr).unwrap();
+        assert!(reason.contains("[git] noise on stderr"), "{reason}");
+        assert!(gitleaks_could_not_look(0, "", "9:52PM INF no leaks found\n").is_none());
+        // `ERR` inside a message is not the level.
+        assert!(gitleaks_could_not_look(0, "", "9:52PM INF path ERR.txt\n").is_none());
+    }
+
     /// Paths under the fixture, as strings, so a failure names what was found.
     fn relative(root: &std::path::Path, found: &[std::path::PathBuf]) -> Vec<String> {
         found

tests/supply_chain_cli.rs

@@ -1987,6 +1987,78 @@ fn staged_exit_1_from_gitleaks_is_a_scan_that_did_not_finish() {
     assert!(said.contains("NOT CHECKED: gitleaks exited 1"), "{said}");
 }
 
+/// What gitleaks 8.30.1 does with a git that wrote to stderr, measured: logs
+/// the line at `ERR`, scans nothing, says `no leaks found`, and exits 0.
+const GITLEAKS_BLINDED: &str = "echo '9:52PM ERR [git] warning: something' >&2\n\
+     echo '9:52PM ERR error=\"stderr is not empty\"' >&2\n\
+     echo '9:52PM INF no leaks found' >&2\n\
+     exit 0";
+
+#[test]
+fn staged_exit_0_from_a_gitleaks_that_logged_an_error_is_not_a_pass() {
+    let root = inheriting_credentials();
+    let tools = every_scanner(&gitleaks_stub(GITLEAKS_PINNED, GITLEAKS_BLINDED));
+    let output = staged(&root, &tools, &[]);
+    let said = text(&output);
+    assert_eq!(code(&output), 2, "{said}");
+    assert!(said.contains("stderr is not empty"), "{said}");
+    assert!(!said.contains("staged diff passed"), "{said}");
+}
+
+/// The two halves of #410 together: a token staged, uphold linked in front of
+/// git as an install puts it with a `[[shim]]` for git declared, and a gitleaks
+/// that reads the index through the argv the real one runs and answers as the
+/// real one does -- a git that wrote to stderr is a diff it did not read. The
+/// shim warned on that argv, so the scan read nothing and the commit passed.
+#[test]
+fn a_staged_token_is_refused_with_the_git_shim_on_path() {
+    let root = tracked();
+    std::fs::write(
+        root.join("policy/principles.toml"),
+        format!(
+            "{WITH_CREDENTIALS}\n[rule.no-wip-branch]\nmessage = \"unfinished\"\n\
+             regexp = '^WIP'\ncommand.before = [\"git push\"]\n\
+             \n[[shim]]\ncommand = \"git\"\nmatch = [\"push:*\"]\n\
+             target = \"git-remote\"\nscope = \"public-target\"\ncollect = \"git-refs\"\n"
+        ),
+    )
+    .unwrap();
+    commit(&root, "inherit credentials and shim git");
+    // Spelled at run time, so this file holds no token of its own.
+    write(
+        &root,
+        "leak.txt",
+        &format!("token = \"ghp_{}\"\n", "a1".repeat(18)),
+    );
+    support::git(&root, &["add", "leak.txt"]);
+
+    let reader = "out=\"$STUB_LOG.diff\"\n\
+         err=$(git -C . diff -U0 --no-ext-diff --staged . 2>&1 >\"$out\")\n\
+         if [ -n \"$err\" ]; then\n\
+           echo \"9:52PM ERR [git] $err\" >&2\n\
+           echo '9:52PM ERR error=\"stderr is not empty\"' >&2\n\
+           exit 0\n\
+         fi\n\
+         while IFS= read -r line; do\n\
+           case \"$line\" in *ghp_*) echo 'RuleID: github-pat'; exit 3;; esac\n\
+         done < \"$out\"\n\
+         exit 0";
+    let tools = every_scanner(&gitleaks_stub(GITLEAKS_PINNED, reader));
+    let front = root.join("front");
+    std::fs::create_dir_all(&front).unwrap();
+    std::os::unix::fs::symlink(env!("CARGO_BIN_EXE_uphold"), front.join("git")).unwrap();
+    let mut path = tools.as_os_str().to_owned();
+    path.push(":");
+    path.push(front.as_os_str());
+    path.push(":/usr/bin:/bin");
+
+    let output = invoke(&root, &path, &["--staged"], &[]);
+    let said = text(&output);
+    assert_eq!(code(&output), 1, "{said}");
+    assert!(said.contains("RuleID: github-pat"), "{said}");
+    assert!(!said.contains("uphold shim"), "{said}");
+}
+
 #[test]
 fn staged_in_a_policy_that_does_not_inherit_credentials_asks_nothing() {
     let root = repository();