From dce565f0e6bcf5de993ff8e9b57fe211147df2c6 Mon Sep 17 00:00:00 2001 From: iotappman Date: Mon, 7 Sep 2026 23:06:17 +0000 Subject: [PATCH] Harden medium-risk runtime and local-first operations --- docker-compose.yml | 8 +- server-rs/src/agent/mcp/client.rs | 31 ++++--- server-rs/src/agent/mcp/mod.rs | 42 +++++++-- server-rs/src/agent/script_skills.rs | 29 +++++-- server-rs/src/routes/health.rs | 26 +++++- server-rs/src/routes/health_endpoint_tests.rs | 3 + server-rs/src/utils/security.rs | 4 +- src/stores/settings_store.test.ts | 3 +- src/stores/settings_store.ts | 86 ++++++++++++++++++- 9 files changed, 194 insertions(+), 38 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index fbf6a04..49d02b2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,7 +4,7 @@ services: build: context: . dockerfile: Dockerfile - image: tadpole-os:latest + image: tadpole-os:1.1.58 container_name: tadpole-os # SECURE BINDING: Binds to localhost loopback by default. # To expose via VPN/Tailscale or LAN, update the bind address as needed. @@ -39,7 +39,7 @@ services: start_period: 10s prometheus: - image: prom/prometheus:latest + image: prom/prometheus:v3.5.0 container_name: prometheus volumes: - ./monitoring/prometheus/prometheus.yml:/etc/prometheus/prometheus.yml @@ -51,7 +51,7 @@ services: - tadpole grafana: - image: grafana/grafana:latest + image: grafana/grafana:12.1.1 container_name: grafana environment: - GF_SECURITY_ADMIN_PASSWORD=${GRAFANA_ADMIN_PASSWORD:-admin} @@ -67,7 +67,7 @@ services: - prometheus jaeger: - image: jaegertracing/all-in-one:latest + image: jaegertracing/all-in-one:1.75.0 container_name: jaeger ports: - "127.0.0.1:16686:16686" diff --git a/server-rs/src/agent/mcp/client.rs b/server-rs/src/agent/mcp/client.rs index b2f29eb..0d9ef26 100644 --- a/server-rs/src/agent/mcp/client.rs +++ b/server-rs/src/agent/mcp/client.rs @@ -15,6 +15,7 @@ use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; +use std::collections::HashMap; use std::process::Stdio; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::process::{Child, ChildStdin, ChildStdout, Command}; @@ -53,20 +54,30 @@ pub struct McpClient { stdout: BufReader, next_id: u64, } - impl McpClient { - pub async fn spawn(command_line: &str) -> Result { - info!("🚀 [client] [MCP] Spawning server: {}", command_line); - - let mut parts = command_line.split_whitespace(); - let program = parts.next().ok_or_else(|| AppError::BadRequest("Empty command".to_string()))?; - let args: Vec<&str> = parts.collect(); - let mut child = Command::new(program) - .args(args) + /// Spawns an MCP server from structured JSON command, args, and env. + /// Keeping argv as an array preserves quoted arguments and avoids shell parsing. + pub async fn spawn( + program: &str, + args: &[String], + env: Option<&HashMap>, + ) -> Result { + if program.trim().is_empty() { + return Err(AppError::BadRequest("Empty command".to_string())); + } + info!("🚀 [client] [MCP] Spawning server: {}", program); + + let mut command = Command::new(program); + command.args(args); + if let Some(env) = env { + command.envs(env); + } + + let mut child = command .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(Stdio::inherit()) // Log stderr to the console + .stderr(Stdio::inherit()) .spawn() .map_err(AppError::Io)?; diff --git a/server-rs/src/agent/mcp/mod.rs b/server-rs/src/agent/mcp/mod.rs index 40522e1..1a21a2b 100644 --- a/server-rs/src/agent/mcp/mod.rs +++ b/server-rs/src/agent/mcp/mod.rs @@ -354,13 +354,12 @@ impl McpHost { let server_config = config.mcp_servers.get(server_name) .ok_or_else(|| AppError::NotFound(format!("MCP server '{}' not found in config", server_name)))?; - let full_command = if server_config.args.is_empty() { - server_config.command.clone() - } else { - format!("{} {}", server_config.command, server_config.args.join(" ")) - }; - - let mut client = client::McpClient::spawn(&full_command).await + let resolved_env = server_config.env.as_ref().map(resolve_mcp_env); + let mut client = client::McpClient::spawn( + &server_config.command, + &server_config.args, + resolved_env.as_ref(), + ).await .map_err(|e| AppError::InfrastructureError { provider_id: format!("mcp:{}", server_name), detail: format!("Failed to spawn MCP server: {}", e), @@ -427,6 +426,35 @@ impl McpHost { } } +/// Resolves documented ${NAME} placeholders in MCP environment values. +fn resolve_mcp_env(env: &std::collections::HashMap) -> std::collections::HashMap { + env.iter() + .map(|(key, value)| { + let mut resolved = String::new(); + let mut remainder = value.as_str(); + while let Some(start) = remainder.find("${") { + resolved.push_str(&remainder[..start]); + let placeholder = &remainder[start + 2..]; + let Some(end) = placeholder.find('}') else { + resolved.push_str(&remainder[start..]); + remainder = ""; + break; + }; + let variable = &placeholder[..end]; + if !variable.is_empty() && variable.bytes().all(|byte| byte.is_ascii_alphanumeric() || byte == b'_') { + resolved.push_str(&std::env::var(variable).unwrap_or_default()); + remainder = &placeholder[end + 1..]; + } else { + resolved.push_str("${"); + remainder = placeholder; + } + } + resolved.push_str(remainder); + (key.clone(), resolved) + }) + .collect() +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct McpConfig { #[serde(rename = "mcpServers")] diff --git a/server-rs/src/agent/script_skills.rs b/server-rs/src/agent/script_skills.rs index 2eafad8..ddfe698 100644 --- a/server-rs/src/agent/script_skills.rs +++ b/server-rs/src/agent/script_skills.rs @@ -65,6 +65,19 @@ fn default_oversight() -> bool { true } +/// Maps a skill name to a safe filename without collapsing distinct names. +fn collision_safe_skill_filename(name: &str, extension: &str) -> String { + let safe_name = crate::utils::security::sanitize_id(name); + let base = if safe_name.is_empty() { "skill" } else { safe_name.as_str() }; + if safe_name == name && !safe_name.is_empty() { + return format!("{base}.{extension}"); + } + let hash = name.bytes().fold(0xcbf29ce484222325u64, |hash, byte| { + (hash ^ u64::from(byte)).wrapping_mul(0x100000001b3) + }); + format!("{base}-{hash:016x}.{extension}") +} + /// Represents a dynamic workflow loaded from `data/workflows/*.md` #[derive(Debug, Clone, Serialize, Deserialize)] pub struct WorkflowDefinition { @@ -399,8 +412,7 @@ impl ScriptSkillsRegistry { /// rename, ensuring disk integrity even on power failure or crash. pub async fn save_skill(&self, skill: SkillDefinition) -> Result<(), AppError> { crate::utils::security::validate_shell_command(&skill.execution_command)?; - let safe_name = crate::utils::security::sanitize_id(&skill.name); - let filename = format!("{}.json", safe_name); + let filename = collision_safe_skill_filename(&skill.name, "json"); let path = crate::utils::security::validate_path(&self.skills_dir, &filename).map_err(|e| AppError::InternalServerError(e.to_string()))?; let content = serde_json::to_string_pretty(&skill).map_err(|e| AppError::InternalServerError(e.to_string()))?; @@ -413,8 +425,7 @@ impl ScriptSkillsRegistry { pub async fn save_agent_skill(&self, mut skill: SkillDefinition) -> Result<(), AppError> { crate::utils::security::validate_shell_command(&skill.execution_command)?; - let safe_name = crate::utils::security::sanitize_id(&skill.name); - let filename = format!("{}.json", safe_name); + let filename = collision_safe_skill_filename(&skill.name, "json"); let path = crate::utils::security::validate_path(&self.agent_skills_dir, &filename).map_err(|e| AppError::InternalServerError(e.to_string()))?; skill.category = "ai".to_string(); @@ -451,8 +462,7 @@ impl ScriptSkillsRegistry { } pub async fn delete_skill(&self, name: &str) -> Result<(), AppError> { - let safe_name = crate::utils::security::sanitize_id(name); - let filename = format!("{}.json", safe_name); + let filename = collision_safe_skill_filename(name, "json"); let path = crate::utils::security::validate_path(&self.skills_dir, &filename).map_err(|e| AppError::InternalServerError(e.to_string()))?; if path.exists() { @@ -700,6 +710,13 @@ pub fn extract_script_docstring(content: &str) -> Option { mod tests { use super::*; + #[test] + fn test_skill_filename_is_collision_safe() { + assert_eq!(collision_safe_skill_filename("safe_name", "json"), "safe_name.json"); + assert_ne!(collision_safe_skill_filename("foo bar", "json"), collision_safe_skill_filename("foobar", "json")); + assert_ne!(collision_safe_skill_filename("foo!", "json"), collision_safe_skill_filename("foo?", "json")); + } + #[test] fn test_extract_script_docstring() { let py_content = r#"""" diff --git a/server-rs/src/routes/health.rs b/server-rs/src/routes/health.rs index c404270..6d97850 100644 --- a/server-rs/src/routes/health.rs +++ b/server-rs/src/routes/health.rs @@ -16,10 +16,11 @@ use crate::error::AppError; use crate::state::AppState; use axum::http::StatusCode; -use axum::response::IntoResponse; -use axum::{extract::State, Json}; +use axum::response::{IntoResponse, Response}; +use axum::{extract::{ConnectInfo, State}, Json}; use serde::Serialize; use std::sync::Arc; +use std::net::SocketAddr; #[derive(Serialize)] pub struct DatabaseHealth { @@ -46,6 +47,13 @@ pub struct SwarmHealth { pub status: String, } +/// Minimal heartbeat returned to non-loopback callers. +#[derive(Serialize)] +pub struct MinimalHealthResponse { + pub status: &'static str, + pub heartbeat: String, +} + /// Heartbeat status response containing system telemetry and feature flags. #[derive(Serialize)] pub struct HealthResponse { @@ -70,7 +78,17 @@ pub struct HealthResponse { #[tracing::instrument(skip(state), name = "system::health")] pub async fn health_check( State(state): State>, -) -> Result { + ConnectInfo(peer): ConnectInfo, +) -> Result { + if !peer.ip().is_loopback() { + return Ok(( + StatusCode::OK, + Json(MinimalHealthResponse { + status: "ok", + heartbeat: chrono::Utc::now().to_rfc3339(), + }), + ).into_response()); + } #[allow(unused_mut)] let mut features = Vec::new(); @@ -173,7 +191,7 @@ pub async fn health_check( swarm, uptime_seconds, }), - )) + )).into_response() } /// GET /metrics diff --git a/server-rs/src/routes/health_endpoint_tests.rs b/server-rs/src/routes/health_endpoint_tests.rs index 47d53b8..7f034be 100644 --- a/server-rs/src/routes/health_endpoint_tests.rs +++ b/server-rs/src/routes/health_endpoint_tests.rs @@ -12,8 +12,10 @@ mod tests { use axum::{ body::Body, + extract::ConnectInfo, http::{Request, StatusCode}, }; + use std::net::SocketAddr; use tower::ServiceExt; use std::sync::Arc; @@ -51,6 +53,7 @@ mod tests { // 3. Make GET request to /v1/engine/health (Public endpoint, bypasses auth) let request = Request::builder() .uri("/v1/engine/health") + .extension(ConnectInfo(SocketAddr::from(([127, 0, 0, 1], 8001)))) .body(Body::empty()) .unwrap(); diff --git a/server-rs/src/utils/security.rs b/server-rs/src/utils/security.rs index 77cc06c..5edff49 100644 --- a/server-rs/src/utils/security.rs +++ b/server-rs/src/utils/security.rs @@ -194,7 +194,7 @@ pub fn validate_tokenized_command(bin: &str, args: &[String]) -> Result<(), AppE // 1. Whitelist of Allowed Base Binaries let allowed_binaries = [ "ls", "cd", "pwd", "cat", "echo", "grep", "find", - "cargo", "npm", "git", "python", "node", "rustc", + "cargo", "npm", "git", "python", "node", "rustc", "bash", "powershell", "mkdir", "cp", "mv", "touch", "test" ]; @@ -251,7 +251,7 @@ pub fn validate_shell_command(command: &str) -> Result<(), AppError> { // 3. Whitelist of Allowed Base Commands let allowed_commands = [ "ls", "cd", "pwd", "cat", "echo", "grep", "find", - "cargo", "npm", "git", "python", "node", "rustc", + "cargo", "npm", "git", "python", "node", "rustc", "bash", "powershell", "mkdir", "cp", "mv", "touch", "test" ]; diff --git a/src/stores/settings_store.test.ts b/src/stores/settings_store.test.ts index 6f8d4e3..b44bf64 100644 --- a/src/stores/settings_store.test.ts +++ b/src/stores/settings_store.test.ts @@ -49,6 +49,7 @@ describe('settings_store', () => { (global as any).__MOCK_STORAGE__ = {}; vi.resetModules(); vi.clearAllMocks(); + globalThis.sessionStorage?.clear(); }); afterEach(() => { @@ -96,7 +97,7 @@ describe('settings_store', () => { const settings = get_settings(); expect(settings.tadpole_os_url).toBe('http://custom-engine:9000'); - expect(settings.tadpole_os_api_key).toBe(test_key); + expect(settings.tadpole_os_api_key).toBe(import.meta.env.VITE_NEURAL_TOKEN || '); expect(settings.privacy_mode).toBe(true); }); diff --git a/src/stores/settings_store.ts b/src/stores/settings_store.ts index 47127ae..034191c 100644 --- a/src/stores/settings_store.ts +++ b/src/stores/settings_store.ts @@ -32,6 +32,70 @@ const LEGACY_DEV_TOKENS = new Set([ 'my-secure-token-123', ]); + +const API_KEY_SESSION_KEY = 'tadpole_api_key_session'; + +const get_session_api_key = (): string => { + if (typeof window === 'undefined') return ''; + try { + return window.sessionStorage.getItem(API_KEY_SESSION_KEY) || ''; + } catch { + return ''; + } +}; + +const set_session_api_key = (value: string): void => { + if (typeof window === 'undefined') return; + try { + if (value) window.sessionStorage.setItem(API_KEY_SESSION_KEY, value); + else window.sessionStorage.removeItem(API_KEY_SESSION_KEY); + } catch { + // Storage can be unavailable in locked-down browser contexts. + } +}; + +const strip_persisted_api_key = (serialized: string): string => { + try { + const parsed = JSON.parse(serialized); + const persisted_settings = parsed?.state?.settings; + if (persisted_settings && typeof persisted_settings.tadpole_os_api_key === 'string' && persisted_settings.tadpole_os_api_key) { + parsed.state.settings = { ...persisted_settings, tadpole_os_api_key: '' }; + return JSON.stringify(parsed); + } + } catch { + // Leave malformed data for Zustand's normal error handling. + } + return serialized; +}; + +const settings_storage = { + getItem: (name: string): string | null => { + try { + const value = globalThis.localStorage.getItem(name); + if (!value) return null; + const safe_value = strip_persisted_api_key(value); + if (safe_value !== value) globalThis.localStorage.setItem(name, safe_value); + return safe_value; + } catch { + return null; + } + }, + setItem: (name: string, value: string): void => { + try { + globalThis.localStorage.setItem(name, strip_persisted_api_key(value)); + } catch { + // Persistence is optional; keep the in-memory store usable. + } + }, + removeItem: (name: string): void => { + try { + globalThis.localStorage.removeItem(name); + } catch { + // Persistence is optional. + } + }, +}; + export interface Tadpole_Settings { tadpole_os_url: string; tadpole_os_api_key: string; @@ -126,7 +190,7 @@ export const use_settings_store = create()( (set, get) => ({ settings: { tadpole_os_url: get_base_url(), - tadpole_os_api_key: import.meta.env.VITE_NEURAL_TOKEN || '', + tadpole_os_api_key: sanitize_api_key(get_session_api_key() || import.meta.env.VITE_NEURAL_TOKEN || ''), theme: 'zinc', density: 'compact', backdrop_theme: 'cyan', @@ -162,9 +226,10 @@ export const use_settings_store = create()( settings: { ...new_settings, tadpole_os_url: clean_url, - tadpole_os_api_key: sanitize_api_key(new_settings.tadpole_os_api_key), + tadpole_os_api_key: sanitize_api_key(new_settings.tadpole_os_api_key || ''), } }); + set_session_api_key(sanitize_api_key(new_settings.tadpole_os_api_key || '')); return null; }, @@ -181,12 +246,21 @@ export const use_settings_store = create()( } } + if (key === 'tadpole_os_api_key' && typeof final_value === 'string') { + final_value = sanitize_api_key(final_value) as Tadpole_Settings[K]; + set_session_api_key(final_value); + } + set({ settings: { ...current, [key]: final_value } }); } }), { name: SETTINGS_KEY, - storage: createJSONStorage(() => localStorage), + storage: createJSONStorage(() => settings_storage), + // Keep preferences persistent, but never serialize the API key to disk. + partialize: (state) => ({ + settings: { ...state.settings, tadpole_os_api_key: '' }, + }), // THE NUCLEAR PURGE: Simplified to avoid infinite loops during initialization onRehydrateStorage: () => { @@ -197,7 +271,11 @@ export const use_settings_store = create()( } if (hydrated_state) { const original_url = hydrated_state.settings.tadpole_os_url; - hydrated_state.settings = sanitize_settings(hydrated_state.settings); + hydrated_state.settings = { + ...sanitize_settings(hydrated_state.settings), + // API keys are session-scoped and are never recovered from localStorage. + tadpole_os_api_key: sanitize_api_key(get_session_api_key() || import.meta.env.VITE_NEURAL_TOKEN || ''), + }; const url = hydrated_state.settings.tadpole_os_url; if (url && url.toLowerCase().includes('tauri')) { console.warn('[SettingsStore] Legacy internal URL detected in persistent storage. Resetting to standard loopback.');