From 0eaa21a3ff6e2a8e045acdf92f198168dd35cfd3 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Mon, 2 Jun 2025 13:46:57 +0300 Subject: [PATCH] diagnostic: update configuration options to use clap's `ArgAction` --- src/diagnostic.rs | 129 ++++++++++++++++++++++++++++++++++++++++++---- src/main.rs | 8 +-- 2 files changed, 122 insertions(+), 15 deletions(-) diff --git a/src/diagnostic.rs b/src/diagnostic.rs index c12b927..a3a46b0 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -607,6 +607,11 @@ impl AnalyzerModule for StyleAnalyzer { fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec { let mut diagnostics = Vec::new(); + // Only perform style analysis if enabled + if !config.enable_style_warnings { + return diagnostics; + } + // Check for missing shebang if !source.starts_with("#!") { let range = Range::new(Position::new(0, 0), Position::new(0, 0)); @@ -802,14 +807,29 @@ impl StyleAnalyzer { pub struct SemanticAnalyzer; impl AnalyzerModule for SemanticAnalyzer { - fn analyze(&self, source: &str, _config: &DiagnosticConfig) -> Vec { + fn analyze(&self, source: &str, config: &DiagnosticConfig) -> Vec { let mut diagnostics = Vec::new(); - // Parse and validate nftables-specific constructs + // Always run semantic validation (syntax/semantic errors) diagnostics.extend(self.validate_table_declarations(source)); - diagnostics.extend(self.validate_chain_declarations(source)); + diagnostics.extend(self.validate_chain_declarations_semantic(source)); diagnostics.extend(self.validate_cidr_notation(source)); - diagnostics.extend(self.check_for_redundant_rules(source)); + + // Best practices checks (only if enabled) + if config.enable_best_practices { + diagnostics.extend(self.validate_chain_best_practices(source)); + diagnostics.extend(self.check_for_redundant_rules(source)); + } + + // Performance hints (only if enabled) + if config.enable_performance_hints { + diagnostics.extend(self.check_performance_hints(source)); + } + + // Security warnings (only if enabled) + if config.enable_security_warnings { + diagnostics.extend(self.check_security_warnings(source)); + } diagnostics } @@ -880,7 +900,7 @@ impl SemanticAnalyzer { diagnostics } - fn validate_chain_declarations(&self, source: &str) -> Vec { + fn validate_chain_declarations_semantic(&self, source: &str) -> Vec { let mut diagnostics = Vec::new(); for (line_idx, line) in source.lines().enumerate() { @@ -888,7 +908,7 @@ impl SemanticAnalyzer { let trimmed = line.trim(); if trimmed.starts_with("type ") && trimmed.contains("hook") { - // Validate chain type and hook + // Validate chain type and hook (semantic validation) if let Some(hook_pos) = trimmed.find("hook") { let hook_part = &trimmed[hook_pos..]; let hook_words: Vec<&str> = hook_part.split_whitespace().collect(); @@ -916,22 +936,109 @@ impl SemanticAnalyzer { } } } + } + } - // Check for missing policy in filter chains - if trimmed.contains("type filter") && !trimmed.contains("policy") { + diagnostics + } + + fn validate_chain_best_practices(&self, source: &str) -> Vec { + let mut diagnostics = Vec::new(); + + for (line_idx, line) in source.lines().enumerate() { + let line_num = line_idx as u32; + let trimmed = line.trim(); + + // Check for missing policy in filter chains (best practice) + if trimmed.contains("type filter") && !trimmed.contains("policy") { + let range = Range::new( + Position::new(line_num, 0), + Position::new(line_num, line.len() as u32), + ); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Warning, + DiagnosticCode::ChainWithoutPolicy, + "Filter chain should have an explicit policy".to_string(), + ); + diagnostics.push(diagnostic); + } + } + + diagnostics + } + + fn check_performance_hints(&self, source: &str) -> Vec { + let mut diagnostics = Vec::new(); + + for (line_idx, line) in source.lines().enumerate() { + let line_num = line_idx as u32; + let trimmed = line.trim(); + + // Check for large sets without timeout (performance hint) + if trimmed.contains("set ") && trimmed.contains("{") && !trimmed.contains("timeout") { + // Simple heuristic: if set definition is long, suggest timeout + if trimmed.len() > 100 { let range = Range::new( Position::new(line_num, 0), Position::new(line_num, line.len() as u32), ); let diagnostic = Diagnostic::new( range, - DiagnosticSeverity::Warning, - DiagnosticCode::ChainWithoutPolicy, - "Filter chain should have an explicit policy".to_string(), + DiagnosticSeverity::Hint, + DiagnosticCode::LargeSetWithoutTimeout, + "Consider adding a timeout to large sets for better performance" + .to_string(), ); diagnostics.push(diagnostic); } } + + // Check for missing counters (performance hint) + if (trimmed.contains(" accept") || trimmed.contains(" drop")) + && !trimmed.contains("counter") + { + let range = Range::new( + Position::new(line_num, 0), + Position::new(line_num, line.len() as u32), + ); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Hint, + DiagnosticCode::MissingCounters, + "Consider adding counters to rules for monitoring and debugging".to_string(), + ); + diagnostics.push(diagnostic); + } + } + + diagnostics + } + + fn check_security_warnings(&self, source: &str) -> Vec { + let mut diagnostics = Vec::new(); + + for (line_idx, line) in source.lines().enumerate() { + let line_num = line_idx as u32; + let trimmed = line.trim(); + + // Check for overly permissive rules (security warning) + if trimmed.contains(" accept") + && (trimmed.contains("0.0.0.0/0") || trimmed.contains("::/0")) + { + let range = Range::new( + Position::new(line_num, 0), + Position::new(line_num, line.len() as u32), + ); + let diagnostic = Diagnostic::new( + range, + DiagnosticSeverity::Warning, + DiagnosticCode::SecurityRisk, + "Rule accepts traffic from anywhere - consider restricting source addresses" + .to_string(), + ); + diagnostics.push(diagnostic); + } } diagnostics diff --git a/src/main.rs b/src/main.rs index 87a35f8..e7fa847 100755 --- a/src/main.rs +++ b/src/main.rs @@ -93,19 +93,19 @@ enum Commands { json: bool, /// Include style warnings in diagnostics - #[arg(long, default_value = "true")] + #[arg(long, action = clap::ArgAction::Set, default_value = "true")] style_warnings: bool, /// Include best practice recommendations in diagnostics - #[arg(long, default_value = "true")] + #[arg(long, action = clap::ArgAction::Set, default_value = "true")] best_practices: bool, /// Include performance hints in diagnostics - #[arg(long, default_value = "true")] + #[arg(long, action = clap::ArgAction::Set, default_value = "true")] performance_hints: bool, /// Include security warnings in diagnostics - #[arg(long, default_value = "true")] + #[arg(long, action = clap::ArgAction::Set, default_value = "true")] security_warnings: bool, /// Diagnostic modules to run (comma-separated: lexical,syntax,style,semantic)