From 93b4574a7035da8075fc33df91da08c2ff3b3dac Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:58:12 +0530 Subject: [PATCH] chore: remove dead code and finish the incomplete fixes Shell::as_str, OutputFormat::as_str, ExitMode::as_str and KeywatchConfig::load had no call sites anywhere in src/ or tests/. --update-baseline loaded and parsed the same baseline file twice per run, once to filter and once to update. is_baseline_file ran realpath(2) on every scanned file now that baseline discovery is the default; it compares file names first, so only candidates that could possibly be the baseline pay for the syscall. The broken-pipe fix only covered the scan summary, so 'hook install | head' and 'init | head' still panicked. The writer moved to utils and is used by every command. update_with_findings only ever appends, so entries for deleted files and rotated credentials suppressed findings forever, and the update-baseline workflow could only produce additive diffs. --prune-baseline rebuilds from what the scan actually found. --- .keywatch-baseline.json | 7 +++ CHANGELOG.md | 6 ++- README.md | 1 + src/cli.rs | 34 ++----------- src/config/mod.rs | 9 ---- src/hooks.rs | 20 ++++---- src/lib.rs | 37 +++++++------- src/scanner.rs | 8 ++- src/utils.rs | 15 ++++++ tests/cli_validation_tests.rs | 4 ++ tests/scanner_tests.rs | 91 +++++++++++++++++++++++++++++++++++ 11 files changed, 164 insertions(+), 68 deletions(-) diff --git a/.keywatch-baseline.json b/.keywatch-baseline.json index 0147f07..1d3a7a7 100644 --- a/.keywatch-baseline.json +++ b/.keywatch-baseline.json @@ -1610,6 +1610,13 @@ "finding_type": "Private Key Content", "matched_content_hash": "43531827dd6142886cfeb10207f046021a4eb6c575828583ad2cb20d9430c72f", "plugin_name": "PrivateKeyDetector" + }, + { + "file_path": "./tests/scanner_tests.rs", + "line_number": 2047, + "finding_type": "AWS Access Key", + "matched_content_hash": "357b7fb7890985d4c94a43012d1f7aefe25757f36d8810388357993bb38bd8e7", + "plugin_name": "AWSKeyDetector" } ] } \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f7fbc6..4343a36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ All notable changes to this project will be documented in this file. - Config discovery searches parent directories up to the repository root - Hook messages abbreviate the home directory as `~` +### Added + +- `--prune-baseline` rewrites the baseline from current findings, dropping entries for deleted files and rotated credentials + ### Changed - Reports redact matched text by default; `--show-secrets` opts into raw values @@ -42,7 +46,7 @@ All notable changes to this project will be documented in this file. - The baseline file is no longer scanned as input to itself - `GenericKeyValueDetector` and `RandomString` no longer flag code identifiers (`let payment_method_token = card_token`, snake_case serde attributes) - `PasswordDetector` no longer flags `$PWD:` -- Piping output to a closed reader no longer panics +- Piping output to a closed reader no longer panics, including `hook install` and `init` ### Performance diff --git a/README.md b/README.md index da5ad30..0ae2bd2 100644 --- a/README.md +++ b/README.md @@ -193,6 +193,7 @@ key-watch verify-integrity - `scan --exit-mode ` - Exit behavior: `always` (always pass), `critical` (fail on HIGH/CRITICAL only), `strict` (fail on any finding, default) - `scan --baseline ` - Suppress known findings from a previous scan. Without this flag, a `.keywatch-baseline.json` is discovered automatically by walking up from the scan target (bounded at the repository root or home directory), so hook scans pick up a committed repo baseline with no configuration - `scan --no-baseline-discovery` - Ignore a discovered baseline (an explicit `--baseline` still loads) +- `scan --prune-baseline` - With `--update-baseline`, rebuild the baseline from current findings instead of merging, dropping stale entries - `scan --update-baseline` - Update the baseline with current findings; creates `.keywatch-baseline.json` when no baseline exists. The baseline stores fingerprints (path + finding type + SHA-256 of the match + detector), never secrets, and is meant to be committed. The `update-baseline` workflow can regenerate it via a reviewable pull request - `hook install [--global]` - Install a git hook - `hook uninstall [--global]` - Remove a git hook diff --git a/src/cli.rs b/src/cli.rs index 16c5759..fcd5da4 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -132,6 +132,10 @@ pub struct ScanArgs { #[arg(long)] pub update_baseline: bool, + /// Rewrite the baseline from the current findings, dropping stale entries + #[arg(long, default_value_t = false)] + pub prune_baseline: bool, + /// Path to .keywatch.toml config file #[arg(long)] pub config: Option, @@ -265,17 +269,6 @@ pub enum Shell { Posix, } -impl Shell { - pub fn as_str(&self) -> &'static str { - match self { - Self::Bash => "bash", - Self::Zsh => "zsh", - Self::Fish => "fish", - Self::Posix => "posix", - } - } -} - #[derive(ValueEnum, Clone, Debug, PartialEq, Eq)] pub enum ExitMode { Always, @@ -288,22 +281,3 @@ pub enum OutputFormat { Json, Sarif, } - -impl OutputFormat { - pub fn as_str(&self) -> &'static str { - match self { - Self::Json => "json", - Self::Sarif => "sarif", - } - } -} - -impl ExitMode { - pub fn as_str(&self) -> &'static str { - match self { - Self::Always => "always", - Self::Critical => "critical", - Self::Strict => "strict", - } - } -} diff --git a/src/config/mod.rs b/src/config/mod.rs index 65cc569..7ad4f8f 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -131,15 +131,6 @@ impl KeywatchConfig { Ok(()) } - /// Load from an explicit path. When `path` is `None`, discovery starts in - /// the current working directory. - /// Returns `None` when no config file is found (config is optional). - pub fn load(path: Option<&str>) -> Result, ConfigError> { - let cwd = - std::env::current_dir().map_err(|source| ConfigError::CurrentDirectory { source })?; - Self::load_for_paths_at_cwd(path, &[], &cwd) - } - /// Load config with scan-path-aware discovery. /// /// Resolution order: diff --git a/src/hooks.rs b/src/hooks.rs index 168564d..fe71d47 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -122,21 +122,21 @@ pub fn install_hook(args: &HookInstallArgs) -> Result<(), HookError> { })?; if install_target.configured_global_path { - println!( + let _ = utils::emit_line(&format!( "Configured git --global core.hooksPath to {}", utils::display_path(&install_target.hooks_dir) - ); + )); } - println!( + let _ = utils::emit_line(&format!( "Installed {} {hook_type_str} hook at {}", scope_label(install_target.is_global), utils::display_path(&install_target.path) - ); - println!( + )); + let _ = utils::emit_line(&format!( "The hook will run automatically during git {}.", hook_type_str.replace('-', " ") - ); + )); Ok(()) } @@ -151,10 +151,10 @@ pub fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), HookError> { let scope = scope_label(install_target.is_global); if !install_target.path.exists() { - println!( + let _ = utils::emit_line(&format!( "No {scope} {hook_type_str} hook found at {}", utils::display_path(&install_target.path) - ); + )); return Ok(()); } @@ -169,10 +169,10 @@ pub fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), HookError> { source, })?; - println!( + let _ = utils::emit_line(&format!( "Removed {scope} {hook_type_str} hook at {}", utils::display_path(&install_target.path) - ); + )); Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index ea8afc0..a2924f0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,10 +35,7 @@ pub fn run_cli() -> Result<(), RunCliError> { hooks::uninstall_hook(&uninstall_args).map_err(Into::into) } }, - Command::Init { shell } => { - print_shell_init(&shell); - Ok(()) - } + Command::Init { shell } => print_shell_init(&shell), Command::VerifyIntegrity => verify_binary_integrity(), } } @@ -50,13 +47,7 @@ pub fn run_cli() -> Result<(), RunCliError> { /// reader to stop listening, so it is reported as success; anything else is a /// real I/O failure and is propagated. fn emit(line: &str) -> Result<(), RunCliError> { - use std::io::Write; - - let mut stdout = std::io::stdout().lock(); - match writeln!(stdout, "{line}") { - Err(error) if error.kind() == std::io::ErrorKind::BrokenPipe => Ok(()), - other => other.map_err(|source| RunCliError::WriteOutput { source }), - } + utils::emit_line(line).map_err(|source| RunCliError::WriteOutput { source }) } fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { @@ -84,8 +75,14 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { let (mut findings, scan_metadata) = scanner::run_scan(args, config.as_ref())?; let mut scan_metadata = scan_metadata; - if let Some(ref baseline_path) = args.baseline { - let baseline = baseline::Baseline::load(std::path::Path::new(baseline_path))?; + // Pruning rewrites the baseline from what the scan actually found, so the + // existing entries must not filter those findings away first. + let prune = args.prune_baseline && args.update_baseline; + let mut loaded_baseline = match args.baseline.as_deref() { + Some(path) => Some(baseline::Baseline::load(std::path::Path::new(path))?), + None => None, + }; + if let Some(baseline) = &loaded_baseline.as_ref().filter(|_| !prune) { let before = findings.len(); findings = baseline.filter_findings(findings); scan_metadata.suppressed_by_baseline = before - findings.len(); @@ -96,8 +93,14 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { .baseline .as_ref() .ok_or(RunCliError::MissingBaselineForUpdate)?; - let mut baseline = baseline::Baseline::load(std::path::Path::new(baseline_path))?; - baseline.update_with_findings(&findings); + let baseline = loaded_baseline + .as_mut() + .ok_or(RunCliError::MissingBaselineForUpdate)?; + if prune { + *baseline = baseline::Baseline::from_findings(&findings); + } else { + baseline.update_with_findings(&findings); + } baseline.save(std::path::Path::new(baseline_path))?; emit(&format!("Baseline updated: {baseline_path}"))?; return Ok(()); @@ -152,7 +155,7 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { std::process::exit(exit_code); } -fn print_shell_init(shell: &Shell) { +fn print_shell_init(shell: &Shell) -> Result<(), RunCliError> { let script = match shell { Shell::Fish => "alias keywatch 'key-watch'\nalias kw 'key-watch'\n", Shell::Bash | Shell::Zsh | Shell::Posix => { @@ -160,7 +163,7 @@ fn print_shell_init(shell: &Shell) { } }; - print!("{script}"); + emit(script.trim_end()) } fn verify_binary_integrity() -> Result<(), RunCliError> { diff --git a/src/scanner.rs b/src/scanner.rs index 4837458..1bfa605 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -645,7 +645,13 @@ fn is_baseline_file(path: &str, baseline: Option<&PathBuf>) -> bool { let Some(baseline) = baseline else { return false; }; - fs::canonicalize(path).is_ok_and(|candidate| candidate == *baseline) + // Compare file names before paying for a realpath(2) on every scanned + // file: only a handful can possibly be the baseline. + let candidate = Path::new(path); + if candidate.file_name() != baseline.file_name() { + return false; + } + fs::canonicalize(candidate).is_ok_and(|candidate| candidate == *baseline) } fn parse_hunk_new_start(header: &str) -> usize { diff --git a/src/utils.rs b/src/utils.rs index ad93e67..dff8ebf 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -8,6 +8,21 @@ static HOME_DIR: LazyLock> = LazyLock::new(|| { .map(PathBuf::from) }); +/// Writes a line to stdout. +/// +/// `println!` panics if stdout is closed, which happens routinely when output +/// is piped (`key-watch hook install | head`). A closed pipe is a normal way +/// for a reader to stop listening, so it is reported as success. +pub fn emit_line(line: &str) -> std::io::Result<()> { + use std::io::Write; + + let mut stdout = std::io::stdout().lock(); + match writeln!(stdout, "{line}") { + Err(error) if error.kind() == std::io::ErrorKind::BrokenPipe => Ok(()), + other => other, + } +} + /// Renders a path for terminal output, abbreviating the home directory as `~`. pub fn display_path(path: &Path) -> String { match HOME_DIR diff --git a/tests/cli_validation_tests.rs b/tests/cli_validation_tests.rs index e86cca9..98c36e7 100644 --- a/tests/cli_validation_tests.rs +++ b/tests/cli_validation_tests.rs @@ -15,6 +15,7 @@ fn test_stdin_with_path_validation_returns_typed_error() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -50,6 +51,7 @@ fn test_staged_with_stdin_validation_returns_typed_error() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -81,6 +83,7 @@ fn test_staged_with_git_history_validation_returns_typed_error() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -112,6 +115,7 @@ fn test_staged_allows_zero_or_many_paths() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, diff --git a/tests/scanner_tests.rs b/tests/scanner_tests.rs index f17588f..6726f78 100644 --- a/tests/scanner_tests.rs +++ b/tests/scanner_tests.rs @@ -119,6 +119,7 @@ sk-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWX\n\ exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -155,6 +156,7 @@ Stripe: sk_test_51ABCDEF12345678901234567890\n\ exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -192,6 +194,7 @@ AZURE_STORAGE=DefaultEndpointsProtocol=https;AccountName=examplestore; exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -230,6 +233,7 @@ b3BlbnNzaC1ldi0xLjAAABgQDQD2FGB3V2t4=\n\ exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -262,6 +266,7 @@ fn test_multiple_detections_in_line() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -306,6 +311,7 @@ fn test_directory_scan_with_exclusions() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -349,6 +355,7 @@ fn test_exclude_pattern_filtering() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -382,6 +389,7 @@ fn test_invalid_cli_exclude_pattern_returns_typed_error() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -433,6 +441,7 @@ fn test_dot_github_directory_is_scanned() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -464,6 +473,7 @@ fn test_scan_no_secrets() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -496,6 +506,7 @@ fn test_non_utf8_file_handling() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -532,6 +543,7 @@ fn test_multiple_files_scan() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -569,6 +581,7 @@ fn test_duplicate_paths_are_scanned_once() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -620,6 +633,7 @@ fn test_mixed_file_and_directory_paths_are_scanned_once() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -663,6 +677,7 @@ fn test_nonexistent_paths_are_ignored_without_counting_as_scanned() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -713,6 +728,7 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -761,6 +777,7 @@ fn test_recursive_symlink_path_is_skipped() -> Result<(), String> { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -802,6 +819,7 @@ fn test_detect_aadhaar() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -841,6 +859,7 @@ fn test_detect_voter_id() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -877,6 +896,7 @@ fn test_detect_pan_card() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -913,6 +933,7 @@ fn test_detect_abha() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -950,6 +971,7 @@ fn test_multiple_indian_ids() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -1012,6 +1034,7 @@ fn test_overlapping_scan_roots_with_exclusions() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -1059,6 +1082,7 @@ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\n exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -1109,6 +1133,7 @@ fn test_stdin_args_validation() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -1172,6 +1197,7 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -1189,6 +1215,7 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -1209,6 +1236,7 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + prune_baseline: false, no_baseline_discovery: true, config: None, no_config_discovery: false, @@ -1997,3 +2025,66 @@ fn test_staged_scan_survives_diff_relative_from_subdirectory() -> Result<(), Str let _ = fs::remove_dir_all(&repo_dir); Ok(()) } + +#[test] +fn test_prune_baseline_drops_stale_entries() -> Result<(), String> { + if !git_available() { + return Ok(()); + } + + // --update-baseline only ever appends, so entries for deleted files and + // rotated credentials kept suppressing forever. + let dir = unique_temp_dir("prune_baseline"); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).map_err(|e| e.to_string())?; + fs::write( + dir.join("a.txt"), + "aws_access_key_id = AKIAIOSFODNN7EXAMPLE\n", + ) + .map_err(|e| e.to_string())?; + fs::write( + dir.join("b.txt"), + "aws_access_key_id = AKIAJJJJJJJJJJJJJJJJ\n", + ) + .map_err(|e| e.to_string())?; + + let run = |extra: &[&str]| { + Command::new(env!("CARGO_BIN_EXE_key-watch")) + .args(["scan", ".", "--baseline", "bl.json"]) + .args(extra) + .env("KEYWATCH_CONFIG_PATH", detectors_config_path()) + .current_dir(&dir) + .output() + .expect("run key-watch") + }; + let entries = || -> usize { + let text = fs::read_to_string(dir.join("bl.json")).expect("read baseline"); + text.matches("\"file_path\"").count() + }; + + assert!(run(&["--update-baseline"]).status.success()); + let with_both = entries(); + assert!(with_both >= 2, "expected entries for both files"); + + fs::remove_file(dir.join("b.txt")).map_err(|e| e.to_string())?; + + assert!(run(&["--update-baseline"]).status.success()); + assert_eq!( + entries(), + with_both, + "a plain update must not drop the stale entry" + ); + + assert!( + run(&["--update-baseline", "--prune-baseline"]) + .status + .success() + ); + assert!( + entries() < with_both, + "--prune-baseline must drop entries the scan no longer finds" + ); + + let _ = fs::remove_dir_all(&dir); + Ok(()) +}