From 716629809cb8d3695e8342c3ade39fb211494837 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sun, 15 Mar 2026 03:17:03 +0000 Subject: [PATCH] fix: eliminate panic paths in production code (#1184) * fix: eliminate panic paths in production code and document infallible operations PolicyRule::new() now returns Result instead of panicking on invalid caller-supplied regex. CreateJobTool returns ToolError when job_manager is unconfigured instead of panicking. Remaining infallible unwrap/expect calls (hardcoded regexes, compile-time constants, guarded accesses) are annotated with SAFETY comments. Where possible, unwraps are replaced with safer patterns: split_last(), if-let, match-destructure, and reusing peek() values. Co-Authored-By: Claude Opus 4.6 (1M context) * fix: use inline lowercase safety comments to match CI pattern The no-panics CI check greps for '// safety:' (lowercase, inline) to suppress false positives. Switch from block SAFETY comments to inline safety comments on the .unwrap() lines. Co-Authored-By: Claude Opus 4.6 (1M context) * test: add regression tests for panic-path fixes - PolicyRule::new returns Err on invalid regex (not panic) - CreateJobTool::execute_sandbox returns ToolError when job_manager is None Co-Authored-By: Claude Opus 4.6 (1M context) * fix: add inline // safety: comments on all infallible unwrap/expect lines The CI no-panics check requires '// safety:' on the same line as unwrap()/expect() to suppress false positives. Move safety annotations from block comments to inline comments on every infallible production unwrap/expect across all touched files. Co-Authored-By: Claude Opus 4.6 (1M context) * chore: trigger CI with skip-regression-check label [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) * refactor: remove redundant block-level SAFETY comments Each unwrap/expect now carries its own inline // safety: annotation, making the standalone block comments above them redundant. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- crates/ironclaw_safety/src/leak_detector.rs | 32 ++-- crates/ironclaw_safety/src/policy.rs | 156 +++++++++++++------- crates/ironclaw_safety/src/sanitizer.rs | 12 +- src/agent/session.rs | 7 +- src/channels/signal.rs | 2 +- src/document_extraction/extractors.rs | 3 +- src/extensions/manager.rs | 22 ++- src/llm/reasoning.rs | 8 +- src/llm/registry.rs | 2 +- src/llm/smart_routing.rs | 43 +++--- src/settings.rs | 9 +- src/setup/channels.rs | 2 +- src/skills/mod.rs | 8 +- src/tools/builtin/job.rs | 31 +++- src/tools/mcp/http_transport.rs | 2 +- src/tools/wasm/wrapper.rs | 2 +- src/workspace/chunker.rs | 5 +- 17 files changed, 216 insertions(+), 130 deletions(-) diff --git a/crates/ironclaw_safety/src/leak_detector.rs b/crates/ironclaw_safety/src/leak_detector.rs index 99794a25..89753940 100644 --- a/crates/ironclaw_safety/src/leak_detector.rs +++ b/crates/ironclaw_safety/src/leak_detector.rs @@ -417,105 +417,105 @@ fn default_patterns() -> Vec { // OpenAI API keys LeakPattern { name: "openai_api_key".to_string(), - regex: Regex::new(r"sk-(?:proj-)?[a-zA-Z0-9]{20,}(?:T3BlbkFJ[a-zA-Z0-9_-]*)?").unwrap(), + regex: Regex::new(r"sk-(?:proj-)?[a-zA-Z0-9]{20,}(?:T3BlbkFJ[a-zA-Z0-9_-]*)?").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Critical, action: LeakAction::Block, }, // Anthropic API keys LeakPattern { name: "anthropic_api_key".to_string(), - regex: Regex::new(r"sk-ant-api[a-zA-Z0-9_-]{90,}").unwrap(), + regex: Regex::new(r"sk-ant-api[a-zA-Z0-9_-]{90,}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Critical, action: LeakAction::Block, }, // AWS Access Key ID LeakPattern { name: "aws_access_key".to_string(), - regex: Regex::new(r"AKIA[0-9A-Z]{16}").unwrap(), + regex: Regex::new(r"AKIA[0-9A-Z]{16}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Critical, action: LeakAction::Block, }, // GitHub tokens LeakPattern { name: "github_token".to_string(), - regex: Regex::new(r"gh[pousr]_[A-Za-z0-9_]{36,}").unwrap(), + regex: Regex::new(r"gh[pousr]_[A-Za-z0-9_]{36,}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Critical, action: LeakAction::Block, }, // GitHub fine-grained PAT LeakPattern { name: "github_fine_grained_pat".to_string(), - regex: Regex::new(r"github_pat_[a-zA-Z0-9]{22}_[a-zA-Z0-9]{59}").unwrap(), + regex: Regex::new(r"github_pat_[a-zA-Z0-9]{22}_[a-zA-Z0-9]{59}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Critical, action: LeakAction::Block, }, // Stripe keys LeakPattern { name: "stripe_api_key".to_string(), - regex: Regex::new(r"sk_(?:live|test)_[a-zA-Z0-9]{24,}").unwrap(), + regex: Regex::new(r"sk_(?:live|test)_[a-zA-Z0-9]{24,}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Critical, action: LeakAction::Block, }, // NEAR AI session tokens LeakPattern { name: "nearai_session".to_string(), - regex: Regex::new(r"sess_[a-zA-Z0-9]{32,}").unwrap(), + regex: Regex::new(r"sess_[a-zA-Z0-9]{32,}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Critical, action: LeakAction::Block, }, // PEM private keys LeakPattern { name: "pem_private_key".to_string(), - regex: Regex::new(r"-----BEGIN\s+(?:RSA\s+)?PRIVATE\s+KEY-----").unwrap(), + regex: Regex::new(r"-----BEGIN\s+(?:RSA\s+)?PRIVATE\s+KEY-----").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Critical, action: LeakAction::Block, }, // SSH private keys LeakPattern { name: "ssh_private_key".to_string(), - regex: Regex::new(r"-----BEGIN\s+(?:OPENSSH|EC|DSA)\s+PRIVATE\s+KEY-----").unwrap(), + regex: Regex::new(r"-----BEGIN\s+(?:OPENSSH|EC|DSA)\s+PRIVATE\s+KEY-----").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Critical, action: LeakAction::Block, }, // Google API keys LeakPattern { name: "google_api_key".to_string(), - regex: Regex::new(r"AIza[0-9A-Za-z_-]{35}").unwrap(), + regex: Regex::new(r"AIza[0-9A-Za-z_-]{35}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::High, action: LeakAction::Block, }, // Slack tokens LeakPattern { name: "slack_token".to_string(), - regex: Regex::new(r"xox[baprs]-[0-9a-zA-Z-]{10,}").unwrap(), + regex: Regex::new(r"xox[baprs]-[0-9a-zA-Z-]{10,}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::High, action: LeakAction::Block, }, // Twilio API keys LeakPattern { name: "twilio_api_key".to_string(), - regex: Regex::new(r"SK[a-fA-F0-9]{32}").unwrap(), + regex: Regex::new(r"SK[a-fA-F0-9]{32}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::High, action: LeakAction::Block, }, // SendGrid API keys LeakPattern { name: "sendgrid_api_key".to_string(), - regex: Regex::new(r"SG\.[a-zA-Z0-9_-]{22}\.[a-zA-Z0-9_-]{43}").unwrap(), + regex: Regex::new(r"SG\.[a-zA-Z0-9_-]{22}\.[a-zA-Z0-9_-]{43}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::High, action: LeakAction::Block, }, // Bearer tokens (redact instead of block, might be intentional) LeakPattern { name: "bearer_token".to_string(), - regex: Regex::new(r"Bearer\s+[a-zA-Z0-9_-]{20,}").unwrap(), + regex: Regex::new(r"Bearer\s+[a-zA-Z0-9_-]{20,}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::High, action: LeakAction::Redact, }, // Authorization header with key LeakPattern { name: "auth_header".to_string(), - regex: Regex::new(r"(?i)authorization:\s*[a-zA-Z]+\s+[a-zA-Z0-9_-]{20,}").unwrap(), + regex: Regex::new(r"(?i)authorization:\s*[a-zA-Z]+\s+[a-zA-Z0-9_-]{20,}").unwrap(), // safety: hardcoded literal severity: LeakSeverity::High, action: LeakAction::Redact, }, @@ -524,7 +524,7 @@ fn default_patterns() -> Vec { // This catches standalone 64-char hex strings (like SHA256 hashes used as secrets). LeakPattern { name: "high_entropy_hex".to_string(), - regex: Regex::new(r"\b[a-fA-F0-9]{64}\b").unwrap(), + regex: Regex::new(r"\b[a-fA-F0-9]{64}\b").unwrap(), // safety: hardcoded literal severity: LeakSeverity::Medium, action: LeakAction::Warn, }, diff --git a/crates/ironclaw_safety/src/policy.rs b/crates/ironclaw_safety/src/policy.rs index db27007b..667c7bfb 100644 --- a/crates/ironclaw_safety/src/policy.rs +++ b/crates/ironclaw_safety/src/policy.rs @@ -54,20 +54,22 @@ pub struct PolicyRule { impl PolicyRule { /// Create a new policy rule. + /// + /// Returns an error if `pattern` is not a valid regex. pub fn new( id: impl Into, description: impl Into, pattern: &str, severity: Severity, action: PolicyAction, - ) -> Self { - Self { + ) -> Result { + Ok(Self { id: id.into(), description: description.into(), severity, - pattern: Regex::new(pattern).expect("Invalid policy regex"), + pattern: Regex::new(pattern)?, action, - } + }) } /// Check if content matches this rule. @@ -130,72 +132,93 @@ impl Default for Policy { fn default() -> Self { let mut policy = Self::new(); - // Add default rules + // All regex patterns below are hardcoded literals validated by tests. // Block attempts to access system files - policy.add_rule(PolicyRule::new( - "system_file_access", - "Attempt to access system files", - r"(?i)(/etc/passwd|/etc/shadow|\.ssh/|\.aws/credentials)", - Severity::Critical, - PolicyAction::Block, - )); + policy.add_rule( + PolicyRule::new( + "system_file_access", + "Attempt to access system files", + r"(?i)(/etc/passwd|/etc/shadow|\.ssh/|\.aws/credentials)", + Severity::Critical, + PolicyAction::Block, + ) + .unwrap(), // safety: hardcoded regex literal + ); // Block cryptocurrency private key patterns - policy.add_rule(PolicyRule::new( - "crypto_private_key", - "Potential cryptocurrency private key", - r"(?i)(private.?key|seed.?phrase|mnemonic).{0,20}[0-9a-f]{64}", - Severity::Critical, - PolicyAction::Block, - )); + policy.add_rule( + PolicyRule::new( + "crypto_private_key", + "Potential cryptocurrency private key", + r"(?i)(private.?key|seed.?phrase|mnemonic).{0,20}[0-9a-f]{64}", + Severity::Critical, + PolicyAction::Block, + ) + .unwrap(), // safety: hardcoded regex literal + ); // Warn on SQL-like patterns - policy.add_rule(PolicyRule::new( - "sql_pattern", - "SQL-like pattern detected", - r"(?i)(DROP\s+TABLE|DELETE\s+FROM|INSERT\s+INTO|UPDATE\s+\w+\s+SET)", - Severity::Medium, - PolicyAction::Warn, - )); + policy.add_rule( + PolicyRule::new( + "sql_pattern", + "SQL-like pattern detected", + r"(?i)(DROP\s+TABLE|DELETE\s+FROM|INSERT\s+INTO|UPDATE\s+\w+\s+SET)", + Severity::Medium, + PolicyAction::Warn, + ) + .unwrap(), // safety: hardcoded regex literal + ); // Block shell command injection patterns. // Only match actual dangerous command sequences, NOT backticked content // (backticks are standard markdown code formatting, not shell injection). - policy.add_rule(PolicyRule::new( - "shell_injection", - "Potential shell command injection", - r"(?i)(;\s*rm\s+-rf|;\s*curl\s+.*\|\s*sh)", - Severity::Critical, - PolicyAction::Block, - )); + policy.add_rule( + PolicyRule::new( + "shell_injection", + "Potential shell command injection", + r"(?i)(;\s*rm\s+-rf|;\s*curl\s+.*\|\s*sh)", + Severity::Critical, + PolicyAction::Block, + ) + .unwrap(), // safety: hardcoded regex literal + ); // Warn on excessive URLs - policy.add_rule(PolicyRule::new( - "excessive_urls", - "Excessive number of URLs detected", - r"(https?://[^\s]+\s*){10,}", - Severity::Low, - PolicyAction::Warn, - )); + policy.add_rule( + PolicyRule::new( + "excessive_urls", + "Excessive number of URLs detected", + r"(https?://[^\s]+\s*){10,}", + Severity::Low, + PolicyAction::Warn, + ) + .unwrap(), // safety: hardcoded regex literal + ); // Block encoded payloads that look like exploits - policy.add_rule(PolicyRule::new( - "encoded_exploit", - "Potential encoded exploit payload", - r"(?i)(base64_decode|eval\s*\(\s*base64|atob\s*\()", - Severity::High, - PolicyAction::Sanitize, - )); + policy.add_rule( + PolicyRule::new( + "encoded_exploit", + "Potential encoded exploit payload", + r"(?i)(base64_decode|eval\s*\(\s*base64|atob\s*\()", + Severity::High, + PolicyAction::Sanitize, + ) + .unwrap(), // safety: hardcoded regex literal + ); // Warn on very long strings without spaces (potential obfuscation) - policy.add_rule(PolicyRule::new( - "obfuscated_string", - "Potential obfuscated content", - r"[^\s]{500,}", - Severity::Medium, - PolicyAction::Warn, - )); + policy.add_rule( + PolicyRule::new( + "obfuscated_string", + "Potential obfuscated content", + r"[^\s]{500,}", + Severity::Medium, + PolicyAction::Warn, + ) + .unwrap(), // safety: hardcoded regex literal + ); policy } @@ -252,4 +275,29 @@ mod tests { assert!(Severity::High > Severity::Medium); assert!(Severity::Medium > Severity::Low); } + + #[test] + fn test_new_returns_error_on_invalid_regex() { + let result = PolicyRule::new( + "bad_rule", + "Invalid regex", + r"[invalid((", + Severity::High, + PolicyAction::Block, + ); + assert!(result.is_err()); + } + + #[test] + fn test_new_returns_ok_on_valid_regex() { + let result = PolicyRule::new( + "good_rule", + "Valid regex", + r"hello\s+world", + Severity::Low, + PolicyAction::Warn, + ); + assert!(result.is_ok()); + assert!(result.unwrap().matches("hello world")); + } } diff --git a/crates/ironclaw_safety/src/sanitizer.rs b/crates/ironclaw_safety/src/sanitizer.rs index fec6636e..ea6804a1 100644 --- a/crates/ironclaw_safety/src/sanitizer.rs +++ b/crates/ironclaw_safety/src/sanitizer.rs @@ -160,30 +160,30 @@ impl Sanitizer { let pattern_matcher = AhoCorasick::builder() .ascii_case_insensitive(true) .build(&pattern_strings) - .expect("Failed to build pattern matcher"); + .expect("Failed to build pattern matcher"); // safety: hardcoded string literals - // Regex patterns for more complex detection + // Regex patterns for more complex detection. let regex_patterns = vec![ RegexPattern { - regex: Regex::new(r"(?i)base64[:\s]+[A-Za-z0-9+/=]{50,}").unwrap(), + regex: Regex::new(r"(?i)base64[:\s]+[A-Za-z0-9+/=]{50,}").unwrap(), // safety: hardcoded literal name: "base64_payload".to_string(), severity: Severity::Medium, description: "Potential encoded payload".to_string(), }, RegexPattern { - regex: Regex::new(r"(?i)eval\s*\(").unwrap(), + regex: Regex::new(r"(?i)eval\s*\(").unwrap(), // safety: hardcoded literal name: "eval_call".to_string(), severity: Severity::High, description: "Potential code evaluation attempt".to_string(), }, RegexPattern { - regex: Regex::new(r"(?i)exec\s*\(").unwrap(), + regex: Regex::new(r"(?i)exec\s*\(").unwrap(), // safety: hardcoded literal name: "exec_call".to_string(), severity: Severity::High, description: "Potential code execution attempt".to_string(), }, RegexPattern { - regex: Regex::new(r"\x00").unwrap(), + regex: Regex::new(r"\x00").unwrap(), // safety: hardcoded literal name: "null_byte".to_string(), severity: Severity::Critical, description: "Null byte injection attempt".to_string(), diff --git a/src/agent/session.rs b/src/agent/session.rs index 193e0309..0c1f1fd3 100644 --- a/src/agent/session.rs +++ b/src/agent/session.rs @@ -92,8 +92,11 @@ impl Session { None => self.create_thread(), Some(id) => { if self.threads.contains_key(&id) { - // Safe: contains_key confirmed the entry exists. - self.threads.get_mut(&id).unwrap() + // Entry existence confirmed by contains_key above. + // get_mut borrows self.threads mutably, so we can't + // combine the check and access into if-let without + // conflicting with the self.create_thread() fallback. + self.threads.get_mut(&id).unwrap() // safety: contains_key guard above } else { // Stale active_thread ID: create a new thread, which // updates self.active_thread to the new thread's ID. diff --git a/src/channels/signal.rs b/src/channels/signal.rs index cc07b079..b8934c5c 100644 --- a/src/channels/signal.rs +++ b/src/channels/signal.rs @@ -32,7 +32,7 @@ const MAX_HTTP_RESPONSE_SIZE: usize = 10 * 1024 * 1024; const MAX_REPLY_TARGETS: usize = 10000; const MAX_ERROR_LOG_BODY: usize = 1024; -const REPLY_TARGETS_CAP: NonZeroUsize = NonZeroUsize::new(MAX_REPLY_TARGETS).unwrap(); +const REPLY_TARGETS_CAP: NonZeroUsize = NonZeroUsize::new(MAX_REPLY_TARGETS).unwrap(); // safety: 10000 is nonzero /// Recipient classification for outbound messages. #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/src/document_extraction/extractors.rs b/src/document_extraction/extractors.rs index ddb30911..5adc9459 100644 --- a/src/document_extraction/extractors.rs +++ b/src/document_extraction/extractors.rs @@ -205,7 +205,8 @@ fn extract_rtf(data: &[u8]) -> Result { let mut word = String::new(); while let Some(&next) = chars.peek() { if next.is_ascii_alphabetic() { - word.push(chars.next().unwrap()); + chars.next(); + word.push(next); } else { break; } diff --git a/src/extensions/manager.rs b/src/extensions/manager.rs index 1d5fb92d..05b07555 100644 --- a/src/extensions/manager.rs +++ b/src/extensions/manager.rs @@ -248,12 +248,14 @@ impl ExtensionManager { self.tunnel_url .as_ref() .filter(|u| !u.is_empty()) - .and_then(|raw| url::Url::parse(raw).ok()) - .and_then(|u| u.host_str().map(String::from)) - .filter(|host| !oauth_defaults::is_loopback_host(host)) - .map(|_| { - let base = self.tunnel_url.as_ref().unwrap().trim_end_matches('/'); - format!("{}/oauth/callback", base) + .and_then(|raw| { + let url = url::Url::parse(raw).ok()?; + let host = url.host_str().map(String::from)?; + if oauth_defaults::is_loopback_host(&host) { + return None; + } + let base = raw.trim_end_matches('/'); + Some(format!("{}/oauth/callback", base)) }) } @@ -1309,8 +1311,12 @@ impl ExtensionManager { match fallback_decision(&primary_result, &entry.fallback_source) { FallbackDecision::Return => primary_result, FallbackDecision::TryFallback => { - let primary_err = primary_result.unwrap_err(); - let fallback = entry.fallback_source.as_ref().unwrap(); + // TryFallback guarantees primary is Err and fallback_source is Some. + let (primary_err, fallback) = match (primary_result, entry.fallback_source.as_ref()) + { + (Err(e), Some(f)) => (e, f), + (other, _) => return other, + }; tracing::info!( extension = %entry.name, primary_error = %primary_err, diff --git a/src/llm/reasoning.rs b/src/llm/reasoning.rs index f2294f58..b00948ae 100644 --- a/src/llm/reasoning.rs +++ b/src/llm/reasoning.rs @@ -155,22 +155,22 @@ pub fn is_silent_reply(text: &str) -> bool { /// Quick-check: bail early if no reasoning/final tags are present at all. static QUICK_TAG_RE: LazyLock = LazyLock::new(|| { - Regex::new(r"(?i)<\s*/?\s*(?:think(?:ing)?|thought|thoughts|antthinking|reasoning|reflection|scratchpad|inner_monologue|final)\b").expect("QUICK_TAG_RE") + Regex::new(r"(?i)<\s*/?\s*(?:think(?:ing)?|thought|thoughts|antthinking|reasoning|reflection|scratchpad|inner_monologue|final)\b").expect("QUICK_TAG_RE") // safety: hardcoded literal }); /// Matches thinking/reasoning open and close tags. Capture group 1 is "/" for close tags. /// Whitespace-tolerant, case-insensitive, attribute-aware. static THINKING_TAG_RE: LazyLock = LazyLock::new(|| { - Regex::new(r"(?i)<\s*(/?)\s*(?:think(?:ing)?|thought|thoughts|antthinking|reasoning|reflection|scratchpad|inner_monologue)\b[^<>]*>").expect("THINKING_TAG_RE") + Regex::new(r"(?i)<\s*(/?)\s*(?:think(?:ing)?|thought|thoughts|antthinking|reasoning|reflection|scratchpad|inner_monologue)\b[^<>]*>").expect("THINKING_TAG_RE") // safety: hardcoded literal }); /// Matches `` / `` tags. Capture group 1 is "/" for close tags. static FINAL_TAG_RE: LazyLock = - LazyLock::new(|| Regex::new(r"(?i)<\s*(/?)\s*final\b[^<>]*>").expect("FINAL_TAG_RE")); + LazyLock::new(|| Regex::new(r"(?i)<\s*(/?)\s*final\b[^<>]*>").expect("FINAL_TAG_RE")); // safety: hardcoded literal /// Matches pipe-delimited reasoning tags: `<|think|>...<|/think|>` etc. static PIPE_REASONING_TAG_RE: LazyLock = LazyLock::new(|| { - Regex::new(r"(?i)<\|(/?)\s*(?:think(?:ing)?|thought|thoughts|antthinking|reasoning|reflection|scratchpad|inner_monologue)\|>").expect("PIPE_REASONING_TAG_RE") + Regex::new(r"(?i)<\|(/?)\s*(?:think(?:ing)?|thought|thoughts|antthinking|reasoning|reflection|scratchpad|inner_monologue)\|>").expect("PIPE_REASONING_TAG_RE") // safety: hardcoded literal }); /// Context for reasoning operations. diff --git a/src/llm/registry.rs b/src/llm/registry.rs index 434c698a..a36e2479 100644 --- a/src/llm/registry.rs +++ b/src/llm/registry.rs @@ -219,7 +219,7 @@ impl ProviderRegistry { pub fn load() -> Self { let builtins: Vec = serde_json::from_str(include_str!("../../providers.json")) - .expect("built-in providers.json must be valid JSON"); + .expect("built-in providers.json must be valid JSON"); // safety: compile-time embedded file let mut all = builtins; diff --git a/src/llm/smart_routing.rs b/src/llm/smart_routing.rs index dbcae429..0c6158f2 100644 --- a/src/llm/smart_routing.rs +++ b/src/llm/smart_routing.rs @@ -248,7 +248,7 @@ fn build_domain_regex(keywords: &[&str]) -> Regex { let pattern = format!(r"(?i)\b({})\b", keywords.join("|")); Regex::new(&pattern).unwrap_or_else(|e| { tracing::warn!(error = %e, "Invalid domain keywords pattern, using minimal fallback"); - Regex::new(r"(?i)\b(api|code|deploy)\b").expect("fallback regex is valid") + Regex::new(r"(?i)\b(api|code|deploy)\b").expect("fallback regex is valid") // safety: hardcoded literal }) } @@ -274,71 +274,71 @@ use std::sync::LazyLock; static RE_REASONING: LazyLock = LazyLock::new(|| { Regex::new( r"(?i)\b(why|how|explain|analyze|analyse|compare|contrast|evaluate|assess|reason|think|consider|implications?|consequences?|trade-?offs?|pros?\s*(and|&)\s*cons?|advantages?|disadvantages?|benefits?|drawbacks?|differs?|difference|versus|vs\.?|better|worse|optimal|best|worst)\b" - ).expect("RE_REASONING is a valid regex") + ).expect("RE_REASONING is a valid regex") // safety: hardcoded literal }); static RE_MULTI_STEP: LazyLock = LazyLock::new(|| { Regex::new( r"(?i)\b(first|then|next|after|before|finally|step|steps|phase|stages?|process|workflow|sequence|procedure|pipeline|chain|series|order|followed by)\b" - ).expect("RE_MULTI_STEP is a valid regex") + ).expect("RE_MULTI_STEP is a valid regex") // safety: hardcoded literal }); static RE_CREATIVITY: LazyLock = LazyLock::new(|| { Regex::new( r"(?i)\b(write|create|generate|compose|design|imagine|brainstorm|ideate|draft|invent|story|poem|essay|article|blog|content|narrative|script|summarize|summarise|rewrite|paraphrase|translate|adapt|tweet|post|thread|outline|structure|format|style|tone|voice)\b" - ).expect("RE_CREATIVITY is a valid regex") + ).expect("RE_CREATIVITY is a valid regex") // safety: hardcoded literal }); static RE_PRECISION: LazyLock = LazyLock::new(|| { Regex::new( r"(?i)\b(\d{4}|\d+\.\d+|exactly|precisely|specific|accurate|correct|verify|confirm|date|time|number|calculate|compute|measure|count)\b" - ).expect("RE_PRECISION is a valid regex") + ).expect("RE_PRECISION is a valid regex") // safety: hardcoded literal }); static RE_CODE: LazyLock = LazyLock::new(|| { Regex::new( r"(?i)(`{1,3}|```|function|const|let|var|import|export|class|def |async|await|=>|\.ts|\.js|\.py|\.rs|\.go|\.sol|\(\)|\[\]|\{\}|<[A-Z][a-z]+>|useState|useEffect|npm|yarn|pnpm|cargo|pip|implement|rebase|merge|commit|branch|PR|pull.?request|columns?|migrations?|module|refactor|debug|fix|bug|error|schema|database|query)" - ).expect("RE_CODE is a valid regex") + ).expect("RE_CODE is a valid regex") // safety: hardcoded literal }); static RE_TOOL: LazyLock = LazyLock::new(|| { Regex::new( r"(?i)\b(file|read|write|search|fetch|run|execute|check|look up|find|open|save|send|post|get|download|upload|install|deploy|build|compile|test|add|update|remove|delete|modify|change|edit|create|resolve|push|pull|clone)\b" - ).expect("RE_TOOL is a valid regex") + ).expect("RE_TOOL is a valid regex") // safety: hardcoded literal }); static RE_SAFETY: LazyLock = LazyLock::new(|| { Regex::new( r"(?i)\b(password|secret|private|confidential|medical|legal|financial|personal|sensitive|ssn|credit.?card|auth|token|key|encrypt|decrypt|hash|vulnerability|exploit|attack|breach)\b" - ).expect("RE_SAFETY is a valid regex") + ).expect("RE_SAFETY is a valid regex") // safety: hardcoded literal }); static RE_CONTEXT: LazyLock = LazyLock::new(|| { Regex::new( r"(?i)\b(previous|earlier|above|before|last|that|those|it|they|we discussed|you said|mentioned|remember|recall|as I said|like I mentioned)\b" - ).expect("RE_CONTEXT is a valid regex") + ).expect("RE_CONTEXT is a valid regex") // safety: hardcoded literal }); static RE_VAGUE: LazyLock = LazyLock::new(|| { Regex::new(r"(?i)\b(it|this|that|something|stuff|thing|things)\b") - .expect("RE_VAGUE is a valid regex") + .expect("RE_VAGUE is a valid regex") // safety: hardcoded literal }); static RE_OPEN_ENDED: LazyLock = LazyLock::new(|| { Regex::new(r"(?i)\b(why|how|what if|explain|describe|elaborate|discuss)\b") - .expect("RE_OPEN_ENDED is a valid regex") + .expect("RE_OPEN_ENDED is a valid regex") // safety: hardcoded literal }); static RE_CONJUNCTIONS: LazyLock = LazyLock::new(|| { Regex::new( r"(?i)\b(and|but|or|however|therefore|because|although|while|whereas|moreover|furthermore)\b", ) - .expect("RE_CONJUNCTIONS is a valid regex") + .expect("RE_CONJUNCTIONS is a valid regex") // safety: hardcoded literal }); static RE_TIER_HINT: LazyLock = LazyLock::new(|| { Regex::new(r"(?i)\[tier:(flash|standard|pro|frontier)\]") - .expect("RE_TIER_HINT is a valid regex") + .expect("RE_TIER_HINT is a valid regex") // safety: hardcoded literal }); /// Default domain regex, compiled once from `DEFAULT_DOMAIN_KEYWORDS`. @@ -363,7 +363,7 @@ static DEFAULT_OVERRIDES: LazyLock> = LazyLock::new(|| { regex: Regex::new( r"(?i)^(hi|hello|hey|thanks|ok|sure|yes|no|yep|nope|cool|nice|great|got it)$", ) - .expect("greeting pattern is valid"), + .expect("greeting pattern is valid"), // safety: hardcoded literal tier: Tier::Flash, }, // Flash tier: quick lookups (end-anchored to avoid matching complex questions @@ -372,29 +372,29 @@ static DEFAULT_OVERRIDES: LazyLock> = LazyLock::new(|| { regex: Regex::new( r"(?i)^what(?:'s|\s+is)?\s+(?:the\s+)?(time|date|day|weather)\b(?:\s+(?:is\s+it|today|now|in\s+\S+))?[?.!]*$", ) - .expect("lookup pattern is valid"), + .expect("lookup pattern is valid"), // safety: hardcoded literal tier: Tier::Flash, }, // Frontier tier: security audits PatternOverride { regex: Regex::new(r"(?i)security.*(audit|review|scan)") - .expect("security audit pattern is valid"), + .expect("security audit pattern is valid"), // safety: hardcoded literal tier: Tier::Frontier, }, PatternOverride { regex: Regex::new(r"(?i)vulnerabilit(y|ies).*(review|scan|check|audit)") - .expect("vulnerability pattern is valid"), + .expect("vulnerability pattern is valid"), // safety: hardcoded literal tier: Tier::Frontier, }, // Pro tier: production deployments PatternOverride { regex: Regex::new(r"(?i)deploy.*(mainnet|production)") - .expect("deploy pattern is valid"), + .expect("deploy pattern is valid"), // safety: hardcoded literal tier: Tier::Pro, }, PatternOverride { regex: Regex::new(r"(?i)production.*(deploy|release|push)") - .expect("production pattern is valid"), + .expect("production pattern is valid"), // safety: hardcoded literal tier: Tier::Pro, }, ] @@ -451,7 +451,7 @@ fn score_complexity_internal( // Check for explicit tier hint (e.g. "[tier:flash]") if let Some(caps) = RE_TIER_HINT.captures(prompt) { - let tier_str = caps.get(1).expect("capture group 1 exists").as_str(); + let tier_str = caps.get(1).expect("capture group 1 exists").as_str(); // safety: RE_TIER_HINT has group 1 let tier = match tier_str.to_lowercase().as_str() { "flash" => Tier::Flash, "standard" => Tier::Standard, @@ -758,7 +758,8 @@ impl SmartRoutingProvider { // Highest priority: explicit tier hints (e.g. "[tier:flash]") if let Some(caps) = RE_TIER_HINT.captures(last_user_msg) { - let tier_str = caps.get(1).expect("capture group 1 exists").as_str(); + // SAFETY: RE_TIER_HINT has exactly one capture group; get(1) is guaranteed Some after match. + let tier_str = caps.get(1).expect("capture group 1 exists").as_str(); // safety: RE_TIER_HINT has group 1 let tier = match tier_str.to_lowercase().as_str() { "flash" => Tier::Flash, "standard" => Tier::Standard, diff --git a/src/settings.rs b/src/settings.rs index 63535aef..482291b6 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -837,19 +837,16 @@ impl Settings { .map_err(|e| format!("Failed to serialize settings: {}", e))?; let parts: Vec<&str> = path.split('.').collect(); - if parts.is_empty() { - return Err("Empty path".to_string()); - } + let (final_key, parent_parts) = + parts.split_last().ok_or_else(|| "Empty path".to_string())?; // Navigate to parent and set the final key let mut current = &mut json; - for part in &parts[..parts.len() - 1] { + for part in parent_parts { current = current .get_mut(*part) .ok_or_else(|| format!("Path not found: {}", path))?; } - - let final_key = parts.last().unwrap(); let obj = current .as_object_mut() .ok_or_else(|| format!("Parent is not an object: {}", path))?; diff --git a/src/setup/channels.rs b/src/setup/channels.rs index 785bffe0..1c184b0b 100644 --- a/src/setup/channels.rs +++ b/src/setup/channels.rs @@ -1016,7 +1016,7 @@ fn validation_placeholder_regex() -> &'static regex::Regex { static PLACEHOLDER_RE: std::sync::OnceLock = std::sync::OnceLock::new(); PLACEHOLDER_RE.get_or_init(|| { regex::Regex::new(r"\{([A-Za-z0-9_]+)\}") - .expect("validation placeholder regex must compile") + .expect("validation placeholder regex must compile") // safety: hardcoded literal }) } diff --git a/src/skills/mod.rs b/src/skills/mod.rs index f81bd535..84cf1cb4 100644 --- a/src/skills/mod.rs +++ b/src/skills/mod.rs @@ -48,7 +48,7 @@ pub const MAX_PROMPT_FILE_SIZE: u64 = 64 * 1024; /// Regex for validating skill names: alphanumeric, hyphens, underscores, dots. static SKILL_NAME_PATTERN: std::sync::LazyLock = - std::sync::LazyLock::new(|| Regex::new(r"^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$").unwrap()); + std::sync::LazyLock::new(|| Regex::new(r"^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$").unwrap()); // safety: hardcoded literal /// Validate a skill name against the allowed pattern. pub fn validate_skill_name(name: &str) -> bool { @@ -268,13 +268,13 @@ pub fn escape_skill_content(content: &str) -> String { // Match `<` followed by optional `/`, optional whitespace/control chars, // then `skill` (case-insensitive). Catches both opening and closing tags: // ` Result { let start = std::time::Instant::now(); - let jm = self.job_manager.as_ref().expect("sandbox deps required"); + let jm = self.job_manager.as_ref().ok_or_else(|| { + ToolError::ExecutionFailed( + "Sandbox execution requires a configured job manager (container runtime not available)".to_string(), + ) + })?; let job_id = Uuid::new_v4(); let (project_dir, browse_id) = resolve_project_dir(explicit_dir, job_id)?; @@ -1379,6 +1383,31 @@ mod tests { assert_eq!(tool.execution_timeout(), Duration::from_secs(30)); } + #[tokio::test] + async fn test_sandbox_without_job_manager_returns_error() { + let manager = Arc::new(ContextManager::new(5)); + // Create tool without sandbox deps — job_manager is None. + let tool = CreateJobTool::new(manager); + assert!(!tool.sandbox_enabled()); + + let result = tool + .execute_sandbox( + "test task", + None, + false, + JobMode::Worker, + vec![], + &JobContext::default(), + ) + .await; + + let err = result.unwrap_err(); + assert!( + matches!(err, ToolError::ExecutionFailed(_)), + "expected ExecutionFailed, got: {err:?}" + ); + } + #[tokio::test] async fn test_list_jobs_tool() { let manager = Arc::new(ContextManager::new(5)); diff --git a/src/tools/mcp/http_transport.rs b/src/tools/mcp/http_transport.rs index ec30d7bb..ec7139c9 100644 --- a/src/tools/mcp/http_transport.rs +++ b/src/tools/mcp/http_transport.rs @@ -39,7 +39,7 @@ impl HttpMcpTransport { http_client: reqwest::Client::builder() .timeout(std::time::Duration::from_secs(30)) .build() - .expect("Failed to create HTTP client"), + .expect("Failed to create HTTP client"), // safety: TLS init with default rustls cannot fail session_manager: None, custom_headers: HashMap::new(), } diff --git a/src/tools/wasm/wrapper.rs b/src/tools/wasm/wrapper.rs index a1b36548..bceb9401 100644 --- a/src/tools/wasm/wrapper.rs +++ b/src/tools/wasm/wrapper.rs @@ -343,7 +343,7 @@ impl near::agent::host::Host for StoreData { .map_err(|e| format!("Failed to create HTTP runtime: {e}"))?, ); } - let rt = self.http_runtime.as_ref().expect("just initialized"); + let rt = self.http_runtime.as_ref().expect("just initialized"); // safety: is_none branch above guarantees Some let result = rt.block_on(async { let client = reqwest::Client::builder() .connect_timeout(Duration::from_secs(10)) diff --git a/src/workspace/chunker.rs b/src/workspace/chunker.rs index c71a4f3f..d8aa4de4 100644 --- a/src/workspace/chunker.rs +++ b/src/workspace/chunker.rs @@ -92,8 +92,9 @@ pub fn chunk_document(content: &str, config: ChunkConfig) -> Vec { let chunk_words = &words[start..end]; // Don't create tiny trailing chunks, merge with previous - if chunk_words.len() < config.min_chunk_size && !chunks.is_empty() { - let last = chunks.pop().unwrap(); + if chunk_words.len() < config.min_chunk_size + && let Some(last) = chunks.pop() + { let combined = format!("{} {}", last, chunk_words.join(" ")); chunks.push(combined); break;