diff --git a/src/api/messaging.rs b/src/api/messaging.rs index 57b160e5a..59811566c 100644 --- a/src/api/messaging.rs +++ b/src/api/messaging.rs @@ -386,6 +386,7 @@ pub(super) async fn toggle_platform( let adapter = crate::messaging::webhook::WebhookAdapter::new( webhook_config.port, &webhook_config.bind, + webhook_config.auth_token.clone(), ); if let Err(error) = manager.register_and_start(adapter).await { tracing::error!(%error, "failed to start webhook adapter on toggle"); diff --git a/src/api/server.rs b/src/api/server.rs index 4b69afcbd..46196dd10 100644 --- a/src/api/server.rs +++ b/src/api/server.rs @@ -6,11 +6,15 @@ use super::{ providers, settings, skills, system, webchat, }; +use axum::Json; +use axum::extract::Request; use axum::Router; 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 serde_json::json; use tower_http::cors::{Any, CorsLayer}; use std::net::SocketAddr; @@ -131,7 +135,11 @@ pub async fn start_http_server( ) .route("/update/apply", post(settings::update_apply)) .route("/webchat/send", post(webchat::webchat_send)) - .route("/webchat/history", get(webchat::webchat_history)); + .route("/webchat/history", get(webchat::webchat_history)) + .layer(middleware::from_fn_with_state( + state.clone(), + api_auth_middleware, + )); let app = Router::new() .nest("/api", api_routes) @@ -157,6 +165,30 @@ pub async fn start_http_server( Ok(handle) } +async fn api_auth_middleware(state: Arc, request: Request, next: Next) -> Response { + let Some(expected_token) = state.auth_token.as_deref() else { + return next.run(request).await; + }; + + let path = request.uri().path(); + if path == "/api/health" || path == "/health" { + return next.run(request).await; + } + + let is_authorized = request + .headers() + .get(header::AUTHORIZATION) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.strip_prefix("Bearer ")) + .is_some_and(|token| token == expected_token); + + if is_authorized { + next.run(request).await + } else { + (StatusCode::UNAUTHORIZED, Json(json!({"error": "unauthorized"}))).into_response() + } +} + async fn static_handler(uri: Uri) -> Response { let path = uri.path().trim_start_matches('/'); diff --git a/src/api/state.rs b/src/api/state.rs index 60ffdb3d4..d3e3d6288 100644 --- a/src/api/state.rs +++ b/src/api/state.rs @@ -37,6 +37,7 @@ pub struct AgentInfo { /// State shared across all API handlers. pub struct ApiState { pub started_at: Instant, + pub auth_token: Option, /// Aggregated event stream from all agents. SSE clients subscribe here. pub event_tx: broadcast::Sender, /// Per-agent SQLite pools for querying channel/conversation data. @@ -182,6 +183,7 @@ impl ApiState { let (event_tx, _) = broadcast::channel(512); Self { started_at: Instant::now(), + auth_token: None, event_tx, agent_pools: arc_swap::ArcSwap::from_pointee(HashMap::new()), agent_configs: arc_swap::ArcSwap::from_pointee(Vec::new()), diff --git a/src/config.rs b/src/config.rs index 8b3cb7d94..cb6cb1950 100644 --- a/src/config.rs +++ b/src/config.rs @@ -60,6 +60,7 @@ pub struct ApiConfig { pub port: u16, /// Address to bind the HTTP server on. pub bind: String, + pub auth_token: Option, } impl Default for ApiConfig { @@ -68,6 +69,7 @@ impl Default for ApiConfig { enabled: true, port: 19898, bind: "127.0.0.1".into(), + auth_token: None, } } } @@ -1072,6 +1074,7 @@ pub struct WebhookConfig { pub enabled: bool, pub port: u16, pub bind: String, + pub auth_token: Option, } // -- TOML deserialization types -- @@ -1112,6 +1115,8 @@ struct TomlApiConfig { port: u16, #[serde(default = "default_api_bind")] bind: String, + #[serde(default)] + auth_token: Option, } impl Default for TomlApiConfig { @@ -1120,6 +1125,7 @@ impl Default for TomlApiConfig { enabled: default_api_enabled(), port: default_api_port(), bind: default_api_bind(), + auth_token: None, } } } @@ -1509,6 +1515,7 @@ struct TomlWebhookConfig { port: u16, #[serde(default = "default_webhook_bind")] bind: String, + auth_token: Option, } #[derive(Deserialize)] @@ -2670,6 +2677,7 @@ impl Config { enabled: w.enabled, port: w.port, bind: w.bind, + auth_token: w.auth_token.as_deref().and_then(resolve_env_value), }), twitch: toml.messaging.twitch.and_then(|t| { let username = t @@ -2711,6 +2719,7 @@ impl Config { enabled: toml.api.enabled, port: toml.api.port, bind: hosted_api_bind(toml.api.bind), + auth_token: toml.api.auth_token.as_deref().and_then(resolve_env_value), }; let metrics = MetricsConfig { diff --git a/src/main.rs b/src/main.rs index 9e81056fe..0b7fc6b4e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -607,11 +607,13 @@ async fn run( let (agent_remove_tx, mut agent_remove_rx) = mpsc::channel::(8); // Start HTTP API server if enabled - let api_state = Arc::new(spacebot::api::ApiState::new_with_provider_sender( + let mut api_state = spacebot::api::ApiState::new_with_provider_sender( provider_tx, agent_tx, agent_remove_tx, - )); + ); + api_state.auth_token = config.api.auth_token.clone(); + let api_state = Arc::new(api_state); // Start background update checker spacebot::update::spawn_update_checker(api_state.update_status.clone()); @@ -1432,6 +1434,7 @@ async fn initialize_agents( let adapter = spacebot::messaging::webhook::WebhookAdapter::new( webhook_config.port, &webhook_config.bind, + webhook_config.auth_token.clone(), ); new_messaging_manager.register(adapter).await; } diff --git a/src/messaging/webhook.rs b/src/messaging/webhook.rs index c4c4be231..e43f742ef 100644 --- a/src/messaging/webhook.rs +++ b/src/messaging/webhook.rs @@ -5,24 +5,26 @@ //! the integration point for scripts, CI pipelines, and other programs //! that need to interact with Spacebot programmatically. -use crate::messaging::traits::{InboundStream, Messaging}; -use crate::{InboundMessage, MessageContent, OutboundResponse}; +use std::collections::HashMap; +use std::sync::Arc; use anyhow::Context as _; use axum::Router; use axum::extract::{Json, State}; -use axum::http::StatusCode; +use axum::http::header::AUTHORIZATION; +use axum::http::{HeaderMap, StatusCode}; use axum::routing::{get, post}; use serde::{Deserialize, Serialize}; - -use std::collections::HashMap; -use std::sync::Arc; use tokio::sync::{RwLock, mpsc}; +use crate::messaging::traits::{InboundStream, Messaging}; +use crate::{InboundMessage, MessageContent, OutboundResponse}; + /// Webhook adapter state. pub struct WebhookAdapter { port: u16, bind: String, + auth_token: Option, inbound_tx: Arc>>>, /// Buffered responses per conversation_id, waiting to be polled. response_buffers: Arc>>>, @@ -34,6 +36,7 @@ pub struct WebhookAdapter { struct AppState { inbound_tx: Arc>>>, response_buffers: Arc>>>, + auth_token: Option, } /// Inbound webhook request body. @@ -72,10 +75,11 @@ struct PollResponse { } impl WebhookAdapter { - pub fn new(port: u16, bind: impl Into) -> Self { + pub fn new(port: u16, bind: impl Into, auth_token: Option) -> Self { Self { port, bind: bind.into(), + auth_token, inbound_tx: Arc::new(RwLock::new(None)), response_buffers: Arc::new(RwLock::new(HashMap::new())), shutdown_tx: Arc::new(RwLock::new(None)), @@ -98,8 +102,15 @@ impl Messaging for WebhookAdapter { let state = AppState { inbound_tx: self.inbound_tx.clone(), response_buffers: self.response_buffers.clone(), + auth_token: self.auth_token.clone(), }; + if self.auth_token.is_none() { + tracing::warn!( + "webhook authentication is disabled because no auth token is configured" + ); + } + let app = Router::new() .route("/send", post(handle_send)) .route("/poll/{conversation_id}", get(handle_poll)) @@ -226,9 +237,14 @@ impl Messaging for WebhookAdapter { // -- Axum handlers -- async fn handle_send( + headers: HeaderMap, State(state): State, Json(request): Json, ) -> Result { + if !is_authorized(&headers, state.auth_token.as_deref()) { + return Err((StatusCode::UNAUTHORIZED, "unauthorized".into())); + } + let tx = state.inbound_tx.read().await; let Some(tx) = tx.as_ref() else { return Err(( @@ -269,9 +285,14 @@ async fn handle_send( } async fn handle_poll( + headers: HeaderMap, State(state): State, axum::extract::Path(conversation_id): axum::extract::Path, -) -> Json { +) -> Result, (StatusCode, String)> { + if !is_authorized(&headers, state.auth_token.as_deref()) { + return Err((StatusCode::UNAUTHORIZED, "unauthorized".into())); + } + let key = format!("webhook:{conversation_id}"); let messages = state .response_buffers @@ -280,9 +301,29 @@ async fn handle_poll( .remove(&key) .unwrap_or_default(); - Json(PollResponse { messages }) + Ok(Json(PollResponse { messages })) } async fn handle_health() -> StatusCode { StatusCode::OK } + +fn is_authorized(headers: &HeaderMap, expected_token: Option<&str>) -> bool { + let Some(expected_token) = expected_token else { + return true; + }; + + if headers + .get("x-webhook-token") + .and_then(|value| value.to_str().ok()) + .is_some_and(|token| token == expected_token) + { + return true; + } + + headers + .get(AUTHORIZATION) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.strip_prefix("Bearer ")) + .is_some_and(|token| token == expected_token) +} diff --git a/src/secrets/store.rs b/src/secrets/store.rs index 007ba5d40..5b11681cc 100644 --- a/src/secrets/store.rs +++ b/src/secrets/store.rs @@ -1,17 +1,210 @@ //! Encrypted credentials storage (AES-256-GCM, redb). -/// Secrets store. -pub struct SecretsStore; +use crate::error::SecretsError; +use aes_gcm::{aead::Aead, Aes256Gcm, KeyInit, Nonce}; +use rand::RngCore; +use redb::{Database, ReadableTable, TableDefinition}; +use sha2::{Digest, Sha256}; +use std::fmt::{Debug, Display, Formatter}; +use std::path::Path; + +const SECRETS_TABLE: TableDefinition<&str, &[u8]> = TableDefinition::new("secrets"); + +pub struct DecryptedSecret(String); + +impl DecryptedSecret { + pub fn expose(&self) -> &str { + &self.0 + } +} + +impl Debug for DecryptedSecret { + fn fmt(&self, formatter: &mut Formatter<'_>) -> std::fmt::Result { + write!(formatter, "DecryptedSecret(***)") + } +} + +impl Display for DecryptedSecret { + fn fmt(&self, formatter: &mut Formatter<'_>) -> std::fmt::Result { + write!(formatter, "***") + } +} + +pub struct SecretsStore { + db: Database, +} impl SecretsStore { - /// Create a new secrets store. - pub fn new() -> Self { - Self + pub fn new(path: impl AsRef) -> Result { + let db = Database::create(path.as_ref()).map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to open secrets database: {error}")) + })?; + + let write_transaction = db.begin_write().map_err(|error| { + SecretsError::Other(anyhow::anyhow!( + "failed to initialize secrets table transaction: {error}" + )) + })?; + { + let _table = write_transaction + .open_table(SECRETS_TABLE) + .map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to open secrets table: {error}")) + })?; + } + write_transaction.commit().map_err(|error| { + SecretsError::Other(anyhow::anyhow!( + "failed to commit secrets table initialization: {error}" + )) + })?; + + Ok(Self { db }) + } + + pub fn set(&self, key: &str, value: &str, master_key: &[u8]) -> Result<(), SecretsError> { + let cipher = build_cipher(master_key)?; + + let mut nonce_bytes = [0_u8; 12]; + rand::rng().fill_bytes(&mut nonce_bytes); + let nonce = Nonce::from_slice(&nonce_bytes); + + let ciphertext = cipher + .encrypt(nonce, value.as_bytes()) + .map_err(|error| SecretsError::EncryptionFailed(error.to_string()))?; + + let mut stored_value = Vec::with_capacity(nonce_bytes.len() + ciphertext.len()); + stored_value.extend_from_slice(&nonce_bytes); + stored_value.extend_from_slice(&ciphertext); + + let write_transaction = self.db.begin_write().map_err(|error| { + SecretsError::Other(anyhow::anyhow!( + "failed to begin write transaction: {error}" + )) + })?; + + { + let mut table = write_transaction + .open_table(SECRETS_TABLE) + .map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to open secrets table: {error}")) + })?; + + table + .insert(key, stored_value.as_slice()) + .map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to insert secret '{key}': {error}")) + })?; + } + + write_transaction.commit().map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to commit secret '{key}': {error}")) + })?; + + Ok(()) + } + + pub fn get(&self, key: &str, master_key: &[u8]) -> Result { + let cipher = build_cipher(master_key)?; + + let read_transaction = self.db.begin_read().map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to begin read transaction: {error}")) + })?; + let table = read_transaction + .open_table(SECRETS_TABLE) + .map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to open secrets table: {error}")) + })?; + + let value = table + .get(key) + .map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to read key '{key}': {error}")) + })? + .ok_or_else(|| SecretsError::NotFound { + key: key.to_string(), + })?; + + let encrypted_value = value.value(); + if encrypted_value.len() < 12 { + return Err(SecretsError::DecryptionFailed( + "stored secret is missing nonce prefix".to_string(), + )); + } + + let nonce = Nonce::from_slice(&encrypted_value[..12]); + let ciphertext = &encrypted_value[12..]; + let plaintext = cipher + .decrypt(nonce, ciphertext) + .map_err(|error| SecretsError::DecryptionFailed(error.to_string()))?; + + let plaintext = String::from_utf8(plaintext) + .map_err(|error| SecretsError::DecryptionFailed(error.to_string()))?; + + Ok(DecryptedSecret(plaintext)) + } + + pub fn delete(&self, key: &str) -> Result<(), SecretsError> { + let write_transaction = self.db.begin_write().map_err(|error| { + SecretsError::Other(anyhow::anyhow!( + "failed to begin write transaction: {error}" + )) + })?; + + { + let mut table = write_transaction + .open_table(SECRETS_TABLE) + .map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to open secrets table: {error}")) + })?; + + table.remove(key).map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to remove key '{key}': {error}")) + })?; + } + + write_transaction.commit().map_err(|error| { + SecretsError::Other(anyhow::anyhow!( + "failed to commit delete for '{key}': {error}" + )) + })?; + + Ok(()) + } + + pub fn list(&self) -> Result, SecretsError> { + let read_transaction = self.db.begin_read().map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to begin read transaction: {error}")) + })?; + let table = read_transaction + .open_table(SECRETS_TABLE) + .map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to open secrets table: {error}")) + })?; + + let mut keys = Vec::new(); + let iter = table.iter().map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to iterate secrets table: {error}")) + })?; + + for entry in iter { + let (key, _value) = entry.map_err(|error| { + SecretsError::Other(anyhow::anyhow!("failed to read secrets entry: {error}")) + })?; + keys.push(key.value().to_string()); + } + + Ok(keys) } } -impl Default for SecretsStore { - fn default() -> Self { - Self::new() +fn build_cipher(master_key: &[u8]) -> Result { + if master_key.is_empty() { + return Err(SecretsError::InvalidKey); } + + let mut hasher = Sha256::new(); + hasher.update(master_key); + let digest = hasher.finalize(); + + Aes256Gcm::new_from_slice(&digest).map_err(|_| SecretsError::InvalidKey) } diff --git a/src/tools/browser.rs b/src/tools/browser.rs index 101a4f948..e650a5ac3 100644 --- a/src/tools/browser.rs +++ b/src/tools/browser.rs @@ -4,6 +4,7 @@ //! via headless Chrome using chromiumoxide. Uses an accessibility-tree based //! ref system for LLM-friendly element addressing. +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use crate::config::BrowserConfig; use chromiumoxide::browser::{Browser, BrowserConfig as ChromeConfig}; @@ -26,6 +27,105 @@ use std::sync::Arc; use tokio::sync::Mutex; use tokio::task::JoinHandle; +/// Validate that a URL is safe for the browser to navigate to. +/// Blocks private/loopback IPs, link-local addresses, and cloud metadata endpoints +/// to prevent server-side request forgery. +fn validate_url(url: &str) -> Result<(), BrowserError> { + let parsed = url::Url::parse(url).map_err(|error| { + BrowserError::new(format!("invalid URL '{url}': {error}")) + })?; + + match parsed.scheme() { + "http" | "https" => {} + other => { + return Err(BrowserError::new(format!( + "scheme '{other}' is not allowed — only http and https are permitted" + ))); + } + } + + let Some(host) = parsed.host_str() else { + return Err(BrowserError::new("URL has no host")); + }; + + // Block cloud metadata endpoints regardless of how the IP resolves + if host == "metadata.google.internal" + || host == "169.254.169.254" + || host == "metadata.aws.internal" + { + return Err(BrowserError::new( + "access to cloud metadata endpoints is blocked", + )); + } + + // If the host parses as an IP address, check against blocked ranges + if let Ok(ip) = host.parse::() { + if is_blocked_ip(ip) { + return Err(BrowserError::new(format!( + "navigation to private/loopback address {ip} is blocked" + ))); + } + } + + // IPv6 addresses in brackets (url crate strips them for host_str) + if let Some(stripped) = host.strip_prefix('[').and_then(|h| h.strip_suffix(']')) { + if let Ok(ip) = stripped.parse::() { + if is_blocked_ip(ip) { + return Err(BrowserError::new(format!( + "navigation to private/loopback address {ip} is blocked" + ))); + } + } + } + + Ok(()) +} + +/// Returns true if the IP address belongs to a private, loopback, or +/// link-local range that should not be reachable from the browser tool. +fn is_blocked_ip(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(v4) => { + v4.is_loopback() // 127.0.0.0/8 + || v4.is_private() // 10/8, 172.16/12, 192.168/16 + || v4.is_link_local() // 169.254.0.0/16 + || v4.is_broadcast() // 255.255.255.255 + || v4.is_unspecified() // 0.0.0.0 + || is_v4_cgnat(v4) // 100.64.0.0/10 + } + IpAddr::V6(v6) => { + v6.is_loopback() // ::1 + || v6.is_unspecified() // :: + || is_v6_unique_local(v6) // fd00::/8 (fc00::/7) + || is_v6_link_local(v6) // fe80::/10 + || is_v4_mapped_blocked(v6) + } + } +} + +fn is_v4_cgnat(ip: Ipv4Addr) -> bool { + let octets = ip.octets(); + octets[0] == 100 && (octets[1] & 0xC0) == 64 // 100.64.0.0/10 +} + +fn is_v6_unique_local(ip: Ipv6Addr) -> bool { + (ip.segments()[0] & 0xFE00) == 0xFC00 // fc00::/7 +} + +fn is_v6_link_local(ip: Ipv6Addr) -> bool { + (ip.segments()[0] & 0xFFC0) == 0xFE80 // fe80::/10 +} + +/// Check if an IPv6 address is a v4-mapped address (::ffff:x.x.x.x) +/// pointing to a blocked IPv4 range. +fn is_v4_mapped_blocked(ip: Ipv6Addr) -> bool { + if let Some(v4) = ip.to_ipv4_mapped() { + is_blocked_ip(IpAddr::V4(v4)) + } else { + false + } +} + /// Tool for browser automation (worker-only). #[derive(Debug, Clone)] pub struct BrowserTool { @@ -412,6 +512,8 @@ impl BrowserTool { return Err(BrowserError::new("url is required for navigate action")); }; + validate_url(&url)?; + let mut state = self.state.lock().await; let page = self.get_or_create_page(&mut state, Some(&url)).await?; @@ -441,6 +543,10 @@ impl BrowserTool { let target_url = url.as_deref().unwrap_or("about:blank"); + if target_url != "about:blank" { + validate_url(target_url)?; + } + let page = browser .new_page(target_url) .await diff --git a/src/tools/cron.rs b/src/tools/cron.rs index 7122b173e..5e4729758 100644 --- a/src/tools/cron.rs +++ b/src/tools/cron.rs @@ -8,6 +8,12 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::sync::Arc; +/// Minimum allowed interval between cron job runs (seconds). +const MIN_CRON_INTERVAL_SECS: u64 = 60; + +/// Maximum allowed prompt length for cron jobs (characters). +const MAX_CRON_PROMPT_LENGTH: usize = 10_000; + /// Tool for managing cron jobs (scheduled recurring tasks). #[derive(Debug, Clone)] pub struct CronTool { @@ -168,8 +174,48 @@ impl CronTool { .delivery_target .ok_or_else(|| CronError("'delivery_target' is required for create".into()))?; + // Validate cron job ID: alphanumeric, hyphens, underscores only + if id.is_empty() + || id.len() > 50 + || !id + .chars() + .all(|c| c.is_alphanumeric() || c == '-' || c == '_') + { + return Err(CronError( + "'id' must be 1-50 characters, alphanumeric with hyphens and underscores only".into(), + )); + } + + // Prevent excessively short intervals that could cause resource exhaustion + if interval_secs < MIN_CRON_INTERVAL_SECS { + return Err(CronError(format!( + "'interval_secs' must be at least {MIN_CRON_INTERVAL_SECS} (got {interval_secs})" + ))); + } + + // Cap prompt length to prevent context flooding + if prompt.len() > MAX_CRON_PROMPT_LENGTH { + return Err(CronError(format!( + "'prompt' exceeds maximum length of {MAX_CRON_PROMPT_LENGTH} characters (got {})", + prompt.len() + ))); + } + + // Validate delivery_target format (must be "adapter:target") + if !delivery_target.contains(':') { + return Err(CronError( + "'delivery_target' must be in 'adapter:target' format (e.g. 'discord:123456789')" + .into(), + )); + } + let active_hours = match (args.active_start_hour, args.active_end_hour) { - (Some(start), Some(end)) => Some((start, end)), + (Some(start), Some(end)) => { + if start > 23 || end > 23 { + return Err(CronError("active hours must be 0-23".into())); + } + Some((start, end)) + } _ => None, }; let run_once = args.run_once.unwrap_or(false); diff --git a/src/tools/shell.rs b/src/tools/shell.rs index 0abde4b0b..7f6628231 100644 --- a/src/tools/shell.rs +++ b/src/tools/shell.rs @@ -128,14 +128,53 @@ impl ShellTool { } } - // Block /proc/self/environ which exposes all env vars on Linux - if command.contains("/proc/self/environ") || command.contains("/proc/*/environ") { + // Block subshell/command substitution that could bypass string-level checks. + // Backtick and $() let an attacker compose a blocked command dynamically. + if command.contains('`') + || command.contains("$(") + || command.contains("<(") + || command.contains(">(") + { + return Err(ShellError { + message: "Subshell and command substitution (`...`, $(...), <(...), >(...)) \ + are not allowed." + .to_string(), + exit_code: -1, + }); + } + + // Block eval/exec which can dynamically construct any command + if contains_shell_builtin(command, "eval") || contains_shell_builtin(command, "exec") { + return Err(ShellError { + message: "eval and exec are not allowed.".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") + || command.contains("/dev/fd/") + || command.contains("/dev/stdin") + { return Err(ShellError { message: "Cannot access process environment — it may contain secrets.".to_string(), exit_code: -1, }); } + // Block additional commands that dump environment state + if contains_shell_builtin(command, "set") + || command.contains("declare -p") + || contains_shell_builtin(command, "compgen") + || contains_shell_builtin(command, "export") + { + return Err(ShellError { + message: "Cannot dump shell state — it may contain secrets.".to_string(), + exit_code: -1, + }); + } + Ok(()) } } @@ -323,6 +362,21 @@ fn format_shell_output(exit_code: i32, stdout: &str, stderr: &str) -> String { output } +/// Check if a shell builtin appears as a standalone command +/// (not as a substring of another word). +fn contains_shell_builtin(command: &str, builtin: &str) -> bool { + for segment in command.split(['|', ';', '&']) { + let trimmed = segment.trim(); + if trimmed == builtin + || trimmed.starts_with(&format!("{builtin} ")) + || trimmed.starts_with(&format!("{builtin}\t")) + { + return true; + } + } + false +} + /// System-internal shell execution that bypasses path restrictions. /// Used by the system itself, not LLM-facing. pub async fn shell(