diff --git a/src/channels/wasm/setup.rs b/src/channels/wasm/setup.rs index ca202b3b..cf448750 100644 --- a/src/channels/wasm/setup.rs +++ b/src/channels/wasm/setup.rs @@ -218,25 +218,31 @@ async fn register_channel( } // Inject credentials from secrets store / environment. - if let Some(secrets) = secrets_store { - match inject_channel_credentials(&channel_arc, secrets.as_ref(), &channel_name).await { - Ok(count) => { - if count > 0 { - tracing::info!( - channel = %channel_name, - credentials_injected = count, - "Channel credentials injected" - ); - } - } - Err(e) => { - tracing::error!( + match inject_channel_credentials( + &channel_arc, + secrets_store + .as_ref() + .map(|s| s.as_ref() as &dyn SecretsStore), + &channel_name, + ) + .await + { + Ok(count) => { + if count > 0 { + tracing::info!( channel = %channel_name, - error = %e, - "Failed to inject channel credentials" + credentials_injected = count, + "Channel credentials injected" ); } } + Err(e) => { + tracing::error!( + channel = %channel_name, + error = %e, + "Failed to inject channel credentials" + ); + } } (channel_name, Box::new(SharedWasmChannel::new(channel_arc))) @@ -247,58 +253,70 @@ async fn register_channel( /// Looks for secrets matching the pattern `{channel_name}_*` and injects them /// as credential placeholders (e.g., `telegram_bot_token` -> `{TELEGRAM_BOT_TOKEN}`). /// -/// Falls back to environment variables with the uppercase name if not found -/// in the secrets store (e.g., `TELEGRAM_BOT_TOKEN`). +/// Falls back to environment variables starting with the uppercase channel name +/// prefix (e.g., `TELEGRAM_` for channel `telegram`) for missing credentials. +/// +/// Returns the number of credentials injected. pub async fn inject_channel_credentials( channel: &Arc, - secrets: &dyn SecretsStore, + secrets: Option<&dyn SecretsStore>, channel_name: &str, ) -> anyhow::Result { - let all_secrets = secrets - .list("default") - .await - .map_err(|e| anyhow::anyhow!("Failed to list secrets: {}", e))?; + if channel_name.trim().is_empty() { + return Ok(0); + } - let prefix = format!("{}_", channel_name); let mut count = 0; let mut injected_placeholders = HashSet::new(); - for secret_meta in all_secrets { - if !secret_meta.name.starts_with(&prefix) { - continue; - } + // 1. Try injecting from persistent secrets store if available + if let Some(secrets) = secrets { + let all_secrets = secrets + .list("default") + .await + .map_err(|e| anyhow::anyhow!("Failed to list secrets: {}", e))?; - let decrypted = match secrets.get_decrypted("default", &secret_meta.name).await { - Ok(d) => d, - Err(e) => { - tracing::warn!( - secret = %secret_meta.name, - error = %e, - "Failed to decrypt secret for channel credential injection" - ); + let prefix = format!("{}_", channel_name.to_ascii_lowercase()); + + for secret_meta in all_secrets { + if !secret_meta.name.to_ascii_lowercase().starts_with(&prefix) { continue; } - }; - let placeholder = secret_meta.name.to_uppercase(); + let decrypted = match secrets.get_decrypted("default", &secret_meta.name).await { + Ok(d) => d, + Err(e) => { + tracing::warn!( + secret = %secret_meta.name, + error = %e, + "Failed to decrypt secret for channel credential injection" + ); + continue; + } + }; - tracing::debug!( - channel = %channel_name, - secret = %secret_meta.name, - placeholder = %placeholder, - "Injecting credential" - ); + let placeholder = secret_meta.name.to_uppercase(); - channel - .set_credential(&placeholder, decrypted.expose().to_string()) - .await; - injected_placeholders.insert(placeholder); - count += 1; + tracing::debug!( + channel = %channel_name, + secret = %secret_meta.name, + placeholder = %placeholder, + "Injecting credential" + ); + + channel + .set_credential(&placeholder, decrypted.expose().to_string()) + .await; + injected_placeholders.insert(placeholder); + count += 1; + } } - // Fall back to environment variables for required secrets not found in the store. - // This allows channels to work when configured via env vars (e.g., TELEGRAM_BOT_TOKEN) - // without requiring the setup wizard to have run. + // 2. Fall back to environment variables for credentials not in the secrets store. + // Only env vars starting with the channel's uppercase prefix are allowed + // (e.g., TELEGRAM_ for channel "telegram") to prevent reading unrelated host + // credentials like AWS_SECRET_ACCESS_KEY. + let prefix = format!("{}_", channel_name.to_ascii_uppercase()); let caps = channel.capabilities(); if let Some(ref http_cap) = caps.tool_capabilities.http { for cred_mapping in http_cap.credentials.values() { @@ -306,6 +324,14 @@ pub async fn inject_channel_credentials( if injected_placeholders.contains(&placeholder) { continue; } + if !placeholder.starts_with(&prefix) { + tracing::warn!( + channel = %channel_name, + placeholder = %placeholder, + "Ignoring non-prefixed credential placeholder in environment fallback" + ); + continue; + } if let Ok(env_value) = std::env::var(&placeholder) && !env_value.is_empty() { diff --git a/src/extensions/manager.rs b/src/extensions/manager.rs index 5e74c344..7cf4b49a 100644 --- a/src/extensions/manager.rs +++ b/src/extensions/manager.rs @@ -2775,9 +2775,9 @@ impl ExtensionManager { } // Inject credentials - match crate::extensions::manager::inject_channel_credentials_from_secrets( + match inject_channel_credentials_from_secrets( &channel_arc, - self.secrets.as_ref(), + Some(self.secrets.as_ref()), &channel_name, &self.user_id, ) @@ -2862,7 +2862,7 @@ impl ExtensionManager { // Re-inject credentials from secrets store into the running channel let cred_count = match inject_channel_credentials_from_secrets( &existing_channel, - self.secrets.as_ref(), + Some(self.secrets.as_ref()), name, &self.user_id, ) @@ -3441,48 +3441,131 @@ impl ExtensionManager { /// Looks for secrets matching the pattern `{channel_name}_*` and injects them /// as credential placeholders (e.g., `telegram_bot_token` -> `{TELEGRAM_BOT_TOKEN}`). /// +/// Falls back to environment variables starting with the uppercase channel name +/// prefix (e.g., `TELEGRAM_` for channel `telegram`) for missing credentials. +/// /// Returns the number of credentials injected. async fn inject_channel_credentials_from_secrets( channel: &Arc, - secrets: &dyn SecretsStore, + secrets: Option<&dyn SecretsStore>, channel_name: &str, user_id: &str, ) -> Result { - let all_secrets = secrets - .list(user_id) - .await - .map_err(|e| format!("Failed to list secrets: {}", e))?; - - let prefix = format!("{}_", channel_name); let mut count = 0; + let mut injected_placeholders = std::collections::HashSet::new(); - for secret_meta in all_secrets { - if !secret_meta.name.starts_with(&prefix) { - continue; - } + // 1. Try injecting from persistent secrets store if available + if let Some(secrets) = secrets { + let all_secrets = secrets + .list(user_id) + .await + .map_err(|e| format!("Failed to list secrets: {}", e))?; - let decrypted = match secrets.get_decrypted(user_id, &secret_meta.name).await { - Ok(d) => d, - Err(e) => { - tracing::warn!( - secret = %secret_meta.name, - error = %e, - "Failed to decrypt secret for channel credential injection" - ); + let prefix = format!("{}_", channel_name.to_ascii_lowercase()); + + for secret_meta in all_secrets { + if !secret_meta.name.to_ascii_lowercase().starts_with(&prefix) { continue; } - }; - let placeholder = secret_meta.name.to_uppercase(); - channel - .set_credential(&placeholder, decrypted.expose().to_string()) - .await; - count += 1; + let decrypted = match secrets.get_decrypted(user_id, &secret_meta.name).await { + Ok(d) => d, + Err(e) => { + tracing::warn!( + secret = %secret_meta.name, + error = %e, + "Failed to decrypt secret for channel credential injection" + ); + continue; + } + }; + + let placeholder = secret_meta.name.to_uppercase(); + channel + .set_credential(&placeholder, decrypted.expose().to_string()) + .await; + injected_placeholders.insert(placeholder); + count += 1; + } } + // 2. Fallback to environment variables for missing credentials + count += inject_env_credentials(channel, channel_name, &injected_placeholders).await; + Ok(count) } +/// Inject missing credentials from environment variables. +/// +/// Only environment variables starting with the uppercase channel name prefix +/// (e.g., `TELEGRAM_` for channel `telegram`) are considered for security. +async fn inject_env_credentials( + channel: &Arc, + channel_name: &str, + already_injected: &std::collections::HashSet, +) -> usize { + if channel_name.trim().is_empty() { + return 0; + } + + let caps = channel.capabilities(); + let Some(ref http_cap) = caps.tool_capabilities.http else { + return 0; + }; + + let placeholders: Vec = http_cap + .credentials + .values() + .map(|m| m.secret_name.to_uppercase()) + .collect(); + + let resolved = resolve_env_credentials(&placeholders, channel_name, already_injected); + let count = resolved.len(); + for (placeholder, value) in resolved { + channel.set_credential(&placeholder, value).await; + } + count +} + +/// Pure helper: from a list of credential placeholder names, return those that +/// pass the channel-prefix security check and have a non-empty env var value. +/// +/// Placeholders already covered by the secrets store (`already_injected`) are +/// skipped. Only names starting with `{CHANNEL_NAME}_` are allowed to prevent +/// a WASM channel from reading unrelated host credentials (e.g. `AWS_SECRET_ACCESS_KEY`). +pub(crate) fn resolve_env_credentials( + placeholders: &[String], + channel_name: &str, + already_injected: &std::collections::HashSet, +) -> Vec<(String, String)> { + if channel_name.trim().is_empty() { + return Vec::new(); + } + + let prefix = format!("{}_", channel_name.to_ascii_uppercase()); + let mut out = Vec::new(); + + for placeholder in placeholders { + if already_injected.contains(placeholder) { + continue; + } + if !placeholder.starts_with(&prefix) { + tracing::warn!( + channel = %channel_name, + placeholder = %placeholder, + "Ignoring non-prefixed credential placeholder in environment fallback" + ); + continue; + } + if let Ok(value) = std::env::var(placeholder) + && !value.is_empty() + { + out.push((placeholder.clone(), value)); + } + } + out +} + /// Infer the extension kind from a URL. fn infer_kind_from_url(url: &str) -> ExtensionKind { if url.ends_with(".wasm") || url.ends_with(".tar.gz") { @@ -3933,4 +4016,95 @@ mod tests { Vec::new(), ) } + + // ── resolve_env_credentials tests ──────────────────────────────────── + + #[test] + fn test_security_prefix_check() { + // Placeholders that don't start with the channel prefix must be rejected. + // All env var names are prefixed with ICTEST1_ to avoid CI collisions. + let placeholders = vec![ + "ICTEST1_BOT_TOKEN".to_string(), // valid: matches channel prefix + "ICTEST2_TOKEN".to_string(), // invalid: wrong channel prefix + "ICTEST1_UNRELATED_OTHER".to_string(), // valid prefix, but env var not set — not injected + ]; + let already_injected = std::collections::HashSet::new(); + + unsafe { std::env::set_var("ICTEST1_BOT_TOKEN", "good-secret") }; + unsafe { std::env::set_var("ICTEST2_TOKEN", "bad-secret") }; + // ICTEST1_UNRELATED_OTHER intentionally not set — tests both prefix rejection and absence + + let resolved = super::resolve_env_credentials(&placeholders, "ictest1", &already_injected); + + // Only ICTEST1_BOT_TOKEN passes the prefix check for channel "ictest1" + assert_eq!(resolved.len(), 1); + assert_eq!(resolved[0].0, "ICTEST1_BOT_TOKEN"); + assert_eq!(resolved[0].1, "good-secret"); + + unsafe { std::env::remove_var("ICTEST1_BOT_TOKEN") }; + unsafe { std::env::remove_var("ICTEST2_TOKEN") }; + } + + #[test] + fn test_already_injected_skipped() { + // Use unique env var names (ictest3_*) to avoid interference with other tests. + let placeholders = vec!["ICTEST3_TOKEN".to_string()]; + let mut already_injected = std::collections::HashSet::new(); + already_injected.insert("ICTEST3_TOKEN".to_string()); + + unsafe { std::env::set_var("ICTEST3_TOKEN", "secret") }; + + let resolved = super::resolve_env_credentials(&placeholders, "ictest3", &already_injected); + + // Already covered by secrets store — env var must be skipped + assert!(resolved.is_empty()); + + unsafe { std::env::remove_var("ICTEST3_TOKEN") }; + } + + #[test] + fn test_missing_env_var_not_injected() { + // Use unique env var names (ictest4_*) to avoid interference with other tests. + let placeholders = vec!["ICTEST4_TOKEN".to_string()]; + let already_injected = std::collections::HashSet::new(); + + unsafe { std::env::remove_var("ICTEST4_TOKEN") }; + + let resolved = super::resolve_env_credentials(&placeholders, "ictest4", &already_injected); + + assert!(resolved.is_empty()); + } + + #[test] + fn test_empty_env_var_not_injected() { + // An env var that exists but is empty must not be injected. + // Use unique env var names (ictest5_*) to avoid interference with other tests. + let placeholders = vec!["ICTEST5_TOKEN".to_string()]; + let already_injected = std::collections::HashSet::new(); + + unsafe { std::env::set_var("ICTEST5_TOKEN", "") }; + + let resolved = super::resolve_env_credentials(&placeholders, "ictest5", &already_injected); + + assert!(resolved.is_empty()); + + unsafe { std::env::remove_var("ICTEST5_TOKEN") }; + } + + #[test] + fn test_empty_channel_name_returns_nothing() { + // An empty channel name must never match any env var (prefix would be "_"). + let placeholders = vec!["_TOKEN".to_string(), "ICTEST6_TOKEN".to_string()]; + let already_injected = std::collections::HashSet::new(); + + unsafe { std::env::set_var("_TOKEN", "bad") }; + unsafe { std::env::set_var("ICTEST6_TOKEN", "bad") }; + + let resolved = super::resolve_env_credentials(&placeholders, "", &already_injected); + + assert!(resolved.is_empty(), "empty channel name must match nothing"); + + unsafe { std::env::remove_var("_TOKEN") }; + unsafe { std::env::remove_var("ICTEST6_TOKEN") }; + } }