Merge pull request #217 from mongodb/development

more changes for v1.78.0

.pre-commit-hooks.yaml

@@ -4,7 +4,7 @@
   entry: scripts/kingfisher-pre-commit-auto.sh
   language: script
   pass_filenames: false
-  stages: [commit]
+  stages: [pre-commit]
 
 - id: kingfisher-docker
   name: kingfisher (docker)
@@ -13,7 +13,7 @@
   language: docker
   args: ["scan", ".", "--staged", "--quiet", "--no-update-check"]
   pass_filenames: false
-  stages: [commit]
+  stages: [pre-commit]
 
 - id: kingfisher
   name: kingfisher
@@ -23,4 +23,4 @@
   args: ["scan", ".", "--staged", "--quiet", "--no-update-check"]
   pass_filenames: false
   types: [file]
-  stages: [commit]
+  stages: [pre-commit]

CHANGELOG.md

@@ -3,6 +3,7 @@
 All notable changes to this project will be documented in this file.
 
 ## [v1.78.0]
+- Added "Skipped Validations" counter to scan summary output to distinguish between validations that failed (HTTP errors, connection failures) and validations that were skipped due to missing preconditions (e.g., missing dependent rules). This provides better visibility into validation coverage for large scans.
 - Improved error messages for `kingfisher validate` command when rules require dependent variables from `depends_on` sections. Now clearly explains which variables are needed and from which dependent rules they are normally captured.
 - Fixed `validate_command` and `revoke_command` generation in scan output to include all required `--var` arguments for rules with `depends_on` sections (e.g., PubNub, Azure Storage). Commands now include dependent variables like `--var SUBSCRIPTIONTOKEN=<value>` or `--var AZURENAME=<value>`.
 - Updated Azure Storage validation to use `AZURENAME` variable (matching the `depends_on_rule` configuration) with `STORAGE_ACCOUNT` maintained as a backward-compatible alias.
@@ -15,6 +16,7 @@ All notable changes to this project will be documented in this file.
 - The `--ignore-certs` flag remains supported as a deprecated alias for `--tls-mode=off` for backward compatibility.
 - Updated documentation to explain TLS validation modes and their security implications.
 - Added comprehensive test coverage for TLS mode functionality including unit tests, integration tests, and rule configuration verification.
+- Fixed deprecated `commit` stage name in `.pre-commit-hooks.yaml` to use `pre-commit` stage name, eliminating pre-commit framework warnings.
 
 ## [v1.77.0]
 - Added `kingfisher revoke` subcommand for revoking leaked credentials directly with the provider.

docs/USAGE.md

@@ -19,6 +19,7 @@ This guide covers all scan targets and usage patterns for Kingfisher.
   - [Confluence](#confluence)
   - [Slack](#slack)
 - [TLS Certificate Validation](#tls-certificate-validation)
+- [Understanding the Scan Summary](#understanding-the-scan-summary)
 - [Environment Variables](#environment-variables)
 - [Exit Codes](#exit-codes)
 
@@ -907,6 +908,51 @@ The legacy `--ignore-certs` flag is still supported as an alias for `--tls-mode=
 
 ---
 
+## Understanding the Scan Summary
+
+After each scan, Kingfisher displays a summary with validation statistics:
+
+```
+==========================================
+Scan Summary:
+==========================================
+ |Findings....................: 15
+ |__Successful Validations....: 3
+ |__Failed Validations........: 5
+ |__Skipped Validations.......: 2
+ |Rules Applied...............: 120
+ |__Blobs Scanned.............: 1,234
+ |Bytes Scanned...............: 45.2 MB
+ |Scan Duration...............: 12s 345ms
+ ...
+```
+
+### Validation Counters
+
+| Counter | Description |
+| ------- | ----------- |
+| **Successful Validations** | Credentials confirmed as active by the provider (e.g., API returned valid response) |
+| **Failed Validations** | Validations that were attempted but failed (HTTP errors, connection timeouts, invalid credentials) |
+| **Skipped Validations** | Validations that could not be attempted due to missing preconditions (e.g., missing dependent rules) |
+
+### Why Validations Are Skipped
+
+Validations are marked as "skipped" when:
+
+- **Missing dependent rules**: Some rules require values from other rules to validate. For example, an AWS Secret Key rule needs the Access Key ID from the AWS Access Key rule. If the dependent rule wasn't matched, validation cannot proceed.
+- **Preconditions not met**: The validation endpoint requires additional context that wasn't available in the scan.
+
+When a validation is skipped, the finding will show:
+
+```
+ |Validation....: Inactive Credential
+ |__Response....: Validation skipped - missing dependent rules: helper-rule-id
+```
+
+This distinction helps you understand validation coverage: **Failed Validations** represent actual validation attempts, while **Skipped Validations** indicate opportunities to improve rule coverage or provide additional context.
+
+---
+
 ## Environment Variables
 
 | Variable          | Purpose                      |

src/scanner/summary.rs

@@ -28,6 +28,7 @@ pub struct ScanSummaryTotals {
     pub findings: usize,
     pub successful_validations: usize,
     pub failed_validations: usize,
+    pub skipped_validations: usize,
     pub blobs_scanned: u64,
     pub bytes_scanned: u64,
 }
@@ -40,6 +41,9 @@ impl ScanSummaryTotals {
                 .successful_validations
                 .saturating_sub(baseline.successful_validations),
             failed_validations: self.failed_validations.saturating_sub(baseline.failed_validations),
+            skipped_validations: self
+                .skipped_validations
+                .saturating_sub(baseline.skipped_validations),
             blobs_scanned: self.blobs_scanned.saturating_sub(baseline.blobs_scanned),
             bytes_scanned: self.bytes_scanned.saturating_sub(baseline.bytes_scanned),
         }
@@ -82,23 +86,28 @@ pub fn compute_scan_totals(
         ds.get_num_matches()
     };
 
-    let (successful_validations, failed_validations) =
-        all_matches.iter().fold((0, 0), |(success, fail), msg| {
+    let (successful_validations, failed_validations, skipped_validations) =
+        all_matches.iter().fold((0, 0, 0), |(success, fail, skipped), msg| {
             let (origin_set, _, match_item) = &**msg;
             if match_item.validation_success {
                 if match_item.validation_response_status != StatusCode::CONTINUE.as_u16() {
                     if args.no_dedup {
-                        (success + origin_set.len(), fail)
+                        (success + origin_set.len(), fail, skipped)
                     } else {
-                        (success + 1, fail)
+                        (success + 1, fail, skipped)
                     }
                 } else {
-                    (success, fail)
+                    (success, fail, skipped)
                 }
+            } else if match_item.validation_response_status
+                == StatusCode::PRECONDITION_REQUIRED.as_u16()
+            {
+                // Skipped validations (e.g., missing dependent rules)
+                (success, fail, skipped + 1)
             } else if match_item.validation_response_status != StatusCode::CONTINUE.as_u16() {
-                (success, fail + 1)
+                (success, fail + 1, skipped)
             } else {
-                (success, fail)
+                (success, fail, skipped)
             }
         });
 
@@ -108,6 +117,7 @@ pub fn compute_scan_totals(
         findings: total_findings,
         successful_validations,
         failed_validations,
+        skipped_validations,
         blobs_scanned: matcher_stats.blobs_scanned,
         bytes_scanned: matcher_stats.bytes_scanned,
     }
@@ -193,6 +203,7 @@ pub fn print_scan_summary(
                 "findings": totals.findings,
                 "successful_validations": totals.successful_validations,
                 "failed_validations": totals.failed_validations,
+                "skipped_validations": totals.skipped_validations,
                 "rules_applied": num_rules,
                 "blobs_scanned": totals.blobs_scanned,
                 "bytes_scanned": totals.bytes_scanned,
@@ -237,6 +248,10 @@ pub fn print_scan_summary(
                 " |__Failed Validations........: {}",
                 delta.failed_validations.separate_with_commas()
             );
+            safe_println!(
+                " |__Skipped Validations.......: {}",
+                delta.skipped_validations.separate_with_commas()
+            );
             safe_println!(
                 " |Blobs Scanned (delta)......: {}",
                 delta.blobs_scanned.separate_with_commas()
@@ -264,6 +279,10 @@ pub fn print_scan_summary(
                 " |__Failed Validations........: {}",
                 totals.failed_validations.separate_with_commas()
             );
+            safe_println!(
+                " |__Skipped Validations.......: {}",
+                totals.skipped_validations.separate_with_commas()
+            );
             safe_println!(" |Rules Applied...............: {}", num_rules.separate_with_commas());
             safe_println!(
                 " |__Blobs Scanned.............: {}",

tests/tls_mode.rs

@@ -12,7 +12,7 @@ use predicates::prelude::*;
 /// Test that `--tls-mode` is recognized as a valid global option.
 #[test]
 fn tls_mode_flag_is_recognized() {
-    let mut cmd = Command::cargo_bin("kingfisher").unwrap();
+    let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"));
     cmd.arg("--tls-mode=strict").arg("--help");
     cmd.assert().success();
 }
@@ -21,7 +21,7 @@ fn tls_mode_flag_is_recognized() {
 #[test]
 fn tls_mode_accepts_all_values() {
     for mode in ["strict", "lax", "off"] {
-        let mut cmd = Command::cargo_bin("kingfisher").unwrap();
+        let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"));
         cmd.arg(format!("--tls-mode={}", mode)).arg("--help");
         cmd.assert().success();
     }
@@ -30,7 +30,7 @@ fn tls_mode_accepts_all_values() {
 /// Test that invalid TLS mode values are rejected.
 #[test]
 fn tls_mode_rejects_invalid_values() {
-    let mut cmd = Command::cargo_bin("kingfisher").unwrap();
+    let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"));
     cmd.arg("--tls-mode=invalid").arg("--help");
     cmd.assert().failure().stderr(predicate::str::contains("invalid"));
 }
@@ -38,7 +38,7 @@ fn tls_mode_rejects_invalid_values() {
 /// Test that `--ignore-certs` is still accepted (deprecated but supported).
 #[test]
 fn ignore_certs_flag_still_works() {
-    let mut cmd = Command::cargo_bin("kingfisher").unwrap();
+    let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"));
     cmd.arg("--ignore-certs").arg("--help");
     cmd.assert().success();
 }
@@ -46,7 +46,7 @@ fn ignore_certs_flag_still_works() {
 /// Test that --tls-mode appears in the help output.
 #[test]
 fn tls_mode_appears_in_help() {
-    let mut cmd = Command::cargo_bin("kingfisher").unwrap();
+    let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"));
     cmd.arg("--help");
     cmd.assert().success().stdout(predicate::str::contains("--tls-mode"));
 }
@@ -54,7 +54,7 @@ fn tls_mode_appears_in_help() {
 /// Test that rules list subcommand runs with tls-mode flag.
 #[test]
 fn rules_list_works_with_tls_mode() {
-    let mut cmd = Command::cargo_bin("kingfisher").unwrap();
+    let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"));
     cmd.arg("--tls-mode=lax").arg("rules").arg("list");
     cmd.assert()
         .success()
@@ -64,7 +64,7 @@ fn rules_list_works_with_tls_mode() {
 /// Test that a scan with `--tls-mode=strict` runs successfully.
 #[test]
 fn scan_with_strict_mode_runs() {
-    let mut cmd = Command::cargo_bin("kingfisher").unwrap();
+    let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"));
     cmd.arg("--tls-mode=strict").arg("scan").arg("--no-validate").arg("-");
     cmd.write_stdin("test input with no secrets");
     cmd.assert().success();
@@ -73,7 +73,7 @@ fn scan_with_strict_mode_runs() {
 /// Test that a scan with `--tls-mode=lax` runs successfully.
 #[test]
 fn scan_with_lax_mode_runs() {
-    let mut cmd = Command::cargo_bin("kingfisher").unwrap();
+    let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"));
     cmd.arg("--tls-mode=lax").arg("scan").arg("--no-validate").arg("-");
     cmd.write_stdin("test input with no secrets");
     cmd.assert().success();
@@ -82,7 +82,7 @@ fn scan_with_lax_mode_runs() {
 /// Test that a scan with `--tls-mode=off` runs successfully.
 #[test]
 fn scan_with_off_mode_runs() {
-    let mut cmd = Command::cargo_bin("kingfisher").unwrap();
+    let mut cmd = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"));
     cmd.arg("--tls-mode=off").arg("scan").arg("--no-validate").arg("-");
     cmd.write_stdin("test input with no secrets");
     cmd.assert().success();