Add safe_command module and update desktop sync, tray, and PROMPT.md

PROMPT.md

@@ -35,6 +35,76 @@
 
 ---
 
+## 🔐 SECURITY REQUIREMENTS
+
+**BotApp inherits ALL security requirements from `botserver/PROMPT.md`.**
+
+For comprehensive Rust security patterns, refer to:
+- `botserver/PROMPT.md` - Full security guidelines
+- `botserver/src/security/` - Security module implementations
+
+### Security Modules Reference (in botserver)
+
+| Module | Purpose |
+|--------|---------|
+| `sql_guard.rs` | SQL injection prevention with table whitelist |
+| `command_guard.rs` | Command injection prevention with command whitelist |
+| `secrets.rs` | Secrets management with zeroizing memory |
+| `validation.rs` | Input validation utilities |
+| `rate_limiter.rs` | Rate limiting middleware |
+| `headers.rs` | Security headers (CSP, HSTS, X-Frame-Options) |
+| `cors.rs` | CORS configuration (no wildcard in production) |
+| `auth.rs` | Authentication & RBAC infrastructure |
+| `panic_handler.rs` | Panic catching middleware |
+| `path_guard.rs` | Path traversal protection |
+| `request_id.rs` | Request ID tracking |
+| `error_sanitizer.rs` | Error message sanitization |
+| `zitadel_auth.rs` | Zitadel authentication integration |
+
+### Desktop-Specific Security
+
+```
+❌ NEVER trust user input from IPC commands
+❌ NEVER expose filesystem paths to frontend without validation
+❌ NEVER store secrets in plain text or localStorage
+❌ NEVER disable CSP in tauri.conf.json for production
+❌ NEVER use allowlist: all in Tauri configuration
+```
+
+```rust
+// ❌ WRONG - trusting user path
+#[tauri::command]
+async fn read_file(path: String) -> Result<String, String> {
+    std::fs::read_to_string(path).map_err(|e| e.to_string())
+}
+
+// ✅ CORRECT - validate and sandbox paths
+#[tauri::command]
+async fn read_file(app: tauri::AppHandle, filename: String) -> Result<String, String> {
+    let safe_name = filename
+        .chars()
+        .filter(|c| c.is_alphanumeric() || *c == '.' || *c == '-')
+        .collect::<String>();
+    if safe_name.contains("..") {
+        return Err("Invalid filename".into());
+    }
+    let base_dir = app.path().app_data_dir().map_err(|e| e.to_string())?;
+    let full_path = base_dir.join(&safe_name);
+    std::fs::read_to_string(full_path).map_err(|e| e.to_string())
+}
+```
+
+### Tauri Security Checklist
+
+- [ ] Minimal `allowlist` in `tauri.conf.json`
+- [ ] CSP configured (not `null` in production)
+- [ ] No `dangerousRemoteDomainIpcAccess`
+- [ ] Validate ALL IPC command parameters
+- [ ] Use `app.path()` for sandboxed directories
+- [ ] No shell command execution from user input
+
+---
+
 ## CARGO.TOML LINT EXCEPTIONS
 
 When a clippy lint has **technical false positives** that cannot be fixed in code,
@@ -243,7 +313,7 @@ BotApp is a **Tauri-based desktop wrapper** for General Bots. It provides native
 ```
 botapp/        # THIS PROJECT - Desktop app wrapper
 botui/         # Web UI (consumed by botapp)
-botserver/     # Main server (business logic)
+botserver/     # Main server (business logic, security modules)
 botlib/        # Shared library
 botbook/       # Documentation
 ```
@@ -317,6 +387,7 @@ Business Logic + Database
 - Desktop-specific features only in botapp
 - Shared logic stays in botserver
 - Zero warnings required
+- ALL IPC inputs must be validated
 ```
 
 ### Tauri Command Pattern
@@ -329,6 +400,10 @@ pub async fn my_command(
     window: tauri::Window,
     param: String,
 ) -> Result<MyResponse, String> {
+    // Validate input first
+    if param.is_empty() || param.len() > 1000 {
+        return Err("Invalid parameter".into());
+    }
     // Implementation
     Ok(MyResponse { /* ... */ })
 }
@@ -340,7 +415,7 @@ fn main() {
             my_command,
         ])
         .run(tauri::generate_context!())
-        .expect("error running app");
+        .map_err(|e| format!("error running app: {e}"))?;
 }
 ```
 
@@ -414,7 +489,7 @@ Key settings (Tauri v2 format):
   },
   "app": {
     "security": {
-      "csp": null
+      "csp": "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'"
     },
     "windows": [{
       "title": "General Bots",
@@ -440,9 +515,10 @@ Key settings (Tauri v2 format):
 
 1. **Check if feature belongs here** - Only desktop-specific features
 2. **Add Tauri command** in `src/main.rs`
-3. **Register handler** in `tauri::Builder`
+3. **Validate ALL inputs** before processing
-4. **Add JS invocation** in `js/app-extensions.js`
+4. **Register handler** in `tauri::Builder`
-5. **Update UI** if needed
+5. **Add JS invocation** in `js/app-extensions.js`
+6. **Update UI** if needed
 
 ### Example: Add Screenshot
 
@@ -551,6 +627,7 @@ cargo test
 - **Desktop-only features** - Shared logic in botserver
 - **Tauri APIs** - No direct fs access from JS
 - **Platform abstractions** - Use cfg for platform code
-- **Security** - Minimal allowlist in tauri.conf.json
+- **Security** - Minimal allowlist in tauri.conf.json, validate ALL inputs
+- **Refer to botserver/PROMPT.md** - For comprehensive Rust security patterns
 - **Version**: Always 6.1.0 - do not change without approval
-- **Session Continuation**: When running out of context, create detailed summary: (1) what was done, (2) what remains, (3) specific files and line numbers, (4) exact next steps.
+- **Session Continuation**: When running out of context, create detailed summary: (1) what was done, (2) what remains, (3) specific files and line numbers, (4) exact next steps.

src/desktop/mod.rs

@@ -1,3 +1,4 @@
 pub mod drive;
+pub mod safe_command;
 pub mod sync;
 pub mod tray;

src/desktop/safe_command.rs

@@ -0,0 +1,209 @@
+use std::collections::HashSet;
+use std::path::PathBuf;
+use std::process::{Child, Command, Output, Stdio};
+use std::sync::LazyLock;
+
+static ALLOWED_COMMANDS: LazyLock<HashSet<&'static str>> = LazyLock::new(|| {
+    HashSet::from([
+        "rclone",
+        "notify-send",
+        "osascript",
+    ])
+});
+
+static FORBIDDEN_SHELL_CHARS: LazyLock<HashSet<char>> = LazyLock::new(|| {
+    HashSet::from([
+        ';', '|', '&', '$', '`', '(', ')', '{', '}', '<', '>', '\n', '\r', '\0',
+    ])
+});
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum SafeCommandError {
+    CommandNotAllowed(String),
+    InvalidArgument(String),
+    ExecutionFailed(String),
+    ShellInjectionAttempt(String),
+}
+
+impl std::fmt::Display for SafeCommandError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::CommandNotAllowed(cmd) => write!(f, "Command not in allowlist: {cmd}"),
+            Self::InvalidArgument(arg) => write!(f, "Invalid argument: {arg}"),
+            Self::ExecutionFailed(msg) => write!(f, "Command execution failed: {msg}"),
+            Self::ShellInjectionAttempt(input) => {
+                write!(f, "Shell injection attempt detected: {input}")
+            }
+        }
+    }
+}
+
+impl std::error::Error for SafeCommandError {}
+
+pub struct SafeCommand {
+    command: String,
+    args: Vec<String>,
+    working_dir: Option<PathBuf>,
+    stdout: Option<Stdio>,
+    stderr: Option<Stdio>,
+}
+
+impl SafeCommand {
+    pub fn new(command: &str) -> Result<Self, SafeCommandError> {
+        let cmd_name = std::path::Path::new(command)
+            .file_name()
+            .and_then(|n| n.to_str())
+            .unwrap_or(command);
+
+        if !ALLOWED_COMMANDS.contains(cmd_name) {
+            return Err(SafeCommandError::CommandNotAllowed(command.to_string()));
+        }
+
+        Ok(Self {
+            command: command.to_string(),
+            args: Vec::new(),
+            working_dir: None,
+            stdout: None,
+            stderr: None,
+        })
+    }
+
+    pub fn arg(mut self, arg: &str) -> Result<Self, SafeCommandError> {
+        validate_argument(arg)?;
+        self.args.push(arg.to_string());
+        Ok(self)
+    }
+
+    #[must_use]
+    pub fn stdout(mut self, stdout: Stdio) -> Self {
+        self.stdout = Some(stdout);
+        self
+    }
+
+    #[must_use]
+    pub fn stderr(mut self, stderr: Stdio) -> Self {
+        self.stderr = Some(stderr);
+        self
+    }
+
+    pub fn output(self) -> Result<Output, SafeCommandError> {
+        let mut cmd = Command::new(&self.command);
+        cmd.args(&self.args);
+
+        if let Some(ref dir) = self.working_dir {
+            cmd.current_dir(dir);
+        }
+
+        if let Some(stdout) = self.stdout {
+            cmd.stdout(stdout);
+        }
+
+        if let Some(stderr) = self.stderr {
+            cmd.stderr(stderr);
+        }
+
+        cmd.output()
+            .map_err(|e| SafeCommandError::ExecutionFailed(e.to_string()))
+    }
+
+    pub fn spawn(self) -> Result<Child, SafeCommandError> {
+        let mut cmd = Command::new(&self.command);
+        cmd.args(&self.args);
+
+        if let Some(ref dir) = self.working_dir {
+            cmd.current_dir(dir);
+        }
+
+        if let Some(stdout) = self.stdout {
+            cmd.stdout(stdout);
+        }
+
+        if let Some(stderr) = self.stderr {
+            cmd.stderr(stderr);
+        }
+
+        cmd.spawn()
+            .map_err(|e| SafeCommandError::ExecutionFailed(e.to_string()))
+    }
+}
+
+fn validate_argument(arg: &str) -> Result<(), SafeCommandError> {
+    if arg.is_empty() {
+        return Err(SafeCommandError::InvalidArgument(
+            "Empty argument".to_string(),
+        ));
+    }
+
+    if arg.len() > 4096 {
+        return Err(SafeCommandError::InvalidArgument(
+            "Argument too long".to_string(),
+        ));
+    }
+
+    for c in arg.chars() {
+        if FORBIDDEN_SHELL_CHARS.contains(&c) {
+            return Err(SafeCommandError::ShellInjectionAttempt(format!(
+                "Forbidden character '{}' in argument",
+                c.escape_default()
+            )));
+        }
+    }
+
+    let dangerous_patterns = ["$(", "`", "&&", "||", ">>", "<<"];
+
+    for pattern in dangerous_patterns {
+        if arg.contains(pattern) {
+            return Err(SafeCommandError::ShellInjectionAttempt(format!(
+                "Dangerous pattern '{pattern}' detected"
+            )));
+        }
+    }
+
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_allowed_command() {
+        assert!(SafeCommand::new("rclone").is_ok());
+        assert!(SafeCommand::new("notify-send").is_ok());
+        assert!(SafeCommand::new("osascript").is_ok());
+    }
+
+    #[test]
+    fn test_disallowed_command() {
+        assert!(SafeCommand::new("rm").is_err());
+        assert!(SafeCommand::new("bash").is_err());
+        assert!(SafeCommand::new("sh").is_err());
+    }
+
+    #[test]
+    fn test_valid_arguments() {
+        let cmd = SafeCommand::new("rclone")
+            .unwrap()
+            .arg("sync")
+            .unwrap()
+            .arg("/home/user/data")
+            .unwrap()
+            .arg("remote:bucket");
+        assert!(cmd.is_ok());
+    }
+
+    #[test]
+    fn test_injection_attempts() {
+        let cmd = SafeCommand::new("rclone").unwrap();
+        assert!(cmd.arg("; rm -rf /").is_err());
+
+        let cmd = SafeCommand::new("rclone").unwrap();
+        assert!(cmd.arg("$(whoami)").is_err());
+
+        let cmd = SafeCommand::new("rclone").unwrap();
+        assert!(cmd.arg("test`id`").is_err());
+
+        let cmd = SafeCommand::new("rclone").unwrap();
+        assert!(cmd.arg("a && b").is_err());
+    }
+}

src/desktop/sync.rs

@@ -1,6 +1,7 @@
+use super::safe_command::SafeCommand;
 use serde::{Deserialize, Serialize};
 use std::path::PathBuf;
-use std::process::{Child, Command, Stdio};
+use std::process::{Child, Stdio};
 use std::sync::Mutex;
 use tauri::{Emitter, Window};
 
@@ -78,8 +79,6 @@ pub fn get_sync_status() -> SyncStatus {
     }
 }
 
-/// # Errors
-/// Returns an error if sync is already running, directory creation fails, or rclone is not found.
 #[tauri::command]
 pub fn start_sync(window: Window, config: Option<SyncConfig>) -> Result<SyncStatus, String> {
     let config = config.unwrap_or_default();
@@ -99,42 +98,49 @@ pub fn start_sync(window: Window, config: Option<SyncConfig>) -> Result<SyncStat
             .map_err(|e| format!("Failed to create local directory: {e}"))?;
     }
 
-    let mut cmd = Command::new("rclone");
+    let remote_spec = format!("{}:{}", config.remote_name, config.remote_path);
 
-    match config.sync_mode {
+    let cmd_result = match config.sync_mode {
-        SyncMode::Push => {
+        SyncMode::Push => SafeCommand::new("rclone")
-            cmd.arg("sync");
+            .and_then(|c| c.arg("sync"))
-            cmd.arg(&config.local_path);
+            .and_then(|c| c.arg(&config.local_path))
-            cmd.arg(format!("{}:{}", config.remote_name, config.remote_path));
+            .and_then(|c| c.arg(&remote_spec)),
-        }
+        SyncMode::Pull => SafeCommand::new("rclone")
-        SyncMode::Pull => {
+            .and_then(|c| c.arg("sync"))
-            cmd.arg("sync");
+            .and_then(|c| c.arg(&remote_spec))
-            cmd.arg(format!("{}:{}", config.remote_name, config.remote_path));
+            .and_then(|c| c.arg(&config.local_path)),
-            cmd.arg(&config.local_path);
+        SyncMode::Bisync => SafeCommand::new("rclone")
-        }
+            .and_then(|c| c.arg("bisync"))
-        SyncMode::Bisync => {
+            .and_then(|c| c.arg(&config.local_path))
-            cmd.arg("bisync");
+            .and_then(|c| c.arg(&remote_spec))
-            cmd.arg(&config.local_path);
+            .and_then(|c| c.arg("--resync")),
-            cmd.arg(format!("{}:{}", config.remote_name, config.remote_path));
+    };
-            cmd.arg("--resync");
-        }
-    }
 
-    cmd.arg("--progress").arg("--verbose").arg("--checksum");
+    let mut cmd_builder = cmd_result
+        .and_then(|c| c.arg("--progress"))
+        .and_then(|c| c.arg("--verbose"))
+        .and_then(|c| c.arg("--checksum"))
+        .map_err(|e| format!("Failed to build rclone command: {e}"))?;
 
     for pattern in &config.exclude_patterns {
-        cmd.arg("--exclude").arg(pattern);
+        cmd_builder = cmd_builder
+            .arg("--exclude")
+            .and_then(|c| c.arg(pattern))
+            .map_err(|e| format!("Invalid exclude pattern: {e}"))?;
     }
 
-    cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
+    let child = cmd_builder
-
+        .stdout(Stdio::piped())
-    let child = cmd.spawn().map_err(|e| {
+        .stderr(Stdio::piped())
-        if e.kind() == std::io::ErrorKind::NotFound {
+        .spawn()
-            "rclone not found. Please install rclone: https://rclone.org/install/".to_string()
+        .map_err(|e| {
-        } else {
+            let err_str = e.to_string();
-            format!("Failed to start rclone: {e}")
+            if err_str.contains("NotFound") || err_str.contains("not found") {
-        }
+                "rclone not found. Please install rclone: https://rclone.org/install/".to_string()
-    })?;
+            } else {
+                format!("Failed to start rclone: {e}")
+            }
+        })?;
 
     {
         let mut process_guard = RCLONE_PROCESS
@@ -160,8 +166,6 @@ pub fn start_sync(window: Window, config: Option<SyncConfig>) -> Result<SyncStat
     })
 }
 
-/// # Errors
-/// Returns an error if no sync process is currently running.
 #[tauri::command]
 pub fn stop_sync() -> Result<SyncStatus, String> {
     let mut process_guard = RCLONE_PROCESS
@@ -188,8 +192,6 @@ pub fn stop_sync() -> Result<SyncStatus, String> {
         })
 }
 
-/// # Errors
-/// Returns an error if rclone configuration fails.
 #[tauri::command]
 pub fn configure_remote(
     remote_name: &str,
@@ -198,23 +200,22 @@ pub fn configure_remote(
     secret_key: &str,
     bucket: &str,
 ) -> Result<(), String> {
-    let output = Command::new("rclone")
+    let output = SafeCommand::new("rclone")
-        .args([
+        .and_then(|c| c.arg("config"))
-            "config",
+        .and_then(|c| c.arg("create"))
-            "create",
+        .and_then(|c| c.arg(remote_name))
-            remote_name,
+        .and_then(|c| c.arg("s3"))
-            "s3",
+        .and_then(|c| c.arg("provider"))
-            "provider",
+        .and_then(|c| c.arg("Minio"))
-            "Minio",
+        .and_then(|c| c.arg("endpoint"))
-            "endpoint",
+        .and_then(|c| c.arg(endpoint))
-            endpoint,
+        .and_then(|c| c.arg("access_key_id"))
-            "access_key_id",
+        .and_then(|c| c.arg(access_key))
-            access_key,
+        .and_then(|c| c.arg("secret_access_key"))
-            "secret_access_key",
+        .and_then(|c| c.arg(secret_key))
-            secret_key,
+        .and_then(|c| c.arg("acl"))
-            "acl",
+        .and_then(|c| c.arg("private"))
-            "private",
+        .map_err(|e| format!("Failed to build rclone command: {e}"))?
-        ])
         .output()
         .map_err(|e| format!("Failed to configure rclone: {e}"))?;
 
@@ -223,22 +224,26 @@ pub fn configure_remote(
         return Err(format!("rclone config failed: {stderr}"));
     }
 
-    let _ = Command::new("rclone")
+    let _ = SafeCommand::new("rclone")
-        .args(["config", "update", remote_name, "bucket", bucket])
+        .and_then(|c| c.arg("config"))
-        .output();
+        .and_then(|c| c.arg("update"))
+        .and_then(|c| c.arg(remote_name))
+        .and_then(|c| c.arg("bucket"))
+        .and_then(|c| c.arg(bucket))
+        .and_then(|c| c.output());
 
     Ok(())
 }
 
-/// # Errors
-/// Returns an error if rclone is not installed or the version check fails.
 #[tauri::command]
 pub fn check_rclone_installed() -> Result<String, String> {
-    let output = Command::new("rclone")
+    let output = SafeCommand::new("rclone")
-        .arg("version")
+        .and_then(|c| c.arg("version"))
+        .map_err(|e| format!("Failed to build rclone command: {e}"))?
         .output()
         .map_err(|e| {
-            if e.kind() == std::io::ErrorKind::NotFound {
+            let err_str = e.to_string();
+            if err_str.contains("NotFound") || err_str.contains("not found") {
                 "rclone not installed".to_string()
             } else {
                 format!("Error checking rclone: {e}")
@@ -254,12 +259,11 @@ pub fn check_rclone_installed() -> Result<String, String> {
     }
 }
 
-/// # Errors
-/// Returns an error if listing rclone remotes fails.
 #[tauri::command]
 pub fn list_remotes() -> Result<Vec<String>, String> {
-    let output = Command::new("rclone")
+    let output = SafeCommand::new("rclone")
-        .args(["listremotes"])
+        .and_then(|c| c.arg("listremotes"))
+        .map_err(|e| format!("Failed to build rclone command: {e}"))?
         .output()
         .map_err(|e| format!("Failed to list remotes: {e}"))?;
 
@@ -284,8 +288,6 @@ pub fn get_sync_folder() -> String {
     )
 }
 
-/// # Errors
-/// Returns an error if the directory cannot be created or the path is not a directory.
 #[tauri::command]
 pub fn set_sync_folder(path: &str) -> Result<(), String> {
     let path = PathBuf::from(path);

src/desktop/tray.rs

@@ -1,3 +1,4 @@
+use super::safe_command::SafeCommand;
 use anyhow::Result;
 use serde::Serialize;
 use std::sync::Arc;
@@ -44,8 +45,6 @@ impl TrayManager {
         }
     }
 
-    /// # Errors
-    /// Returns an error if the tray system fails to initialize.
     pub async fn start(&self) -> Result<()> {
         match self.running_mode {
             RunningMode::Desktop => {
@@ -119,8 +118,6 @@ impl TrayManager {
         }
     }
 
-    /// # Errors
-    /// Returns an error if the status update fails.
     pub async fn update_status(&self, status: &str) -> Result<()> {
         let active = self.tray_active.read().await;
         let is_active = *active;
@@ -132,8 +129,6 @@ impl TrayManager {
         Ok(())
     }
 
-    /// # Errors
-    /// Returns an error if setting the tooltip fails.
     pub async fn set_tooltip(&self, tooltip: &str) -> Result<()> {
         let active = self.tray_active.read().await;
         let is_active = *active;
@@ -145,8 +140,6 @@ impl TrayManager {
         Ok(())
     }
 
-    /// # Errors
-    /// Returns an error if the notification fails to display.
     pub async fn show_notification(&self, title: &str, body: &str) -> Result<()> {
         let active = self.tray_active.read().await;
         let is_active = *active;
@@ -157,19 +150,23 @@ impl TrayManager {
 
             #[cfg(target_os = "linux")]
             {
-                let _ = std::process::Command::new("notify-send")
+                if let Ok(cmd) = SafeCommand::new("notify-send")
-                    .arg(title)
+                    .and_then(|c| c.arg(title))
-                    .arg(body)
+                    .and_then(|c| c.arg(body))
-                    .spawn();
+                {
+                    let _ = cmd.spawn();
+                }
             }
 
             #[cfg(target_os = "macos")]
             {
                 let script = format!("display notification \"{body}\" with title \"{title}\"");
-                let _ = std::process::Command::new("osascript")
+                if let Ok(cmd) = SafeCommand::new("osascript")
-                    .arg("-e")
+                    .and_then(|c| c.arg("-e"))
-                    .arg(&script)
+                    .and_then(|c| c.arg(&script))
-                    .spawn();
+                {
+                    let _ = cmd.spawn();
+                }
             }
         }
         Ok(())