From 9798514b71c25f492519d8875b3ca0621a56c546 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:55:56 +0530 Subject: [PATCH] fix(scan): make git-history and staged modes behave like the others git-history ignored --exclude and the baseline-file exclusion, so it scanned every historical revision of a committed baseline, and it keyed every finding under a synthetic path that no baseline entry could ever match. Because git log -p emits the same diff framing as git diff --cached, history now reuses the staged parser: real file paths, real line numbers, exclusions applied. git log gets the same config hardening as git diff. diff.relative made git emit cwd-relative paths and drop changes outside the current directory, so running the staged scan from a subdirectory silently reported clean while a staged secret sat one level up. It is now pinned off alongside the other diff settings. --- CHANGELOG.md | 2 + README.md | 2 +- src/scanner.rs | 35 ++++++++++++----- tests/scanner_tests.rs | 87 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43ef23f..2f7fbc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ All notable changes to this project will be documented in this file. ### Fixed +- `scan --git-history` applies `--exclude`, skips the baseline file, and reports real file paths instead of a synthetic `` key that no baseline could match +- `scan --staged` is not fooled by `diff.relative`, which made git drop changes outside the current directory - `--output` files are created readable only by their owner - Config is not trusted from a world-writable directory, so a `.keywatch.toml` dropped in `/tmp` cannot weaken scans beneath it - `KEYWATCH_CONFIG_PATH` is ignored in trusted mode when it points inside the tree being scanned diff --git a/README.md b/README.md index e21ad72..da5ad30 100644 --- a/README.md +++ b/README.md @@ -184,7 +184,7 @@ key-watch verify-integrity - `scan --no-config-discovery` - Ignore discovered repository config unless `--config` is explicit - `scan --format ` - Choose the report format written to stdout or the output file - `scan --stdin` - Read content from stdin instead of files -- `scan --git-history` - Scan git history (`git log -p`) for committed secrets +- `scan --git-history` - Scan git history (`git log -p`) for committed secrets; findings carry real file paths, and `--exclude` and baselines apply - `scan --staged [path...]` - Scan only the added lines of the staged diff (run from inside the repository; paths narrow the diff as git pathspecs); findings keep real file paths and line numbers, so `--baseline` and `--exclude` compose. Files git renders as binary (e.g. `-diff` in `.gitattributes`) are listed in `excluded_files` rather than scanned. Note: a multi-line secret added across separate commits can span hunks the diff scan never sees together — the pre-push whole-tree scan remains the backstop for that case - `scan --output ` - Save report to file - `scan --verbose` - Print full JSON output (matched text is redacted; see `--show-secrets`) diff --git a/src/scanner.rs b/src/scanner.rs index 01af085..4837458 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -354,10 +354,19 @@ pub fn run_scan( .unwrap_or_else(|| Path::new(".")); let mut command = std::process::Command::new("git"); command.current_dir(git_root).args([ + "--literal-pathspecs", "-c", "diff.external=", "-c", "color.ui=false", + "-c", + "diff.mnemonicPrefix=false", + "-c", + "diff.noprefix=false", + "-c", + "core.quotePath=false", + "-c", + "diff.relative=false", "log", "-p", "-U0", @@ -366,13 +375,20 @@ pub fn run_scan( "--no-color", ]); - let (findings, total_lines) = scan_git_output( + // `git log -p` emits the same diff framing as `git diff --cached`, so + // history reuses the staged parser. That gives it real file paths + // instead of a synthetic "" key — which no baseline + // entry could ever match — plus --exclude and baseline-file + // exclusion, none of which this mode previously applied. + let exclude_patterns = compile_exclude_patterns(args, config)?; + let history = scan_git_output( command, ScannerError::GitLogNonZero, |reader| { - scan_stream( + scan_staged_diff( reader, - "", + &exclude_patterns, + excluded_baseline.as_ref(), &multiline_detectors, &line_detectors, ) @@ -380,14 +396,11 @@ pub fn run_scan( |source| ScannerError::RunGitLog { source }, )?; - let metadata = ScanMetadata { - files_scanned: 1, - total_lines, - excluded_files: Vec::new(), - suppressed_by_baseline: 0, - }; + let mut metadata = history.metadata; + // Blobs are only re-readable from the index, not from history. + metadata.excluded_files.extend(history.undiffable_files); - return Ok((findings, metadata)); + return Ok((history.findings, metadata)); } if args.staged { @@ -409,6 +422,8 @@ pub fn run_scan( "diff.noprefix=false", "-c", "core.quotePath=false", + "-c", + "diff.relative=false", "diff", "--cached", "-U0", diff --git a/tests/scanner_tests.rs b/tests/scanner_tests.rs index 13d69cb..f17588f 100644 --- a/tests/scanner_tests.rs +++ b/tests/scanner_tests.rs @@ -1910,3 +1910,90 @@ fn test_baseline_suppression_is_reported() -> Result<(), String> { let _ = fs::remove_dir_all(&repo_dir); Ok(()) } + +#[test] +fn test_git_history_attributes_real_paths_and_honours_excludes() -> Result<(), String> { + if !git_available() { + return Ok(()); + } + + // History findings used to be keyed under a synthetic "" + // path, which no baseline entry could match, and this mode ignored + // --exclude entirely. + let repo_dir = unique_temp_dir("git_history_paths"); + let _ = fs::remove_dir_all(&repo_dir); + init_git_repo(&repo_dir)?; + commit_file( + &repo_dir, + "leak.txt", + "aws_access_key_id = AKIAIOSFODNN7EXAMPLE\n", + "add", + )?; + + let verbose = run_git_history_scan(&repo_dir, &["--verbose", "--no-baseline-discovery"])?; + let stdout = String::from_utf8_lossy(&verbose.stdout); + assert!( + stdout.contains("\"file_path\": \"leak.txt\""), + "history findings must carry the real path, got:\n{stdout}" + ); + assert!( + !stdout.contains(""), + "the synthetic path must be gone, got:\n{stdout}" + ); + + let excluded = run_git_history_scan( + &repo_dir, + &["--exclude", "leak.txt", "--no-baseline-discovery"], + )?; + assert_eq!( + excluded.status.code(), + Some(0), + "--exclude must apply to git-history scans" + ); + + let _ = fs::remove_dir_all(&repo_dir); + Ok(()) +} + +#[test] +fn test_staged_scan_survives_diff_relative_from_subdirectory() -> Result<(), String> { + if !git_available() { + return Ok(()); + } + + // diff.relative makes git emit cwd-relative paths and drop changes + // outside the cwd, which silently hid staged secrets. + let repo_dir = unique_temp_dir("staged_diff_relative"); + let _ = fs::remove_dir_all(&repo_dir); + init_git_repo(&repo_dir)?; + fs::create_dir_all(repo_dir.join("sub")).map_err(|e| e.to_string())?; + fs::write(repo_dir.join("sub/keep.txt"), "clean\n").map_err(|e| e.to_string())?; + commit_file(&repo_dir, "root.txt", "clean\n", "init")?; + let status = Command::new("git") + .args(["config", "diff.relative", "true"]) + .current_dir(&repo_dir) + .status() + .map_err(|e| e.to_string())?; + assert!(status.success()); + stage_file( + &repo_dir, + "root.txt", + "clean\naws_access_key_id = AKIAIOSFODNN7EXAMPLE\n", + )?; + + let output = Command::new(env!("CARGO_BIN_EXE_key-watch")) + .args(["scan", "--staged", "--no-baseline-discovery"]) + .env("KEYWATCH_CONFIG_PATH", detectors_config_path()) + .current_dir(repo_dir.join("sub")) + .output() + .expect("run key-watch"); + + assert_eq!( + output.status.code(), + Some(1), + "diff.relative must not hide a staged secret outside the cwd" + ); + + let _ = fs::remove_dir_all(&repo_dir); + Ok(()) +}