diff --git a/src/basic/compiler/blocks/mail.rs b/src/basic/compiler/blocks/mail.rs index 18f4c55b4..3c5352f5b 100644 --- a/src/basic/compiler/blocks/mail.rs +++ b/src/basic/compiler/blocks/mail.rs @@ -75,13 +75,13 @@ pub fn convert_mail_line_with_substitution(line: &str) -> String { pub fn convert_mail_block(recipient: &str, lines: &[String]) -> String { let mut subject = String::new(); let mut body_lines: Vec = Vec::new(); - let mut in_subject = true; + // let mut in_subject = true; // Removed unused variable let mut skip_blank = true; for line in lines.iter() { if line.to_uppercase().starts_with("SUBJECT:") { subject = line[8..].trim().to_string(); - in_subject = false; + // in_subject = false; // Removed unused assignment skip_blank = true; continue; } diff --git a/src/basic/compiler/mod.rs b/src/basic/compiler/mod.rs index 90b3dc8b1..a192e1fb3 100644 --- a/src/basic/compiler/mod.rs +++ b/src/basic/compiler/mod.rs @@ -4,8 +4,8 @@ use crate::basic::keywords::table_definition::process_table_definitions; use crate::basic::keywords::webhook::execute_webhook_registration; use crate::core::shared::models::TriggerKind; use crate::core::shared::state::AppState; -use diesel::{QueryableByName, sql_query}; -use diesel::sql_types::Text; +use diesel::QueryableByName; +// use diesel::sql_types::Text; // Removed unused import use diesel::ExpressionMethods; use diesel::QueryDsl; use diesel::RunQueryDsl; @@ -771,7 +771,7 @@ impl BasicCompiler { table_name: &str, bot_id: uuid::Uuid, ) -> Result, Box> { - use std::path::Path; + // use std::path::Path; // Find the tables.bas file in the bot's data directory let bot_name = self.get_bot_name_by_id(bot_id)?; diff --git a/src/basic/keywords/db_api.rs b/src/basic/keywords/db_api.rs index e6aa45a97..e33f36659 100644 --- a/src/basic/keywords/db_api.rs +++ b/src/basic/keywords/db_api.rs @@ -6,7 +6,8 @@ use crate::core::shared::sanitize_identifier; use crate::core::urls::ApiUrls; use crate::security::error_sanitizer::log_and_sanitize; use crate::security::sql_guard::{ - build_safe_count_query, build_safe_select_query, is_table_allowed_with_conn, validate_table_name, + build_safe_count_query, build_safe_select_by_id_query, build_safe_select_query, + is_table_allowed_with_conn, validate_table_name, }; use axum::{ extract::{Path, Query, State}, @@ -257,10 +258,21 @@ pub async fn get_record_handler( } }; - let query = format!( - "SELECT row_to_json(t.*) as data FROM {} t WHERE id = $1", - table_name - ); + let query = match build_safe_select_by_id_query(&table_name) { + Ok(q) => q, + Err(e) => { + warn!("Failed to build safe query for {}: {}", table_name, e); + return ( + StatusCode::BAD_REQUEST, + Json(RecordResponse { + success: false, + data: None, + message: Some("Invalid table name".to_string()), + }), + ) + .into_response(); + } + }; let row: Result, _> = sql_query(&query) .bind::(record_id) @@ -700,7 +712,17 @@ pub async fn count_records_handler( return (StatusCode::FORBIDDEN, Json(json!({ "error": e }))).into_response(); } - let query = format!("SELECT COUNT(*) as count FROM {}", table_name); + let query = match build_safe_count_query(&table_name) { + Ok(q) => q, + Err(e) => { + warn!("Failed to build count query: {}", e); + return ( + StatusCode::BAD_REQUEST, + Json(json!({ "error": "Invalid table name" })), + ) + .into_response(); + } + }; let result: Result = sql_query(&query).get_result(&mut conn); match result { @@ -747,14 +769,18 @@ pub async fn search_records_handler( } }; + let safe_search = search_term.replace('%', "\\%").replace('_', "\\_"); + let query = format!( "SELECT row_to_json(t.*) as data FROM {} t WHERE COALESCE(t.title::text, '') || ' ' || COALESCE(t.name::text, '') || ' ' || COALESCE(t.description::text, '') - ILIKE '%{}%' LIMIT {}", - table_name, search_term, limit + ILIKE '%' || $1 || '%' LIMIT {}", + table_name, limit ); - let rows: Result, _> = sql_query(&query).get_results(&mut conn); + let rows: Result, _> = sql_query(&query) + .bind::(&safe_search) + .get_results(&mut conn); match rows { Ok(data) => { diff --git a/src/basic/keywords/table_migration.rs b/src/basic/keywords/table_migration.rs index 0c49adc54..d741ae112 100644 --- a/src/basic/keywords/table_migration.rs +++ b/src/basic/keywords/table_migration.rs @@ -7,7 +7,7 @@ use crate::core::shared::sanitize_identifier; use crate::core::shared::state::AppState; use diesel::prelude::*; use diesel::sql_query; -use diesel::sql_types::{Text, Nullable}; +use diesel::sql_types::Text; use log::{error, info, warn}; use std::error::Error; use std::sync::Arc; diff --git a/src/basic/mod.rs b/src/basic/mod.rs index def0abb8e..35a273d2f 100644 --- a/src/basic/mod.rs +++ b/src/basic/mod.rs @@ -912,14 +912,14 @@ impl ScriptService { fn convert_mail_block(recipient: &str, lines: &[String]) -> String { let mut subject = String::new(); let mut body_lines: Vec = Vec::new(); - let mut in_subject = true; + // let mut in_subject = true; // Removed unused variable let mut skip_blank = true; - for (i, line) in lines.iter().enumerate() { + for line in lines.iter() { // Check if this line is a subject line if line.to_uppercase().starts_with("SUBJECT:") { subject = line[8..].trim().to_string(); - in_subject = false; + // in_subject = false; // Removed unused assignment skip_blank = true; continue; } @@ -1203,7 +1203,7 @@ impl ScriptService { // Split into multiple TALK statements to avoid expression complexity limit // Use chunks of 5 lines per TALK statement let chunk_size = 5; - for (chunk_idx, chunk) in talk_block_lines.chunks(chunk_size).enumerate() { + for chunk in talk_block_lines.chunks(chunk_size) { // Convert all talk lines in this chunk to a single TALK statement let mut combined_talk = String::new(); for (i, talk_line) in chunk.iter().enumerate() { @@ -1415,7 +1415,7 @@ impl ScriptService { /// to avoid creating local variables that shadow outer scope variables. pub fn convert_select_case_syntax(script: &str) -> String { let mut result = String::new(); - let mut lines: Vec<&str> = script.lines().collect(); + let lines: Vec<&str> = script.lines().collect(); let mut i = 0; log::info!("[TOOL] Converting SELECT/CASE syntax to if-else chains"); @@ -1479,7 +1479,7 @@ impl ScriptService { // Close the last case arm (no else if, so we need the closing brace) result.push_str(" }\n"); current_case_body.clear(); - in_case = false; + //in_case = false; // Removed unused assignment } // No extra closing brace needed - the last } else if ... { already closed the chain i += 1; @@ -1502,7 +1502,7 @@ impl ScriptService { // Close the current case arm (no else if, so we need the closing brace) result.push_str(" }\n"); current_case_body.clear(); - in_case = false; + //in_case = false; // Removed unused assignment } // No extra closing brace needed break; diff --git a/src/core/bot/tool_executor.rs b/src/core/bot/tool_executor.rs index 60a461523..f236f214d 100644 --- a/src/core/bot/tool_executor.rs +++ b/src/core/bot/tool_executor.rs @@ -2,7 +2,7 @@ /// Works across all LLM providers (GLM, OpenAI, Claude, etc.) use log::{error, info, warn}; use serde_json::Value; -use std::collections::HashMap; +// use std::collections::HashMap; use std::fs::OpenOptions; use std::io::Write; use std::path::Path; diff --git a/src/security/command_guard.rs b/src/security/command_guard.rs index 49fb5abd5..e77686970 100644 --- a/src/security/command_guard.rs +++ b/src/security/command_guard.rs @@ -74,6 +74,8 @@ static ALLOWED_COMMANDS: LazyLock> = LazyLock::new(|| { "systemctl", "sudo", "visudo", + "id", + "netsh", ]) }); diff --git a/src/security/protection/installer.rs b/src/security/protection/installer.rs index 1116c350a..e6f4a652d 100644 --- a/src/security/protection/installer.rs +++ b/src/security/protection/installer.rs @@ -1,7 +1,6 @@ use anyhow::{Context, Result}; use std::fs; use std::path::Path; -use std::process::Command; use tracing::{error, info, warn}; use crate::security::command_guard::SafeCommand; @@ -84,12 +83,12 @@ impl ProtectionInstaller { #[cfg(windows)] pub fn check_admin() -> bool { - let result = Command::new("powershell") - .args([ + let result = SafeCommand::new("powershell") + .and_then(|cmd| cmd.args(&[ "-Command", "([Security.Principal.WindowsPrincipal] [Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)" - ]) - .output(); + ])) + .and_then(|cmd| cmd.execute()); match result { Ok(output) => { @@ -102,9 +101,9 @@ impl ProtectionInstaller { #[cfg(not(windows))] pub fn check_root() -> bool { - Command::new("id") - .arg("-u") - .output() + SafeCommand::new("id") + .and_then(|cmd| cmd.arg("-u")) + .and_then(|cmd| cmd.execute()) .map(|o| String::from_utf8_lossy(&o.stdout).trim() == "0") .unwrap_or(false) } @@ -268,26 +267,23 @@ impl ProtectionInstaller { fn configure_windows_security(&self) -> Result<()> { info!("Configuring Windows security settings..."); - // Enable Windows Defender real-time protection - let _ = Command::new("powershell") - .args([ + let _ = SafeCommand::new("powershell") + .and_then(|cmd| cmd.args(&[ "-Command", "Set-MpPreference -DisableRealtimeMonitoring $false; Set-MpPreference -DisableIOAVProtection $false; Set-MpPreference -DisableScriptScanning $false" - ]) - .output(); + ])) + .and_then(|cmd| cmd.execute()); - // Enable Windows Firewall - let _ = Command::new("netsh") - .args(["advfirewall", "set", "allprofiles", "state", "on"]) - .output(); + let _ = SafeCommand::new("netsh") + .and_then(|cmd| cmd.args(&["advfirewall", "set", "allprofiles", "state", "on"])) + .and_then(|cmd| cmd.execute()); - // Enable Windows Defender scanning for mapped drives - let _ = Command::new("powershell") - .args([ + let _ = SafeCommand::new("powershell") + .and_then(|cmd| cmd.args(&[ "-Command", "Set-MpPreference -DisableRemovableDriveScanning $false -DisableScanningMappedNetworkDrivesForFullScan $false" - ]) - .output(); + ])) + .and_then(|cmd| cmd.execute()); info!("Windows security configuration completed"); Ok(()) @@ -313,12 +309,11 @@ impl ProtectionInstaller { Ok(()) } - #[cfg(not(windows))] #[cfg(not(windows))] fn validate_sudoers(&self) -> Result<()> { - let output = std::process::Command::new("visudo") - .args(["-c", "-f", SUDOERS_FILE]) - .output() + let output = SafeCommand::new("visudo") + .and_then(|cmd| cmd.args(&["-c", "-f", SUDOERS_FILE])) + .and_then(|cmd| cmd.execute()) .context("Failed to run visudo validation")?; if !output.status.success() { @@ -330,7 +325,6 @@ impl ProtectionInstaller { Ok(()) } - #[cfg(not(windows))] #[cfg(not(windows))] fn install_lmd(&self) -> Result { let maldet_path = Path::new("/usr/local/sbin/maldet"); @@ -398,7 +392,6 @@ impl ProtectionInstaller { Ok(true) } - #[cfg(not(windows))] #[cfg(not(windows))] fn update_databases(&self) -> Result<()> { info!("Updating security tool databases..."); @@ -442,12 +435,12 @@ impl ProtectionInstaller { fn update_windows_signatures(&self) -> Result<()> { info!("Updating Windows Defender signatures..."); - let result = Command::new("powershell") - .args([ + let result = SafeCommand::new("powershell") + .and_then(|cmd| cmd.args(&[ "-Command", "Update-MpSignature; Write-Host 'Windows Defender signatures updated'", - ]) - .output(); + ])) + .and_then(|cmd| cmd.execute()); match result { Ok(output) => { @@ -571,13 +564,9 @@ impl ProtectionInstaller { #[cfg(windows)] { for (tool_name, tool_cmd) in WINDOWS_TOOLS { - let check = Command::new(tool_cmd) - .arg("--version") - .or_else(|_| { - Command::new("powershell") - .args(["-Command", &format!("Get-Command {}", tool_cmd)]) - }) - .output(); + let check = SafeCommand::new(tool_cmd) + .and_then(|cmd| cmd.arg("--version")) + .and_then(|cmd| cmd.execute()); let installed = check.map(|o| o.status.success()).unwrap_or(false); result.tools.push(ToolVerification { diff --git a/src/security/rate_limiter.rs b/src/security/rate_limiter.rs index 365160ae2..4794fe677 100644 --- a/src/security/rate_limiter.rs +++ b/src/security/rate_limiter.rs @@ -68,11 +68,20 @@ pub struct CombinedRateLimiter { impl CombinedRateLimiter { pub fn new(http_config: HttpRateLimitConfig, system_limits: SystemLimits) -> Self { + const DEFAULT_RPS: NonZeroU32 = match NonZeroU32::new(100) { + Some(v) => v, + None => unreachable!(), + }; + const DEFAULT_BURST: NonZeroU32 = match NonZeroU32::new(200) { + Some(v) => v, + None => unreachable!(), + }; + let quota = Quota::per_second( - NonZeroU32::new(http_config.requests_per_second).unwrap_or(NonZeroU32::new(100).expect("100 is non-zero")), + NonZeroU32::new(http_config.requests_per_second).unwrap_or(DEFAULT_RPS), ) .allow_burst( - NonZeroU32::new(http_config.burst_size).unwrap_or(NonZeroU32::new(200).expect("200 is non-zero")), + NonZeroU32::new(http_config.burst_size).unwrap_or(DEFAULT_BURST), ); Self {