fix: enable WASM credential injection in No-DB environments (#845)

* fix(wasm): enable credential injection in no-DB environments via env var fallback When a secrets store is unavailable (e.g. no-DB mode), WASM channel credentials were silently not injected, causing channels to start without credentials. Fix by: - Changing `inject_channel_credentials_from_secrets` to accept `Option<&dyn SecretsStore>` — secrets store is tried first when present - Adding env var fallback (`inject_env_credentials`) for credentials not covered by the secrets store - Enforcing a channel-name prefix security check on env var names to prevent WASM channels from reading unrelated host credentials (e.g. `AWS_SECRET_ACCESS_KEY`) - Extracting pure `resolve_env_credentials` helper for testability - Adding case-insensitive prefix matching for secrets store lookup Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(wasm): inject credentials at startup when no secrets store (setup.rs path) The startup path (setup_wasm_channels -> register_channel) was guarded by `if let Some(secrets) = secrets_store`, so in No-DB mode credentials were never injected and the channel started without them. Fix by: - Changing inject_channel_credentials to accept Option<&dyn SecretsStore> - Always calling it (removing the if-let guard) — env var fallback runs even when secrets_store is None - Adding channel-name prefix security check to the env var fallback path (e.g. TELEGRAM_ for channel "telegram"), consistent with manager.rs Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * fix(test): correct misleading comment on ICTEST1_UNRELATED_OTHER placeholder * fix(wasm): guard against empty channel name in credential injection An empty channel_name would produce prefix "_", allowing any env var starting with "_" to pass the security check and be injected. Add an early-return guard in resolve_env_credentials, inject_env_credentials, and inject_channel_credentials. Add a test to cover this path. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --------- Co-authored-by: lizican123 <[email protected]> Co-authored-by: Claude Sonnet 4.6 <[email protected]>
2026-08-25 14:53:34 +00:00 · 2026-03-10 11:08:50 -07:00
parent 6e1ed939cc
commit 8da202e0d2
2 changed files with 279 additions and 79 deletions
@@ -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<crate::channels::wasm::WasmChannel>,
-    secrets: &dyn SecretsStore,
+    secrets: Option<&dyn SecretsStore>,
    channel_name: &str,
    user_id: &str,
 ) -> Result<usize, String> {
-    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<crate::channels::wasm::WasmChannel>,
+    channel_name: &str,
+    already_injected: &std::collections::HashSet<String>,
+) -> 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<String> = 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<String>,
+) -> 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") };
+    }
 }