From c6ba0a9edca628b483d74cca2fda7a890ba18aed Mon Sep 17 00:00:00 2001 From: Prathmesh Desai <142252602+pd241008@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:07:42 +0530 Subject: [PATCH 1/7] refactor: make Report the single source of truth with accessors Report kept duplicate state (rendered `lines` plus structured `entries`) that could drift apart, and only exposed a raw epoch timestamp. - derive rendered output from entries at join()/to_json() time instead of maintaining a parallel `lines` vec - expose findings()/entries()/severity_counts() accessors; fields are now private - record a human-readable ISO-8601 timestamp alongside the epoch value - merge duplicate section headers on render so interleaved section() calls can no longer split a section in the output - update diff.rs and main.rs to the accessor API --- src/diff.rs | 12 +-- src/main.rs | 8 +- src/report.rs | 223 +++++++++++++++++++++++++++++++++++++++---- tests/integration.rs | 30 +++--- 4 files changed, 232 insertions(+), 41 deletions(-) diff --git a/src/diff.rs b/src/diff.rs index 5849b5e..d8a9946 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -72,35 +72,35 @@ fn key(e: &Entry) -> (String, String) { pub fn compute(previous: &Report, current: &Report) -> DiffResult { let prev: BTreeSet<(String, String)> = previous - .entries + .entries() .iter() .filter(|e| e.severity != Severity::Info) .map(key) .collect(); let cur: BTreeSet<(String, String)> = current - .entries + .entries() .iter() .filter(|e| e.severity != Severity::Info) .map(key) .collect(); let new_findings: Vec = current - .entries + .entries() .iter() .filter(|e| e.severity != Severity::Info && !prev.contains(&key(e))) .cloned() .collect(); let resolved_findings: Vec = previous - .entries + .entries() .iter() .filter(|e| e.severity != Severity::Info && !cur.contains(&key(e))) .cloned() .collect(); DiffResult { - previous_findings: previous.findings, - current_findings: current.findings, + previous_findings: previous.findings(), + current_findings: current.findings(), new_findings, resolved_findings, } diff --git a/src/main.rs b/src/main.rs index 889b249..08747ad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -88,14 +88,18 @@ fn main() { eprintln!("error: could not write report to {}: {}", path, e); std::process::exit(1); } - eprintln!("report written to {} ({} findings)", path, report.findings); + eprintln!( + "report written to {} ({} findings)", + path, + report.findings() + ); } else if cli.json { println!("{}", report.to_json()); } else { print!("{}", report.join()); } - if report.findings > 0 { + if report.findings() > 0 { std::process::exit(2); } } diff --git a/src/report.rs b/src/report.rs index 79c7f18..b21aeb6 100644 --- a/src/report.rs +++ b/src/report.rs @@ -44,12 +44,24 @@ impl SeverityCounts { } } +/// Scan report — a single source of truth. +/// +/// Findings are stored once, as structured [`Entry`] values, plus an ordered +/// list of section titles. Both the plain-text view ([`Report::join`]) and the +/// JSON view ([`Report::to_json`]) are *derived* at output time, so the +/// rendered text can never drift out of sync with the entries it came from. +/// +/// `current_section` is transient writer state used to attribute entries to +/// the section they were logged under; it is not part of the serialized form. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(default)] pub struct Report { - pub lines: Vec, - pub findings: u32, - pub severity_counts: SeverityCounts, - pub entries: Vec, + timestamp_epoch: u64, + timestamp_iso: String, + sections: Vec, + entries: Vec, + findings: u32, + severity_counts: SeverityCounts, #[serde(skip)] current_section: String, } @@ -62,29 +74,58 @@ impl Default for Report { impl Report { pub fn new() -> Self { + let secs = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); Report { - lines: vec![now_string()], + timestamp_epoch: secs, + timestamp_iso: epoch_to_iso(secs), + sections: Vec::new(), + entries: Vec::new(), findings: 0, severity_counts: SeverityCounts::default(), - entries: Vec::new(), current_section: String::new(), } } pub fn section(&mut self, title: &str) { - self.lines.push(String::new()); - self.lines.push(format!("== {} ==", title)); + self.sections.push(title.to_string()); self.current_section = title.to_string(); } + /// All entries in the order they were recorded. + pub fn entries(&self) -> &[Entry] { + &self.entries + } + + /// Number of warning + critical findings. + pub fn findings(&self) -> u32 { + self.findings + } + + /// Per-severity counts. + pub fn severity_counts(&self) -> SeverityCounts { + self.severity_counts + } + + /// Scan timestamp as Unix epoch seconds. + pub fn timestamp_epoch(&self) -> u64 { + self.timestamp_epoch + } + + /// Scan timestamp as an ISO-8601 UTC string. + pub fn timestamp_iso(&self) -> &str { + &self.timestamp_iso + } + fn push(&mut self, severity: Severity, message: String) { let section = self.current_section.clone(); self.entries.push(Entry { severity, - message: message.clone(), + message, section, }); - self.lines.push(render_line(severity, &message)); } pub fn log(&mut self, msg: impl Into) { @@ -108,8 +149,46 @@ impl Report { self.push(Severity::Critical, msg.into()); } + /// Rendered plain-text lines, derived from the entries. + fn render_lines(&self) -> Vec { + let mut out = vec![format!( + "scan time: {} (epoch:{})", + self.timestamp_iso, self.timestamp_epoch + )]; + + // Preamble: entries recorded before any section() call (rare). + for e in self.entries.iter().filter(|e| e.section.is_empty()) { + out.push(e.render()); + } + + for (idx, title) in self.sections.iter().enumerate() { + // Render each distinct section title only once, even if + // section() was called repeatedly with the same title. + if self.sections[..idx].contains(title) { + continue; + } + out.push(String::new()); + out.push(format!("== {} ==", title)); + for e in self.entries.iter().filter(|e| &e.section == title) { + out.push(e.render()); + } + } + + // Defensive: entries referencing a section that was never declared + // still get rendered instead of silently disappearing. + for e in self + .entries + .iter() + .filter(|e| !e.section.is_empty() && !self.sections.contains(&e.section)) + { + out.push(e.render()); + } + + out + } + pub fn join(&self) -> String { - self.lines.join("\n") + self.render_lines().join("\n") } pub fn to_json(&self) -> String { @@ -117,13 +196,13 @@ impl Report { entries.sort_by_key(|b| std::cmp::Reverse(b.severity)); #[derive(Serialize)] struct JsonReport<'a> { - lines: &'a [String], + lines: Vec, findings: u32, severity_counts: SeverityCounts, entries: Vec<&'a Entry>, } let json = JsonReport { - lines: &self.lines, + lines: self.render_lines(), findings: self.findings, severity_counts: self.severity_counts, entries, @@ -140,10 +219,116 @@ pub fn render_line(severity: Severity, message: &str) -> String { } } -pub fn now_string() -> String { - let secs = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_secs(); - format!("epoch:{}", secs) +/// Convert Unix epoch seconds to a UTC ISO-8601 timestamp (no external deps). +/// +/// Uses the civil-from-days algorithm (Hinnant, 2017) so leap years — +/// including the 2000-02-29 and 2100 edge cases — are handled correctly. +fn epoch_to_iso(epoch_secs: u64) -> String { + let days = (epoch_secs / 86_400) as i64; + let rem = epoch_secs % 86_400; + let (h, m, s) = (rem / 3_600, (rem % 3_600) / 60, rem % 60); + + let z = days + 719_468; + let era = if z >= 0 { z } else { z - 146_096 } / 146_097; + let doe = z - era * 146_097; // [0, 146096] + let yoe = (doe - doe / 1_460 + doe / 36_524 - doe / 146_096) / 365; // [0, 399] + let y = yoe + era * 400; + let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); // [0, 365] + let mp = (5 * doy + 2) / 153; // [0, 11] + let d = doy - (153 * mp + 2) / 5 + 1; // [1, 31] + let month = if mp < 10 { mp + 3 } else { mp - 9 }; // [1, 12] + let year = if month <= 2 { y + 1 } else { y }; + + format!( + "{:04}-{:02}-{:02}T{:02}:{:02}:{:02}Z", + year, month, d, h, m, s + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_epoch_to_iso_known_values() { + assert_eq!(epoch_to_iso(0), "1970-01-01T00:00:00Z"); + assert_eq!(epoch_to_iso(1_000_000_000), "2001-09-09T01:46:40Z"); + // Leap day. + assert_eq!(epoch_to_iso(951_782_400), "2000-02-29T00:00:00Z"); + // Non-leap century year: 2100-03-01 follows 2100-02-28. + assert_eq!(epoch_to_iso(4_107_542_400), "2100-03-01T00:00:00Z"); + } + + #[test] + fn test_timestamp_line_has_iso_and_epoch() { + let report = Report::new(); + let out = report.join(); + let first = out.lines().next().unwrap(); + assert!(first.starts_with("scan time: "), "got: {}", first); + // ISO-8601 UTC timestamp followed by the raw epoch. + assert!(first.contains("T"), "missing ISO-8601 time: {}", first); + assert!(first.contains("Z (epoch:"), "got: {}", first); + } + + #[test] + fn test_join_matches_entries_and_sections() { + let mut report = Report::new(); + report.log("preamble"); + report.section("S"); + report.warn("warning line"); + report.critical("critical line"); + + let out = report.join(); + assert!(out.contains("== S ==")); + assert!(out.contains("[!] warning line")); + assert!(out.contains("[CRIT] critical line")); + // Section header appears exactly once even though state is not duplicated. + assert_eq!(out.matches("== S ==").count(), 1); + } + + #[test] + fn test_repeated_section_title_merges_entries() { + let mut report = Report::new(); + report.section("Recent"); + report.log("first"); + report.section("Recent"); + report.log("second"); + + let out = report.join(); + assert_eq!(out.matches("== Recent ==").count(), 1); + let first = out.find("first").unwrap(); + let second = out.find("second").unwrap(); + assert!(first < second, "entry order must be preserved"); + } + + #[test] + fn test_legacy_json_without_sections_round_trips() { + // Old baselines (pre-refactor) had a `lines` field and no + // `sections`/timestamp fields; they must still deserialize for --diff. + let legacy = r#"{ + "lines": ["epoch:1", "== S =="], + "findings": 1, + "severity_counts": {"info": 0, "warning": 1, "critical": 0}, + "entries": [ + {"severity": "Warning", "message": "legacy entry", "section": "S"} + ] + }"#; + let restored: Report = serde_json::from_str(legacy).unwrap(); + assert_eq!(restored.findings, 1); + assert_eq!(restored.entries.len(), 1); + assert_eq!(restored.entries[0].message, "legacy entry"); + } + + #[test] + fn test_json_includes_derived_lines() { + let mut report = Report::new(); + report.section("JSON Test"); + report.flag("bad thing"); + + let json = report.to_json(); + assert!(json.contains("\"lines\"")); + assert!(json.contains("== JSON Test ==")); + assert!(json.contains("[!] bad thing")); + assert!(json.contains("scan time:")); + } } diff --git a/tests/integration.rs b/tests/integration.rs index e1b6b88..86cab91 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -11,9 +11,11 @@ use std::io::Write; #[test] fn test_report_new_has_timestamp() { let report = Report::new(); - assert_eq!(report.findings, 0); - assert!(!report.lines.is_empty()); - assert!(report.lines[0].starts_with("epoch:")); + assert_eq!(report.findings(), 0); + let output = report.join(); + let first = output.lines().next().unwrap(); + assert!(first.starts_with("scan time: "), "got: {}", first); + assert!(first.contains("epoch:")); } #[test] @@ -27,7 +29,7 @@ fn test_report_section_and_log() { assert!(output.contains("== Test Section ==")); assert!(output.contains("a log line")); assert!(output.contains("another log line")); - assert_eq!(report.findings, 0); + assert_eq!(report.findings(), 0); } #[test] @@ -35,7 +37,7 @@ fn test_report_flag_increments_findings() { let mut report = Report::new(); report.flag("suspicious thing 1"); report.flag("suspicious thing 2"); - assert_eq!(report.findings, 2); + assert_eq!(report.findings(), 2); let output = report.join(); assert!(output.contains("[!] suspicious thing 1")); @@ -62,10 +64,10 @@ fn test_report_severity_markers_and_counts() { report.warn("warning line"); report.critical("critical line"); - assert_eq!(report.findings, 2); - assert_eq!(report.severity_counts.info, 1); - assert_eq!(report.severity_counts.warning, 1); - assert_eq!(report.severity_counts.critical, 1); + assert_eq!(report.findings(), 2); + assert_eq!(report.severity_counts().info, 1); + assert_eq!(report.severity_counts().warning, 1); + assert_eq!(report.severity_counts().critical, 1); let out = report.join(); assert!(out.contains("info line")); @@ -98,11 +100,11 @@ fn test_report_round_trip_via_json() { let json = report.to_json(); let restored: Report = serde_json::from_str(&json).unwrap(); - assert_eq!(restored.findings, report.findings); - assert_eq!(restored.severity_counts, report.severity_counts); - assert_eq!(restored.entries.len(), report.entries.len()); - assert_eq!(restored.entries[0].message, "critical line"); - assert_eq!(restored.entries[0].severity, Severity::Critical); + assert_eq!(restored.findings(), report.findings()); + assert_eq!(restored.severity_counts(), report.severity_counts()); + assert_eq!(restored.entries().len(), report.entries().len()); + assert_eq!(restored.entries()[0].message, "critical line"); + assert_eq!(restored.entries()[0].severity, Severity::Critical); } // ===== Diff tests ===== From 12d9fa330e2cedacfcc5f1de51e2335a4efa2f64 Mon Sep 17 00:00:00 2001 From: Prathmesh Desai <142252602+pd241008@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:08:24 +0530 Subject: [PATCH 2/7] fix: scan recent files recursively instead of one level deep Files dropped in subdirectories of temp/download folders (e.g. /tmp/subdir/payload) were never seen by the recent-files check. - walk scanned roots to a fixed depth cap (4) so deeply nested or adversarially constructed trees cannot blow up the scan - do not follow symlinks during traversal to avoid cycles and escapes outside the scanned directory - unit-test that nested files are found and beyond-cap files are not --- src/scanner/recent_files.rs | 85 ++++++++++++++++++++++++++++++++----- 1 file changed, 75 insertions(+), 10 deletions(-) diff --git a/src/scanner/recent_files.rs b/src/scanner/recent_files.rs index b5b6e13..e874861 100644 --- a/src/scanner/recent_files.rs +++ b/src/scanner/recent_files.rs @@ -2,6 +2,10 @@ use crate::report::Report; use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; +/// Maximum recursion depth when walking scan directories. Prevents runaway +/// traversals through deeply nested (or adversarially constructed) trees. +const MAX_SCAN_DEPTH: usize = 4; + pub fn run(dirs: &[String], days: u64, report: &mut Report) { report.section(&format!("Recently modified files (last {} days)", days)); let cutoff = SystemTime::now() @@ -13,19 +17,80 @@ pub fn run(dirs: &[String], days: u64, report: &mut Report) { if !dir.is_dir() { continue; } - if let Ok(entries) = std::fs::read_dir(dir) { - for entry in entries.flatten() { - let path = entry.path(); - if let Ok(meta) = entry.metadata() { - if meta.is_file() { - if let Ok(modified) = meta.modified() { - if modified > cutoff { - report.log(format!("Recently modified: {}", path.display())); - } - } + scan_dir(dir, cutoff, 0, report); + } +} + +fn scan_dir(dir: &Path, cutoff: SystemTime, depth: usize, report: &mut Report) { + if depth > MAX_SCAN_DEPTH { + return; + } + if let Ok(entries) = std::fs::read_dir(dir) { + for entry in entries.flatten() { + // Skip symlinks entirely: a planted link could point the walk at + // arbitrary locations (e.g. re-scan /etc recursively). + let path = entry.path(); + let Ok(meta) = entry.metadata() else { + continue; + }; + if meta.is_file() { + if let Ok(modified) = meta.modified() { + if modified > cutoff { + report.log(format!("Recently modified: {}", path.display())); } } + } else if meta.is_dir() { + scan_dir(&path, cutoff, depth + 1, report); } } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_scan_finds_files_in_subdirectories() { + let tmp = std::env::temp_dir().join("sentrix_recursive_test"); + let _ = std::fs::remove_dir_all(&tmp); + let nested = tmp.join("level1").join("level2"); + std::fs::create_dir_all(&nested).unwrap(); + std::fs::write(nested.join("deep.txt"), "x").unwrap(); + + let mut report = Report::new(); + run(&[tmp.to_string_lossy().to_string()], 1, &mut report); + + let out = report.join(); + assert!( + out.contains("deep.txt"), + "nested file should be reported, got: {}", + out + ); + let _ = std::fs::remove_dir_all(&tmp); + } + + #[test] + fn test_depth_cap_stops_walking() { + // Build a chain deeper than MAX_SCAN_DEPTH; the innermost file must + // not be reported. + let tmp = std::env::temp_dir().join("sentrix_depth_test"); + let _ = std::fs::remove_dir_all(&tmp); + let mut cur = tmp.clone(); + for i in 0..MAX_SCAN_DEPTH + 2 { + cur = cur.join(format!("d{}", i)); + } + std::fs::create_dir_all(&cur).unwrap(); + std::fs::write(cur.join("too_deep.txt"), "x").unwrap(); + + let mut report = Report::new(); + run(&[tmp.to_string_lossy().to_string()], 1, &mut report); + + let out = report.join(); + assert!( + !out.contains("too_deep.txt"), + "file beyond depth cap must not be reported" + ); + let _ = std::fs::remove_dir_all(&tmp); + } +} From 37c808f7ff8400ecb5faff986c5bc7f692aa7e7a Mon Sep 17 00:00:00 2001 From: Prathmesh Desai <142252602+pd241008@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:08:49 +0530 Subject: [PATCH 3/7] fix: stop macOS network-extension check from flagging most bundle IDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pattern list included "com.", so virtually every macOS bundle ID ( nearly all start with com. ) produced a finding — the check reported noise instead of signal. - match generic fragments like "com." via allowlist instead: bundle IDs containing any allowlisted fragment are skipped before patterns are applied - seed the allowlist with common vendor prefixes so the default config is usable out of the box - make patterns configurable via [macos] network_extension_patterns / network_extension_allowlist in sentrix.toml - document both keys in sentrix.example.toml --- sentrix.example.toml | 31 ++++++++++++++++++++++++++++++ src/config.rs | 34 +++++++++++++++++++++++++++++++++ src/config_loader.rs | 5 +++++ src/platform/macos.rs | 44 +++++++++++++++++++++++++++++++++++++------ 4 files changed, 108 insertions(+), 6 deletions(-) diff --git a/sentrix.example.toml b/sentrix.example.toml index 0101aa9..4f47340 100644 --- a/sentrix.example.toml +++ b/sentrix.example.toml @@ -67,6 +67,37 @@ wmi_event_consumer_patterns = [ suspicious_plist_patterns = ["curl", "wget", "/tmp/", "base64"] suspicious_cron_patterns = ["curl", "wget", "base64", "/tmp/"] suspicious_launchctl_output = ["curl", "wget", "/tmp/", "base64", "mshta", "powershell"] + +# Network/system extension name patterns that trigger a flag. +network_extension_patterns = ["filter", "proxy", "dns", "vpn", "firewall", "monitor", "capture"] + +# Identifier prefixes that exempt an extension from flagging. Without an +# allowlist, standard macOS bundle IDs (almost all starting with "com.") +# would flood the report with false positives. +network_extension_allowlist = [ + "com.apple.", + "com.cisco.", + "com.crowdstrike.", + "com.dtna.", + "com.egnyte.", + "com.google.", + "com.cloudflare.", + "com.jamf.", + "com.kandji.", + "com.malwarebytes.", + "com.microsoft.", + "com.netskope.", + "com.paloaltonetworks.", + "com.sentinelone.", + "com.sophos.", + "com.1e.", + "com.1password.", + "com.zscaler.", + "org.mozilla.", + "ch.protonvpn.", + "net.tunnelblick.", +] + shell_rc_files = [".zshrc", ".bash_profile", ".zprofile"] # Linux-specific settings diff --git a/src/config.rs b/src/config.rs index 53369c2..ccf857c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -102,6 +102,40 @@ pub const MACOS_KEXT_SCAN_DIRS: &[&str] = &["/Library/Extensions", "/System/Libr pub const MACOS_NETWORK_EXTENSION_DIRS: &[&str] = &["/Library/SystemExtensions", "/Library/NetworkExtensions"]; +#[cfg(target_os = "macos")] +pub const MACOS_NETWORK_EXT_PATTERNS: &[&str] = &[ + "filter", "proxy", "dns", "vpn", "firewall", "monitor", "capture", +]; + +/// Identifier prefixes (bundle IDs, vendor names) of well-known legitimate +/// network/system extensions. Matching entries are logged, not flagged. +/// The old heuristic flagged anything containing "com.", which matches +/// virtually every macOS bundle ID and produced mass false positives. +#[cfg(target_os = "macos")] +pub const MACOS_NETWORK_EXT_ALLOWLIST: &[&str] = &[ + "com.apple.", + "com.cisco.", + "com.crowdstrike.", + "com.dtna.", + "com.egnyte.", + "com.google.", + "com.cloudflare.", + "com.jamf.", + "com.kandji.", + "com.malwarebytes.", + "com.microsoft.", + "com.netskope.", + "com.paloaltonetworks.", + "com.sentinelone.", + "com.sophos.", + "com.1e.", + "com.1password.", + "com.zscaler.", + "org.mozilla.", + "ch.protonvpn.", + "net.tunnelblick.", +]; + #[cfg(target_os = "windows")] pub const SUSPICIOUS_SERVICE_PATTERNS: &[&str] = &[ "\\temp\\", diff --git a/src/config_loader.rs b/src/config_loader.rs index b612c78..262bbf8 100644 --- a/src/config_loader.rs +++ b/src/config_loader.rs @@ -19,6 +19,11 @@ pub struct PlatformConfig { pub suspicious_plist_patterns: Option>, pub suspicious_cron_patterns: Option>, pub suspicious_launchctl_output: Option>, + /// macOS only: name patterns that flag a network/system extension. + pub network_extension_patterns: Option>, + /// macOS only: identifier prefixes that exempt a network/system extension + /// from flagging (e.g. "com.apple.", "com.microsoft."). + pub network_extension_allowlist: Option>, pub shell_rc_files: Option>, pub persistence_scan_dirs: Option>, } diff --git a/src/platform/macos.rs b/src/platform/macos.rs index 601781f..4383743 100644 --- a/src/platform/macos.rs +++ b/src/platform/macos.rs @@ -1,7 +1,8 @@ use crate::config::{ launch_agent_dirs, path_is_suspicious, suspicious_dirs, MACOS_KEXT_SCAN_DIRS, - MACOS_NETWORK_EXTENSION_DIRS, MACOS_SHELL_RC_FILES, RECENT_FILE_DAYS, SUSPICIOUS_CRON_PATTERNS, - SUSPICIOUS_LAUNCHCTL_OUTPUT, SUSPICIOUS_PLIST_PATTERNS, + MACOS_NETWORK_EXTENSION_DIRS, MACOS_NETWORK_EXT_ALLOWLIST, MACOS_NETWORK_EXT_PATTERNS, + MACOS_SHELL_RC_FILES, RECENT_FILE_DAYS, SUSPICIOUS_CRON_PATTERNS, SUSPICIOUS_LAUNCHCTL_OUTPUT, + SUSPICIOUS_PLIST_PATTERNS, }; use crate::config_loader::UserConfig; use crate::report::Report; @@ -223,7 +224,29 @@ pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) } // Network extension scanning + // Patterns and allowlist are configurable; built-ins live in config.rs. + // The allowlist keeps well-known vendors from being flagged merely for + // following Apple's reverse-DNS bundle-ID convention (see config.rs). report.section("Persistence (network extensions)"); + let ext_patterns: Vec = user_config + .and_then(|c| c.macos.as_ref()) + .and_then(|c| c.network_extension_patterns.clone()) + .unwrap_or_else(|| { + MACOS_NETWORK_EXT_PATTERNS + .iter() + .map(|s| s.to_string()) + .collect() + }); + let ext_allowlist: Vec = user_config + .and_then(|c| c.macos.as_ref()) + .and_then(|c| c.network_extension_allowlist.clone()) + .unwrap_or_else(|| { + MACOS_NETWORK_EXT_ALLOWLIST + .iter() + .map(|s| s.to_string()) + .collect() + }); + for net_ext_dir in MACOS_NETWORK_EXTENSION_DIRS { let dir = Path::new(net_ext_dir); if !dir.is_dir() { @@ -235,10 +258,19 @@ pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) let name = path.file_name().unwrap_or_default().to_string_lossy(); let name_lower = name.to_lowercase(); - let suspicious_patterns = [ - "com.", "filter", "proxy", "dns", "vpn", "firewall", "monitor", "capture", - ]; - let is_suspicious = suspicious_patterns.iter().any(|p| name_lower.contains(p)); + // Allowlist wins first: bundle IDs like "com.apple.", or + // known vendor prefixes, are normal macOS identifiers. + if ext_allowlist + .iter() + .any(|p| name_lower.contains(&p.to_lowercase())) + { + report.log(format!("Network extension: {}", path.display())); + continue; + } + + let is_suspicious = ext_patterns + .iter() + .any(|p| name_lower.contains(&p.to_lowercase())); if is_suspicious { report.flag(format!( "Suspicious network extension: {} ({})", From b96743062559d6a41cffc274bcbf52746679e794 Mon Sep 17 00:00:00 2001 From: Prathmesh Desai <142252602+pd241008@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:09:08 +0530 Subject: [PATCH 4/7] fix: restore Windows checks on wmic-less systems and parse output correctly wmic is deprecated and removed from recent Windows 11 builds, so the service/WMI checks silently did nothing; CSV splitting on commas broke paths that contain commas; and UTF-16 output decoded as UTF-8 produced mojibake that no pattern could match. - fall back to PowerShell CIM (Get-CimInstance) queries when wmic is absent for process, service, scheduled-task and WMI event-consumer checks - decode command output via BOM sniffing (UTF-8 / UTF-16LE / UTF-16BE), falling back to lossy UTF-8 - parse CSV with a quote-aware parser and select columns by header name instead of fixed positions, so values containing commas and reordered columns both work - unit-test decoding and CSV parsing (cfg(windows), run on CI Windows runners) --- src/platform/mod.rs | 5 + src/platform/win_helpers.rs | 223 +++++++++++++++++++++++ src/platform/windows.rs | 340 ++++++++++++++++++++++-------------- 3 files changed, 435 insertions(+), 133 deletions(-) create mode 100644 src/platform/win_helpers.rs diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 61e3a31..43fb7ce 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -12,3 +12,8 @@ pub use windows::*; mod macos; #[cfg(target_os = "macos")] pub use macos::*; + +// Windows-only helpers for decoding/parsing external command output. +// cfg-gated so non-Windows builds never see it. +#[cfg(target_os = "windows")] +pub mod win_helpers; diff --git a/src/platform/win_helpers.rs b/src/platform/win_helpers.rs new file mode 100644 index 0000000..2fd4b93 --- /dev/null +++ b/src/platform/win_helpers.rs @@ -0,0 +1,223 @@ +//! Helpers for decoding and parsing Windows command output. +//! +//! Two pitfalls this module fixes: +//! +//! 1. **Encoding** — `wmic` emits UTF-16LE (with a BOM) on most builds, and +//! other tools can emit BOM-less UTF-16 or legacy codepages. Naive +//! `String::from_utf8_lossy` on UTF-16 data yields NUL-interleaved text +//! in which no detection pattern can ever match. +//! 2. **CSV** — quoted fields (e.g. `ExecutablePath` values containing +//! commas) break naive `split(',')`, producing wrong field indexes and +//! missed detections. This parser respects RFC-4180-style quotes. + +/// Decode raw process stdout into clean text. +/// +/// Handles UTF-16LE/BE (with or without BOM), UTF-8 BOM, and falls back to +/// lossy UTF-8 for anything else (e.g. legacy codepage output, where ASCII +/// pattern matching still mostly works). +pub fn decode_output(raw: &[u8]) -> String { + // UTF-16 BOMs are unambiguous — decode as wide text. + if raw.starts_with(&[0xFF, 0xFE]) { + return decode_utf16(&raw[2..], true); + } + if raw.starts_with(&[0xFE, 0xFF]) { + return decode_utf16(&raw[2..], false); + } + // UTF-8 BOM. + let raw = raw.strip_prefix(&[0xEF, 0xBB, 0xBF][..]).unwrap_or(raw); + + // BOM-less UTF-16LE is common from wmic/powershell piped output: ASCII + // chars encode as `char, 0x00`, so lots of NUL bytes is a reliable tell. + // Valid UTF-8 text never contains NUL bytes, so this never misfires on it. + let nul_count = raw.iter().filter(|&&b| b == 0).count(); + if raw.len() >= 2 && nul_count > raw.len() / 4 { + return decode_utf16(raw, true); + } + + String::from_utf8_lossy(raw).into_owned() +} + +fn decode_utf16(bytes: &[u8], little_endian: bool) -> String { + let units: Vec = bytes + .chunks_exact(2) + .map(|c| { + if little_endian { + u16::from_le_bytes([c[0], c[1]]) + } else { + u16::from_be_bytes([c[0], c[1]]) + } + }) + .collect(); + String::from_utf16_lossy(&units) +} + +/// Split one CSV line into fields, honoring double-quoted fields. +/// +/// `""` inside a quoted field decodes to a literal `"` (RFC 4180). Quotes are +/// stripped from quoted fields; the outer CSV wrappers used by `tasklist /fo +/// csv` therefore come back clean. +pub fn parse_csv_line(line: &str) -> Vec { + let mut fields = Vec::new(); + let mut cur = String::new(); + let mut in_quotes = false; + let mut chars = line.chars().peekable(); + + while let Some(c) = chars.next() { + if in_quotes { + if c == '"' { + if chars.peek() == Some(&'"') { + cur.push('"'); + chars.next(); + } else { + in_quotes = false; + } + } else { + cur.push(c); + } + } else { + match c { + '"' => in_quotes = true, + ',' => { + fields.push(std::mem::take(&mut cur)); + } + _ => cur.push(c), + } + } + } + fields.push(cur); + fields +} + +/// A CSV table parsed from command output (`wmic /format:csv`, +/// `ConvertTo-Csv`, ...): a header row plus data rows, with column lookup by +/// name instead of fragile positional indexes. +pub struct CsvTable { + headers: Vec, + pub rows: Vec>, +} + +impl CsvTable { + /// Parse decoded text into a table. The first non-empty line is the + /// header; remaining non-empty lines are data rows. + pub fn parse(text: &str) -> Self { + let mut headers: Vec = Vec::new(); + let mut rows: Vec> = Vec::new(); + for line in text.lines() { + let line = line.trim(); + if line.is_empty() { + continue; + } + if headers.is_empty() { + headers = parse_csv_line(line) + .into_iter() + .map(|f| f.trim().to_lowercase()) + .collect(); + } else { + rows.push(parse_csv_line(line)); + } + } + CsvTable { headers, rows } + } + + fn col_index(&self, name: &str) -> Option { + self.headers.iter().position(|h| h == name) + } + + /// Fetch a trimmed cell from a data row by header name. + pub fn get<'a>(&self, row: &'a [String], name: &str) -> Option<&'a str> { + self.col_index(name) + .and_then(|i| row.get(i)) + .map(|s| s.trim()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_decode_utf16le_with_bom() { + // "powershell" in UTF-16LE with BOM. + let mut raw: Vec = vec![0xFF, 0xFE]; + raw.extend("powershell".encode_utf16().flat_map(|u| u.to_le_bytes())); + assert_eq!(decode_output(&raw), "powershell"); + } + + #[test] + fn test_decode_bomless_utf16le() { + let raw: Vec = "mshta" + .encode_utf16() + .flat_map(|u| u.to_le_bytes()) + .collect(); + assert_eq!(decode_output(&raw), "mshta"); + } + + #[test] + fn test_decode_utf16be_with_bom() { + let mut raw: Vec = vec![0xFE, 0xFF]; + raw.extend("certutil".encode_utf16().flat_map(|u| u.to_be_bytes())); + assert_eq!(decode_output(&raw), "certutil"); + } + + #[test] + fn test_decode_utf8_passthrough() { + assert_eq!(decode_output(b"plain ascii"), "plain ascii"); + assert_eq!(decode_output("\u{00e9}".as_bytes()), "\u{00e9}"); + let mut bommed: Vec = vec![0xEF, 0xBB, 0xBF]; + bommed.extend(b"ok"); + assert_eq!(decode_output(&bommed), "ok"); + } + + #[test] + fn test_parse_csv_line_with_quoted_commas() { + let line = r#"Node,"C:\path with,comma\evil.exe","evil, name",1234"#; + let fields = parse_csv_line(line); + assert_eq!(fields[0], "Node"); + assert_eq!(fields[1], r"C:\path with,comma\evil.exe"); + assert_eq!(fields[2], "evil, name"); + assert_eq!(fields[3], "1234"); + } + + #[test] + fn test_parse_csv_line_strips_tasklist_quotes() { + let line = r#""chrome.exe","1234","Console""#; + let fields = parse_csv_line(line); + assert_eq!(fields, vec!["chrome.exe", "1234", "Console"]); + } + + #[test] + fn test_parse_csv_line_escaped_quotes() { + let line = r#"a,"say ""hi""",b"#; + let fields = parse_csv_line(line); + assert_eq!(fields[1], r#"say "hi""#); + } + + #[test] + fn test_csv_table_lookup_by_header_name() { + let table = CsvTable::parse( + "Node,ExecutablePath,Name,ProcessId\r\nDESKTOP,C:\\path\\evil.exe,evil.exe,4242\r\n", + ); + assert_eq!(table.rows.len(), 1); + let row = &table.rows[0]; + assert_eq!(table.get(row, "executablepath"), Some(r"C:\path\evil.exe")); + assert_eq!(table.get(row, "name"), Some("evil.exe")); + assert_eq!(table.get(row, "processid"), Some("4242")); + assert_eq!(table.get(row, "missing"), None); + } + + #[test] + fn test_csv_table_handles_quoted_paths_with_commas() { + let text = concat!( + r#""ProcessId","Name","ExecutablePath""#, + "\r\n", + r#""7","svc, host","C:\\Program Files, Sub\\a.exe""#, + ); + let table = CsvTable::parse(text); + let row = &table.rows[0]; + assert_eq!(table.get(row, "name"), Some("svc, host")); + assert_eq!( + table.get(row, "executablepath"), + Some(r"C:\Program Files, Sub\a.exe") + ); + } +} diff --git a/src/platform/windows.rs b/src/platform/windows.rs index f077b01..a7a420d 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -4,79 +4,135 @@ use crate::config::{ WMI_EVENT_CONSUMER_PATTERNS, }; use crate::config_loader::UserConfig; +use crate::platform::win_helpers::{decode_output, CsvTable}; use crate::report::Report; use std::process::Command; use winreg::enums::*; use winreg::RegKey; +/// Run an external command and return its decoded stdout, or `None` if the +/// program could not be launched (missing, blocked, ...). +fn run_capture(program: &str, args: &[&str]) -> Option { + Command::new(program) + .args(args) + .output() + .ok() + .map(|out| decode_output(&out.stdout)) +} + +/// Run a PowerShell command; returns decoded stdout or `None`. +fn run_ps(ps_script: &str) -> Option { + run_capture( + "powershell", + &["-NoProfile", "-NonInteractive", "-Command", ps_script], + ) +} + +/// Run `wmic` and fall back to an equivalent PowerShell CIM query when wmic +/// is unavailable (deprecated/removed on recent Windows 11 builds). +fn run_wmic_or_ps(wmic_args: &[&str], ps_script: &str) -> Option { + if let Some(text) = run_capture("wmic", wmic_args) { + if !text.trim().is_empty() { + return Some(text); + } + } + run_ps(ps_script) +} + +/// Extract a full executable path from a tasklist `/v` row, which shows +/// only the image name — resolve it against the Path environment variable. +fn resolve_image_name(name: &str) -> String { + for dir in std::env::split_paths(&std::env::var_os("PATH").unwrap_or_default()) { + let candidate = dir.join(name); + if candidate.is_file() { + return candidate.to_string_lossy().into_owned(); + } + } + String::new() +} + pub fn check_processes(report: &mut Report, _user_config: Option<&UserConfig>) { report.section("Suspicious process locations"); let sus_dirs = suspicious_dirs(); - // Try wmic first for full executable paths - let output = Command::new("wmic") - .args([ + // Prefer wmic for full executable paths; fall back to PowerShell CIM + // (Get-CimInstance) on builds where wmic is deprecated/removed, then to + // tasklist as a last resort. + let text = run_wmic_or_ps( + &[ "process", "get", "Name,ExecutablePath,ProcessId", "/format:csv", - ]) - .output(); + ], + "Get-CimInstance Win32_Process | Where-Object ExecutablePath | \ + Select-Object ExecutablePath,Name,ProcessId | ConvertTo-Csv -NoTypeInformation", + ); - if let Ok(out) = output { - let text = String::from_utf8_lossy(&out.stdout); - for line in text.lines().skip(1) { - let line = line.trim(); - if line.is_empty() { - continue; - } - // CSV format: Node,ExecutablePath,Name,ProcessId - let fields: Vec<&str> = line.split(',').collect(); - if fields.len() < 4 { + if let Some(text) = text { + let table = CsvTable::parse(&text); + let mut matched = 0usize; + for row in &table.rows { + let Some(exe_path) = table.get(row, "executablepath") else { continue; - } - let exe_path = fields[1].trim(); - let name = fields[2].trim(); - let pid = fields[3].trim(); - - if exe_path.is_empty() || exe_path == "ExecutablePath" { + }; + if exe_path.is_empty() { continue; } + let name = table.get(row, "name").unwrap_or("unknown"); + let pid = table.get(row, "processid").unwrap_or("?"); - for d in &sus_dirs { - if let Some(dstr) = d.to_str() { - if exe_path.to_lowercase().contains(&dstr.to_lowercase()) { - report.flag(format!( - "PID {} ({}) running from suspicious location: {}", - pid, name, exe_path - )); - } - } + if sus_dirs + .iter() + .filter_map(|d| d.to_str()) + .any(|d| exe_path.to_lowercase().contains(&d.to_lowercase())) + { + report.flag(format!( + "PID {} ({}) running from suspicious location: {}", + pid, name, exe_path + )); + matched += 1; } } + if matched == 0 { + report.log("(i) No processes running from suspicious locations."); + } } else { - // Fallback to tasklist if wmic is unavailable - report.log("(i) wmic unavailable, falling back to tasklist."); - let output = Command::new("tasklist").args(["/v", "/fo", "csv"]).output(); - if let Ok(out) = output { - let text = String::from_utf8_lossy(&out.stdout); - for line in text.lines().skip(1) { - let fields: Vec<&str> = line.split("\",\"").collect(); - if fields.is_empty() { + // Last-resort fallback: tasklist has no ExecutablePath column, so we + // scan for suspicious path fragments and resolve image names via PATH. + report.log("(i) wmic/CIM unavailable, falling back to tasklist."); + if let Some(text) = run_capture("tasklist", &["/v", "/fo", "csv"]) { + let mut matched = 0usize; + for line in text.lines() { + let line = line.trim(); + if line.is_empty() { continue; } - let name = fields[0].trim_matches('"'); - for d in &sus_dirs { - if let Some(dstr) = d.to_str() { - if line.to_lowercase().contains(&dstr.to_lowercase()) { - report.flag(format!( - "Process line references suspicious path: {} ({})", - name, line - )); - } + let fields = crate::platform::win_helpers::parse_csv_line(line); + let name = fields.first().map(|s| s.trim()).unwrap_or("unknown"); + let sus_hit = sus_dirs + .iter() + .filter_map(|d| d.to_str()) + .find(|d| line.to_lowercase().contains(&d.to_lowercase())); + if let Some(d) = sus_hit { + let resolved = resolve_image_name(name); + if resolved.is_empty() { + report.flag(format!( + "Process image name references suspicious dir ({}): {}", + d, name + )); + } else { + report.flag(format!( + "Process running from suspicious location: {} ({})", + resolved, name + )); } + matched += 1; } } + if matched == 0 { + report.log("(i) No processes matched suspicious locations via tasklist."); + } } else { report.log("(i) Could not run tasklist to enumerate processes."); } @@ -84,56 +140,25 @@ pub fn check_processes(report: &mut Report, _user_config: Option<&UserConfig>) { } pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) { - let autorun_patterns: Vec = user_config - .and_then(|c| c.windows.as_ref()) - .and_then(|c| c.suspicious_autorun_patterns.clone()) - .unwrap_or_else(|| { - SUSPICIOUS_AUTORUN_PATTERNS - .iter() - .map(|s| s.to_string()) - .collect() - }); + let win_cfg = user_config.and_then(|c| c.windows.as_ref()); - let task_patterns: Vec = user_config - .and_then(|c| c.windows.as_ref()) + let autorun_patterns = win_cfg + .and_then(|c| c.suspicious_autorun_patterns.clone()) + .unwrap_or_else(|| strings(SUSPICIOUS_AUTORUN_PATTERNS)); + let task_patterns = win_cfg .and_then(|c| c.suspicious_task_actions.clone()) - .unwrap_or_else(|| { - SUSPICIOUS_TASK_ACTIONS - .iter() - .map(|s| s.to_string()) - .collect() - }); - - let powershell_patterns: Vec = user_config - .and_then(|c| c.windows.as_ref()) + .unwrap_or_else(|| strings(SUSPICIOUS_TASK_ACTIONS)); + let powershell_patterns = win_cfg .and_then(|c| c.suspicious_powershell_patterns.clone()) - .unwrap_or_else(|| { - SUSPICIOUS_POWERSHELL_PATTERNS - .iter() - .map(|s| s.to_string()) - .collect() - }); - - let service_patterns: Vec = user_config - .and_then(|c| c.windows.as_ref()) + .unwrap_or_else(|| strings(SUSPICIOUS_POWERSHELL_PATTERNS)); + let service_patterns = win_cfg .and_then(|c| c.suspicious_service_patterns.clone()) - .unwrap_or_else(|| { - SUSPICIOUS_SERVICE_PATTERNS - .iter() - .map(|s| s.to_string()) - .collect() - }); - - let wmi_patterns: Vec = user_config - .and_then(|c| c.windows.as_ref()) + .unwrap_or_else(|| strings(SUSPICIOUS_SERVICE_PATTERNS)); + let wmi_patterns = win_cfg .and_then(|c| c.wmi_event_consumer_patterns.clone()) - .unwrap_or_else(|| { - WMI_EVENT_CONSUMER_PATTERNS - .iter() - .map(|s| s.to_string()) - .collect() - }); + .unwrap_or_else(|| strings(WMI_EVENT_CONSUMER_PATTERNS)); + // Registry Run keys (winreg crate — no external process needed). report.section("Persistence (Registry Run keys)"); let hives: [(&RegKey, &str); 2] = [ (&RegKey::predef(HKEY_CURRENT_USER), "HKCU"), @@ -161,12 +186,12 @@ pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) } } + // Scheduled tasks: wmic is not useful here; schtasks first, PowerShell + // Get-ScheduledTask fallback (works on builds where schtasks output + // formatting is localized or the tool is restricted). report.section("Persistence (Scheduled Tasks)"); - let output = Command::new("schtasks") - .args(["/query", "/fo", "list", "/v"]) - .output(); - if let Ok(out) = output { - let text = String::from_utf8_lossy(&out.stdout); + let mut tasks_logged = false; + if let Some(text) = run_capture("schtasks", &["/query", "/fo", "list", "/v"]) { let mut current_task = String::new(); for line in text.lines() { let line = line.trim(); @@ -190,13 +215,48 @@ pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) current_task, current_action )); } + tasks_logged = true; } } - } else { - report.log("(i) Could not run schtasks to enumerate scheduled tasks."); + } + if !tasks_logged { + // PowerShell fallback: same heuristic on task actions. + let ps = "Get-ScheduledTask | ForEach-Object { $t = $_; \ + $_.Actions | ForEach-Object { \ + '{0}\\{1}|{2}' -f $t.TaskPath, $t.TaskName, $_.Execute } }"; + if let Some(text) = run_ps(ps) { + for line in text.lines() { + let line = line.trim(); + if line.is_empty() { + continue; + } + let (task, action) = match line.split_once('|') { + Some((t, a)) => (t, a), + None => continue, + }; + let lower = action.to_lowercase(); + if task_patterns + .iter() + .any(|pat| lower.contains(&pat.to_lowercase())) + { + report.flag(format!( + "Suspicious scheduled task action: {} -> {}", + task, action + )); + } else { + report.log(format!("Scheduled task: {} -> {}", task, action)); + } + tasks_logged = true; + } + } + } + if !tasks_logged { + report.log( + "(i) Could not enumerate scheduled tasks (schtasks and PowerShell both unavailable).", + ); } - // Startup folder check + // Startup folder check. report.section("Persistence (Startup folder)"); let appdata = std::env::var("APPDATA").unwrap_or_default(); if !appdata.is_empty() { @@ -214,25 +274,29 @@ pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) } } } + } else { + report.log("(i) APPDATA not set; skipping startup folder scan."); } - // WMI event subscription check + // WMI event subscriptions: wmic first, PowerShell CIM fallback. report.section("Persistence (WMI event subscriptions)"); - let output = Command::new("wmic") - .args([ + let wmi_text = run_wmic_or_ps( + &[ "/namespace:\\\\root\\subscription", "path", "__EventConsumer", "get", "CommandLineTemplate", "/format:csv", - ]) - .output(); - if let Ok(out) = output { - let text = String::from_utf8_lossy(&out.stdout); - for line in text.lines().skip(1) { + ], + "Get-CimInstance -Namespace root/subscription -ClassName __EventConsumer | \ + Where-Object CommandLineTemplate | Select-Object -ExpandProperty CommandLineTemplate", + ); + if let Some(text) = wmi_text { + let mut any = false; + for line in text.lines() { let line = line.trim(); - if line.is_empty() || line.contains("CommandLineTemplate") { + if line.is_empty() || line.to_lowercase().contains("commandlinetemplate") { continue; } let lower = line.to_lowercase(); @@ -244,32 +308,33 @@ pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) } else { report.log(format!("WMI event consumer: {}", line)); } + any = true; + } + if !any { + report.log("(i) No WMI event consumers found."); } } else { report.log("(i) Could not query WMI event consumers."); } - // Services check + // Services: wmic first, PowerShell Get-CimInstance fallback. report.section("Persistence (Services)"); - let output = Command::new("wmic") - .args(["service", "get", "Name,PathName,StartMode", "/format:csv"]) - .output(); - if let Ok(out) = output { - let text = String::from_utf8_lossy(&out.stdout); - for line in text.lines().skip(1) { - let line = line.trim(); - if line.is_empty() || line.contains("PathName") { - continue; - } - let fields: Vec<&str> = line.split(',').collect(); - if fields.len() < 3 { + let svc_text = run_wmic_or_ps( + &["service", "get", "Name,PathName,StartMode", "/format:csv"], + "Get-CimInstance Win32_Service | Select-Object Name,PathName,StartMode | \ + ConvertTo-Csv -NoTypeInformation", + ); + if let Some(text) = svc_text { + let table = CsvTable::parse(&text); + let mut any = false; + for row in &table.rows { + let Some(path_name) = table.get(row, "pathname") else { continue; - } - let name = fields[1].trim(); - let path_name = fields[2].trim(); + }; if path_name.is_empty() { continue; } + let name = table.get(row, "name").unwrap_or("unknown"); let lower = path_name.to_lowercase(); if service_patterns .iter() @@ -282,19 +347,19 @@ pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) } else { report.log(format!("Service: {} -> {}", name, path_name)); } + any = true; + } + if !any { + report.log("(i) No services returned path information."); } } else { - report.log("(i) Could not query WMI services."); + report.log("(i) Could not enumerate services (wmic and PowerShell both unavailable)."); } - // PowerShell script block logging check + // PowerShell script block logging check. report.section("Persistence (PowerShell script block logging)"); - let ps_script = "Get-WinEvent -FilterHashtable @{LogName='Microsoft-Windows-PowerShell/Operational';Id=4104} -MaxEvents 50 2>$null | ForEach-Object { $_.Properties[2].Value }"; - let output = Command::new("powershell") - .args(["-NoProfile", "-NonInteractive", "-Command", ps_script]) - .output(); - if let Ok(out) = output { - let text = String::from_utf8_lossy(&out.stdout); + let ps_script = "Get-WinEvent -FilterHashtable @{LogName='Microsoft-Windows-PowerShell/Operational';Id=4104} -MaxEvents 50 -ErrorAction SilentlyContinue | ForEach-Object { $_.Properties[2].Value }"; + if let Some(text) = run_ps(ps_script) { let mut suspicious_count = 0; for line in text.lines() { let trimmed = line.trim(); @@ -308,9 +373,14 @@ pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) { suspicious_count += 1; if suspicious_count <= 5 { + let cut = trimmed + .char_indices() + .nth(200) + .map(|(i, _)| i) + .unwrap_or(trimmed.len()); report.flag(format!( "Suspicious PowerShell script block: {}", - &trimmed[..trimmed.len().min(200)] + &trimmed[..cut] )); } } @@ -338,3 +408,7 @@ pub fn check_persistence(report: &mut Report, user_config: Option<&UserConfig>) report, ); } + +fn strings(consts: &[&str]) -> Vec { + consts.iter().map(|s| s.to_string()).collect() +} From 349baaba0d5d35f85ad65cc6a0767ae8bb34e548 Mon Sep 17 00:00:00 2001 From: Prathmesh Desai <142252602+pd241008@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:10:04 +0530 Subject: [PATCH 5/7] fix: lint tests and examples with clippy in CI cargo clippy without --all-targets skipped #[test] code, letting lints like boolean-comparison violations (== true asserts) into the tree. - run clippy with --all-targets so tests are linted - replace the assert_eq!(x, true) in diff tests with a plain assert! --- .github/workflows/ci.yml | 2 +- tests/integration.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a223fe9..d515351 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,7 @@ jobs: run: cargo fmt --check - name: Run clippy - run: cargo clippy -- -D warnings + run: cargo clippy --all-targets -- -D warnings - name: Run tests run: cargo test diff --git a/tests/integration.rs b/tests/integration.rs index 86cab91..4c6d76b 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -144,9 +144,9 @@ fn test_diff_no_changes() { let result = diff::compute(&previous, ¤t); assert!(result.new_findings.is_empty()); assert!(result.resolved_findings.is_empty()); - assert_eq!( + assert!( result.to_text().contains("No new findings since last scan"), - true + "diff text should state there are no new findings" ); } From e2a2be27b746711f938ee401094704b95671f9b2 Mon Sep 17 00:00:00 2001 From: Prathmesh Desai <142252602+pd241008@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:10:16 +0530 Subject: [PATCH 6/7] ci: add cargo-llvm-cov coverage job and correct roadmap claims PROGRESS.md marked coverage tooling and a badge as complete, but CI had no coverage job and the repo carried no coverage tooling at all. - add a coverage job to CI: cargo-llvm-cov generates an lcov report and uploads it to Codecov (non-blocking) - correct the PROGRESS.md roadmap item to reflect the newly added tooling instead of claiming it already existed --- .github/workflows/ci.yml | 23 +++++++++++++++++++++++ docs/PROGRESS.md | 24 +++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d515351..0b7ba7f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,3 +36,26 @@ jobs: - name: Build release run: cargo build --release + + coverage: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + + - name: Cache dependencies + uses: Swatinem/rust-cache@v2 + + - name: Install cargo-llvm-cov + uses: taiki-e/install-action@cargo-llvm-cov + + - name: Generate coverage + run: cargo llvm-cov --workspace --lcov --output-path lcov.info + + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v4 + with: + files: lcov.info + fail_ci_if_error: false diff --git a/docs/PROGRESS.md b/docs/PROGRESS.md index 922acf9..d924394 100644 --- a/docs/PROGRESS.md +++ b/docs/PROGRESS.md @@ -75,6 +75,13 @@ All three platforms have process metadata, persistence, and recent file detectio **Note:** Neither Windows nor macOS can detect deleted-but-running binaries (Linux `/proc` advantage). +**Windows tooling notes:** `wmic` is deprecated/removed on recent Windows 11 +builds, so the services, process, and WMI checks fall back to PowerShell CIM +(`Get-CimInstance` / `Get-ScheduledTask`) when `wmic` is unavailable. All +command output is decoded BOM-aware (wmic emits UTF-16LE), and CSV parsing is +quote-aware with header-based column lookup (`win_helpers.rs`), so paths +containing commas or non-ASCII text are handled correctly. + ### 4. Configurable Detection Patterns **Status: Complete (100%)** @@ -85,6 +92,10 @@ All detection patterns can now be overridden via an external TOML configuration - `--config path/to/config.toml` CLI flag - Optional external TOML config file that overrides built-in defaults - Support for all platform-specific patterns (Windows, macOS, Linux) +- macOS network-extension patterns and allowlist (`network_extension_patterns`, + `network_extension_allowlist`) — the allowlist prevents well-known vendors + (Apple, Microsoft, CrowdStrike, ...) from being flagged merely for using + standard reverse-DNS bundle IDs - Uses `toml` + `serde` crates for robust parsing with proper error messages - Example config file: `sentrix.example.toml` @@ -116,9 +127,16 @@ timestamp line. ### 6. Test Coverage -**Status: Complete (20 tests)** +**Status: Complete (28 tests)** - `config_loader` — 3 unit tests (valid config, empty config, invalid TOML) +- `report` — 6 unit tests (ISO-8601 timestamp conversion incl. leap-day and + century edge cases, timestamp line format, section rendering and merge + behavior, legacy-JSON round-trip for old `--diff` baselines, JSON lines) +- `scanner::recent_files` — 2 unit tests (nested subdirectories are scanned, + depth cap stops runaway walks) +- `platform::win_helpers` (Windows target) — 9 unit tests (UTF-16LE/BE and + BOM handling, quote-aware CSV, header-based CSV table lookup) - Integration tests — 17 tests covering: - Report behavior (timestamp, section, log, flag, JSON serialization) - Severity markers/counts, JSON entries sorted by severity, JSON round-trip @@ -128,6 +146,10 @@ timestamp line. - Config override flow preservation - `--diff` computations (new/resolved findings, no-changes, info ignored) +**Coverage measurement:** CI runs `cargo llvm-cov` on ubuntu-latest and +uploads the LCOV report to Codecov (`coverage` job in `.github/workflows/ci.yml`). +Add a repo badge linking to the Codecov page once enabled there. + ### 7. Nice-to-Haves | Feature | Status | Notes | From cde3db88f564ba4827adab7ac9f55edcc95cd9cb Mon Sep 17 00:00:00 2001 From: Prathmesh Desai <142252602+pd241008@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:10:26 +0530 Subject: [PATCH 7/7] docs: fix binary name and sync docs with the Report accessor API - README used target/release/Sentrix while the binary is lowercase target/release/sentrix per Cargo.toml - sync README sample output with the new human-readable scan-time line and update the config override table for the new macOS keys - refresh ARCHITECTURE.md's report module section, which still described the removed lines field and duplicated-state design --- README.md | 39 ++++++++++++++++++++++----------------- docs/ARCHITECTURE.md | 30 ++++++++++++------------------ 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index e586aa5..453dbf5 100644 --- a/README.md +++ b/README.md @@ -225,8 +225,8 @@ cargo build --release ``` Output binary: -- Linux/macOS: `target/release/Sentrix` -- Windows: `target\release\Sentrix.exe` +- Linux/macOS: `target/release/sentrix` +- Windows: `target\release\sentrix.exe` ### Cross-Compilation @@ -243,13 +243,13 @@ cargo build --release --target x86_64-pc-windows-gnu ## Usage ``` -./Sentrix # full scan, prints to stdout -./Sentrix --quick # skip the recent-file-modification pass -./Sentrix --out report.txt # write report to file -./Sentrix --json # output report as JSON -./Sentrix --json --out report.json # write JSON report (reuseable for --diff) -./Sentrix --diff report.json # compare against a previous JSON report -./Sentrix --config custom.toml # use custom detection patterns +./sentrix # full scan, prints to stdout +./sentrix --quick # skip the recent-file-modification pass +./sentrix --out report.txt # write report to file +./sentrix --json # output report as JSON +./sentrix --json --out report.json # write JSON report (reuseable for --diff) +./sentrix --diff report.json # compare against a previous JSON report +./sentrix --config custom.toml # use custom detection patterns ``` **Privileges:** @@ -269,9 +269,9 @@ Run once saving a JSON report, then compare a later run against it to see only what changed since the last scan: ```bash -./Sentrix --json --out baseline.json # first run: save baseline -./Sentrix --diff baseline.json # later run: show new/resolved findings -./Sentrix --diff baseline.json --json # diff as JSON for pipelines +./sentrix --json --out baseline.json # first run: save baseline +./sentrix --diff baseline.json # later run: show new/resolved findings +./sentrix --diff baseline.json --json # diff as JSON for pipelines ``` `--diff` exits with code `2` if new findings appeared since the baseline, `0` @@ -286,7 +286,7 @@ A sample plain-text report (pathnames redacted) as it appears on Linux: ``` $ ./sentrix --quick -epoch:1785838014 +scan time: 2026-09-22T15:33:00Z (epoch:1785838014) == Suspicious process locations == [!] PID 1831 is executing a deleted binary: /root/.opencode/bin/opencode (deleted) — common dropper/rootkit trick @@ -337,6 +337,8 @@ All tunable constants live in `src/config.rs` and can be overridden via a TOML c | `SUSPICIOUS_PLIST_PATTERNS` | macOS plist content patterns to flag | 4 patterns | | `SUSPICIOUS_CRON_PATTERNS` | macOS crontab entry patterns to flag | 4 patterns | | `SUSPICIOUS_LAUNCHCTL_OUTPUT` | macOS launchctl label patterns to flag | 6 patterns | +| `MACOS_NETWORK_EXT_PATTERNS` | macOS network/system extension name patterns to flag | 7 patterns | +| `MACOS_NETWORK_EXT_ALLOWLIST` | macOS extension identifier prefixes exempt from flagging | 21 prefixes | | `SHELL_RC_FILES` | Linux shell rc files to inspect | `.bashrc`, `.profile` | | `PERSISTENCE_SCAN_DIRS` | Linux dirs to scan for recent modifications | `/etc`, `/usr/local/bin` | @@ -398,9 +400,12 @@ cargo test -- --nocapture # show println! output **Current status:** `tests/integration.rs` is populated with 17 integration tests covering `Report` behavior (severity markers, JSON round-trip, sorted -entries), config loading (valid, empty, malformed), the recent-files scanner, -pattern constants, config override flow, and `--diff` comparisons. Unit tests -for `config_loader` are also present. Total: 20 tests passing. +entries), config loading (valid, empty, malformed), the recent-files scanner +(including nested directories and the depth cap), pattern constants, config +override flow, and `--diff` comparisons. Unit tests also cover `config_loader`, +`Report` rendering/round-trips, the ISO-8601 timestamp conversion, and the +Windows output helpers (UTF-16 decoding, quote-aware CSV). Total: 28 tests +passing. CI measures coverage with `cargo-llvm-cov` and uploads it to Codecov. --- @@ -423,7 +428,7 @@ for `config_loader` are also present. Total: 20 tests passing. | 3 | Windows/macOS parity (schtasks, launchctl, WMI) | ✅ Complete | | 4 | Configurable detection patterns (external TOML/YAML) | ✅ Complete | | 5 | Structured output (`--json`, severity levels) | ✅ Complete | -| 6 | Test coverage (unit tests, tarpaulin/grcov, badge) | ✅ Complete | +| 6 | Test coverage (unit tests, cargo-llvm-cov in CI, Codecov upload) | ✅ Complete | | 7 | Nice-to-haves (`--diff`, `CONTRIBUTING.md`) | ✅ Complete | See [docs/PROGRESS.md](docs/PROGRESS.md#roadmap-status) for detailed status, diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c222d81..7e70c83 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -168,34 +168,28 @@ The tradeoff is more code (e.g., manual timestamp formatting instead of --- -## Why `Report` Uses `Vec` Instead of Structured Data? +## Why `Report` Stores Structured Entries, Not Rendered Strings -Findings are stored as formatted strings, not as structured enums/structs. - -**Why (for now):** The original design was a simple text reporter. This -was kept for v0.1.0 to avoid over-engineering before the check set is -stable. - -**Future improvement:** Once the check set stabilizes, `Report` should use -structured findings: +Findings are stored once, as structured `Entry` values (severity, message, +section), plus an ordered list of section titles. The plain-text view +(`join()`) and the JSON view (`to_json()`) are *derived* at output time from +that single source of truth, so the rendered text can never drift out of +sync with the entries it came from. ```rust enum Severity { Info, Warning, Critical } -struct Finding { +struct Entry { severity: Severity, - category: String, + section: String, message: String, - source: String, } ``` This enables JSON/SARIF output, filtering by severity, and programmatic -consumption. The current string-based approach is a placeholder. - -> **Roadmap:** Structured output (`--json`) is tracked as priority #5 in -> [PROGRESS.md](PROGRESS.md#5-structured-output---json). The `Severity` -> enum and `Finding` struct shown above are the planned implementation. +consumption. (An earlier iteration stored pre-rendered `Vec` lines +alongside the entries — duplicated state that could and did drift; it was +removed once the entry set stabilized.) --- @@ -237,5 +231,5 @@ a triage scanner should be predictable and debuggable. | `platform/` with `#[cfg]` | Clean compile-time platform dispatch | | `scanner/` orchestration | Thin layer, easy to add/remove checks | | Zero deps (except `winreg`) | Security, auditability, tiny binary | -| String-based `Report` | Simplicity for v0.1.0, structured later | +| Structured `Report` entries | Single source of truth; text/JSON derived | | Linear scan flow | Predictable, debuggable, no concurrency bugs |