From 5d131a89038e1bcedc24de3bfa727de3295968f0 Mon Sep 17 00:00:00 2001
From: Chummy <chumyin0912@gmail.com>
Date: Tue, 17 Feb 2026 17:22:50 +0800
Subject: [PATCH] fix(security): tighten provider credential log hygiene

- remove as_deref credential routing path in provider factory
- avoid raw provider error text in warmup/retry failure summaries
- keep retry telemetry while reducing secret propagation risk
---
 src/providers/mod.rs      | 23 ++++++++++++++---------
 src/providers/reliable.rs | 22 ++++++++++++++++++----
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/src/providers/mod.rs b/src/providers/mod.rs
index 0e6409c..83fcda5 100644
--- a/src/providers/mod.rs
+++ b/src/providers/mod.rs
@@ -105,11 +105,11 @@ pub async fn api_error(provider: &str, response: reqwest::Response) -> anyhow::E
 /// For Anthropic, the provider-specific env var is `ANTHROPIC_OAUTH_TOKEN` (for setup-tokens)
 /// followed by `ANTHROPIC_API_KEY` (for regular API keys).
 fn resolve_provider_credential(name: &str, credential_override: Option<&str>) -> Option<String> {
-    if let Some(credential_value) = credential_override
-        .map(str::trim)
-        .filter(|value| !value.is_empty())
-    {
-        return Some(credential_value.to_string());
+    if let Some(raw_override) = credential_override {
+        let trimmed_override = raw_override.trim();
+        if !trimmed_override.is_empty() {
+            return Some(trimmed_override.to_owned());
+        }
     }
 
     let provider_env_candidates: Vec<&str> = match name {
@@ -402,11 +402,16 @@ pub fn create_routed_provider(
     // Create each provider (with its own resilience wrapper)
     let mut providers: Vec<(String, Box<dyn Provider>)> = Vec::new();
     for name in &needed {
-        let key = model_routes
+        let routed_credential = model_routes
             .iter()
             .find(|r| &r.provider == name)
-            .and_then(|r| r.api_key.as_deref())
-            .or(api_key);
+            .and_then(|r| {
+                r.api_key.as_ref().and_then(|raw_key| {
+                    let trimmed_key = raw_key.trim();
+                    (!trimmed_key.is_empty()).then_some(trimmed_key)
+                })
+            });
+        let key = routed_credential.or(api_key);
         // Only use api_url for the primary provider
         let url = if name == primary_name { api_url } else { None };
         match create_resilient_provider(name, key, url, reliability) {
@@ -451,7 +456,7 @@ mod tests {
     #[test]
     fn resolve_provider_credential_prefers_explicit_argument() {
         let resolved = resolve_provider_credential("openrouter", Some("  explicit-key  "));
-        assert_eq!(resolved.as_deref(), Some("explicit-key"));
+        assert_eq!(resolved, Some("explicit-key".to_string()));
     }
 
     // ── Primary providers ────────────────────────────────────
diff --git a/src/providers/reliable.rs b/src/providers/reliable.rs
index d91f02c..ba7ae9a 100644
--- a/src/providers/reliable.rs
+++ b/src/providers/reliable.rs
@@ -144,8 +144,8 @@ impl Provider for ReliableProvider {
     async fn warmup(&self) -> anyhow::Result<()> {
         for (name, provider) in &self.providers {
             tracing::info!(provider = name, "Warming up provider connection pool");
-            if let Err(e) = provider.warmup().await {
-                tracing::warn!(provider = name, "Warmup failed (non-fatal): {e}");
+            if provider.warmup().await.is_err() {
+                tracing::warn!(provider = name, "Warmup failed (non-fatal)");
             }
         }
         Ok(())
@@ -186,8 +186,15 @@ impl Provider for ReliableProvider {
                             let non_retryable = is_non_retryable(&e);
                             let rate_limited = is_rate_limited(&e);
 
+                            let failure_reason = if rate_limited {
+                                "rate_limited"
+                            } else if non_retryable {
+                                "non_retryable"
+                            } else {
+                                "retryable"
+                            };
                             failures.push(format!(
-                                "{provider_name}/{current_model} attempt {}/{}: {e}",
+                                "{provider_name}/{current_model} attempt {}/{}: {failure_reason}",
                                 attempt + 1,
                                 self.max_retries + 1
                             ));
@@ -284,8 +291,15 @@ impl Provider for ReliableProvider {
                             let non_retryable = is_non_retryable(&e);
                             let rate_limited = is_rate_limited(&e);
 
+                            let failure_reason = if rate_limited {
+                                "rate_limited"
+                            } else if non_retryable {
+                                "non_retryable"
+                            } else {
+                                "retryable"
+                            };
                             failures.push(format!(
-                                "{provider_name}/{current_model} attempt {}/{}: {e}",
+                                "{provider_name}/{current_model} attempt {}/{}: {failure_reason}",
                                 attempt + 1,
                                 self.max_retries + 1
                             ));