diff --git a/deny.toml b/deny.toml index a49c4f2..1f65d8f 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. diff --git a/src/shim.rs b/src/shim.rs index 9855a99..36a2d70 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 1e31b69..847416e 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 314da4c..e7b8c68 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();