From 159c7f2795a7301e60911924f16eb575459c8f4b Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:48:47 +0530 Subject: [PATCH 1/3] fix(trust): bound where configuration and suppression may come from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three inputs decided what gets detected without being treated as untrusted. Config discovery walked to the filesystem root when the scan target was not inside a repository, so a .keywatch.toml in a world-writable directory such as /tmp could disable detectors for every scan beneath it. Config found in a world-writable directory is now ignored. KEYWATCH_CONFIG_PATH was read before the trusted-mode check, so a repository could redirect the detector set through .envrc or a devcontainer even for --no-config-discovery. It is an operator channel, so it is still honoured — unless, in trusted mode, it points back into the tree being scanned. A discovered baseline removed findings silently. Scans now report the number suppressed and the file responsible, so a committed suppression list cannot hide what it is doing. --- CHANGELOG.md | 3 +++ src/config/mod.rs | 19 +++++++++++++++++ src/config/tests/discovery.rs | 30 ++++++++++++++++++++++++++ src/detector.rs | 18 ++++++++++++++++ src/lib.rs | 12 +++++++++++ src/report.rs | 5 ++++- src/scanner.rs | 4 ++++ tests/report_tests.rs | 5 +++++ tests/scanner_tests.rs | 40 +++++++++++++++++++++++++++++++++++ 9 files changed, 135 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63706a4..f76dab7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ All notable changes to this project will be documented in this file. ### Fixed +- Config is not trusted from a world-writable directory, so a `.keywatch.toml` dropped in `/tmp` cannot weaken scans beneath it +- `KEYWATCH_CONFIG_PATH` is ignored in trusted mode when it points inside the tree being scanned +- Baseline suppression reports how many findings it hid, instead of applying silently - `CreditCardDetector` requires an issuer prefix and a valid Luhn checksum, instead of matching any 13-16 digit run - `HighEntropyDetector` could never fire (its 4.0 threshold is the ceiling for hex) and now runs, restricted to lines naming a credential - PKCS#8 private key headers (`BEGIN PRIVATE KEY`, `BEGIN ENCRYPTED PRIVATE KEY`) are detected diff --git a/src/config/mod.rs b/src/config/mod.rs index 52ec3d2..ceb06e6 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -264,6 +264,12 @@ pub(crate) fn find_file_upwards( .find(|candidate_path| candidate_path.exists()) .and_then(|candidate_path| candidate_path.to_str().map(str::to_string)); if found.is_some() { + // A world-writable directory is not a trust boundary: on a shared + // host any user could drop a config into /tmp and weaken every + // scan beneath it. + if is_world_writable(dir) { + return None; + } return found; } if dir.join(".git").exists() || home.as_deref() == Some(dir) { @@ -272,3 +278,16 @@ pub(crate) fn find_file_upwards( } None } + +#[cfg(unix)] +fn is_world_writable(dir: &Path) -> bool { + use std::os::unix::fs::PermissionsExt; + fs::metadata(dir) + .map(|metadata| metadata.permissions().mode() & 0o002 != 0) + .unwrap_or(false) +} + +#[cfg(not(unix))] +fn is_world_writable(_dir: &Path) -> bool { + false +} diff --git a/src/config/tests/discovery.rs b/src/config/tests/discovery.rs index 87ce6a1..aa5606e 100644 --- a/src/config/tests/discovery.rs +++ b/src/config/tests/discovery.rs @@ -259,3 +259,33 @@ fn test_load_none_uses_supplied_cwd() { .collect(); assert!(names.contains(&"CwdRule".to_string())); } + +#[test] +fn test_config_in_world_writable_directory_is_ignored() { + // On a shared host any user can drop a config into a world-writable + // directory; trusting it would let them disable detectors for everyone + // scanning beneath it. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let dir = TempDir::new().unwrap(); + write_file( + &dir, + ".keywatch.toml", + &minimal_rule_toml("HostileRule", r"\\bX\\b", "LOW"), + ); + let mut perms = std::fs::metadata(dir.path()).unwrap().permissions(); + perms.set_mode(0o777); + std::fs::set_permissions(dir.path(), perms).unwrap(); + let scan_file = dir.path().join("secrets.txt"); + std::fs::write(&scan_file, "content").unwrap(); + + let paths = vec![scan_file.to_str().unwrap().to_string()]; + assert!( + KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .is_none(), + "config from a world-writable directory must not be trusted" + ); + } +} diff --git a/src/detector.rs b/src/detector.rs index 7e6a15d..3841755 100644 --- a/src/detector.rs +++ b/src/detector.rs @@ -255,11 +255,29 @@ struct DetectorConfig { validate: Option, } +/// True when `path` sits inside `dir`, i.e. the scanned repository supplied +/// it rather than the operator. +fn is_within(path: &std::path::Path, dir: &std::path::Path) -> bool { + match (fs::canonicalize(path), fs::canonicalize(dir)) { + (Ok(path), Ok(dir)) => path.starts_with(dir), + _ => false, + } +} + fn find_detectors_config(include_repository_config: bool) -> Option { std::env::var("KEYWATCH_CONFIG_PATH") .map(std::path::PathBuf::from) .ok() .filter(|path| path.exists()) + // KEYWATCH_CONFIG_PATH is an operator channel. A repository can reach + // it through .envrc/direnv or a devcontainer, so in trusted mode an + // value pointing back into the tree being scanned is ignored. + .filter(|path| { + include_repository_config + || std::env::current_dir() + .map(|cwd| !is_within(path, &cwd)) + .unwrap_or(true) + }) .or_else(|| { if !include_repository_config { return None; diff --git a/src/lib.rs b/src/lib.rs index cfa411b..8f4bc08 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -82,9 +82,12 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { }; let (mut findings, scan_metadata) = scanner::run_scan(args, config.as_ref())?; + let mut scan_metadata = scan_metadata; if let Some(ref baseline_path) = args.baseline { let baseline = baseline::Baseline::load(std::path::Path::new(baseline_path))?; + let before = findings.len(); findings = baseline.filter_findings(findings); + scan_metadata.suppressed_by_baseline = before - findings.len(); } if args.update_baseline { @@ -105,6 +108,7 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { elapsed.as_secs(), elapsed.subsec_millis() / 100 ); + let suppressed = scan_metadata.suppressed_by_baseline; let severity_counts = report::get_severity_counts(&findings); let exit_code = calculate_exit_code(&findings, &args.exit_mode); let findings_count = findings.len(); @@ -114,6 +118,14 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { } .map_err(|source| RunCliError::ReportSerialize { source })?; + if suppressed > 0 && !args.verbose { + emit(&format!( + "Suppressed {} finding(s) via {}", + suppressed, + args.baseline.as_deref().unwrap_or("baseline") + ))?; + } + let summary = match findings_count { _ if args.verbose => report_out.clone(), 0 => "No secrets found.".to_string(), diff --git a/src/report.rs b/src/report.rs index c912167..3faca20 100644 --- a/src/report.rs +++ b/src/report.rs @@ -109,11 +109,12 @@ pub struct Finding { pub plugin_name: String, } -#[derive(Serialize, Clone)] +#[derive(Serialize, Clone, Default)] pub struct ScanMetadata { pub files_scanned: usize, pub total_lines: usize, pub excluded_files: Vec, + pub suppressed_by_baseline: usize, } #[derive(Serialize)] @@ -123,6 +124,7 @@ pub struct Report { pub files_scanned: usize, pub total_lines: usize, pub excluded_files: Vec, + pub suppressed_by_baseline: usize, pub scan_time: String, } @@ -142,6 +144,7 @@ pub fn create_report( files_scanned: metadata.files_scanned, total_lines: metadata.total_lines, excluded_files: metadata.excluded_files, + suppressed_by_baseline: metadata.suppressed_by_baseline, scan_time, }; diff --git a/src/scanner.rs b/src/scanner.rs index d0e33b9..01af085 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -384,6 +384,7 @@ pub fn run_scan( files_scanned: 1, total_lines, excluded_files: Vec::new(), + suppressed_by_baseline: 0, }; return Ok((findings, metadata)); @@ -465,6 +466,7 @@ pub fn run_scan( files_scanned: 1, total_lines, excluded_files: Vec::new(), + suppressed_by_baseline: 0, }; return Ok((findings, metadata)); @@ -573,6 +575,7 @@ pub fn run_scan( files_scanned, total_lines, excluded_files, + suppressed_by_baseline: 0, }; Ok((findings, metadata)) @@ -831,6 +834,7 @@ fn scan_staged_diff( files_scanned: scanned_files.len(), total_lines, excluded_files, + suppressed_by_baseline: 0, }; Ok(StagedScan { diff --git a/tests/report_tests.rs b/tests/report_tests.rs index 605043f..aa49607 100644 --- a/tests/report_tests.rs +++ b/tests/report_tests.rs @@ -14,6 +14,7 @@ fn test_create_report() { files_scanned: 5, total_lines: 100, excluded_files: vec![], + suppressed_by_baseline: 0, }; let report = create_report(findings, metadata, "0.5s".to_string()) @@ -41,6 +42,7 @@ fn test_report_with_findings() { files_scanned: 1, total_lines: 50, excluded_files: vec![], + suppressed_by_baseline: 0, }; let report = create_report(findings, metadata, "0.1s".to_string()) @@ -66,6 +68,7 @@ fn test_create_report_includes_excluded_files_and_plugin_metadata() { files_scanned: 2, total_lines: 80, excluded_files: vec!["ignored.log".to_string(), "vendor/secrets.txt".to_string()], + suppressed_by_baseline: 0, }; let report = create_report(findings, metadata, "1.2s".to_string()) @@ -96,6 +99,7 @@ fn test_create_sarif_report_uses_camel_case_fields_and_hides_matched_content() { files_scanned: 1, total_lines: 12, excluded_files: vec![], + suppressed_by_baseline: 0, }; let sarif = create_sarif_report(findings, metadata, "2026-08-01T00:00:00Z".to_string()) @@ -188,6 +192,7 @@ fn test_create_sarif_report_maps_all_severities_to_expected_levels() { files_scanned: 4, total_lines: 4, excluded_files: vec![], + suppressed_by_baseline: 0, }; let sarif = create_sarif_report(findings, metadata, "2026-08-01T00:00:00Z".to_string()) diff --git a/tests/scanner_tests.rs b/tests/scanner_tests.rs index 9137372..dcff5c4 100644 --- a/tests/scanner_tests.rs +++ b/tests/scanner_tests.rs @@ -1842,3 +1842,43 @@ fn test_staged_scan_reads_blobs_git_renders_as_binary() -> Result<(), String> { let _ = fs::remove_dir_all(&repo_dir); Ok(()) } + +#[test] +fn test_baseline_suppression_is_reported() -> Result<(), String> { + if !git_available() { + return Ok(()); + } + + // A committed baseline is repo-controlled data that silently removes + // findings; the count must be visible so suppression cannot hide. + let repo_dir = unique_temp_dir("baseline_suppression_visible"); + let _ = fs::remove_dir_all(&repo_dir); + init_git_repo(&repo_dir)?; + fs::write( + repo_dir.join("secrets.txt"), + "aws_access_key_id = AKIAIOSFODNN7EXAMPLE\n", + ) + .map_err(|e| e.to_string())?; + + let run = |extra: &[&str]| { + Command::new(env!("CARGO_BIN_EXE_key-watch")) + .args(["scan", "."]) + .args(extra) + .env("KEYWATCH_CONFIG_PATH", detectors_config_path()) + .current_dir(&repo_dir) + .output() + .expect("run key-watch") + }; + + assert!(run(&["--update-baseline"]).status.success()); + let scan = run(&[]); + let stdout = String::from_utf8_lossy(&scan.stdout); + + assert!( + stdout.contains("Suppressed") && stdout.contains("finding(s)"), + "the suppressed count must be printed, got:\n{stdout}" + ); + + let _ = fs::remove_dir_all(&repo_dir); + Ok(()) +} From 5b213e402202d54f581d36d8fcf56e48df033d31 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:49:36 +0530 Subject: [PATCH 2/3] refactor: share the ~ path renderer between hooks and scan output --- src/hooks.rs | 28 ++++------------------------ src/lib.rs | 5 ++++- src/utils.rs | 22 ++++++++++++++++++++++ 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/hooks.rs b/src/hooks.rs index 7aa78a3..168564d 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -4,7 +4,6 @@ use std::env; use std::fs; use std::path::{Path, PathBuf}; use std::process::Command as ProcessCommand; -use std::sync::LazyLock; mod error; pub use error::HookError; @@ -20,30 +19,11 @@ fn shell_escape(input: &str) -> String { format!("'{}'", input.replace('\'', "'\"'\"'")) } -/// The user's home directory, resolved once per process. -static HOME_DIR: LazyLock> = LazyLock::new(|| { - env::var_os("HOME") - .or_else(|| env::var_os("USERPROFILE")) - .map(PathBuf::from) -}); - /// `"global"` or `"local"`, for messages describing a hook's scope. const fn scope_label(is_global: bool) -> &'static str { if is_global { "global" } else { "local" } } -/// Renders a path for terminal output, abbreviating the home directory as `~`. -fn display_path(path: &Path) -> String { - match HOME_DIR - .as_deref() - .and_then(|home| path.strip_prefix(home).ok()) - { - Some(rest) if rest.as_os_str().is_empty() => "~".to_string(), - Some(rest) => format!("~/{}", rest.display()), - None => path.display().to_string(), - } -} - fn build_repo_section(allowed: Option<&str>, blocked: Option<&str>) -> String { let escaped_allowed = allowed.map(shell_escape); let escaped_blocked = blocked.map(shell_escape); @@ -144,14 +124,14 @@ pub fn install_hook(args: &HookInstallArgs) -> Result<(), HookError> { if install_target.configured_global_path { println!( "Configured git --global core.hooksPath to {}", - display_path(&install_target.hooks_dir) + utils::display_path(&install_target.hooks_dir) ); } println!( "Installed {} {hook_type_str} hook at {}", scope_label(install_target.is_global), - display_path(&install_target.path) + utils::display_path(&install_target.path) ); println!( "The hook will run automatically during git {}.", @@ -173,7 +153,7 @@ pub fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), HookError> { if !install_target.path.exists() { println!( "No {scope} {hook_type_str} hook found at {}", - display_path(&install_target.path) + utils::display_path(&install_target.path) ); return Ok(()); } @@ -191,7 +171,7 @@ pub fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), HookError> { println!( "Removed {scope} {hook_type_str} hook at {}", - display_path(&install_target.path) + utils::display_path(&install_target.path) ); Ok(()) diff --git a/src/lib.rs b/src/lib.rs index 8f4bc08..b2a6cf2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -122,7 +122,10 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { emit(&format!( "Suppressed {} finding(s) via {}", suppressed, - args.baseline.as_deref().unwrap_or("baseline") + args.baseline + .as_deref() + .map(|path| utils::display_path(std::path::Path::new(path))) + .unwrap_or_else(|| "baseline".to_string()) ))?; } diff --git a/src/utils.rs b/src/utils.rs index bb55bc3..1f69afe 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,3 +1,25 @@ +use std::path::{Path, PathBuf}; +use std::sync::LazyLock; + +/// The user's home directory. +static HOME_DIR: LazyLock> = LazyLock::new(|| { + std::env::var_os("HOME") + .or_else(|| std::env::var_os("USERPROFILE")) + .map(PathBuf::from) +}); + +/// Renders a path for terminal output, abbreviating the home directory as `~`. +pub fn display_path(path: &Path) -> String { + match HOME_DIR + .as_deref() + .and_then(|home| path.strip_prefix(home).ok()) + { + Some(rest) if rest.as_os_str().is_empty() => "~".to_string(), + Some(rest) => format!("~/{}", rest.display()), + None => path.display().to_string(), + } +} + use std::fs::File; use std::io::{Result, Write}; From d23ab3f4ece12b68221fe34abb24db6324da9966 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:50:10 +0530 Subject: [PATCH 3/3] fix(config): drop '.' components from discovered paths --- src/config/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/config/mod.rs b/src/config/mod.rs index ceb06e6..65cc569 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -252,6 +252,12 @@ pub(crate) fn find_file_upwards( } else { cwd.join(search_dir) }; + // Drop "." components so discovered paths read as "/repo/.keywatch.toml" + // rather than "/repo/./.keywatch.toml" when reported to the user. + let search_dir: PathBuf = search_dir + .components() + .filter(|component| !matches!(component, std::path::Component::CurDir)) + .collect(); let home = std::env::var_os("HOME") .or_else(|| std::env::var_os("USERPROFILE"))