From 92fb2860ae1b622b463c557880255b592f2cebe4 Mon Sep 17 00:00:00 2001 From: HackingGate Date: Thu, 22 Oct 2026 22:14:02 +0900 Subject: [PATCH 1/2] cargo-deny admits the Zlib licence, which foldhash carries under serde_json 1.0.154 serde_json 1.0.154 (#404) depends on foldhash 0.2.0, licensed Zlib, which the allow list did not name. CI does not run cargo-deny, so main stayed green while the pre-push `uphold supply-chain` refused every push from it. Zlib is permissive and OSI approved, with no copyleft and no attribution required in the binary. Claude-Session: https://claude.ai/code/session_01HM96sKSAyDhLk9smu82u5V --- deny.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/deny.toml b/deny.toml index a49c4f2f..1f65d8fc 100644 --- a/deny.toml +++ b/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. From 1a9adaf6e173112bcd04966b5534c38f44c32fb3 Mon Sep 17 00:00:00 2001 From: HackingGate Date: Thu, 22 Oct 2026 22:07:20 +0900 Subject: [PATCH 2/2] 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 Two defects met in `uphold supply-chain --staged` on any host where `git` is the uphold shim. The shim called `git diff -U0 --no-ext-diff --staged .` unclear and wrote that to stderr. Both readings of the unknown options find `diff`; only the noun moved, and no `match` entry names `diff` with any noun, so the noun could not decide anything. `reading` now answers `Absent` there. A verb an entry does pair with a noun keeps the doubt. gitleaks 8.30.1 treats any git stderr as a failed diff: it logs `ERR error="stderr is not empty"`, scans nothing, and exits 0. uphold read the 0 as a pass. An `ERR` line under exit 0 is now could-not-look, exit 2, naming the line, so a warning from any git on PATH no longer blinds the scan in silence. The CLI test stages a token with uphold linked in front of git and a gitleaks stub that reads the index through the real argv and answers as gitleaks does; it fails with either fix reverted. Checked live against gitleaks 8.30.1 as well: 1.35.0 passes the staged token, this refuses it. Closes #410. Claude-Session: https://claude.ai/code/session_01HM96sKSAyDhLk9smu82u5V --- src/shim.rs | 33 ++++++++++++++++++ src/supply.rs | 36 +++++++++++++++++++- tests/supply_chain_cli.rs | 72 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) diff --git a/src/shim.rs b/src/shim.rs index 9855a999..36a2d70f 100644 --- a/src/shim.rs +++ b/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] diff --git a/src/supply.rs b/src/supply.rs index 1e31b69f..847416ec 100644 --- a/src/supply.rs +++ b/src/supply.rs @@ -1838,8 +1838,18 @@ fn gitleaks_version_mismatch() -> Option { /// 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 { - 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 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 { found diff --git a/tests/supply_chain_cli.rs b/tests/supply_chain_cli.rs index 314da4c9..e7b8c68c 100644 --- a/tests/supply_chain_cli.rs +++ b/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();