From 879f94a2576a09d745b8728acdb02a42ec817c31 Mon Sep 17 00:00:00 2001 From: "Rodrigo Rodriguez (Pragmatismo)" Date: Thu, 8 Jan 2026 13:16:02 -0300 Subject: [PATCH] Add safe_command module and update desktop sync, tray, and PROMPT.md --- PROMPT.md | 93 ++++++++++++++-- src/desktop/mod.rs | 1 + src/desktop/safe_command.rs | 209 ++++++++++++++++++++++++++++++++++++ src/desktop/sync.rs | 138 ++++++++++++------------ src/desktop/tray.rs | 29 +++-- 5 files changed, 378 insertions(+), 92 deletions(-) create mode 100644 src/desktop/safe_command.rs diff --git a/PROMPT.md b/PROMPT.md index 1a8dc1e..fd71564 100644 --- a/PROMPT.md +++ b/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 { + 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 { + let safe_name = filename + .chars() + .filter(|c| c.is_alphanumeric() || *c == '.' || *c == '-') + .collect::(); + 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 { + // 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` -4. **Add JS invocation** in `js/app-extensions.js` -5. **Update UI** if needed +3. **Validate ALL inputs** before processing +4. **Register handler** in `tauri::Builder` +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. \ No newline at end of file diff --git a/src/desktop/mod.rs b/src/desktop/mod.rs index b161c02..432558d 100644 --- a/src/desktop/mod.rs +++ b/src/desktop/mod.rs @@ -1,3 +1,4 @@ pub mod drive; +pub mod safe_command; pub mod sync; pub mod tray; diff --git a/src/desktop/safe_command.rs b/src/desktop/safe_command.rs new file mode 100644 index 0000000..1c526e9 --- /dev/null +++ b/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> = LazyLock::new(|| { + HashSet::from([ + "rclone", + "notify-send", + "osascript", + ]) +}); + +static FORBIDDEN_SHELL_CHARS: LazyLock> = 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, + working_dir: Option, + stdout: Option, + stderr: Option, +} + +impl SafeCommand { + pub fn new(command: &str) -> Result { + 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 { + 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 { + 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 { + 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()); + } +} diff --git a/src/desktop/sync.rs b/src/desktop/sync.rs index 7826fbd..913ac1a 100644 --- a/src/desktop/sync.rs +++ b/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) -> Result { let config = config.unwrap_or_default(); @@ -99,42 +98,49 @@ pub fn start_sync(window: Window, config: Option) -> Result { - cmd.arg("sync"); - cmd.arg(&config.local_path); - cmd.arg(format!("{}:{}", config.remote_name, config.remote_path)); - } - SyncMode::Pull => { - cmd.arg("sync"); - cmd.arg(format!("{}:{}", config.remote_name, config.remote_path)); - cmd.arg(&config.local_path); - } - SyncMode::Bisync => { - cmd.arg("bisync"); - cmd.arg(&config.local_path); - cmd.arg(format!("{}:{}", config.remote_name, config.remote_path)); - cmd.arg("--resync"); - } - } + let cmd_result = match config.sync_mode { + SyncMode::Push => SafeCommand::new("rclone") + .and_then(|c| c.arg("sync")) + .and_then(|c| c.arg(&config.local_path)) + .and_then(|c| c.arg(&remote_spec)), + SyncMode::Pull => SafeCommand::new("rclone") + .and_then(|c| c.arg("sync")) + .and_then(|c| c.arg(&remote_spec)) + .and_then(|c| c.arg(&config.local_path)), + SyncMode::Bisync => SafeCommand::new("rclone") + .and_then(|c| c.arg("bisync")) + .and_then(|c| c.arg(&config.local_path)) + .and_then(|c| c.arg(&remote_spec)) + .and_then(|c| c.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.spawn().map_err(|e| { - if e.kind() == std::io::ErrorKind::NotFound { - "rclone not found. Please install rclone: https://rclone.org/install/".to_string() - } else { - format!("Failed to start rclone: {e}") - } - })?; + let child = cmd_builder + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| { + let err_str = e.to_string(); + 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) -> Result Result { let mut process_guard = RCLONE_PROCESS @@ -188,8 +192,6 @@ pub fn stop_sync() -> Result { }) } -/// # 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") - .args([ - "config", - "create", - remote_name, - "s3", - "provider", - "Minio", - "endpoint", - endpoint, - "access_key_id", - access_key, - "secret_access_key", - secret_key, - "acl", - "private", - ]) + let output = SafeCommand::new("rclone") + .and_then(|c| c.arg("config")) + .and_then(|c| c.arg("create")) + .and_then(|c| c.arg(remote_name)) + .and_then(|c| c.arg("s3")) + .and_then(|c| c.arg("provider")) + .and_then(|c| c.arg("Minio")) + .and_then(|c| c.arg("endpoint")) + .and_then(|c| c.arg(endpoint)) + .and_then(|c| c.arg("access_key_id")) + .and_then(|c| c.arg(access_key)) + .and_then(|c| c.arg("secret_access_key")) + .and_then(|c| c.arg(secret_key)) + .and_then(|c| c.arg("acl")) + .and_then(|c| c.arg("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") - .args(["config", "update", remote_name, "bucket", bucket]) - .output(); + let _ = SafeCommand::new("rclone") + .and_then(|c| c.arg("config")) + .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 { - let output = Command::new("rclone") - .arg("version") + let output = SafeCommand::new("rclone") + .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 { } } -/// # Errors -/// Returns an error if listing rclone remotes fails. #[tauri::command] pub fn list_remotes() -> Result, String> { - let output = Command::new("rclone") - .args(["listremotes"]) + let output = SafeCommand::new("rclone") + .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); diff --git a/src/desktop/tray.rs b/src/desktop/tray.rs index 569df76..150d692 100644 --- a/src/desktop/tray.rs +++ b/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") - .arg(title) - .arg(body) - .spawn(); + if let Ok(cmd) = SafeCommand::new("notify-send") + .and_then(|c| c.arg(title)) + .and_then(|c| c.arg(body)) + { + let _ = cmd.spawn(); + } } #[cfg(target_os = "macos")] { let script = format!("display notification \"{body}\" with title \"{title}\""); - let _ = std::process::Command::new("osascript") - .arg("-e") - .arg(&script) - .spawn(); + if let Ok(cmd) = SafeCommand::new("osascript") + .and_then(|c| c.arg("-e")) + .and_then(|c| c.arg(&script)) + { + let _ = cmd.spawn(); + } } } Ok(())