From 69abd217d70830b72d16e9020bc81eb3f3da51ff Mon Sep 17 00:00:00 2001 From: Giulio V Date: Wed, 11 Mar 2026 11:30:56 +0100 Subject: [PATCH 1/9] style: fix cargo fmt violations in config module The TTS providers merge (#2994) introduced import ordering and line-wrapping that doesn't match rustfmt output, causing lint failures on all open PRs targeting master. --- src/config/mod.rs | 12 ++++++------ src/config/schema.rs | 24 ++++-------------------- 2 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/config/mod.rs b/src/config/mod.rs index 9884b06f2..afb4b15ac 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -7,17 +7,17 @@ pub use schema::{ build_runtime_proxy_client_with_timeouts, runtime_proxy_config, set_runtime_proxy_config, AgentConfig, AuditConfig, AutonomyConfig, BrowserComputerUseConfig, BrowserConfig, BuiltinHooksConfig, ChannelsConfig, ClassificationRule, ComposioConfig, Config, CostConfig, - CronConfig, DelegateAgentConfig, DiscordConfig, DockerRuntimeConfig, EmbeddingRouteConfig, - EstopConfig, FeishuConfig, GatewayConfig, HardwareConfig, HardwareTransport, HeartbeatConfig, - HooksConfig, HttpRequestConfig, IMessageConfig, IdentityConfig, LarkConfig, MatrixConfig, - MemoryConfig, ModelRouteConfig, MultimodalConfig, NextcloudTalkConfig, ObservabilityConfig, + CronConfig, DelegateAgentConfig, DiscordConfig, DockerRuntimeConfig, EdgeTtsConfig, + ElevenLabsTtsConfig, EmbeddingRouteConfig, EstopConfig, FeishuConfig, GatewayConfig, + GoogleTtsConfig, HardwareConfig, HardwareTransport, HeartbeatConfig, HooksConfig, + HttpRequestConfig, IMessageConfig, IdentityConfig, LarkConfig, MatrixConfig, MemoryConfig, + ModelRouteConfig, MultimodalConfig, NextcloudTalkConfig, ObservabilityConfig, OpenAiTtsConfig, OtpConfig, OtpMethod, PeripheralBoardConfig, PeripheralsConfig, ProxyConfig, ProxyScope, QdrantConfig, QueryClassificationConfig, ReliabilityConfig, ResourceLimitsConfig, RuntimeConfig, SandboxBackend, SandboxConfig, SchedulerConfig, SecretsConfig, SecurityConfig, SkillsConfig, SkillsPromptInjectionMode, SlackConfig, StorageConfig, StorageProviderConfig, StorageProviderSection, StreamMode, TelegramConfig, TranscriptionConfig, TtsConfig, - EdgeTtsConfig, ElevenLabsTtsConfig, GoogleTtsConfig, OpenAiTtsConfig, TunnelConfig, - WebFetchConfig, WebSearchConfig, WebhookConfig, + TunnelConfig, WebFetchConfig, WebSearchConfig, WebhookConfig, }; pub fn name_and_presence(channel: Option<&T>) -> (&'static str, bool) { diff --git a/src/config/schema.rs b/src/config/schema.rs index c39008de5..b91716885 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -4263,11 +4263,7 @@ impl Config { // Decrypt TTS provider API keys if let Some(ref mut openai) = config.tts.openai { - decrypt_optional_secret( - &store, - &mut openai.api_key, - "config.tts.openai.api_key", - )?; + decrypt_optional_secret(&store, &mut openai.api_key, "config.tts.openai.api_key")?; } if let Some(ref mut elevenlabs) = config.tts.elevenlabs { decrypt_optional_secret( @@ -4277,11 +4273,7 @@ impl Config { )?; } if let Some(ref mut google) = config.tts.google { - decrypt_optional_secret( - &store, - &mut google.api_key, - "config.tts.google.api_key", - )?; + decrypt_optional_secret(&store, &mut google.api_key, "config.tts.google.api_key")?; } if let Some(ref mut ns) = config.channels_config.nostr { @@ -4921,11 +4913,7 @@ impl Config { // Encrypt TTS provider API keys if let Some(ref mut openai) = config_to_save.tts.openai { - encrypt_optional_secret( - &store, - &mut openai.api_key, - "config.tts.openai.api_key", - )?; + encrypt_optional_secret(&store, &mut openai.api_key, "config.tts.openai.api_key")?; } if let Some(ref mut elevenlabs) = config_to_save.tts.elevenlabs { encrypt_optional_secret( @@ -4935,11 +4923,7 @@ impl Config { )?; } if let Some(ref mut google) = config_to_save.tts.google { - encrypt_optional_secret( - &store, - &mut google.api_key, - "config.tts.google.api_key", - )?; + encrypt_optional_secret(&store, &mut google.api_key, "config.tts.google.api_key")?; } if let Some(ref mut ns) = config_to_save.channels_config.nostr { From ed7c191fcd2b7731dd6fc7d78a9209d5010d4c61 Mon Sep 17 00:00:00 2001 From: Giulio V Date: Wed, 11 Mar 2026 11:41:11 +0100 Subject: [PATCH 2/9] style: fix cargo fmt + clippy violations from TTS merge - config/mod.rs: reorder TTS imports alphabetically (rustfmt) - config/schema.rs: collapse single-arg encrypt/decrypt calls (rustfmt) - main.rs: Box::pin large onboard futures to fix clippy::large_futures These violations were introduced by the TTS providers merge (#2994) and are blocking CI lint on all open PRs targeting master. --- src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 2bcaf45b5..32c976ca4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -717,9 +717,9 @@ async fn main() -> Result<()> { bail!("--channels-only does not accept --force"); } let config = if channels_only { - onboard::run_channels_repair_wizard().await + Box::pin(onboard::run_channels_repair_wizard()).await } else if interactive { - onboard::run_wizard(force).await + Box::pin(onboard::run_wizard(force)).await } else { onboard::run_quick_setup( api_key.as_deref(), From a1c65558d8682964efeaa280b5d136d3bf804bc6 Mon Sep 17 00:00:00 2001 From: Simian Astronaut 7 Date: Wed, 11 Mar 2026 08:57:34 -0400 Subject: [PATCH 3/9] fix(gateway): restrict admin endpoints to localhost and replace process::exit with graceful shutdown Admin endpoints (/admin/shutdown, /admin/paircode, /admin/paircode/new) were completely unauthenticated, allowing any network client to shut down the gateway or read/generate pairing codes. Add require_localhost() guard that returns 403 for non-loopback IPs. Replace std::process::exit(0) in shutdown handler with a tokio watch channel for graceful shutdown, allowing proper destructor cleanup and connection draining. Replace the 500ms sleep race in the restart command with a poll loop that waits for the port to actually become free. Co-Authored-By: Claude Opus 4.6 --- src/gateway/mod.rs | 104 ++++++++++++++++++++++++++++++++++++--------- src/main.rs | 20 ++++++++- 2 files changed, 103 insertions(+), 21 deletions(-) diff --git a/src/gateway/mod.rs b/src/gateway/mod.rs index fcfff7281..d65af32dc 100644 --- a/src/gateway/mod.rs +++ b/src/gateway/mod.rs @@ -310,6 +310,8 @@ pub struct AppState { pub cost_tracker: Option>, /// SSE broadcast channel for real-time events pub event_tx: tokio::sync::broadcast::Sender, + /// Shutdown signal sender for graceful shutdown + pub shutdown_tx: tokio::sync::watch::Sender, } /// Run the HTTP gateway using axum with proper HTTP/1.1 compliance. @@ -622,6 +624,8 @@ pub async fn run_gateway(host: &str, port: u16, config: Config) -> Result<()> { event_tx.clone(), )); + let (shutdown_tx, mut shutdown_rx) = tokio::sync::watch::channel(false); + let state = AppState { config: config_state, provider, @@ -645,6 +649,7 @@ pub async fn run_gateway(host: &str, port: u16, config: Config) -> Result<()> { tools_registry, cost_tracker, event_tx, + shutdown_tx, }; // Config PUT needs larger body limit (1MB) @@ -704,11 +709,15 @@ pub async fn run_gateway(host: &str, port: u16, config: Config) -> Result<()> { // ā”€ā”€ SPA fallback: non-API GET requests serve index.html ā”€ā”€ .fallback(get(static_files::handle_spa_fallback)); - // Run the server + // Run the server with graceful shutdown axum::serve( listener, app.into_make_service_with_connect_info::(), ) + .with_graceful_shutdown(async move { + let _ = shutdown_rx.changed().await; + tracing::info!("šŸ¦€ ZeroClaw Gateway shutting down..."); + }) .await?; Ok(()) @@ -1533,29 +1542,44 @@ struct AdminResponse { message: String, } -/// POST /admin/shutdown ā€” graceful shutdown from CLI -async fn handle_admin_shutdown(State(_state): State) -> impl IntoResponse { +/// Reject requests that do not originate from a loopback address. +fn require_localhost(peer: &SocketAddr) -> Result<(), (StatusCode, Json)> { + if peer.ip().is_loopback() { + Ok(()) + } else { + Err(( + StatusCode::FORBIDDEN, + Json(serde_json::json!({ + "error": "Admin endpoints are restricted to localhost" + })), + )) + } +} + +/// POST /admin/shutdown ā€” graceful shutdown from CLI (localhost only) +async fn handle_admin_shutdown( + State(state): State, + ConnectInfo(peer): ConnectInfo, +) -> Result)> { + require_localhost(&peer)?; tracing::info!("šŸ”Œ Admin shutdown request received ā€” initiating graceful shutdown"); - // The server will shut down when this response is sent - // In a real implementation, we might use a shutdown channel let body = AdminResponse { success: true, message: "Gateway shutdown initiated".to_string(), }; - // Spawn a task to gracefully exit after responding - tokio::spawn(async move { - tokio::time::sleep(std::time::Duration::from_millis(100)).await; - tracing::info!("šŸ¦€ ZeroClaw Gateway shutting down..."); - std::process::exit(0); - }); + let _ = state.shutdown_tx.send(true); - (StatusCode::OK, Json(body)) + Ok((StatusCode::OK, Json(body))) } -/// GET /admin/paircode ā€” fetch current pairing code -async fn handle_admin_paircode(State(state): State) -> impl IntoResponse { +/// GET /admin/paircode ā€” fetch current pairing code (localhost only) +async fn handle_admin_paircode( + State(state): State, + ConnectInfo(peer): ConnectInfo, +) -> Result)> { + require_localhost(&peer)?; let code = state.pairing.pairing_code(); let body = if let Some(c) = code { @@ -1578,11 +1602,15 @@ async fn handle_admin_paircode(State(state): State) -> impl IntoRespon }) }; - (StatusCode::OK, Json(body)) + Ok((StatusCode::OK, Json(body))) } -/// POST /admin/paircode/new ā€” generate a new pairing code -async fn handle_admin_paircode_new(State(state): State) -> impl IntoResponse { +/// POST /admin/paircode/new ā€” generate a new pairing code (localhost only) +async fn handle_admin_paircode_new( + State(state): State, + ConnectInfo(peer): ConnectInfo, +) -> Result)> { + require_localhost(&peer)?; match state.pairing.generate_new_pairing_code() { Some(code) => { tracing::info!("šŸ” New pairing code generated via admin endpoint"); @@ -1592,7 +1620,7 @@ async fn handle_admin_paircode_new(State(state): State) -> impl IntoRe "pairing_code": code, "message": "New pairing code generated ā€” use this one-time code to pair" }); - (StatusCode::OK, Json(body)) + Ok((StatusCode::OK, Json(body))) } None => { let body = serde_json::json!({ @@ -1601,7 +1629,7 @@ async fn handle_admin_paircode_new(State(state): State) -> impl IntoRe "pairing_code": null, "message": "Pairing is disabled for this gateway" }); - (StatusCode::BAD_REQUEST, Json(body)) + Ok((StatusCode::BAD_REQUEST, Json(body))) } } } @@ -1688,6 +1716,7 @@ mod tests { tools_registry: Arc::new(Vec::new()), cost_tracker: None, event_tx: tokio::sync::broadcast::channel(16).0, + shutdown_tx: tokio::sync::watch::channel(false).0, }; let response = handle_metrics(State(state)).await.into_response(); @@ -1737,6 +1766,7 @@ mod tests { tools_registry: Arc::new(Vec::new()), cost_tracker: None, event_tx: tokio::sync::broadcast::channel(16).0, + shutdown_tx: tokio::sync::watch::channel(false).0, }; let response = handle_metrics(State(state)).await.into_response(); @@ -2103,6 +2133,7 @@ mod tests { tools_registry: Arc::new(Vec::new()), cost_tracker: None, event_tx: tokio::sync::broadcast::channel(16).0, + shutdown_tx: tokio::sync::watch::channel(false).0, }; let mut headers = HeaderMap::new(); @@ -2167,6 +2198,7 @@ mod tests { tools_registry: Arc::new(Vec::new()), cost_tracker: None, event_tx: tokio::sync::broadcast::channel(16).0, + shutdown_tx: tokio::sync::watch::channel(false).0, }; let headers = HeaderMap::new(); @@ -2243,6 +2275,7 @@ mod tests { tools_registry: Arc::new(Vec::new()), cost_tracker: None, event_tx: tokio::sync::broadcast::channel(16).0, + shutdown_tx: tokio::sync::watch::channel(false).0, }; let response = handle_webhook( @@ -2291,6 +2324,7 @@ mod tests { tools_registry: Arc::new(Vec::new()), cost_tracker: None, event_tx: tokio::sync::broadcast::channel(16).0, + shutdown_tx: tokio::sync::watch::channel(false).0, }; let mut headers = HeaderMap::new(); @@ -2344,6 +2378,7 @@ mod tests { tools_registry: Arc::new(Vec::new()), cost_tracker: None, event_tx: tokio::sync::broadcast::channel(16).0, + shutdown_tx: tokio::sync::watch::channel(false).0, }; let mut headers = HeaderMap::new(); @@ -2402,6 +2437,7 @@ mod tests { tools_registry: Arc::new(Vec::new()), cost_tracker: None, event_tx: tokio::sync::broadcast::channel(16).0, + shutdown_tx: tokio::sync::watch::channel(false).0, }; let response = handle_nextcloud_talk_webhook( @@ -2456,6 +2492,7 @@ mod tests { tools_registry: Arc::new(Vec::new()), cost_tracker: None, event_tx: tokio::sync::broadcast::channel(16).0, + shutdown_tx: tokio::sync::watch::channel(false).0, }; let mut headers = HeaderMap::new(); @@ -2862,4 +2899,33 @@ mod tests { // Should be allowed again assert!(limiter.allow("burst-ip")); } + + #[test] + fn require_localhost_accepts_ipv4_loopback() { + let peer = SocketAddr::from(([127, 0, 0, 1], 12345)); + assert!(require_localhost(&peer).is_ok()); + } + + #[test] + fn require_localhost_accepts_ipv6_loopback() { + let peer = SocketAddr::from((std::net::Ipv6Addr::LOCALHOST, 12345)); + assert!(require_localhost(&peer).is_ok()); + } + + #[test] + fn require_localhost_rejects_non_loopback_ipv4() { + let peer = SocketAddr::from(([192, 168, 1, 100], 12345)); + let err = require_localhost(&peer).unwrap_err(); + assert_eq!(err.0, StatusCode::FORBIDDEN); + } + + #[test] + fn require_localhost_rejects_non_loopback_ipv6() { + let peer = SocketAddr::from(( + std::net::Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1), + 12345, + )); + let err = require_localhost(&peer).unwrap_err(); + assert_eq!(err.0, StatusCode::FORBIDDEN); + } } diff --git a/src/main.rs b/src/main.rs index 2bcaf45b5..dc67c0f4b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -795,8 +795,24 @@ async fn main() -> Result<()> { match shutdown_gateway(&host, port).await { Ok(()) => { info!(" āœ“ Existing gateway on {addr} shut down gracefully"); - // Small delay to allow port to be released - tokio::time::sleep(tokio::time::Duration::from_millis(500)).await; + // Poll until the port is free (connection refused) or timeout + let deadline = + tokio::time::Instant::now() + tokio::time::Duration::from_secs(5); + loop { + match tokio::net::TcpStream::connect(&addr).await { + Err(_) => break, // port is free + Ok(_) if tokio::time::Instant::now() >= deadline => { + warn!( + " Timed out waiting for port {port} to be released" + ); + break; + } + Ok(_) => { + tokio::time::sleep(tokio::time::Duration::from_millis(50)) + .await; + } + } + } } Err(e) => { info!(" No existing gateway to shut down: {e}"); From f894bc414071b92275e23dec53751aab01101f08 Mon Sep 17 00:00:00 2001 From: Simian Astronaut 7 Date: Wed, 11 Mar 2026 08:58:05 -0400 Subject: [PATCH 4/9] fix(slack): only send bearer token on first redirect hop Slack private file fetching was sending the bot token on every redirect hop. Since Slack CDN redirects use pre-signed URLs, sending the bearer token to CDN hosts is unnecessary credential exposure. Co-Authored-By: Claude Opus 4.6 --- src/channels/slack.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/channels/slack.rs b/src/channels/slack.rs index aae540652..57f80b274 100644 --- a/src/channels/slack.rs +++ b/src/channels/slack.rs @@ -736,12 +736,11 @@ impl SlackChannel { for redirect_hop in 0..=SLACK_MEDIA_REDIRECT_MAX_HOPS { let redacted_current = Self::redact_slack_url(¤t_url); - let response = match client - .get(current_url.clone()) - .bearer_auth(&self.bot_token) - .send() - .await - { + let mut req = client.get(current_url.clone()); + if redirect_hop == 0 { + req = req.bearer_auth(&self.bot_token); + } + let response = match req.send().await { Ok(response) => response, Err(err) => { tracing::warn!("Slack file fetch failed for {}: {}", redacted_current, err); From 9a6de3b20461932e5bf9a81a387c148893f69800 Mon Sep 17 00:00:00 2001 From: Simian Astronaut 7 Date: Wed, 11 Mar 2026 08:58:13 -0400 Subject: [PATCH 5/9] fix(tts): move Google API key to header and sanitize provider inputs Google TTS was passing the API key as a URL query parameter, which can appear in logs and proxy access records. Move it to the x-goog-api-key header instead. Add input validation for ElevenLabs voice IDs (reject non-alphanumeric/dash/underscore characters) and restrict Edge TTS binary_path to allowed basenames (edge-tts, edge-playback). Co-Authored-By: Claude Opus 4.6 --- src/channels/tts.rs | 50 +++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/src/channels/tts.rs b/src/channels/tts.rs index faa810e96..16b11cf1c 100644 --- a/src/channels/tts.rs +++ b/src/channels/tts.rs @@ -181,6 +181,12 @@ impl TtsProvider for ElevenLabsTtsProvider { } async fn synthesize(&self, text: &str, voice: &str) -> Result> { + if !voice + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') + { + bail!("ElevenLabs voice ID contains invalid characters: {voice}"); + } let url = format!("https://api.elevenlabs.io/v1/text-to-speech/{voice}"); let body = serde_json::json!({ "text": text, @@ -280,10 +286,7 @@ impl TtsProvider for GoogleTtsProvider { } async fn synthesize(&self, text: &str, voice: &str) -> Result> { - let url = format!( - "https://texttospeech.googleapis.com/v1/text:synthesize?key={}", - self.api_key - ); + let url = "https://texttospeech.googleapis.com/v1/text:synthesize"; let body = serde_json::json!({ "input": { "text": text }, "voice": { @@ -297,7 +300,8 @@ impl TtsProvider for GoogleTtsProvider { let resp = self .client - .post(&url) + .post(url) + .header("x-goog-api-key", &self.api_key) .json(&body) .send() .await @@ -356,11 +360,25 @@ pub struct EdgeTtsProvider { } impl EdgeTtsProvider { + /// Allowed basenames for the Edge TTS binary. + const ALLOWED_BINARIES: &[&str] = &["edge-tts", "edge-playback"]; + /// Create a new Edge TTS provider from config. - pub fn new(config: &crate::config::EdgeTtsConfig) -> Self { - Self { - binary_path: config.binary_path.clone(), + pub fn new(config: &crate::config::EdgeTtsConfig) -> Result { + let basename = std::path::Path::new(&config.binary_path) + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or(""); + if !Self::ALLOWED_BINARIES.contains(&basename) { + bail!( + "Edge TTS binary_path must use one of {:?}, got: {}", + Self::ALLOWED_BINARIES, + config.binary_path + ); } + Ok(Self { + binary_path: config.binary_path.clone(), + }) } } @@ -472,8 +490,14 @@ impl TtsManager { } if let Some(ref edge_cfg) = config.edge { - let p = EdgeTtsProvider::new(edge_cfg); - providers.insert("edge".to_string(), Box::new(p)); + match EdgeTtsProvider::new(edge_cfg) { + Ok(p) => { + providers.insert("edge".to_string(), Box::new(p)); + } + Err(e) => { + tracing::warn!("Skipping Edge TTS provider: {e}"); + } + } } let max_text_length = if config.max_text_length == 0 { @@ -557,7 +581,7 @@ mod tests { let mut config = default_tts_config(); config.default_provider = "edge".to_string(); config.edge = Some(crate::config::EdgeTtsConfig { - binary_path: "edge-tts".to_string(), + binary_path: "edge-tts".into(), }); let manager = TtsManager::new(&config).unwrap(); @@ -569,7 +593,7 @@ mod tests { let mut config = default_tts_config(); config.default_provider = "edge".to_string(); config.edge = Some(crate::config::EdgeTtsConfig { - binary_path: "edge-tts".to_string(), + binary_path: "edge-tts".into(), }); let manager = TtsManager::new(&config).unwrap(); @@ -589,7 +613,7 @@ mod tests { config.default_provider = "edge".to_string(); config.max_text_length = 10; config.edge = Some(crate::config::EdgeTtsConfig { - binary_path: "edge-tts".to_string(), + binary_path: "edge-tts".into(), }); let manager = TtsManager::new(&config).unwrap(); From e6bfb1ebee0b6fe963c650d9a313cbfc7fedff4d Mon Sep 17 00:00:00 2001 From: Simian Astronaut 7 Date: Wed, 11 Mar 2026 08:58:20 -0400 Subject: [PATCH 6/9] fix(slack): add HTTP timeouts, proper jitter, and cache bounds HTTP clients had no timeouts and could hang forever; add 30s total and 10s connect timeouts. Replace clock-nanos-based jitter with rand::random for proper randomness. Add a 1000-entry cap to the user display name cache with expired-entry pruning. Fix truncate_text to avoid scanning the full string twice when checking for truncation. Co-Authored-By: Claude Opus 4.6 --- src/channels/slack.rs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/channels/slack.rs b/src/channels/slack.rs index 57f80b274..c8a669573 100644 --- a/src/channels/slack.rs +++ b/src/channels/slack.rs @@ -44,6 +44,7 @@ const SLACK_ATTACHMENT_IMAGE_INLINE_FALLBACK_MAX_BYTES: usize = 512 * 1024; const SLACK_ATTACHMENT_TEXT_DOWNLOAD_MAX_BYTES: usize = 256 * 1024; const SLACK_ATTACHMENT_TEXT_INLINE_MAX_CHARS: usize = 12_000; const SLACK_ATTACHMENT_FILENAME_MAX_CHARS: usize = 128; +const SLACK_USER_CACHE_MAX_ENTRIES: usize = 1000; const SLACK_ATTACHMENT_SAVE_SUBDIR: &str = "slack_files"; const SLACK_ATTACHMENT_MAX_FILES_PER_MESSAGE: usize = 8; const SLACK_ATTACHMENT_RENDER_CONCURRENCY: usize = 3; @@ -98,7 +99,7 @@ impl SlackChannel { } fn http_client(&self) -> reqwest::Client { - crate::config::build_runtime_proxy_client("channel.slack") + crate::config::build_runtime_proxy_client_with_timeouts("channel.slack", 30, 10) } /// Check if a Slack user ID is in the allowlist. @@ -255,6 +256,10 @@ impl SlackChannel { let Ok(mut cache) = self.user_display_name_cache.lock() else { return; }; + if cache.len() >= SLACK_USER_CACHE_MAX_ENTRIES { + let now = Instant::now(); + cache.retain(|_, v| v.expires_at > now); + } cache.insert( user_id.to_string(), CachedSlackDisplayName { @@ -714,7 +719,10 @@ impl SlackChannel { fn slack_media_http_client_no_redirect(&self) -> anyhow::Result { let builder = crate::config::apply_runtime_proxy_to_builder( - reqwest::Client::builder().redirect(reqwest::redirect::Policy::none()), + reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .timeout(Duration::from_secs(30)) + .connect_timeout(Duration::from_secs(10)), "channel.slack", ); builder @@ -1214,11 +1222,12 @@ impl SlackChannel { out.push(ch); count += 1; } + let was_truncated = count >= max_chars && value.chars().nth(max_chars).is_some(); let mut out = out.trim().to_string(); if out.is_empty() { return None; } - if value.chars().count() > count { + if was_truncated { out.push_str("\nā€¦[truncated]"); } Some(out) @@ -1793,15 +1802,11 @@ impl SlackChannel { truncated.parse::().ok() } - fn jitter_ms_from_clock(max_jitter_ms: u64) -> u64 { + fn jitter_ms(max_jitter_ms: u64) -> u64 { if max_jitter_ms == 0 { return 0; } - let nanos = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|d| u64::from(d.subsec_nanos())) - .unwrap_or(0); - nanos % (max_jitter_ms + 1) + rand::random::() % (max_jitter_ms + 1) } fn compute_exponential_backoff_delay( @@ -1827,7 +1832,7 @@ impl SlackChannel { } fn compute_socket_mode_retry_delay(attempt: u32) -> Duration { - let jitter_ms = Self::jitter_ms_from_clock(SLACK_SOCKET_MODE_MAX_JITTER_MS); + let jitter_ms = Self::jitter_ms(SLACK_SOCKET_MODE_MAX_JITTER_MS); Self::compute_exponential_backoff_delay( SLACK_SOCKET_MODE_INITIAL_BACKOFF_SECS, attempt, @@ -1916,7 +1921,7 @@ impl SlackChannel { let retry_after_secs = Self::parse_retry_after_secs(&headers) .unwrap_or(SLACK_HISTORY_DEFAULT_RETRY_AFTER_SECS); - let jitter_ms = Self::jitter_ms_from_clock(SLACK_HISTORY_MAX_JITTER_MS); + let jitter_ms = Self::jitter_ms(SLACK_HISTORY_MAX_JITTER_MS); let wait = Self::compute_retry_delay(retry_after_secs, attempt, jitter_ms); total_wait += wait; let next_retry_at = Self::next_retry_timestamp(wait); From 162a2b65a5c784808565f0811e6584ffa6131a12 Mon Sep 17 00:00:00 2001 From: Simian Astronaut 7 Date: Wed, 11 Mar 2026 10:05:52 -0400 Subject: [PATCH 7/9] fix(multi): harden Edge TTS path validation, add subprocess timeout, restore Slack symlink protection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Edge TTS: reject binary_path containing path separators to prevent arbitrary executable paths bypassing the basename allowlist. Wrap subprocess execution in tokio::time::timeout (60s) matching HTTP providers. Slack: restore symlink_metadata check before rename to prevent symlink-following attacks on attachment output paths. Docs: add TTS module misplacement note to refactor-candidates.md ā€” tts.rs belongs in src/tools/, not src/channels/. Co-Authored-By: Claude Opus 4.6 --- docs/maintainers/refactor-candidates.md | 4 +++ src/channels/slack.rs | 15 +++++++++ src/channels/tts.rs | 43 +++++++++++++++---------- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/docs/maintainers/refactor-candidates.md b/docs/maintainers/refactor-candidates.md index 4e19321e8..fb1ce3630 100644 --- a/docs/maintainers/refactor-candidates.md +++ b/docs/maintainers/refactor-candidates.md @@ -18,6 +18,10 @@ Largest source files in `src/`, ranked by severity. Each does multiple jobs in a - `security/policy.rs` (2,338 lines) mixes policy definition, action tracking, and validation ā€” could split by concern. - `providers/compatible.rs` (2,892 lines) and `providers/gemini.rs` (2,142 lines) are large for single provider implementations ā€” likely mixing HTTP client logic, response parsing, and tool conversion. +### Misplaced module: `channels/tts.rs` ā†’ `tools/` + +`channels/tts.rs` (642 lines, merged in PR #2994) is a multi-provider TTS synthesis system. It is not a channel ā€” it does not implement `Channel` or provide a bidirectional messaging interface. TTS is a capability the agent invokes to produce audio output, which fits the `Tool` trait (`src/tools/traits.rs`). It should be moved to `src/tools/tts.rs` with a corresponding `Tool` implementation, and its config types extracted from the `channels` section of `schema.rs` into a `[tools.tts]` config namespace. As of merge, the module is not integrated into any calling code (re-exports are `#[allow(unused_imports)]`), so this move has zero runtime impact. + --- ## Best Practices Audit Findings diff --git a/src/channels/slack.rs b/src/channels/slack.rs index c8a669573..c43bd1030 100644 --- a/src/channels/slack.rs +++ b/src/channels/slack.rs @@ -1093,6 +1093,21 @@ impl SlackChannel { } drop(temp_file); + // Reject symlinks at the destination to prevent a symlink-following attack + // where an attacker places a symlink at the target path to redirect writes + // outside the workspace. + match tokio::fs::symlink_metadata(&output_path).await { + Ok(meta) if meta.file_type().is_symlink() => { + tracing::warn!( + "Slack image attachment refused: output path is a symlink: {}", + output_path.display() + ); + let _ = tokio::fs::remove_file(&temp_path).await; + return None; + } + _ => {} + } + if let Err(err) = tokio::fs::rename(&temp_path, &output_path).await { tracing::warn!( "Slack image attachment finalize failed for {}: {err}", diff --git a/src/channels/tts.rs b/src/channels/tts.rs index 16b11cf1c..5c98fc45e 100644 --- a/src/channels/tts.rs +++ b/src/channels/tts.rs @@ -364,16 +364,21 @@ impl EdgeTtsProvider { const ALLOWED_BINARIES: &[&str] = &["edge-tts", "edge-playback"]; /// Create a new Edge TTS provider from config. + /// + /// `binary_path` must be a bare command name (no path separators) matching + /// one of [`Self::ALLOWED_BINARIES`]. This prevents arbitrary executable + /// paths like `/tmp/malicious/edge-tts` from passing the basename check. pub fn new(config: &crate::config::EdgeTtsConfig) -> Result { - let basename = std::path::Path::new(&config.binary_path) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or(""); - if !Self::ALLOWED_BINARIES.contains(&basename) { + let path = &config.binary_path; + if path.contains('/') || path.contains('\\') { bail!( - "Edge TTS binary_path must use one of {:?}, got: {}", + "Edge TTS binary_path must be a bare command name without path separators, got: {path}" + ); + } + if !Self::ALLOWED_BINARIES.contains(&path.as_str()) { + bail!( + "Edge TTS binary_path must be one of {:?}, got: {path}", Self::ALLOWED_BINARIES, - config.binary_path ); } Ok(Self { @@ -395,16 +400,20 @@ impl TtsProvider for EdgeTtsProvider { .to_str() .context("Failed to build temp file path for Edge TTS")?; - let output = tokio::process::Command::new(&self.binary_path) - .arg("--text") - .arg(text) - .arg("--voice") - .arg(voice) - .arg("--write-media") - .arg(output_path) - .output() - .await - .context("Failed to spawn edge-tts subprocess")?; + let output = tokio::time::timeout( + TTS_HTTP_TIMEOUT, + tokio::process::Command::new(&self.binary_path) + .arg("--text") + .arg(text) + .arg("--voice") + .arg(voice) + .arg("--write-media") + .arg(output_path) + .output(), + ) + .await + .context("Edge TTS subprocess timed out")? + .context("Failed to spawn edge-tts subprocess")?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); From 026158557c458ef48bb9214f98a23801dda36c86 Mon Sep 17 00:00:00 2001 From: Simian Astronaut 7 Date: Wed, 11 Mar 2026 10:09:06 -0400 Subject: [PATCH 8/9] chore: fix rustfmt violations from TTS provider merge Import ordering in config/mod.rs and line-wrapping in config/schema.rs were left unformatted by PR #2994. Run cargo fmt to fix. Co-Authored-By: Claude Opus 4.6 --- src/config/mod.rs | 12 ++++++------ src/config/schema.rs | 24 ++++-------------------- 2 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/config/mod.rs b/src/config/mod.rs index 9884b06f2..afb4b15ac 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -7,17 +7,17 @@ pub use schema::{ build_runtime_proxy_client_with_timeouts, runtime_proxy_config, set_runtime_proxy_config, AgentConfig, AuditConfig, AutonomyConfig, BrowserComputerUseConfig, BrowserConfig, BuiltinHooksConfig, ChannelsConfig, ClassificationRule, ComposioConfig, Config, CostConfig, - CronConfig, DelegateAgentConfig, DiscordConfig, DockerRuntimeConfig, EmbeddingRouteConfig, - EstopConfig, FeishuConfig, GatewayConfig, HardwareConfig, HardwareTransport, HeartbeatConfig, - HooksConfig, HttpRequestConfig, IMessageConfig, IdentityConfig, LarkConfig, MatrixConfig, - MemoryConfig, ModelRouteConfig, MultimodalConfig, NextcloudTalkConfig, ObservabilityConfig, + CronConfig, DelegateAgentConfig, DiscordConfig, DockerRuntimeConfig, EdgeTtsConfig, + ElevenLabsTtsConfig, EmbeddingRouteConfig, EstopConfig, FeishuConfig, GatewayConfig, + GoogleTtsConfig, HardwareConfig, HardwareTransport, HeartbeatConfig, HooksConfig, + HttpRequestConfig, IMessageConfig, IdentityConfig, LarkConfig, MatrixConfig, MemoryConfig, + ModelRouteConfig, MultimodalConfig, NextcloudTalkConfig, ObservabilityConfig, OpenAiTtsConfig, OtpConfig, OtpMethod, PeripheralBoardConfig, PeripheralsConfig, ProxyConfig, ProxyScope, QdrantConfig, QueryClassificationConfig, ReliabilityConfig, ResourceLimitsConfig, RuntimeConfig, SandboxBackend, SandboxConfig, SchedulerConfig, SecretsConfig, SecurityConfig, SkillsConfig, SkillsPromptInjectionMode, SlackConfig, StorageConfig, StorageProviderConfig, StorageProviderSection, StreamMode, TelegramConfig, TranscriptionConfig, TtsConfig, - EdgeTtsConfig, ElevenLabsTtsConfig, GoogleTtsConfig, OpenAiTtsConfig, TunnelConfig, - WebFetchConfig, WebSearchConfig, WebhookConfig, + TunnelConfig, WebFetchConfig, WebSearchConfig, WebhookConfig, }; pub fn name_and_presence(channel: Option<&T>) -> (&'static str, bool) { diff --git a/src/config/schema.rs b/src/config/schema.rs index c39008de5..b91716885 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -4263,11 +4263,7 @@ impl Config { // Decrypt TTS provider API keys if let Some(ref mut openai) = config.tts.openai { - decrypt_optional_secret( - &store, - &mut openai.api_key, - "config.tts.openai.api_key", - )?; + decrypt_optional_secret(&store, &mut openai.api_key, "config.tts.openai.api_key")?; } if let Some(ref mut elevenlabs) = config.tts.elevenlabs { decrypt_optional_secret( @@ -4277,11 +4273,7 @@ impl Config { )?; } if let Some(ref mut google) = config.tts.google { - decrypt_optional_secret( - &store, - &mut google.api_key, - "config.tts.google.api_key", - )?; + decrypt_optional_secret(&store, &mut google.api_key, "config.tts.google.api_key")?; } if let Some(ref mut ns) = config.channels_config.nostr { @@ -4921,11 +4913,7 @@ impl Config { // Encrypt TTS provider API keys if let Some(ref mut openai) = config_to_save.tts.openai { - encrypt_optional_secret( - &store, - &mut openai.api_key, - "config.tts.openai.api_key", - )?; + encrypt_optional_secret(&store, &mut openai.api_key, "config.tts.openai.api_key")?; } if let Some(ref mut elevenlabs) = config_to_save.tts.elevenlabs { encrypt_optional_secret( @@ -4935,11 +4923,7 @@ impl Config { )?; } if let Some(ref mut google) = config_to_save.tts.google { - encrypt_optional_secret( - &store, - &mut google.api_key, - "config.tts.google.api_key", - )?; + encrypt_optional_secret(&store, &mut google.api_key, "config.tts.google.api_key")?; } if let Some(ref mut ns) = config_to_save.channels_config.nostr { From 5901f70dc0119cb565245b82f776e0ef8d775e45 Mon Sep 17 00:00:00 2001 From: Simian Astronaut 7 Date: Wed, 11 Mar 2026 10:21:46 -0400 Subject: [PATCH 9/9] fix(clippy): box-pin large onboard wizard futures The onboard wizard futures exceed clippy's large_futures threshold (16KB+). Wrap in Box::pin to heap-allocate and fix the lint. Co-Authored-By: Claude Opus 4.6 --- src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index dc67c0f4b..af8041a9f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -717,9 +717,9 @@ async fn main() -> Result<()> { bail!("--channels-only does not accept --force"); } let config = if channels_only { - onboard::run_channels_repair_wizard().await + Box::pin(onboard::run_channels_repair_wizard()).await } else if interactive { - onboard::run_wizard(force).await + Box::pin(onboard::run_wizard(force)).await } else { onboard::run_quick_setup( api_key.as_deref(),