From 191c9b958c9483e50c485f39a1454d0211a4f313 Mon Sep 17 00:00:00 2001 From: PiX <69745008+pixincreate@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:00:10 +0530 Subject: [PATCH] test: cover the behaviour that was shipping unverified git_available() turned twelve tests into silent no-ops if git were missing, including every --staged and baseline integration test; git is a hard dependency of those modes, so it now fails loudly. The staged scan's git-config hardening had no test at all, despite being the fix for a bypass: deleting any of the overrides made the scan report clean or attribute findings to a mangled path, and nothing failed. One test now sets all five hostile settings at once and asserts the path and line survive. Also newly covered: the scanner exits 2 outside a repository, which is what makes the hook fail closed; path operands actually narrow the staged diff; a config at the repository root is discovered, which depends on candidate lookup preceding the .git stop check; and the prefilter selects both owners of overlapping keywords, without which Stripe detection would silently stop running behind Adyen's shorter keyword. CI never ran KeyWatch against itself, so a committed secret or a stale baseline would ship green. --- .github/workflows/ci.yml | 28 +++++++ CHANGELOG.md | 1 + src/config/tests/discovery.rs | 31 ++++++++ src/scanner.rs | 38 ++++++++++ tests/scanner_tests.rs | 133 +++++++++++++++++++++++++++++++++- 5 files changed, 230 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f92274e..8b26839 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,34 @@ env: RUST_BACKTRACE: short jobs: + self-scan: + name: Self scan + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@v7 + + - name: Install Rust + uses: dtolnay/rust-toolchain@master + with: + toolchain: stable + + - name: Build + run: cargo build --release --locked + + # KeyWatch must be clean under its own rules, and the committed + # baseline must still describe this tree. + - name: Scan this repository + run: ./target/release/key-watch scan . + + - name: Baseline is current + run: | + ./target/release/key-watch scan . --update-baseline + git diff --exit-code -- .keywatch-baseline.json + + - name: Framework integration uses the staged scan + run: grep -q "scan --staged" .pre-commit-hooks.yaml distribution: name: distribution runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 4343a36..cd9ac9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ All notable changes to this project will be documented in this file. ### Added +- CI scans this repository with KeyWatch and fails if the committed baseline has drifted - `--prune-baseline` rewrites the baseline from current findings, dropping entries for deleted files and rotated credentials ### Changed diff --git a/src/config/tests/discovery.rs b/src/config/tests/discovery.rs index aa5606e..efcd6e1 100644 --- a/src/config/tests/discovery.rs +++ b/src/config/tests/discovery.rs @@ -289,3 +289,34 @@ fn test_config_in_world_writable_directory_is_ignored() { ); } } + +#[test] +fn test_config_at_repository_root_applies_to_nested_paths() { + // The candidate check must run before the .git stop check, or a config + // sitting AT the repo root stops being discovered. Both existing walk + // tests pass either way, so this pins the ordering. + let dir = TempDir::new().unwrap(); + let repo_root = dir.path().join("repo"); + std::fs::create_dir_all(repo_root.join(".git")).unwrap(); + std::fs::write( + repo_root.join(".keywatch.toml"), + minimal_rule_toml("RootRule", r"\\bROOT\\b", "HIGH"), + ) + .unwrap(); + let nested = repo_root.join("a/b"); + std::fs::create_dir_all(&nested).unwrap(); + let scan_file = nested.join("secrets.txt"); + std::fs::write(&scan_file, "content").unwrap(); + + let paths = vec![scan_file.to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .expect("a config at the repository root must apply to nested paths"); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); + assert!(names.contains(&"RootRule".to_string())); +} diff --git a/src/scanner.rs b/src/scanner.rs index 1bfa605..fefe9b5 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -1084,6 +1084,44 @@ mod tests { ); } + #[test] + fn test_prefilter_selects_detectors_with_overlapping_keywords() { + // detectors.toml has genuinely overlapping keywords ("sk_" for Adyen, + // "sk_test_" for Stripe). A non-overlapping search reports only the + // shorter one and the longer detector silently never runs. + let short = Detector::new( + "Short", + r"sk_\w+", + "Short", + "HIGH", + &[], + &["sk_".to_string()], + None, + ) + .unwrap(); + let long = Detector::new( + "Long", + r"sk_test_\w+", + "Long", + "HIGH", + &[], + &["sk_test_".to_string()], + None, + ) + .unwrap(); + let detectors = vec![&short, &long]; + let prefilter = KeywordPrefilter::new(&detectors); + + let mut candidates = Vec::new(); + prefilter.candidates_into("sk_test_51abcdef", &mut candidates); + + assert_eq!( + candidates, + vec![true, true], + "both the shorter and longer keyword owners must be selected" + ); + } + #[test] fn test_scan_staged_diff_preserves_plus_prefixed_added_content() { let detector = make_detector("Line", r"SECRET_\w+", "Test", "HIGH"); diff --git a/tests/scanner_tests.rs b/tests/scanner_tests.rs index 6726f78..c2856de 100644 --- a/tests/scanner_tests.rs +++ b/tests/scanner_tests.rs @@ -15,8 +15,16 @@ fn unique_temp_dir(name: &str) -> PathBuf { temp_dir().join(format!("keywatch_{name}_{stamp}_{}", std::process::id())) } +/// Git backs the --staged and --git-history modes, so its absence is a +/// broken environment rather than a reason to pass silently. Twelve tests +/// used to return Ok(()) here, reporting green while covering nothing. fn git_available() -> bool { - Command::new("git").arg("--version").output().is_ok() + let output = Command::new("git") + .arg("--version") + .output() + .expect("git is required to test KeyWatch's git-backed scan modes"); + assert!(output.status.success(), "`git --version` failed"); + true } fn init_git_repo(path: &Path) -> Result<(), String> { @@ -2088,3 +2096,126 @@ fn test_prune_baseline_drops_stale_entries() -> Result<(), String> { let _ = fs::remove_dir_all(&dir); Ok(()) } + +#[test] +fn test_staged_scan_survives_hostile_git_config() -> Result<(), String> { + if !git_available() { + return Ok(()); + } + + // Every override in the staged git invocation exists because one of these + // settings breaks parsing. Without this test, deleting any of them is + // invisible: the scan reports clean or attributes findings to a mangled + // path, and no other test notices. + let repo_dir = unique_temp_dir("hostile_git_config"); + let _ = fs::remove_dir_all(&repo_dir); + init_git_repo(&repo_dir)?; + for (key, value) in [ + ("color.ui", "always"), + ("diff.mnemonicPrefix", "true"), + ("diff.noprefix", "true"), + ("core.quotePath", "true"), + ("diff.relative", "true"), + ] { + let status = Command::new("git") + .args(["config", key, value]) + .current_dir(&repo_dir) + .status() + .map_err(|e| e.to_string())?; + assert!(status.success(), "git config {key} failed"); + } + commit_file(&repo_dir, "config.txt", "one\ntwo\n", "init")?; + stage_file( + &repo_dir, + "config.txt", + "one\ntwo\naws_access_key_id = AKIAIOSFODNN7EXAMPLE\n", + )?; + + let output = Command::new(env!("CARGO_BIN_EXE_key-watch")) + .args(["scan", "--staged", "--verbose", "--no-baseline-discovery"]) + .env("KEYWATCH_CONFIG_PATH", detectors_config_path()) + .current_dir(&repo_dir) + .output() + .expect("run key-watch"); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(1), "stdout:\n{stdout}"); + assert!( + stdout.contains("\"file_path\": \"config.txt\""), + "path attribution must survive prefix settings, got:\n{stdout}" + ); + assert!( + stdout.contains("\"line_number\": 3"), + "line attribution must survive, got:\n{stdout}" + ); + + let _ = fs::remove_dir_all(&repo_dir); + Ok(()) +} + +#[test] +fn test_staged_scan_outside_a_repository_fails_closed() { + // templates/pre-commit.sh documents that any exit above 1 blocks the + // commit; nothing pinned that the scanner actually produces it. + let dir = unique_temp_dir("staged_outside_repo"); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir).expect("create dir"); + + let output = Command::new(env!("CARGO_BIN_EXE_key-watch")) + .args(["scan", "--staged", "--no-baseline-discovery"]) + .env("KEYWATCH_CONFIG_PATH", detectors_config_path()) + .env("GIT_CEILING_DIRECTORIES", &dir) + .current_dir(&dir) + .output() + .expect("run key-watch"); + + assert_eq!( + output.status.code(), + Some(2), + "a git failure must exit 2 so the hook fails closed, stderr:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn test_staged_scan_paths_narrow_the_diff() -> Result<(), String> { + if !git_available() { + return Ok(()); + } + + let repo_dir = unique_temp_dir("staged_path_narrowing"); + let _ = fs::remove_dir_all(&repo_dir); + init_git_repo(&repo_dir)?; + commit_file(&repo_dir, "clean.txt", "nothing\n", "init")?; + stage_file(&repo_dir, "clean.txt", "nothing\nstill nothing\n")?; + stage_file( + &repo_dir, + "secret.txt", + "aws_access_key_id = AKIAIOSFODNN7EXAMPLE\n", + )?; + + let run = |path: &str| { + Command::new(env!("CARGO_BIN_EXE_key-watch")) + .args(["scan", "--staged", "--no-baseline-discovery", "--", path]) + .env("KEYWATCH_CONFIG_PATH", detectors_config_path()) + .current_dir(&repo_dir) + .output() + .expect("run key-watch") + }; + + assert_eq!( + run("clean.txt").status.code(), + Some(0), + "narrowing to a clean path must not report the other file" + ); + assert_eq!( + run("secret.txt").status.code(), + Some(1), + "narrowing to the secret path must still report it" + ); + + let _ = fs::remove_dir_all(&repo_dir); + Ok(()) +}