fix(stuff): working on next release

♬ Electric Callboy - Revery

Author: Mte90

src/agent/sandbox.rs

@@ -121,89 +121,27 @@ pub struct ToolSandbox {
 
 impl SandboxLike for ToolSandbox {
     fn temp_dir(&self) -> &Path {
-        &self.temp_dir
+        ToolSandbox::temp_dir(self)
     }
     fn resolve_safe_path(&self, path: &str) -> Result<PathBuf, String> {
-        let full = validate_path_containment(&self.temp_dir, path)?;
-        if !full.exists() {
-            return Err(format!("Path does not exist: {}", path));
-        }
-        Ok(full)
+        ToolSandbox::resolve_safe_path(self, path)
     }
     fn run_with_timeout(
         &self,
         cmd: &str,
         args: &[&str],
         timeout_secs: Option<u64>,
     ) -> Result<ToolResult, String> {
-        let dur = Duration::from_secs(timeout_secs.unwrap_or(self.timeout_secs));
-        let child = unsafe {
-            Command::new(cmd)
-                .args(args)
-                .stdout(Stdio::piped())
-                .stderr(Stdio::piped())
-                .pre_exec(disable_core_dumps)
-                .spawn()
-        }
-        .map_err(|e| format!("spawn {}: {}", cmd, e))?;
-        let dur = child
-            .wait_with_timeout(dur)
-            .map_err(|e| format!("timeout/err: {}", e))?;
-        let ec = dur.status.code().unwrap_or(-1);
-        let out = format!(
-            "{}{}",
-            String::from_utf8_lossy(&dur.stdout),
-            String::from_utf8_lossy(&dur.stderr)
-        );
-        // Trim whitespace from output for reliable matching
-        Ok(ToolResult {
-            tool_call_id: "sandbox".to_string(),
-            success: ec == 0,
-            output: out.trim().to_string(),
-        })
+        ToolSandbox::run_with_timeout(self, cmd, args, timeout_secs)
     }
     fn validate_test_source(&self, path: &str, content: &str) -> Result<(), String> {
-        validate_test_source_content(path, content)
+        ToolSandbox::validate_test_source(self, path, content)
     }
     fn create_temp_file(&self, path: &str, content: &str) -> Result<PathBuf, String> {
-        self.validate_test_source(path, content)
-            .map_err(|e| format!("Validation failed: {}", e))?;
-
-        // Pre-condition: validate path containment before write
-        let full = validate_path_containment(&self.temp_dir, path)?;
-
-        // Write the file
-        std::fs::File::create(&full)
-            .and_then(|mut f| f.write_all(content.as_bytes()))
-            .map_err(|e| format!("Write failed: {}", e))?;
-
-        // Post-condition: verify containment after write (catches symlink swaps)
-        let canonical_after = full
-            .canonicalize()
-            .map_err(|e| format!("Cannot canonicalize after write {:?}: {}", full, e))?;
-        let canonical_root = self
-            .temp_dir
-            .canonicalize()
-            .map_err(|e| format!("Cannot canonicalize sandbox root: {}", e))?;
-        if !canonical_after.starts_with(&canonical_root) {
-            // File was created via symlink outside sandbox - delete and error
-            let _ = std::fs::remove_file(&full);
-            return Err(format!("Path escapes sandbox after write: {}", path));
-        }
-
-        Ok(full)
+        ToolSandbox::create_temp_file(self, path, content)
     }
     fn is_path_allowed(&self, path: &Path) -> bool {
-        let canonical = match path.canonicalize() {
-            Ok(p) => p,
-            Err(_) => {
-                return match path.parent() {
-                    Some(parent) => parent.starts_with(&self.temp_dir),
-                    None => false,
-                };
-            }
-        };
-        canonical.starts_with(&self.temp_dir)
+        ToolSandbox::is_path_allowed(self, path)
     }
 }
 

src/agent/tools.rs

@@ -69,6 +69,7 @@ impl Tool for PatternSearchTool {
         let output = std::process::Command::new("grep")
             .arg("-rn")
             .arg(format!("-C{}", ctx))
+            .arg("--")
             .arg(pattern)
             .arg(path)
             .output()

src/report/ai_aggregation.rs

@@ -216,9 +216,16 @@ impl AiAggregationPhase {
             // Check for CWE mismatches
             let cwes: Vec<_> = findings.iter().filter_map(|f| f.cwe_id.as_ref()).collect();
             if cwes.iter().collect::<std::collections::HashSet<_>>().len() > 1 {
-                let conflict =
-                    conflict_resolver::ConflictResolver::resolve_cwe_conflict(location, findings);
-                conflicts.push(conflict);
+                match conflict_resolver::ConflictResolver::resolve_cwe_conflict(location, findings)
+                {
+                    Ok(conflict) => {
+                        conflicts.push(conflict);
+                    }
+                    Err(e) => {
+                        // Log the error but continue - findings with invalid CWEs are incomparable
+                        tracing::warn!("CWE conflict resolution skipped at {}: {}", location, e);
+                    }
+                }
                 continue;
             }
 

src/report/ai_aggregation/conflict_resolver.rs

@@ -1,5 +1,6 @@
 //! Conflict resolution logic for AI aggregation
 
+use super::ScanError;
 use super::models::*;
 use crate::findings::{Severity, VulnerabilityFinding};
 
@@ -49,10 +50,36 @@ impl ConflictResolver {
     }
 
     /// Resolve CWE conflict by keeping most specific (lowest CWE number)
+    ///
+    /// Returns an error if any finding has an unparseable CWE ID.
+    /// Unparseable CWEs indicate data corruption or model hallucination;
+    /// silently sorting them last biases conflict resolution toward findings
+    /// with parseable CWEs, which is worse than failing explicitly.
     pub fn resolve_cwe_conflict(
         location: &str,
         findings: &[&VulnerabilityFinding],
-    ) -> FindingConflict {
+    ) -> Result<FindingConflict, ScanError> {
+        // Validate all CWEs first - fail fast on unparseable values
+        for (i, finding) in findings.iter().enumerate() {
+            if let Some(cwe) = &finding.cwe_id {
+                // Check if CWE is parseable (format: CWE-NNN or just NNN)
+                let cwe_num = cwe.trim_start_matches("CWE-").parse::<u32>();
+
+                if cwe_num.is_err() {
+                    return Err(ScanError::Parse {
+                        message: format!(
+                            "CWE conflict resolution failed: finding {} has unparseable CWE '{}' at {} \
+                             - findings with invalid CWEs are not comparable",
+                            i + 1,
+                            cwe,
+                            location
+                        ),
+                        source: None,
+                    });
+                }
+            }
+        }
+
         let mut sorted: Vec<_> = findings.iter().collect();
         sorted.sort_by(|a, b| {
             let cwe_num = |cwe: &Option<String>| {
@@ -67,7 +94,7 @@ impl ConflictResolver {
         let conflict_type = ConflictType::CweMismatch;
         let resolution = ConflictResolution::KeptOne;
 
-        FindingConflict {
+        Ok(FindingConflict {
             findings: findings.iter().map(|f| (*f).clone()).collect(),
             conflict_type,
             resolution,
@@ -76,7 +103,7 @@ impl ConflictResolver {
                 most_specific.cwe_id.as_deref().unwrap_or("unknown"),
                 location
             ),
-        }
+        })
     }
 
     /// Resolve confidence conflict by averaging

src/semgrep/parser.rs

@@ -136,18 +136,16 @@ pub fn parse_json_output(
             .get("extra")
             .and_then(|e| e.get("severity"))
             .and_then(|s| s.as_str());
+        let keyword_severity = parse_severity(&check_id);
         let base_severity = match extra_severity {
             Some("ERROR") => Severity::High,
             Some("WARNING") => Severity::Medium,
             Some("INFO") | Some("INVENTORY") => Severity::Low,
-            _ => parse_severity(&check_id),
-        };
-        let keyword_severity = parse_severity(&check_id);
-        let severity = if keyword_severity > base_severity {
-            keyword_severity
-        } else {
-            base_severity
+            _ => keyword_severity.unwrap_or_default(),
         };
+        let severity = keyword_severity
+            .filter(|ks| *ks > base_severity)
+            .unwrap_or(base_severity);
 
         let cwe_id = result
             .get("extra")

src/semgrep/rules.rs

@@ -1,8 +1,13 @@
 use crate::findings::Severity;
 
-/// Default per-run semgrep timeout (client side) in seconds.
+/// Per-file semgrep timeout in seconds (passed to `--timeout` flag).
 pub const DEFAULT_SEMGREP_TIMEOUT_SECS: u64 = 300;
 
+/// Wall-clock timeout for the entire semgrep run in seconds.
+/// Separate from `DEFAULT_SEMGREP_TIMEOUT_SECS` to avoid aborting
+/// large projects that legitimately need more total time.
+pub const DEFAULT_SEMGREP_WALL_CLOCK_SECS: u64 = 1800;
+
 /// Bundled taint-mode rules for Python (derived from presets/oss-python.toml)
 const TAINT_PYTHON_RULES: &str = include_str!("rules/taint-python.yml");
 
@@ -31,8 +36,10 @@ const TAINT_CSHARP_RULES: &str = include_str!("rules/taint-csharp.yml");
 pub struct SemgrepRunner {
     pub rulesets: Vec<String>,
     pub exclude_rules: Vec<String>,
-    /// Per-run timeout: passed to semgrep `--timeout` and enforced client-side.
+    /// Per-file timeout: passed to semgrep `--timeout` flag.
     pub timeout_secs: u64,
+    /// Wall-clock timeout for the entire semgrep run (client-side).
+    pub wall_clock_timeout_secs: u64,
     /// Inline rule YAML documents (preset `custom_rules`), materialized to
     /// temp .yml files at scan time.
     pub custom_rules: Vec<String>,
@@ -52,6 +59,7 @@ impl SemgrepRunner {
             rulesets,
             exclude_rules,
             timeout_secs: DEFAULT_SEMGREP_TIMEOUT_SECS,
+            wall_clock_timeout_secs: DEFAULT_SEMGREP_WALL_CLOCK_SECS,
             custom_rules: Vec::new(),
             languages: Vec::new(),
             max_target_bytes: None,
@@ -65,12 +73,18 @@ impl SemgrepRunner {
         self
     }
 
-    /// Builder: set per-run timeout in seconds.
+    /// Builder: set per-file timeout in seconds.
     pub fn with_timeout(mut self, timeout_secs: u64) -> Self {
         self.timeout_secs = timeout_secs;
         self
     }
 
+    /// Builder: set wall-clock timeout for the entire semgrep run.
+    pub fn with_wall_clock_timeout(mut self, wall_clock_timeout_secs: u64) -> Self {
+        self.wall_clock_timeout_secs = wall_clock_timeout_secs;
+        self
+    }
+
     /// Builder: attach inline rule YAML documents from preset `custom_rules`.
     pub fn with_custom_rules(mut self, custom_rules: Vec<String>) -> Self {
         self.custom_rules = custom_rules;
@@ -196,22 +210,26 @@ pub struct RawFinding {
     pub message: Option<String>,
 }
 
-/// Map check_id to severity based on keyword matching
-pub fn parse_severity(check_id: &str) -> Severity {
-    // Keyword-based matching (substring), preserve raise-only semantics
+/// Map check_id to severity based on keyword matching.
+/// Returns `None` when no keyword matches, so the parser can distinguish
+/// a real keyword-derived severity (which may upgrade an explicit
+/// `extra.severity`) from an unknown fallback (which must not).
+pub fn parse_severity(check_id: &str) -> Option<Severity> {
     match check_id.to_lowercase().as_str() {
-        s if s.contains("critical") => Severity::Critical,
-        s if s.contains("high") => Severity::High,
-        s if s.contains("medium") => Severity::Medium,
-        s if s.contains("low") => Severity::Low,
-        _ => Severity::Info,
+        s if s.contains("critical") => Some(Severity::Critical),
+        s if s.contains("high") => Some(Severity::High),
+        s if s.contains("medium") => Some(Severity::Medium),
+        s if s.contains("low") => Some(Severity::Low),
+        s if s.contains("info") => Some(Severity::Info),
+        _ => None,
     }
 }
 
 /// Collect rule IDs from a list of YAML documents.
 /// Each document is expected to have a `rules` list with `id` fields.
-/// Returns a BTreeSet of all rule IDs found, or an error naming the document index
-/// and the serde error if any document is invalid YAML.
+/// Returns a BTreeSet of all rule IDs found. Malformed documents are
+/// skipped with a warning instead of aborting the entire collection,
+/// so a single bad rule does not suppress all taint rules.
 pub fn collect_rule_ids(docs: &[String]) -> Result<std::collections::BTreeSet<String>, String> {
     use serde::Deserialize;
 
@@ -228,12 +246,16 @@ pub fn collect_rule_ids(docs: &[String]) -> Result<std::collections::BTreeSet<St
     let mut ids = std::collections::BTreeSet::new();
 
     for (i, doc) in docs.iter().enumerate() {
-        let parsed: RulesDoc = serde_yaml::from_str(doc)
-            .map_err(|e| format!("Invalid YAML at document index {}: {}", i, e))?;
-
-        if let Some(rules) = parsed.rules {
-            for rule in rules {
-                ids.insert(rule.id);
+        match serde_yaml::from_str::<RulesDoc>(doc) {
+            Ok(parsed) => {
+                if let Some(rules) = parsed.rules {
+                    for rule in rules {
+                        ids.insert(rule.id);
+                    }
+                }
+            }
+            Err(e) => {
+                tracing::warn!("Skipping malformed custom rule at index {}: {}", i, e);
             }
         }
     }
@@ -242,9 +264,9 @@ pub fn collect_rule_ids(docs: &[String]) -> Result<std::collections::BTreeSet<St
 }
 
 /// Filter rules from a YAML document, excluding those whose IDs are in the exclude set.
-/// Returns Ok(None) if no rules remain after filtering, Ok(Some(string)) with the
-/// re-serialized document otherwise. On error, returns a string naming the problem.
-/// The exclude set is not silently ignored — it must be valid.
+/// Returns Ok(None) if no rules remain after filtering or the document is malformed,
+/// Ok(Some(string)) with the re-serialized document otherwise. Malformed documents
+/// are skipped with a warning instead of aborting the entire scan.
 pub fn filter_rules_excluding_ids(
     doc: &str,
     exclude: &std::collections::BTreeSet<String>,
@@ -265,8 +287,13 @@ pub fn filter_rules_excluding_ids(
         rest: std::collections::HashMap<String, serde_yaml::Value>,
     }
 
-    let parsed: RulesDoc =
-        serde_yaml::from_str(doc).map_err(|e| format!("Failed to parse YAML document: {}", e))?;
+    let parsed: RulesDoc = match serde_yaml::from_str(doc) {
+        Ok(p) => p,
+        Err(e) => {
+            tracing::warn!("Skipping malformed derived taint rule document: {}", e);
+            return Ok(None);
+        }
+    };
 
     let filtered: Vec<Rule> = parsed
         .rules

src/semgrep/rules/taint-csharp.yml

@@ -11,6 +11,8 @@ rules:
     pattern-sources:
       - pattern: Request.QueryString[$KEY]
       - pattern: Request.Form[$KEY]
+      - pattern: Request.Query[$KEY]
+      - pattern: Request.Headers[$KEY]
     pattern-sinks:
       - pattern: new SqlCommand($SQL)
       - pattern: $CMD.ExecuteReader()
@@ -28,8 +30,13 @@ rules:
     pattern-sources:
       - pattern: Request.QueryString[$KEY]
       - pattern: Request.Form[$KEY]
+      - pattern: Request.Query[$KEY]
+      - pattern: Request.Headers[$KEY]
     pattern-sinks:
       - pattern: Process.Start($CMD)
+      - pattern: Process.Start($CMD, ...)
+      - pattern: System.Diagnostics.Process.Start($CMD)
+      - pattern: System.Diagnostics.Process.Start($CMD, ...)
 
   - id: csharp-taint-path-traversal-high
     languages: [csharp]
@@ -43,6 +50,8 @@ rules:
     pattern-sources:
       - pattern: Request.QueryString[$KEY]
       - pattern: Request.Form[$KEY]
+      - pattern: Request.Query[$KEY]
+      - pattern: Request.Headers[$KEY]
     pattern-sinks:
       - pattern: File.ReadAllText($PATH)
       - pattern: File.Open($PATH, $MODE)
@@ -59,5 +68,7 @@ rules:
     pattern-sources:
       - pattern: Request.QueryString[$KEY]
       - pattern: Request.Form[$KEY]
+      - pattern: Request.Query[$KEY]
+      - pattern: Request.Headers[$KEY]
     pattern-sinks:
       - pattern: new BinaryFormatter().Deserialize($STREAM)