From c0e553358185d0213e2472da527d796aa669f06b Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:52:56 +0530 Subject: [PATCH 1/2] fix(report): stop reports from leaking secrets and exclusion noise A report is routinely written to a file or uploaded as a CI artifact, and it carried every matched credential in plaintext while SARIF output had always omitted them. Matched text is now redacted to its first four characters and a length, which is enough to identify a finding without reproducing it; --show-secrets opts back in. --output files are created 0600 rather than inheriting umask. excluded_files listed every excluded path, so excluding target/** in a Rust repository produced 46,310 entries and a 4.6 MB report describing 61 scanned files. It is now a count plus a bounded sample: the same scan emits 120 KB. --- CHANGELOG.md | 6 ++++ README.md | 3 +- src/cli.rs | 4 +++ src/lib.rs | 1 + src/report.rs | 54 +++++++++++++++++++++++++++++++++-- src/utils.rs | 13 ++++++++- tests/cli_validation_tests.rs | 4 +++ tests/exit_tests.rs | 30 +++++++++++++++++-- tests/report_tests.rs | 15 +++++++--- tests/scanner_tests.rs | 28 ++++++++++++++++++ 10 files changed, 148 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f76dab7..43ef23f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,8 +16,14 @@ All notable changes to this project will be documented in this file. - Config discovery searches parent directories up to the repository root - Hook messages abbreviate the home directory as `~` +### Changed + +- Reports redact matched text by default; `--show-secrets` opts into raw values +- Reports summarise exclusions as a count plus a sample instead of listing every path + ### Fixed +- `--output` files are created readable only by their owner - Config is not trusted from a world-writable directory, so a `.keywatch.toml` dropped in `/tmp` cannot weaken scans beneath it - `KEYWATCH_CONFIG_PATH` is ignored in trusted mode when it points inside the tree being scanned - Baseline suppression reports how many findings it hid, instead of applying silently diff --git a/README.md b/README.md index 2a2f78e..e21ad72 100644 --- a/README.md +++ b/README.md @@ -187,7 +187,8 @@ key-watch verify-integrity - `scan --git-history` - Scan git history (`git log -p`) for committed secrets - `scan --staged [path...]` - Scan only the added lines of the staged diff (run from inside the repository; paths narrow the diff as git pathspecs); findings keep real file paths and line numbers, so `--baseline` and `--exclude` compose. Files git renders as binary (e.g. `-diff` in `.gitattributes`) are listed in `excluded_files` rather than scanned. Note: a multi-line secret added across separate commits can span hunks the diff scan never sees together — the pre-push whole-tree scan remains the backstop for that case - `scan --output ` - Save report to file -- `scan --verbose` - Print full JSON output +- `scan --verbose` - Print full JSON output (matched text is redacted; see `--show-secrets`) +- `scan --show-secrets` - Include raw matched text in reports. Off by default: reports are routinely written to files or uploaded as CI artifacts, and `--output` files are created with owner-only permissions - `scan --exclude ` - Comma-separated glob patterns to exclude - `scan --exit-mode ` - Exit behavior: `always` (always pass), `critical` (fail on HIGH/CRITICAL only), `strict` (fail on any finding, default) - `scan --baseline ` - Suppress known findings from a previous scan. Without this flag, a `.keywatch-baseline.json` is discovered automatically by walking up from the scan target (bounded at the repository root or home directory), so hook scans pick up a committed repo baseline with no configuration diff --git a/src/cli.rs b/src/cli.rs index f729e1f..16c5759 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -107,6 +107,10 @@ pub struct ScanArgs { #[arg(short, long, default_value_t = false)] pub verbose: bool, + /// Include raw matched text in reports (default: redacted) + #[arg(long, default_value_t = false)] + pub show_secrets: bool, + /// Paths to exclude from scanning (comma-separated, supports glob patterns) #[arg(long)] pub exclude: Option, diff --git a/src/lib.rs b/src/lib.rs index b2a6cf2..ea8afc0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -61,6 +61,7 @@ fn emit(line: &str) -> Result<(), RunCliError> { fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { let start = Instant::now(); + report::set_show_secrets(args.show_secrets); // Resolve the baseline like config: an explicit --baseline wins, otherwise // discover .keywatch-baseline.json in the scanned tree. --update-baseline diff --git a/src/report.rs b/src/report.rs index 3faca20..ebf02b7 100644 --- a/src/report.rs +++ b/src/report.rs @@ -1,5 +1,33 @@ use serde::de::{self, Visitor}; use serde::{Deserialize, Deserializer, Serialize}; +use std::sync::atomic::{AtomicBool, Ordering}; + +/// Whether reports may contain raw matched text. Off unless the operator asks +/// for it: a report is routinely written to a file or uploaded as a CI +/// artifact, which would turn the scanner into an exfiltration channel. +static SHOW_SECRETS: AtomicBool = AtomicBool::new(false); + +pub fn set_show_secrets(show: bool) { + SHOW_SECRETS.store(show, Ordering::Relaxed); +} + +/// Keeps enough to identify a finding without reproducing the credential: +/// the first four characters and the length. +pub fn redact(matched: &str) -> String { + let visible: String = matched.chars().take(4).collect(); + format!("{visible}... ({} chars, redacted)", matched.chars().count()) +} + +fn serialize_matched_content(matched: &str, serializer: S) -> Result +where + S: serde::Serializer, +{ + if SHOW_SECRETS.load(Ordering::Relaxed) { + serializer.serialize_str(matched) + } else { + serializer.serialize_str(&redact(matched)) + } +} use std::{fmt, str::FromStr}; mod sarif; @@ -105,6 +133,7 @@ pub struct Finding { pub line_number: usize, pub finding_type: String, pub severity: Severity, + #[serde(serialize_with = "serialize_matched_content")] pub matched_content: String, pub plugin_name: String, } @@ -117,13 +146,34 @@ pub struct ScanMetadata { pub suppressed_by_baseline: usize, } +/// How many paths an exclusion removed, and a bounded sample of them. +/// +/// The full list is not reported: excluding `target/**` in a Rust repository +/// produced 46,310 entries and a 4.6 MB report for 61 scanned files. +#[derive(Serialize, Clone, Default)] +pub struct ExcludedSummary { + pub count: usize, + pub sample: Vec, +} + +impl ExcludedSummary { + const SAMPLE_LIMIT: usize = 20; + + pub fn from_paths(paths: &[String]) -> Self { + Self { + count: paths.len(), + sample: paths.iter().take(Self::SAMPLE_LIMIT).cloned().collect(), + } + } +} + #[derive(Serialize)] pub struct Report { pub status: ScanStatus, pub findings: Vec, pub files_scanned: usize, pub total_lines: usize, - pub excluded_files: Vec, + pub excluded: ExcludedSummary, pub suppressed_by_baseline: usize, pub scan_time: String, } @@ -143,7 +193,7 @@ pub fn create_report( findings, files_scanned: metadata.files_scanned, total_lines: metadata.total_lines, - excluded_files: metadata.excluded_files, + excluded: ExcludedSummary::from_paths(&metadata.excluded_files), suppressed_by_baseline: metadata.suppressed_by_baseline, scan_time, }; diff --git a/src/utils.rs b/src/utils.rs index 1f69afe..1bc2eae 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -23,8 +23,19 @@ pub fn display_path(path: &Path) -> String { use std::fs::File; use std::io::{Result, Write}; +/// Writes a report file readable only by its owner. +/// +/// `File::create` uses 0666 & ~umask, i.e. world-readable by default, and a +/// report can carry matched text when `--show-secrets` is set. pub fn write_to_file(path: &str, content: &str) -> Result<()> { - let mut file = File::create(path)?; + let mut options = std::fs::OpenOptions::new(); + options.write(true).create(true).truncate(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options.open(path)?; file.write_all(content.as_bytes())?; Ok(()) } diff --git a/tests/cli_validation_tests.rs b/tests/cli_validation_tests.rs index 1c02091..e86cca9 100644 --- a/tests/cli_validation_tests.rs +++ b/tests/cli_validation_tests.rs @@ -10,6 +10,7 @@ fn test_stdin_with_path_validation_returns_typed_error() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -44,6 +45,7 @@ fn test_staged_with_stdin_validation_returns_typed_error() { staged: true, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -74,6 +76,7 @@ fn test_staged_with_git_history_validation_returns_typed_error() { staged: true, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -104,6 +107,7 @@ fn test_staged_allows_zero_or_many_paths() { staged: true, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, diff --git a/tests/exit_tests.rs b/tests/exit_tests.rs index e6469b7..bfef9a0 100644 --- a/tests/exit_tests.rs +++ b/tests/exit_tests.rs @@ -295,8 +295,12 @@ fn test_scan_verbose_output() { let stdout = String::from_utf8_lossy(&output.stdout); assert!( - stdout.contains("AKIAIOSFODNN7EXAMPLE"), - "Verbose output should contain secret info" + !stdout.contains("AKIAIOSFODNN7EXAMPLE"), + "reports must redact matched text by default, got:\n{stdout}" + ); + assert!( + stdout.contains("redacted") && stdout.contains("AWS Access Key"), + "the finding must still be identifiable, got:\n{stdout}" ); assert!( stdout.contains("\"status\": \"FAIL\""), @@ -344,3 +348,25 @@ fn test_init_command() { "Should contain kw alias" ); } + +#[test] +fn test_show_secrets_opts_into_raw_matched_content() { + let dir = setup_scan_dir("show_secrets", true); + let file = dir.join("leak.txt"); + fs::write(&file, "aws_access_key_id = AKIAIOSFODNN7EXAMPLE\n").expect("write"); + + let output = Command::new(env!("CARGO_BIN_EXE_key-watch")) + .args(["scan"]) + .arg(&file) + .args(["--verbose", "--show-secrets"]) + .output() + .expect("run key-watch"); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert!( + stdout.contains("AKIAIOSFODNN7EXAMPLE"), + "--show-secrets must include raw matched text, got:\n{stdout}" + ); + + let _ = fs::remove_dir_all(&dir); +} diff --git a/tests/report_tests.rs b/tests/report_tests.rs index aa49607..00fe00b 100644 --- a/tests/report_tests.rs +++ b/tests/report_tests.rs @@ -24,7 +24,7 @@ fn test_create_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["excluded"]["count"], 0); assert_eq!(json["scan_time"], "0.5s"); } @@ -51,7 +51,11 @@ fn test_report_with_findings() { assert_eq!(json["status"], "FAIL"); assert_eq!(json["findings"][0]["finding_type"], "AWS Key"); - assert_eq!(json["findings"][0]["matched_content"], "AKIATESTKEY"); + // Matched text is redacted unless --show-secrets is passed. + assert_eq!( + json["findings"][0]["matched_content"], + "AKIA... (11 chars, redacted)" + ); } #[test] @@ -76,11 +80,14 @@ fn test_create_report_includes_excluded_files_and_plugin_metadata() { let json = parse_json(&report); assert_eq!( - json["excluded_files"], + json["excluded"]["sample"], 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["findings"][0]["matched_content"], + "tok_... (12 chars, redacted)" + ); assert_eq!(json["total_lines"], 80); } diff --git a/tests/scanner_tests.rs b/tests/scanner_tests.rs index dcff5c4..13d69cb 100644 --- a/tests/scanner_tests.rs +++ b/tests/scanner_tests.rs @@ -114,6 +114,7 @@ sk-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWX\n\ staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -149,6 +150,7 @@ Stripe: sk_test_51ABCDEF12345678901234567890\n\ staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -185,6 +187,7 @@ AZURE_STORAGE=DefaultEndpointsProtocol=https;AccountName=examplestore; staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -222,6 +225,7 @@ b3BlbnNzaC1ldi0xLjAAABgQDQD2FGB3V2t4=\n\ staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -253,6 +257,7 @@ fn test_multiple_detections_in_line() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -296,6 +301,7 @@ fn test_directory_scan_with_exclusions() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -338,6 +344,7 @@ fn test_exclude_pattern_filtering() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: Some("*.log".to_string()), exit_mode: ExitMode::Strict, baseline: None, @@ -370,6 +377,7 @@ fn test_invalid_cli_exclude_pattern_returns_typed_error() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: Some("[".to_string()), exit_mode: ExitMode::Strict, baseline: None, @@ -420,6 +428,7 @@ fn test_dot_github_directory_is_scanned() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -450,6 +459,7 @@ fn test_scan_no_secrets() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -481,6 +491,7 @@ fn test_non_utf8_file_handling() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -516,6 +527,7 @@ fn test_multiple_files_scan() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -552,6 +564,7 @@ fn test_duplicate_paths_are_scanned_once() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -602,6 +615,7 @@ fn test_mixed_file_and_directory_paths_are_scanned_once() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -644,6 +658,7 @@ fn test_nonexistent_paths_are_ignored_without_counting_as_scanned() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -693,6 +708,7 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -740,6 +756,7 @@ fn test_recursive_symlink_path_is_skipped() -> Result<(), String> { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -780,6 +797,7 @@ fn test_detect_aadhaar() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -818,6 +836,7 @@ fn test_detect_voter_id() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -853,6 +872,7 @@ fn test_detect_pan_card() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -888,6 +908,7 @@ fn test_detect_abha() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -924,6 +945,7 @@ fn test_multiple_indian_ids() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -985,6 +1007,7 @@ fn test_overlapping_scan_roots_with_exclusions() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: Some("subdir/secret.txt".to_string()), exit_mode: ExitMode::Strict, baseline: None, @@ -1031,6 +1054,7 @@ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\n staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -1080,6 +1104,7 @@ fn test_stdin_args_validation() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -1142,6 +1167,7 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -1158,6 +1184,7 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, @@ -1177,6 +1204,7 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { staged: false, output: None, verbose: false, + show_secrets: false, exclude: None, exit_mode: ExitMode::Strict, baseline: None, From 0c5d1691e1c97a6033c82f28f3fcf66dd8f5564f Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:01:25 +0530 Subject: [PATCH 2/2] fix: drop the now-unused File import --- src/utils.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils.rs b/src/utils.rs index 1bc2eae..ad93e67 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -20,7 +20,6 @@ pub fn display_path(path: &Path) -> String { } } -use std::fs::File; use std::io::{Result, Write}; /// Writes a report file readable only by its owner.