report: add --finding-status=STATUS filter (#162)

This allows a user to include only findings with a particular assigned status when running `report`.

CHANGELOG.md

@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Additions
 
 - The README now includes several animated GIFs that demonstrate simple example use cases ([#154](https://github.com/praetorian-inc/noseyparker/pull/154)).
+- The `report` command now offers a new `--finding-status=STATUS` filtering option, which causes only the findings with the requested status to be reported.
 
 ### Changes
 

crates/noseyparker-cli/src/args.rs

@@ -829,9 +829,16 @@ pub struct ReportArgs {
     )]
     pub datastore: PathBuf,
 
+    #[command(flatten)]
+    pub filter_args: ReportFilterArgs,
+
     #[command(flatten)]
     pub output_args: OutputArgs<ReportOutputFormat>,
+}
 
+#[derive(Args, Debug)]
+#[command(next_help_heading = "Filtering Options")]
+pub struct ReportFilterArgs {
     /// Limit the number of matches per finding to at most N
     ///
     /// A negative value means "no limit".
@@ -842,6 +849,24 @@ pub struct ReportArgs {
         allow_negative_numbers = true
     )]
     pub max_matches: i64,
+
+    /// Include only findings with the assigned status
+    #[arg(long)]
+    pub finding_status: Option<FindingStatus>,
+}
+
+#[derive(ValueEnum, Debug, Display, Clone, Copy)]
+#[clap(rename_all = "lower")]
+#[strum(serialize_all = "lowercase")]
+pub enum FindingStatus {
+    /// Findings with `accept` matches
+    Accept,
+    /// Findings with `reject` matches
+    Reject,
+    /// Findings with both `accept` and `reject` matches
+    Mixed,
+    /// Findings without any `accept` or `reject` matches
+    Null,
 }
 
 // -----------------------------------------------------------------------------

crates/noseyparker-cli/src/cmd_report.rs

@@ -13,7 +13,7 @@ use noseyparker::match_type::{Group, Groups, Match};
 use noseyparker::provenance::Provenance;
 use noseyparker::provenance_set::ProvenanceSet;
 
-use crate::args::{GlobalArgs, ReportArgs, ReportOutputFormat};
+use crate::args::{FindingStatus, GlobalArgs, ReportArgs, ReportOutputFormat};
 use crate::reportable::Reportable;
 
 mod human_format;
@@ -30,10 +30,10 @@ pub fn run(global_args: &GlobalArgs, args: &ReportArgs) -> Result<()> {
         .get_writer()
         .context("Failed to get output writer")?;
 
-    let max_matches = if args.max_matches <= 0 {
+    let max_matches = if args.filter_args.max_matches <= 0 {
         None
     } else {
-        Some(args.max_matches.try_into().unwrap())
+        Some(args.filter_args.max_matches.try_into().unwrap())
     };
 
     // enable output styling:
@@ -50,6 +50,7 @@ pub fn run(global_args: &GlobalArgs, args: &ReportArgs) -> Result<()> {
     let reporter = DetailsReporter {
         datastore,
         max_matches,
+        finding_status: args.filter_args.finding_status,
         styles,
     };
     reporter.report(args.output_args.format, output)
@@ -58,10 +59,36 @@ pub fn run(global_args: &GlobalArgs, args: &ReportArgs) -> Result<()> {
 struct DetailsReporter {
     datastore: Datastore,
     max_matches: Option<usize>,
+    finding_status: Option<FindingStatus>,
     styles: Styles,
 }
 
 impl DetailsReporter {
+    fn include_finding(&self, metadata: &FindingMetadata) -> bool {
+        match self.finding_status {
+            None => true,
+            Some(status) => match (status, metadata.statuses.0.as_slice()) {
+                (FindingStatus::Accept, &[Status::Accept]) => true,
+                (FindingStatus::Reject, &[Status::Reject]) => true,
+                (FindingStatus::Null, &[]) => true,
+                (FindingStatus::Mixed, &[Status::Accept, Status::Reject]) => true,
+                (FindingStatus::Mixed, &[Status::Reject, Status::Accept]) => true,
+                _ => false,
+            },
+        }
+    }
+
+    fn get_finding_metadata(&self) -> Result<Vec<FindingMetadata>> {
+        let datastore = &self.datastore;
+        let mut group_metadata = datastore
+            .get_finding_metadata()
+            .context("Failed to get match group metadata from datastore")?;
+
+        group_metadata.retain(|md| self.include_finding(&md));
+
+        Ok(group_metadata)
+    }
+
     fn get_matches(&self, metadata: &FindingMetadata) -> Result<Vec<ReportMatch>> {
         Ok(self
             .datastore
@@ -123,10 +150,7 @@ impl DetailsReporter {
         sep: Option<&str>,
         end: Option<&str>,
     ) -> Result<()> {
-        let datastore = &self.datastore;
-        let group_metadata = datastore
-            .get_finding_metadata()
-            .context("Failed to get match group metadata from datastore")?;
+        let group_metadata = self.get_finding_metadata()?;
 
         if let Some(begin) = begin {
             write!(writer, "{}", begin)?;

crates/noseyparker-cli/src/cmd_report/human_format.rs

@@ -2,11 +2,7 @@ use super::*;
 
 impl DetailsReporter {
     pub fn human_format<W: std::io::Write>(&self, mut writer: W) -> Result<()> {
-        let datastore = &self.datastore;
-        let group_metadata = datastore
-            .get_finding_metadata()
-            .context("Failed to get match group metadata from datastore")?;
-
+        let group_metadata = self.get_finding_metadata()?;
         let num_findings = group_metadata.len();
         for (finding_num, metadata) in group_metadata.into_iter().enumerate() {
             let finding_num = finding_num + 1;

crates/noseyparker-cli/src/cmd_report/sarif_format.rs

@@ -107,10 +107,7 @@ impl DetailsReporter {
     }
 
     pub fn sarif_format<W: std::io::Write>(&self, mut writer: W) -> Result<()> {
-        let datastore: &Datastore = &self.datastore;
-        let group_metadata = datastore
-            .get_finding_metadata()
-            .context("Failed to get match group metadata from datastore")?;
+        let group_metadata = self.get_finding_metadata()?;
 
         let mut findings = Vec::with_capacity(group_metadata.len());
         for metadata in group_metadata {

crates/noseyparker-cli/tests/help/snapshots/test_noseyparker__help__help_report-2.snap

@@ -16,6 +16,23 @@ Options:
   -h, --help
           Print help (see a summary with '-h')
 
+Filtering Options:
+      --max-matches <N>
+          Limit the number of matches per finding to at most N
+
+          A negative value means "no limit".
+
+          [default: 3]
+
+      --finding-status <FINDING_STATUS>
+          Include only findings with the assigned status
+
+          Possible values:
+          - accept: Findings with `accept` matches
+          - reject: Findings with `reject` matches
+          - mixed:  Findings with both `accept` and `reject` matches
+          - null:   Findings without any `accept` or `reject` matches
+
 Output Options:
   -o, --output <PATH>
           Write output to the specified path
@@ -33,13 +50,6 @@ Output Options:
           - jsonl: JSON Lines format
           - sarif: SARIF format (experimental)
 
-      --max-matches <N>
-          Limit the number of matches per finding to at most N
-
-          A negative value means "no limit".
-
-          [default: 3]
-
 Global Options:
   -v, --verbose...
           Enable verbose output

crates/noseyparker-cli/tests/help/snapshots/test_noseyparker__help__help_report_short-2.snap

@@ -10,11 +10,17 @@ Options:
   -d, --datastore <PATH>  Use the specified datastore [env: NP_DATASTORE=] [default: datastore.np]
   -h, --help              Print help (see more with '--help')
 
+Filtering Options:
+      --max-matches <N>
+          Limit the number of matches per finding to at most N [default: 3]
+      --finding-status <FINDING_STATUS>
+          Include only findings with the assigned status [possible values: accept, reject, mixed,
+          null]
+
 Output Options:
   -o, --output <PATH>    Write output to the specified path
   -f, --format <FORMAT>  Write output in the specified format [default: human] [possible values:
                          human, json, jsonl, sarif]
-      --max-matches <N>  Limit the number of matches per finding to at most N [default: 3]
 
 Global Options:
   -v, --verbose...       Enable verbose output
@@ -23,4 +29,3 @@ Global Options:
                          never, always]
       --progress <MODE>  Enable or disable progress bars [default: auto] [possible values: auto,
                          never, always]
-

crates/noseyparker-cli/tests/report/mod.rs

@@ -0,0 +1,133 @@
+use super::*;
+pub use pretty_assertions::{assert_eq, assert_ne};
+
+#[test]
+fn report_nonexistent_default_datastore() {
+    let scan_env = ScanEnv::new();
+    let ds = scan_env.root.child("datastore.np");
+    ds.assert(predicates::path::missing());
+
+    assert_cmd_snapshot!(noseyparker!("report")
+        .current_dir(scan_env.root.path())
+        .assert()
+        .failure());
+
+    ds.assert(predicates::path::missing());
+}
+
+/// Test that the `report` command's `--max-matches` can be given a negative value (which means "no
+/// limit" for the option) without requiring an equals sign for the value. That is, instead of
+/// _requiring_ that the option be written `--max-matches=-1`, it should work fine to write
+/// `--max-matches -1`.
+///
+/// N.B., Suppoorting that argument parsing requires passing the `allow_negative_numbers=true` in
+/// the correct spot in the `clap` code.
+#[test]
+fn report_unlimited_matches() {
+    let scan_env = ScanEnv::new();
+    let input = scan_env.input_file_with_secret("input.txt");
+
+    noseyparker_success!("scan", "-d", scan_env.dspath(), input.path())
+        .stdout(match_scan_stats("104 B", 1, 1, 1));
+
+    with_settings!({
+        filters => get_report_stdout_filters(),
+    }, {
+        assert_cmd_snapshot!(noseyparker_success!("report", "-d", scan_env.dspath(), "--max-matches", "-1"));
+    });
+}
+
+/// Test that the `report` command uses colors as expected when *not* running under a pty:
+///
+/// - When running with the output explicitly written to a file, colors are not used
+///
+/// - When running with with the output explicitly written to a file and `--color=always`
+///   specified, colors are used
+#[test]
+fn report_output_colors1() {
+    let scan_env = ScanEnv::new();
+    let input = scan_env.input_file_with_secret("input.txt");
+
+    let output1 = scan_env.child("findings.txt");
+    let output2 = scan_env.child("findings.colored.txt");
+
+    noseyparker_success!("scan", "-d", scan_env.dspath(), input.path())
+        .stdout(match_scan_stats("104 B", 1, 1, 1));
+
+    noseyparker_success!("report", "-d", scan_env.dspath(), "-o", output1.path());
+    noseyparker_success!("report", "-d", scan_env.dspath(), "-o", output2.path(), "--color=always");
+
+    let output1_contents = std::fs::read_to_string(output1.path()).unwrap();
+    let output2_contents = std::fs::read_to_string(output2.path()).unwrap();
+
+    assert_ne!(output1_contents, output2_contents);
+    with_settings!({
+        filters => get_report_stdout_filters(),
+    }, {
+        assert_snapshot!(output1_contents);
+    });
+    assert_eq!(&output1_contents, &console::strip_ansi_codes(&output2_contents));
+}
+
+fn read_json_file(fname: &Path) -> serde_json::Value {
+    let file = std::fs::File::open(fname).unwrap();
+    let mut reader = std::io::BufReader::new(file);
+    let findings = serde_json::from_reader(&mut reader).unwrap();
+    findings
+}
+
+/// Test that the `report --finding-status` option works as expected.
+/// In the case of a newly-created datastore, there will be no statuses assigned at all, so we do
+/// only some basic checks.
+#[test]
+fn report_finding_status() {
+    use serde_json::json;
+
+    let scan_env = ScanEnv::new();
+    let input = scan_env.input_file_with_secret("input.txt");
+    noseyparker_success!("scan", "-d", scan_env.dspath(), input.path())
+        .stdout(match_scan_stats("104 B", 1, 1, 1));
+
+    let report = |out: &ChildPath, status: &str| {
+        noseyparker_success!(
+            "report",
+            "-d",
+            scan_env.dspath(),
+            "--format=json",
+            "-o",
+            out.path(),
+            "--finding-status",
+            status
+        );
+    };
+
+    // case 1: accept
+    let output = scan_env.child("findings.accept.json");
+    report(&output, "accept");
+    let findings = read_json_file(output.path());
+    assert_eq!(findings, json!([]));
+
+    // case 2: reject
+    let output = scan_env.child("findings.reject.json");
+    report(&output, "reject");
+    let findings = read_json_file(output.path());
+    assert_eq!(findings, json!([]));
+
+    // case 3: mixed
+    let output = scan_env.child("findings.mixed.json");
+    report(&output, "mixed");
+    let findings = read_json_file(output.path());
+    assert_eq!(findings, json!([]));
+
+    // case 4: null
+    let output = scan_env.child("findings.null.json");
+    report(&output, "null");
+    let findings = read_json_file(output.path());
+    assert!(findings.is_array());
+    assert_eq!(findings.as_array().unwrap().len(), 1);
+}
+
+// Test that the `report` command uses colors as expected when running under a pty:
+// - When running with the output going to stdout (default), colors are used
+// - When running with the explicitly written to a file, colors are not used
+// XXX to get a pty, look at the `pty-process` crate: https://docs.rs/pty-process/latest/pty_process/blocking/struct.Command.html

crates/noseyparker-cli/tests/report/snapshots/test_noseyparker__report__report_nonexistent_default_datastore-2.snap

@@ -0,0 +1,5 @@
+---
+source: crates/noseyparker-cli/tests/report/mod.rs
+expression: stdout
+---
+

crates/noseyparker-cli/tests/scan/basic/snapshots/test_noseyparker__scan__basic__report_nonexistent_default_datastore-3.snap → crates/noseyparker-cli/tests/report/snapshots/test_noseyparker__report__report_nonexistent_default_datastore-3.snap

@@ -1,6 +1,5 @@
 ---
-source: crates/noseyparker-cli/tests/scan/basic/mod.rs
+source: crates/noseyparker-cli/tests/report/mod.rs
 expression: stderr
 ---
 Error: Failed to open datastore at datastore.np: unable to open database file: datastore.np/datastore.db: Error code 14: Unable to open the database file
-

crates/noseyparker-cli/tests/report/snapshots/test_noseyparker__report__report_nonexistent_default_datastore.snap

@@ -0,0 +1,5 @@
+---
+source: crates/noseyparker-cli/tests/report/mod.rs
+expression: status
+---
+exit status: 2

crates/noseyparker-cli/tests/scan/basic/snapshots/test_noseyparker__scan__basic__report_output_colors1.snap → crates/noseyparker-cli/tests/report/snapshots/test_noseyparker__report__report_output_colors1.snap

@@ -1,5 +1,5 @@
 ---
-source: crates/noseyparker-cli/tests/scan/basic/mod.rs
+source: crates/noseyparker-cli/tests/report/mod.rs
 expression: output1_contents
 ---
 Finding 1/1
@@ -14,7 +14,3 @@ Group: ghp_XIxB7KMNdAr3zqWtQqhE94qglHqOzn1D1stg
         # This is fake configuration data
         USERNAME=the_dude
         GITHUB_KEY=ghp_XIxB7KMNdAr3zqWtQqhE94qglHqOzn1D1stg
-
-
-
-

crates/noseyparker-cli/tests/scan/basic/snapshots/test_noseyparker__scan__basic__report_unlimited_matches-2.snap → crates/noseyparker-cli/tests/report/snapshots/test_noseyparker__report__report_unlimited_matches-2.snap

@@ -1,5 +1,5 @@
 ---
-source: crates/noseyparker-cli/tests/scan/basic/mod.rs
+source: crates/noseyparker-cli/tests/report/mod.rs
 expression: stdout
 ---
 Finding 1/1
@@ -14,7 +14,3 @@ Group: ghp_XIxB7KMNdAr3zqWtQqhE94qglHqOzn1D1stg
         # This is fake configuration data
         USERNAME=the_dude
         GITHUB_KEY=ghp_XIxB7KMNdAr3zqWtQqhE94qglHqOzn1D1stg
-
-
-
-