fix: security review fixes — SQL injection, panic, and race condition

database.rs:
- search_by_tags: replace string-interpolated LIKE with parameterized ?N bindings
- search_by_notes: replace format!() SQL with prepared statement + ?1/?2 params
- search_by_set_file: same — use ?1 for pattern and ?2 for limit

backtest.rs (extract_and_store / monitor_backtest_completion):
- extract_and_store now returns bool — callers only delete report file if true
- guard both success paths with conditional delete + warning on failure
- replace .parent().unwrap() panic with explicit match + error log + graceful return

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Cargo.lock

@@ -481,7 +481,7 @@ dependencies = [
 
 [[package]]
 name = "mt5-quant"
-version = "1.32.0"
+version = "1.32.1"
 dependencies = [
  "anyhow",
  "base64",

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mt5-quant"
-version = "1.32.0"
+version = "1.32.1"
 edition = "2021"
 description = "MCP server for MT5 strategy development on macOS/Linux"
 authors = ["masdevid <masdevid@example.com>"]
@@ -11,6 +11,8 @@ keywords = ["mt5", "mql5", "trading", "mcp", "backtest"]
 categories = ["finance", "development-tools"]
 homepage = "https://github.com/masdevid/mt5-quant"
 documentation = "https://github.com/masdevid/mt5-quant"
+
+[package.metadata]
 maintenance = { status = "actively-developed" }
 
 [[bin]]

server.json

@@ -7,13 +7,13 @@
     "url": "https://github.com/masdevid/mt5-quant",
     "source": "github"
   },
-  "version": "1.31.5",
+  "version": "1.32.0",
   "packages": [
     {
       "registryType": "mcpb",
-      "version": "1.31.5",
-      "identifier": "https://github.com/masdevid/mt5-quant/releases/download/v1.31.5/mcp-mt5-quant-macos-arm64.tar.gz",
-      "fileSha256": "97e05d3bc97270866d5736660bd19f071fd0fabbc474ba481825d69ec2b4bdf5",
+      "version": "1.32.0",
+      "identifier": "https://github.com/masdevid/mt5-quant/releases/download/v1.32.0/mcp-mt5-quant-macos-arm64.tar.gz",
+      "fileSha256": "8b4e058d7b42265e8a89c0dcd9567cc7bd06b8358f7b2799966a1e92cea816f7",
       "transport": {
         "type": "stdio"
       },

src/mcp_server.rs

@@ -4,6 +4,7 @@ use tokio::sync::{mpsc, Mutex};
 
 use crate::{models::Config as ModelsConfig, tools::ToolHandler, McpError, McpRequest, McpResponse};
 
+#[allow(dead_code)]
 type NotificationCallback = Arc<dyn Fn(&str, serde_json::Value) + Send + Sync>;
 
 /// Auto-verify result stored after first initialization
@@ -59,6 +60,7 @@ impl McpServer {
         *handler_guard = Some(new_handler);
     }
 
+    #[allow(dead_code)]
     pub async fn get_notification_sender(&self) -> Option<mpsc::UnboundedSender<Notification>> {
         let guard = self.notification_tx.lock().await;
         guard.clone()

src/models/report.rs

@@ -14,8 +14,6 @@ pub struct Report {
     pub from_date: String,
     pub to_date: String,
     pub metrics_file: PathBuf,
-    pub deals_csv: PathBuf,
-    pub deals_json: PathBuf,
     pub analysis_file: Option<PathBuf>,
 }
 
@@ -42,8 +40,6 @@ pub struct PipelineMetadata {
 pub struct FilePaths {
     pub metrics: String,
     pub analysis: String,
-    pub deals_csv: String,
-    pub deals_json: String,
 }
 
 #[allow(dead_code)]

src/pipeline/backtest.rs

@@ -258,6 +258,27 @@ impl BacktestPipeline {
         let timeout_secs = params.timeout;
         let report_id_clone = report_id.clone();
         let notification_callback = self.notification_callback.clone();
+        let config_clone = self.config.clone();
+        let params_clone = BacktestParams {
+            expert: params.expert.clone(),
+            symbol: params.symbol.clone(),
+            from_date: params.from_date.clone(),
+            to_date: params.to_date.clone(),
+            timeframe: params.timeframe.clone(),
+            deposit: params.deposit,
+            model: params.model,
+            leverage: params.leverage,
+            set_file: params.set_file.clone(),
+            skip_compile: params.skip_compile,
+            skip_clean: params.skip_clean,
+            skip_analyze: params.skip_analyze,
+            deep_analyze: params.deep_analyze,
+            shutdown: params.shutdown,
+            kill_existing: params.kill_existing,
+            timeout: params.timeout,
+            gui: params.gui,
+            startup_delay_secs: params.startup_delay_secs,
+        };
         tokio::spawn(async move {
             Self::monitor_backtest_completion(
                 report_dir_clone,
@@ -265,12 +286,81 @@ impl BacktestPipeline {
                 timeout_secs,
                 report_id_clone,
                 notification_callback,
+                config_clone,
+                params_clone,
             ).await;
         });
 
         Ok(job)
     }
 
+    /// Extract deals from a completed report and store them in the DB.
+    /// Returns true if extraction and DB registration succeeded.
+    async fn extract_and_store(
+        report_path: &Path,
+        report_dir: &Path,
+        report_id: &str,
+        config: &Config,
+        params: &BacktestParams,
+    ) -> bool {
+        let extractor = ReportExtractor::new();
+        let start_time = chrono::Utc::now();
+        match extractor.extract(
+            &report_path.to_string_lossy(),
+            &report_dir.to_string_lossy(),
+        ) {
+            Ok(extraction) => {
+                let duration = (chrono::Utc::now() - start_time).num_seconds();
+                let db = ReportDb::new(&Config::db_path());
+                if db.init().is_err() {
+                    tracing::warn!("launch_backtest: failed to init DB for {}", report_id);
+                    return false;
+                }
+                let entry = ReportEntry {
+                    id: report_id.to_string(),
+                    expert: params.expert.clone(),
+                    symbol: params.symbol.clone(),
+                    timeframe: params.timeframe.clone(),
+                    model: params.model as i64,
+                    from_date: params.from_date.clone(),
+                    to_date: params.to_date.clone(),
+                    created_at: chrono::Utc::now().to_rfc3339(),
+                    set_file_original: params.set_file.clone(),
+                    set_snapshot_path: None,
+                    report_dir: report_dir.to_string_lossy().to_string(),
+                    charts_dir: None,
+                    net_profit: Some(extraction.metrics.net_profit),
+                    profit_factor: Some(extraction.metrics.profit_factor),
+                    max_dd_pct: Some(extraction.metrics.max_dd_pct),
+                    sharpe_ratio: Some(extraction.metrics.sharpe_ratio),
+                    total_trades: Some(extraction.metrics.total_trades as i64),
+                    win_rate_pct: Some(extraction.metrics.win_rate_pct),
+                    recovery_factor: Some(extraction.metrics.recovery_factor),
+                    deposit: Some(params.deposit as f64),
+                    currency: config.backtest_currency.clone(),
+                    leverage: Some(params.leverage as i64),
+                    duration_seconds: Some(duration),
+                    tags: Vec::new(),
+                    notes: None,
+                    verdict: None,
+                };
+                if let Err(e) = db.insert(&entry) {
+                    tracing::warn!("launch_backtest: failed to register report in DB: {}", e);
+                    return false;
+                }
+                if let Err(e) = db.insert_deals(report_id, &extraction.deals) {
+                    tracing::warn!("launch_backtest: failed to store deals in DB: {}", e);
+                }
+                tracing::info!("launch_backtest: extracted {} deals for {}", extraction.deals.len(), report_id);
+                true
+            }
+            Err(e) => {
+                tracing::warn!("launch_backtest: extraction failed for {}: {}", report_id, e);
+                false
+            }
+        }
+    }
+
     /// Background task to monitor backtest completion and update status file.
     async fn monitor_backtest_completion(
         report_dir: PathBuf,
@@ -278,6 +368,8 @@ impl BacktestPipeline {
         timeout_secs: u64,
         report_id: String,
         notification_callback: Option<NotificationCallback>,
+        config: Config,
+        params: BacktestParams,
     ) {
         let start = tokio::time::Instant::now();
         let deadline = start + Duration::from_secs(timeout_secs);
@@ -292,6 +384,12 @@ impl BacktestPipeline {
                 let candidate = expected_report.with_extension(ext.trim_start_matches('.'));
                 if candidate.exists() {
                     tracing::info!("Backtest {} completed: report found at {}", report_id, candidate.display());
+                    let extracted = Self::extract_and_store(&candidate, &report_dir, &report_id, &config, &params).await;
+                    if extracted {
+                        let _ = fs::remove_file(&candidate);
+                    } else {
+                        tracing::warn!("Backtest {}: extraction failed, keeping report file at {}", report_id, candidate.display());
+                    }
                     Self::update_job_status(&report_dir, "completed", Some(candidate.to_string_lossy().to_string())).await;
                     if let Some(ref callback) = notification_callback {
                         callback("backtest_completed", json!({
@@ -310,8 +408,22 @@ impl BacktestPipeline {
 
             if !in_grace && !mt5_alive {
                 // MT5 exited without report - check for any newer report
-                if let Some(path) = Self::find_newest_report(expected_report.parent().unwrap(), poll_start) {
+                let reports_parent = match expected_report.parent() {
+                    Some(p) => p,
+                    None => {
+                        tracing::error!("Backtest {}: expected_report path has no parent", report_id);
+                        Self::update_job_status(&report_dir, "failed", None).await;
+                        return;
+                    }
+                };
+                if let Some(path) = Self::find_newest_report(reports_parent, poll_start) {
                     tracing::info!("Backtest {} completed: found fallback report {}", report_id, path.display());
+                    let extracted = Self::extract_and_store(&path, &report_dir, &report_id, &config, &params).await;
+                    if extracted {
+                        let _ = fs::remove_file(&path);
+                    } else {
+                        tracing::warn!("Backtest {}: extraction failed, keeping fallback report at {}", report_id, path.display());
+                    }
                     Self::update_job_status(&report_dir, "completed", Some(path.to_string_lossy().to_string())).await;
                     if let Some(ref callback) = notification_callback {
                         callback("backtest_completed", json!({
@@ -1047,8 +1159,6 @@ impl BacktestPipeline {
             files: FilePaths {
                 metrics: report_dir.join("metrics.json").to_string_lossy().to_string(),
                 analysis: report_dir.join("analysis.json").to_string_lossy().to_string(),
-                deals_csv: report_dir.join("deals.csv").to_string_lossy().to_string(),
-                deals_json: report_dir.join("deals.json").to_string_lossy().to_string(),
             },
             no_trades,
         };

src/storage/database.rs

@@ -564,26 +564,39 @@ impl ReportDb {
     /// Search reports by tags (at least one tag must match)
     pub fn search_by_tags(&self, tags: &[String], limit: usize) -> Result<Vec<ReportEntry>> {
         let conn = self.connect()?;
-        let mut sql = "SELECT id, expert, symbol, timeframe, model, from_date, to_date, \
+        let base_sql = "SELECT id, expert, symbol, timeframe, model, from_date, to_date, \
             created_at, set_file_original, set_snapshot_path, report_dir, charts_dir, \
             net_profit, profit_factor, max_dd_pct, sharpe_ratio, total_trades, \
             win_rate_pct, recovery_factor, deposit, currency, leverage, \
             duration_seconds, tags, notes, verdict \
-            FROM reports WHERE 1=1"
-            .to_string();
+            FROM reports WHERE 1=1";
 
-        // Build OR conditions for tags - use JSON1 extension for tag matching
-        if !tags.is_empty() {
-            let tag_conditions: Vec<String> = tags.iter()
-                .map(|tag| format!("tags LIKE '%{}%'", tag.replace("'", "''")))
+        let (sql, params): (String, Vec<rusqlite::types::Value>) = if tags.is_empty() {
+            (
+                format!("{} ORDER BY created_at DESC LIMIT ?1", base_sql),
+                vec![(limit as i64).into()],
+            )
+        } else {
+            let placeholders = tags.iter().enumerate()
+                .map(|(i, _)| format!("tags LIKE ?{}", i + 1))
+                .collect::<Vec<_>>()
+                .join(" OR ");
+            let sql = format!(
+                "{} AND ({}) ORDER BY created_at DESC LIMIT ?{}",
+                base_sql,
+                placeholders,
+                tags.len() + 1
+            );
+            let mut p: Vec<rusqlite::types::Value> = tags.iter()
+                .map(|t| format!("%{}%", t).into())
                 .collect();
-            sql.push_str(&format!(" AND ({})", tag_conditions.join(" OR ")));
-        }
-        sql.push_str(&format!(" ORDER BY created_at DESC LIMIT {}", limit));
+            p.push((limit as i64).into());
+            (sql, p)
+        };
 
         let mut stmt = conn.prepare(&sql)?;
         let entries: Vec<ReportEntry> = stmt
-            .query_map([], |row| {
+            .query_map(rusqlite::params_from_iter(params.iter()), |row| {
                 let tags_str: String = row.get(23).unwrap_or_else(|_| "[]".to_string());
                 let tags: Vec<String> = serde_json::from_str(&tags_str).unwrap_or_default();
                 Ok(ReportEntry {
@@ -624,19 +637,18 @@ impl ReportDb {
     /// Search reports by notes text (case-insensitive LIKE)
     pub fn search_by_notes(&self, query: &str, limit: usize) -> Result<Vec<ReportEntry>> {
         let conn = self.connect()?;
-        let pattern = format!("%{}%", query.replace("'", "''"));
+        let pattern = format!("%{}%", query);
         let mut stmt = conn.prepare(
-            &format!("SELECT id, expert, symbol, timeframe, model, from_date, to_date, \
+            "SELECT id, expert, symbol, timeframe, model, from_date, to_date, \
             created_at, set_file_original, set_snapshot_path, report_dir, charts_dir, \
             net_profit, profit_factor, max_dd_pct, sharpe_ratio, total_trades, \
             win_rate_pct, recovery_factor, deposit, currency, leverage, \
             duration_seconds, tags, notes, verdict \
-            FROM reports WHERE notes LIKE '{}' ORDER BY created_at DESC LIMIT {}",
-            pattern, limit)
+            FROM reports WHERE notes LIKE ?1 ORDER BY created_at DESC LIMIT ?2"
         )?;
 
         let entries: Vec<ReportEntry> = stmt
-            .query_map([], |row| {
+            .query_map(rusqlite::params![pattern, limit as i64], |row| {
                 let tags_str: String = row.get(23).unwrap_or_else(|_| "[]".to_string());
                 let tags: Vec<String> = serde_json::from_str(&tags_str).unwrap_or_default();
                 Ok(ReportEntry {
@@ -677,20 +689,19 @@ impl ReportDb {
     /// Find reports by set file (original or snapshot)
     pub fn search_by_set_file(&self, set_file: &str, limit: usize) -> Result<Vec<ReportEntry>> {
         let conn = self.connect()?;
-        let pattern = format!("%{}%", set_file.replace("'", "''"));
+        let pattern = format!("%{}%", set_file);
         let mut stmt = conn.prepare(
-            &format!("SELECT id, expert, symbol, timeframe, model, from_date, to_date, \
+            "SELECT id, expert, symbol, timeframe, model, from_date, to_date, \
             created_at, set_file_original, set_snapshot_path, report_dir, charts_dir, \
             net_profit, profit_factor, max_dd_pct, sharpe_ratio, total_trades, \
             win_rate_pct, recovery_factor, deposit, currency, leverage, \
             duration_seconds, tags, notes, verdict \
-            FROM reports WHERE (set_file_original LIKE '{}' OR set_snapshot_path LIKE '{}') \
-            ORDER BY created_at DESC LIMIT {}",
-            pattern, pattern, limit)
+            FROM reports WHERE (set_file_original LIKE ?1 OR set_snapshot_path LIKE ?1) \
+            ORDER BY created_at DESC LIMIT ?2"
         )?;
 
         let entries: Vec<ReportEntry> = stmt
-            .query_map([], |row| {
+            .query_map(rusqlite::params![pattern, limit as i64], |row| {
                 let tags_str: String = row.get(23).unwrap_or_else(|_| "[]".to_string());
                 let tags: Vec<String> = serde_json::from_str(&tags_str).unwrap_or_default();
                 Ok(ReportEntry {

src/tools/definitions/utility.rs

@@ -289,7 +289,7 @@ pub fn tool_get_wine_prefix_info() -> Value {
 pub fn tool_get_backtest_crash_info() -> Value {
     json!({
         "name": "get_backtest_crash_info",
-        "description": "Investigate backtest crashes/failures. Checks for incomplete markers, missing deals.csv, error logs. Can scan recent reports.",
+        "description": "Investigate backtest crashes/failures. Checks for incomplete markers, missing metrics.json, DB deal count, error logs. Can scan recent reports.",
         "inputSchema": {
             "type": "object",
             "properties": {

src/tools/handlers/backtest.rs

@@ -355,7 +355,6 @@ pub async fn handle_get_backtest_status(_config: &Config, args: &Value) -> Resul
     
     // Check for completed artifacts
     let metrics_exists = report_path.join("metrics.json").exists();
-    let deals_exists = report_path.join("deals.csv").exists();
     let is_complete = stage == "DONE" || (report_found && metrics_exists);
     
     // Calculate elapsed time if job exists
@@ -400,7 +399,6 @@ pub async fn handle_get_backtest_status(_config: &Config, args: &Value) -> Resul
             "mt5_running": mt5_running,
             "report_found": report_found,
             "metrics_extracted": metrics_exists,
-            "deals_extracted": deals_exists,
             "elapsed_seconds": elapsed_seconds,
             "message": message,
             "job": job.map(|j| {

src/tools/handlers/mod.rs

@@ -179,6 +179,7 @@ pub(crate) fn dir_size(path: &Path) -> u64 {
         .sum()
 }
 
+#[allow(dead_code)]
 pub(crate) fn past_complete_month() -> (String, String) {
     let now = chrono::Utc::now();
     let today = chrono::NaiveDate::from_ymd_opt(now.year(), now.month(), 1)

src/tools/handlers/utility.rs

@@ -3,6 +3,7 @@ use serde_json::{json, Value};
 use std::fs;
 use std::path::{Path, PathBuf};
 use crate::models::Config;
+use crate::storage::ReportDb;
 
 /// Validate that `user_path` resolves to a location within `allowed_base`.
 /// Returns the canonicalized absolute path on success.
@@ -785,7 +786,6 @@ pub async fn handle_export_report(_config: &Config, args: &Value) -> Result<Valu
     
     let path = Path::new(report_dir);
     let metrics_path = path.join("metrics.json");
-    let _deals_path = path.join("deals.csv");
     
     // Read metrics
     let metrics: Value = if metrics_path.exists() {
@@ -1640,21 +1640,33 @@ pub async fn handle_get_backtest_crash_info(config: &Config, args: &Value) -> Re
                 }
             }
             
-            // Check if deals.csv is missing or empty
-            let deals_csv = path.join("deals.csv");
-            if !deals_csv.exists() {
-                result["crashes_found"].as_array_mut().unwrap().push(json!({
-                    "report_dir": dir,
-                    "type": "missing_deals",
-                    "reason": "deals.csv not found - backtest likely failed",
-                }));
-            } else if let Ok(meta) = deals_csv.metadata() {
-                if meta.len() < 100 {
-                    result["crashes_found"].as_array_mut().unwrap().push(json!({
-                        "report_dir": dir,
-                        "type": "empty_deals",
-                        "reason": "deals.csv is nearly empty - no trades were made",
-                    }));
+            // Check deal count in DB
+            let db = ReportDb::new(&Config::db_path());
+            if db.init().is_ok() {
+                match db.get_by_report_dir(dir) {
+                    Ok(Some(entry)) => {
+                        let deal_count = db.get_deals(&entry.id)
+                            .map(|d| d.len())
+                            .unwrap_or(0);
+                        if deal_count == 0 {
+                            result["crashes_found"].as_array_mut().unwrap().push(json!({
+                                "report_dir": dir,
+                                "type": "empty_deals",
+                                "reason": "No deals stored in DB - EA did not trade or extraction failed",
+                            }));
+                        }
+                    }
+                    Ok(None) | Err(_) => {
+                        // Not in DB at all means extraction never completed
+                        let metrics_exists = path.join("metrics.json").exists();
+                        if !metrics_exists {
+                            result["crashes_found"].as_array_mut().unwrap().push(json!({
+                                "report_dir": dir,
+                                "type": "missing_deals",
+                                "reason": "Report not found in DB and no metrics.json - backtest likely crashed before extraction",
+                            }));
+                        }
+                    }
                 }
             }
         }
@@ -1676,10 +1688,10 @@ pub async fn handle_get_backtest_crash_info(config: &Config, args: &Value) -> Re
                             if let Ok(modified) = meta.modified() {
                                 if modified >= cutoff {
                                     // Check for failure indicators
-                                    let has_deals = path.join("deals.csv").exists();
+                                    let has_metrics = path.join("metrics.json").exists();
                                     let has_incomplete = path.join(".incomplete").exists();
-                                    
-                                    if !has_deals || has_incomplete {
+
+                                    if !has_metrics || has_incomplete {
                                         failures += 1;
                                     }
                                 }
@@ -1701,7 +1713,7 @@ pub async fn handle_get_backtest_crash_info(config: &Config, args: &Value) -> Re
         
         if types.contains(&"missing_deals".to_string()) {
             result["common_patterns"].as_array_mut().unwrap().push(
-                json!("Missing deals.csv suggests MT5 crashed during backtest")
+                json!("Report not in DB and no metrics.json suggests MT5 crashed before extraction completed")
             );
         }
         if types.contains(&"incomplete".to_string()) {