From 0acaaa0680656b85900f4d8339dc3d26d87a2687 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Mon, 24 Nov 2025 10:36:58 -0800 Subject: [PATCH] Updated Summary to include scan date, kingfisher version ran, and latest kingfisher version available --- .github/workflows/release.yml | 17 +++++ CHANGELOG.md | 3 + src/main.rs | 23 +++++-- src/scanner/runner.rs | 8 ++- src/scanner/summary.rs | 23 +++++++ src/update.rs | 117 +++++++++++++++++++++++++++------- tests/fingerprint_dedup.rs | 6 +- tests/smoke_update.rs | 12 ++-- 8 files changed, 168 insertions(+), 41 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ed7f8f5..0183d0a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -273,6 +273,23 @@ jobs: with: path: target/release/kingfisher-* merge-multiple: true + - name: Generate aggregate checksums + run: | + set -euo pipefail + VERSION="${{ steps.version.outputs.version }}" + OUTPUT="kingfisher_${VERSION}_checksums.txt" + + cd target/release + shopt -s nullglob + files=(kingfisher-*.tgz kingfisher-*.zip kingfisher-*.deb kingfisher-*.rpm) + + if [ ${#files[@]} -eq 0 ]; then + echo "No release artifacts found for checksum generation" >&2 + exit 1 + fi + + sha256sum "${files[@]}" > "$OUTPUT" + echo "Wrote checksums to target/release/$OUTPUT" - name: Extract latest changelog section run: | awk ' diff --git a/CHANGELOG.md b/CHANGELOG.md index 621e5b0..827890b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ All notable changes to this project will be documented in this file. ## [v1.67.0] - Added checksum to GitLab rule - Fixed deduplication to consider rule identifiers so overlapping patterns are not merged before validation +- After scan summaries, emit the styled outdated-version notice to stderr when a newer release is available +- Reduced false positives across a number of rules +- Updated Summary to include scan date, kingfisher version ran, and latest kingfisher version available ## [v1.66.0] - Updating to support Bitbucket App Passwords diff --git a/src/main.rs b/src/main.rs index b52fcd3..187bd2c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -27,7 +27,7 @@ static GLOBAL: System = System; // static GLOBAL: System = System; use std::{ - io::{IsTerminal, Read}, + io::{IsTerminal, Read, Write}, sync::{Arc, Mutex}, }; @@ -194,7 +194,7 @@ async fn async_main(args: CommandLineArgs) -> Result<()> { let clone_dir = temp_dir.path().to_path_buf(); let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); - let update_msg = check_for_update(&global_args, None); + let update_status = check_for_update(&global_args, None); match command { Command::Scan(scan_command) => match scan_command.into_operation()? { ScanOperation::Scan(mut scan_args) => { @@ -213,8 +213,19 @@ async fn async_main(args: CommandLineArgs) -> Result<()> { } let rules_db = Arc::new(load_and_record_rules(&scan_args, &datastore)?); - run_scan(&global_args, &scan_args, &rules_db, Arc::clone(&datastore)) - .await?; + run_scan( + &global_args, + &scan_args, + &rules_db, + Arc::clone(&datastore), + &update_status, + ) + .await?; + if update_status.is_outdated { + if let Some(styled) = &update_status.styled_message { + let _ = writeln!(std::io::stderr(), "{}", styled); + } + } let exit_code = determine_exit_code(&datastore); if let Err(e) = temp_dir.close() { @@ -324,8 +335,8 @@ async fn async_main(args: CommandLineArgs) -> Result<()> { anyhow::bail!("SelfUpdate command should not reach this branch") } } - if let Some(msg) = update_msg { - info!("{msg}"); + if let Some(message) = &update_status.message { + info!("{}", message); } Ok(()) } diff --git a/src/scanner/runner.rs b/src/scanner/runner.rs index 39decc3..b054a99 100644 --- a/src/scanner/runner.rs +++ b/src/scanner/runner.rs @@ -4,6 +4,7 @@ use std::{ }; use anyhow::{bail, Context, Result}; +use chrono::Local; use crossbeam_skiplist::SkipMap; use indicatif::ProgressBar; use tokio::time::{Duration, Instant}; @@ -41,8 +42,9 @@ pub async fn run_scan( scan_args: &scan::ScanArgs, rules_db: &RulesDatabase, datastore: Arc>, + update_status: &crate::update::UpdateStatus, ) -> Result<()> { - run_async_scan(global_args, scan_args, Arc::clone(&datastore), rules_db) + run_async_scan(global_args, scan_args, Arc::clone(&datastore), rules_db, update_status) .await .context("Failed to run scan command") } @@ -52,6 +54,7 @@ pub async fn run_async_scan( args: &scan::ScanArgs, datastore: Arc>, rules_db: &RulesDatabase, + update_status: &crate::update::UpdateStatus, ) -> Result<()> { // Ensure all provided paths exist before proceeding for path in &args.input_specifier_args.path_inputs { @@ -71,6 +74,7 @@ pub async fn run_async_scan( } let start_time = Instant::now(); + let scan_started_at = chrono::Local::now(); trace!("Args:\n{global_args:#?}\n{args:#?}"); let progress_enabled = global_args.use_progress(); @@ -287,12 +291,14 @@ pub async fn run_async_scan( .context("Failed to run report command")?; print_scan_summary( start_time, + scan_started_at, &datastore, global_args, args, rules_db, &matcher_stats, if enable_profiling { Some(shared_profiler.as_ref()) } else { None }, + update_status, ); Ok(()) } diff --git a/src/scanner/summary.rs b/src/scanner/summary.rs index 9d4f30e..16d0798 100644 --- a/src/scanner/summary.rs +++ b/src/scanner/summary.rs @@ -3,6 +3,7 @@ use std::{ sync::{Arc, Mutex}, }; +use chrono::Local; use http::StatusCode; use indicatif::HumanBytes; use serde_json::json; @@ -19,6 +20,7 @@ use crate::{ matcher::MatcherStats, rule_profiling::ConcurrentRuleProfiler, rules_database::RulesDatabase, + update::{UpdateCheckStatus, UpdateStatus}, }; macro_rules! safe_println { @@ -37,6 +39,7 @@ macro_rules! safe_println { pub fn print_scan_summary( start_time: Instant, + scan_started_at: chrono::DateTime, datastore: &Arc>, global_args: &global::GlobalArgs, args: &scan::ScanArgs, @@ -44,6 +47,7 @@ pub fn print_scan_summary( rules_db: &RulesDatabase, matcher_stats: &Mutex, profiler: Option<&ConcurrentRuleProfiler>, + update_status: &UpdateStatus, ) { if global_args.quiet { if args.rule_stats { @@ -137,12 +141,28 @@ pub fn print_scan_summary( "blobs_scanned": matcher_stats.blobs_scanned, "bytes_scanned": matcher_stats.bytes_scanned, "scan_duration": duration.as_secs_f64(), + "scan_date": scan_started_at.to_rfc3339(), + "kingfisher": { + "version_used": update_status.running_version.clone(), + "latest_version": update_status.latest_version.clone(), + "update_check_status": update_status.check_status.as_str(), + "update_check_message": update_status.message.clone(), + }, "findings_by_rule": sorted_findings }); safe_println!("{}", summary.to_string()); } else if args.output_args.format == ReportOutputFormat::Pretty || args.output_args.output.is_some() { + let scan_date = scan_started_at.format("%Y-%m-%d %H:%M:%S %Z"); + let latest_version = match update_status.check_status { + UpdateCheckStatus::Disabled => "Update check disabled (--no-update-check)".to_string(), + UpdateCheckStatus::Failed => "Unknown (update check failed)".to_string(), + UpdateCheckStatus::Ok => { + update_status.latest_version.clone().unwrap_or_else(|| "Unknown".to_string()) + } + }; + safe_println!("\n=========================================="); safe_println!("Scan Summary:"); safe_println!("=========================================="); @@ -165,6 +185,9 @@ pub fn print_scan_summary( HumanBytes(matcher_stats.bytes_scanned) ); safe_println!(" |Scan Duration...............: {}", humantime::format_duration(duration)); + safe_println!(" |Scan Date...................: {}", scan_date); + safe_println!(" |Kingfisher Version..........: {}", &update_status.running_version); + safe_println!(" |__Latest Version............: {}", latest_version); } if args.rule_stats { diff --git a/src/update.rs b/src/update.rs index 85877f5..a498119 100644 --- a/src/update.rs +++ b/src/update.rs @@ -23,6 +23,33 @@ use tracing::error; use crate::{cli::global::GlobalArgs, reporter::styles::Styles}; +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum UpdateCheckStatus { + Disabled, + Failed, + Ok, +} + +impl UpdateCheckStatus { + pub fn as_str(&self) -> &'static str { + match self { + UpdateCheckStatus::Disabled => "disabled", + UpdateCheckStatus::Failed => "failed", + UpdateCheckStatus::Ok => "ok", + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UpdateStatus { + pub message: Option, + pub styled_message: Option, + pub is_outdated: bool, + pub running_version: String, + pub latest_version: Option, + pub check_status: UpdateCheckStatus, +} + fn styled_heading(styles: &Styles, text: &str) -> String { styles.style_finding_active_heading.apply_to(text).to_string() } @@ -31,9 +58,18 @@ fn styled_heading(styles: &Styles, text: &str) -> String { /// /// * `base_url` lets tests point at a mock server. /// * Self-update is skipped when the user disabled it **or** the binary is a Homebrew install. -pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Option { +pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> UpdateStatus { + let running_version = cargo_crate_version!().to_string(); + if global_args.no_update_check { - return None; + return UpdateStatus { + message: Some("Update check disabled (--no-update-check)".to_string()), + styled_message: None, + is_outdated: false, + running_version, + latest_version: None, + check_status: UpdateCheckStatus::Disabled, + }; } // Respect the user's color preferences when printing update @@ -88,49 +124,75 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Opt // Build the updater. let Ok(updater) = builder.build() else { - let _ = writeln!( - std::io::stderr(), - "{}", - styled_heading(&styles, "Failed to configure update checker") - ); - return None; + let plain = "Failed to configure update checker".to_string(); + let styled_message = styled_heading(&styles, &plain); + let _ = writeln!(std::io::stderr(), "{}", styled_message); + return UpdateStatus { + message: Some(plain), + styled_message: Some(styled_message), + is_outdated: false, + running_version, + latest_version: None, + check_status: UpdateCheckStatus::Failed, + }; }; // Query GitHub. let Ok(release) = updater.get_latest_release() else { - let _ = writeln!( - std::io::stderr(), - "{}", - styled_heading(&styles, "Failed to check for updates") - ); - return None; + let plain = "Failed to check for updates".to_string(); + let styled_message = styled_heading(&styles, &plain); + let _ = writeln!(std::io::stderr(), "{}", styled_message); + return UpdateStatus { + message: Some(plain), + styled_message: Some(styled_message), + is_outdated: false, + running_version, + latest_version: None, + check_status: UpdateCheckStatus::Failed, + }; }; - let running_v = cargo_crate_version!(); - // ───────────── Case 1: running == latest ───────────── - if release.version == running_v { - let plain = format!("Kingfisher {running_v} is up to date"); + if release.version == running_version { + let plain = format!("Kingfisher {running_version} is up to date"); let _ = writeln!(std::io::stderr(), "{plain}"); - return Some(plain); + return UpdateStatus { + message: Some(plain.clone()), + styled_message: Some(plain), + is_outdated: false, + running_version, + latest_version: Some(release.version), + check_status: UpdateCheckStatus::Ok, + }; } // Try semantic version comparison. If parsing fails, fall back to the // self-update code-path (which will treat the strings lexicographically). - if let (Ok(curr), Ok(latest)) = (Version::parse(running_v), Version::parse(&release.version)) { + if let (Ok(curr), Ok(latest)) = + (Version::parse(&running_version), Version::parse(&release.version)) + { // ───────── Case 2: running > latest (dev build) ───────── if curr > latest { let plain = format!("Running Kingfisher {curr} which is newer than latest released {latest}"); - let _ = writeln!(std::io::stderr(), "{}", styled_heading(&styles, &plain)); - return Some(plain); + let styled_message = styled_heading(&styles, &plain); + let _ = writeln!(std::io::stderr(), "{}", styled_message); + return UpdateStatus { + message: Some(plain), + styled_message: Some(styled_message), + is_outdated: false, + running_version, + latest_version: Some(release.version), + check_status: UpdateCheckStatus::Ok, + }; } // else fall through to Case 3 (latest > running) } // ───────────── Case 3: latest > running ───────────── let plain = format!("New Kingfisher release {} available", release.version); - let _ = writeln!(std::io::stderr(), "{}", styled_heading(&styles, &plain)); + let styled_message = styled_heading(&styles, &plain); + let _ = writeln!(std::io::stderr(), "{}", styled_message); // Attempt self-update when allowed and feasible. if global_args.self_update { @@ -172,5 +234,12 @@ pub fn check_for_update(global_args: &GlobalArgs, base_url: Option<&str>) -> Opt } } - Some(plain) + UpdateStatus { + message: Some(plain), + styled_message: Some(styled_message), + is_outdated: true, + running_version, + latest_version: Some(release.version), + check_status: UpdateCheckStatus::Ok, + } } diff --git a/tests/fingerprint_dedup.rs b/tests/fingerprint_dedup.rs index dee2fd3..7a4c701 100644 --- a/tests/fingerprint_dedup.rs +++ b/tests/fingerprint_dedup.rs @@ -208,11 +208,7 @@ fn dedup_preserves_distinct_rules_with_same_fingerprint() -> Result<()> { let deduped = reporter.deduplicate_matches(matches, /* no_dedup= */ false); - assert_eq!( - deduped.len(), - 2, - "matches from distinct rules must not be deduplicated" - ); + assert_eq!(deduped.len(), 2, "matches from distinct rules must not be deduplicated"); Ok(()) } diff --git a/tests/smoke_update.rs b/tests/smoke_update.rs index 8224d42..9453674 100644 --- a/tests/smoke_update.rs +++ b/tests/smoke_update.rs @@ -8,7 +8,9 @@ use wiremock::{ #[tokio::test] async fn no_update_when_flag_set() { let args = GlobalArgs { no_update_check: true, ..Default::default() }; - assert!(check_for_update(&args, None).is_none()); + let status = check_for_update(&args, None); + assert_eq!(status.check_status.as_str(), "disabled"); + assert!(status.latest_version.is_none()); } #[tokio::test] @@ -33,14 +35,14 @@ async fn detects_new_release() { } // run the update checker on a blocking thread - let msg = tokio::task::spawn_blocking({ + let status = tokio::task::spawn_blocking({ let uri = server.uri(); // move into closure let args = GlobalArgs::default(); move || check_for_update(&args, Some(&uri)) }) .await - .expect("blocking task panicked") - .expect("update checker returned None"); + .expect("blocking task panicked"); - assert!(msg.contains("99.999.0")); + assert!(status.is_outdated); + assert!(status.message.contains("99.999.0")); }