Merge pull request #3 from MindPatch/feature/comprehensive-fixes

🚀 Comprehensive code quality and security fixes

Author: MindPatch

Cargo.lock

@@ -81,6 +81,9 @@ regex = "1.10"
 # URL handling for webhooks
 url = "2.4"
 
+# Semantic version parsing for vulnerability matching
+semver = "1.0"
+
 [dev-dependencies]
 tokio-test = "0.4"
 tempfile = "3.8"

Cargo.toml

@@ -193,7 +193,7 @@ impl GitMonitor {
 
         // Create scan configuration
         let scan_config = ScanConfig {
-            target_path: repo_dir.clone(),
+            target_path: repo_dir.to_path_buf(),
             output_file: None,
             recursive: true,
             ecosystems: repo_config.ecosystems.clone(),

src/automation/git_monitor.rs

@@ -182,7 +182,7 @@ impl PolicyEngine {
         // This is a best-effort approach since vulnerability doesn't directly reference packages
         // We'll try to match based on package names mentioned in the vulnerability
 
-        for (_package_key, package) in package_map {
+        for package in package_map.values() {
             // Check if package name is mentioned in vulnerability summary
             let package_name = &package.name;
 

src/automation/policy.rs

@@ -221,49 +221,75 @@ fn should_notify(
 ) -> bool {
     let result = &filtered_result.scan_result;
 
+    // Check if there are any vulnerabilities first
+    if result.vulnerabilities.is_empty() {
+        info!("No vulnerabilities found, skipping notification");
+        return false;
+    }
+
     // Check minimum severity
     if let Some(min_severity) = &filters.min_severity {
         let has_qualifying_severity = result.vulnerabilities.iter().any(|v| {
             if let Some(severity) = &v.severity {
-                severity_level(severity) >= severity_level(min_severity)
+                let level = severity_level(severity);
+                let min_level = severity_level(min_severity);
+                info!("Checking severity: '{}' (level {}) >= '{}' (level {})", 
+                     severity, level, min_severity, min_level);
+                level >= min_level
             } else {
                 false
             }
         });
 
         if !has_qualifying_severity {
+            info!("No vulnerabilities meet minimum severity requirement of '{}'", min_severity);
             return false;
         }
     }
 
     // Check repository filter
     if let Some(repos) = &filters.repositories {
         if !repos.contains(&result.repository) {
+            info!("Repository '{}' not in notification filter list", result.repository);
             return false;
         }
     }
 
-    // Check if there are any vulnerabilities
-    if result.vulnerabilities.is_empty() {
-        return false;
-    }
-
-    // If only_new_vulnerabilities is true, we'd need to compare with previous scan
-    // For now, we'll treat all vulnerabilities as "new" since we don't have persistent storage yet
+    // For only_new_vulnerabilities filter:
+    // If we don't have persistent storage to compare against previous scans,
+    // we'll be more lenient and allow notifications for significant findings
     if filters.only_new_vulnerabilities {
         // Note: In a full implementation, this would compare against stored previous scan results
-        // from a database. For now, we consider all vulnerabilities as potentially new.
-        if result.vulnerabilities.is_empty() {
-            return false;
-        }
+        // For now, we'll allow notifications if there are any vulnerabilities, since we can't
+        // reliably determine what's "new" without persistent storage
+        info!("only_new_vulnerabilities=true, but no previous scan data available. Allowing notification for {} vulnerabilities.", result.vulnerabilities.len());
     }
 
+    info!("Notification criteria met: {} vulnerabilities found", result.vulnerabilities.len());
     true
 }
 
 /// Convert severity string to numeric level for comparison
 fn severity_level(severity: &str) -> u8 {
-    match severity.to_lowercase().as_str() {
+    let severity_lower = severity.to_lowercase();
+    
+    // Handle CVSS format (e.g., "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H")
+    if severity_lower.starts_with("cvss:") {
+        // For CVSS format, determine severity based on the score components
+        // C:H/I:H/A:H indicates High impact across all three categories
+        if severity.contains("C:H") && severity.contains("I:H") && severity.contains("A:H") {
+            return 4; // Critical - High impact on all three (Confidentiality, Integrity, Availability)
+        } else if severity.contains("C:H") || severity.contains("I:H") || severity.contains("A:H") {
+            return 3; // High - High impact on at least one category
+        } else if severity.contains("C:M") || severity.contains("I:M") || severity.contains("A:M") {
+            return 2; // Medium - Medium impact
+        } else {
+            return 1; // Low - Low or no significant impact
+        }
+    }
+    
+    // Handle simple severity strings
+    match severity_lower.as_str() {
         s if s.contains("critical") => 4,
         s if s.contains("high") => 3,
         s if s.contains("medium") => 2,

src/automation/scheduler.rs

@@ -39,12 +39,22 @@ impl WebhookNotifier {
 
     /// Send notification to a specific webhook
     async fn send_webhook_notification(&self, webhook: &Webhook, message: &NotificationMessage) -> Result<()> {
+        // Validate webhook URL before attempting to send
+        if webhook.url.contains("YOUR_WEBHOOK_ID") || webhook.url.contains("YOUR_WEBHOOK_TOKEN") {
+            return Err(anyhow::anyhow!(
+                "Webhook '{}' has placeholder URL. Please update with actual webhook URL from Discord/Slack.", 
+                webhook.name
+            ));
+        }
+
         let payload = match webhook.webhook_type {
             WebhookType::Discord => self.create_discord_payload(message),
             WebhookType::Slack => self.create_slack_payload(message),
             WebhookType::Generic => self.create_generic_payload(message),
         };
 
+        info!("Sending notification to {}: {}", webhook.name, message.title);
+        
         let response = self.client
             .post(&webhook.url)
             .json(&payload)
@@ -53,10 +63,12 @@ impl WebhookNotifier {
             .await?;
 
         if response.status().is_success() {
+            info!("Successfully sent notification to {}", webhook.name);
             Ok(())
         } else {
             let status = response.status();
             let body = response.text().await.unwrap_or_default();
+            error!("Webhook request failed for {}: status {}, body: {}", webhook.name, status, body);
             Err(anyhow::anyhow!("Webhook request failed with status {}: {}", status, body))
         }
     }
@@ -224,15 +236,15 @@ impl WebhookNotifier {
 
         // Determine overall severity - use severity field directly
         let severity = if current_scan.vulnerabilities.iter().any(|v| 
-            v.severity.as_ref().map_or(false, |s| s.to_lowercase().contains("critical"))
+            v.severity.as_ref().is_some_and(|s| s.to_lowercase().contains("critical"))
         ) {
             "critical"
         } else if current_scan.vulnerabilities.iter().any(|v| 
-            v.severity.as_ref().map_or(false, |s| s.to_lowercase().contains("high"))
+            v.severity.as_ref().is_some_and(|s| s.to_lowercase().contains("high"))
         ) {
             "high"
         } else if current_scan.vulnerabilities.iter().any(|v| 
-            v.severity.as_ref().map_or(false, |s| s.to_lowercase().contains("medium"))
+            v.severity.as_ref().is_some_and(|s| s.to_lowercase().contains("medium"))
         ) {
             "medium"
         } else {

src/automation/webhooks.rs

@@ -172,7 +172,7 @@ impl Cli {
                             .target_path(path)
                             .output_file(output)
                             .recursive(!no_recursive)
-                            .ecosystems(ecosystems.map(|e| parse_ecosystems(e)).transpose()?)
+                            .ecosystems(ecosystems.map(parse_ecosystems).transpose()?)
                             .include_dev_dependencies(!no_dev_deps)
                             .format(format)
                             .quiet(quiet)
@@ -579,7 +579,7 @@ async fn execute_automation_run(config_path: PathBuf, workspace: PathBuf, reposi
 
         for scan_result in &results {
             // Group vulnerabilities by package name
-            let mut package_vulns: std::collections::HashMap<String, Vec<crate::types::Vulnerability>> = std::collections::HashMap::new();
+            let mut vulnerabilities_by_package: std::collections::HashMap<String, Vec<crate::types::Vulnerability>> = std::collections::HashMap::new();
 
             for vulnerability in &scan_result.vulnerabilities {
                 // Extract package name from vulnerability ID (often in format "package-name@version")
@@ -593,11 +593,11 @@ async fn execute_automation_run(config_path: PathBuf, workspace: PathBuf, reposi
                     references: vulnerability.references.clone(),
                 };
 
-                package_vulns.entry(package_name).or_insert_with(Vec::new).push(vuln);
+                vulnerabilities_by_package.entry(package_name).or_default().push(vuln);
             }
 
             // Create PackageVulnerability entries
-            for (package_name, vulns) in package_vulns {
+            for (package_name, vulns) in vulnerabilities_by_package {
                 let package_vuln = crate::types::PackageVulnerability {
                     package: crate::types::Package {
                         name: package_name,

src/cli.rs

@@ -28,6 +28,9 @@ pub enum VulfyError {
     #[error("Package parsing error: {message}")]
     PackageParsing { message: String },
 
+    #[error("Version parsing error: {message}")]
+    VersionParsing { message: String },
+
     #[error("OSV API error: {message}")]
     OsvApi { message: String },