diff --git a/Cargo.lock b/Cargo.lock index bb572829c..d0820942f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8086,6 +8086,7 @@ dependencies = [ "dirs", "fastembed", "futures", + "hex", "ignore", "indoc", "lance-index", diff --git a/Cargo.toml b/Cargo.toml index f5bb2692b..1181dedb7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ fastembed = "4" # Encoding base64 = "0.22" +hex = "0.4" # Logging and tracing tracing = "0.1" diff --git a/src/agent/cortex_chat.rs b/src/agent/cortex_chat.rs index 6a95f1b4d..aed22fff0 100644 --- a/src/agent/cortex_chat.rs +++ b/src/agent/cortex_chat.rs @@ -52,16 +52,16 @@ pub enum CortexChatEvent { /// Prompt hook that forwards tool events to an mpsc channel for SSE streaming. #[derive(Clone)] struct CortexChatHook { - event_tx: mpsc::UnboundedSender, + event_tx: mpsc::Sender, } impl CortexChatHook { - fn new(event_tx: mpsc::UnboundedSender) -> Self { + fn new(event_tx: mpsc::Sender) -> Self { Self { event_tx } } - fn send(&self, event: CortexChatEvent) { - let _ = self.event_tx.send(event); + async fn send(&self, event: CortexChatEvent) { + let _ = self.event_tx.send(event).await; } } @@ -75,7 +75,7 @@ impl PromptHook for CortexChatHook { ) -> ToolCallHookAction { self.send(CortexChatEvent::ToolStarted { tool: tool_name.to_string(), - }); + }).await; ToolCallHookAction::Continue } @@ -95,7 +95,7 @@ impl PromptHook for CortexChatHook { self.send(CortexChatEvent::ToolCompleted { tool: tool_name.to_string(), result_preview: preview, - }); + }).await; HookAction::Continue } @@ -232,7 +232,7 @@ impl CortexChatSession { thread_id: &str, user_text: &str, channel_context_id: Option<&str>, - ) -> Result, anyhow::Error> { + ) -> Result, anyhow::Error> { let _guard = self.send_lock.lock().await; // Save the user message @@ -271,7 +271,7 @@ impl CortexChatSession { .tool_server_handle(self.tool_server.clone()) .build(); - let (event_tx, event_rx) = mpsc::unbounded_channel(); + let (event_tx, event_rx) = mpsc::channel(256); let hook = CortexChatHook::new(event_tx.clone()); // Clone what the spawned task needs @@ -296,7 +296,7 @@ impl CortexChatSession { .await; let _ = event_tx.send(CortexChatEvent::Done { full_text: response, - }); + }).await; } Err(error) => { let error_text = format!("Cortex chat error: {error}"); @@ -305,7 +305,7 @@ impl CortexChatSession { .await; let _ = event_tx.send(CortexChatEvent::Error { message: error_text, - }); + }).await; } } }); diff --git a/src/api/server.rs b/src/api/server.rs index ca4c59972..99e6f8a33 100644 --- a/src/api/server.rs +++ b/src/api/server.rs @@ -8,14 +8,14 @@ use super::{ use axum::Json; use axum::Router; -use axum::extract::{Request, State}; +use axum::extract::{DefaultBodyLimit, Request, State}; use axum::http::{StatusCode, Uri, header}; use axum::middleware::{self, Next}; use axum::response::{Html, IntoResponse, Response}; use axum::routing::{delete, get, post, put}; use rust_embed::Embed; +use tower_http::cors::CorsLayer; use serde_json::json; -use tower_http::cors::{Any, CorsLayer}; use std::net::SocketAddr; use std::sync::Arc; @@ -36,9 +36,19 @@ pub async fn start_http_server( shutdown_rx: tokio::sync::watch::Receiver, ) -> anyhow::Result> { let cors = CorsLayer::new() - .allow_origin(Any) - .allow_methods(Any) - .allow_headers(Any); + .allow_origin(tower_http::cors::AllowOrigin::mirror_request()) + .allow_methods([ + axum::http::Method::GET, + axum::http::Method::POST, + axum::http::Method::PUT, + axum::http::Method::DELETE, + axum::http::Method::OPTIONS, + ]) + .allow_headers([ + header::CONTENT_TYPE, + header::AUTHORIZATION, + header::ACCEPT, + ]); let api_routes = Router::new() .route("/health", get(system::health)) @@ -157,6 +167,7 @@ pub async fn start_http_server( .nest("/api", api_routes) .fallback(static_handler) .layer(cors) + .layer(DefaultBodyLimit::max(10 * 1024 * 1024)) // 10 MiB .with_state(state); let listener = tokio::net::TcpListener::bind(bind).await?; diff --git a/src/config.rs b/src/config.rs index 6e1c1586a..ffc34c830 100644 --- a/src/config.rs +++ b/src/config.rs @@ -127,7 +127,7 @@ impl<'de> serde::Deserialize<'de> for ApiType { } /// Configuration for a single LLM provider. -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct ProviderConfig { pub api_type: ApiType, pub base_url: String, @@ -135,8 +135,19 @@ pub struct ProviderConfig { pub name: Option, } +impl std::fmt::Debug for ProviderConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ProviderConfig") + .field("api_type", &self.api_type) + .field("base_url", &self.base_url) + .field("api_key", &"[REDACTED]") + .field("name", &self.name) + .finish() + } +} + /// LLM provider credentials (instance-level). -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct LlmConfig { pub anthropic_key: Option, pub openai_key: Option, @@ -159,6 +170,32 @@ pub struct LlmConfig { pub providers: HashMap, } +impl std::fmt::Debug for LlmConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("LlmConfig") + .field("anthropic_key", &self.anthropic_key.as_ref().map(|_| "[REDACTED]")) + .field("openai_key", &self.openai_key.as_ref().map(|_| "[REDACTED]")) + .field("openrouter_key", &self.openrouter_key.as_ref().map(|_| "[REDACTED]")) + .field("zhipu_key", &self.zhipu_key.as_ref().map(|_| "[REDACTED]")) + .field("groq_key", &self.groq_key.as_ref().map(|_| "[REDACTED]")) + .field("together_key", &self.together_key.as_ref().map(|_| "[REDACTED]")) + .field("fireworks_key", &self.fireworks_key.as_ref().map(|_| "[REDACTED]")) + .field("deepseek_key", &self.deepseek_key.as_ref().map(|_| "[REDACTED]")) + .field("xai_key", &self.xai_key.as_ref().map(|_| "[REDACTED]")) + .field("mistral_key", &self.mistral_key.as_ref().map(|_| "[REDACTED]")) + .field("gemini_key", &self.gemini_key.as_ref().map(|_| "[REDACTED]")) + .field("ollama_key", &self.ollama_key.as_ref().map(|_| "[REDACTED]")) + .field("ollama_base_url", &self.ollama_base_url) + .field("opencode_zen_key", &self.opencode_zen_key.as_ref().map(|_| "[REDACTED]")) + .field("nvidia_key", &self.nvidia_key.as_ref().map(|_| "[REDACTED]")) + .field("minimax_key", &self.minimax_key.as_ref().map(|_| "[REDACTED]")) + .field("moonshot_key", &self.moonshot_key.as_ref().map(|_| "[REDACTED]")) + .field("zai_coding_plan_key", &self.zai_coding_plan_key.as_ref().map(|_| "[REDACTED]")) + .field("providers", &self.providers) + .finish() + } +} + impl LlmConfig { /// Check if any provider configuration is set. pub fn has_any_key(&self) -> bool { @@ -198,7 +235,7 @@ pub(crate) const GEMINI_PROVIDER_BASE_URL: &str = "https://generativelanguage.googleapis.com/v1beta/openai"; /// Defaults inherited by all agents. Individual agents can override any field. -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct DefaultsConfig { pub routing: RoutingConfig, pub max_concurrent_branches: usize, @@ -222,6 +259,31 @@ pub struct DefaultsConfig { pub worker_log_mode: crate::settings::WorkerLogMode, } +impl std::fmt::Debug for DefaultsConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DefaultsConfig") + .field("routing", &self.routing) + .field("max_concurrent_branches", &self.max_concurrent_branches) + .field("max_concurrent_workers", &self.max_concurrent_workers) + .field("max_turns", &self.max_turns) + .field("branch_max_turns", &self.branch_max_turns) + .field("context_window", &self.context_window) + .field("compaction", &self.compaction) + .field("memory_persistence", &self.memory_persistence) + .field("coalesce", &self.coalesce) + .field("ingestion", &self.ingestion) + .field("cortex", &self.cortex) + .field("browser", &self.browser) + .field("mcp", &self.mcp) + .field("brave_search_key", &self.brave_search_key.as_ref().map(|_| "[REDACTED]")) + .field("history_backfill_count", &self.history_backfill_count) + .field("cron", &self.cron) + .field("opencode", &self.opencode) + .field("worker_log_mode", &self.worker_log_mode) + .finish() + } +} + /// MCP server configuration. #[derive(Debug, Clone, PartialEq, Eq)] pub struct McpServerConfig { @@ -785,7 +847,7 @@ pub struct MessagingConfig { pub twitch: Option, } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct DiscordConfig { pub enabled: bool, pub token: String, @@ -795,6 +857,17 @@ pub struct DiscordConfig { pub allow_bot_messages: bool, } +impl std::fmt::Debug for DiscordConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DiscordConfig") + .field("enabled", &self.enabled) + .field("token", &"[REDACTED]") + .field("dm_allowed_users", &self.dm_allowed_users) + .field("allow_bot_messages", &self.allow_bot_messages) + .finish() + } +} + /// A single slash command definition for the Slack adapter. /// /// Maps a Slack slash command (e.g. `/ask`) to a target agent. @@ -809,7 +882,7 @@ pub struct SlackCommandConfig { pub description: Option, } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct SlackConfig { pub enabled: bool, pub bot_token: String, @@ -820,6 +893,18 @@ pub struct SlackConfig { pub commands: Vec, } +impl std::fmt::Debug for SlackConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SlackConfig") + .field("enabled", &self.enabled) + .field("bot_token", &"[REDACTED]") + .field("app_token", &"[REDACTED]") + .field("dm_allowed_users", &self.dm_allowed_users) + .field("commands", &self.commands) + .finish() + } +} + /// Hot-reloadable Discord permission filters. /// /// Derived from bindings + discord config. Shared with the Discord adapter @@ -952,7 +1037,7 @@ impl DiscordPermissions { } } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct TelegramConfig { pub enabled: bool, pub token: String, @@ -960,6 +1045,16 @@ pub struct TelegramConfig { pub dm_allowed_users: Vec, } +impl std::fmt::Debug for TelegramConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TelegramConfig") + .field("enabled", &self.enabled) + .field("token", &"[REDACTED]") + .field("dm_allowed_users", &self.dm_allowed_users) + .finish() + } +} + /// Hot-reloadable Telegram permission filters. /// /// Shared with the Telegram adapter via `Arc>` for hot-reloading. @@ -1014,7 +1109,7 @@ impl TelegramPermissions { } } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct TwitchConfig { pub enabled: bool, pub username: String, @@ -1025,6 +1120,18 @@ pub struct TwitchConfig { pub trigger_prefix: Option, } +impl std::fmt::Debug for TwitchConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TwitchConfig") + .field("enabled", &self.enabled) + .field("username", &self.username) + .field("oauth_token", &"[REDACTED]") + .field("channels", &self.channels) + .field("trigger_prefix", &self.trigger_prefix) + .finish() + } +} + /// Hot-reloadable Twitch permission filters. /// /// Shared with the Twitch adapter via `Arc>` for hot-reloading. @@ -2147,18 +2254,23 @@ impl Config { .providers .into_iter() .map(|(provider_id, config)| { - ( + let api_key = resolve_env_value(&config.api_key).ok_or_else(|| { + anyhow::anyhow!( + "failed to resolve API key for provider '{}'", + provider_id + ) + })?; + Ok(( provider_id.to_lowercase(), ProviderConfig { api_type: config.api_type, base_url: config.base_url, - api_key: resolve_env_value(&config.api_key) - .expect("Failed to resolve API key for provider"), + api_key, name: config.name, }, - ) + )) }) - .collect(), + .collect::>()?, }; if let Some(anthropic_key) = llm.anthropic_key.clone() { diff --git a/src/hooks/spacebot.rs b/src/hooks/spacebot.rs index 26fe87ce3..deb3b3708 100644 --- a/src/hooks/spacebot.rs +++ b/src/hooks/spacebot.rs @@ -43,35 +43,24 @@ impl SpacebotHook { let _ = self.event_tx.send(event); } - /// Scan content for potential secret leaks. - fn scan_for_leaks(&self, content: &str) -> Option { + /// Check a string against the leak pattern set. + fn match_patterns(content: &str) -> Option { use regex::Regex; use std::sync::LazyLock; static LEAK_PATTERNS: LazyLock> = LazyLock::new(|| { vec![ - // OpenAI keys Regex::new(r"sk-[a-zA-Z0-9]{20,}").expect("hardcoded regex"), - // Anthropic keys Regex::new(r"sk-ant-[a-zA-Z0-9_-]{20,}").expect("hardcoded regex"), - // OpenRouter keys Regex::new(r"sk-or-[a-zA-Z0-9_-]{20,}").expect("hardcoded regex"), - // PEM private keys Regex::new(r"-----BEGIN.*PRIVATE KEY-----").expect("hardcoded regex"), - // GitHub personal access tokens Regex::new(r"ghp_[a-zA-Z0-9]{36}").expect("hardcoded regex"), - // Google API keys Regex::new(r"AIza[0-9A-Za-z_-]{35}").expect("hardcoded regex"), - // Discord bot tokens (base64 user ID . timestamp . HMAC) Regex::new(r"[MN][A-Za-z0-9]{23,}\.[A-Za-z0-9_-]{6}\.[A-Za-z0-9_-]{27,}") .expect("hardcoded regex"), - // Slack bot tokens Regex::new(r"xoxb-[0-9]{10,}-[0-9A-Za-z-]+").expect("hardcoded regex"), - // Slack app tokens Regex::new(r"xapp-[0-9]-[A-Z0-9]+-[0-9]+-[a-f0-9]+").expect("hardcoded regex"), - // Telegram bot tokens Regex::new(r"\d{8,}:[A-Za-z0-9_-]{35}").expect("hardcoded regex"), - // Brave Search API keys Regex::new(r"BSA[a-zA-Z0-9]{20,}").expect("hardcoded regex"), ] }); @@ -84,6 +73,73 @@ impl SpacebotHook { None } + + /// Scan content for potential secret leaks, including encoded forms. + /// + /// Checks raw content first, then attempts URL-decoding, base64-decoding, + /// and hex-decoding to catch secrets that an LLM might encode to evade + /// plaintext pattern matching. + fn scan_for_leaks(&self, content: &str) -> Option { + use base64::Engine; + use regex::Regex; + use std::sync::LazyLock; + + if let Some(matched) = Self::match_patterns(content) { + return Some(matched); + } + + // Percent-encoded secrets (e.g. sk%2Dant%2D...) + let url_decoded = urlencoding::decode(content).unwrap_or(std::borrow::Cow::Borrowed("")); + if url_decoded != content { + if let Some(matched) = Self::match_patterns(&url_decoded) { + return Some(format!("url-encoded: {matched}")); + } + } + + // Base64-wrapped secrets. Minimum 24 chars avoids false positives on + // short alphanumeric strings while catching any encoded API key. + static BASE64_SEGMENT: LazyLock = LazyLock::new(|| { + Regex::new(r"[A-Za-z0-9+/]{24,}={0,2}").expect("hardcoded regex") + }); + for segment in BASE64_SEGMENT.find_iter(content) { + if let Ok(decoded_bytes) = + base64::engine::general_purpose::STANDARD.decode(segment.as_str()) + { + if let Ok(decoded) = std::str::from_utf8(&decoded_bytes) { + if let Some(matched) = Self::match_patterns(decoded) { + return Some(format!("base64-encoded: {matched}")); + } + } + } + if let Ok(decoded_bytes) = + base64::engine::general_purpose::URL_SAFE.decode(segment.as_str()) + { + if let Ok(decoded) = std::str::from_utf8(&decoded_bytes) { + if let Some(matched) = Self::match_patterns(decoded) { + return Some(format!("base64-encoded: {matched}")); + } + } + } + } + + // Hex-encoded secrets. Minimum 40 hex chars (20 bytes) to reduce + // false positives while catching any hex-wrapped API key. + static HEX_SEGMENT: LazyLock = LazyLock::new(|| { + Regex::new(r"(?i)(?:0x)?([0-9a-f]{40,})").expect("hardcoded regex") + }); + for caps in HEX_SEGMENT.captures_iter(content) { + let hex_str = caps.get(1).map_or("", |m| m.as_str()); + if let Ok(decoded_bytes) = hex::decode(hex_str) { + if let Ok(decoded) = std::str::from_utf8(&decoded_bytes) { + if let Some(matched) = Self::match_patterns(decoded) { + return Some(format!("hex-encoded: {matched}")); + } + } + } + } + + None + } } // Timer map for tool call duration measurement. Entries are inserted in diff --git a/src/main.rs b/src/main.rs index 0b7fc6b4e..482911d57 100644 --- a/src/main.rs +++ b/src/main.rs @@ -129,7 +129,7 @@ struct ActiveChannel { fn main() -> anyhow::Result<()> { rustls::crypto::ring::default_provider() .install_default() - .expect("failed to install rustls crypto provider"); + .map_err(|_| anyhow::anyhow!("failed to install rustls crypto provider"))?; let cli = Cli::parse(); let command = cli.command.unwrap_or(Command::Start { foreground: false }); @@ -559,12 +559,15 @@ fn get_agent_config<'a>( config: &'a spacebot::config::Config, agent_id: Option<&str>, ) -> anyhow::Result<&'a spacebot::config::AgentConfig> { - let agent_id = agent_id.unwrap_or_else(|| { - if config.agents.is_empty() { - panic!("no agents configured"); + let agent_id = match agent_id { + Some(id) => id, + None => { + if config.agents.is_empty() { + anyhow::bail!("no agents configured"); + } + &config.agents[0].id } - &config.agents[0].id - }); + }; config .agents @@ -1373,9 +1376,9 @@ async fn initialize_agents( { let adapter = spacebot::messaging::discord::DiscordAdapter::new( &discord_config.token, - discord_permissions - .clone() - .expect("discord permissions initialized when discord is enabled"), + discord_permissions.clone().ok_or_else(|| { + anyhow::anyhow!("discord permissions not initialized when discord is enabled") + })?, ); new_messaging_manager.register(adapter).await; } @@ -1395,9 +1398,9 @@ async fn initialize_agents( match spacebot::messaging::slack::SlackAdapter::new( &slack_config.bot_token, &slack_config.app_token, - slack_permissions - .clone() - .expect("slack permissions initialized when slack is enabled"), + slack_permissions.clone().ok_or_else(|| { + anyhow::anyhow!("slack permissions not initialized when slack is enabled") + })?, slack_config.commands.clone(), ) { Ok(adapter) => { @@ -1421,9 +1424,9 @@ async fn initialize_agents( { let adapter = spacebot::messaging::telegram::TelegramAdapter::new( &telegram_config.token, - telegram_permissions - .clone() - .expect("telegram permissions initialized when telegram is enabled"), + telegram_permissions.clone().ok_or_else(|| { + anyhow::anyhow!("telegram permissions not initialized when telegram is enabled") + })?, ); new_messaging_manager.register(adapter).await; } @@ -1454,9 +1457,9 @@ async fn initialize_agents( &twitch_config.oauth_token, twitch_config.channels.clone(), twitch_config.trigger_prefix.clone(), - twitch_permissions - .clone() - .expect("twitch permissions initialized when twitch is enabled"), + twitch_permissions.clone().ok_or_else(|| { + anyhow::anyhow!("twitch permissions not initialized when twitch is enabled") + })?, ); new_messaging_manager.register(adapter).await; } diff --git a/src/mcp.rs b/src/mcp.rs index c19129df1..8f111f178 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -306,6 +306,13 @@ impl McpConnection { .collect::>(); let mut child_command = tokio::process::Command::new(&resolved_command); + child_command.env_clear(); + // Keep PATH so stdio servers launched via shims/shebangs + // (for example `npx` -> `/usr/bin/env node`) can still + // resolve their runtime without inheriting unrelated secrets. + if let Ok(path) = std::env::var("PATH") { + child_command.env("PATH", path); + } child_command.args(&resolved_args); child_command.envs(&resolved_env); diff --git a/src/memory/lance.rs b/src/memory/lance.rs index d07d9ace4..0942a9387 100644 --- a/src/memory/lance.rs +++ b/src/memory/lance.rs @@ -124,6 +124,7 @@ impl EmbeddingTable { /// Delete an embedding by memory ID. pub async fn delete(&self, memory_id: &str) -> Result<()> { + Self::validate_memory_id(memory_id)?; let predicate = format!("id = '{}'", memory_id); self.table .delete(&predicate) @@ -196,6 +197,7 @@ impl EmbeddingTable { threshold: f32, limit: usize, ) -> Result> { + Self::validate_memory_id(memory_id)?; // First, retrieve the embedding for this memory use lancedb::query::{ExecutableQuery, QueryBase}; @@ -356,4 +358,16 @@ impl EmbeddingTable { ), ]) } + + /// Validate that a memory ID is a well-formed UUID to prevent predicate injection. + fn validate_memory_id(memory_id: &str) -> Result<()> { + if memory_id.len() != 36 + || !memory_id + .chars() + .all(|c| c.is_ascii_hexdigit() || c == '-') + { + return Err(DbError::LanceDb(format!("invalid memory ID format: {}", memory_id)).into()); + } + Ok(()) + } } diff --git a/src/messaging/slack.rs b/src/messaging/slack.rs index 19e896665..22f6596b7 100644 --- a/src/messaging/slack.rs +++ b/src/messaging/slack.rs @@ -141,7 +141,9 @@ async fn handle_message_event( let state_guard = states.read().await; let adapter_state = state_guard .get_user_state::>() - .expect("SlackAdapterState must be in user_state"); + .ok_or_else(|| Box::::from( + "SlackAdapterState not found in user_state", + ))?; let user_id = msg_event.sender.user.as_ref().map(|u| u.0.clone()); @@ -240,7 +242,9 @@ async fn handle_app_mention_event( let state_guard = states.read().await; let adapter_state = state_guard .get_user_state::>() - .expect("SlackAdapterState must be in user_state"); + .ok_or_else(|| Box::::from( + "SlackAdapterState not found in user_state", + ))?; let user_id = mention.user.0.clone(); @@ -339,7 +343,9 @@ async fn handle_command_event( let state_guard = states.read().await; let adapter_state = state_guard .get_user_state::>() - .expect("SlackAdapterState must be in user_state"); + .ok_or_else(|| Box::::from( + "SlackAdapterState not found in user_state", + ))?; let command_str = event.command.0.clone(); let team_id = event.team_id.0.clone(); @@ -478,7 +484,9 @@ async fn handle_interaction_event( let state_guard = states.read().await; let adapter_state = state_guard .get_user_state::>() - .expect("SlackAdapterState must be in user_state"); + .ok_or_else(|| Box::::from( + "SlackAdapterState not found in user_state", + ))?; let user_id = block_actions .user diff --git a/src/tools/exec.rs b/src/tools/exec.rs index 9a4c67ee6..a805ed786 100644 --- a/src/tools/exec.rs +++ b/src/tools/exec.rs @@ -209,6 +209,37 @@ impl Tool for ExecTool { } } + // Block env vars that enable library injection or alter runtime + // loading behavior — these allow arbitrary code execution. + const DANGEROUS_ENV_VARS: &[&str] = &[ + "LD_PRELOAD", + "LD_LIBRARY_PATH", + "DYLD_INSERT_LIBRARIES", + "DYLD_LIBRARY_PATH", + "PYTHONPATH", + "PYTHONSTARTUP", + "NODE_OPTIONS", + "RUBYOPT", + "PERL5OPT", + "PERL5LIB", + "BASH_ENV", + "ENV", + ]; + for env_var in &args.env { + if DANGEROUS_ENV_VARS + .iter() + .any(|blocked| env_var.key.eq_ignore_ascii_case(blocked)) + { + return Err(ExecError { + message: format!( + "Cannot set {}: this environment variable enables code injection.", + env_var.key + ), + exit_code: -1, + }); + } + } + let mut cmd = Command::new(&args.program); cmd.args(&args.args); diff --git a/src/tools/file.rs b/src/tools/file.rs index 3007c0b21..f8e9a49df 100644 --- a/src/tools/file.rs +++ b/src/tools/file.rs @@ -51,6 +51,26 @@ impl FileTool { ))); } + // Reject paths containing symlinks to prevent TOCTOU races where a + // path component is replaced with a symlink between resolution and I/O. + { + let mut check = workspace_canonical.clone(); + if let Ok(relative) = canonical.strip_prefix(&workspace_canonical) { + for component in relative.components() { + check.push(component); + if let Ok(metadata) = std::fs::symlink_metadata(&check) { + if metadata.file_type().is_symlink() { + return Err(FileError( + "ACCESS DENIED: Symlinks are not allowed within the workspace \ + for security reasons. Use direct paths instead." + .to_string(), + )); + } + } + } + } + } + Ok(canonical) } } @@ -180,6 +200,18 @@ impl Tool for FileTool { match args.operation.as_str() { "read" => do_file_read(&path).await, "write" => { + // Identity files remain readable, but writes must go through + // the dedicated identity API to keep update flow consistent. + let file_name = path.file_name().and_then(|n| n.to_str()).unwrap_or(""); + const PROTECTED_FILES: &[&str] = &["SOUL.md", "IDENTITY.md", "USER.md"]; + if PROTECTED_FILES.iter().any(|f| file_name.eq_ignore_ascii_case(f)) { + return Err(FileError( + "ACCESS DENIED: Identity files are protected and cannot be modified \ + through file operations. Use the identity management API instead." + .to_string(), + )); + } + let content = args.content.ok_or_else(|| { FileError("Content is required for write operation".to_string()) })?; diff --git a/src/tools/memory_save.rs b/src/tools/memory_save.rs index 7563d8fc6..2e1400a9f 100644 --- a/src/tools/memory_save.rs +++ b/src/tools/memory_save.rs @@ -206,6 +206,29 @@ impl Tool for MemorySaveTool { _ => crate::memory::types::RelationType::RelatedTo, }; + // Verify the target memory exists before creating a graph edge + // to prevent dangling associations from LLM hallucinated IDs. + match store.load(&assoc.target_id).await { + Ok(Some(_)) => {} + Ok(None) => { + tracing::warn!( + memory_id = %memory.id, + target_id = %assoc.target_id, + "skipping association to non-existent memory" + ); + continue; + } + Err(error) => { + tracing::warn!( + memory_id = %memory.id, + target_id = %assoc.target_id, + %error, + "failed to verify target memory for association" + ); + continue; + } + } + let association = Association::new(&memory.id, &assoc.target_id, relation_type) .with_weight(assoc.weight); diff --git a/src/tools/shell.rs b/src/tools/shell.rs index 7f6628231..a461d3572 100644 --- a/src/tools/shell.rs +++ b/src/tools/shell.rs @@ -89,7 +89,7 @@ impl ShellTool { } } - // Block access to secret environment variables + // Block access to secret environment variables via any expansion syntax for var in SECRET_ENV_VARS { if command.contains(&format!("${var}")) || command.contains(&format!("${{{var}}}")) @@ -100,6 +100,19 @@ impl ShellTool { exit_code: -1, }); } + // Block unbraced $VAR at word boundaries (covers `echo $SECRET` patterns) + let dollar_var = format!("${var}"); + if let Some(pos) = command.find(&dollar_var) { + let after = pos + dollar_var.len(); + let next_char = command[after..].chars().next(); + // $VAR is a match if followed by non-alphanumeric/underscore or end-of-string + if next_char.is_none() || (!next_char.unwrap().is_alphanumeric() && next_char.unwrap() != '_') { + return Err(ShellError { + message: "Cannot access secret environment variables.".to_string(), + exit_code: -1, + }); + } + } } // Block broad env dumps that would expose secrets @@ -128,6 +141,25 @@ impl ShellTool { } } + // Block shell builtins that dump all variables + for dump_cmd in ["set", "declare -p", "export -p", "compgen -e", "compgen -v"] { + let trimmed = command.trim(); + if trimmed == dump_cmd + || trimmed.starts_with(&format!("{dump_cmd} ")) + || trimmed.starts_with(&format!("{dump_cmd}|")) + || trimmed.starts_with(&format!("{dump_cmd}>")) + || trimmed.contains(&format!("| {dump_cmd}")) + || trimmed.contains(&format!("; {dump_cmd}")) + || trimmed.contains(&format!("&& {dump_cmd}")) + { + return Err(ShellError { + message: "Cannot dump environment variables — they may contain secrets." + .to_string(), + exit_code: -1, + }); + } + } + // Block subshell/command substitution that could bypass string-level checks. // Backtick and $() let an attacker compose a blocked command dynamically. if command.contains('`') @@ -151,6 +183,17 @@ impl ShellTool { }); } + // Block interpreter one-liners that can bypass shell-level restrictions + for interpreter in ["python3 -c", "python -c", "perl -e", "ruby -e", "node -e", "node --eval"] { + if command.contains(interpreter) { + return Err(ShellError { + message: "Inline interpreter execution is not permitted — use script files instead." + .to_string(), + exit_code: -1, + }); + } + } + // Block /proc entries and /dev paths that expose environment or fd contents if command.contains("/proc/self/environ") || command.contains("/proc/*/environ")