From 27ee99208c6e9f6a97641e6a600705d75afe1ca4 Mon Sep 17 00:00:00 2001 From: Fede Barcelona Date: Wed, 29 Oct 2025 10:12:07 +0100 Subject: [PATCH] feat(domain): use string for version The 'semver' crate cannot handle a wide variety of package versions found in the wild, such as Debian versions ('1.1.35-1.2+deb13u2') or other formats like '31.0-api'. This change replaces 'semver::Version' with 'String' to store package versions, and uses the 'version-compare' crate to compare them when suggesting a fix. --- .pre-commit-config.yaml | 1 + Cargo.lock | 9 ++- Cargo.toml | 3 +- Justfile | 1 + src/domain/scanresult/package.rs | 57 ++++++++++++++----- src/domain/scanresult/scan_result.rs | 29 +++++----- src/domain/scanresult/vulnerability.rs | 7 +-- ...ysdig_image_scanner_json_scan_result_v1.rs | 11 +--- tests/general.rs | 7 +-- 9 files changed, 77 insertions(+), 48 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 68df062..450612f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,6 +4,7 @@ repos: hooks: - id: fmt - id: cargo-check + always_run: true - id: clippy - repo: https://github.com/pre-commit/pre-commit-hooks diff --git a/Cargo.lock b/Cargo.lock index 43e75c3..c5214ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2035,7 +2035,7 @@ dependencies = [ [[package]] name = "sysdig-lsp" -version = "0.7.3" +version = "0.7.4" dependencies = [ "async-trait", "bollard", @@ -2063,6 +2063,7 @@ dependencies = [ "tracing", "tracing-subscriber", "tracing-test", + "version-compare", ] [[package]] @@ -2515,6 +2516,12 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" +[[package]] +name = "version-compare" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "852e951cb7832cb45cb1169900d19760cfa39b82bc0ea9c0e5a14ae88411c98b" + [[package]] name = "want" version = "0.3.1" diff --git a/Cargo.toml b/Cargo.toml index 7b963b2..fb55296 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sysdig-lsp" -version = "0.7.3" +version = "0.7.4" edition = "2024" authors = [ "Sysdig Inc." ] readme = "README.md" @@ -33,6 +33,7 @@ tokio = { version = "1.43.0", features = ["full"] } tower-lsp = "0.20.0" tracing = "0.1.41" tracing-subscriber = "0.3.19" +version-compare = "0.2.0" [dev-dependencies] rstest = "0.21.0" diff --git a/Justfile b/Justfile index 7eb6c3e..3cae87e 100644 --- a/Justfile +++ b/Justfile @@ -4,6 +4,7 @@ test: fix: cargo fix --allow-staged --allow-dirty + cargo machete --fix fmt: cargo fmt diff --git a/src/domain/scanresult/package.rs b/src/domain/scanresult/package.rs index dc8df95..597b5bd 100644 --- a/src/domain/scanresult/package.rs +++ b/src/domain/scanresult/package.rs @@ -4,7 +4,7 @@ use crate::domain::scanresult::package_type::PackageType; use crate::domain::scanresult::severity::Severity; use crate::domain::scanresult::vulnerability::Vulnerability; use crate::domain::scanresult::weak_hash::WeakHash; -use semver::Version; +use std::cmp::Ordering; use std::collections::{HashMap, HashSet}; use std::fmt::Debug; use std::hash::{Hash, Hasher}; @@ -13,7 +13,7 @@ use std::sync::{Arc, RwLock}; pub struct Package { package_type: PackageType, name: String, - version: Version, + version: String, path: String, found_in_layer: Arc, vulnerabilities: RwLock>>, @@ -36,7 +36,7 @@ impl Package { pub(in crate::domain::scanresult) fn new( package_type: PackageType, name: String, - version: Version, + version: String, path: String, found_in_layer: Arc, ) -> Self { @@ -59,7 +59,7 @@ impl Package { &self.name } - pub fn version(&self) -> &Version { + pub fn version(&self) -> &String { &self.version } @@ -111,13 +111,13 @@ impl Package { .collect() } - pub fn suggested_fix_version(&self) -> Option { + pub fn suggested_fix_version(&self) -> Option { let vulnerabilities = self.vulnerabilities(); if vulnerabilities.is_empty() { return None; } - let candidate_versions: Vec = vulnerabilities + let candidate_versions: Vec = vulnerabilities .iter() .filter_map(|vuln| vuln.fix_version().cloned()) .collect::>() @@ -137,7 +137,7 @@ impl Package { Severity::Unknown, ]; - let mut scores: HashMap> = HashMap::new(); + let mut scores: HashMap> = HashMap::new(); for candidate in &candidate_versions { let mut score: HashMap = HashMap::new(); @@ -168,7 +168,16 @@ impl Package { } // If scores are identical, lower version is better - a.cmp(b) + if version_compare::compare_to(a, b, version_compare::Cmp::Eq).unwrap_or(false) { + return Ordering::Equal; + } + if version_compare::compare_to(a, b, version_compare::Cmp::Le).unwrap_or(false) { + return Ordering::Less; + } + if version_compare::compare_to(a, b, version_compare::Cmp::Ge).unwrap_or(false) { + return Ordering::Greater; + } + Ordering::Less }); sorted_candidates.first().cloned() @@ -218,7 +227,6 @@ mod tests { use crate::domain::scanresult::vulnerability::Vulnerability; use chrono::NaiveDate; use rstest::{fixture, rstest}; - use semver::Version; use std::sync::Arc; #[fixture] @@ -236,7 +244,7 @@ mod tests { Arc::new(Package::new( PackageType::Os, "a_name".to_string(), - Version::parse(version).unwrap(), + version.to_string(), "a_path".to_string(), layer, )) @@ -253,7 +261,7 @@ mod tests { NaiveDate::from_ymd_opt(2023, 1, 1).unwrap(), None, false, - fix_version.map(|v| Version::parse(v).unwrap()), + fix_version.map(|v| v.to_string()), )) } @@ -293,6 +301,25 @@ mod tests { a_vulnerability("CVE-2022-41724", Severity::Medium, Some("2.9.0")), a_vulnerability("CVE-2022-41725", Severity::Medium, Some("2.9.0")), ], Some("2.9.0"))] + #[case("handles_debian_version", "1.1.35-1.2+deb13u2", vec![ + a_vulnerability("CVE-1", Severity::High, Some("1.1.35-1.2+deb13u3")), + a_vulnerability("CVE-2", Severity::High, Some("1.1.35-1.3")), + ], Some("1.1.35-1.2+deb13u3"))] + #[case("chooses_lower_version_with_debian_tilde", "257.8-1~deb13u1", vec![ + a_vulnerability("CVE-1", Severity::High, Some("257.8-1~deb13u2")), + a_vulnerability("CVE-2", Severity::High, Some("257.8-1~deb13u3")), + ], Some("257.8-1~deb13u2"))] + #[case("handles_jre_and_android_versions", "31.1-jre", vec![ + a_vulnerability("CVE-1", Severity::High, Some("32.0.0-android")), + ], Some("32.0.0-android"))] + #[case("handles_api_version", "31.0-api", vec![a_vulnerability("CVE-1", Severity::High, Some("31.1-api"))], Some("31.1-api"))] + #[case("handles_build_metadata_version", "1.0.15-1+b3", vec![a_vulnerability("CVE-1", Severity::High, Some("1.0.15-2"))], Some("1.0.15-2"))] + #[case("handles_simple_float_version", "2.6", vec![a_vulnerability("CVE-1", Severity::High, Some("2.7"))], Some("2.7"))] + #[case("handles_revision_version", "1.7.0-5", vec![a_vulnerability("CVE-1", Severity::High, Some("1.7.0-6"))], Some("1.7.0-6"))] + #[case("handles_date_based_version", "6.5+20250216-2", vec![a_vulnerability("CVE-1", Severity::High, Some("6.5+20250216-3"))], Some("6.5+20250216-3"))] + #[case("handles_jenkins_version", "3107.v665000b_51092", vec![a_vulnerability("CVE-1", Severity::High, Some("3107.v665000b_51093"))], Some("3107.v665000b_51093"))] + #[case("handles_dot_separated_version", "3206.3208", vec![a_vulnerability("CVE-1", Severity::High, Some("3206.3209"))], Some("3206.3209"))] + #[case("handles_complex_debian_version", "2.12.7+dfsg+really2.9.14-2.1+deb13u1", vec![a_vulnerability("CVE-1", Severity::High, Some("2.12.7+dfsg+really2.9.14-2.1+deb13u2"))], Some("2.12.7+dfsg+really2.9.14-2.1+deb13u2"))] fn test_suggested_fix_version( #[case] _description: &str, #[case] version: &str, @@ -300,13 +327,15 @@ mod tests { #[case] vulnerabilities: Vec>, #[case] expected_fix: Option<&str>, ) { - assert_eq!(package.version(), &Version::parse(version).unwrap()); + assert_eq!(package.version(), &version); for vuln in &vulnerabilities { package.add_vulnerability_found(vuln.clone()); } - let expected = expected_fix.map(|v| Version::parse(v).unwrap()); - assert_eq!(package.suggested_fix_version(), expected); + assert_eq!( + package.suggested_fix_version(), + expected_fix.map(|x| x.to_string()) + ); } } diff --git a/src/domain/scanresult/scan_result.rs b/src/domain/scanresult/scan_result.rs index 51f4d14..5cad63f 100644 --- a/src/domain/scanresult/scan_result.rs +++ b/src/domain/scanresult/scan_result.rs @@ -14,7 +14,6 @@ use crate::domain::scanresult::severity::Severity; use crate::domain::scanresult::vulnerability::Vulnerability; use chrono::{DateTime, NaiveDate, Utc}; use itertools::Itertools; -use semver::Version; use std::collections::HashMap; use std::sync::Arc; @@ -110,7 +109,7 @@ impl ScanResult { &mut self, package_type: PackageType, name: String, - version: Version, + version: String, path: String, found_in_layer: Arc, ) -> Arc { @@ -141,7 +140,7 @@ impl ScanResult { disclosure_date: NaiveDate, solution_date: Option, exploitable: bool, - fix_version: Option, + fix_version: Option, ) -> Arc { self.vulnerabilities .entry(cve.clone()) @@ -314,7 +313,7 @@ mod tests { let package = scan_result.add_package( PackageType::Os, "musl".to_string(), - Version::parse("1.2.3").unwrap(), + "1.2.3".to_string(), "/lib/ld-musl-x86_64.so.1".to_string(), layer.clone(), ); @@ -337,7 +336,7 @@ mod tests { Utc::now().naive_utc().date(), None, false, - Some(Version::parse("1.2.4").unwrap()), + Some("1.2.4".to_string()), ); assert_eq!(scan_result.vulnerabilities().len(), 1); @@ -358,7 +357,7 @@ mod tests { let package = scan_result.add_package( PackageType::Os, "musl".to_string(), - Version::parse("1.2.3").unwrap(), + "1.2.3".to_string(), "/lib/ld-musl-x86_64.so.1".to_string(), layer.clone(), ); @@ -368,7 +367,7 @@ mod tests { Utc::now().naive_utc().date(), None, false, - Some(Version::parse("1.2.4").unwrap()), + Some("1.2.4".to_string()), ); package.add_vulnerability_found(vuln.clone()); @@ -463,7 +462,7 @@ mod tests { Utc::now().naive_utc().date(), None, false, - Some(Version::parse("1.2.4").unwrap()), + Some("1.2.4".to_string()), ); vuln.add_accepted_risk(risk.clone()); @@ -489,7 +488,7 @@ mod tests { let package = scan_result.add_package( PackageType::Os, "musl".to_string(), - Version::parse("1.2.3").unwrap(), + "1.2.3".to_string(), "/lib/ld-musl-x86_64.so.1".to_string(), layer.clone(), ); @@ -572,13 +571,13 @@ mod tests { let package = scan_result.add_package( PackageType::Os, "musl".to_string(), - Version::parse("1.2.3").unwrap(), + "1.2.3".to_string(), "/path".to_string(), layer.clone(), ); assert_eq!(package.package_type(), &PackageType::Os); assert_eq!(package.name(), "musl"); - assert_eq!(package.version(), &Version::parse("1.2.3").unwrap()); + assert_eq!(package.version(), "1.2.3"); assert_eq!(package.path(), "/path"); assert!(format!("{:?}", package).contains("musl")); assert_eq!(package.clone(), package); @@ -590,7 +589,7 @@ mod tests { now.naive_utc().date(), Some(now.naive_utc().date()), true, - Some(Version::parse("1.2.4").unwrap()), + Some("1.2.4".to_string()), ); assert_eq!(vuln.cve(), "CVE-1"); assert_eq!(vuln.severity(), Severity::High); @@ -598,7 +597,7 @@ mod tests { assert_eq!(vuln.solution_date(), Some(now.naive_utc().date())); assert!(vuln.exploitable()); assert!(vuln.fixable()); - assert_eq!(vuln.fix_version(), Some(&Version::parse("1.2.4").unwrap())); + assert_eq!(vuln.fix_version(), Some(&"1.2.4".to_string())); assert!(format!("{:?}", vuln).contains("CVE-1")); // AcceptedRisk @@ -673,14 +672,14 @@ mod tests { let pkg = scan_result.add_package( PackageType::Os, "pkg".to_string(), - Version::parse("1.0.0").unwrap(), + "1.0.0".to_string(), "/path".to_string(), layer.clone(), ); let pkg2 = scan_result.add_package( PackageType::Os, "pkg".to_string(), - Version::parse("1.0.0").unwrap(), + "1.0.0".to_string(), "/path".to_string(), layer.clone(), ); diff --git a/src/domain/scanresult/vulnerability.rs b/src/domain/scanresult/vulnerability.rs index 79b83d8..d1fb46b 100644 --- a/src/domain/scanresult/vulnerability.rs +++ b/src/domain/scanresult/vulnerability.rs @@ -4,7 +4,6 @@ use crate::domain::scanresult::package::Package; use crate::domain::scanresult::severity::Severity; use crate::domain::scanresult::weak_hash::WeakHash; use chrono::NaiveDate; -use semver::Version; use std::collections::HashSet; use std::fmt::Debug; use std::hash::{Hash, Hasher}; @@ -16,7 +15,7 @@ pub struct Vulnerability { disclosure_date: NaiveDate, solution_date: Option, exploitable: bool, - fix_version: Option, + fix_version: Option, found_in_packages: RwLock>>, accepted_risks: RwLock>>, } @@ -41,7 +40,7 @@ impl Vulnerability { disclosure_date: NaiveDate, solution_date: Option, exploitable: bool, - fix_version: Option, + fix_version: Option, ) -> Self { Self { cve, @@ -79,7 +78,7 @@ impl Vulnerability { self.fix_version.is_some() } - pub fn fix_version(&self) -> Option<&Version> { + pub fn fix_version(&self) -> Option<&String> { self.fix_version.as_ref() } diff --git a/src/infra/sysdig_image_scanner_json_scan_result_v1.rs b/src/infra/sysdig_image_scanner_json_scan_result_v1.rs index 5302072..bd96f9f 100644 --- a/src/infra/sysdig_image_scanner_json_scan_result_v1.rs +++ b/src/infra/sysdig_image_scanner_json_scan_result_v1.rs @@ -13,7 +13,6 @@ use crate::domain::scanresult::{ scan_type::ScanType, severity::Severity, }; -use semver::Version; impl From for ScanResult { fn from(report: JsonScanResultV1) -> Self { @@ -56,15 +55,13 @@ fn add_risk_accepts(result: &JsonResult, scan_result: &mut ScanResult) { fn add_vulnerabilities(result: &JsonResult, scan_result: &mut ScanResult) { for v in result.vulnerabilities.values() { - let fix_version = v.fix_version.as_ref().and_then(|s| Version::parse(s).ok()); - let vuln = scan_result.add_vulnerability( v.name.clone(), v.severity.clone().into(), v.disclosure_date, v.solution_date, v.exploitable, - fix_version, + v.fix_version.clone(), ); v.risk_accept_refs @@ -89,14 +86,10 @@ fn add_packages(result: &JsonResult, scan_result: &mut ScanResult) { continue; }; - let Ok(version) = Version::parse(&json_pkg.version) else { - continue; - }; - let pkg = scan_result.add_package( json_pkg.package_type.clone().into(), json_pkg.name.clone(), - version, + json_pkg.version.clone(), json_pkg.path.clone(), layer_where_this_package_is_found, ); diff --git a/tests/general.rs b/tests/general.rs index 0a3f33e..a5fd65b 100644 --- a/tests/general.rs +++ b/tests/general.rs @@ -2,7 +2,6 @@ mod common; use common::TestSetup; use rstest::{fixture, rstest}; -use semver::Version; use serde_json::json; use std::collections::HashMap; use sysdig_lsp::domain::scanresult::architecture::Architecture; @@ -135,7 +134,7 @@ fn scan_result() -> ScanResult { let package1 = result.add_package( PackageType::Os, "package1".to_string(), - Version::parse("1.0.0").unwrap(), + "1.0.0".to_string(), "/usr/lib/package1".to_string(), layer.clone(), ); @@ -143,7 +142,7 @@ fn scan_result() -> ScanResult { result.add_package( PackageType::Os, "package2".to_string(), - Version::parse("2.0.0").unwrap(), + "2.0.0".to_string(), "/usr/lib/package2".to_string(), layer, ); @@ -154,7 +153,7 @@ fn scan_result() -> ScanResult { chrono::NaiveDate::from_ymd_opt(2021, 1, 1).unwrap(), None, false, - Some(Version::parse("1.0.1").unwrap()), + Some("1.0.1".to_string()), ); package1.add_vulnerability_found(vulnerability);