From 4e0757fe6cc0e680022131ccb9397d51322dadec Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:08:03 +0530 Subject: [PATCH 01/26] feat: add config file, SARIF output, format selection, and pre-commit system hook --- .pre-commit-hooks.yaml | 19 ++++++ src/cli.rs | 23 +++++++ src/config.rs | 121 ++++++++++++++++++++++++++++++++++ src/lib.rs | 17 +++-- src/report.rs | 146 ++++++++++++++++++++++++++++++++++++++++- src/scanner.rs | 62 ++++++++++++++++- tests/scanner_tests.rs | 96 ++++++++++++++++++++------- 7 files changed, 453 insertions(+), 31 deletions(-) create mode 100644 .pre-commit-hooks.yaml create mode 100644 src/config.rs diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml new file mode 100644 index 0000000..63157c1 --- /dev/null +++ b/.pre-commit-hooks.yaml @@ -0,0 +1,19 @@ +- id: keywatch-scan + name: KeyWatch scan + description: Scan staged files for secrets, API keys, tokens, and credentials + entry: key-watch scan + language: rust + files: '' + pass_filenames: true + verbose: false + args: ['--exit-mode=strict'] + +- id: keywatch-scan-system + name: KeyWatch scan (system) + description: Scan staged files for secrets, API keys, tokens, and credentials + entry: key-watch scan + language: system + files: '' + pass_filenames: true + verbose: false + args: ['--exit-mode=strict'] diff --git a/src/cli.rs b/src/cli.rs index 007c9f4..5a94b8a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -72,6 +72,14 @@ pub struct ScanArgs { /// Update the baseline file with current findings instead of scanning #[arg(long)] pub update_baseline: bool, + + /// Path to .keywatch.toml config file + #[arg(long)] + pub config: Option, + + /// Output format for the report (json or sarif) + #[arg(long, value_enum, default_value_t = OutputFormat::Json)] + pub format: OutputFormat, } impl ScanArgs { @@ -216,6 +224,21 @@ pub enum ExitMode { Strict, } +#[derive(ValueEnum, Clone, Debug, PartialEq, Eq)] +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 { diff --git a/src/config.rs b/src/config.rs new file mode 100644 index 0000000..ef970ce --- /dev/null +++ b/src/config.rs @@ -0,0 +1,121 @@ +use serde::Deserialize; +use std::collections::HashMap; +use std::fs; + +/// User-facing configuration that extends and overrides the built-in detectors. +/// +/// Loaded from `--config ` or `.keywatch.toml` in the scan root. +/// Merges with `detectors.toml`: custom rules are appended, overrides are +/// applied by detector name. +#[derive(Deserialize, Default)] +pub struct KeywatchConfig { + /// Additional detectors beyond those in detectors.toml + pub rules: Option>, + /// Per-detector overrides (by name) + pub overrides: Option>, + /// Extra exclude patterns (merged with --exclude CLI flag) + pub exclude: Option>, +} + +#[derive(Deserialize, Clone)] +pub struct CustomRule { + pub name: String, + pub pattern: String, + pub finding_type: String, + #[serde(default = "default_severity")] + pub severity: String, + #[allow(dead_code)] + pub description: Option, +} + +#[derive(Deserialize, Clone)] +pub struct DetectorOverride { + pub enabled: Option, + pub severity: Option, +} + +fn default_severity() -> String { + "MEDIUM".to_string() +} + +impl KeywatchConfig { + /// Load config from the given path. Returns None if the file doesn't exist + /// (config is optional). + pub fn load(path: Option<&str>) -> Result, String> { + let config_path = match path { + Some(p) => Some(p.to_string()), + None => find_config_in_cwd(), + }; + + let config_path = match config_path { + Some(p) => p, + None => return Ok(None), + }; + + let contents = fs::read_to_string(&config_path) + .map_err(|err| format!("Failed to read config '{}': {}", config_path, err))?; + + let config: KeywatchConfig = + toml::from_str(&contents).map_err(|err| format!("Invalid config: {}", err))?; + + Ok(Some(config)) + } +} + +fn find_config_in_cwd() -> Option { + let candidates = [".keywatch.toml", "keywatch.toml", ".kw.toml"]; + candidates + .iter() + .find(|name| std::path::Path::new(name).exists()) + .map(|&name| name.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_minimal_config() { + let toml_str = r#" +[[rules]] +name = "TestDetector" +pattern = "\\bTEST_SECRET_[A-Z]+\\b" +finding_type = "Test Secret" +severity = "HIGH" +"#; + let config: KeywatchConfig = toml::from_str(toml_str).expect("should parse"); + assert!(config.rules.is_some()); + assert_eq!(config.rules.as_ref().unwrap().len(), 1); + assert_eq!(config.rules.as_ref().unwrap()[0].name, "TestDetector"); + assert!(config.overrides.is_none()); + } + + #[test] + fn test_parse_config_with_overrides() { + let toml_str = r#" +[overrides] +AWSKeyDetector = { enabled = false } +GitHubTokenDetector = { severity = "CRITICAL" } +"#; + let config: KeywatchConfig = toml::from_str(toml_str).expect("should parse"); + let overrides = config.overrides.expect("overrides present"); + assert_eq!(overrides.len(), 2); + + let aws = overrides.get("AWSKeyDetector").expect("found"); + assert_eq!(aws.enabled, Some(false)); + assert!(aws.severity.is_none()); + + let gh = overrides.get("GitHubTokenDetector").expect("found"); + assert_eq!(gh.severity.as_deref(), Some("CRITICAL")); + assert!(gh.enabled.is_none()); + } + + #[test] + fn test_empty_config_is_default() { + let toml_str = ""; + let config: KeywatchConfig = toml::from_str(toml_str).expect("empty should parse"); + assert!(config.rules.is_none()); + assert!(config.overrides.is_none()); + assert!(config.exclude.is_none()); + } +} diff --git a/src/lib.rs b/src/lib.rs index 75dd57f..f187e82 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,6 @@ pub mod baseline; pub mod cli; +pub mod config; pub mod detector; pub mod hooks; pub mod report; @@ -9,7 +10,7 @@ pub mod utils; use clap::Parser; use cli::{ CliOptions, Command, ExitMode, HookAction, HookInstallArgs, HookType, HookUninstallArgs, - ScanArgs, Shell, + OutputFormat, ScanArgs, Shell, }; use report::Finding; use std::env; @@ -44,7 +45,8 @@ pub fn run_cli() -> Result<(), String> { fn run_scan_command(args: &ScanArgs) -> Result<(), String> { let start = Instant::now(); - let (mut findings, scan_metadata) = scanner::run_scan(args)?; + let config = config::KeywatchConfig::load(args.config.as_deref())?; + let (mut findings, scan_metadata) = scanner::run_scan(args, config.as_ref())?; if let Some(ref baseline_path) = args.baseline { let baseline = baseline::Baseline::load(std::path::Path::new(baseline_path))?; @@ -72,11 +74,14 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), String> { let severity_counts = report::get_severity_counts(&findings); let exit_code = calculate_exit_code(&findings, &args.exit_mode); let findings_count = findings.len(); - let report_json = report::create_report(findings, scan_metadata, scan_time) - .map_err(|err| format!("Failed to serialize report: {}", err))?; + let report_out = match args.format { + OutputFormat::Json => report::create_report(findings, scan_metadata, scan_time), + OutputFormat::Sarif => report::create_sarif_report(findings, scan_metadata, scan_time), + } + .map_err(|err| format!("Failed to serialize report: {}", err))?; if args.verbose { - println!("{report_json}"); + println!("{report_out}"); } else if findings_count == 0 { println!("No secrets found."); } else { @@ -91,7 +96,7 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), String> { } if let Some(ref output_path) = args.output { - utils::write_to_file(output_path, &report_json) + utils::write_to_file(output_path, &report_out) .map_err(|err| format!("Failed to write report to '{}': {}", output_path, err))?; } diff --git a/src/report.rs b/src/report.rs index adff5a9..e97e418 100644 --- a/src/report.rs +++ b/src/report.rs @@ -1,5 +1,5 @@ use serde::Serialize; -use std::{fmt, str::FromStr}; +use std::{collections::HashMap, fmt, str::FromStr}; #[derive(Serialize, Clone, PartialEq, Copy, Debug)] #[serde(rename_all = "UPPERCASE")] @@ -118,6 +118,150 @@ pub fn create_report( serde_json::to_string_pretty(&report) } +/// Generate a SARIF 2.1.0 report from findings. +pub fn create_sarif_report( + findings: Vec, + _metadata: ScanMetadata, + scan_time: String, +) -> Result { + #[derive(Serialize)] + struct SarifLog { + #[serde(rename = "$schema")] + schema: &'static str, + version: &'static str, + runs: Vec, + } + + #[derive(Serialize)] + struct SarifRun { + tool: SarifTool, + results: Vec, + properties: HashMap, + } + + #[derive(Serialize)] + struct SarifTool { + driver: SarifDriver, + } + + #[derive(Serialize)] + struct SarifDriver { + name: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + version: Option, + information_uri: &'static str, + semantic_version: Option, + } + + #[derive(Serialize)] + struct SarifResult { + rule_id: String, + level: &'static str, + message: SarifMessage, + locations: Vec, + properties: HashMap, + } + + #[derive(Serialize)] + struct SarifMessage { + text: String, + } + + #[derive(Serialize)] + struct SarifLocation { + physical_location: SarifPhysicalLocation, + } + + #[derive(Serialize)] + struct SarifPhysicalLocation { + artifact_location: SarifArtifactLocation, + region: SarifRegion, + } + + #[derive(Serialize)] + struct SarifArtifactLocation { + uri: String, + } + + #[derive(Serialize)] + struct SarifRegion { + start_line: usize, + } + + fn severity_to_sarif_level(s: Severity) -> &'static str { + match s { + Severity::Critical | Severity::High => "error", + Severity::Medium => "warning", + Severity::Low => "note", + } + } + + let results: Vec = findings + .into_iter() + .map(|f| { + let rule_id = f.finding_type; + let level = severity_to_sarif_level(f.severity); + let rule_id_clone = rule_id.clone(); + let severity_str = format!("{:?}", f.severity); + let uri = f.file_path; + let start_line = f.line_number; + + let mut properties = HashMap::new(); + properties.insert( + "precision".to_string(), + serde_json::Value::String("very-high".to_string()), + ); + properties.insert( + "severity".to_string(), + serde_json::Value::String(severity_str), + ); + + SarifResult { + rule_id, + level, + message: SarifMessage { + text: format!("Potential {} detected", rule_id_clone), + }, + locations: vec![SarifLocation { + physical_location: SarifPhysicalLocation { + artifact_location: SarifArtifactLocation { uri }, + region: SarifRegion { start_line }, + }, + }], + properties, + } + }) + .collect(); + + let status = if results.is_empty() { "pass" } else { "fail" }; + + let mut properties = HashMap::new(); + properties.insert( + "status".to_string(), + serde_json::Value::String(status.to_string()), + ); + properties.insert("scanTime".to_string(), serde_json::Value::String(scan_time)); + + let log = SarifLog { + schema: "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", + version: "2.1.0", + runs: vec![SarifRun { + tool: SarifTool { + driver: SarifDriver { + name: "KeyWatch", + version: option_env!("CARGO_PKG_VERSION").map(|v| v.to_string()), + information_uri: "https://github.com/pixincreate/KeyWatch", + semantic_version: option_env!("CARGO_PKG_VERSION").map(|v| v.to_string()), + }, + }, + results, + properties, + }], + }; + + serde_json::to_string_pretty(&log) +} + pub fn get_severity_counts(findings: &[Finding]) -> (usize, usize, usize, usize) { let mut counts = (0, 0, 0, 0); for finding in findings { diff --git a/src/scanner.rs b/src/scanner.rs index e7840d6..0abe9f8 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -1,4 +1,5 @@ use crate::cli::ScanArgs; +use crate::config::KeywatchConfig; use crate::detector::{Detector, initialize_detectors}; use crate::report::{Finding, ScanMetadata}; use glob::Pattern; @@ -170,8 +171,49 @@ fn scan_stream( Ok((findings, total_lines)) } -pub fn run_scan(args: &ScanArgs) -> Result<(Vec, ScanMetadata), String> { - let detectors = initialize_detectors().map_err(|err| err.to_string())?; +pub fn run_scan( + args: &ScanArgs, + config: Option<&KeywatchConfig>, +) -> Result<(Vec, ScanMetadata), String> { + let mut detectors = initialize_detectors().map_err(|err| err.to_string())?; + + if let Some(cfg) = config { + if let Some(ref rules) = cfg.rules { + for rule in rules { + match Detector::new( + &rule.name, + &rule.pattern, + &rule.finding_type, + &rule.severity, + &[], + &[], + None, + ) { + Ok(det) => detectors.push(det), + Err(e) => eprintln!("Warning: custom rule '{}' skipped: {}", rule.name, e), + } + } + } + + if let Some(ref overrides) = cfg.overrides { + detectors.retain(|det| { + if let Some(override_config) = overrides.get(&det.name) { + if override_config.enabled == Some(false) { + return false; + } + } + true + }); + + for det in &mut detectors { + if let Some(override_config) = overrides.get(&det.name) { + if let Some(ref sev) = override_config.severity { + det.severity.clone_from(sev); + } + } + } + } + } let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() .partition(|detector| detector.regex.as_str().contains("(?s)")); @@ -283,6 +325,22 @@ pub fn run_scan(args: &ScanArgs) -> Result<(Vec, ScanMetadata), String> .transpose()? .unwrap_or_default(); + let exclude_patterns = if let Some(cfg) = config { + if let Some(ref cfg_excludes) = cfg.exclude { + let mut patterns = exclude_patterns; + for pattern_str in cfg_excludes { + patterns.push(Pattern::new(pattern_str).map_err(|err| { + format!("Invalid config exclude pattern '{}': {}", pattern_str, err) + })?); + } + patterns + } else { + exclude_patterns + } + } else { + exclude_patterns + }; + let results: Vec<(Vec, usize, usize, Option)> = unique_paths .into_par_iter() .map(|(path, roots)| { diff --git a/tests/scanner_tests.rs b/tests/scanner_tests.rs index f2ce937..dfbd231 100644 --- a/tests/scanner_tests.rs +++ b/tests/scanner_tests.rs @@ -1,4 +1,4 @@ -use key_watch::cli::{ExitMode, ScanArgs}; +use key_watch::cli::{ExitMode, OutputFormat, ScanArgs}; use key_watch::scanner::run_scan; use std::env::temp_dir; use std::fs; @@ -114,9 +114,11 @@ sk-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWX\n\ exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(!findings.is_empty(), "Should find secrets"); fs::remove_file(test_file).expect("Cleanup"); @@ -144,9 +146,11 @@ Stripe: sk_test_51ABCDEF12345678901234567890\n\ exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(!findings.is_empty(), "Should find API tokens"); fs::remove_file(test_file).expect("Cleanup"); @@ -175,9 +179,11 @@ AZURE_STORAGE=DefaultEndpointsProtocol=https;AccountName=examplestore; exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(!findings.is_empty(), "Should find cloud credentials"); fs::remove_file(test_file).expect("Cleanup"); @@ -207,9 +213,11 @@ b3BlbnNzaC1ldi0xLjAAABgQDQD2FGB3V2t4=\n\ exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(!findings.is_empty(), "Should find private keys"); fs::remove_file(test_file).expect("Cleanup"); @@ -233,9 +241,11 @@ fn test_multiple_detections_in_line() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!( findings.len() >= 2, "Should find multiple secrets on one line" @@ -271,9 +281,11 @@ fn test_directory_scan_with_exclusions() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert_eq!( metadata.files_scanned, 2, "Should scan 2 files (.git excluded)" @@ -308,9 +320,11 @@ fn test_exclude_pattern_filtering() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (_findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (_findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( metadata .excluded_files @@ -347,9 +361,11 @@ fn test_dot_github_directory_is_scanned() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert_eq!(metadata.files_scanned, 1, "Should scan .github files"); assert!(!findings.is_empty(), "Should find secrets inside .github"); @@ -372,9 +388,11 @@ fn test_scan_no_secrets() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(findings.is_empty(), "Should not find secrets in plain text"); fs::remove_file(temp_file).expect("Cleanup"); @@ -398,9 +416,11 @@ fn test_non_utf8_file_handling() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(findings.is_empty(), "Should gracefully handle binary files"); fs::remove_file(test_file).expect("Cleanup"); @@ -428,9 +448,11 @@ fn test_multiple_files_scan() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( !findings.is_empty(), "Should find secrets in multiple files" @@ -459,9 +481,11 @@ fn test_duplicate_paths_are_scanned_once() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert_eq!( metadata.files_scanned, 1, "Duplicate paths should be deduped" @@ -504,9 +528,11 @@ fn test_mixed_file_and_directory_paths_are_scanned_once() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert_eq!( metadata.files_scanned, 1, "File should only be scanned once" @@ -541,9 +567,11 @@ fn test_nonexistent_paths_are_ignored_without_counting_as_scanned() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( findings.is_empty(), "Missing paths should not produce findings" @@ -585,6 +613,8 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; let (findings, metadata) = run_scan(&options)?; @@ -662,9 +692,11 @@ fn test_detect_aadhaar() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let aadhaar_findings: Vec<_> = findings .iter() .filter(|f| f.finding_type == "Aadhaar Card Number") @@ -695,9 +727,11 @@ fn test_detect_voter_id() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let voter_findings: Vec<_> = findings .iter() .filter(|f| f.finding_type == "Voter ID (EPIC)") @@ -725,9 +759,11 @@ fn test_detect_pan_card() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let pan_findings: Vec<_> = findings .iter() .filter(|f| f.finding_type == "PAN Card Number") @@ -755,9 +791,11 @@ fn test_detect_abha() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let abha_findings: Vec<_> = findings .iter() .filter(|f| f.finding_type == "ABHA Health ID") @@ -786,9 +824,11 @@ fn test_multiple_indian_ids() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let finding_types: Vec<_> = findings.iter().map(|f| f.finding_type.clone()).collect(); assert!( @@ -842,9 +882,11 @@ fn test_overlapping_scan_roots_with_exclusions() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( metadata @@ -883,9 +925,11 @@ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\n exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options)?; + let (findings, _) = run_scan(&options, None)?; let aws_suppressed = findings .iter() @@ -927,6 +971,8 @@ fn test_stdin_args_validation() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; assert!(options.validate().is_ok()); @@ -982,6 +1028,8 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; let one_path = ScanArgs { paths: vec!["/tmp/requested-root".to_string()], @@ -993,6 +1041,8 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; let two_paths = ScanArgs { paths: vec![ @@ -1007,6 +1057,8 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + format: OutputFormat::Json, }; assert!(zero_paths.validate().is_ok()); From b6b977c482a3b771c749713beb3044d529c3eaf0 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:11:49 +0530 Subject: [PATCH 02/26] docs: add architecture section to README with Mermaid diagrams and data flow --- README.md | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/README.md b/README.md index 2a6e1f4..3524250 100644 --- a/README.md +++ b/README.md @@ -191,6 +191,146 @@ password = 'known-test-password' # keywatch:ignore - KeyWatch refuses to overwrite a non-KeyWatch global hook file - KeyWatch also refuses to remove a non-KeyWatch global hook file +## Architecture + +### Layers & Data Flow + +```mermaid +flowchart LR + subgraph Input["📥 INPUT"] + direction LR + FILES[("Files / Dirs")] + STDIN[("stdin")] + GIT[("git log -p")] + end + + subgraph Config["⚙️ CONFIGURATION"] + DET_TOML[("detectors.toml
827 lines of built-in rules")] + KW_TOML[(".keywatch.toml
Custom rules, overrides,
exclude patterns")] + end + + subgraph Pipeline["🔁 DETECTION PIPELINE"] + direction TB + COLLECT["1. Collect files
Recursive dir walk
Skip .git, skip binary"] + EXCLUDE["2. Exclude filter
CLI --exclude +
config.exclude"] + KEYWORD["3. Keyword pre-filter
Fast path: skip files
without matching keywords"] + REGEX["4. Regex match
Line detectors (single-line)
Multiline detectors ((?s))"] + ENTROPY["5. Shannon entropy
Only flag secrets above
configurable threshold"] + ALLOW["6. Allowlist + suppress
Detector allowlist regexes
Inline keywatch:ignore"] + end + + subgraph Post["📋 POST-PROCESSING"] + BASELINE["Baseline filter
SHA-256 fingerprint
Suppress known findings"] + end + + subgraph Output["📤 OUTPUT"] + JSON_OUT["JSON report
create_report()"] + SARIF_OUT["SARIF 2.1.0
create_sarif_report()"] + SUMMARY["Summary line
CRIT: 2, HIGH: 5, ..."] + end + + Input --> COLLECT + DET_TOML --> KEYWORD + DET_TOML --> REGEX + DET_TOML --> ENTROPY + DET_TOML --> ALLOW + KW_TOML --> EXCLUDE + COLLECT --> EXCLUDE + EXCLUDE --> KEYWORD --> REGEX --> ENTROPY --> ALLOW + ALLOW --> BASELINE + BASELINE --> JSON_OUT + BASELINE --> SARIF_OUT + BASELINE --> SUMMARY +``` + +### Core Data Types + +```mermaid +classDiagram + class Detector { + +String name + +Regex regex + +String finding_type + +String severity + +Vec~Regex~ allowlist + +Vec~String~ keywords + +Option~f64~ entropy_threshold + +has_keywords(content) bool + +has_sufficient_entropy(matched) bool + } + + class Finding { + +String file_path + +usize line_number + +String finding_type + +Severity severity + +String matched_content + +String plugin_name + } + + class Severity { + <> + CRITICAL + HIGH + MEDIUM + LOW + } + + class ScanMetadata { + +usize files_scanned + +usize total_lines + +Vec~String~ excluded_files + } + + class KeywatchConfig { + +Option~Vec~CustomRule~~ rules + +Option~HashMap~String, DetectorOverride~~ overrides + +Option~Vec~String~~ exclude + } + + class Baseline { + +String version + +Vec~BaselineEntry~ entries + +filter_findings(findings) Vec~Finding~ + +update_with_findings(findings) + } + + Finding --> Severity + Detector --> Finding : produces + KeywatchConfig --> Detector : extends/overrides + Baseline --> Finding : filters +``` + +### Data Flow (One Scan) + +``` +CLI args (paths, --stdin, --git-history, --exclude, --format, --baseline, --config) + │ + ▼ +load config (optional .keywatch.toml) ───► merge custom rules into detectors + │ merge exclude patterns + ▼ +collect files ───► exclude filter ───► parallel scan (rayon) + │ + ├─ keyword pre-filter (skip if no match) + ├─ regex match (line + multiline) + ├─ entropy threshold check + ├─ allowlist check + └─ inline suppression check + │ + ▼ + Vec + ScanMetadata + │ + ▼ + baseline filter (optional, --baseline) + │ + ▼ + report serialization (JSON or SARIF 2.1.0) + │ + ▼ + stdout / file / exit code +``` + ## Development ```sh From d2078f6e4e4246fc6805ba35e1c2b58078657c67 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:14:07 +0530 Subject: [PATCH 03/26] docs(changelog): add config, SARIF, format, and architecture entries --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60c31ab..d6b4f7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,15 @@ All notable changes to this project will be documented in this file. - **GitHub Action** — composite action (`action.yml`) for CI/CD integration - **Docker support** — multi-stage Dockerfile with `--locked` flag, stripped binary, non-root user, and git installed for `--git-history` scanning and hook installation - `.dockerignore` for optimized Docker builds +- **Config file support** — `.keywatch.toml` with custom rules, detector overrides, and exclude patterns +- **SARIF 2.1.0 output** — `--format sarif` enables GitHub Code Scanning and SARIF viewer integration +- **`--config` CLI flag** — specify a custom path to `.keywatch.toml` +- **Pre-commit `language: system`** — generated hooks use `language: system` for faster execution ### Changed +- `get_severity_counts()` now returns 4-tuple (Critical, High, Medium, Low) instead of 3-tuple +- `run_scan()` accepts optional `config` parameter for merging user configuration - Simplified distribution to a single shipped binary: `key-watch` - Git hook installation now supports first-class global hooks via `core.hooksPath` - Installation guidance is now cargo-first, with manual GitHub Releases setup documented step by step @@ -49,6 +55,10 @@ All notable changes to this project will be documented in this file. - `scripts/install.sh` in favor of documented `cargo install` and manual release-binary setup - ~1650 lines of redundant context-based detectors; kept only prefix-based detectors plus GenericKeyValueDetector +### Documentation + +- README architecture section with module diagrams, data flow trace, and type relationships + ## [1.1.0] - 2026-05-05 ### Added From 76012860c2d3bf4dffd00e368ffc382dd1ffb7ed Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:25:55 +0530 Subject: [PATCH 04/26] refactor: extract hook lifecycle from lib.rs into hooks.rs --- src/config.rs | 84 ++++++-- src/hooks.rs | 518 +++++++++++++++++++++++++++++++++++++++++++++++- src/lib.rs | 524 +------------------------------------------------ src/scanner.rs | 58 +----- 4 files changed, 592 insertions(+), 592 deletions(-) diff --git a/src/config.rs b/src/config.rs index ef970ce..d657cc7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,7 +1,11 @@ +use crate::detector::Detector; use serde::Deserialize; use std::collections::HashMap; use std::fs; +/// Default severity for custom rules that omit `severity` in their TOML entry. +const DEFAULT_SEVERITY: &str = "MEDIUM"; + /// User-facing configuration that extends and overrides the built-in detectors. /// /// Loaded from `--config ` or `.keywatch.toml` in the scan root. @@ -17,28 +21,43 @@ pub struct KeywatchConfig { pub exclude: Option>, } -#[derive(Deserialize, Clone)] -pub struct CustomRule { - pub name: String, - pub pattern: String, - pub finding_type: String, - #[serde(default = "default_severity")] - pub severity: String, - #[allow(dead_code)] - pub description: Option, -} - -#[derive(Deserialize, Clone)] -pub struct DetectorOverride { - pub enabled: Option, - pub severity: Option, -} - -fn default_severity() -> String { - "MEDIUM".to_string() -} - impl KeywatchConfig { + /// Apply this config to a detector set: append custom rules and apply + /// per-detector overrides (disable and/or re-severity by name). + pub fn apply_to(&self, detectors: &mut Vec) { + if let Some(rules) = &self.rules { + for rule in rules { + match Detector::new( + &rule.name, + &rule.pattern, + &rule.finding_type, + &rule.severity, + &[], + &[], + None, + ) { + Ok(det) => detectors.push(det), + Err(err) => { + eprintln!("Warning: custom rule '{}' skipped: {}", rule.name, err) + } + } + } + } + + if let Some(overrides) = &self.overrides { + detectors.retain(|det| match overrides.get(&det.name) { + Some(override_config) => override_config.enabled != Some(false), + None => true, + }); + + for det in detectors.iter_mut() { + if let Some(severity) = overrides.get(&det.name).and_then(|o| o.severity.as_ref()) { + det.severity.clone_from(severity); + } + } + } + } + /// Load config from the given path. Returns None if the file doesn't exist /// (config is optional). pub fn load(path: Option<&str>) -> Result, String> { @@ -62,6 +81,29 @@ impl KeywatchConfig { } } +#[derive(Deserialize, Clone)] +pub struct CustomRule { + pub name: String, + pub pattern: String, + pub finding_type: String, + #[serde(default = "default_severity")] + pub severity: String, + #[allow(dead_code)] + pub description: Option, +} + +#[derive(Deserialize, Clone)] +pub struct DetectorOverride { + pub enabled: Option, + pub severity: Option, +} + +/// Serde `default` attribute requires a function; it returns the +/// [`DEFAULT_SEVERITY`] constant. +fn default_severity() -> String { + DEFAULT_SEVERITY.to_string() +} + fn find_config_in_cwd() -> Option { let candidates = [".keywatch.toml", "keywatch.toml", ".kw.toml"]; candidates diff --git a/src/hooks.rs b/src/hooks.rs index 72fafca..86de171 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -1,10 +1,17 @@ -use crate::cli::HookInstallArgs; +use crate::cli::{HookInstallArgs, HookUninstallArgs}; +use crate::utils; +use std::env; +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command as ProcessCommand; const PRE_PUSH_TEMPLATE: &str = include_str!("../templates/pre-push.sh"); const PRE_COMMIT_TEMPLATE: &str = include_str!("../templates/pre-commit.sh"); const DEFAULT_BINARY_NAME: &str = "key-watch"; +const KEYWATCH_MARKER: &str = "# Installed by KeyWatch"; + fn shell_escape(input: &str) -> String { format!("'{}'", input.replace('\'', "'\"'\"'")) } @@ -66,3 +73,512 @@ fn hook_binary_name() -> String { }) .unwrap_or_else(|| DEFAULT_BINARY_NAME.to_string()) } + +/// Install a git hook of the given type. +/// +/// Writes the rendered hook script to the resolved target path and makes it +/// executable. For global installs, a managed hooks directory is created and +/// configured via `git config --global core.hooksPath` if none is set. +pub fn install_hook(args: &HookInstallArgs) -> Result<(), String> { + let hook_content = match args.hook_type { + crate::cli::HookType::PrePush => generate_pre_push_hook(args), + crate::cli::HookType::PreCommit => generate_pre_commit_hook(args), + }; + let hook_type_str = args.hook_type.as_str(); + + let install_target = resolve_hook_install_target(hook_type_str, args.global)?; + + if !install_target.is_global { + ensure_local_hook_target_is_safe_to_create(&install_target.path)?; + } + + if let Some(parent) = install_target.path.parent() { + fs::create_dir_all(parent).map_err(|err| { + format!( + "Failed to create hook directory '{}': {}", + parent.display(), + err + ) + })?; + } + + if install_target.is_global { + ensure_global_hook_target_is_safe(&install_target.path)?; + } + + let hook_path = install_target.path.to_string_lossy().into_owned(); + utils::write_to_file(&hook_path, &hook_content) + .map_err(|err| format!("Failed to install hook '{}': {}", hook_path, err))?; + utils::make_executable(&hook_path) + .map_err(|err| format!("Failed to make hook executable '{}': {}", hook_path, err))?; + + if install_target.configured_global_path { + println!( + "Configured git --global core.hooksPath to {}", + install_target.hooks_dir.display() + ); + } + + if install_target.is_global { + println!("Installed global {hook_type_str} hook at {hook_path}"); + } else { + println!("Installed {hook_type_str} hook at {hook_path}"); + } + println!( + "The hook will run automatically during git {}.", + hook_type_str.replace('-', " ") + ); + + Ok(()) +} + +/// Uninstall a git hook of the given type. +/// +/// Only removes hooks that KeyWatch previously installed (verified via the +/// marker comment). Missing hooks are reported as a no-op success. +pub fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), String> { + let hook_type_str = args.hook_type.as_str(); + let install_target = resolve_hook_uninstall_target(hook_type_str, args.global)?; + + if !install_target.path.exists() { + let scope = if install_target.is_global { + "global" + } else { + "local" + }; + println!( + "No {scope} {hook_type_str} hook found at {}", + install_target.path.display() + ); + return Ok(()); + } + + ensure_hook_target_is_keywatch_managed( + &install_target.path, + install_target.is_global, + "remove", + )?; + + fs::remove_file(&install_target.path).map_err(|err| { + format!( + "Failed to remove hook '{}': {}", + install_target.path.display(), + err + ) + })?; + + if install_target.is_global { + println!( + "Removed global {hook_type_str} hook at {}", + install_target.path.display() + ); + } else { + println!( + "Removed {hook_type_str} hook at {}", + install_target.path.display() + ); + } + + Ok(()) +} + +struct HookInstallTarget { + path: PathBuf, + hooks_dir: PathBuf, + is_global: bool, + configured_global_path: bool, +} + +impl HookInstallTarget { + fn local(hook_type: &str) -> Result { + let hooks_dir = resolve_local_hooks_dir()?; + Ok(Self { + path: hooks_dir.join(hook_type), + hooks_dir, + is_global: false, + configured_global_path: false, + }) + } + + fn global(hook_type: &str) -> Result { + if let Some(hooks_dir) = read_global_hooks_path()? { + return Ok(Self { + path: hooks_dir.join(hook_type), + hooks_dir, + is_global: true, + configured_global_path: false, + }); + } + + let managed_dir = managed_global_hooks_dir( + env::var_os("XDG_CONFIG_HOME"), + env::var_os("HOME"), + env::var_os("APPDATA"), + env::var_os("USERPROFILE"), + )?; + + fs::create_dir_all(&managed_dir).map_err(|err| { + format!( + "Failed to create global hooks directory '{}': {}", + managed_dir.display(), + err + ) + })?; + configure_global_hooks_path(&managed_dir)?; + + Ok(Self { + path: managed_dir.join(hook_type), + hooks_dir: managed_dir, + is_global: true, + configured_global_path: true, + }) + } +} + +fn resolve_hook_install_target(hook_type: &str, global: bool) -> Result { + if global { + HookInstallTarget::global(hook_type) + } else { + HookInstallTarget::local(hook_type) + } +} + +fn resolve_hook_uninstall_target( + hook_type: &str, + global: bool, +) -> Result { + if !global { + return HookInstallTarget::local(hook_type); + } + + let hooks_dir = match read_global_hooks_path()? { + Some(path) => path, + None => managed_global_hooks_dir( + env::var_os("XDG_CONFIG_HOME"), + env::var_os("HOME"), + env::var_os("APPDATA"), + env::var_os("USERPROFILE"), + )?, + }; + + Ok(HookInstallTarget { + path: hooks_dir.join(hook_type), + hooks_dir, + is_global: true, + configured_global_path: false, + }) +} + +fn resolve_local_hooks_dir() -> Result { + resolve_local_hooks_dir_from(Path::new(".")) +} + +fn resolve_local_hooks_dir_from(cwd: &Path) -> Result { + let output = ProcessCommand::new("git") + .current_dir(cwd) + .args(["rev-parse", "--path-format=absolute", "--git-path", "hooks"]) + .output() + .map_err(|err| format!("Failed to resolve git hooks directory: {}", err))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + return Err(if stderr.is_empty() { + "Local hook installation requires running inside a git repository".to_string() + } else { + format!("Failed to resolve git hooks directory: {}", stderr) + }); + } + + let hooks_path = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if hooks_path.is_empty() { + return Err("Failed to resolve git hooks directory".to_string()); + } + + Ok(PathBuf::from(hooks_path)) +} + +fn read_global_hooks_path() -> Result, String> { + let output = ProcessCommand::new("git") + .args(["config", "--global", "--path", "--get", "core.hooksPath"]) + .output() + .map_err(|err| format!("Failed to read git config core.hooksPath: {}", err))?; + + if output.status.success() { + let value = String::from_utf8_lossy(&output.stdout).trim().to_string(); + return Ok(if value.is_empty() { + None + } else { + Some(PathBuf::from(value)) + }); + } + + if output.status.code() == Some(1) { + // Exit code 1 means the key is not set. + return Ok(None); + } + + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + Err(format!( + "Failed to read git config core.hooksPath: {}", + stderr + )) +} + +fn managed_global_hooks_dir( + xdg_config_home: Option, + home: Option, + appdata: Option, + userprofile: Option, +) -> Result { + if let Some(xdg) = xdg_config_home { + return Ok(PathBuf::from(xdg).join("key-watch").join("hooks")); + } + if let Some(home) = home { + return Ok(PathBuf::from(home) + .join(".config") + .join("key-watch") + .join("hooks")); + } + if let Some(appdata) = appdata { + return Ok(PathBuf::from(appdata).join("key-watch").join("hooks")); + } + if let Some(userprofile) = userprofile { + return Ok(PathBuf::from(userprofile) + .join(".config") + .join("key-watch") + .join("hooks")); + } + Err("Could not determine a directory for global git hooks".to_string()) +} + +fn configure_global_hooks_path(hooks_dir: &Path) -> Result<(), String> { + let output = ProcessCommand::new("git") + .args(["config", "--global", "core.hooksPath"]) + .arg(hooks_dir) + .output() + .map_err(|err| format!("Failed to configure git global core.hooksPath: {}", err))?; + + if output.status.success() { + Ok(()) + } else { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + Err(format!( + "git config --global core.hooksPath {} failed: {}", + hooks_dir.display(), + stderr + )) + } +} + +fn ensure_global_hook_target_is_safe(hook_path: &Path) -> Result<(), String> { + ensure_hook_target_is_keywatch_managed(hook_path, true, "overwrite") +} + +fn ensure_local_hook_target_is_safe_to_create(hook_path: &Path) -> Result<(), String> { + resolve_local_hooks_dir()?; + if hook_path.exists() { + ensure_hook_target_is_keywatch_managed(hook_path, false, "overwrite")?; + } + Ok(()) +} + +/// Refuse to touch an existing hook unless KeyWatch installed it. +fn ensure_hook_target_is_keywatch_managed( + hook_path: &Path, + is_global: bool, + action: &str, +) -> Result<(), String> { + if !hook_path.exists() { + return Ok(()); + } + + let content = fs::read_to_string(hook_path).map_err(|err| { + format!( + "Failed to inspect existing hook '{}': {}", + hook_path.display(), + err + ) + })?; + + if content.contains(KEYWATCH_MARKER) { + return Ok(()); + } + + let scope = if is_global { "global" } else { "local" }; + Err(format!( + "Refusing to {action} existing {scope} hook at '{}'. Merge it manually or remove it yourself.", + hook_path.display() + )) +} + +#[cfg(test)] +mod tests { + use super::{ + ensure_global_hook_target_is_safe, ensure_local_hook_target_is_safe_to_create, + managed_global_hooks_dir, resolve_hook_uninstall_target, resolve_local_hooks_dir_from, + }; + use std::env; + use std::fs; + use std::process::Command; + use std::time::{SystemTime, UNIX_EPOCH}; + + fn unique_temp_dir(name: &str) -> std::path::PathBuf { + let timestamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("System time should be after Unix epoch") + .as_millis(); + std::env::temp_dir().join(format!("keywatch_{name}_{timestamp}")) + } + + fn init_git_repo(path: &std::path::Path) { + fs::create_dir_all(path).expect("create repo dir"); + let status = Command::new("git") + .args(["init", "--quiet"]) + .current_dir(path) + .status() + .expect("run git init"); + assert!(status.success(), "git init should succeed"); + } + + #[test] + fn test_managed_global_hooks_dir_prefers_xdg() { + let path = managed_global_hooks_dir( + Some("/tmp/xdg".into()), + Some("/tmp/home".into()), + None, + None, + ) + .expect("xdg path should resolve"); + + assert_eq!(path, std::path::PathBuf::from("/tmp/xdg/key-watch/hooks")); + } + + #[test] + fn test_managed_global_hooks_dir_falls_back_to_home() { + let path = managed_global_hooks_dir(None, Some("/tmp/home".into()), None, None) + .expect("home path should resolve"); + + assert_eq!( + path, + std::path::PathBuf::from("/tmp/home/.config/key-watch/hooks") + ); + } + + #[test] + fn test_global_hook_safety_allows_keywatch_hook() { + let temp_dir = unique_temp_dir("global_hook_safe"); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + let hook_path = temp_dir.join("pre-commit"); + + fs::write(&hook_path, "#!/bin/bash\n# Installed by KeyWatch\n").expect("write hook file"); + + ensure_global_hook_target_is_safe(&hook_path).expect("keywatch hook should be reusable"); + + fs::remove_file(&hook_path).expect("remove hook file"); + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_global_hook_safety_rejects_foreign_hook() { + let temp_dir = unique_temp_dir("global_hook_foreign"); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + let hook_path = temp_dir.join("pre-commit"); + + fs::write(&hook_path, "#!/bin/bash\necho custom hook\n").expect("write hook file"); + + let error = ensure_global_hook_target_is_safe(&hook_path) + .expect_err("foreign hook should be rejected"); + assert!(error.contains("Refusing to overwrite existing global hook")); + + fs::remove_file(&hook_path).expect("remove hook file"); + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_resolve_local_hooks_dir_resolves_correctly() { + let temp_dir = unique_temp_dir("local_hooks_dir_resolution"); + init_git_repo(&temp_dir); + + let resolved = + resolve_local_hooks_dir_from(&temp_dir).expect("should resolve hooks dir inside repo"); + + assert!(resolved.is_absolute(), "hooks path should be absolute"); + assert!( + resolved.ends_with(".git/hooks"), + "hooks path should end with .git/hooks, but was: {}", + resolved.display() + ); + + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_git_init_creates_hooks_dir() { + let temp_dir = unique_temp_dir("local_hook_missing_git"); + init_git_repo(&temp_dir); + + assert!(temp_dir.join(".git/hooks").exists()); + + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_local_hook_safety_rejects_foreign_hook() { + let temp_dir = unique_temp_dir("local_hook_foreign"); + init_git_repo(&temp_dir); + let git_dir = temp_dir.join(".git"); + let git_hooks_dir = git_dir.join("hooks"); + fs::create_dir_all(&git_hooks_dir).expect("create hooks dir"); + let hook_path = git_hooks_dir.join("pre-commit"); + fs::write(&hook_path, "#!/bin/bash\necho custom hook\n").expect("write hook file"); + + let error = ensure_local_hook_target_is_safe_to_create(&hook_path) + .expect_err("foreign local hook should be rejected"); + assert!(error.contains("Refusing to overwrite existing local hook")); + + fs::remove_file(&hook_path).expect("remove hook file"); + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_global_uninstall_target_does_not_configure_missing_hooks_path() { + let install_target = resolve_hook_uninstall_target("pre-commit", true) + .expect("global uninstall target should resolve"); + let expected_hooks_dir = managed_global_hooks_dir( + env::var_os("XDG_CONFIG_HOME"), + env::var_os("HOME"), + env::var_os("APPDATA"), + env::var_os("USERPROFILE"), + ) + .expect("managed hooks dir should resolve"); + + assert!(install_target.is_global); + assert!(!install_target.configured_global_path); + assert_eq!(install_target.hooks_dir, expected_hooks_dir); + assert_eq!(install_target.path, expected_hooks_dir.join("pre-commit")); + } + + #[test] + fn test_managed_global_hooks_dir_prefers_appdata_when_home_missing() { + let path = managed_global_hooks_dir( + None, + None, + Some("C:/Users/test/AppData/Roaming".into()), + None, + ) + .expect("appdata path should resolve"); + + assert_eq!( + path, + std::path::PathBuf::from("C:/Users/test/AppData/Roaming/key-watch/hooks") + ); + } + + #[test] + fn test_managed_global_hooks_dir_errors_without_known_base_dir() { + let error = managed_global_hooks_dir(None, None, None, None) + .expect_err("missing env inputs should fail"); + + assert!(error.contains("Could not determine a directory for global git hooks")); + } +} diff --git a/src/lib.rs b/src/lib.rs index f187e82..b84c11e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,19 +8,12 @@ pub mod scanner; pub mod utils; use clap::Parser; -use cli::{ - CliOptions, Command, ExitMode, HookAction, HookInstallArgs, HookType, HookUninstallArgs, - OutputFormat, ScanArgs, Shell, -}; +use cli::{CliOptions, Command, ExitMode, HookAction, OutputFormat, ScanArgs, Shell}; use report::Finding; use std::env; -use std::fs; -use std::path::{Path, PathBuf}; -use std::process::Command as ProcessCommand; use std::time::Instant; use crate::report::Severity; -pub use hooks::{generate_pre_commit_hook, generate_pre_push_hook}; pub const EXIT_CODE_RUNTIME_ERROR: i32 = 2; @@ -31,8 +24,8 @@ pub fn run_cli() -> Result<(), String> { match options.command { Command::Scan(args) => run_scan_command(&args), Command::Hook(args) => match args.action { - HookAction::Install(install_args) => install_hook(&install_args), - HookAction::Uninstall(uninstall_args) => uninstall_hook(&uninstall_args), + HookAction::Install(install_args) => hooks::install_hook(&install_args), + HookAction::Uninstall(uninstall_args) => hooks::uninstall_hook(&uninstall_args), }, Command::Init { shell } => { print_shell_init(&shell); @@ -103,104 +96,6 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), String> { std::process::exit(exit_code); } -fn install_hook(args: &HookInstallArgs) -> Result<(), String> { - let hook_content = match args.hook_type { - HookType::PrePush => hooks::generate_pre_push_hook(args), - HookType::PreCommit => hooks::generate_pre_commit_hook(args), - }; - - let hook_type_str = args.hook_type.as_str(); - let install_target = resolve_hook_install_target(hook_type_str, args.global)?; - - if !install_target.is_global { - ensure_local_hook_target_is_safe_to_create(&install_target.path)?; - } - - if let Some(parent) = install_target.path.parent() { - fs::create_dir_all(parent).map_err(|err| { - format!( - "Failed to create hook directory '{}': {}", - parent.display(), - err - ) - })?; - } - - if install_target.is_global { - ensure_global_hook_target_is_safe(&install_target.path)?; - } - - let hook_path = install_target.path.to_string_lossy().into_owned(); - utils::write_to_file(&hook_path, &hook_content) - .map_err(|err| format!("Failed to install hook '{}': {}", hook_type_str, err))?; - utils::make_executable(&hook_path) - .map_err(|err| format!("Failed to make hook executable '{}': {}", hook_path, err))?; - - if install_target.configured_global_path { - println!( - "Configured git --global core.hooksPath to {}", - install_target.hooks_dir.display() - ); - } - - if install_target.is_global { - println!("Installed global {hook_type_str} hook at {hook_path}"); - } else { - println!("Installed {hook_type_str} hook at {hook_path}"); - } - println!( - "The hook will run automatically during git {}.", - hook_type_str.replace('-', " ") - ); - Ok(()) -} - -fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), String> { - let hook_type_str = args.hook_type.as_str(); - let install_target = resolve_hook_uninstall_target(hook_type_str, args.global)?; - - if !install_target.path.exists() { - let scope = if install_target.is_global { - "global" - } else { - "local" - }; - println!( - "No {scope} {hook_type_str} hook found at {}", - install_target.path.display() - ); - return Ok(()); - } - - ensure_hook_target_is_keywatch_managed( - &install_target.path, - install_target.is_global, - "remove", - )?; - - fs::remove_file(&install_target.path).map_err(|err| { - format!( - "Failed to remove hook '{}': {}", - install_target.path.display(), - err - ) - })?; - - if install_target.is_global { - println!( - "Removed global {hook_type_str} hook at {}", - install_target.path.display() - ); - } else { - println!( - "Removed {hook_type_str} hook at {}", - install_target.path.display() - ); - } - - Ok(()) -} - fn print_shell_init(shell: &Shell) { let script = match shell { Shell::Fish => "alias keywatch 'key-watch'\nalias kw 'key-watch'\n", @@ -212,249 +107,6 @@ fn print_shell_init(shell: &Shell) { print!("{script}"); } -struct HookInstallTarget { - path: PathBuf, - hooks_dir: PathBuf, - is_global: bool, - configured_global_path: bool, -} - -impl HookInstallTarget { - fn local(hook_type: &str) -> Result { - let hooks_dir = resolve_local_hooks_dir()?; - Ok(Self { - path: hooks_dir.join(hook_type), - hooks_dir, - is_global: false, - configured_global_path: false, - }) - } - - fn global(hook_type: &str) -> Result { - let configured = read_global_hooks_path()?; - let hooks_dir = match configured { - Some(path) => path, - None => { - let managed_dir = managed_global_hooks_dir( - env::var_os("XDG_CONFIG_HOME"), - env::var_os("HOME"), - env::var_os("APPDATA"), - env::var_os("USERPROFILE"), - )?; - fs::create_dir_all(&managed_dir).map_err(|err| { - format!( - "Failed to create global hooks directory '{}': {}", - managed_dir.display(), - err - ) - })?; - configure_global_hooks_path(&managed_dir)?; - return Ok(Self { - path: managed_dir.join(hook_type), - hooks_dir: managed_dir, - is_global: true, - configured_global_path: true, - }); - } - }; - - Ok(Self { - path: hooks_dir.join(hook_type), - hooks_dir, - is_global: true, - configured_global_path: false, - }) - } -} - -fn resolve_hook_install_target(hook_type: &str, global: bool) -> Result { - if global { - HookInstallTarget::global(hook_type) - } else { - HookInstallTarget::local(hook_type) - } -} - -fn resolve_hook_uninstall_target( - hook_type: &str, - global: bool, -) -> Result { - if global { - let hooks_dir = match read_global_hooks_path()? { - Some(path) => path, - None => managed_global_hooks_dir( - env::var_os("XDG_CONFIG_HOME"), - env::var_os("HOME"), - env::var_os("APPDATA"), - env::var_os("USERPROFILE"), - )?, - }; - - Ok(HookInstallTarget { - path: hooks_dir.join(hook_type), - hooks_dir, - is_global: true, - configured_global_path: false, - }) - } else { - HookInstallTarget::local(hook_type) - } -} - -fn resolve_local_hooks_dir() -> Result { - resolve_local_hooks_dir_from(Path::new(".")) -} - -fn resolve_local_hooks_dir_from(cwd: &Path) -> Result { - let output = ProcessCommand::new("git") - .current_dir(cwd) - .args(["rev-parse", "--path-format=absolute", "--git-path", "hooks"]) - .output() - .map_err(|err| format!("Failed to resolve git hooks directory: {}", err))?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - return Err(if stderr.is_empty() { - "Local hook installation requires running inside a git repository".to_string() - } else { - format!("Local hook installation requires running inside a git repository: {stderr}") - }); - } - - let hooks_path = String::from_utf8(output.stdout) - .map_err(|err| format!("Invalid UTF-8 from git rev-parse output: {}", err))? - .trim() - .to_string(); - - if hooks_path.is_empty() { - return Err("Failed to resolve git hooks directory".to_string()); - } - - Ok(PathBuf::from(hooks_path)) -} - -fn read_global_hooks_path() -> Result, String> { - let output = ProcessCommand::new("git") - .args(["config", "--global", "--path", "--get", "core.hooksPath"]) - .output() - .map_err(|err| format!("Failed to read git global core.hooksPath: {}", err))?; - - if output.status.success() { - let hooks_path = String::from_utf8(output.stdout) - .map_err(|err| format!("Invalid UTF-8 from git config output: {}", err))? - .trim() - .to_string(); - - if hooks_path.is_empty() { - Ok(None) - } else { - Ok(Some(PathBuf::from(hooks_path))) - } - } else if output.status.code() == Some(1) { - Ok(None) - } else { - Err(String::from_utf8_lossy(&output.stderr).trim().to_string()) - } -} - -fn managed_global_hooks_dir( - xdg_config_home: Option, - home: Option, - appdata: Option, - userprofile: Option, -) -> Result { - if let Some(xdg_config_home) = xdg_config_home { - return Ok(PathBuf::from(xdg_config_home) - .join("key-watch") - .join("hooks")); - } - - if let Some(home) = home { - return Ok(PathBuf::from(home) - .join(".config") - .join("key-watch") - .join("hooks")); - } - - if let Some(appdata) = appdata { - return Ok(PathBuf::from(appdata).join("key-watch").join("hooks")); - } - - if let Some(userprofile) = userprofile { - return Ok(PathBuf::from(userprofile) - .join(".config") - .join("key-watch") - .join("hooks")); - } - - Err("Could not determine a directory for global git hooks".to_string()) -} - -fn configure_global_hooks_path(hooks_dir: &Path) -> Result<(), String> { - let output = ProcessCommand::new("git") - .args([ - "config", - "--global", - "core.hooksPath", - hooks_dir.to_string_lossy().as_ref(), - ]) - .output() - .map_err(|err| format!("Failed to configure git global core.hooksPath: {}", err))?; - - if output.status.success() { - Ok(()) - } else { - let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - Err(format!( - "git config --global core.hooksPath {} failed: {}", - hooks_dir.display(), - stderr - )) - } -} - -fn ensure_global_hook_target_is_safe(hook_path: &Path) -> Result<(), String> { - ensure_hook_target_is_keywatch_managed(hook_path, true, "overwrite") -} - -fn ensure_local_hook_target_is_safe_to_create(hook_path: &Path) -> Result<(), String> { - resolve_local_hooks_dir()?; - - if hook_path.exists() { - ensure_hook_target_is_keywatch_managed(hook_path, false, "overwrite")?; - } - - Ok(()) -} - -fn ensure_hook_target_is_keywatch_managed( - hook_path: &Path, - is_global: bool, - action: &str, -) -> Result<(), String> { - if !hook_path.exists() { - return Ok(()); - } - - let existing_hook = fs::read_to_string(hook_path).map_err(|err| { - format!( - "Failed to inspect existing hook '{}': {}", - hook_path.display(), - err - ) - })?; - - if existing_hook.contains("# Installed by KeyWatch") { - return Ok(()); - } - - let scope = if is_global { "global" } else { "local" }; - Err(format!( - "Refusing to {action} existing {scope} hook at '{}'. Merge it manually or remove it yourself.", - hook_path.display() - )) -} - fn verify_binary_integrity() -> Result<(), String> { let exe_path = env::current_exe().map_err(|err| format!("Failed to get executable path: {}", err))?; @@ -496,177 +148,9 @@ fn calculate_exit_code(findings: &[Finding], exit_mode: &ExitMode) -> i32 { #[cfg(test)] mod tests { - use super::{ - Severity, calculate_exit_code, ensure_global_hook_target_is_safe, - ensure_local_hook_target_is_safe_to_create, managed_global_hooks_dir, - resolve_hook_uninstall_target, resolve_local_hooks_dir_from, - }; + use super::{Severity, calculate_exit_code}; use crate::cli::ExitMode; use crate::report::Finding; - use std::env; - use std::fs; - use std::process::Command; - use std::time::{SystemTime, UNIX_EPOCH}; - - fn unique_temp_dir(name: &str) -> std::path::PathBuf { - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .expect("System time should be after Unix epoch") - .as_millis(); - std::env::temp_dir().join(format!("keywatch_{name}_{timestamp}")) - } - - fn init_git_repo(path: &std::path::Path) { - fs::create_dir_all(path).expect("create repo dir"); - let status = Command::new("git") - .args(["init", "--quiet"]) - .current_dir(path) - .status() - .expect("run git init"); - assert!(status.success(), "git init should succeed"); - } - - #[test] - fn test_managed_global_hooks_dir_prefers_xdg() { - let path = managed_global_hooks_dir( - Some("/tmp/xdg".into()), - Some("/tmp/home".into()), - None, - None, - ) - .expect("xdg path should resolve"); - - assert_eq!(path, std::path::PathBuf::from("/tmp/xdg/key-watch/hooks")); - } - - #[test] - fn test_managed_global_hooks_dir_falls_back_to_home() { - let path = managed_global_hooks_dir(None, Some("/tmp/home".into()), None, None) - .expect("home path should resolve"); - - assert_eq!( - path, - std::path::PathBuf::from("/tmp/home/.config/key-watch/hooks") - ); - } - - #[test] - fn test_global_hook_safety_allows_keywatch_hook() { - let temp_dir = unique_temp_dir("global_hook_safe"); - fs::create_dir_all(&temp_dir).expect("create temp dir"); - let hook_path = temp_dir.join("pre-commit"); - - fs::write(&hook_path, "#!/bin/bash\n# Installed by KeyWatch\n").expect("write hook file"); - - ensure_global_hook_target_is_safe(&hook_path).expect("keywatch hook should be reusable"); - - fs::remove_file(&hook_path).expect("remove hook file"); - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_global_hook_safety_rejects_foreign_hook() { - let temp_dir = unique_temp_dir("global_hook_foreign"); - fs::create_dir_all(&temp_dir).expect("create temp dir"); - let hook_path = temp_dir.join("pre-commit"); - - fs::write(&hook_path, "#!/bin/bash\necho custom hook\n").expect("write hook file"); - - let error = ensure_global_hook_target_is_safe(&hook_path) - .expect_err("foreign hook should be rejected"); - assert!(error.contains("Refusing to overwrite existing global hook")); - - fs::remove_file(&hook_path).expect("remove hook file"); - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_resolve_local_hooks_dir_resolves_correctly() { - let temp_dir = unique_temp_dir("local_hooks_dir_resolution"); - init_git_repo(&temp_dir); - - let resolved = - resolve_local_hooks_dir_from(&temp_dir).expect("should resolve hooks dir inside repo"); - - assert!(resolved.is_absolute(), "hooks path should be absolute"); - assert!( - resolved.ends_with(".git/hooks"), - "hooks path should end with .git/hooks, but was: {}", - resolved.display() - ); - - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_git_init_creates_hooks_dir() { - let temp_dir = unique_temp_dir("local_hook_missing_git"); - init_git_repo(&temp_dir); - - assert!(temp_dir.join(".git/hooks").exists()); - - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_local_hook_safety_rejects_foreign_hook() { - let temp_dir = unique_temp_dir("local_hook_foreign"); - init_git_repo(&temp_dir); - let git_dir = temp_dir.join(".git"); - let git_hooks_dir = git_dir.join("hooks"); - fs::create_dir_all(&git_hooks_dir).expect("create hooks dir"); - let hook_path = git_hooks_dir.join("pre-commit"); - fs::write(&hook_path, "#!/bin/bash\necho custom hook\n").expect("write hook file"); - - let error = ensure_local_hook_target_is_safe_to_create(&hook_path) - .expect_err("foreign local hook should be rejected"); - assert!(error.contains("Refusing to overwrite existing local hook")); - - fs::remove_file(&hook_path).expect("remove hook file"); - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_global_uninstall_target_does_not_configure_missing_hooks_path() { - let install_target = resolve_hook_uninstall_target("pre-commit", true) - .expect("global uninstall target should resolve"); - let expected_hooks_dir = managed_global_hooks_dir( - env::var_os("XDG_CONFIG_HOME"), - env::var_os("HOME"), - env::var_os("APPDATA"), - env::var_os("USERPROFILE"), - ) - .expect("managed hooks dir should resolve"); - - assert!(install_target.is_global); - assert!(!install_target.configured_global_path); - assert_eq!(install_target.hooks_dir, expected_hooks_dir); - assert_eq!(install_target.path, expected_hooks_dir.join("pre-commit")); - } - - #[test] - fn test_managed_global_hooks_dir_prefers_appdata_when_home_missing() { - let path = managed_global_hooks_dir( - None, - None, - Some("C:/Users/test/AppData/Roaming".into()), - None, - ) - .expect("appdata path should resolve"); - - assert_eq!( - path, - std::path::PathBuf::from("C:/Users/test/AppData/Roaming/key-watch/hooks") - ); - } - - #[test] - fn test_managed_global_hooks_dir_errors_without_known_base_dir() { - let error = managed_global_hooks_dir(None, None, None, None) - .expect_err("missing env inputs should fail"); - - assert!(error.contains("Could not determine a directory for global git hooks")); - } #[test] fn test_calculate_exit_code_across_modes() { diff --git a/src/scanner.rs b/src/scanner.rs index 0abe9f8..47c99f8 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -178,41 +178,7 @@ pub fn run_scan( let mut detectors = initialize_detectors().map_err(|err| err.to_string())?; if let Some(cfg) = config { - if let Some(ref rules) = cfg.rules { - for rule in rules { - match Detector::new( - &rule.name, - &rule.pattern, - &rule.finding_type, - &rule.severity, - &[], - &[], - None, - ) { - Ok(det) => detectors.push(det), - Err(e) => eprintln!("Warning: custom rule '{}' skipped: {}", rule.name, e), - } - } - } - - if let Some(ref overrides) = cfg.overrides { - detectors.retain(|det| { - if let Some(override_config) = overrides.get(&det.name) { - if override_config.enabled == Some(false) { - return false; - } - } - true - }); - - for det in &mut detectors { - if let Some(override_config) = overrides.get(&det.name) { - if let Some(ref sev) = override_config.severity { - det.severity.clone_from(sev); - } - } - } - } + cfg.apply_to(&mut detectors); } let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() @@ -309,7 +275,7 @@ pub fn run_scan( } let unique_paths: Vec<_> = unique_paths.into_iter().collect(); - let exclude_patterns: Vec = args + let mut exclude_patterns: Vec = args .exclude .as_ref() .map(|exclude_str| { @@ -325,21 +291,13 @@ pub fn run_scan( .transpose()? .unwrap_or_default(); - let exclude_patterns = if let Some(cfg) = config { - if let Some(ref cfg_excludes) = cfg.exclude { - let mut patterns = exclude_patterns; - for pattern_str in cfg_excludes { - patterns.push(Pattern::new(pattern_str).map_err(|err| { - format!("Invalid config exclude pattern '{}': {}", pattern_str, err) - })?); - } - patterns - } else { - exclude_patterns + if let Some(excludes) = config.and_then(|cfg| cfg.exclude.as_ref()) { + for pattern_str in excludes { + exclude_patterns.push(Pattern::new(pattern_str).map_err(|err| { + format!("Invalid config exclude pattern '{}': {}", pattern_str, err) + })?); } - } else { - exclude_patterns - }; + } let results: Vec<(Vec, usize, usize, Option)> = unique_paths .into_par_iter() From 97ad337124f28aa4e1e10af5f45cb89a86e722b6 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sat, 1 Aug 2026 00:33:12 +0530 Subject: [PATCH 05/26] docs: add SVG architecture diagram, replace mermaid sections in README --- CHANGELOG.md | 2 +- README.md | 159 ++++++++-------------------------- docs/architecture.svg | 192 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 124 deletions(-) create mode 100644 docs/architecture.svg diff --git a/CHANGELOG.md b/CHANGELOG.md index d6b4f7d..a6cf133 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,7 @@ All notable changes to this project will be documented in this file. ### Documentation -- README architecture section with module diagrams, data flow trace, and type relationships +- README architecture section rewritten with an SVG architecture diagram (`docs/architecture.svg`), layer-by-layer overview, data flow trace, and core data type reference ## [1.1.0] - 2026-05-05 diff --git a/README.md b/README.md index 3524250..1e963d2 100644 --- a/README.md +++ b/README.md @@ -193,113 +193,17 @@ password = 'known-test-password' # keywatch:ignore ## Architecture -### Layers & Data Flow - -```mermaid -flowchart LR - subgraph Input["📥 INPUT"] - direction LR - FILES[("Files / Dirs")] - STDIN[("stdin")] - GIT[("git log -p")] - end - - subgraph Config["⚙️ CONFIGURATION"] - DET_TOML[("detectors.toml
827 lines of built-in rules")] - KW_TOML[(".keywatch.toml
Custom rules, overrides,
exclude patterns")] - end - - subgraph Pipeline["🔁 DETECTION PIPELINE"] - direction TB - COLLECT["1. Collect files
Recursive dir walk
Skip .git, skip binary"] - EXCLUDE["2. Exclude filter
CLI --exclude +
config.exclude"] - KEYWORD["3. Keyword pre-filter
Fast path: skip files
without matching keywords"] - REGEX["4. Regex match
Line detectors (single-line)
Multiline detectors ((?s))"] - ENTROPY["5. Shannon entropy
Only flag secrets above
configurable threshold"] - ALLOW["6. Allowlist + suppress
Detector allowlist regexes
Inline keywatch:ignore"] - end - - subgraph Post["📋 POST-PROCESSING"] - BASELINE["Baseline filter
SHA-256 fingerprint
Suppress known findings"] - end - - subgraph Output["📤 OUTPUT"] - JSON_OUT["JSON report
create_report()"] - SARIF_OUT["SARIF 2.1.0
create_sarif_report()"] - SUMMARY["Summary line
CRIT: 2, HIGH: 5, ..."] - end - - Input --> COLLECT - DET_TOML --> KEYWORD - DET_TOML --> REGEX - DET_TOML --> ENTROPY - DET_TOML --> ALLOW - KW_TOML --> EXCLUDE - COLLECT --> EXCLUDE - EXCLUDE --> KEYWORD --> REGEX --> ENTROPY --> ALLOW - ALLOW --> BASELINE - BASELINE --> JSON_OUT - BASELINE --> SARIF_OUT - BASELINE --> SUMMARY -``` +KeyWatch architecture -### Core Data Types +### Architecture Overview -```mermaid -classDiagram - class Detector { - +String name - +Regex regex - +String finding_type - +String severity - +Vec~Regex~ allowlist - +Vec~String~ keywords - +Option~f64~ entropy_threshold - +has_keywords(content) bool - +has_sufficient_entropy(matched) bool - } - - class Finding { - +String file_path - +usize line_number - +String finding_type - +Severity severity - +String matched_content - +String plugin_name - } - - class Severity { - <> - CRITICAL - HIGH - MEDIUM - LOW - } - - class ScanMetadata { - +usize files_scanned - +usize total_lines - +Vec~String~ excluded_files - } - - class KeywatchConfig { - +Option~Vec~CustomRule~~ rules - +Option~HashMap~String, DetectorOverride~~ overrides - +Option~Vec~String~~ exclude - } - - class Baseline { - +String version - +Vec~BaselineEntry~ entries - +filter_findings(findings) Vec~Finding~ - +update_with_findings(findings) - } - - Finding --> Severity - Detector --> Finding : produces - KeywatchConfig --> Detector : extends/overrides - Baseline --> Finding : filters -``` +KeyWatch is organized into five layers. Data flows top to bottom: input sources and configuration feed the detection pipeline, findings pass through post-processing, and results are serialized to stdout or a file. + +1. **Input** — the CLI accepts files, directories, stdin, or git history (`--git-history`). Flags control exclusion (`--exclude`), baselineing (`--baseline`, `--update-baseline`), output format (`--format`), config path (`--config`), and exit behavior (`--exit-mode`). +2. **Configuration** — `detectors.toml` ships with the binary and holds the built-in rules. An optional `.keywatch.toml` adds custom rules, per-detector severity/enable overrides, and exclude patterns. Configuration merges — it never replaces defaults. +3. **Detection pipeline** — six stages run per file: collect files (recursive walk, skipping `.git` and binary files), apply exclude globs, pre-filter by keyword (fast path that avoids regex on irrelevant files), match regexes (single-line and multiline `(?s)`), gate on Shannon entropy, and apply allowlists plus inline `keywatch:ignore` suppression. Files are scanned in parallel with rayon. +4. **Post-processing** — an optional baseline filter suppresses findings already recorded in the baseline file, keyed by a salted SHA-256 fingerprint of the matched content. +5. **Output** — findings serialize as JSON or SARIF 2.1.0 and are written to stdout or an output file, followed by a severity summary and an exit code derived from the exit mode. ### Data Flow (One Scan) @@ -311,26 +215,35 @@ load config (optional .keywatch.toml) ───► merge custom rules into detec │ merge exclude patterns ▼ collect files ───► exclude filter ───► parallel scan (rayon) - │ - ├─ keyword pre-filter (skip if no match) - ├─ regex match (line + multiline) - ├─ entropy threshold check - ├─ allowlist check - └─ inline suppression check - │ - ▼ - Vec + ScanMetadata - │ - ▼ - baseline filter (optional, --baseline) - │ - ▼ - report serialization (JSON or SARIF 2.1.0) - │ - ▼ - stdout / file / exit code + │ + ├─ keyword pre-filter (skip if no match) + ├─ regex match (line + multiline) + ├─ entropy threshold check + ├─ allowlist check + └─ inline suppression check + │ + ▼ + Vec + ScanMetadata + │ + ▼ + baseline filter (optional, --baseline) + │ + ▼ + report serialization (JSON or SARIF 2.1.0) + │ + ▼ + stdout / file / exit code ``` +### Core Data Types + +- **Detector** — a named rule: regex, finding type, severity, optional keywords for pre-filtering, an entropy threshold, and an allowlist. +- **Finding** — one detected secret: file path, line number, finding type, severity, matched content, and the detector that produced it. +- **Severity** — `Critical`, `High`, `Medium`, `Low`. +- **KeywatchConfig** — parsed `.keywatch.toml`: custom rules, per-detector overrides, and exclude patterns. +- **Baseline** — versioned collection of fingerprint entries; filters out already-known findings. +- **ScanMetadata** — files scanned, total lines, and excluded files, reported alongside findings. + ## Development ```sh diff --git a/docs/architecture.svg b/docs/architecture.svg new file mode 100644 index 0000000..92717d4 --- /dev/null +++ b/docs/architecture.svg @@ -0,0 +1,192 @@ + + + + + + + + + + + + + + + + INPUT + scan arguments from the CLI + + + Files / dirs + --paths ... + + + stdin + --stdin + + + git history + --git-history + + flags: --exclude, --baseline, --update-baseline, --format, --config, --exit-mode, --verbose + + + + CONFIGURATION + merged, never replaced + + + detectors.toml + built-in rules, 827 lines + + + .keywatch.toml + rules, overrides, excludes + + loaded at start; custom rules appended to detectors, overrides applied by name, + exclude patterns merged with CLI --exclude + + + + DETECTION PIPELINE + one pass over each file, parallelized with rayon across files + + + + + 1 + Collect files + recursive dir walk + skip .git, binary files + dedupe overlapping roots + + + + + 2 + Exclude filter + glob patterns from + CLI --exclude + config + files never read + + + + + 3 + Keyword pre-filter + fast path: skip if file + has no detector keyword + avoids regex cost + + + + + 4 + Regex match + line detectors (single-line) + multiline detectors ((?s)) + chunked, 50-line overlap + + + + + 5 + Entropy gate + Shannon entropy of match + must pass detector threshold + filters readable words + + + + + 6 + Allowlist + suppress + detector allowlist regexes + inline "keywatch:ignore" + emit Finding + + + + + + + + + + + POST-PROCESSING + + + Baseline filter (--baseline) + SHA-256 fingerprint match drops known findings + --update-baseline records new ones + + + + OUTPUT + + + JSON + --format json + + + SARIF 2.1.0 + --format sarif + + + Summary + severity counts + + written to stdout or --output file; exit code from --exit-mode (strict / critical / always) + + + + + paths / stdin / git log + + + + detectors + overrides + + exclude patterns + + + Vec<Finding> + ScanMetadata + + + filtered findings + + + + DATA MODEL + + + Detector + name, regex, keywords, allowlist + + + Finding + file, line, type, severity + + + Severity + critical / high / medium / low + + + KeywatchConfig + rules, overrides, exclude + + + Baseline + fingerprint entries, version + + + ScanMetadata + files scanned, lines, excluded + + + + + + + From ef5a83638fbe87d4ba8c5d4e035422d701266fb6 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:40:43 +0530 Subject: [PATCH 06/26] refactor: split configuration module --- src/config.rs | 163 ---------------------- src/config/mod.rs | 167 +++++++++++++++++++++++ src/config/tests.rs | 23 ++++ src/config/tests/application.rs | 231 ++++++++++++++++++++++++++++++++ src/config/tests/discovery.rs | 148 ++++++++++++++++++++ 5 files changed, 569 insertions(+), 163 deletions(-) delete mode 100644 src/config.rs create mode 100644 src/config/mod.rs create mode 100644 src/config/tests.rs create mode 100644 src/config/tests/application.rs create mode 100644 src/config/tests/discovery.rs diff --git a/src/config.rs b/src/config.rs deleted file mode 100644 index d657cc7..0000000 --- a/src/config.rs +++ /dev/null @@ -1,163 +0,0 @@ -use crate::detector::Detector; -use serde::Deserialize; -use std::collections::HashMap; -use std::fs; - -/// Default severity for custom rules that omit `severity` in their TOML entry. -const DEFAULT_SEVERITY: &str = "MEDIUM"; - -/// User-facing configuration that extends and overrides the built-in detectors. -/// -/// Loaded from `--config ` or `.keywatch.toml` in the scan root. -/// Merges with `detectors.toml`: custom rules are appended, overrides are -/// applied by detector name. -#[derive(Deserialize, Default)] -pub struct KeywatchConfig { - /// Additional detectors beyond those in detectors.toml - pub rules: Option>, - /// Per-detector overrides (by name) - pub overrides: Option>, - /// Extra exclude patterns (merged with --exclude CLI flag) - pub exclude: Option>, -} - -impl KeywatchConfig { - /// Apply this config to a detector set: append custom rules and apply - /// per-detector overrides (disable and/or re-severity by name). - pub fn apply_to(&self, detectors: &mut Vec) { - if let Some(rules) = &self.rules { - for rule in rules { - match Detector::new( - &rule.name, - &rule.pattern, - &rule.finding_type, - &rule.severity, - &[], - &[], - None, - ) { - Ok(det) => detectors.push(det), - Err(err) => { - eprintln!("Warning: custom rule '{}' skipped: {}", rule.name, err) - } - } - } - } - - if let Some(overrides) = &self.overrides { - detectors.retain(|det| match overrides.get(&det.name) { - Some(override_config) => override_config.enabled != Some(false), - None => true, - }); - - for det in detectors.iter_mut() { - if let Some(severity) = overrides.get(&det.name).and_then(|o| o.severity.as_ref()) { - det.severity.clone_from(severity); - } - } - } - } - - /// Load config from the given path. Returns None if the file doesn't exist - /// (config is optional). - pub fn load(path: Option<&str>) -> Result, String> { - let config_path = match path { - Some(p) => Some(p.to_string()), - None => find_config_in_cwd(), - }; - - let config_path = match config_path { - Some(p) => p, - None => return Ok(None), - }; - - let contents = fs::read_to_string(&config_path) - .map_err(|err| format!("Failed to read config '{}': {}", config_path, err))?; - - let config: KeywatchConfig = - toml::from_str(&contents).map_err(|err| format!("Invalid config: {}", err))?; - - Ok(Some(config)) - } -} - -#[derive(Deserialize, Clone)] -pub struct CustomRule { - pub name: String, - pub pattern: String, - pub finding_type: String, - #[serde(default = "default_severity")] - pub severity: String, - #[allow(dead_code)] - pub description: Option, -} - -#[derive(Deserialize, Clone)] -pub struct DetectorOverride { - pub enabled: Option, - pub severity: Option, -} - -/// Serde `default` attribute requires a function; it returns the -/// [`DEFAULT_SEVERITY`] constant. -fn default_severity() -> String { - DEFAULT_SEVERITY.to_string() -} - -fn find_config_in_cwd() -> Option { - let candidates = [".keywatch.toml", "keywatch.toml", ".kw.toml"]; - candidates - .iter() - .find(|name| std::path::Path::new(name).exists()) - .map(|&name| name.to_string()) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_parse_minimal_config() { - let toml_str = r#" -[[rules]] -name = "TestDetector" -pattern = "\\bTEST_SECRET_[A-Z]+\\b" -finding_type = "Test Secret" -severity = "HIGH" -"#; - let config: KeywatchConfig = toml::from_str(toml_str).expect("should parse"); - assert!(config.rules.is_some()); - assert_eq!(config.rules.as_ref().unwrap().len(), 1); - assert_eq!(config.rules.as_ref().unwrap()[0].name, "TestDetector"); - assert!(config.overrides.is_none()); - } - - #[test] - fn test_parse_config_with_overrides() { - let toml_str = r#" -[overrides] -AWSKeyDetector = { enabled = false } -GitHubTokenDetector = { severity = "CRITICAL" } -"#; - let config: KeywatchConfig = toml::from_str(toml_str).expect("should parse"); - let overrides = config.overrides.expect("overrides present"); - assert_eq!(overrides.len(), 2); - - let aws = overrides.get("AWSKeyDetector").expect("found"); - assert_eq!(aws.enabled, Some(false)); - assert!(aws.severity.is_none()); - - let gh = overrides.get("GitHubTokenDetector").expect("found"); - assert_eq!(gh.severity.as_deref(), Some("CRITICAL")); - assert!(gh.enabled.is_none()); - } - - #[test] - fn test_empty_config_is_default() { - let toml_str = ""; - let config: KeywatchConfig = toml::from_str(toml_str).expect("empty should parse"); - assert!(config.rules.is_none()); - assert!(config.overrides.is_none()); - assert!(config.exclude.is_none()); - } -} diff --git a/src/config/mod.rs b/src/config/mod.rs new file mode 100644 index 0000000..ba8aae3 --- /dev/null +++ b/src/config/mod.rs @@ -0,0 +1,167 @@ +use crate::detector::Detector; +use crate::report::Severity; +use serde::Deserialize; +use std::collections::HashMap; +use std::fs; +use std::path::{Path, PathBuf}; + +#[cfg(test)] +mod tests; + +/// User-facing configuration that extends and overrides the built-in detectors. +/// +/// Loaded from `--config ` or discovered from the first CLI scan path +/// (directory itself, or parent directory for a file path). Merges with +/// `detectors.toml`: custom rules are appended, overrides are applied by +/// detector name. +#[derive(Deserialize, Default)] +pub struct KeywatchConfig { + pub rules: Option>, + pub overrides: Option>, + pub exclude: Option>, +} + +impl KeywatchConfig { + /// Validate then apply this config to a detector set. + /// + /// All custom detectors are built in a scratch buffer first; on any error + /// the passed-in vector is left untouched (fail-closed / transactional). + pub fn apply_to(&self, detectors: &mut Vec) -> Result<(), String> { + // Phase 1 — validate: build every new detector before touching `detectors`. + let mut staged: Vec = Vec::new(); + if let Some(rules) = &self.rules { + for rule in rules { + let det = Detector::new( + &rule.name, + &rule.pattern, + &rule.finding_type, + rule.severity.as_str(), + &[], + &[], + None, + ) + .map_err(|err| format!("custom rule '{}': {}", rule.name, err))?; + staged.push(det); + } + } + + // Phase 2 — commit: all validation passed, mutate. + detectors.extend(staged); + + if let Some(overrides) = &self.overrides { + detectors.retain(|det| match overrides.get(&det.name) { + Some(ov) => ov.enabled != Some(false), + None => true, + }); + for det in detectors.iter_mut() { + if let Some(sev) = overrides.get(&det.name).and_then(|o| o.severity) { + det.severity = sev; + } + } + } + + 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, String> { + let cwd = std::env::current_dir() + .map_err(|err| format!("Failed to determine current directory: {}", err))?; + Self::load_for_paths_at_cwd(path, &[], &cwd) + } + + /// Load config with scan-path-aware discovery. + /// + /// Resolution order: + /// 1. `explicit_path` — used as-is; returns `Err` if the file is absent. + /// 2. First CLI scan path that is a directory — search that directory. + /// 3. First CLI scan path that is a file — search its parent directory. + /// 4. Current working directory — only when `scan_paths` is empty. + /// + /// Candidate filenames tried in order: `.keywatch.toml`, `keywatch.toml`, + /// `.kw.toml`. + pub fn load_for_paths( + explicit_path: Option<&str>, + scan_paths: &[String], + ) -> Result, String> { + let cwd = std::env::current_dir() + .map_err(|err| format!("Failed to determine current directory: {}", err))?; + Self::load_for_paths_at_cwd(explicit_path, scan_paths, &cwd) + } + + fn load_for_paths_at_cwd( + explicit_path: Option<&str>, + scan_paths: &[String], + cwd: &Path, + ) -> Result, String> { + let config_path: Option = if let Some(p) = explicit_path { + if !Path::new(p).exists() { + return Err(format!("Config file not found: '{}'", p)); + } + Some(p.to_string()) + } else { + find_config_candidates(scan_paths, cwd) + }; + + let config_path = match config_path { + Some(p) => p, + None => return Ok(None), + }; + + let contents = fs::read_to_string(&config_path) + .map_err(|err| format!("Failed to read config '{}': {}", config_path, err))?; + + toml::from_str(&contents) + .map(Some) + .map_err(|err| format!("Invalid config: {}", err)) + } +} + +#[derive(Deserialize, Clone)] +pub struct CustomRule { + pub name: String, + pub pattern: String, + pub finding_type: String, + #[serde(default = "default_severity")] + pub severity: Severity, + #[allow(dead_code)] + pub description: Option, +} + +#[derive(Deserialize, Clone)] +pub struct DetectorOverride { + pub enabled: Option, + pub severity: Option, +} + +fn default_severity() -> Severity { + Severity::Medium +} + +/// Candidate filenames tried in order when auto-discovering a config file. +const CONFIG_NAMES: [&str; 3] = [".keywatch.toml", "keywatch.toml", ".kw.toml"]; + +fn find_config_candidates(scan_paths: &[String], cwd: &Path) -> Option { + let search_dir: PathBuf = match scan_paths.first() { + Some(first) => { + let p = Path::new(first); + if p.is_dir() { + p.to_path_buf() + } else { + p.parent() + .map(Path::to_path_buf) + .filter(|parent| parent != Path::new("")) + .unwrap_or_else(|| PathBuf::from(".")) + } + } + None => cwd.to_path_buf(), + }; + + CONFIG_NAMES + .iter() + .map(|name| search_dir.join(name)) + .find(|p| p.exists()) + .and_then(|p| p.to_str().map(str::to_string)) +} diff --git a/src/config/tests.rs b/src/config/tests.rs new file mode 100644 index 0000000..a659eaf --- /dev/null +++ b/src/config/tests.rs @@ -0,0 +1,23 @@ +mod application; +mod discovery; + +use crate::detector::Detector; +use std::io::Write; +use tempfile::TempDir; + +fn write_file(dir: &TempDir, name: &str, content: &str) -> std::path::PathBuf { + let path = dir.path().join(name); + let mut file = std::fs::File::create(&path).unwrap(); + file.write_all(content.as_bytes()).unwrap(); + path +} + +fn minimal_rule_toml(name: &str, pattern: &str, severity: &str) -> String { + format!( + "[[rules]]\nname = \"{name}\"\npattern = \"{pattern}\"\nfinding_type = \"Test\"\nseverity = \"{severity}\"\n" + ) +} + +fn make_detectors() -> Vec { + vec![Detector::new("ExistingDet", r"\bFOO\b", "Foo", "HIGH", &[], &[], None).unwrap()] +} diff --git a/src/config/tests/application.rs b/src/config/tests/application.rs new file mode 100644 index 0000000..95d3b04 --- /dev/null +++ b/src/config/tests/application.rs @@ -0,0 +1,231 @@ +use super::make_detectors; +use crate::config::{DetectorOverride, KeywatchConfig}; +use crate::report::Severity; + +#[test] +fn test_severity_deserialize_case_insensitive() { + #[derive(serde::Deserialize)] + struct Wrap { + s: Severity, + } + + for (input, expected) in [ + ("critical", Severity::Critical), + ("CRITICAL", Severity::Critical), + ("high", Severity::High), + ("HIGH", Severity::High), + ("medium", Severity::Medium), + ("MEDIUM", Severity::Medium), + ("low", Severity::Low), + ("LOW", Severity::Low), + ] { + let toml_str = format!("s = \"{input}\""); + let wrap: Wrap = toml::from_str(&toml_str) + .unwrap_or_else(|err| panic!("failed to parse '{input}': {err}")); + assert_eq!(wrap.s, expected, "input = {input}"); + } +} + +#[test] +fn test_severity_deserialize_invalid_aborts() { + #[derive(Debug, serde::Deserialize)] + struct Wrap { + #[allow(dead_code)] + s: Severity, + } + + let result: Result = toml::from_str("s = \"BOGUS\""); + assert!( + result.is_err(), + "invalid severity must fail deserialization" + ); + + let msg = result.unwrap_err().to_string(); + assert!( + msg.contains("BOGUS"), + "error should mention the bad value: {msg}" + ); +} + +#[test] +fn test_custom_rule_default_severity_is_medium() { + let toml_str = r#" +[[rules]] +name = "NoSev" +pattern = "\\bFOO\\b" +finding_type = "Test" +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let rule = &config.rules.unwrap()[0]; + assert_eq!(rule.severity, Severity::Medium); +} + +#[test] +fn test_invalid_regex_returns_error() { + let toml_str = r#" +[[rules]] +name = "Bad" +pattern = "[invalid" +finding_type = "Test" +severity = "HIGH" +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let mut detectors = make_detectors(); + let original_len = detectors.len(); + let result = config.apply_to(&mut detectors); + + assert!(result.is_err(), "invalid regex must return Err"); + let msg = result.unwrap_err(); + assert!(msg.contains("Bad"), "error must name the rule: {msg}"); + assert_eq!( + detectors.len(), + original_len, + "detector vector must not be mutated on error" + ); +} + +#[test] +fn test_no_partial_mutation_second_rule_invalid() { + let toml_str = r#" +[[rules]] +name = "GoodRule" +pattern = "\\bGOOD\\b" +finding_type = "Good" +severity = "LOW" + +[[rules]] +name = "BadRule" +pattern = "[invalid" +finding_type = "Bad" +severity = "HIGH" +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let mut detectors = make_detectors(); + let original_len = detectors.len(); + let result = config.apply_to(&mut detectors); + + assert!(result.is_err()); + assert_eq!( + detectors.len(), + original_len, + "good rule must not be added when a later rule is invalid" + ); +} + +#[test] +fn test_invalid_custom_severity_aborts_at_parse() { + let toml_str = r#" +[[rules]] +name = "BadSev" +pattern = "\\bFOO\\b" +finding_type = "Test" +severity = "INVALID_SEVERITY" +"#; + + let result: Result = toml::from_str(toml_str); + assert!( + result.is_err(), + "invalid custom severity must fail TOML parsing" + ); +} + +#[test] +fn test_invalid_override_severity_aborts_at_parse() { + let toml_str = r#" +[overrides] +SomeDet = { severity = "NOT_A_SEVERITY" } +"#; + + let result: Result = toml::from_str(toml_str); + assert!( + result.is_err(), + "invalid override severity must fail TOML parsing" + ); +} + +#[test] +fn test_override_severity_applies_typed() { + let toml_str = r#" +[overrides] +ExistingDet = { severity = "LOW" } +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let mut detectors = make_detectors(); + config.apply_to(&mut detectors).unwrap(); + assert_eq!(detectors[0].severity, Severity::Low); +} + +#[test] +fn test_override_disable() { + let toml_str = r#" +[overrides] +ExistingDet = { enabled = false } +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let mut detectors = make_detectors(); + config.apply_to(&mut detectors).unwrap(); + assert!(detectors.is_empty(), "disabled detector must be removed"); +} + +#[test] +fn test_parse_minimal_config() { + let toml_str = r#" +[[rules]] +name = "TestDetector" +pattern = "\\bTEST_SECRET_[A-Z]+\\b" +finding_type = "Test Secret" +severity = "HIGH" +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).expect("should parse"); + assert!(config.rules.is_some()); + assert_eq!(config.rules.as_ref().unwrap().len(), 1); + assert_eq!(config.rules.as_ref().unwrap()[0].name, "TestDetector"); + assert!(config.overrides.is_none()); +} + +#[test] +fn test_parse_config_with_overrides() { + let toml_str = r#" +[overrides] +AWSKeyDetector = { enabled = false } +GitHubTokenDetector = { severity = "CRITICAL" } +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).expect("should parse"); + let overrides = config.overrides.expect("overrides present"); + assert_eq!(overrides.len(), 2); + + let aws = overrides.get("AWSKeyDetector").expect("found"); + assert_eq!(aws.enabled, Some(false)); + assert!(aws.severity.is_none()); + + let gh = overrides.get("GitHubTokenDetector").expect("found"); + assert_eq!(gh.severity, Some(Severity::Critical)); + assert!(gh.enabled.is_none()); +} + +#[test] +fn test_parse_override_severity_field_is_typed() { + let toml_str = r#" +[overrides] +Det = { severity = "medium" } +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let ov: &DetectorOverride = config.overrides.as_ref().unwrap().get("Det").unwrap(); + assert_eq!(ov.severity, Some(Severity::Medium)); +} + +#[test] +fn test_empty_config_is_default() { + let config: KeywatchConfig = toml::from_str("").expect("empty should parse"); + assert!(config.rules.is_none()); + assert!(config.overrides.is_none()); + assert!(config.exclude.is_none()); +} diff --git a/src/config/tests/discovery.rs b/src/config/tests/discovery.rs new file mode 100644 index 0000000..3dea02e --- /dev/null +++ b/src/config/tests/discovery.rs @@ -0,0 +1,148 @@ +use super::{minimal_rule_toml, write_file}; +use crate::config::KeywatchConfig; +use tempfile::TempDir; + +#[test] +fn test_config_discovery_directory_root() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + ".keywatch.toml", + &minimal_rule_toml("DirRule", r"\\bDIR\\b", "HIGH"), + ); + + let paths = vec![dir.path().to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .expect("config should be found in directory"); + let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + assert!(names.contains(&"DirRule".to_string())); +} + +#[test] +fn test_config_discovery_file_parent() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + ".keywatch.toml", + &minimal_rule_toml("FileParentRule", r"\\bFP\\b", "MEDIUM"), + ); + let scan_file = write_file(&dir, "secrets.txt", "some content"); + + let paths = vec![scan_file.to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .expect("config should be found in file's parent directory"); + let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + assert!(names.contains(&"FileParentRule".to_string())); +} + +#[test] +fn test_explicit_config_takes_precedence() { + let dir = TempDir::new().unwrap(); + write_file(&dir, ".keywatch.toml", "# empty"); + let explicit_dir = TempDir::new().unwrap(); + let explicit_cfg = write_file( + &explicit_dir, + "explicit.toml", + &minimal_rule_toml("ExplicitRule", r"\\bEXP\\b", "CRITICAL"), + ); + + let paths = vec![dir.path().to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(Some(explicit_cfg.to_str().unwrap()), &paths) + .unwrap() + .expect("explicit config must be loaded"); + let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + assert!(names.contains(&"ExplicitRule".to_string())); +} + +#[test] +fn test_no_paths_no_config_in_cwd_returns_ok() { + let result = KeywatchConfig::load_for_paths(None, &[]); + assert!(result.is_ok()); +} + +#[test] +fn test_explicit_nonexistent_path_returns_error() { + let result = KeywatchConfig::load_for_paths(Some("/nonexistent/path/config.toml"), &[]); + assert!(result.is_err()); + let msg = result.err().unwrap(); + assert!( + msg.contains("not found"), + "error should say 'not found': {msg}" + ); +} + +#[test] +fn test_candidate_filename_order_keywatch_toml_before_kw_toml() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + "keywatch.toml", + &minimal_rule_toml("KW", r"\\bKW\\b", "LOW"), + ); + write_file( + &dir, + ".kw.toml", + &minimal_rule_toml("KWShort", r"\\bKWS\\b", "LOW"), + ); + + let paths = vec![dir.path().to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .expect("should find keywatch.toml"); + let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + assert!( + names.contains(&"KW".to_string()), + "keywatch.toml should win" + ); + assert!( + !names.contains(&"KWShort".to_string()), + ".kw.toml must not be picked" + ); +} + +#[test] +fn test_candidate_filename_dotted_wins_over_plain() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + ".keywatch.toml", + &minimal_rule_toml("Dotted", r"\\bDT\\b", "LOW"), + ); + write_file( + &dir, + "keywatch.toml", + &minimal_rule_toml("Plain", r"\\bPL\\b", "LOW"), + ); + + let paths = vec![dir.path().to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .expect("should find .keywatch.toml"); + let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + assert!( + names.contains(&"Dotted".to_string()), + ".keywatch.toml should win" + ); + assert!( + !names.contains(&"Plain".to_string()), + "keywatch.toml must not be picked" + ); +} + +#[test] +fn test_load_none_uses_supplied_cwd() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + ".keywatch.toml", + &minimal_rule_toml("CwdRule", r"\\bCWD\\b", "LOW"), + ); + + let config = KeywatchConfig::load_for_paths_at_cwd(None, &[], dir.path()) + .unwrap() + .expect("config should be found in supplied cwd"); + let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + assert!(names.contains(&"CwdRule".to_string())); +} From 28897e875ad6d447cdf214eba0b5cd0d7a32884b Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:40:51 +0530 Subject: [PATCH 07/26] feat: apply configuration during scans --- src/cli.rs | 4 +++ src/lib.rs | 8 ++++- src/scanner.rs | 2 +- tests/scanner_tests.rs | 74 +++++++++++++++++++++++++++++------------- 4 files changed, 63 insertions(+), 25 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 5a94b8a..365acfa 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -77,6 +77,10 @@ pub struct ScanArgs { #[arg(long)] pub config: Option, + /// Disable automatic config discovery (an explicit --config still loads) + #[arg(long, default_value_t = false)] + pub no_config_discovery: bool, + /// Output format for the report (json or sarif) #[arg(long, value_enum, default_value_t = OutputFormat::Json)] pub format: OutputFormat, diff --git a/src/lib.rs b/src/lib.rs index b84c11e..181a576 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,6 +7,8 @@ pub mod report; pub mod scanner; pub mod utils; +pub use hooks::{generate_pre_commit_hook, generate_pre_push_hook}; + use clap::Parser; use cli::{CliOptions, Command, ExitMode, HookAction, OutputFormat, ScanArgs, Shell}; use report::Finding; @@ -38,7 +40,11 @@ pub fn run_cli() -> Result<(), String> { fn run_scan_command(args: &ScanArgs) -> Result<(), String> { let start = Instant::now(); - let config = config::KeywatchConfig::load(args.config.as_deref())?; + let config = if args.config.is_some() || !args.no_config_discovery { + config::KeywatchConfig::load_for_paths(args.config.as_deref(), &args.paths)? + } else { + None + }; let (mut findings, scan_metadata) = scanner::run_scan(args, config.as_ref())?; if let Some(ref baseline_path) = args.baseline { diff --git a/src/scanner.rs b/src/scanner.rs index 47c99f8..f2e4ac1 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -178,7 +178,7 @@ pub fn run_scan( let mut detectors = initialize_detectors().map_err(|err| err.to_string())?; if let Some(cfg) = config { - cfg.apply_to(&mut detectors); + cfg.apply_to(&mut detectors)?; } let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() diff --git a/tests/scanner_tests.rs b/tests/scanner_tests.rs index dfbd231..1d6758c 100644 --- a/tests/scanner_tests.rs +++ b/tests/scanner_tests.rs @@ -20,13 +20,13 @@ fn git_available() -> bool { } fn init_git_repo(path: &Path) -> Result<(), String> { - fs::create_dir_all(path).map_err(|error| format!("create repo dir: {error}"))?; + fs::create_dir_all(path).map_err(|e| format!("create repo dir: {e}"))?; let status = Command::new("git") .args(["init", "--quiet"]) .current_dir(path) .status() - .map_err(|error| format!("git init: {error}"))?; + .map_err(|e| format!("git init: {e}"))?; if !status.success() { return Err("git init failed".to_string()); } @@ -36,7 +36,7 @@ fn init_git_repo(path: &Path) -> Result<(), String> { .args(["config", key, value]) .current_dir(path) .status() - .map_err(|error| format!("git config {key}: {error}"))?; + .map_err(|e| format!("git config {key}: {e}"))?; if !status.success() { return Err(format!("git config {key} failed")); } @@ -47,13 +47,13 @@ fn init_git_repo(path: &Path) -> Result<(), String> { fn commit_file(path: &Path, file_name: &str, contents: &str, message: &str) -> Result<(), String> { let file_path = path.join(file_name); - fs::write(&file_path, contents).map_err(|error| format!("write file: {error}"))?; + fs::write(&file_path, contents).map_err(|e| format!("write file: {e}"))?; let status = Command::new("git") .args(["add", file_name]) .current_dir(path) .status() - .map_err(|error| format!("git add: {error}"))?; + .map_err(|e| format!("git add: {e}"))?; if !status.success() { return Err("git add failed".to_string()); } @@ -62,7 +62,7 @@ fn commit_file(path: &Path, file_name: &str, contents: &str, message: &str) -> R .args(["commit", "-m", message, "--quiet"]) .current_dir(path) .status() - .map_err(|error| format!("git commit: {error}"))?; + .map_err(|e| format!("git commit: {e}"))?; if !status.success() { return Err("git commit failed".to_string()); } @@ -81,12 +81,12 @@ fn run_git_history_scan(current_dir: &Path, extra_args: &[&str]) -> Result Result<(), String> { - std::os::unix::fs::symlink(original, link).map_err(|error| format!("create symlink: {error}")) + std::os::unix::fs::symlink(original, link).map_err(|e| format!("create symlink: {e}")) } #[test] @@ -115,6 +115,7 @@ sk-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWX\n\ baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -147,6 +148,7 @@ Stripe: sk_test_51ABCDEF12345678901234567890\n\ baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -180,6 +182,7 @@ AZURE_STORAGE=DefaultEndpointsProtocol=https;AccountName=examplestore; baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -214,6 +217,7 @@ b3BlbnNzaC1ldi0xLjAAABgQDQD2FGB3V2t4=\n\ baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -242,6 +246,7 @@ fn test_multiple_detections_in_line() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -282,6 +287,7 @@ fn test_directory_scan_with_exclusions() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -321,6 +327,7 @@ fn test_exclude_pattern_filtering() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -362,6 +369,7 @@ fn test_dot_github_directory_is_scanned() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -389,6 +397,7 @@ fn test_scan_no_secrets() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -417,6 +426,7 @@ fn test_non_utf8_file_handling() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -449,6 +459,7 @@ fn test_multiple_files_scan() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -482,6 +493,7 @@ fn test_duplicate_paths_are_scanned_once() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -529,6 +541,7 @@ fn test_mixed_file_and_directory_paths_are_scanned_once() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -568,6 +581,7 @@ fn test_nonexistent_paths_are_ignored_without_counting_as_scanned() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -593,9 +607,9 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { let outside_file = test_dir.join("outside-secret.txt"); let link_path = test_dir.join("linked-secret.txt"); let _ = fs::remove_dir_all(&test_dir); - fs::create_dir_all(&test_dir).map_err(|error| format!("create test dir: {error}"))?; + fs::create_dir_all(&test_dir).map_err(|e| format!("create test dir: {e}"))?; fs::write(&outside_file, "AWS Key: AKIAABCDEFGHIJKLMNOP\n") - .map_err(|error| format!("write outside secret: {error}"))?; + .map_err(|e| format!("write outside secret: {e}"))?; symlink_file(&outside_file, &link_path)?; let options = ScanArgs { @@ -614,10 +628,11 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options)?; + let (findings, metadata) = run_scan(&options, None)?; assert!(findings.is_empty(), "Symlink target should not be scanned"); assert_eq!( @@ -625,7 +640,7 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { "Symlink should not count as scanned" ); - fs::remove_dir_all(&test_dir).map_err(|error| format!("cleanup: {error}"))?; + fs::remove_dir_all(&test_dir).map_err(|e| format!("cleanup: {e}"))?; Ok(()) } @@ -637,9 +652,9 @@ fn test_recursive_symlink_path_is_skipped() -> Result<(), String> { let scan_root = test_dir.join("scan-root"); let link_path = scan_root.join("linked-secret.txt"); let _ = fs::remove_dir_all(&test_dir); - fs::create_dir_all(&scan_root).map_err(|error| format!("create scan root: {error}"))?; + fs::create_dir_all(&scan_root).map_err(|e| format!("create scan root: {e}"))?; fs::write(&outside_file, "AWS Key: AKIAABCDEFGHIJKLMNOP\n") - .map_err(|error| format!("write outside secret: {error}"))?; + .map_err(|e| format!("write outside secret: {e}"))?; symlink_file(&outside_file, &link_path)?; let options = ScanArgs { @@ -657,9 +672,12 @@ fn test_recursive_symlink_path_is_skipped() -> Result<(), String> { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options)?; + let (findings, metadata) = run_scan(&options, None)?; assert!( findings.is_empty(), @@ -670,7 +688,7 @@ fn test_recursive_symlink_path_is_skipped() -> Result<(), String> { "Recursive symlink should not count as scanned" ); - fs::remove_dir_all(&test_dir).map_err(|error| format!("cleanup: {error}"))?; + fs::remove_dir_all(&test_dir).map_err(|e| format!("cleanup: {e}"))?; Ok(()) } @@ -693,6 +711,7 @@ fn test_detect_aadhaar() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -728,6 +747,7 @@ fn test_detect_voter_id() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -760,6 +780,7 @@ fn test_detect_pan_card() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -792,6 +813,7 @@ fn test_detect_abha() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -825,6 +847,7 @@ fn test_multiple_indian_ids() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -883,6 +906,7 @@ fn test_overlapping_scan_roots_with_exclusions() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -910,8 +934,7 @@ fn test_inline_suppression_ignores_marked_lines() -> Result<(), String> { let content = "\ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\nemail = user@example.com // keywatch:ignore\nFirebase: AIzaSy012345678901234567890123456789012\n"; - fs::write(&test_file, content) - .map_err(|error| format!("Unable to write test file: {error}"))?; + fs::write(&test_file, content).map_err(|e| format!("Unable to write test file: {}", e))?; let path_str = test_file.to_string_lossy().to_string(); @@ -926,6 +949,7 @@ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\n baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -955,7 +979,7 @@ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\n "Firebase key without suppression should still be found" ); - fs::remove_file(test_file).map_err(|error| format!("Cleanup failed: {error}"))?; + fs::remove_file(test_file).map_err(|e| format!("Cleanup failed: {}", e))?; Ok(()) } @@ -972,6 +996,7 @@ fn test_stdin_args_validation() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -990,17 +1015,17 @@ fn test_stdin_scanning_integration() -> Result<(), String> { .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .map_err(|error| format!("spawn key-watch --stdin: {error}"))?; + .map_err(|e| format!("spawn key-watch --stdin: {}", e))?; let mut stdin = child.stdin.take().ok_or("Failed to capture stdin")?; stdin .write_all(b"AWS Key: AKIAABCDEFGHIJKLMNOP\npassword = 'secret123'\n") - .map_err(|error| format!("write to stdin: {error}"))?; + .map_err(|e| format!("write to stdin: {}", e))?; drop(stdin); let output = child .wait_with_output() - .map_err(|error| format!("wait: {error}"))?; + .map_err(|e| format!("wait: {}", e))?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); @@ -1029,6 +1054,7 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; let one_path = ScanArgs { @@ -1042,6 +1068,7 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; let two_paths = ScanArgs { @@ -1058,6 +1085,7 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { baseline: None, update_baseline: false, config: None, + no_config_discovery: false, format: OutputFormat::Json, }; @@ -1154,7 +1182,7 @@ fn test_git_history_does_not_execute_textconv_helpers() -> Result<(), String> { .args(["config", "diff.keywatchmarker.textconv", &helper]) .current_dir(&repo_dir) .status() - .map_err(|error| format!("git config textconv: {error}"))?; + .map_err(|e| format!("git config textconv: {e}"))?; if !status.success() { return Err("git config textconv failed".to_string()); } From 5c23f11cf4c5d49914f4b09ae125d458f2a52fcb Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:40:59 +0530 Subject: [PATCH 08/26] feat: add redacted SARIF reporting --- src/report.rs | 192 +++++++++-------------------------------- src/report/sarif.rs | 157 ++++++++++++++++++++++++++++++++++ tests/report_tests.rs | 193 ++++++++++++++++++++++++++++++++++++------ 3 files changed, 364 insertions(+), 178 deletions(-) create mode 100644 src/report/sarif.rs diff --git a/src/report.rs b/src/report.rs index e97e418..e80ab27 100644 --- a/src/report.rs +++ b/src/report.rs @@ -1,5 +1,9 @@ -use serde::Serialize; -use std::{collections::HashMap, fmt, str::FromStr}; +use serde::de::{self, Visitor}; +use serde::{Deserialize, Deserializer, Serialize}; +use std::{fmt, str::FromStr}; +mod sarif; + +pub use sarif::create_sarif_report; #[derive(Serialize, Clone, PartialEq, Copy, Debug)] #[serde(rename_all = "UPPERCASE")] @@ -10,6 +14,18 @@ pub enum Severity { Low, } +impl Severity { + /// Return the canonical uppercase string representation. + pub fn as_str(self) -> &'static str { + match self { + Severity::Critical => "CRITICAL", + Severity::High => "HIGH", + Severity::Medium => "MEDIUM", + Severity::Low => "LOW", + } + } +} + /// Error returned when a string cannot be parsed as a [`Severity`]. #[derive(Debug, PartialEq)] pub struct ParseSeverityError { @@ -44,15 +60,27 @@ impl FromStr for Severity { } } -impl Severity { - /// Return the canonical uppercase string representation. - pub fn as_str(self) -> &'static str { - match self { - Severity::Critical => "CRITICAL", - Severity::High => "HIGH", - Severity::Medium => "MEDIUM", - Severity::Low => "LOW", +/// Deserialize [`Severity`] from a string using the existing case-insensitive +/// [`FromStr`] implementation. Serialization output (`UPPERCASE`) is governed +/// by the `#[serde(rename_all = "UPPERCASE")]` derive above and is not +/// affected by this impl. +impl<'de> Deserialize<'de> for Severity { + fn deserialize>(deserializer: D) -> Result { + struct SeverityVisitor; + + impl Visitor<'_> for SeverityVisitor { + type Value = Severity; + + fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("one of CRITICAL, HIGH, MEDIUM, LOW (case-insensitive)") + } + + fn visit_str(self, value: &str) -> Result { + Severity::from_str(value).map_err(de::Error::custom) + } } + + deserializer.deserialize_str(SeverityVisitor) } } @@ -118,150 +146,6 @@ pub fn create_report( serde_json::to_string_pretty(&report) } -/// Generate a SARIF 2.1.0 report from findings. -pub fn create_sarif_report( - findings: Vec, - _metadata: ScanMetadata, - scan_time: String, -) -> Result { - #[derive(Serialize)] - struct SarifLog { - #[serde(rename = "$schema")] - schema: &'static str, - version: &'static str, - runs: Vec, - } - - #[derive(Serialize)] - struct SarifRun { - tool: SarifTool, - results: Vec, - properties: HashMap, - } - - #[derive(Serialize)] - struct SarifTool { - driver: SarifDriver, - } - - #[derive(Serialize)] - struct SarifDriver { - name: &'static str, - #[serde(skip_serializing_if = "Option::is_none")] - version: Option, - information_uri: &'static str, - semantic_version: Option, - } - - #[derive(Serialize)] - struct SarifResult { - rule_id: String, - level: &'static str, - message: SarifMessage, - locations: Vec, - properties: HashMap, - } - - #[derive(Serialize)] - struct SarifMessage { - text: String, - } - - #[derive(Serialize)] - struct SarifLocation { - physical_location: SarifPhysicalLocation, - } - - #[derive(Serialize)] - struct SarifPhysicalLocation { - artifact_location: SarifArtifactLocation, - region: SarifRegion, - } - - #[derive(Serialize)] - struct SarifArtifactLocation { - uri: String, - } - - #[derive(Serialize)] - struct SarifRegion { - start_line: usize, - } - - fn severity_to_sarif_level(s: Severity) -> &'static str { - match s { - Severity::Critical | Severity::High => "error", - Severity::Medium => "warning", - Severity::Low => "note", - } - } - - let results: Vec = findings - .into_iter() - .map(|f| { - let rule_id = f.finding_type; - let level = severity_to_sarif_level(f.severity); - let rule_id_clone = rule_id.clone(); - let severity_str = format!("{:?}", f.severity); - let uri = f.file_path; - let start_line = f.line_number; - - let mut properties = HashMap::new(); - properties.insert( - "precision".to_string(), - serde_json::Value::String("very-high".to_string()), - ); - properties.insert( - "severity".to_string(), - serde_json::Value::String(severity_str), - ); - - SarifResult { - rule_id, - level, - message: SarifMessage { - text: format!("Potential {} detected", rule_id_clone), - }, - locations: vec![SarifLocation { - physical_location: SarifPhysicalLocation { - artifact_location: SarifArtifactLocation { uri }, - region: SarifRegion { start_line }, - }, - }], - properties, - } - }) - .collect(); - - let status = if results.is_empty() { "pass" } else { "fail" }; - - let mut properties = HashMap::new(); - properties.insert( - "status".to_string(), - serde_json::Value::String(status.to_string()), - ); - properties.insert("scanTime".to_string(), serde_json::Value::String(scan_time)); - - let log = SarifLog { - schema: "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", - version: "2.1.0", - runs: vec![SarifRun { - tool: SarifTool { - driver: SarifDriver { - name: "KeyWatch", - version: option_env!("CARGO_PKG_VERSION").map(|v| v.to_string()), - information_uri: "https://github.com/pixincreate/KeyWatch", - semantic_version: option_env!("CARGO_PKG_VERSION").map(|v| v.to_string()), - }, - }, - results, - properties, - }], - }; - - serde_json::to_string_pretty(&log) -} - pub fn get_severity_counts(findings: &[Finding]) -> (usize, usize, usize, usize) { let mut counts = (0, 0, 0, 0); for finding in findings { diff --git a/src/report/sarif.rs b/src/report/sarif.rs new file mode 100644 index 0000000..4ade62b --- /dev/null +++ b/src/report/sarif.rs @@ -0,0 +1,157 @@ +use super::{Finding, ScanMetadata, Severity}; +use serde::Serialize; +use std::collections::HashMap; + +/// Generate a SARIF 2.1.0 report from findings. +pub fn create_sarif_report( + findings: Vec, + _metadata: ScanMetadata, + scan_time: String, +) -> Result { + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifLog { + #[serde(rename = "$schema")] + schema: &'static str, + version: &'static str, + runs: Vec, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifRun { + tool: SarifTool, + results: Vec, + properties: HashMap, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifTool { + driver: SarifDriver, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifDriver { + name: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + version: Option, + information_uri: &'static str, + semantic_version: Option, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifResult { + rule_id: String, + level: &'static str, + message: SarifMessage, + locations: Vec, + properties: HashMap, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifMessage { + text: String, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifLocation { + physical_location: SarifPhysicalLocation, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifPhysicalLocation { + artifact_location: SarifArtifactLocation, + region: SarifRegion, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifArtifactLocation { + uri: String, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifRegion { + start_line: usize, + } + + fn severity_to_sarif_level(s: Severity) -> &'static str { + match s { + Severity::Critical | Severity::High => "error", + Severity::Medium => "warning", + Severity::Low => "note", + } + } + + let results: Vec = findings + .into_iter() + .map(|f| { + let rule_id = f.finding_type; + let level = severity_to_sarif_level(f.severity); + let rule_id_clone = rule_id.clone(); + let severity_str = f.severity.as_str(); + let uri = f.file_path; + let start_line = f.line_number; + + let mut properties = HashMap::new(); + properties.insert( + "precision".to_string(), + serde_json::Value::String("very-high".to_string()), + ); + properties.insert( + "severity".to_string(), + serde_json::Value::String(severity_str.to_string()), + ); + + SarifResult { + rule_id, + level, + message: SarifMessage { + text: format!("Potential {} detected", rule_id_clone), + }, + locations: vec![SarifLocation { + physical_location: SarifPhysicalLocation { + artifact_location: SarifArtifactLocation { uri }, + region: SarifRegion { start_line }, + }, + }], + properties, + } + }) + .collect(); + + let status = if results.is_empty() { "pass" } else { "fail" }; + + let mut properties = HashMap::new(); + properties.insert( + "status".to_string(), + serde_json::Value::String(status.to_string()), + ); + properties.insert("scanTime".to_string(), serde_json::Value::String(scan_time)); + + let log = SarifLog { + schema: "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", + version: "2.1.0", + runs: vec![SarifRun { + tool: SarifTool { + driver: SarifDriver { + name: "KeyWatch", + version: option_env!("CARGO_PKG_VERSION").map(|v| v.to_string()), + information_uri: "https://github.com/pixincreate/KeyWatch", + semantic_version: option_env!("CARGO_PKG_VERSION").map(|v| v.to_string()), + }, + }, + results, + properties, + }], + }; + + serde_json::to_string_pretty(&log) +} diff --git a/tests/report_tests.rs b/tests/report_tests.rs index be90b69..605043f 100644 --- a/tests/report_tests.rs +++ b/tests/report_tests.rs @@ -1,4 +1,11 @@ -use key_watch::report::{Finding, ScanMetadata, Severity, create_report, get_severity_counts}; +use key_watch::report::{ + Finding, ScanMetadata, Severity, create_report, create_sarif_report, get_severity_counts, +}; +use serde_json::Value; + +fn parse_json(output: &str) -> Value { + serde_json::from_str(output).expect("valid JSON output") +} #[test] fn test_create_report() { @@ -11,18 +18,13 @@ fn test_create_report() { let report = create_report(findings, metadata, "0.5s".to_string()) .expect("create_report should succeed"); - assert!( - report.contains("\"status\": \"PASS\""), - "Empty findings should be PASS" - ); - assert!( - report.contains("\"files_scanned\": 5"), - "Should include files_scanned" - ); - assert!( - report.contains("\"scan_time\": \"0.5s\""), - "Should include scan_time" - ); + let json = parse_json(&report); + + assert_eq!(json["status"], "PASS"); + assert_eq!(json["files_scanned"], 5); + assert_eq!(json["total_lines"], 100); + assert_eq!(json["excluded_files"], serde_json::json!([])); + assert_eq!(json["scan_time"], "0.5s"); } #[test] @@ -43,11 +45,11 @@ fn test_report_with_findings() { let report = create_report(findings, metadata, "0.1s".to_string()) .expect("create_report should succeed"); - assert!( - report.contains("\"status\": \"FAIL\""), - "Should report FAIL status" - ); - assert!(report.contains("AWS Key"), "Should include finding type"); + let json = parse_json(&report); + + assert_eq!(json["status"], "FAIL"); + assert_eq!(json["findings"][0]["finding_type"], "AWS Key"); + assert_eq!(json["findings"][0]["matched_content"], "AKIATESTKEY"); } #[test] @@ -68,13 +70,156 @@ fn test_create_report_includes_excluded_files_and_plugin_metadata() { let report = create_report(findings, metadata, "1.2s".to_string()) .expect("create_report should succeed"); + let json = parse_json(&report); + + assert_eq!( + json["excluded_files"], + serde_json::json!(["ignored.log", "vendor/secrets.txt"]) + ); + assert_eq!(json["findings"][0]["plugin_name"], "TokenDetector"); + assert_eq!(json["findings"][0]["matched_content"], "tok_test_123"); + assert_eq!(json["total_lines"], 80); +} + +#[test] +fn test_create_sarif_report_uses_camel_case_fields_and_hides_matched_content() { + let secret = "AKIATESTKEY".to_string(); + let findings = vec![Finding { + file_path: "src/secret.txt".to_string(), + line_number: 12, + finding_type: "AWS Key".to_string(), + severity: Severity::Critical, + matched_content: secret.clone(), + plugin_name: "AwsKeyDetector".to_string(), + }]; + let metadata = ScanMetadata { + files_scanned: 1, + total_lines: 12, + excluded_files: vec![], + }; - assert!(report.contains("\"excluded_files\"")); - assert!(report.contains("ignored.log")); - assert!(report.contains("vendor/secrets.txt")); - assert!(report.contains("\"plugin_name\": \"TokenDetector\"")); - assert!(report.contains("\"matched_content\": \"tok_test_123\"")); - assert!(report.contains("\"total_lines\": 80")); + let sarif = create_sarif_report(findings, metadata, "2026-08-01T00:00:00Z".to_string()) + .expect("create_sarif_report should succeed"); + let json = parse_json(&sarif); + + assert_eq!( + json["$schema"], + "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json" + ); + assert_eq!(json["version"], "2.1.0"); + + let driver = &json["runs"][0]["tool"]["driver"]; + assert_eq!(driver["name"], "KeyWatch"); + assert!(driver.get("informationUri").is_some()); + assert!(driver.get("semanticVersion").is_some()); + assert!(driver.get("information_uri").is_none()); + assert!(driver.get("semantic_version").is_none()); + + let result = &json["runs"][0]["results"][0]; + assert_eq!(result["ruleId"], "AWS Key"); + assert_eq!(result["level"], "error"); + assert_eq!(result["message"]["text"], "Potential AWS Key detected"); + assert_eq!( + result["locations"][0]["physicalLocation"]["artifactLocation"]["uri"], + "src/secret.txt" + ); + assert_eq!( + result["locations"][0]["physicalLocation"]["region"]["startLine"], + 12 + ); + assert!(result.get("rule_id").is_none()); + assert!(result.get("physical_location").is_none()); + assert!( + result["locations"][0]["physicalLocation"] + .get("artifact_location") + .is_none() + ); + assert!( + result["locations"][0]["physicalLocation"]["region"] + .get("start_line") + .is_none() + ); + + assert_eq!( + result["properties"]["severity"], + Severity::Critical.as_str() + ); + assert_eq!(result["properties"]["precision"], "very-high"); + assert!(!sarif.contains(&secret)); +} + +#[test] +fn test_create_sarif_report_maps_all_severities_to_expected_levels() { + let findings = vec![ + Finding { + file_path: "critical.txt".to_string(), + line_number: 1, + finding_type: "CriticalRule".to_string(), + severity: Severity::Critical, + matched_content: "critical-secret".to_string(), + plugin_name: "CriticalDetector".to_string(), + }, + Finding { + file_path: "high.txt".to_string(), + line_number: 2, + finding_type: "HighRule".to_string(), + severity: Severity::High, + matched_content: "high-secret".to_string(), + plugin_name: "HighDetector".to_string(), + }, + Finding { + file_path: "medium.txt".to_string(), + line_number: 3, + finding_type: "MediumRule".to_string(), + severity: Severity::Medium, + matched_content: "medium-secret".to_string(), + plugin_name: "MediumDetector".to_string(), + }, + Finding { + file_path: "low.txt".to_string(), + line_number: 4, + finding_type: "LowRule".to_string(), + severity: Severity::Low, + matched_content: "low-secret".to_string(), + plugin_name: "LowDetector".to_string(), + }, + ]; + let metadata = ScanMetadata { + files_scanned: 4, + total_lines: 4, + excluded_files: vec![], + }; + + let sarif = create_sarif_report(findings, metadata, "2026-08-01T00:00:00Z".to_string()) + .expect("create_sarif_report should succeed"); + let json = parse_json(&sarif); + let results = json["runs"][0]["results"] + .as_array() + .expect("results array"); + + let levels: Vec<&str> = results + .iter() + .map(|result| result["level"].as_str().expect("level string")) + .collect(); + assert_eq!(levels, vec!["error", "error", "warning", "note"]); + + let severities: Vec<&str> = results + .iter() + .map(|result| { + result["properties"]["severity"] + .as_str() + .expect("severity string") + }) + .collect(); + assert_eq!( + severities, + vec![ + Severity::Critical.as_str(), + Severity::High.as_str(), + Severity::Medium.as_str(), + Severity::Low.as_str() + ] + ); } #[test] From 6fff24a0e163267f16dd2e44f251928219062905 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:41:09 +0530 Subject: [PATCH 09/26] fix: isolate action config discovery --- .github/actions/keywatch-scan/action.yml | 12 +++++- scripts/validate-keywatch-action.py | 10 ++++- tests/hooks_tests.rs | 50 ++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/.github/actions/keywatch-scan/action.yml b/.github/actions/keywatch-scan/action.yml index 6de3401..3860188 100644 --- a/.github/actions/keywatch-scan/action.yml +++ b/.github/actions/keywatch-scan/action.yml @@ -23,6 +23,10 @@ inputs: description: 'Deprecated: verbose scanner output is disabled to avoid logging matched secrets' required: false default: 'false' + config: + description: 'Path to an explicit trusted .keywatch.toml config file' + required: false + default: '' outputs: findings-count: @@ -133,6 +137,7 @@ runs: INPUT_EXIT_MODE: ${{ inputs.exit-mode }} INPUT_OUTPUT: ${{ inputs.output }} INPUT_VERBOSE: ${{ inputs.verbose }} + INPUT_CONFIG: ${{ inputs.config }} run: | set -euo pipefail @@ -205,7 +210,7 @@ runs: if ((${#extra_args[@]})); then for arg in "${extra_args[@]}"; do case "$arg" in - --verbose|--verbose=*|-v*|--format|--format=*|-f|-f*|--output|--output=*|-o|-o*|--exit-mode|--exit-mode=*) + --verbose|--verbose=*|-v*|--format|--format=*|-f|-f*|--output|--output=*|-o|-o*|--exit-mode|--exit-mode=*|--config|--config=*|--no-config-discovery|--no-config-discovery=*) echo "ERROR: '$arg' is managed by action inputs and cannot be passed through args" >&2 exit 1 ;; @@ -224,8 +229,11 @@ runs: mkdir -p "$(dirname -- "$report_path")" fi - keywatch_args=(scan) + keywatch_args=(scan --no-config-discovery) keywatch_args+=(--exit-mode "$INPUT_EXIT_MODE") + if [ -n "$INPUT_CONFIG" ]; then + keywatch_args+=(--config "$INPUT_CONFIG") + fi if ((${#extra_args[@]})); then keywatch_args+=("${extra_args[@]}") fi diff --git a/scripts/validate-keywatch-action.py b/scripts/validate-keywatch-action.py index f99fd36..f8946a3 100755 --- a/scripts/validate-keywatch-action.py +++ b/scripts/validate-keywatch-action.py @@ -27,6 +27,7 @@ class ScanScenario(NamedTuple): expected_stderr: tuple[str, ...] = () expected_summary: tuple[str, ...] = () preseed_report: bool = False + config: str = "" def run_blocks(text: str) -> list[str]: @@ -74,10 +75,12 @@ def write_keywatch_stub(bin_dir: Path) -> Path: def run_scan_scenarios(scan_block: str) -> None: scenarios = ( - ScanScenario("glob-expands", "scan/*.txt", "", 0, "valid", 0, ("exit_code=0", "findings_count=2"), ("scan/match.txt",), ("scan/*.txt", "--verbose")), + ScanScenario("glob-expands", "scan/*.txt", "", 0, "valid", 0, ("exit_code=0", "findings_count=2"), ("--no-config-discovery", "scan/match.txt"), ("scan/*.txt", "--verbose")), + ScanScenario("explicit-config", ".", "", 0, "valid", 0, expected_capture=("--no-config-discovery\n", "--config\ntrusted.toml\n"), config="trusted.toml"), ScanScenario("semicolon-literal", "literal;touch_pwned", "", 0, "valid", 0, expected_capture=("literal;touch_pwned",)), ScanScenario("path-option-is-literal", "--verbose", "", 0, "valid", 0, expected_capture=("--\n--verbose\n",)), ScanScenario("format-long-value-rejected", ".", "--format sarif", 0, "valid", 1, expected_stderr=("managed by action inputs",)), + ScanScenario("config-passthrough-rejected", ".", "--config .keywatch.toml", 0, "valid", 1, expected_stderr=("managed by action inputs",)), ScanScenario("verbose-long-value-rejected", ".", "--verbose=true", 0, "valid", 1, expected_stderr=("managed by action inputs",)), ScanScenario("verbose-compact-short-rejected", ".", "-vv", 0, "valid", 1, expected_stderr=("managed by action inputs",)), ScanScenario("verbose-mode-long-allowed", ".", "--verbose-mode", 0, "valid", 0, expected_capture=("--verbose-mode",)), @@ -119,6 +122,7 @@ def run_scan_scenarios(scan_block: str) -> None: "INPUT_EXIT_MODE": "strict", "INPUT_OUTPUT": "", "INPUT_VERBOSE": "false", + "INPUT_CONFIG": scenario.config, "KEYWATCH_CAPTURE": str(workspace / "capture"), "KEYWATCH_STUB_EXIT": str(scenario.scanner_exit), "KEYWATCH_REPORT_MODE": scenario.report_mode, @@ -185,7 +189,8 @@ def main() -> int: require(fragment not in shell, f"forbidden shell fragment remains: {fragment}") require("${{ inputs." not in shell, "inputs must be routed through step env, not run blocks") - require("keywatch_args=(scan)" in shell, "scanner argv must be built with a Bash array") + require("keywatch_args=(scan --no-config-discovery)" in shell, "Action scans must disable untrusted config discovery") + require("keywatch_args+=(--config \"$INPUT_CONFIG\")" in shell, "explicit trusted config input must be supported") require("read -r -a paths" in shell, "paths input must be parsed without shell evaluation") require("compgen -G \"$path_token\"" in shell, "path globs must expand without shell evaluation") require("expanded_paths+=(\"$path_token\")" in shell, "unmatched globs and metachar literals must stay literal") @@ -194,6 +199,7 @@ def main() -> int: require("managed by action inputs" in shell, "args must not override verbose/output/exit-mode inputs") require("--verbose|--verbose=*|-v*" in shell, "all verbose arg forms must be rejected") require("--format|--format=*|-f|-f*" in shell, "all format arg forms must be rejected") + require("--config|--config=*|--no-config-discovery|--no-config-discovery=*" in shell, "config discovery controls must be managed by the Action") require("verbose output is disabled" in shell, "verbose mode must not log matched secrets") require("false|0|no|off|\"\")" in shell, "verbose=false must be an explicit non-verbose case") require("asset_arch=\"aarch64\"" in shell, "Darwin ARM64 must map to aarch64 release assets") diff --git a/tests/hooks_tests.rs b/tests/hooks_tests.rs index 055c481..5f0b5f7 100644 --- a/tests/hooks_tests.rs +++ b/tests/hooks_tests.rs @@ -639,6 +639,56 @@ fn test_cli_scan_defaults_to_strict_exit_mode() { } } +#[test] +fn test_cli_scan_can_disable_config_discovery() { + use clap::Parser; + + let options = CliOptions::try_parse_from([ + "key-watch", + "scan", + ".", + "--no-config-discovery", + "--config", + "trusted.toml", + ]) + .expect("trusted explicit config with disabled discovery should parse"); + + match options.command { + Command::Scan(scan_args) => { + assert!(scan_args.no_config_discovery); + assert_eq!(scan_args.config.as_deref(), Some("trusted.toml")); + } + _ => panic!("expected scan command"), + } +} + +#[cfg(unix)] +#[test] +fn test_cli_scan_ignores_repository_config_when_discovery_is_disabled() { + let temp_dir = unique_temp_dir("disabled_config_discovery"); + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + fs::write(temp_dir.join(".keywatch.toml"), "exclude = [\"**/*\"]\n") + .expect("write repository config"); + fs::write( + temp_dir.join("secret.txt"), + "AWS_ACCESS_KEY_ID=AKIAABCDEFGHIJKLMNOP\n", + ) + .expect("write secret fixture"); + + let output = std::process::Command::new(env!("CARGO_BIN_EXE_key-watch")) + .args([ + "scan", + temp_dir.to_str().expect("temp path should be UTF-8"), + "--no-config-discovery", + ]) + .output() + .expect("run key-watch"); + + assert_eq!(output.status.code(), Some(1)); + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + #[test] fn test_cli_pre_commit_rejects_blocked_repos() { use clap::Parser; From 3fab24d3cabf78e8e990d0220f03f847a8d80cbe Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:41:18 +0530 Subject: [PATCH 10/26] test: restore baseline regression coverage --- tests/baseline_tests.rs | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/baseline_tests.rs b/tests/baseline_tests.rs index d49c83a..5067c50 100644 --- a/tests/baseline_tests.rs +++ b/tests/baseline_tests.rs @@ -13,6 +13,33 @@ fn make_finding(file: &str, line: usize, ftype: &str, content: &str, plugin: &st } } +#[test] +fn test_baseline_filters_known_findings() { + let known_finding = make_finding( + "test.txt", + 5, + "AWS Key", + "AKIAIOSFODNN7EXAMPLE", + "AWSAccessKeyDetector", + ); + let baseline = Baseline::from_findings(std::slice::from_ref(&known_finding)); + + let findings = vec![ + known_finding, + make_finding( + "other.txt", + 1, + "API Key", + "sk-abc", + "GenericKeyValueDetector", + ), + ]; + + let filtered = baseline.filter_findings(findings); + assert_eq!(filtered.len(), 1); + assert_eq!(filtered[0].file_path, "other.txt"); +} + #[test] fn test_baseline_filters_moved_finding_in_same_file() { let known_finding = make_finding( @@ -97,6 +124,16 @@ fn test_baseline_save_and_load() -> Result<(), String> { Ok(()) } +#[test] +fn test_baseline_from_findings() { + let findings = vec![ + make_finding("f1.txt", 1, "A", "x", "D1"), + make_finding("f2.txt", 2, "B", "y", "D2"), + ]; + let baseline = Baseline::from_findings(&findings); + assert_eq!(baseline.entries.len(), 2); +} + #[test] fn test_baseline_from_findings_deduplicates_and_keeps_first_metadata() { let findings = vec![ @@ -109,7 +146,7 @@ fn test_baseline_from_findings_deduplicates_and_keeps_first_metadata() { } #[test] -fn test_baseline_update_merges_new_findings_once() { +fn test_baseline_update_merges_new_findings() { let mut baseline = Baseline::from_findings(&[make_finding("old.txt", 1, "X", "old", "D")]); let new_findings = vec![ make_finding("old.txt", 1, "X", "old", "D"), From 9ccfbfae5169f95fa2654a1cb472620058ecbec6 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sat, 1 Aug 2026 23:41:26 +0530 Subject: [PATCH 11/26] docs: document scan configuration options --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 1e963d2..1b54dfa 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,9 @@ key-watch verify-integrity ## Options - `scan ...` - Scan one or more files or directories +- `scan --config ` - Load configuration from an explicit `.keywatch.toml` path +- `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 --output ` - Save report to file From a7c8b68c7d71d1203c58fde8a95d87d8857c02fd Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 09:46:39 +0530 Subject: [PATCH 12/26] style: use descriptive config names --- src/config/mod.rs | 44 ++++++++++++++++++--------------- src/config/tests/application.rs | 10 ++++---- src/config/tests/discovery.rs | 42 ++++++++++++++++++++++++++----- 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/src/config/mod.rs b/src/config/mod.rs index ba8aae3..f20c21e 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -28,10 +28,10 @@ impl KeywatchConfig { /// the passed-in vector is left untouched (fail-closed / transactional). pub fn apply_to(&self, detectors: &mut Vec) -> Result<(), String> { // Phase 1 — validate: build every new detector before touching `detectors`. - let mut staged: Vec = Vec::new(); + let mut staged_detectors: Vec = Vec::new(); if let Some(rules) = &self.rules { for rule in rules { - let det = Detector::new( + let detector = Detector::new( &rule.name, &rule.pattern, &rule.finding_type, @@ -41,21 +41,24 @@ impl KeywatchConfig { None, ) .map_err(|err| format!("custom rule '{}': {}", rule.name, err))?; - staged.push(det); + staged_detectors.push(detector); } } // Phase 2 — commit: all validation passed, mutate. - detectors.extend(staged); + detectors.extend(staged_detectors); if let Some(overrides) = &self.overrides { - detectors.retain(|det| match overrides.get(&det.name) { - Some(ov) => ov.enabled != Some(false), + detectors.retain(|detector| match overrides.get(&detector.name) { + Some(detector_override) => detector_override.enabled != Some(false), None => true, }); - for det in detectors.iter_mut() { - if let Some(sev) = overrides.get(&det.name).and_then(|o| o.severity) { - det.severity = sev; + for detector in detectors.iter_mut() { + if let Some(severity) = overrides + .get(&detector.name) + .and_then(|detector_override| detector_override.severity) + { + detector.severity = severity; } } } @@ -96,17 +99,17 @@ impl KeywatchConfig { scan_paths: &[String], cwd: &Path, ) -> Result, String> { - let config_path: Option = if let Some(p) = explicit_path { - if !Path::new(p).exists() { - return Err(format!("Config file not found: '{}'", p)); + let config_path: Option = if let Some(path) = explicit_path { + if !Path::new(path).exists() { + return Err(format!("Config file not found: '{}'", path)); } - Some(p.to_string()) + Some(path.to_string()) } else { find_config_candidates(scan_paths, cwd) }; let config_path = match config_path { - Some(p) => p, + Some(path) => path, None => return Ok(None), }; @@ -146,11 +149,12 @@ const CONFIG_NAMES: [&str; 3] = [".keywatch.toml", "keywatch.toml", ".kw.toml"]; fn find_config_candidates(scan_paths: &[String], cwd: &Path) -> Option { let search_dir: PathBuf = match scan_paths.first() { Some(first) => { - let p = Path::new(first); - if p.is_dir() { - p.to_path_buf() + let scan_path = Path::new(first); + if scan_path.is_dir() { + scan_path.to_path_buf() } else { - p.parent() + scan_path + .parent() .map(Path::to_path_buf) .filter(|parent| parent != Path::new("")) .unwrap_or_else(|| PathBuf::from(".")) @@ -162,6 +166,6 @@ fn find_config_candidates(scan_paths: &[String], cwd: &Path) -> Option { CONFIG_NAMES .iter() .map(|name| search_dir.join(name)) - .find(|p| p.exists()) - .and_then(|p| p.to_str().map(str::to_string)) + .find(|candidate_path| candidate_path.exists()) + .and_then(|candidate_path| candidate_path.to_str().map(str::to_string)) } diff --git a/src/config/tests/application.rs b/src/config/tests/application.rs index 95d3b04..66b06cd 100644 --- a/src/config/tests/application.rs +++ b/src/config/tests/application.rs @@ -6,7 +6,7 @@ use crate::report::Severity; fn test_severity_deserialize_case_insensitive() { #[derive(serde::Deserialize)] struct Wrap { - s: Severity, + severity: Severity, } for (input, expected) in [ @@ -19,10 +19,10 @@ fn test_severity_deserialize_case_insensitive() { ("low", Severity::Low), ("LOW", Severity::Low), ] { - let toml_str = format!("s = \"{input}\""); + let toml_str = format!("severity = \"{input}\""); let wrap: Wrap = toml::from_str(&toml_str) .unwrap_or_else(|err| panic!("failed to parse '{input}': {err}")); - assert_eq!(wrap.s, expected, "input = {input}"); + assert_eq!(wrap.severity, expected, "input = {input}"); } } @@ -31,10 +31,10 @@ fn test_severity_deserialize_invalid_aborts() { #[derive(Debug, serde::Deserialize)] struct Wrap { #[allow(dead_code)] - s: Severity, + severity: Severity, } - let result: Result = toml::from_str("s = \"BOGUS\""); + let result: Result = toml::from_str("severity = \"BOGUS\""); assert!( result.is_err(), "invalid severity must fail deserialization" diff --git a/src/config/tests/discovery.rs b/src/config/tests/discovery.rs index 3dea02e..4bfa0be 100644 --- a/src/config/tests/discovery.rs +++ b/src/config/tests/discovery.rs @@ -15,7 +15,12 @@ fn test_config_discovery_directory_root() { let config = KeywatchConfig::load_for_paths(None, &paths) .unwrap() .expect("config should be found in directory"); - let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); assert!(names.contains(&"DirRule".to_string())); } @@ -33,7 +38,12 @@ fn test_config_discovery_file_parent() { let config = KeywatchConfig::load_for_paths(None, &paths) .unwrap() .expect("config should be found in file's parent directory"); - let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); assert!(names.contains(&"FileParentRule".to_string())); } @@ -52,7 +62,12 @@ fn test_explicit_config_takes_precedence() { let config = KeywatchConfig::load_for_paths(Some(explicit_cfg.to_str().unwrap()), &paths) .unwrap() .expect("explicit config must be loaded"); - let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); assert!(names.contains(&"ExplicitRule".to_string())); } @@ -91,7 +106,12 @@ fn test_candidate_filename_order_keywatch_toml_before_kw_toml() { let config = KeywatchConfig::load_for_paths(None, &paths) .unwrap() .expect("should find keywatch.toml"); - let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); assert!( names.contains(&"KW".to_string()), "keywatch.toml should win" @@ -120,7 +140,12 @@ fn test_candidate_filename_dotted_wins_over_plain() { let config = KeywatchConfig::load_for_paths(None, &paths) .unwrap() .expect("should find .keywatch.toml"); - let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); assert!( names.contains(&"Dotted".to_string()), ".keywatch.toml should win" @@ -143,6 +168,11 @@ fn test_load_none_uses_supplied_cwd() { let config = KeywatchConfig::load_for_paths_at_cwd(None, &[], dir.path()) .unwrap() .expect("config should be found in supplied cwd"); - let names: Vec<_> = config.rules.unwrap().into_iter().map(|r| r.name).collect(); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); assert!(names.contains(&"CwdRule".to_string())); } From 474e8d7c3eb11a316b8ca9477f8ad1f24fa254ba Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 09:46:45 +0530 Subject: [PATCH 13/26] style: use descriptive SARIF names --- src/report/sarif.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/report/sarif.rs b/src/report/sarif.rs index 4ade62b..bb92689 100644 --- a/src/report/sarif.rs +++ b/src/report/sarif.rs @@ -82,8 +82,8 @@ pub fn create_sarif_report( start_line: usize, } - fn severity_to_sarif_level(s: Severity) -> &'static str { - match s { + fn severity_to_sarif_level(severity: Severity) -> &'static str { + match severity { Severity::Critical | Severity::High => "error", Severity::Medium => "warning", Severity::Low => "note", @@ -92,13 +92,13 @@ pub fn create_sarif_report( let results: Vec = findings .into_iter() - .map(|f| { - let rule_id = f.finding_type; - let level = severity_to_sarif_level(f.severity); + .map(|finding| { + let rule_id = finding.finding_type; + let level = severity_to_sarif_level(finding.severity); let rule_id_clone = rule_id.clone(); - let severity_str = f.severity.as_str(); - let uri = f.file_path; - let start_line = f.line_number; + let severity_str = finding.severity.as_str(); + let uri = finding.file_path; + let start_line = finding.line_number; let mut properties = HashMap::new(); properties.insert( @@ -143,9 +143,10 @@ pub fn create_sarif_report( tool: SarifTool { driver: SarifDriver { name: "KeyWatch", - version: option_env!("CARGO_PKG_VERSION").map(|v| v.to_string()), + version: option_env!("CARGO_PKG_VERSION").map(|version| version.to_string()), information_uri: "https://github.com/pixincreate/KeyWatch", - semantic_version: option_env!("CARGO_PKG_VERSION").map(|v| v.to_string()), + semantic_version: option_env!("CARGO_PKG_VERSION") + .map(|version| version.to_string()), }, }, results, From 44a449023bd2ed337725301880c01407c32ec0e2 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 09:56:19 +0530 Subject: [PATCH 14/26] fix: prevent repository config from bypassing pre-push scan --- templates/pre-push.sh | 2 +- tests/hooks_tests.rs | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/templates/pre-push.sh b/templates/pre-push.sh index d7aec3f..ce5b801 100644 --- a/templates/pre-push.sh +++ b/templates/pre-push.sh @@ -134,7 +134,7 @@ main() { fi remote_url=$(resolve_remote_url "$remote_name" "$remote_url_arg") enforce_repository_policy "$remote_url" || exit 1 - "$KEYWATCH_BIN" scan . --exit-mode critical + "$KEYWATCH_BIN" scan . --exit-mode critical --no-config-discovery exit $? } diff --git a/tests/hooks_tests.rs b/tests/hooks_tests.rs index 5f0b5f7..6a5160d 100644 --- a/tests/hooks_tests.rs +++ b/tests/hooks_tests.rs @@ -520,6 +520,44 @@ fn test_pre_push_scans_unnormalizable_remote_when_no_filters_exist() { fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); } +#[cfg(unix)] +#[test] +fn test_pre_push_ignores_repository_config_that_disables_scanning() { + let temp_dir = unique_temp_dir("pre_push_ignores_repository_config"); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + fs::write(temp_dir.join(".keywatch.toml"), "exclude = [\"**/*\"]\n") + .expect("write repository config"); + fs::write( + temp_dir.join("secret.txt"), + "master_api_key = \"abcdefghijklmnopqrstuvwxyz1234\"\n", + ) + .expect("write critical secret"); + + let hook = generate_pre_push_hook(&hook_install_args(HookType::PrePush, None, None, None)); + let keywatch_script = format!( + "#!/bin/bash\nKEYWATCH_CONFIG_PATH=\"{}\" exec \"{}\" \"$@\"\n", + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("detectors.toml") + .display(), + env!("CARGO_BIN_EXE_key-watch") + ); + let output = run_hook( + &hook, + &["origin"], + "#!/bin/bash\nexit 1\n", + &keywatch_script, + &temp_dir, + ); + + assert_eq!( + output.status.code(), + Some(1), + "repository config must not suppress pre-push scanning: {}", + String::from_utf8_lossy(&output.stderr) + ); + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + #[test] fn test_hook_shell_escaping() { let options = hook_install_args( From 1c6a09ccb1e6c1a247b810dc89596c61d89e7df6 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 10:02:41 +0530 Subject: [PATCH 15/26] fix: ignore repository detector overrides in trusted scans --- src/detector.rs | 100 +++++++++++++++++++++++++++---------------- src/scanner.rs | 9 +++- tests/hooks_tests.rs | 67 ++++++++++++++++++++++++++++- 3 files changed, 135 insertions(+), 41 deletions(-) diff --git a/src/detector.rs b/src/detector.rs index e729813..fb3d388 100644 --- a/src/detector.rs +++ b/src/detector.rs @@ -20,20 +20,28 @@ pub enum DetectorError { } impl fmt::Display for DetectorError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { match self { DetectorError::InvalidPattern { detector, source } => { - write!(f, "invalid pattern in detector '{}': {}", detector, source) + write!( + formatter, + "invalid pattern in detector '{}': {}", + detector, source + ) } DetectorError::InvalidAllowlistPattern { detector, source } => { write!( - f, + formatter, "invalid allowlist pattern in detector '{}': {}", detector, source ) } DetectorError::InvalidSeverity { detector, source } => { - write!(f, "invalid severity in detector '{}': {}", detector, source) + write!( + formatter, + "invalid severity in detector '{}': {}", + detector, source + ) } } } @@ -73,9 +81,9 @@ impl Detector { })?; let mut compiled_allowlist = Vec::new(); - for pat in allowlist { + for pattern in allowlist { let compiled = - Regex::new(pat).map_err(|source| DetectorError::InvalidAllowlistPattern { + Regex::new(pattern).map_err(|source| DetectorError::InvalidAllowlistPattern { detector: name.to_string(), source, })?; @@ -97,10 +105,10 @@ impl Detector { if self.keywords.is_empty() { return true; } - let lower = content.to_lowercase(); + let lowercase_content = content.to_lowercase(); self.keywords .iter() - .any(|kw| lower.contains(&kw.to_lowercase())) + .any(|keyword| lowercase_content.contains(&keyword.to_lowercase())) } pub fn has_sufficient_entropy(&self, matched: &str) -> bool { @@ -111,18 +119,18 @@ impl Detector { } } -fn shannon_entropy(s: &str) -> f64 { - if s.is_empty() { +fn shannon_entropy(input: &str) -> f64 { + if input.is_empty() { return 0.0; } let mut counts = std::collections::HashMap::new(); - for ch in s.chars() { - *counts.entry(ch).or_insert(0) += 1; + for character in input.chars() { + *counts.entry(character).or_insert(0) += 1; } - let len = s.len() as f64; - counts.values().fold(0.0, |acc, &count| { - let p = count as f64 / len; - acc - p * p.log2() + let input_length = input.len() as f64; + counts.values().fold(0.0, |entropy, &count| { + let probability = count as f64 / input_length; + entropy - probability * probability.log2() }) } @@ -142,31 +150,49 @@ struct DetectorConfig { entropy: Option, } -fn find_detectors_config() -> std::path::PathBuf { +fn find_detectors_config(include_repository_config: bool) -> Option { std::env::var("KEYWATCH_CONFIG_PATH") .map(std::path::PathBuf::from) .ok() - .filter(|p| p.exists()) + .filter(|path| path.exists()) .or_else(|| { - let p = std::path::PathBuf::from("detectors.toml"); - if p.exists() { Some(p) } else { None } + if !include_repository_config { + return None; + } + + let repository_config = std::path::PathBuf::from("detectors.toml"); + repository_config.exists().then_some(repository_config) }) .or_else(|| { dirs::config_dir() - .map(|p| p.join("keywatch").join("detectors.toml")) - .filter(|p| p.exists()) + .map(|config_directory| config_directory.join("keywatch").join("detectors.toml")) + .filter(|path| path.exists()) }) .or_else(|| { std::env::current_exe() .ok() - .and_then(|p| p.parent().map(|d| d.join("detectors.toml"))) - .filter(|p| p.exists()) + .and_then(|executable_path| { + executable_path + .parent() + .map(|directory| directory.join("detectors.toml")) + }) + .filter(|path| path.exists()) }) - .unwrap_or_else(|| std::path::PathBuf::from("detectors.toml")) } pub fn initialize_detectors() -> Result, Box> { - let config_path = find_detectors_config(); + initialize_detectors_from_config(true) +} + +pub(crate) fn initialize_trusted_detectors() -> Result, Box> { + initialize_detectors_from_config(false) +} + +fn initialize_detectors_from_config( + include_repository_config: bool, +) -> Result, Box> { + let config_path = find_detectors_config(include_repository_config) + .ok_or("Failed to locate detectors.toml")?; let toml_contents = fs::read_to_string(&config_path) .map_err(|err| format!("Failed to read {}: {}", config_path.display(), err))?; @@ -174,26 +200,26 @@ pub fn initialize_detectors() -> Result, Box, _>>()?) diff --git a/src/scanner.rs b/src/scanner.rs index f2e4ac1..8c12cb7 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -1,6 +1,6 @@ use crate::cli::ScanArgs; use crate::config::KeywatchConfig; -use crate::detector::{Detector, initialize_detectors}; +use crate::detector::{Detector, initialize_detectors, initialize_trusted_detectors}; use crate::report::{Finding, ScanMetadata}; use glob::Pattern; use rayon::prelude::*; @@ -175,7 +175,12 @@ pub fn run_scan( args: &ScanArgs, config: Option<&KeywatchConfig>, ) -> Result<(Vec, ScanMetadata), String> { - let mut detectors = initialize_detectors().map_err(|err| err.to_string())?; + let mut detectors = if args.no_config_discovery { + initialize_trusted_detectors() + } else { + initialize_detectors() + } + .map_err(|err| err.to_string())?; if let Some(cfg) = config { cfg.apply_to(&mut detectors)?; diff --git a/tests/hooks_tests.rs b/tests/hooks_tests.rs index 6a5160d..68b6df0 100644 --- a/tests/hooks_tests.rs +++ b/tests/hooks_tests.rs @@ -94,6 +94,40 @@ fn run_hook(hook: &str, git_script: &str, keywatch_script: &str, cwd: &Path) -> run_hook_with_args(hook, git_script, keywatch_script, cwd, &[]) } +#[cfg(unix)] +fn run_hook_with_packaged_keywatch(hook: &str, cwd: &Path) -> Output { + let bin_dir = cwd.join("bin"); + fs::create_dir_all(&bin_dir).expect("create fake bin dir"); + write_executable(&bin_dir.join("git"), "#!/bin/bash\nexit 1\n"); + fs::copy( + env!("CARGO_BIN_EXE_key-watch"), + bin_dir.join(generated_binary_name()), + ) + .expect("copy KeyWatch binary"); + fs::copy( + Path::new(env!("CARGO_MANIFEST_DIR")).join("detectors.toml"), + bin_dir.join("detectors.toml"), + ) + .expect("copy packaged detectors"); + + let hook_path = cwd.join("hook.sh"); + fs::write(&hook_path, hook).expect("write hook"); + let path = format!( + "{}:{}", + bin_dir.display(), + std::env::var("PATH").unwrap_or_default() + ); + + std::process::Command::new("bash") + .arg(&hook_path) + .arg("origin") + .current_dir(cwd) + .env("PATH", path) + .env_remove("KEYWATCH_CONFIG_PATH") + .output() + .expect("run hook with packaged KeyWatch") +} + #[cfg(unix)] fn keywatch_script_that_records_args(marker: &Path) -> String { format!( @@ -424,7 +458,7 @@ fn test_pre_push_uses_named_remote_push_url_when_argv_url_is_absent() { assert!(output.status.success()); assert_eq!( fs::read_to_string(&marker).expect("read scanner args"), - "scan . --exit-mode critical\n" + "scan . --exit-mode critical --no-config-discovery\n" ); fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); } @@ -515,7 +549,7 @@ fn test_pre_push_scans_unnormalizable_remote_when_no_filters_exist() { assert!(output.status.success()); assert_eq!( fs::read_to_string(&marker).expect("read scanner args"), - "scan . --exit-mode critical\n" + "scan . --exit-mode critical --no-config-discovery\n" ); fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); } @@ -558,6 +592,31 @@ fn test_pre_push_ignores_repository_config_that_disables_scanning() { fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); } +#[cfg(unix)] +#[test] +fn test_pre_push_ignores_repository_detector_overrides() { + let temp_dir = unique_temp_dir("pre_push_ignores_repository_detectors"); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + fs::write(temp_dir.join("detectors.toml"), "detectors = []\n") + .expect("write repository detector overrides"); + fs::write( + temp_dir.join("secret.txt"), + "master_api_key = \"abcdefghijklmnopqrstuvwxyz1234\"\n", + ) + .expect("write high-severity secret"); + + let hook = generate_pre_push_hook(&hook_install_args(HookType::PrePush, None, None, None)); + let output = run_hook_with_packaged_keywatch(&hook, &temp_dir); + + assert_eq!( + output.status.code(), + Some(1), + "repository detectors must not suppress pre-push scanning: {}", + String::from_utf8_lossy(&output.stderr) + ); + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + #[test] fn test_hook_shell_escaping() { let options = hook_install_args( @@ -715,6 +774,10 @@ fn test_cli_scan_ignores_repository_config_when_discovery_is_disabled() { .expect("write secret fixture"); let output = std::process::Command::new(env!("CARGO_BIN_EXE_key-watch")) + .env( + "KEYWATCH_CONFIG_PATH", + Path::new(env!("CARGO_MANIFEST_DIR")).join("detectors.toml"), + ) .args([ "scan", temp_dir.to_str().expect("temp path should be UTF-8"), From 8592a75cffeaaec58ec30493b734a5ac846d5585 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:15:12 +0530 Subject: [PATCH 16/26] refactor: type CLI validation errors --- src/cli.rs | 61 +++++++++++--- tests/hooks_tests.rs | 185 +++++++++++++++++++++++-------------------- 2 files changed, 149 insertions(+), 97 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 365acfa..c50550d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,4 +1,42 @@ use clap::{Args, Parser, Subcommand, ValueEnum}; +use std::error::Error; +use std::fmt::{Display, Formatter}; + +#[derive(Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum CliValidationError { + GitHistoryWithStdin, + GitHistoryWithMultiplePaths, + StdinWithPaths, + MissingScanInput, + PreCommitRepositoryFilters, + PrePushExclude, +} + +impl Display for CliValidationError { + fn fmt(&self, formatter: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::GitHistoryWithStdin => { + formatter.write_str("Cannot specify both --git-history and --stdin") + } + Self::GitHistoryWithMultiplePaths => { + formatter.write_str("Cannot specify more than one path with --git-history") + } + Self::StdinWithPaths => formatter.write_str("Cannot specify both --stdin and paths"), + Self::MissingScanInput => { + formatter.write_str("Must specify paths, use --stdin, or use --git-history") + } + Self::PreCommitRepositoryFilters => formatter.write_str( + "--allowed-repos and --blocked-repos are only supported for pre-push hooks", + ), + Self::PrePushExclude => { + formatter.write_str("--exclude is only supported for pre-commit hooks") + } + } + } +} + +impl Error for CliValidationError {} /// KeyWatch: A secret scanner for your files and directories. #[derive(Parser, Debug)] @@ -9,7 +47,7 @@ pub struct CliOptions { } impl CliOptions { - pub fn validate(&self) -> Result<(), String> { + pub fn validate(&self) -> Result<(), CliValidationError> { match &self.command { Command::Scan(args) => args.validate(), Command::Hook(args) => args.validate(), @@ -87,21 +125,21 @@ pub struct ScanArgs { } impl ScanArgs { - pub fn validate(&self) -> Result<(), String> { + pub fn validate(&self) -> Result<(), CliValidationError> { if self.git_history { if self.stdin { - return Err("Cannot specify both --git-history and --stdin".to_string()); + return Err(CliValidationError::GitHistoryWithStdin); } if self.paths.len() > 1 { - return Err("Cannot specify more than one path with --git-history".to_string()); + return Err(CliValidationError::GitHistoryWithMultiplePaths); } return Ok(()); } if self.stdin && !self.paths.is_empty() { - return Err("Cannot specify both --stdin and paths".to_string()); + return Err(CliValidationError::StdinWithPaths); } if !self.stdin && self.paths.is_empty() { - return Err("Must specify paths, use --stdin, or use --git-history".to_string()); + return Err(CliValidationError::MissingScanInput); } Ok(()) } @@ -114,7 +152,7 @@ pub struct HookArgs { } impl HookArgs { - fn validate(&self) -> Result<(), String> { + fn validate(&self) -> Result<(), CliValidationError> { match &self.action { HookAction::Install(args) => args.validate(), HookAction::Uninstall(_) => Ok(()), @@ -155,19 +193,16 @@ pub struct HookInstallArgs { } impl HookInstallArgs { - fn validate(&self) -> Result<(), String> { + fn validate(&self) -> Result<(), CliValidationError> { match self.hook_type { HookType::PreCommit => { if self.allowed_repos.is_some() || self.blocked_repos.is_some() { - return Err( - "--allowed-repos and --blocked-repos are only supported for pre-push hooks" - .to_string(), - ); + return Err(CliValidationError::PreCommitRepositoryFilters); } } HookType::PrePush => { if self.exclude.is_some() { - return Err("--exclude is only supported for pre-commit hooks".to_string()); + return Err(CliValidationError::PrePushExclude); } } } diff --git a/tests/hooks_tests.rs b/tests/hooks_tests.rs index 68b6df0..4730519 100644 --- a/tests/hooks_tests.rs +++ b/tests/hooks_tests.rs @@ -1,5 +1,7 @@ -use key_watch::cli::{CliOptions, Command, ExitMode, HookAction, HookInstallArgs, HookType, Shell}; -use key_watch::hooks::{generate_pre_commit_hook, generate_pre_push_hook}; +use key_watch::cli::{ + CliOptions, CliValidationError, Command, ExitMode, HookAction, HookInstallArgs, HookType, Shell, +}; +use key_watch::{generate_pre_commit_hook, generate_pre_push_hook}; #[cfg(unix)] use std::{ fs, @@ -59,12 +61,12 @@ fn generated_binary_name() -> String { } #[cfg(unix)] -fn run_hook_with_args( +fn run_hook( hook: &str, + hook_args: &[&str], git_script: &str, keywatch_script: &str, cwd: &Path, - hook_args: &[&str], ) -> Output { let bin_dir = cwd.join("bin"); fs::create_dir_all(&bin_dir).expect("create fake bin dir"); @@ -89,11 +91,6 @@ fn run_hook_with_args( .expect("run hook") } -#[cfg(unix)] -fn run_hook(hook: &str, git_script: &str, keywatch_script: &str, cwd: &Path) -> Output { - run_hook_with_args(hook, git_script, keywatch_script, cwd, &[]) -} - #[cfg(unix)] fn run_hook_with_packaged_keywatch(hook: &str, cwd: &Path) -> Output { let bin_dir = cwd.join("bin"); @@ -135,7 +132,6 @@ fn keywatch_script_that_records_args(marker: &Path) -> String { marker.display() ) } - #[test] fn test_hook_generation_pre_commit() { let options = hook_install_args(HookType::PreCommit, None, None, Some("*.log,*.tmp")); @@ -223,7 +219,7 @@ fn test_pre_commit_failure_output_does_not_print_matched_secret() { let git_script = "#!/bin/bash\nif [ \"$1\" = \"diff\" ]; then printf 'secret.txt\\0'; exit 0; fi\nexit 1\n"; let keywatch_script = "#!/bin/bash\nprintf 'MATCHED_SECRET_VALUE\\n'\nprintf 'MATCHED_SECRET_VALUE\\n' >&2\nexit 1\n"; - let output = run_hook(&hook, git_script, keywatch_script, &temp_dir); + let output = run_hook(&hook, &[], git_script, keywatch_script, &temp_dir); let combined = format!( "{}{}", String::from_utf8_lossy(&output.stdout), @@ -254,7 +250,7 @@ fn test_pre_commit_skips_staged_symlinks() { let git_script = "#!/bin/bash\nif [ \"$1\" = \"diff\" ]; then printf 'linked.txt\\0'; exit 0; fi\nexit 1\n"; let keywatch_script = format!("#!/bin/bash\nprintf ran > '{}'\nexit 1\n", marker.display()); - let output = run_hook(&hook, git_script, &keywatch_script, &temp_dir); + let output = run_hook(&hook, &[], git_script, &keywatch_script, &temp_dir); assert!( output.status.success(), @@ -283,7 +279,7 @@ fn test_pre_push_allows_normalized_https_and_scp_equivalent() { )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://github.com/org/repo.git/\\n'; exit 0; fi\nexit 1\n"; let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook(&hook, git_script, keywatch_script, &temp_dir); + let output = run_hook(&hook, &["origin"], git_script, keywatch_script, &temp_dir); assert!( output.status.success(), @@ -307,7 +303,13 @@ fn test_pre_push_allows_https_userinfo_case_and_default_port() { None, )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://x-access-token:T@GitHub.COM:443/ORG/REPO.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook(&hook, git_script, "#!/bin/bash\nexit 0\n", &temp_dir); + let output = run_hook( + &hook, + &["origin"], + git_script, + "#!/bin/bash\nexit 0\n", + &temp_dir, + ); assert!(output.status.success()); fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); @@ -327,84 +329,34 @@ fn test_pre_push_blocks_https_userinfo_case_and_default_port() { None, )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://user@GitHub.COM:443/ORG/REPO.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook(&hook, git_script, "#!/bin/bash\nexit 0\n", &temp_dir); - - assert_eq!(output.status.code(), Some(1)); - fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); -} - -#[cfg(unix)] -#[test] -fn test_pre_push_rejects_spoofed_substring_remote() { - let temp_dir = unique_temp_dir("pre_push_spoofed_remote"); - let _ = fs::remove_dir_all(&temp_dir); - fs::create_dir_all(&temp_dir).expect("create temp dir"); - - let hook = generate_pre_push_hook(&hook_install_args( - HookType::PrePush, - Some("git@evil.example:org/repo.git"), - None, - None, - )); - let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://evil.example/github.com/org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook(&hook, git_script, keywatch_script, &temp_dir); - - assert_eq!(output.status.code(), Some(1)); - - fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); -} - -#[cfg(unix)] -#[test] -fn test_pre_push_blocks_when_actual_pushed_remote_is_unallowed() { - let temp_dir = unique_temp_dir("pre_push_blocks_actual_unallowed"); - let _ = fs::remove_dir_all(&temp_dir); - fs::create_dir_all(&temp_dir).expect("create temp dir"); - - let hook = generate_pre_push_hook(&hook_install_args( - HookType::PrePush, - Some("github.com/org/repo"), - None, - None, - )); - let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'git@github.com:org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook_with_args( + let output = run_hook( &hook, + &["origin"], git_script, - keywatch_script, + "#!/bin/bash\nexit 0\n", &temp_dir, - &["mirror", "https://evil.example/org/repo.git"], ); assert_eq!(output.status.code(), Some(1)); - fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); } #[cfg(unix)] #[test] -fn test_pre_push_blocks_when_actual_pushed_remote_is_blocked() { - let temp_dir = unique_temp_dir("pre_push_blocks_actual_blocked"); +fn test_pre_push_rejects_spoofed_substring_remote() { + let temp_dir = unique_temp_dir("pre_push_spoofed_remote"); let _ = fs::remove_dir_all(&temp_dir); fs::create_dir_all(&temp_dir).expect("create temp dir"); let hook = generate_pre_push_hook(&hook_install_args( HookType::PrePush, + Some("github.com/org/repo"), None, - Some("https://evil.example/org/repo.git"), None, )); - let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'git@github.com:org/repo.git\\n'; exit 0; fi\nexit 1\n"; + let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://evil.example/github.com/org/repo.git\\n'; exit 0; fi\nexit 1\n"; let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook_with_args( - &hook, - git_script, - keywatch_script, - &temp_dir, - &["origin", "https://evil.example/org/repo.git"], - ); + let output = run_hook(&hook, &["origin"], git_script, keywatch_script, &temp_dir); assert_eq!(output.status.code(), Some(1)); @@ -426,7 +378,7 @@ fn test_pre_push_blocks_normalized_https_and_scp_equivalent() { )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'git@github.com:org/repo.git\\n'; exit 0; fi\nexit 1\n"; let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook(&hook, git_script, keywatch_script, &temp_dir); + let output = run_hook(&hook, &["origin"], git_script, keywatch_script, &temp_dir); assert_eq!(output.status.code(), Some(1)); @@ -447,12 +399,12 @@ fn test_pre_push_uses_named_remote_push_url_when_argv_url_is_absent() { None, )); let git_script = "#!/bin/bash\nif [ \"$1 $2 $3\" = \"remote get-url --push\" ] && [ \"$4\" = \"mirror\" ]; then printf 'https://push.example/org/repo.git\\n'; exit 0; fi\nif [ \"$1 $2 $3\" = \"remote get-url mirror\" ]; then printf 'https://fetch.example/org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook_with_args( + let output = run_hook( &hook, + &["mirror"], git_script, &keywatch_script_that_records_args(&marker), &temp_dir, - &["mirror"], ); assert!(output.status.success()); @@ -476,12 +428,12 @@ fn test_pre_push_falls_back_to_named_remote_fetch_url() { None, )); let git_script = "#!/bin/bash\nif [ \"$1 $2 $3\" = \"remote get-url --push\" ]; then exit 1; fi\nif [ \"$1 $2\" = \"remote get-url\" ] && [ \"$3\" = \"mirror\" ]; then printf 'https://fetch.example/org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook_with_args( + let output = run_hook( &hook, + &["mirror"], git_script, "#!/bin/bash\nexit 0\n", &temp_dir, - &["mirror"], ); assert!(output.status.success()); @@ -501,7 +453,7 @@ fn test_pre_push_ignores_invalid_configured_entries() { None, )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'git@github.com:org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook(&hook, git_script, "#!/bin/bash\nexit 0\n", &temp_dir); + let output = run_hook(&hook, &[], git_script, "#!/bin/bash\nexit 0\n", &temp_dir); assert!(output.status.success()); fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); @@ -520,7 +472,7 @@ fn test_pre_push_fails_closed_for_unnormalizable_remote_when_filters_exist() { None, )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'not-a-repo\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook(&hook, git_script, "#!/bin/bash\nexit 0\n", &temp_dir); + let output = run_hook(&hook, &[], git_script, "#!/bin/bash\nexit 0\n", &temp_dir); assert_eq!(output.status.code(), Some(1)); assert!( @@ -541,6 +493,7 @@ fn test_pre_push_scans_unnormalizable_remote_when_no_filters_exist() { let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'not-a-repo\\n'; exit 0; fi\nexit 1\n"; let output = run_hook( &hook, + &[], git_script, &keywatch_script_that_records_args(&marker), &temp_dir, @@ -633,6 +586,62 @@ fn test_hook_shell_escaping() { ); } +#[cfg(unix)] +#[test] +fn test_pre_push_blocks_unallowed_actual_push_remote_even_when_origin_is_allowed() { + let temp_dir = unique_temp_dir("pre_push_blocks_actual_remote"); + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + + let hook = generate_pre_push_hook(&hook_install_args( + HookType::PrePush, + Some("github.com/org/repo"), + None, + None, + )); + let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://github.com/org/repo.git\\n'; exit 0; fi\nexit 1\n"; + let keywatch_script = "#!/bin/bash\nexit 0\n"; + let output = run_hook( + &hook, + &["origin", "https://evil.example/acme/blocked.git"], + git_script, + keywatch_script, + &temp_dir, + ); + + assert_eq!(output.status.code(), Some(1)); + + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + +#[cfg(unix)] +#[test] +fn test_pre_push_blocks_blocked_actual_push_remote_even_when_origin_is_safe() { + let temp_dir = unique_temp_dir("pre_push_blocks_blocked_actual_remote"); + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + + let hook = generate_pre_push_hook(&hook_install_args( + HookType::PrePush, + None, + Some("https://evil.example/acme/blocked.git"), + None, + )); + let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://github.com/org/repo.git\n'; exit 0; fi\nexit 1\n"; + let keywatch_script = "#!/bin/bash\nexit 0\n"; + let output = run_hook( + &hook, + &["origin", "https://evil.example/acme/blocked.git"], + git_script, + keywatch_script, + &temp_dir, + ); + + assert_eq!(output.status.code(), Some(1)); + + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + #[test] fn test_hook_missing_binary_path() { let options = hook_install_args(HookType::PrePush, None, None, None); @@ -807,8 +816,10 @@ fn test_cli_pre_commit_rejects_blocked_repos() { let error = options .validate() .expect_err("pre-commit should reject blocked repos"); - assert!( - error.contains("--allowed-repos and --blocked-repos are only supported for pre-push hooks") + assert_eq!(error, CliValidationError::PreCommitRepositoryFilters); + assert_eq!( + error.to_string(), + "--allowed-repos and --blocked-repos are only supported for pre-push hooks" ); } @@ -849,8 +860,10 @@ fn test_cli_pre_commit_rejects_repo_filters() { let error = options .validate() .expect_err("pre-commit should reject repo filters"); - assert!( - error.contains("--allowed-repos and --blocked-repos are only supported for pre-push hooks") + assert_eq!(error, CliValidationError::PreCommitRepositoryFilters); + assert_eq!( + error.to_string(), + "--allowed-repos and --blocked-repos are only supported for pre-push hooks" ); } @@ -871,7 +884,11 @@ fn test_cli_pre_push_rejects_exclude() { let error = options .validate() .expect_err("pre-push should reject exclude patterns"); - assert!(error.contains("--exclude is only supported for pre-commit hooks")); + assert_eq!(error, CliValidationError::PrePushExclude); + assert_eq!( + error.to_string(), + "--exclude is only supported for pre-commit hooks" + ); } #[test] From f93cfa93791d79e4d72eac004ca878fa117f0eff Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:15:34 +0530 Subject: [PATCH 17/26] refactor: type detector initialization errors --- src/detector.rs | 41 ++++++++++++++++++++++++++++----------- src/detector/error.rs | 43 +++++++++++++++++++++++++++++++++++++++++ tests/detector_tests.rs | 11 ++++++++++- 3 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 src/detector/error.rs diff --git a/src/detector.rs b/src/detector.rs index fb3d388..0097cc4 100644 --- a/src/detector.rs +++ b/src/detector.rs @@ -1,3 +1,7 @@ +mod error; + +pub use error::DetectorInitError; + use crate::report::{ParseSeverityError, Severity}; use regex::Regex; use serde::Deserialize; @@ -47,7 +51,16 @@ impl fmt::Display for DetectorError { } } -impl std::error::Error for DetectorError {} +impl std::error::Error for DetectorError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::InvalidPattern { source, .. } | Self::InvalidAllowlistPattern { source, .. } => { + Some(source) + } + Self::InvalidSeverity { source, .. } => Some(source), + } + } +} pub struct Detector { pub name: String, @@ -180,33 +193,38 @@ fn find_detectors_config(include_repository_config: bool) -> Option Result, Box> { +pub fn initialize_detectors() -> Result, DetectorInitError> { initialize_detectors_from_config(true) } -pub(crate) fn initialize_trusted_detectors() -> Result, Box> { +pub(crate) fn initialize_trusted_detectors() -> Result, DetectorInitError> { initialize_detectors_from_config(false) } fn initialize_detectors_from_config( include_repository_config: bool, -) -> Result, Box> { +) -> Result, DetectorInitError> { let config_path = find_detectors_config(include_repository_config) - .ok_or("Failed to locate detectors.toml")?; - let toml_contents = fs::read_to_string(&config_path) - .map_err(|err| format!("Failed to read {}: {}", config_path.display(), err))?; + .ok_or(DetectorInitError::ConfigNotFound)?; + let toml_contents = + fs::read_to_string(&config_path).map_err(|source| DetectorInitError::ReadConfig { + path: config_path.clone(), + source, + })?; let config: DetectorsConfig = toml::from_str(&toml_contents) - .map_err(|err| format!("Failed to parse detectors.toml: {}", err))?; + .map_err(|source| DetectorInitError::ParseConfig { source })?; let mut seen_names = std::collections::HashSet::new(); for detector_config in &config.detectors { if !seen_names.insert(detector_config.name.as_str()) { - return Err(format!("duplicate detector name '{}'", detector_config.name).into()); + return Err(DetectorInitError::DuplicateName { + detector: detector_config.name.clone(), + }); } } - Ok(config + config .detectors .into_iter() .map(|detector_config| { @@ -222,5 +240,6 @@ fn initialize_detectors_from_config( detector_config.entropy, ) }) - .collect::, _>>()?) + .collect::, _>>() + .map_err(|source| DetectorInitError::InvalidDetector { source }) } diff --git a/src/detector/error.rs b/src/detector/error.rs new file mode 100644 index 0000000..b936743 --- /dev/null +++ b/src/detector/error.rs @@ -0,0 +1,43 @@ +use super::DetectorError; +use std::{fmt, io, path::PathBuf}; + +#[derive(Debug)] +#[non_exhaustive] +pub enum DetectorInitError { + ConfigNotFound, + ReadConfig { path: PathBuf, source: io::Error }, + ParseConfig { source: toml::de::Error }, + DuplicateName { detector: String }, + InvalidDetector { source: DetectorError }, +} + +impl fmt::Display for DetectorInitError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + DetectorInitError::ConfigNotFound => { + write!(formatter, "Failed to locate detectors.toml") + } + DetectorInitError::ReadConfig { path, source } => { + write!(formatter, "Failed to read {}: {}", path.display(), source) + } + DetectorInitError::ParseConfig { source } => { + write!(formatter, "Failed to parse detectors.toml: {}", source) + } + DetectorInitError::DuplicateName { detector } => { + write!(formatter, "duplicate detector name '{}'", detector) + } + DetectorInitError::InvalidDetector { source } => write!(formatter, "{}", source), + } + } +} + +impl std::error::Error for DetectorInitError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + DetectorInitError::ConfigNotFound | DetectorInitError::DuplicateName { .. } => None, + DetectorInitError::ReadConfig { source, .. } => Some(source), + DetectorInitError::ParseConfig { source } => Some(source), + DetectorInitError::InvalidDetector { source } => Some(source), + } + } +} diff --git a/tests/detector_tests.rs b/tests/detector_tests.rs index ad6bfb6..59a6ef1 100644 --- a/tests/detector_tests.rs +++ b/tests/detector_tests.rs @@ -1,4 +1,4 @@ -use key_watch::detector::{Detector, DetectorError}; +use key_watch::detector::{Detector, DetectorError, DetectorInitError}; use key_watch::report::Severity; use std::str::FromStr; @@ -210,3 +210,12 @@ fn test_initialize_detectors_all_names_unique() { ); } } + +#[test] +fn test_detector_init_error_duplicate_name_display() { + let error = DetectorInitError::DuplicateName { + detector: "Dup".to_string(), + }; + + assert_eq!(error.to_string(), "duplicate detector name 'Dup'"); +} From cd1ecb741dc80b1baf696f7e62961f91f8e5d6ec Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:15:59 +0530 Subject: [PATCH 18/26] refactor: type configuration errors --- src/config/mod.rs | 94 ++++++++++++++++++++++++++++----- src/config/tests/application.rs | 21 ++++++-- src/config/tests/discovery.rs | 13 ++--- 3 files changed, 104 insertions(+), 24 deletions(-) diff --git a/src/config/mod.rs b/src/config/mod.rs index f20c21e..fea7c08 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1,7 +1,10 @@ use crate::detector::Detector; +use crate::detector::DetectorError; use crate::report::Severity; use serde::Deserialize; use std::collections::HashMap; +use std::error::Error as StdError; +use std::fmt; use std::fs; use std::path::{Path, PathBuf}; @@ -21,12 +24,70 @@ pub struct KeywatchConfig { pub exclude: Option>, } +#[derive(Debug)] +#[non_exhaustive] +pub enum ConfigError { + CurrentDirectory { + source: std::io::Error, + }, + NotFound { + path: String, + }, + Read { + path: String, + source: std::io::Error, + }, + Invalid { + source: toml::de::Error, + }, + CustomRule { + name: String, + source: DetectorError, + }, +} + +impl fmt::Display for ConfigError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ConfigError::CurrentDirectory { source } => { + write!( + formatter, + "Failed to determine current directory: {}", + source + ) + } + ConfigError::NotFound { path } => { + write!(formatter, "Config file not found: '{}'", path) + } + ConfigError::Read { path, source } => { + write!(formatter, "Failed to read config '{}': {}", path, source) + } + ConfigError::Invalid { source } => write!(formatter, "Invalid config: {}", source), + ConfigError::CustomRule { name, source } => { + write!(formatter, "custom rule '{}': {}", name, source) + } + } + } +} + +impl StdError for ConfigError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + match self { + ConfigError::CurrentDirectory { source } => Some(source), + ConfigError::NotFound { .. } => None, + ConfigError::Read { source, .. } => Some(source), + ConfigError::Invalid { source } => Some(source), + ConfigError::CustomRule { source, .. } => Some(source), + } + } +} + impl KeywatchConfig { /// Validate then apply this config to a detector set. /// /// All custom detectors are built in a scratch buffer first; on any error /// the passed-in vector is left untouched (fail-closed / transactional). - pub fn apply_to(&self, detectors: &mut Vec) -> Result<(), String> { + pub fn apply_to(&self, detectors: &mut Vec) -> Result<(), ConfigError> { // Phase 1 — validate: build every new detector before touching `detectors`. let mut staged_detectors: Vec = Vec::new(); if let Some(rules) = &self.rules { @@ -40,7 +101,10 @@ impl KeywatchConfig { &[], None, ) - .map_err(|err| format!("custom rule '{}': {}", rule.name, err))?; + .map_err(|source| ConfigError::CustomRule { + name: rule.name.clone(), + source, + })?; staged_detectors.push(detector); } } @@ -69,9 +133,9 @@ impl KeywatchConfig { /// 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, String> { - let cwd = std::env::current_dir() - .map_err(|err| format!("Failed to determine current directory: {}", err))?; + 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) } @@ -88,9 +152,9 @@ impl KeywatchConfig { pub fn load_for_paths( explicit_path: Option<&str>, scan_paths: &[String], - ) -> Result, String> { - let cwd = std::env::current_dir() - .map_err(|err| format!("Failed to determine current directory: {}", err))?; + ) -> Result, ConfigError> { + let cwd = + std::env::current_dir().map_err(|source| ConfigError::CurrentDirectory { source })?; Self::load_for_paths_at_cwd(explicit_path, scan_paths, &cwd) } @@ -98,10 +162,12 @@ impl KeywatchConfig { explicit_path: Option<&str>, scan_paths: &[String], cwd: &Path, - ) -> Result, String> { + ) -> Result, ConfigError> { let config_path: Option = if let Some(path) = explicit_path { if !Path::new(path).exists() { - return Err(format!("Config file not found: '{}'", path)); + return Err(ConfigError::NotFound { + path: path.to_string(), + }); } Some(path.to_string()) } else { @@ -113,12 +179,14 @@ impl KeywatchConfig { None => return Ok(None), }; - let contents = fs::read_to_string(&config_path) - .map_err(|err| format!("Failed to read config '{}': {}", config_path, err))?; + let contents = fs::read_to_string(&config_path).map_err(|source| ConfigError::Read { + path: config_path.clone(), + source, + })?; toml::from_str(&contents) .map(Some) - .map_err(|err| format!("Invalid config: {}", err)) + .map_err(|source| ConfigError::Invalid { source }) } } diff --git a/src/config/tests/application.rs b/src/config/tests/application.rs index 66b06cd..c2752fb 100644 --- a/src/config/tests/application.rs +++ b/src/config/tests/application.rs @@ -1,5 +1,6 @@ use super::make_detectors; -use crate::config::{DetectorOverride, KeywatchConfig}; +use crate::config::{ConfigError, DetectorOverride, KeywatchConfig}; +use crate::detector::DetectorError; use crate::report::Severity; #[test] @@ -76,9 +77,19 @@ severity = "HIGH" let original_len = detectors.len(); let result = config.apply_to(&mut detectors); - assert!(result.is_err(), "invalid regex must return Err"); - let msg = result.unwrap_err(); - assert!(msg.contains("Bad"), "error must name the rule: {msg}"); + let error = result.expect_err("invalid regex must return Err"); + match &error { + ConfigError::CustomRule { name, source } => { + assert_eq!(name, "Bad"); + assert!(matches!(source, &DetectorError::InvalidPattern { .. })); + } + other => panic!("expected CustomRule error, got {other:?}"), + } + let display = error.to_string(); + assert!( + display.starts_with("custom rule 'Bad': "), + "display prefix must be stable: {display}" + ); assert_eq!( detectors.len(), original_len, @@ -107,7 +118,7 @@ severity = "HIGH" let original_len = detectors.len(); let result = config.apply_to(&mut detectors); - assert!(result.is_err()); + assert!(matches!(result, Err(ConfigError::CustomRule { .. }))); assert_eq!( detectors.len(), original_len, diff --git a/src/config/tests/discovery.rs b/src/config/tests/discovery.rs index 4bfa0be..44a4b38 100644 --- a/src/config/tests/discovery.rs +++ b/src/config/tests/discovery.rs @@ -1,5 +1,5 @@ use super::{minimal_rule_toml, write_file}; -use crate::config::KeywatchConfig; +use crate::config::{ConfigError, KeywatchConfig}; use tempfile::TempDir; #[test] @@ -81,11 +81,12 @@ fn test_no_paths_no_config_in_cwd_returns_ok() { fn test_explicit_nonexistent_path_returns_error() { let result = KeywatchConfig::load_for_paths(Some("/nonexistent/path/config.toml"), &[]); assert!(result.is_err()); - let msg = result.err().unwrap(); - assert!( - msg.contains("not found"), - "error should say 'not found': {msg}" - ); + match result.err().unwrap() { + ConfigError::NotFound { path } => { + assert_eq!(path, "/nonexistent/path/config.toml"); + } + other => panic!("expected NotFound error, got {other:?}"), + } } #[test] From f3bb7c9a58835ddd4400599f614c00b6654aa2d2 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:16:18 +0530 Subject: [PATCH 19/26] refactor: type scanner errors --- src/scanner.rs | 100 ++++++++++++++++++++++++++--------------- src/scanner/error.rs | 82 +++++++++++++++++++++++++++++++++ tests/scanner_tests.rs | 85 +++++++++++++++++++++++++---------- 3 files changed, 208 insertions(+), 59 deletions(-) create mode 100644 src/scanner/error.rs diff --git a/src/scanner.rs b/src/scanner.rs index 8c12cb7..f790535 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -8,6 +8,10 @@ use std::fs; use std::io::{BufRead, BufReader}; use std::path::Path; +mod error; + +pub use error::ScannerError; + const INLINE_SUPPRESS: &str = "keywatch:ignore"; fn is_inline_suppressed(line: &str) -> bool { @@ -114,12 +118,12 @@ fn scan_content( (findings, total_lines) } -fn scan_stream( - reader: R, +fn scan_stream( + reader: ReaderType, path: &str, multiline_detectors: &[&Detector], line_detectors: &[&Detector], -) -> Result<(Vec, usize), String> { +) -> Result<(Vec, usize), ScannerError> { const CHUNK_SIZE: usize = 1000; const OVERLAP_LINES: usize = 50; @@ -129,7 +133,10 @@ fn scan_stream( let mut line_offset = 0; for line_result in reader.lines() { - let line = line_result.map_err(|e| format!("Read error on {}: {}", path, e))?; + let line = line_result.map_err(|source| ScannerError::ReadStream { + path: path.to_string(), + source, + })?; total_lines += 1; scan_line_detectors( @@ -174,16 +181,17 @@ fn scan_stream( pub fn run_scan( args: &ScanArgs, config: Option<&KeywatchConfig>, -) -> Result<(Vec, ScanMetadata), String> { +) -> Result<(Vec, ScanMetadata), ScannerError> { let mut detectors = if args.no_config_discovery { initialize_trusted_detectors() } else { initialize_detectors() } - .map_err(|err| err.to_string())?; + .map_err(|source| ScannerError::DetectorInit { source })?; if let Some(cfg) = config { - cfg.apply_to(&mut detectors)?; + cfg.apply_to(&mut detectors) + .map_err(|source| ScannerError::Config { source })?; } let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() @@ -208,9 +216,9 @@ pub fn run_scan( ]) .stdout(std::process::Stdio::piped()) .spawn() - .map_err(|err| format!("Failed to run git log: {}", err))?; + .map_err(|source| ScannerError::RunGitLog { source })?; - let stdout = child.stdout.take().ok_or("Failed to capture git stdout")?; + let stdout = child.stdout.take().ok_or(ScannerError::CaptureGitStdout)?; let reader = BufReader::new(stdout); let (findings, total_lines) = scan_stream( reader, @@ -221,9 +229,9 @@ pub fn run_scan( let status = child .wait() - .map_err(|e| format!("git process error: {}", e))?; + .map_err(|source| ScannerError::GitProcess { source })?; if !status.success() { - return Err("git log exited with non-zero status".to_string()); + return Err(ScannerError::GitLogNonZero); } let metadata = ScanMetadata { @@ -288,8 +296,12 @@ pub fn run_scan( .split(',') .filter(|pattern| !pattern.trim().is_empty()) .map(|pattern| { - Pattern::new(pattern.trim()) - .map_err(|err| format!("Invalid exclude pattern '{}': {}", pattern, err)) + Pattern::new(pattern.trim()).map_err(|source| { + ScannerError::InvalidExcludePattern { + pattern: pattern.to_string(), + source, + } + }) }) .collect::, _>>() }) @@ -298,8 +310,11 @@ pub fn run_scan( if let Some(excludes) = config.and_then(|cfg| cfg.exclude.as_ref()) { for pattern_str in excludes { - exclude_patterns.push(Pattern::new(pattern_str).map_err(|err| { - format!("Invalid config exclude pattern '{}': {}", pattern_str, err) + exclude_patterns.push(Pattern::new(pattern_str).map_err(|source| { + ScannerError::InvalidConfigExcludePattern { + pattern: pattern_str.to_string(), + source, + } })?); } } @@ -429,8 +444,8 @@ mod tests { use crate::detector::Detector; use std::io::Cursor; - fn make_detector(name: &str, pattern: &str, ftype: &str, sev: &str) -> Detector { - Detector::new(name, pattern, ftype, sev, &[], &[], None).unwrap() + fn make_detector(name: &str, pattern: &str, finding_type: &str, severity: &str) -> Detector { + Detector::new(name, pattern, finding_type, severity, &[], &[], None).unwrap() } #[test] @@ -446,16 +461,25 @@ mod tests { "HIGH", ), ]; - let (multi, line): (Vec<_>, Vec<_>) = detectors + let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() - .partition(|d| d.regex.as_str().contains("(?s)")); + .partition(|detector| detector.regex.as_str().contains("(?s)")); - let (findings, lines) = scan_stream(reader, "", &multi, &line).unwrap(); + let (findings, total_lines) = + scan_stream(reader, "", &multiline_detectors, &line_detectors).unwrap(); - assert_eq!(lines, 2); + assert_eq!(total_lines, 2); assert_eq!(findings.len(), 2); - assert!(findings.iter().any(|f| f.finding_type == "AWS Key")); - assert!(findings.iter().any(|f| f.finding_type == "Password")); + assert!( + findings + .iter() + .any(|finding| finding.finding_type == "AWS Key") + ); + assert!( + findings + .iter() + .any(|finding| finding.finding_type == "Password") + ); } #[test] @@ -468,11 +492,12 @@ mod tests { "Password", "HIGH", )]; - let (multi, line): (Vec<_>, Vec<_>) = detectors + let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() - .partition(|d| d.regex.as_str().contains("(?s)")); + .partition(|detector| detector.regex.as_str().contains("(?s)")); - let (findings, _) = scan_stream(reader, "", &multi, &line).unwrap(); + let (findings, _) = + scan_stream(reader, "", &multiline_detectors, &line_detectors).unwrap(); assert!( findings.is_empty(), "Suppressed line should produce no findings" @@ -489,11 +514,12 @@ mod tests { "Private Key", "HIGH", )]; - let (multi, line): (Vec<_>, Vec<_>) = detectors + let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() - .partition(|d| d.regex.as_str().contains("(?s)")); + .partition(|detector| detector.regex.as_str().contains("(?s)")); - let (findings, _) = scan_stream(reader, "", &multi, &line).unwrap(); + let (findings, _) = + scan_stream(reader, "", &multiline_detectors, &line_detectors).unwrap(); assert_eq!(findings.len(), 1); assert_eq!(findings[0].finding_type, "Private Key"); } @@ -501,8 +527,11 @@ mod tests { #[test] fn test_scan_stream_large_input_chunked() { let mut content = String::new(); - for i in 0..2500 { - content.push_str(&format!("line {}: password = 'secret{}'\n", i, i)); + for line_number in 0..2500 { + content.push_str(&format!( + "line {}: password = 'secret{}'\n", + line_number, line_number + )); } let reader = Cursor::new(content); let detectors = [make_detector( @@ -511,12 +540,13 @@ mod tests { "Password", "HIGH", )]; - let (multi, line): (Vec<_>, Vec<_>) = detectors + let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() - .partition(|d| d.regex.as_str().contains("(?s)")); + .partition(|detector| detector.regex.as_str().contains("(?s)")); - let (findings, lines) = scan_stream(reader, "", &multi, &line).unwrap(); - assert_eq!(lines, 2500); + let (findings, total_lines) = + scan_stream(reader, "", &multiline_detectors, &line_detectors).unwrap(); + assert_eq!(total_lines, 2500); assert_eq!( findings.len(), 2500, diff --git a/src/scanner/error.rs b/src/scanner/error.rs new file mode 100644 index 0000000..9c73fe9 --- /dev/null +++ b/src/scanner/error.rs @@ -0,0 +1,82 @@ +use crate::config::ConfigError; +use crate::detector::DetectorInitError; +use std::error::Error as StdError; +use std::{fmt, io}; + +#[derive(Debug)] +#[non_exhaustive] +pub enum ScannerError { + DetectorInit { + source: DetectorInitError, + }, + Config { + source: ConfigError, + }, + ReadStream { + path: String, + source: io::Error, + }, + RunGitLog { + source: io::Error, + }, + CaptureGitStdout, + GitProcess { + source: io::Error, + }, + GitLogNonZero, + InvalidExcludePattern { + pattern: String, + source: glob::PatternError, + }, + InvalidConfigExcludePattern { + pattern: String, + source: glob::PatternError, + }, +} + +impl fmt::Display for ScannerError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ScannerError::DetectorInit { source } => write!(formatter, "{}", source), + ScannerError::Config { source } => write!(formatter, "{}", source), + ScannerError::ReadStream { path, source } => { + write!(formatter, "Read error on {}: {}", path, source) + } + ScannerError::RunGitLog { source } => { + write!(formatter, "Failed to run git log: {}", source) + } + ScannerError::CaptureGitStdout => write!(formatter, "Failed to capture git stdout"), + ScannerError::GitProcess { source } => { + write!(formatter, "git process error: {}", source) + } + ScannerError::GitLogNonZero => write!(formatter, "git log exited with non-zero status"), + ScannerError::InvalidExcludePattern { pattern, source } => { + write!( + formatter, + "Invalid exclude pattern '{}': {}", + pattern, source + ) + } + ScannerError::InvalidConfigExcludePattern { pattern, source } => write!( + formatter, + "Invalid config exclude pattern '{}': {}", + pattern, source + ), + } + } +} + +impl StdError for ScannerError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + match self { + ScannerError::DetectorInit { source } => Some(source), + ScannerError::Config { source } => Some(source), + ScannerError::ReadStream { source, .. } => Some(source), + ScannerError::RunGitLog { source } => Some(source), + ScannerError::CaptureGitStdout | ScannerError::GitLogNonZero => None, + ScannerError::GitProcess { source } => Some(source), + ScannerError::InvalidExcludePattern { source, .. } + | ScannerError::InvalidConfigExcludePattern { source, .. } => Some(source), + } + } +} diff --git a/tests/scanner_tests.rs b/tests/scanner_tests.rs index 1d6758c..8195ec4 100644 --- a/tests/scanner_tests.rs +++ b/tests/scanner_tests.rs @@ -1,5 +1,5 @@ use key_watch::cli::{ExitMode, OutputFormat, ScanArgs}; -use key_watch::scanner::run_scan; +use key_watch::scanner::{ScannerError, run_scan}; use std::env::temp_dir; use std::fs; use std::path::{Path, PathBuf}; @@ -20,13 +20,13 @@ fn git_available() -> bool { } fn init_git_repo(path: &Path) -> Result<(), String> { - fs::create_dir_all(path).map_err(|e| format!("create repo dir: {e}"))?; + fs::create_dir_all(path).map_err(|error| format!("create repo dir: {error}"))?; let status = Command::new("git") .args(["init", "--quiet"]) .current_dir(path) .status() - .map_err(|e| format!("git init: {e}"))?; + .map_err(|error| format!("git init: {error}"))?; if !status.success() { return Err("git init failed".to_string()); } @@ -36,7 +36,7 @@ fn init_git_repo(path: &Path) -> Result<(), String> { .args(["config", key, value]) .current_dir(path) .status() - .map_err(|e| format!("git config {key}: {e}"))?; + .map_err(|error| format!("git config {key}: {error}"))?; if !status.success() { return Err(format!("git config {key} failed")); } @@ -47,13 +47,13 @@ fn init_git_repo(path: &Path) -> Result<(), String> { fn commit_file(path: &Path, file_name: &str, contents: &str, message: &str) -> Result<(), String> { let file_path = path.join(file_name); - fs::write(&file_path, contents).map_err(|e| format!("write file: {e}"))?; + fs::write(&file_path, contents).map_err(|error| format!("write file: {error}"))?; let status = Command::new("git") .args(["add", file_name]) .current_dir(path) .status() - .map_err(|e| format!("git add: {e}"))?; + .map_err(|error| format!("git add: {error}"))?; if !status.success() { return Err("git add failed".to_string()); } @@ -62,7 +62,7 @@ fn commit_file(path: &Path, file_name: &str, contents: &str, message: &str) -> R .args(["commit", "-m", message, "--quiet"]) .current_dir(path) .status() - .map_err(|e| format!("git commit: {e}"))?; + .map_err(|error| format!("git commit: {error}"))?; if !status.success() { return Err("git commit failed".to_string()); } @@ -81,12 +81,12 @@ fn run_git_history_scan(current_dir: &Path, extra_args: &[&str]) -> Result Result<(), String> { - std::os::unix::fs::symlink(original, link).map_err(|e| format!("create symlink: {e}")) + std::os::unix::fs::symlink(original, link).map_err(|error| format!("create symlink: {error}")) } #[test] @@ -344,6 +344,42 @@ fn test_exclude_pattern_filtering() { fs::remove_dir_all(test_dir).expect("Cleanup"); } +#[test] +fn test_invalid_cli_exclude_pattern_returns_typed_error() { + let options = ScanArgs { + paths: vec![".".to_string()], + stdin: false, + git_history: false, + output: None, + verbose: false, + exclude: Some("[".to_string()), + exit_mode: ExitMode::Strict, + baseline: None, + update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, + }; + + let error = match run_scan(&options, None) { + Ok(_) => panic!("invalid exclude should fail"), + Err(error) => error, + }; + + match &error { + ScannerError::InvalidExcludePattern { pattern, source: _ } => { + assert_eq!(pattern, "["); + } + other_error => panic!("expected invalid exclude pattern error, got {other_error:?}"), + } + assert!( + error + .to_string() + .starts_with("Invalid exclude pattern '[': "), + "legacy display prefix changed: {error}" + ); +} + #[test] fn test_dot_github_directory_is_scanned() { let temp_dir = temp_dir(); @@ -607,9 +643,9 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { let outside_file = test_dir.join("outside-secret.txt"); let link_path = test_dir.join("linked-secret.txt"); let _ = fs::remove_dir_all(&test_dir); - fs::create_dir_all(&test_dir).map_err(|e| format!("create test dir: {e}"))?; + fs::create_dir_all(&test_dir).map_err(|error| format!("create test dir: {error}"))?; fs::write(&outside_file, "AWS Key: AKIAABCDEFGHIJKLMNOP\n") - .map_err(|e| format!("write outside secret: {e}"))?; + .map_err(|error| format!("write outside secret: {error}"))?; symlink_file(&outside_file, &link_path)?; let options = ScanArgs { @@ -632,7 +668,7 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options, None)?; + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!(findings.is_empty(), "Symlink target should not be scanned"); assert_eq!( @@ -640,7 +676,7 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { "Symlink should not count as scanned" ); - fs::remove_dir_all(&test_dir).map_err(|e| format!("cleanup: {e}"))?; + fs::remove_dir_all(&test_dir).map_err(|error| format!("cleanup: {error}"))?; Ok(()) } @@ -652,9 +688,9 @@ fn test_recursive_symlink_path_is_skipped() -> Result<(), String> { let scan_root = test_dir.join("scan-root"); let link_path = scan_root.join("linked-secret.txt"); let _ = fs::remove_dir_all(&test_dir); - fs::create_dir_all(&scan_root).map_err(|e| format!("create scan root: {e}"))?; + fs::create_dir_all(&scan_root).map_err(|error| format!("create scan root: {error}"))?; fs::write(&outside_file, "AWS Key: AKIAABCDEFGHIJKLMNOP\n") - .map_err(|e| format!("write outside secret: {e}"))?; + .map_err(|error| format!("write outside secret: {error}"))?; symlink_file(&outside_file, &link_path)?; let options = ScanArgs { @@ -677,7 +713,7 @@ fn test_recursive_symlink_path_is_skipped() -> Result<(), String> { format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options, None)?; + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( findings.is_empty(), @@ -688,7 +724,7 @@ fn test_recursive_symlink_path_is_skipped() -> Result<(), String> { "Recursive symlink should not count as scanned" ); - fs::remove_dir_all(&test_dir).map_err(|e| format!("cleanup: {e}"))?; + fs::remove_dir_all(&test_dir).map_err(|error| format!("cleanup: {error}"))?; Ok(()) } @@ -934,7 +970,8 @@ fn test_inline_suppression_ignores_marked_lines() -> Result<(), String> { let content = "\ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\nemail = user@example.com // keywatch:ignore\nFirebase: AIzaSy012345678901234567890123456789012\n"; - fs::write(&test_file, content).map_err(|e| format!("Unable to write test file: {}", e))?; + fs::write(&test_file, content) + .map_err(|error| format!("Unable to write test file: {error}"))?; let path_str = test_file.to_string_lossy().to_string(); @@ -953,7 +990,7 @@ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\n format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options, None)?; + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let aws_suppressed = findings .iter() @@ -979,7 +1016,7 @@ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\n "Firebase key without suppression should still be found" ); - fs::remove_file(test_file).map_err(|e| format!("Cleanup failed: {}", e))?; + fs::remove_file(test_file).map_err(|error| format!("Cleanup failed: {error}"))?; Ok(()) } @@ -1015,17 +1052,17 @@ fn test_stdin_scanning_integration() -> Result<(), String> { .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .map_err(|e| format!("spawn key-watch --stdin: {}", e))?; + .map_err(|error| format!("spawn key-watch --stdin: {error}"))?; let mut stdin = child.stdin.take().ok_or("Failed to capture stdin")?; stdin .write_all(b"AWS Key: AKIAABCDEFGHIJKLMNOP\npassword = 'secret123'\n") - .map_err(|e| format!("write to stdin: {}", e))?; + .map_err(|error| format!("write to stdin: {error}"))?; drop(stdin); let output = child .wait_with_output() - .map_err(|e| format!("wait: {}", e))?; + .map_err(|error| format!("wait: {error}"))?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); @@ -1182,7 +1219,7 @@ fn test_git_history_does_not_execute_textconv_helpers() -> Result<(), String> { .args(["config", "diff.keywatchmarker.textconv", &helper]) .current_dir(&repo_dir) .status() - .map_err(|e| format!("git config textconv: {e}"))?; + .map_err(|error| format!("git config textconv: {error}"))?; if !status.success() { return Err("git config textconv failed".to_string()); } From 7c500193d8be3d3cd0e3abf988e5cec8fec49818 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:16:37 +0530 Subject: [PATCH 20/26] refactor: type baseline errors --- src/baseline.rs | 97 ++++++++++++++++++++++++++++++++++++----- tests/baseline_tests.rs | 30 +++++++++++-- 2 files changed, 114 insertions(+), 13 deletions(-) diff --git a/src/baseline.rs b/src/baseline.rs index fbd7efe..502cf26 100644 --- a/src/baseline.rs +++ b/src/baseline.rs @@ -1,6 +1,11 @@ use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; -use std::{collections::HashSet, fs, path::Path}; +use std::{ + collections::HashSet, + error::Error, + fmt, fs, io, + path::{Path, PathBuf}, +}; use crate::report::Finding; @@ -67,6 +72,71 @@ pub struct Baseline { pub entries: Vec, } +#[derive(Debug)] +#[non_exhaustive] +pub enum BaselineError { + Read { + path: PathBuf, + source: io::Error, + }, + Parse { + path: PathBuf, + source: serde_json::Error, + }, + Serialize { + source: serde_json::Error, + }, + Write { + path: PathBuf, + source: io::Error, + }, +} + +impl fmt::Display for BaselineError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Read { path, source } => { + write!( + formatter, + "Failed to read baseline '{}': {}", + path.display(), + source + ) + } + Self::Parse { path, source } => { + write!( + formatter, + "Failed to parse baseline '{}': {}", + path.display(), + source + ) + } + Self::Serialize { source } => { + write!(formatter, "Failed to serialize baseline: {}", source) + } + Self::Write { path, source } => { + write!( + formatter, + "Failed to write baseline '{}': {}", + path.display(), + source + ) + } + } + } +} + +impl Error for BaselineError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::Read { source, .. } => Some(source), + Self::Parse { source, .. } => Some(source), + Self::Serialize { source } => Some(source), + Self::Write { source, .. } => Some(source), + } + } +} + impl Baseline { pub fn new() -> Self { Self { @@ -75,30 +145,37 @@ impl Baseline { } } - pub fn load(path: &Path) -> Result { + pub fn load(path: &Path) -> Result { if !path.exists() { return Ok(Self::new()); } - let contents = fs::read_to_string(path) - .map_err(|err| format!("Failed to read baseline '{}': {}", path.display(), err))?; + let contents = fs::read_to_string(path).map_err(|source| BaselineError::Read { + path: path.to_path_buf(), + source, + })?; if contents.trim().is_empty() { return Ok(Self::new()); } - let baseline: Baseline = serde_json::from_str(&contents) - .map_err(|err| format!("Failed to parse baseline '{}': {}", path.display(), err))?; + let baseline: Baseline = + serde_json::from_str(&contents).map_err(|source| BaselineError::Parse { + path: path.to_path_buf(), + source, + })?; Ok(baseline) } - pub fn save(&self, path: &Path) -> Result<(), String> { + pub fn save(&self, path: &Path) -> Result<(), BaselineError> { let json = serde_json::to_string_pretty(self) - .map_err(|err| format!("Failed to serialize baseline: {}", err))?; + .map_err(|source| BaselineError::Serialize { source })?; - fs::write(path, json) - .map_err(|err| format!("Failed to write baseline '{}': {}", path.display(), err))?; + fs::write(path, json).map_err(|source| BaselineError::Write { + path: path.to_path_buf(), + source, + })?; Ok(()) } diff --git a/tests/baseline_tests.rs b/tests/baseline_tests.rs index 5067c50..fe2d58f 100644 --- a/tests/baseline_tests.rs +++ b/tests/baseline_tests.rs @@ -1,5 +1,6 @@ -use key_watch::baseline::{Baseline, BaselineEntry}; +use key_watch::baseline::{Baseline, BaselineEntry, BaselineError}; use key_watch::report::{Finding, Severity}; +use std::fs; use tempfile::tempdir; fn make_finding(file: &str, line: usize, ftype: &str, content: &str, plugin: &str) -> Finding { @@ -116,14 +117,37 @@ fn test_baseline_save_and_load() -> Result<(), String> { }], }; - baseline.save(&temp_file)?; - let loaded = Baseline::load(&temp_file)?; + baseline + .save(&temp_file) + .map_err(|error| error.to_string())?; + let loaded = Baseline::load(&temp_file).map_err(|error| error.to_string())?; assert_eq!(loaded.entries.len(), 1); assert_eq!(loaded.entries[0].file_path, "a.txt"); assert_eq!(loaded.entries[0].line_number, 1); Ok(()) } +#[test] +fn test_baseline_load_reports_parse_error_for_invalid_json() -> Result<(), String> { + let temp_dir = tempdir().map_err(|err| err.to_string())?; + let temp_file = temp_dir.path().join("baseline.json"); + fs::write(&temp_file, "{not valid json").map_err(|error| error.to_string())?; + + match Baseline::load(&temp_file) { + Err(BaselineError::Parse { path, source }) => { + assert_eq!(path, temp_file); + assert!( + BaselineError::Parse { path, source } + .to_string() + .starts_with("Failed to parse baseline '") + ); + } + other => panic!("expected parse error, got {other:?}"), + } + + Ok(()) +} + #[test] fn test_baseline_from_findings() { let findings = vec![ From 177637297a83177fe3d1047b144c45a1dae3472f Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:16:58 +0530 Subject: [PATCH 21/26] refactor: type hook lifecycle errors --- src/hooks.rs | 148 ++++++++++++++++++++------------------- src/hooks/error.rs | 168 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+), 69 deletions(-) create mode 100644 src/hooks/error.rs diff --git a/src/hooks.rs b/src/hooks.rs index 86de171..70708c8 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -5,6 +5,9 @@ use std::fs; use std::path::{Path, PathBuf}; use std::process::Command as ProcessCommand; +mod error; +pub use error::HookError; + const PRE_PUSH_TEMPLATE: &str = include_str!("../templates/pre-push.sh"); const PRE_COMMIT_TEMPLATE: &str = include_str!("../templates/pre-commit.sh"); @@ -79,7 +82,7 @@ fn hook_binary_name() -> String { /// Writes the rendered hook script to the resolved target path and makes it /// executable. For global installs, a managed hooks directory is created and /// configured via `git config --global core.hooksPath` if none is set. -pub fn install_hook(args: &HookInstallArgs) -> Result<(), String> { +pub fn install_hook(args: &HookInstallArgs) -> Result<(), HookError> { let hook_content = match args.hook_type { crate::cli::HookType::PrePush => generate_pre_push_hook(args), crate::cli::HookType::PreCommit => generate_pre_commit_hook(args), @@ -93,12 +96,9 @@ pub fn install_hook(args: &HookInstallArgs) -> Result<(), String> { } if let Some(parent) = install_target.path.parent() { - fs::create_dir_all(parent).map_err(|err| { - format!( - "Failed to create hook directory '{}': {}", - parent.display(), - err - ) + fs::create_dir_all(parent).map_err(|source| HookError::CreateHookDirectory { + path: parent.to_path_buf(), + source, })?; } @@ -107,10 +107,14 @@ pub fn install_hook(args: &HookInstallArgs) -> Result<(), String> { } let hook_path = install_target.path.to_string_lossy().into_owned(); - utils::write_to_file(&hook_path, &hook_content) - .map_err(|err| format!("Failed to install hook '{}': {}", hook_path, err))?; - utils::make_executable(&hook_path) - .map_err(|err| format!("Failed to make hook executable '{}': {}", hook_path, err))?; + utils::write_to_file(&hook_path, &hook_content).map_err(|source| HookError::InstallHook { + path: PathBuf::from(&hook_path), + source, + })?; + utils::make_executable(&hook_path).map_err(|source| HookError::MakeHookExecutable { + path: PathBuf::from(&hook_path), + source, + })?; if install_target.configured_global_path { println!( @@ -136,7 +140,7 @@ pub fn install_hook(args: &HookInstallArgs) -> Result<(), String> { /// /// Only removes hooks that KeyWatch previously installed (verified via the /// marker comment). Missing hooks are reported as a no-op success. -pub fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), String> { +pub fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), HookError> { let hook_type_str = args.hook_type.as_str(); let install_target = resolve_hook_uninstall_target(hook_type_str, args.global)?; @@ -159,12 +163,9 @@ pub fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), String> { "remove", )?; - fs::remove_file(&install_target.path).map_err(|err| { - format!( - "Failed to remove hook '{}': {}", - install_target.path.display(), - err - ) + fs::remove_file(&install_target.path).map_err(|source| HookError::RemoveHook { + path: install_target.path.clone(), + source, })?; if install_target.is_global { @@ -190,7 +191,7 @@ struct HookInstallTarget { } impl HookInstallTarget { - fn local(hook_type: &str) -> Result { + fn local(hook_type: &'static str) -> Result { let hooks_dir = resolve_local_hooks_dir()?; Ok(Self { path: hooks_dir.join(hook_type), @@ -200,7 +201,7 @@ impl HookInstallTarget { }) } - fn global(hook_type: &str) -> Result { + fn global(hook_type: &'static str) -> Result { if let Some(hooks_dir) = read_global_hooks_path()? { return Ok(Self { path: hooks_dir.join(hook_type), @@ -217,12 +218,11 @@ impl HookInstallTarget { env::var_os("USERPROFILE"), )?; - fs::create_dir_all(&managed_dir).map_err(|err| { - format!( - "Failed to create global hooks directory '{}': {}", - managed_dir.display(), - err - ) + fs::create_dir_all(&managed_dir).map_err(|source| { + HookError::CreateGlobalHooksDirectory { + path: managed_dir.clone(), + source, + } })?; configure_global_hooks_path(&managed_dir)?; @@ -235,7 +235,10 @@ impl HookInstallTarget { } } -fn resolve_hook_install_target(hook_type: &str, global: bool) -> Result { +fn resolve_hook_install_target( + hook_type: &'static str, + global: bool, +) -> Result { if global { HookInstallTarget::global(hook_type) } else { @@ -244,9 +247,9 @@ fn resolve_hook_install_target(hook_type: &str, global: bool) -> Result Result { +) -> Result { if !global { return HookInstallTarget::local(hook_type); } @@ -269,39 +272,39 @@ fn resolve_hook_uninstall_target( }) } -fn resolve_local_hooks_dir() -> Result { +fn resolve_local_hooks_dir() -> Result { resolve_local_hooks_dir_from(Path::new(".")) } -fn resolve_local_hooks_dir_from(cwd: &Path) -> Result { +fn resolve_local_hooks_dir_from(cwd: &Path) -> Result { let output = ProcessCommand::new("git") .current_dir(cwd) .args(["rev-parse", "--path-format=absolute", "--git-path", "hooks"]) .output() - .map_err(|err| format!("Failed to resolve git hooks directory: {}", err))?; + .map_err(|source| HookError::ResolveGitHooksDirectory { source })?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); return Err(if stderr.is_empty() { - "Local hook installation requires running inside a git repository".to_string() + HookError::MissingLocalRepository } else { - format!("Failed to resolve git hooks directory: {}", stderr) + HookError::ResolveGitHooksDirectoryFromGit { stderr } }); } let hooks_path = String::from_utf8_lossy(&output.stdout).trim().to_string(); if hooks_path.is_empty() { - return Err("Failed to resolve git hooks directory".to_string()); + return Err(HookError::EmptyGitHooksDirectory); } Ok(PathBuf::from(hooks_path)) } -fn read_global_hooks_path() -> Result, String> { +fn read_global_hooks_path() -> Result, HookError> { let output = ProcessCommand::new("git") .args(["config", "--global", "--path", "--get", "core.hooksPath"]) .output() - .map_err(|err| format!("Failed to read git config core.hooksPath: {}", err))?; + .map_err(|source| HookError::ReadGlobalHooksPath { source })?; if output.status.success() { let value = String::from_utf8_lossy(&output.stdout).trim().to_string(); @@ -318,10 +321,7 @@ fn read_global_hooks_path() -> Result, String> { } let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - Err(format!( - "Failed to read git config core.hooksPath: {}", - stderr - )) + Err(HookError::ReadGlobalHooksPathFromGit { stderr }) } fn managed_global_hooks_dir( @@ -329,7 +329,7 @@ fn managed_global_hooks_dir( home: Option, appdata: Option, userprofile: Option, -) -> Result { +) -> Result { if let Some(xdg) = xdg_config_home { return Ok(PathBuf::from(xdg).join("key-watch").join("hooks")); } @@ -348,33 +348,32 @@ fn managed_global_hooks_dir( .join("key-watch") .join("hooks")); } - Err("Could not determine a directory for global git hooks".to_string()) + Err(HookError::MissingGlobalHooksBaseDir) } -fn configure_global_hooks_path(hooks_dir: &Path) -> Result<(), String> { +fn configure_global_hooks_path(hooks_dir: &Path) -> Result<(), HookError> { let output = ProcessCommand::new("git") .args(["config", "--global", "core.hooksPath"]) .arg(hooks_dir) .output() - .map_err(|err| format!("Failed to configure git global core.hooksPath: {}", err))?; + .map_err(|source| HookError::ConfigureGlobalHooksPath { source })?; if output.status.success() { Ok(()) } else { let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - Err(format!( - "git config --global core.hooksPath {} failed: {}", - hooks_dir.display(), - stderr - )) + Err(HookError::ConfigureGlobalHooksPathWithGit { + hooks_dir: hooks_dir.to_path_buf(), + stderr, + }) } } -fn ensure_global_hook_target_is_safe(hook_path: &Path) -> Result<(), String> { +fn ensure_global_hook_target_is_safe(hook_path: &Path) -> Result<(), HookError> { ensure_hook_target_is_keywatch_managed(hook_path, true, "overwrite") } -fn ensure_local_hook_target_is_safe_to_create(hook_path: &Path) -> Result<(), String> { +fn ensure_local_hook_target_is_safe_to_create(hook_path: &Path) -> Result<(), HookError> { resolve_local_hooks_dir()?; if hook_path.exists() { ensure_hook_target_is_keywatch_managed(hook_path, false, "overwrite")?; @@ -386,35 +385,34 @@ fn ensure_local_hook_target_is_safe_to_create(hook_path: &Path) -> Result<(), St fn ensure_hook_target_is_keywatch_managed( hook_path: &Path, is_global: bool, - action: &str, -) -> Result<(), String> { + action: &'static str, +) -> Result<(), HookError> { if !hook_path.exists() { return Ok(()); } - let content = fs::read_to_string(hook_path).map_err(|err| { - format!( - "Failed to inspect existing hook '{}': {}", - hook_path.display(), - err - ) - })?; + let content = + fs::read_to_string(hook_path).map_err(|source| HookError::InspectExistingHook { + path: hook_path.to_path_buf(), + source, + })?; if content.contains(KEYWATCH_MARKER) { return Ok(()); } let scope = if is_global { "global" } else { "local" }; - Err(format!( - "Refusing to {action} existing {scope} hook at '{}'. Merge it manually or remove it yourself.", - hook_path.display() - )) + Err(HookError::RefuseExistingHook { + action, + scope, + path: hook_path.to_path_buf(), + }) } #[cfg(test)] mod tests { use super::{ - ensure_global_hook_target_is_safe, ensure_local_hook_target_is_safe_to_create, + HookError, ensure_global_hook_target_is_safe, ensure_local_hook_target_is_safe_to_create, managed_global_hooks_dir, resolve_hook_uninstall_target, resolve_local_hooks_dir_from, }; use std::env; @@ -488,7 +486,11 @@ mod tests { let error = ensure_global_hook_target_is_safe(&hook_path) .expect_err("foreign hook should be rejected"); - assert!(error.contains("Refusing to overwrite existing global hook")); + assert!( + error + .to_string() + .contains("Refusing to overwrite existing global hook") + ); fs::remove_file(&hook_path).expect("remove hook file"); fs::remove_dir_all(&temp_dir).expect("remove temp dir"); @@ -534,7 +536,11 @@ mod tests { let error = ensure_local_hook_target_is_safe_to_create(&hook_path) .expect_err("foreign local hook should be rejected"); - assert!(error.contains("Refusing to overwrite existing local hook")); + assert!( + error + .to_string() + .contains("Refusing to overwrite existing local hook") + ); fs::remove_file(&hook_path).expect("remove hook file"); fs::remove_dir_all(&temp_dir).expect("remove temp dir"); @@ -579,6 +585,10 @@ mod tests { let error = managed_global_hooks_dir(None, None, None, None) .expect_err("missing env inputs should fail"); - assert!(error.contains("Could not determine a directory for global git hooks")); + assert!(matches!(error, HookError::MissingGlobalHooksBaseDir)); + assert_eq!( + error.to_string(), + "Could not determine a directory for global git hooks" + ); } } diff --git a/src/hooks/error.rs b/src/hooks/error.rs new file mode 100644 index 0000000..0eb61fb --- /dev/null +++ b/src/hooks/error.rs @@ -0,0 +1,168 @@ +use std::error::Error; +use std::fmt::{Display, Formatter}; +use std::path::PathBuf; + +#[derive(Debug)] +#[non_exhaustive] +pub enum HookError { + CreateHookDirectory { + path: PathBuf, + source: std::io::Error, + }, + InstallHook { + path: PathBuf, + source: std::io::Error, + }, + MakeHookExecutable { + path: PathBuf, + source: std::io::Error, + }, + CreateGlobalHooksDirectory { + path: PathBuf, + source: std::io::Error, + }, + ResolveGitHooksDirectory { + source: std::io::Error, + }, + MissingLocalRepository, + ResolveGitHooksDirectoryFromGit { + stderr: String, + }, + EmptyGitHooksDirectory, + ReadGlobalHooksPath { + source: std::io::Error, + }, + ReadGlobalHooksPathFromGit { + stderr: String, + }, + MissingGlobalHooksBaseDir, + ConfigureGlobalHooksPath { + source: std::io::Error, + }, + ConfigureGlobalHooksPathWithGit { + hooks_dir: PathBuf, + stderr: String, + }, + InspectExistingHook { + path: PathBuf, + source: std::io::Error, + }, + RefuseExistingHook { + action: &'static str, + scope: &'static str, + path: PathBuf, + }, + RemoveHook { + path: PathBuf, + source: std::io::Error, + }, +} + +impl Display for HookError { + fn fmt(&self, formatter: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::CreateHookDirectory { path, source } => write!( + formatter, + "Failed to create hook directory '{}': {}", + path.display(), + source + ), + Self::InstallHook { path, source } => write!( + formatter, + "Failed to install hook '{}': {}", + path.display(), + source + ), + Self::MakeHookExecutable { path, source } => write!( + formatter, + "Failed to make hook executable '{}': {}", + path.display(), + source + ), + Self::CreateGlobalHooksDirectory { path, source } => write!( + formatter, + "Failed to create global hooks directory '{}': {}", + path.display(), + source + ), + Self::ResolveGitHooksDirectory { source } => { + write!(formatter, "Failed to resolve git hooks directory: {source}") + } + Self::MissingLocalRepository => formatter + .write_str("Local hook installation requires running inside a git repository"), + Self::ResolveGitHooksDirectoryFromGit { stderr } => { + write!(formatter, "Failed to resolve git hooks directory: {stderr}") + } + Self::EmptyGitHooksDirectory => { + formatter.write_str("Failed to resolve git hooks directory") + } + Self::ReadGlobalHooksPath { source } => { + write!( + formatter, + "Failed to read git config core.hooksPath: {source}" + ) + } + Self::ReadGlobalHooksPathFromGit { stderr } => write!( + formatter, + "Failed to read git config core.hooksPath: {stderr}" + ), + Self::MissingGlobalHooksBaseDir => { + formatter.write_str("Could not determine a directory for global git hooks") + } + Self::ConfigureGlobalHooksPath { source } => write!( + formatter, + "Failed to configure git global core.hooksPath: {source}" + ), + Self::ConfigureGlobalHooksPathWithGit { hooks_dir, stderr } => write!( + formatter, + "git config --global core.hooksPath {} failed: {}", + hooks_dir.display(), + stderr + ), + Self::InspectExistingHook { path, source } => write!( + formatter, + "Failed to inspect existing hook '{}': {}", + path.display(), + source + ), + Self::RefuseExistingHook { + action, + scope, + path, + } => write!( + formatter, + "Refusing to {action} existing {scope} hook at '{}'. Merge it manually or remove it yourself.", + path.display() + ), + Self::RemoveHook { path, source } => write!( + formatter, + "Failed to remove hook '{}': {}", + path.display(), + source + ), + } + } +} + +impl Error for HookError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::CreateHookDirectory { source, .. } + | Self::InstallHook { source, .. } + | Self::MakeHookExecutable { source, .. } + | Self::CreateGlobalHooksDirectory { source, .. } + | Self::ResolveGitHooksDirectory { source } + | Self::ReadGlobalHooksPath { source } + | Self::ConfigureGlobalHooksPath { source } + | Self::InspectExistingHook { source, .. } + | Self::RemoveHook { source, .. } => Some(source), + Self::MissingLocalRepository + | Self::ResolveGitHooksDirectoryFromGit { .. } + | Self::EmptyGitHooksDirectory + | Self::ReadGlobalHooksPathFromGit { .. } + | Self::MissingGlobalHooksBaseDir + | Self::ConfigureGlobalHooksPathWithGit { .. } + | Self::RefuseExistingHook { .. } => None, + } + } +} From fe3b9022de1c145dcf523254f8f819610207dd87 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:17:16 +0530 Subject: [PATCH 22/26] style: clarify report identifiers --- src/report.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/report.rs b/src/report.rs index e80ab27..c912167 100644 --- a/src/report.rs +++ b/src/report.rs @@ -33,9 +33,9 @@ pub struct ParseSeverityError { } impl fmt::Display for ParseSeverityError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { write!( - f, + formatter, "invalid severity '{}': expected one of CRITICAL, HIGH, MEDIUM, LOW", self.input ) @@ -65,17 +65,19 @@ impl FromStr for Severity { /// by the `#[serde(rename_all = "UPPERCASE")]` derive above and is not /// affected by this impl. impl<'de> Deserialize<'de> for Severity { - fn deserialize>(deserializer: D) -> Result { + fn deserialize>( + deserializer: DeserializerType, + ) -> Result { struct SeverityVisitor; impl Visitor<'_> for SeverityVisitor { type Value = Severity; - fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("one of CRITICAL, HIGH, MEDIUM, LOW (case-insensitive)") + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("one of CRITICAL, HIGH, MEDIUM, LOW (case-insensitive)") } - fn visit_str(self, value: &str) -> Result { + fn visit_str(self, value: &str) -> Result { Severity::from_str(value).map_err(de::Error::custom) } } @@ -85,8 +87,8 @@ impl<'de> Deserialize<'de> for Severity { } impl fmt::Display for Severity { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(self.as_str()) } } From 558006f0fca2848e68420125ac4000748d3f651f Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:17:36 +0530 Subject: [PATCH 23/26] refactor: compose typed CLI runtime errors --- src/lib.rs | 33 +++++++----- src/run_error.rs | 98 +++++++++++++++++++++++++++++++++++ tests/cli_validation_tests.rs | 34 ++++++++++++ tests/run_cli_error_tests.rs | 31 +++++++++++ 4 files changed, 184 insertions(+), 12 deletions(-) create mode 100644 src/run_error.rs create mode 100644 tests/cli_validation_tests.rs create mode 100644 tests/run_cli_error_tests.rs diff --git a/src/lib.rs b/src/lib.rs index 181a576..e8644c9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,10 +4,12 @@ pub mod config; pub mod detector; pub mod hooks; pub mod report; +mod run_error; pub mod scanner; pub mod utils; pub use hooks::{generate_pre_commit_hook, generate_pre_push_hook}; +pub use run_error::RunCliError; use clap::Parser; use cli::{CliOptions, Command, ExitMode, HookAction, OutputFormat, ScanArgs, Shell}; @@ -19,15 +21,19 @@ use crate::report::Severity; pub const EXIT_CODE_RUNTIME_ERROR: i32 = 2; -pub fn run_cli() -> Result<(), String> { +pub fn run_cli() -> Result<(), RunCliError> { let options = CliOptions::parse(); options.validate()?; match options.command { Command::Scan(args) => run_scan_command(&args), Command::Hook(args) => match args.action { - HookAction::Install(install_args) => hooks::install_hook(&install_args), - HookAction::Uninstall(uninstall_args) => hooks::uninstall_hook(&uninstall_args), + HookAction::Install(install_args) => { + hooks::install_hook(&install_args).map_err(Into::into) + } + HookAction::Uninstall(uninstall_args) => { + hooks::uninstall_hook(&uninstall_args).map_err(Into::into) + } }, Command::Init { shell } => { print_shell_init(&shell); @@ -37,7 +43,7 @@ pub fn run_cli() -> Result<(), String> { } } -fn run_scan_command(args: &ScanArgs) -> Result<(), String> { +fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { let start = Instant::now(); let config = if args.config.is_some() || !args.no_config_discovery { @@ -56,7 +62,7 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), String> { let baseline_path = args .baseline .as_ref() - .ok_or("--update-baseline requires --baseline ")?; + .ok_or(RunCliError::MissingBaselineForUpdate)?; let mut baseline = baseline::Baseline::load(std::path::Path::new(baseline_path))?; baseline.update_with_findings(&findings); baseline.save(std::path::Path::new(baseline_path))?; @@ -77,7 +83,7 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), String> { OutputFormat::Json => report::create_report(findings, scan_metadata, scan_time), OutputFormat::Sarif => report::create_sarif_report(findings, scan_metadata, scan_time), } - .map_err(|err| format!("Failed to serialize report: {}", err))?; + .map_err(|source| RunCliError::ReportSerialize { source })?; if args.verbose { println!("{report_out}"); @@ -95,8 +101,12 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), String> { } if let Some(ref output_path) = args.output { - utils::write_to_file(output_path, &report_out) - .map_err(|err| format!("Failed to write report to '{}': {}", output_path, err))?; + utils::write_to_file(output_path, &report_out).map_err(|source| { + RunCliError::ReportWrite { + path: output_path.clone(), + source, + } + })?; } std::process::exit(exit_code); @@ -113,12 +123,11 @@ fn print_shell_init(shell: &Shell) { print!("{script}"); } -fn verify_binary_integrity() -> Result<(), String> { - let exe_path = - env::current_exe().map_err(|err| format!("Failed to get executable path: {}", err))?; +fn verify_binary_integrity() -> Result<(), RunCliError> { + let exe_path = env::current_exe().map_err(|source| RunCliError::ExecutablePath { source })?; let metadata = exe_path .metadata() - .map_err(|err| format!("Failed to get executable metadata: {}", err))?; + .map_err(|source| RunCliError::ExecutableMetadata { source })?; #[cfg(unix)] { diff --git a/src/run_error.rs b/src/run_error.rs new file mode 100644 index 0000000..f77fce3 --- /dev/null +++ b/src/run_error.rs @@ -0,0 +1,98 @@ +use std::error::Error as StdError; +use std::fmt::{self, Display, Formatter}; +use std::io; + +use crate::baseline::BaselineError; +use crate::cli::CliValidationError; +use crate::config::ConfigError; +use crate::hooks::HookError; +use crate::scanner::ScannerError; + +#[derive(Debug)] +#[non_exhaustive] +pub enum RunCliError { + Cli { source: CliValidationError }, + Config { source: ConfigError }, + Scanner { source: ScannerError }, + Baseline { source: BaselineError }, + Hooks { source: HookError }, + MissingBaselineForUpdate, + ReportSerialize { source: serde_json::Error }, + ReportWrite { path: String, source: io::Error }, + ExecutablePath { source: io::Error }, + ExecutableMetadata { source: io::Error }, +} + +impl Display for RunCliError { + fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result { + match self { + Self::Cli { source } => write!(formatter, "{source}"), + Self::Config { source } => write!(formatter, "{source}"), + Self::Scanner { source } => write!(formatter, "{source}"), + Self::Baseline { source } => write!(formatter, "{source}"), + Self::Hooks { source } => write!(formatter, "{source}"), + Self::MissingBaselineForUpdate => { + formatter.write_str("--update-baseline requires --baseline ") + } + Self::ReportSerialize { source } => { + write!(formatter, "Failed to serialize report: {source}") + } + Self::ReportWrite { path, source } => { + write!(formatter, "Failed to write report to '{path}': {source}") + } + Self::ExecutablePath { source } => { + write!(formatter, "Failed to get executable path: {source}") + } + Self::ExecutableMetadata { source } => { + write!(formatter, "Failed to get executable metadata: {source}") + } + } + } +} + +impl StdError for RunCliError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + match self { + Self::Cli { source } => Some(source), + Self::Config { source } => Some(source), + Self::Scanner { source } => Some(source), + Self::Baseline { source } => Some(source), + Self::Hooks { source } => Some(source), + Self::MissingBaselineForUpdate => None, + Self::ReportSerialize { source } => Some(source), + Self::ReportWrite { source, .. } => Some(source), + Self::ExecutablePath { source } => Some(source), + Self::ExecutableMetadata { source } => Some(source), + } + } +} + +impl From for RunCliError { + fn from(source: CliValidationError) -> Self { + Self::Cli { source } + } +} + +impl From for RunCliError { + fn from(source: ConfigError) -> Self { + Self::Config { source } + } +} + +impl From for RunCliError { + fn from(source: ScannerError) -> Self { + Self::Scanner { source } + } +} + +impl From for RunCliError { + fn from(source: BaselineError) -> Self { + Self::Baseline { source } + } +} + +impl From for RunCliError { + fn from(source: HookError) -> Self { + Self::Hooks { source } + } +} diff --git a/tests/cli_validation_tests.rs b/tests/cli_validation_tests.rs new file mode 100644 index 0000000..cde1fea --- /dev/null +++ b/tests/cli_validation_tests.rs @@ -0,0 +1,34 @@ +use key_watch::RunCliError; +use key_watch::cli::{CliValidationError, ExitMode, OutputFormat, ScanArgs}; + +#[test] +fn test_stdin_with_path_validation_returns_typed_error() { + let options = ScanArgs { + paths: vec!["secret.txt".to_string()], + stdin: true, + git_history: false, + output: None, + verbose: false, + exclude: None, + exit_mode: ExitMode::Strict, + baseline: None, + update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, + }; + + let error = options + .validate() + .expect_err("stdin with paths should be rejected"); + + assert_eq!(error, CliValidationError::StdinWithPaths); + assert_eq!(error.to_string(), "Cannot specify both --stdin and paths"); +} + +#[test] +fn test_run_cli_error_wraps_cli_validation_display() { + let error = RunCliError::from(CliValidationError::StdinWithPaths); + + assert_eq!(error.to_string(), "Cannot specify both --stdin and paths"); +} diff --git a/tests/run_cli_error_tests.rs b/tests/run_cli_error_tests.rs new file mode 100644 index 0000000..66a9f14 --- /dev/null +++ b/tests/run_cli_error_tests.rs @@ -0,0 +1,31 @@ +use std::fs; +use std::process::Command; + +#[test] +fn test_scan_stdin_with_path_exits_two_with_validation_error() { + let temp_dir = tempfile::tempdir().expect("Create test dir"); + fs::write( + temp_dir.path().join("secret.txt"), + "AWS_KEY=AKIAIOSFODNN7EXAMPLE", + ) + .expect("Write test file"); + + let output = Command::new(env!("CARGO_BIN_EXE_key-watch")) + .current_dir(temp_dir.path()) + .arg("scan") + .arg("secret.txt") + .arg("--stdin") + .output() + .expect("Run key-watch"); + + assert_eq!( + output.status.code(), + Some(2), + "Should exit 2 for invalid stdin/path usage" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!( + stderr.trim(), + "Error: Cannot specify both --stdin and paths" + ); +} From 3561e185f678cd377136ce811849886f328fb774 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:17:55 +0530 Subject: [PATCH 24/26] docs: record typed error API break --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6cf133..8525628 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Breaking Changes + +- Public Rust APIs now return module-local typed errors instead of `String` or boxed errors. This affects CLI validation, baseline, configuration, detector initialization, scanner, hook, and `run_cli()` return types and requires a major-version release. + ### Added - **CRITICAL severity support** — findings can now be scored as Critical, High, Medium, or Low From 69f9135d38e117fb091f110505f1b47b0d411301 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:34:25 +0530 Subject: [PATCH 25/26] docs: replace architecture svg with mermaid diagrams --- README.md | 120 +++++++++++++++++++------- docs/architecture.svg | 192 ------------------------------------------ 2 files changed, 91 insertions(+), 221 deletions(-) delete mode 100644 docs/architecture.svg diff --git a/README.md b/README.md index 1b54dfa..521d77e 100644 --- a/README.md +++ b/README.md @@ -196,7 +196,58 @@ password = 'known-test-password' # keywatch:ignore ## Architecture -KeyWatch architecture +### System Overview + +```mermaid +flowchart TD + CLI["key-watch CLI"] + Scan["scan command"] + Hooks["hook install / uninstall"] + Setup["init / verify-integrity"] + + Sources["Scan sources
files, directories, stdin, or git history"] + BuiltIns["detectors.toml
built-in rules"] + UserConfig[".keywatch.toml or --config
custom rules, overrides, excludes"] + Detectors["Merged detector set"] + Pipeline["Detection pipeline"] + Findings["Findings + ScanMetadata"] + BaselineAction{"Baseline action"} + BaselineFilter["Filter known findings
--baseline"] + BaselineUpdate["Write updated baseline
--update-baseline"] + BaselineFile["Baseline JSON + exit"] + Report["JSON or SARIF 2.1.0 report"] + Destination["stdout or --output
summary + exit code"] + + HookTargets["Git hook targets
local or global"] + PreCommit["pre-commit
scan staged files"] + PrePush["pre-push
check policy, then scan repository"] + + CLI --> Scan + CLI --> Hooks + CLI --> Setup + + Hooks --> HookTargets + HookTargets --> PreCommit + HookTargets --> PrePush + PreCommit --> Scan + PrePush --> Scan + + Scan --> Sources + Scan --> BuiltIns + Scan --> UserConfig + BuiltIns --> Detectors + UserConfig --> Detectors + Sources --> Pipeline + Detectors --> Pipeline + Pipeline --> Findings + Findings --> BaselineAction + BaselineAction -->|none| Report + BaselineAction -->|filter| BaselineFilter + BaselineAction -->|update| BaselineUpdate + BaselineFilter --> Report + BaselineUpdate --> BaselineFile + Report --> Destination +``` ### Architecture Overview @@ -208,34 +259,45 @@ KeyWatch is organized into five layers. Data flows top to bottom: input sources 4. **Post-processing** — an optional baseline filter suppresses findings already recorded in the baseline file, keyed by a salted SHA-256 fingerprint of the matched content. 5. **Output** — findings serialize as JSON or SARIF 2.1.0 and are written to stdout or an output file, followed by a severity summary and an exit code derived from the exit mode. -### Data Flow (One Scan) - -``` -CLI args (paths, --stdin, --git-history, --exclude, --format, --baseline, --config) - │ - ▼ -load config (optional .keywatch.toml) ───► merge custom rules into detectors - │ merge exclude patterns - ▼ -collect files ───► exclude filter ───► parallel scan (rayon) - │ - ├─ keyword pre-filter (skip if no match) - ├─ regex match (line + multiline) - ├─ entropy threshold check - ├─ allowlist check - └─ inline suppression check - │ - ▼ - Vec + ScanMetadata - │ - ▼ - baseline filter (optional, --baseline) - │ - ▼ - report serialization (JSON or SARIF 2.1.0) - │ - ▼ - stdout / file / exit code +### Detection Pipeline + +```mermaid +flowchart TD + Start["scan command"] + Config["Load optional configuration"] + Detectors["Initialize built-in and custom detectors"] + Mode{"Input mode"} + + Paths["Files or directories"] + Stdin["stdin stream"] + History["git log patch stream"] + + Collect["Collect targets
recursive walk, skip symlinks and .git"] + Dedupe["Sort and deduplicate targets"] + Exclude["Apply CLI and config exclude globs"] + Read["Read text files
skip binary and non-UTF-8 content"] + Parallel["Scan files in parallel with rayon"] + Stream["Scan stream in overlapping chunks"] + + Keyword["Keyword pre-filter"] + Regex["Line and multiline regex matching"] + Entropy["Entropy threshold"] + Suppress["Detector allowlist + inline suppression"] + Emit["Emit Finding"] + Result["Return findings + metadata"] + + Start --> Config --> Detectors --> Mode + Mode -->|paths| Paths + Mode -->|stdin| Stdin + Mode -->|git history| History + + Paths --> Collect --> Dedupe --> Exclude --> Read --> Parallel + Stdin --> Stream + History --> Stream + + Parallel --> Keyword + Stream --> Keyword + Keyword --> Regex --> Entropy --> Suppress --> Emit --> Result ``` ### Core Data Types diff --git a/docs/architecture.svg b/docs/architecture.svg deleted file mode 100644 index 92717d4..0000000 --- a/docs/architecture.svg +++ /dev/null @@ -1,192 +0,0 @@ - - - - - - - - - - - - - - - - INPUT - scan arguments from the CLI - - - Files / dirs - --paths ... - - - stdin - --stdin - - - git history - --git-history - - flags: --exclude, --baseline, --update-baseline, --format, --config, --exit-mode, --verbose - - - - CONFIGURATION - merged, never replaced - - - detectors.toml - built-in rules, 827 lines - - - .keywatch.toml - rules, overrides, excludes - - loaded at start; custom rules appended to detectors, overrides applied by name, - exclude patterns merged with CLI --exclude - - - - DETECTION PIPELINE - one pass over each file, parallelized with rayon across files - - - - - 1 - Collect files - recursive dir walk - skip .git, binary files - dedupe overlapping roots - - - - - 2 - Exclude filter - glob patterns from - CLI --exclude + config - files never read - - - - - 3 - Keyword pre-filter - fast path: skip if file - has no detector keyword - avoids regex cost - - - - - 4 - Regex match - line detectors (single-line) - multiline detectors ((?s)) - chunked, 50-line overlap - - - - - 5 - Entropy gate - Shannon entropy of match - must pass detector threshold - filters readable words - - - - - 6 - Allowlist + suppress - detector allowlist regexes - inline "keywatch:ignore" - emit Finding - - - - - - - - - - - POST-PROCESSING - - - Baseline filter (--baseline) - SHA-256 fingerprint match drops known findings - --update-baseline records new ones - - - - OUTPUT - - - JSON - --format json - - - SARIF 2.1.0 - --format sarif - - - Summary - severity counts - - written to stdout or --output file; exit code from --exit-mode (strict / critical / always) - - - - - paths / stdin / git log - - - - detectors + overrides - - exclude patterns - - - Vec<Finding> + ScanMetadata - - - filtered findings - - - - DATA MODEL - - - Detector - name, regex, keywords, allowlist - - - Finding - file, line, type, severity - - - Severity - critical / high / medium / low - - - KeywatchConfig - rules, overrides, exclude - - - Baseline - fingerprint entries, version - - - ScanMetadata - files scanned, lines, excluded - - - - - - - From 09fa0843afc2efba3e33fb75d9df8359a0c776c6 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:34:40 +0530 Subject: [PATCH 26/26] docs: update architecture changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8525628..24179cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,7 +61,7 @@ All notable changes to this project will be documented in this file. ### Documentation -- README architecture section rewritten with an SVG architecture diagram (`docs/architecture.svg`), layer-by-layer overview, data flow trace, and core data type reference +- README architecture documentation now uses two inline GitHub-compatible Mermaid diagrams for the system overview and detection pipeline, alongside the layer-by-layer overview and core data type reference ## [1.1.0] - 2026-05-05