feat(domain): Use String for Package Versions to Support Non-SemVer Formats (#27)

The `semver` crate is strict and cannot parse many package versions
found in the wild, such as those from Debian (e.g.,
`1.1.35-1.2+deb13u2`) or other non-standard formats (e.g., `31.0-api`,
`32.0.0-android`). This caused panics when the application encountered
these versions.

This PR resolves the issue by:

1. Replacing the `semver::Version` type with a simple `String` for
storing package versions.
2. Introducing the `version-compare` crate, which is more flexible and
capable of comparing different versioning schemes.
3. Expanding the test suite to validate the correct behavior for a wide
range of version formats, ensuring the suggested fix logic is robust.

Author: tembleking

.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

Cargo.lock

@@ -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"

Cargo.toml

@@ -4,6 +4,7 @@ test:
 
 fix:
     cargo fix --allow-staged --allow-dirty
+    cargo machete --fix
 
 fmt:
     cargo fmt

Justfile

@@ -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<Layer>,
     vulnerabilities: RwLock<HashSet<WeakHash<Vulnerability>>>,
@@ -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<Layer>,
     ) -> 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<Version> {
+    pub fn suggested_fix_version(&self) -> Option<String> {
         let vulnerabilities = self.vulnerabilities();
         if vulnerabilities.is_empty() {
             return None;
         }
 
-        let candidate_versions: Vec<Version> = vulnerabilities
+        let candidate_versions: Vec<String> = vulnerabilities
             .iter()
             .filter_map(|vuln| vuln.fix_version().cloned())
             .collect::<HashSet<_>>()
@@ -137,7 +137,7 @@ impl Package {
             Severity::Unknown,
         ];
 
-        let mut scores: HashMap<Version, HashMap<Severity, usize>> = HashMap::new();
+        let mut scores: HashMap<String, HashMap<Severity, usize>> = HashMap::new();
 
         for candidate in &candidate_versions {
             let mut score: HashMap<Severity, usize> = 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,20 +301,41 @@ 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,
         #[with(version)] package: Arc<Package>,
         #[case] vulnerabilities: Vec<Arc<Vulnerability>>,
         #[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())
+        );
     }
 }

src/domain/scanresult/package.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<Layer>,
     ) -> Arc<Package> {
@@ -141,7 +140,7 @@ impl ScanResult {
         disclosure_date: NaiveDate,
         solution_date: Option<NaiveDate>,
         exploitable: bool,
-        fix_version: Option<Version>,
+        fix_version: Option<String>,
     ) -> Arc<Vulnerability> {
         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,15 +589,15 @@ 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);
         assert_eq!(vuln.disclosure_date(), now.naive_utc().date());
         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(),
         );

src/domain/scanresult/scan_result.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<NaiveDate>,
     exploitable: bool,
-    fix_version: Option<Version>,
+    fix_version: Option<String>,
     found_in_packages: RwLock<HashSet<WeakHash<Package>>>,
     accepted_risks: RwLock<HashSet<WeakHash<AcceptedRisk>>>,
 }
@@ -41,7 +40,7 @@ impl Vulnerability {
         disclosure_date: NaiveDate,
         solution_date: Option<NaiveDate>,
         exploitable: bool,
-        fix_version: Option<Version>,
+        fix_version: Option<String>,
     ) -> 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()
     }