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 60adaf1..827890b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ 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 - Improved boundaries for several rules diff --git a/Cargo.toml b/Cargo.toml index 2776297..1e48257 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ publish = false [package] name = "kingfisher" -version = "1.66.0" +version = "1.67.0" description = "MongoDB's blazingly fast and accurate secret scanning and validation tool" edition.workspace = true rust-version.workspace = true diff --git a/data/rules/aws.yml b/data/rules/aws.yml index 6cd938a..58cd186 100644 --- a/data/rules/aws.yml +++ b/data/rules/aws.yml @@ -11,6 +11,9 @@ rules: \b pattern_requirements: min_digits: 2 + ignore_if_contains: + - "EXAMPLE" + - "TEST" min_entropy: 3.2 visible: false confidence: medium @@ -41,6 +44,9 @@ rules: ) pattern_requirements: min_digits: 2 + ignore_if_contains: + - "EXAMPLE" + - "TEST" min_entropy: 4.5 confidence: medium examples: diff --git a/data/rules/datadog.yml b/data/rules/datadog.yml index 67c8a5f..8607d94 100644 --- a/data/rules/datadog.yml +++ b/data/rules/datadog.yml @@ -6,7 +6,7 @@ rules: \b (?:datadog|dd) (?:.|[\n\r]){0,64}? - (?:SECRET|PRIVATE|ACCESS|KEY|TOKEN)? + (?:SECRET|PRIVATE|ACCESS|KEY|TOKEN) (?:.|[\n\r]){0,32}? \b ( diff --git a/data/rules/flickr.yml b/data/rules/flickr.yml index fb104eb..faac4c7 100644 --- a/data/rules/flickr.yml +++ b/data/rules/flickr.yml @@ -5,8 +5,8 @@ rules: (?xi) \b flickr - (?:.|[\n\r]){0,32}? - (?:SECRET|PRIVATE|ACCESS|KEY|TOKEN)? + (?:.|[\n\r]){0,16}? + (?:SECRET|PRIVATE|ACCESS|KEY|TOKEN) (?:.|[\n\r]){0,32}? \b ( diff --git a/data/rules/gitlab.yml b/data/rules/gitlab.yml index 96a98bc..ad1a1e7 100644 --- a/data/rules/gitlab.yml +++ b/data/rules/gitlab.yml @@ -120,21 +120,37 @@ rules: - '"403 Forbidden"' negative: true url: https://gitlab.com/api/v4/ci/pipeline_triggers/{{ TOKEN }} - - name: GitLab Private Token - Updated Format + - name: GitLab Private Token - Routable Format id: kingfisher.gitlab.4 pattern: | - (?x) + (?xi) \b ( - glpat-[A-Za-z0-9_-]{36,38}\.01\.[a-z0-9]{9} + glpat- + (?[0-9A-Za-z_-]{27,300}) + \. + (?01) + \. + (?[0-9a-z]{2}) + (?[0-9a-z]{7}) ) \b pattern_requirements: min_digits: 2 + # GitLab's RoutableTokenGenerator renders the CRC32 digest as lowercase + # base36 with a fixed width of 7 characters. The regex and checksum + # expectation mirror that encoding so we only report matches that carry a + # valid GitLab-style checksum. + checksum: + actual: + template: "{{ MATCH | suffix: 7 }}" + requires_capture: crc32 + expected: "{{ \"glpat-\" | append: BASE64_PAYLOAD | append: \".01.\" | append: BASE36_PAYLOAD_LENGTH | crc32 | base36: 7 }}" + skip_if_missing: true min_entropy: 3.5 confidence: medium examples: - - glpat-5m8CwMZi4bwlRSCKzG0-3W86MQp1OmV5Y2UK.01.1012mzo24 + - glpat-ymiBP0-I-J6ghspoBPoZxtSC3g7MyHYG0X0r.01.101erjmwl references: - https://github.com/diffblue/gitlab/blob/39c63ee83369bf5353256a6b95f3116728edd102/doc/api/personal_access_tokens.md - https://docs.gitlab.com/api/personal_access_tokens/ @@ -150,4 +166,4 @@ rules: - type: WordMatch words: - '"id"' - url: https://gitlab.com/api/v4/personal_access_tokens/self \ No newline at end of file + url: https://gitlab.com/api/v4/personal_access_tokens/self diff --git a/data/rules/jdbc.yml b/data/rules/jdbc.yml index 3a2ab14..c0136f7 100644 --- a/data/rules/jdbc.yml +++ b/data/rules/jdbc.yml @@ -10,10 +10,12 @@ rules: : [^\s"'<>,(){}\[\]]{10,448} ) - pattern_requirements: + pattern_requirements: + min_special_chars: 2 + special_chars: ";=/?@&" ignore_if_contains: - - "*****" - - "xxxxx" + - "****" + - "xxxx" min_entropy: 3.3 confidence: medium validation: diff --git a/data/rules/mongodb.yml b/data/rules/mongodb.yml index 523f4c9..f9da274 100644 --- a/data/rules/mongodb.yml +++ b/data/rules/mongodb.yml @@ -83,8 +83,8 @@ rules: \b pattern_requirements: ignore_if_contains: - - "*****" - - "xxxxx" + - "****" + - "xxxx" min_entropy: 3 examples: - client = mongoc_client_new ("mongodb+srv://someuser:hunter2@my-atlas-rd941.mongodb.net/test?retryWrites=true&w=majority"); diff --git a/data/rules/mysql.yml b/data/rules/mysql.yml index 4c7cbf3..d9b16b3 100644 --- a/data/rules/mysql.yml +++ b/data/rules/mysql.yml @@ -34,8 +34,8 @@ rules: ) pattern_requirements: ignore_if_contains: - - "*****" - - "xxxxx" + - "****" + - "xxxx" min_entropy: 3.3 confidence: medium examples: diff --git a/data/rules/postgres.yml b/data/rules/postgres.yml index 47089b6..8cc51a7 100644 --- a/data/rules/postgres.yml +++ b/data/rules/postgres.yml @@ -26,8 +26,8 @@ rules: ) pattern_requirements: ignore_if_contains: - - "*****" - - "xxxxx" + - "****" + - "xxxx" min_entropy: 3.3 confidence: medium examples: diff --git a/data/rules/uri.yml b/data/rules/uri.yml index 45fde52..e3e3a7d 100644 --- a/data/rules/uri.yml +++ b/data/rules/uri.yml @@ -18,8 +18,13 @@ rules: ) pattern_requirements: ignore_if_contains: - - "*****" - - "xxxxx" + - "****" + - "xxxx" + - "username:" + - "user:" + - ":password" + - ":pass" + - ">:<" min_entropy: 4.0 confidence: medium examples: @@ -31,7 +36,7 @@ rules: method: GET url: '{{ TOKEN }}' response_matcher: - - report_response: true + - report_response: false type: StatusMatch status: - 200 diff --git a/data/rules/youtube.yml b/data/rules/youtube.yml new file mode 100644 index 0000000..96b82fb --- /dev/null +++ b/data/rules/youtube.yml @@ -0,0 +1,31 @@ +rules: + - name: YouTube API Key + id: kingfisher.youtube.1 + pattern: | + (?xi) + \b + ( + AIza[a-zA-Z0-9_\-\\]{35} + ) + \b + min_entropy: 2.0 + confidence: medium + examples: + - '"youtube":{"api_key":"AIzaSyCKHtojSg-UNteBWlLUR2kHSgjGpUScYjk"' + validation: + type: Http + content: + request: + method: GET + url: https://www.googleapis.com/youtube/v3/videos?part=id&id=dummy&key={{ TOKEN }} + response_matcher: + - report_response: true + - type: WordMatch + words: + - "API key not valid" + - "keyInvalid" + - "API_KEY_INVALID" + - "forbidden" + match_all_words: false + negative: true + - type: JsonValid diff --git a/src/decompress.rs b/src/decompress.rs index 595d11a..f64c559 100644 --- a/src/decompress.rs +++ b/src/decompress.rs @@ -1,5 +1,3 @@ -//! src/utils/decompress.rs (or wherever you keep the module) - use std::{ fs, io::Read, diff --git a/src/liquid_filters.rs b/src/liquid_filters.rs index 66a2fab..d334360 100644 --- a/src/liquid_filters.rs +++ b/src/liquid_filters.rs @@ -728,6 +728,83 @@ fn value_to_usize(value: &Value) -> Option { .or_else(|| view.to_kstr().parse::().ok()) } +#[derive(Debug, FilterParameters)] +struct Base36Args { + #[parameter( + description = "Pad the encoded value to at least this width", + arg_type = "integer" + )] + width: Option, +} + +#[derive(Clone, ParseFilter, FilterReflection, Default)] +#[filter( + name = "base36", + description = "Encode the provided integer value using Base36.", + parameters(Base36Args), + parsed(Base36) +)] +pub struct Base36Filter; + +#[derive(Debug, FromFilterParameters, Display_filter)] +#[name = "base36"] +struct Base36 { + #[parameters] + args: Base36Args, +} + +impl Filter for Base36 { + fn evaluate(&self, input: &dyn ValueView, runtime: &dyn Runtime) -> Result { + let args = self.args.evaluate(runtime)?; + let value = input + .as_scalar() + .and_then(|scalar| { + if let Some(int) = scalar.to_integer() { + Some(if int < 0 { 0 } else { int as u64 }) + } else if let Some(float) = scalar.to_float() { + Some(if float.is_sign_negative() { 0 } else { float.floor() as u64 }) + } else if let Some(boolean) = scalar.to_bool() { + Some(u64::from(boolean)) + } else { + scalar.to_kstr().to_string().parse::().ok() + } + }) + .or_else(|| input.to_kstr().to_string().parse::().ok()) + .unwrap_or(0); + + let mut encoded = encode_base36(value); + if let Some(width) = args.width.and_then(|value| { + let scalar = Value::scalar(value); + value_to_usize(&scalar) + }) { + if encoded.len() < width { + let mut padded = String::with_capacity(width); + for _ in 0..(width - encoded.len()) { + padded.push('0'); + } + padded.push_str(&encoded); + encoded = padded; + } + } + + Ok(Value::scalar(encoded)) + } +} + +fn encode_base36(mut value: u64) -> String { + const ALPHABET: &[u8; 36] = b"0123456789abcdefghijklmnopqrstuvwxyz"; + if value == 0 { + return "0".to_string(); + } + let mut buf = Vec::new(); + while value > 0 { + let rem = (value % 36) as usize; + buf.push(ALPHABET[rem] as char); + value /= 36; + } + buf.iter().rev().collect() +} + // {{ value | b64url_enc }} – URL-safe base64 w/o padding static_filter!( /// Base64 URL-safe (no ‘=’ padding). @@ -844,6 +921,7 @@ pub fn register_all(builder: liquid::ParserBuilder) -> liquid::ParserBuilder { .filter(Crc32HexFilter::default()) .filter(Crc32LeB64Filter::default()) .filter(Base62Filter::default()) + .filter(Base36Filter::default()) .filter(HmacSha256::default()) .filter(HmacSha1::default()) .filter(HmacSha384::default()) @@ -911,6 +989,12 @@ mod tests { assert_eq!(render(r#"{{ "hello" | crc32 | base62: 6 }}"#), "0zNvy2"); } + #[test] + fn base36_filter() { + assert_eq!(render(r#"{{ 123456 | base36 }}"#), "2n9c"); + assert_eq!(render(r#"{{ 123456 | base36: 6 }}"#), "002n9c"); + } + #[test] fn crc32_dec_filter() { assert_eq!(render(r#"{{ "hello" | crc32_dec }}"#), "907060870"); 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/reporter.rs b/src/reporter.rs index 48f53d3..4cabba4 100644 --- a/src/reporter.rs +++ b/src/reporter.rs @@ -333,11 +333,11 @@ impl DetailsReporter { } use std::collections::HashMap; - let mut by_fp: HashMap = HashMap::new(); + let mut by_fp: HashMap<(u64, String), ReportMatch> = HashMap::new(); for rm in matches { - let fp = rm.m.finding_fingerprint; - if let Some(existing) = by_fp.get_mut(&fp) { + let key = (rm.m.finding_fingerprint, rm.m.rule.id().to_string()); + if let Some(existing) = by_fp.get_mut(&key) { // merge origin sets (keep first origin, append the rest) for o in rm.origin.iter() { if !existing.origin.iter().any(|e| e == o) { @@ -355,7 +355,7 @@ impl DetailsReporter { } continue; } - by_fp.insert(fp, rm); + by_fp.insert(key, rm); } by_fp.into_values().collect() } diff --git a/src/scanner/runner.rs b/src/scanner/runner.rs index 39decc3..a8b51a8 100644 --- a/src/scanner/runner.rs +++ b/src/scanner/runner.rs @@ -41,8 +41,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 +53,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 +73,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 +290,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..615733f 100644 --- a/src/update.rs +++ b/src/update.rs @@ -23,6 +23,46 @@ 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, +} + +impl Default for UpdateStatus { + fn default() -> Self { + UpdateStatus { + message: None, + styled_message: None, + is_outdated: false, + running_version: cargo_crate_version!().to_string(), + latest_version: None, + check_status: UpdateCheckStatus::Disabled, + } + } +} + fn styled_heading(styles: &Styles, text: &str) -> String { styles.style_finding_active_heading.apply_to(text).to_string() } @@ -31,9 +71,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 +137,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 +247,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 5771cc4..7a4c701 100644 --- a/tests/fingerprint_dedup.rs +++ b/tests/fingerprint_dedup.rs @@ -20,10 +20,10 @@ use kingfisher::{ use smallvec::smallvec; // ---- helpers ------------------------------------------------------------------------------- -fn make_match(fp: u64) -> Match { +fn make_match(fp: u64, rule_id: &str) -> Match { let syntax = RuleSyntax { name: "Example Rule".to_string(), - id: "RULE.1".to_string(), + id: rule_id.to_string(), pattern: "dummy".to_string(), min_entropy: 0.0, confidence: Confidence::Medium, @@ -99,8 +99,8 @@ fn git_origin(commit_id: &str) -> OriginSet { #[test] fn reporter_deduplicates_across_git_commits() -> Result<()> { // Build two matches with the same fingerprint. - let m1 = make_match(0xBADC0FFE); - let m2 = make_match(0xBADC0FFE); + let m1 = make_match(0xBADC0FFE, "RULE.1"); + let m2 = make_match(0xBADC0FFE, "RULE.1"); // Different commit ids -- old dedup logic *fails* to merge them. let origin_a = git_origin("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); @@ -156,3 +156,59 @@ fn reporter_deduplicates_across_git_commits() -> Result<()> { Ok(()) } + +#[test] +fn dedup_preserves_distinct_rules_with_same_fingerprint() -> Result<()> { + let shared_fp = 0xDEADC0DE; + let m1 = make_match(shared_fp, "RULE.OPENAI"); + let m2 = make_match(shared_fp, "RULE.DEEPSEEK"); + + let origin = git_origin("cccccccccccccccccccccccccccccccccccccccc"); + + let reporter = DetailsReporter { + datastore: Arc::new(Mutex::new(FindingsStore::new(PathBuf::from("/tmp")))), + styles: Styles::new(false), + only_valid: false, + }; + + let matches = vec![ + ReportMatch { + origin: origin.clone(), + blob_metadata: BlobMetadata { + id: BlobId::new(b"dummy"), + num_bytes: 10, + mime_essence: None, + language: None, + }, + m: m1, + comment: None, + match_confidence: Confidence::Medium, + visible: true, + validation_response_body: String::new(), + validation_response_status: 0, + validation_success: false, + }, + ReportMatch { + origin, + blob_metadata: BlobMetadata { + id: BlobId::new(b"dummy"), + num_bytes: 10, + mime_essence: None, + language: None, + }, + m: m2, + comment: None, + match_confidence: Confidence::Medium, + visible: true, + validation_response_body: String::new(), + validation_response_status: 0, + validation_success: false, + }, + ]; + + let deduped = reporter.deduplicate_matches(matches, /* no_dedup= */ false); + + assert_eq!(deduped.len(), 2, "matches from distinct rules must not be deduplicated"); + + Ok(()) +} diff --git a/tests/int_allowlist.rs b/tests/int_allowlist.rs index dcd9067..ca813c7 100644 --- a/tests/int_allowlist.rs +++ b/tests/int_allowlist.rs @@ -24,6 +24,7 @@ use kingfisher::{ rule_loader::RuleLoader, rules_database::RulesDatabase, scanner::run_async_scan, + update::UpdateStatus, }; use tempfile::TempDir; use tokio::runtime::Runtime; @@ -165,10 +166,17 @@ fn run_skiplist(skip_regex: Vec, skip_skipword: Vec) -> Result Result<()> { let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); let runtime = Runtime::new()?; let rules_db = Arc::new(load_and_record_rules(&scan_args, &datastore)?); + let update_status = UpdateStatus::default(); runtime.block_on(async { - 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 })?; let ds = datastore.lock().unwrap(); diff --git a/tests/int_dedup.rs b/tests/int_dedup.rs index d059e73..9607ca1 100644 --- a/tests/int_dedup.rs +++ b/tests/int_dedup.rs @@ -28,6 +28,7 @@ use kingfisher::{ rule_loader::RuleLoader, rules_database::RulesDatabase, scanner::run_async_scan, + update::UpdateStatus, }; use tempfile::TempDir; use tokio::runtime::Runtime; @@ -186,6 +187,7 @@ rules: let loaded = RuleLoader::from_rule_specifiers(&scan_args.rules).load(&scan_args)?; let resolved = loaded.resolve_enabled_rules()?; let rules_db = Arc::new(RulesDatabase::from_rules(resolved.into_iter().cloned().collect())?); + let update_status = UpdateStatus::default(); // Fresh FindingsStore for this run let store_path = work.path().join("store"); @@ -198,6 +200,7 @@ rules: &scan_args, Arc::clone(&datastore), &rules_db, + &update_status, ))?; let x = Ok(datastore.lock().unwrap().get_matches().len()); diff --git a/tests/int_github.rs b/tests/int_github.rs index 91af8db..71e2d3a 100644 --- a/tests/int_github.rs +++ b/tests/int_github.rs @@ -24,6 +24,7 @@ use kingfisher::{ findings_store::FindingsStore, git_url::GitUrl, scanner::{load_and_record_rules, run_scan}, + update::UpdateStatus, }; use tempfile::TempDir; use tokio::runtime::Runtime; @@ -174,9 +175,10 @@ fn test_github_remote_scan() -> Result<()> { let runtime = Runtime::new().expect("Failed to create Tokio runtime"); // Load rules let rules_db = Arc::new(load_and_record_rules(&scan_args, &datastore)?); + let update_status = UpdateStatus::default(); // Run the scan using runtime.block_on runtime.block_on(async { - 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 })?; // Get scan results let ds = datastore.lock().unwrap(); diff --git a/tests/int_gitlab.rs b/tests/int_gitlab.rs index 6651fa7..f850a83 100644 --- a/tests/int_gitlab.rs +++ b/tests/int_gitlab.rs @@ -24,6 +24,7 @@ use kingfisher::{ findings_store::FindingsStore, git_url::GitUrl, scanner::{load_and_record_rules, run_scan}, + update::UpdateStatus, }; use tempfile::TempDir; use tokio::runtime::Runtime; @@ -171,9 +172,10 @@ fn test_gitlab_remote_scan() -> Result<()> { let rt = Runtime::new()?; let rules_db = Arc::new(load_and_record_rules(&scan_args, &datastore)?); + let update_status = UpdateStatus::default(); rt.block_on(async { - 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 })?; let ds = datastore.lock().unwrap(); @@ -323,9 +325,10 @@ fn test_gitlab_remote_scan_no_history() -> Result<()> { let rt = Runtime::new()?; let rules_db = Arc::new(load_and_record_rules(&scan_args, &datastore)?); + let update_status = UpdateStatus::default(); rt.block_on(async { - 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 })?; let ds = datastore.lock().unwrap(); diff --git a/tests/int_redact.rs b/tests/int_redact.rs index c885f28..6da5516 100644 --- a/tests/int_redact.rs +++ b/tests/int_redact.rs @@ -24,6 +24,7 @@ use kingfisher::{ rule_loader::RuleLoader, rules_database::RulesDatabase, scanner::run_async_scan, + update::UpdateStatus, }; use tempfile::TempDir; use url::Url; @@ -148,9 +149,11 @@ async fn test_redact_hashes_finding_values() -> Result<()> { let loaded = RuleLoader::from_rule_specifiers(&scan_args.rules).load(&scan_args)?; let resolved = loaded.resolve_enabled_rules()?; let rules_db = RulesDatabase::from_rules(resolved.into_iter().cloned().collect())?; + let update_status = UpdateStatus::default(); let datastore = Arc::new(Mutex::new(FindingsStore::new(temp_dir.path().to_path_buf()))); - run_async_scan(&global_args, &scan_args, Arc::clone(&datastore), &rules_db).await?; + run_async_scan(&global_args, &scan_args, Arc::clone(&datastore), &rules_db, &update_status) + .await?; let ds = datastore.lock().unwrap(); let matches = ds.get_matches(); diff --git a/tests/int_slack.rs b/tests/int_slack.rs index 999a4c0..b391c07 100644 --- a/tests/int_slack.rs +++ b/tests/int_slack.rs @@ -24,6 +24,7 @@ use kingfisher::{ rule_loader::RuleLoader, rules_database::RulesDatabase, scanner::run_async_scan, + update::UpdateStatus, }; use tempfile::TempDir; use url::Url; @@ -294,8 +295,10 @@ async fn test_scan_slack_messages() -> Result<()> { }; let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); + let update_status = UpdateStatus::default(); - run_async_scan(&global_args, &scan_args, Arc::clone(&datastore), &ctx.rules_db).await?; + run_async_scan(&global_args, &scan_args, Arc::clone(&datastore), &ctx.rules_db, &update_status) + .await?; let findings = { let ds = datastore.lock().unwrap(); diff --git a/tests/int_validation_cache.rs b/tests/int_validation_cache.rs index 970ad0a..c509a7d 100644 --- a/tests/int_validation_cache.rs +++ b/tests/int_validation_cache.rs @@ -28,6 +28,7 @@ use kingfisher::{ rule_loader::RuleLoader, rules_database::RulesDatabase, scanner::run_async_scan, + update::UpdateStatus, }; use tempfile::TempDir; use url::Url; @@ -242,8 +243,10 @@ async fn test_validation_cache_and_depvars() -> Result<()> { ignore_certs: false, user_agent_suffix: None, }; + let update_status = UpdateStatus::default(); - run_async_scan(&global_args, &scan_args, Arc::clone(&datastore), &rules_db).await?; + run_async_scan(&global_args, &scan_args, Arc::clone(&datastore), &rules_db, &update_status) + .await?; /* --------------------------------------------------------- * * 6. Assertions * diff --git a/tests/int_vulnerable_files.rs b/tests/int_vulnerable_files.rs index 3e8bc4a..5d67b43 100644 --- a/tests/int_vulnerable_files.rs +++ b/tests/int_vulnerable_files.rs @@ -26,6 +26,7 @@ use kingfisher::{ rule_loader::RuleLoader, rules_database::RulesDatabase, scanner::run_async_scan, + update::UpdateStatus, }; use tempfile::TempDir; use url::Url; @@ -297,8 +298,16 @@ impl TestContext { }; let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); + let update_status = UpdateStatus::default(); - run_async_scan(&global_args, &scan_args, Arc::clone(&datastore), &self.rules_db).await?; + run_async_scan( + &global_args, + &scan_args, + Arc::clone(&datastore), + &self.rules_db, + &update_status, + ) + .await?; let findings = { let ds = datastore.lock().unwrap(); diff --git a/tests/live_db_validation.rs b/tests/live_db_validation.rs index b41c43c..25836ef 100644 --- a/tests/live_db_validation.rs +++ b/tests/live_db_validation.rs @@ -17,7 +17,6 @@ const STARTUP_POLL_INTERVAL: Duration = Duration::from_millis(250); async fn wait_for_port(host: &str, port: u16) -> Result<()> { let deadline = Instant::now() + STARTUP_TIMEOUT; - let mut last_err = None; loop { match TcpStream::connect((host, port)).await { @@ -26,10 +25,9 @@ async fn wait_for_port(host: &str, port: u16) -> Result<()> { return Ok(()); } Err(err) => { - last_err = Some(err); if Instant::now() >= deadline { return Err(anyhow!( - "timed out after {:?} waiting for {host}:{port}: {last_err:?}", + "timed out after {:?} waiting for {host}:{port}: {err}", STARTUP_TIMEOUT, )); } diff --git a/tests/smoke_update.rs b/tests/smoke_update.rs index 8224d42..74b8b31 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,18 @@ 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 + .as_deref() + .expect("update check should return a message") + .contains("99.999.0")); }