diff --git a/man/man1/soroban-debug-analyze.1 b/man/man1/soroban-debug-analyze.1 index 31f054ff..80f33fc0 100644 --- a/man/man1/soroban-debug-analyze.1 +++ b/man/man1/soroban-debug-analyze.1 @@ -33,8 +33,20 @@ Enable only the specified rule id(s). Repeatable \fB\-\-disable\-rule\fR \fI\fR Disable the specified rule id(s). Repeatable .TP -\fB\-\-min\-severity\fR \fI\fR [default: low] +\fB\-\-min\-severity\fR \fI\fR [default: low] Minimum severity to include: low, medium, or high +.br + +.br +\fIPossible values:\fR +.RS 14 +.IP \(bu 2 +low +.IP \(bu 2 +medium +.IP \(bu 2 +high +.RE .TP \fB\-h\fR, \fB\-\-help\fR Print help diff --git a/src/analyzer/security.rs b/src/analyzer/security.rs index 0eab5edd..ea736b27 100644 --- a/src/analyzer/security.rs +++ b/src/analyzer/security.rs @@ -236,6 +236,12 @@ impl SecurityAnalyzer { suppressed_count, }; } + + /// Get the list of registered security rules. + /// Returns a vector of references to the rule trait objects. + pub fn get_rules(&self) -> Vec<&dyn SecurityRule> { + self.rules.iter().map(|r| r.as_ref()).collect() + } } impl Default for SecurityAnalyzer { diff --git a/src/cli/args.rs b/src/cli/args.rs index 2fa7241d..f57563f0 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -87,6 +87,16 @@ pub enum SnapshotCompression { Zstd, } +/// Minimum severity level for security findings. +/// Used with the `analyze` command to filter results by severity. +#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum, Default)] +pub enum MinSeverity { + #[default] + Low, + Medium, + High, +} + impl Verbosity { /// Convert verbosity to log level string for RUST_LOG pub fn to_log_level(self) -> String { @@ -755,7 +765,7 @@ pub struct OptimizeArgs { #[cfg(test)] mod tests { - use super::{Cli, Commands, OutputFormat, SymbolicProfile}; + use super::{Cli, Commands, OutputFormat, SymbolicProfile, MinSeverity}; use clap::Parser; #[test] @@ -1457,8 +1467,8 @@ pub struct AnalyzeArgs { pub disable_rule: Vec, /// Minimum severity to include: low, medium, or high. - #[arg(long, default_value = "low", value_name = "SEVERITY")] - pub min_severity: String, + #[arg(long, value_enum, default_value_t = MinSeverity::Low)] + pub min_severity: MinSeverity, } #[derive(Parser)] diff --git a/src/cli/commands.rs b/src/cli/commands.rs index 7d85a3d2..ac9d3c5d 100644 --- a/src/cli/commands.rs +++ b/src/cli/commands.rs @@ -205,17 +205,96 @@ fn symbolic_config_from_args(args: &SymbolicArgs) -> Result { Ok(config) } -fn parse_min_severity(value: &str) -> Result { - match value.to_ascii_lowercase().as_str() { - "low" => Ok(crate::analyzer::security::Severity::Low), - "medium" | "med" => Ok(crate::analyzer::security::Severity::Medium), - "high" => Ok(crate::analyzer::security::Severity::High), - other => Err(DebuggerError::InvalidArguments(format!( - "Unsupported --min-severity '{}'. Use low, medium, or high.", - other - )) - .into()), +/// Convert MinSeverity enum to analyzer Severity enum. +fn convert_min_severity(value: crate::cli::args::MinSeverity) -> crate::analyzer::security::Severity { + match value { + crate::cli::args::MinSeverity::Low => crate::analyzer::security::Severity::Low, + crate::cli::args::MinSeverity::Medium => crate::analyzer::security::Severity::Medium, + crate::cli::args::MinSeverity::High => crate::analyzer::security::Severity::High, + } +} + +/// Find the closest matching rule IDs using Levenshtein distance. +fn suggest_rule_ids(unknown: &str, known_rules: &[String], max_distance: usize) -> Vec { + use std::cmp; + + // Calculate Levenshtein distance between two strings + let levenshtein = |a: &str, b: &str| { + let a_len = a.len(); + let b_len = b.len(); + let mut matrix = vec![vec![0; b_len + 1]; a_len + 1]; + + for i in 0..=a_len { + matrix[i][0] = i; + } + for j in 0..=b_len { + matrix[0][j] = j; + } + + for (i, a_char) in a.chars().enumerate() { + for (j, b_char) in b.chars().enumerate() { + let cost = if a_char == b_char { 0 } else { 1 }; + matrix[i + 1][j + 1] = cmp::min( + cmp::min( + matrix[i][j + 1] + 1, // deletion + matrix[i + 1][j] + 1, // insertion + ), + matrix[i][j] + cost, // substitution + ); + } + } + matrix[a_len][b_len] + }; + + let mut suggestions: Vec<_> = known_rules + .iter() + .map(|rule| { + let distance = levenshtein(&unknown.to_lowercase(), &rule.to_lowercase()); + (distance, rule.clone()) + }) + .filter(|(distance, _)| *distance <= max_distance) + .collect(); + + suggestions.sort_by_key(|(distance, _)| *distance); + suggestions.into_iter().map(|(_, rule)| rule).collect() +} + +/// Validate rule IDs in enable_rules and disable_rules lists. +fn validate_rule_ids( + enable_rules: &[String], + disable_rules: &[String], + registered_rules: &[String], +) -> Result<()> { + let mut invalid_rules = Vec::new(); + + // Check enable_rules + for rule in enable_rules { + if !registered_rules.contains(rule) { + invalid_rules.push(("enable", rule.clone())); + } + } + + // Check disable_rules + for rule in disable_rules { + if !registered_rules.contains(rule) { + invalid_rules.push(("disable", rule.clone())); + } + } + + if !invalid_rules.is_empty() { + let mut message = String::from("Invalid rule IDs provided:\n"); + for (filter_type, rule) in &invalid_rules { + message.push_str(&format!(" --{}-rule '{}': not found\n", filter_type, rule)); + let suggestions = suggest_rule_ids(rule, registered_rules, 2); + if !suggestions.is_empty() { + message.push_str(&format!(" Did you mean: {}?\n", suggestions.join(", "))); + } + } + message.push_str(&format!("\nAvailable rules: {}\n", registered_rules.join(", "))); + return Err(DebuggerError::InvalidArguments(message).into()); } + + Ok(()) } fn render_security_report(output: &AnalyzeCommandOutput) -> String { @@ -2791,10 +2870,20 @@ pub fn analyze(args: AnalyzeArgs, _verbosity: Verbosity) -> Result<()> { analyzer = analyzer.load_suppressions_from_file(&supp_path)?; } } + // Get registered rule IDs for validation + let registered_rules: Vec = analyzer + .get_rules() + .iter() + .map(|rule| rule.id().to_string()) + .collect(); + + // Validate rule IDs + validate_rule_ids(&args.enable_rule, &args.disable_rule, ®istered_rules)?; + let filter = crate::analyzer::security::AnalyzerFilter { enable_rules: args.enable_rule.clone(), disable_rules: args.disable_rule.clone(), - min_severity: parse_min_severity(&args.min_severity)?, + min_severity: convert_min_severity(args.min_severity), }; let contract_path = args.contract.to_string_lossy().to_string(); let report = analyzer.analyze( diff --git a/tests/schemas/symbolic_replay_bundle.json b/tests/schemas/symbolic_replay_bundle.json new file mode 100644 index 00000000..fae57c43 --- /dev/null +++ b/tests/schemas/symbolic_replay_bundle.json @@ -0,0 +1,116 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Soroban Debugger Symbolic Replay Bundle", + "description": "JSON schema for symbolic replay bundles exported from the soroban-debug symbolic analyzer. This bundle captures the configuration and metadata needed to reproduce a symbolic execution run.", + "type": "object", + "required": ["schema_version", "command", "contract", "invocation", "config"], + "properties": { + "schema_version": { + "type": "integer", + "description": "The version of this schema. Current version is 1. Used to track compatibility and allow evolution of the bundle format.", + "minimum": 1, + "maximum": 1 + }, + "command": { + "type": "string", + "description": "The command that generated this bundle. Always 'symbolic' for replay bundles.", + "enum": ["symbolic"] + }, + "contract": { + "type": "object", + "description": "Information about the contract being analyzed.", + "required": ["sha256"], + "properties": { + "sha256": { + "type": "string", + "description": "SHA-256 hash of the contract WASM file. Used to verify that the bundle applies to the correct contract.", + "pattern": "^[a-f0-9]{64}$" + }, + "path_hint": { + "type": ["string", "null"], + "description": "Optional file path hint for locating the original contract WASM file. Not authoritative; use sha256 for verification." + } + } + }, + "invocation": { + "type": "object", + "description": "Information about the function being analyzed.", + "required": ["function"], + "properties": { + "function": { + "type": "string", + "description": "Name of the exported contract function that was analyzed." + } + } + }, + "config": { + "type": "object", + "description": "Symbolic exploration configuration. All fields are optional and default to the balanced profile when omitted.", + "properties": { + "seed": { + "type": ["integer", "null"], + "description": "Seed used to shuffle input combinations during exploration. When provided, ensures deterministic and reproducible runs. Passing this value to --replay or --seed reproduces the identical exploration order.", + "minimum": 0 + }, + "max_paths": { + "type": ["integer", "null"], + "description": "Maximum number of distinct execution paths to record. Exploration stops when this limit is reached.", + "minimum": 1 + }, + "max_input_combinations": { + "type": ["integer", "null"], + "description": "Maximum number of input combinations to generate deterministically before exploration begins.", + "minimum": 1 + }, + "max_breadth": { + "type": ["integer", "null"], + "description": "Legacy alias for controlling generated-value branching width. Preserved for backward compatibility.", + "minimum": 1 + }, + "max_depth": { + "type": ["integer", "null"], + "description": "Maximum recursion depth for symbolic exploration.", + "minimum": 1 + }, + "timeout_secs": { + "type": ["integer", "null"], + "description": "Timeout in seconds for the entire symbolic exploration. Exploration stops when this limit is reached.", + "minimum": 0 + } + } + }, + "storage_seed": { + "type": ["object", "null"], + "description": "Optional initial storage state to seed the symbolic exploration. Allows testing how different storage states affect contract behavior.", + "properties": { + "format": { + "type": "string", + "description": "Format of the storage seed data. Currently only 'json' is supported.", + "enum": ["json"] + }, + "data": { + "type": "string", + "description": "Storage seed data as a JSON string. Should be a valid JSON object representing the key-value storage state." + } + }, + "required": ["format", "data"] + }, + "metadata": { + "type": ["object", "null"], + "description": "Optional metadata about the execution results.", + "properties": { + "paths_explored": { + "type": "integer", + "description": "Number of distinct execution paths discovered.", + "minimum": 0 + }, + "panics_found": { + "type": "integer", + "description": "Number of execution paths that resulted in panics.", + "minimum": 0 + } + }, + "required": ["paths_explored", "panics_found"] + } + } +} diff --git a/tests/symbolic_input_tests.rs b/tests/symbolic_input_tests.rs index f1c5f83d..b46a3df1 100644 --- a/tests/symbolic_input_tests.rs +++ b/tests/symbolic_input_tests.rs @@ -1,15 +1,103 @@ // tests/symbolic_input_tests.rs -// Test the symbolic input generation logic. +// Test the symbolic input generation logic and CLI argument validation. #[cfg(test)] mod tests { - // We need to access SymbolicConfig and generate_seeds_for_type. - // Since symbolic.rs is part of the lib, we can use it if it's public. + use soroban_debugger::cli::args::{Cli, Commands, SymbolicArgs}; + use clap::Parser; - // BUT wait, generate_seeds_for_type is private. - // I should test it via a public API or make it public for tests. + /// Test that --seed and --replay are mutually exclusive. + #[test] + fn symbolic_seed_and_replay_are_mutually_exclusive() { + let result = Cli::try_parse_from([ + "soroban-debug", + "symbolic", + "--contract", + "contract.wasm", + "--function", + "test", + "--seed", + "12345", + "--replay", + "67890", + ]); - // In src/analyzer/symbolic.rs, generate_seeds_for_type is NOT pub. - // I'll make it pub(crate) so I can test it from integration tests if I use the crate. - // Or I'll just add a unit test in symbolic.rs. + // Clap should reject this due to conflicts_with + assert!(result.is_err(), "Expected error when both --seed and --replay are provided"); + let err = result.unwrap_err(); + let err_msg = err.to_string(); + assert!( + err_msg.contains("seed") && err_msg.contains("replay"), + "Error message should mention both seed and replay flags: {}", + err_msg + ); + } + + /// Test that --seed alone is accepted. + #[test] + fn symbolic_seed_alone_is_valid() { + let cli = Cli::try_parse_from([ + "soroban-debug", + "symbolic", + "--contract", + "contract.wasm", + "--function", + "test", + "--seed", + "12345", + ]) + .expect("Failed to parse CLI with --seed"); + + let Commands::Symbolic(args) = cli.command.expect("symbolic command expected") else { + panic!("symbolic command expected"); + }; + + assert_eq!(args.seed, Some(12345)); + assert_eq!(args.replay, None); + } + + /// Test that --replay alone is accepted. + #[test] + fn symbolic_replay_alone_is_valid() { + let cli = Cli::try_parse_from([ + "soroban-debug", + "symbolic", + "--contract", + "contract.wasm", + "--function", + "test", + "--replay", + "67890", + ]) + .expect("Failed to parse CLI with --replay"); + + let Commands::Symbolic(args) = cli.command.expect("symbolic command expected") else { + panic!("symbolic command expected"); + }; + + assert_eq!(args.replay, Some(67890)); + assert_eq!(args.seed, None); + } + + /// Test that neither --seed nor --replay is required. + #[test] + fn symbolic_neither_seed_nor_replay_is_valid() { + let cli = Cli::try_parse_from([ + "soroban-debug", + "symbolic", + "--contract", + "contract.wasm", + "--function", + "test", + ]) + .expect("Failed to parse CLI without --seed or --replay"); + + let Commands::Symbolic(args) = cli.command.expect("symbolic command expected") else { + panic!("symbolic command expected"); + }; + + assert_eq!(args.seed, None); + assert_eq!(args.replay, None); + } } +