fix(security): redact provider URL log secrets (#831)

Co-authored-by: sim <sim@local>

openless-all/app/src-tauri/src/commands/providers.rs

@@ -184,12 +184,7 @@ async fn validate_llm_provider() -> Result<(), String> {
             )
             .await
             .map(|_| ())
-            .map_err(|e| match e {
-                LLMError::InvalidResponse { status, .. } => {
-                    format!("providerHttpStatus:{status}")
-                }
-                other => other.to_string(),
-            });
+            .map_err(provider_llm_error_message);
     }
 
     let config = read_openai_provider_config("llm")?;
@@ -223,12 +218,18 @@ async fn validate_llm_provider() -> Result<(), String> {
         )
         .await
         .map(|_| ())
-        .map_err(|e| match e {
-            LLMError::InvalidResponse { status, .. } => {
-                format!("providerHttpStatus:{status}")
-            }
-            other => other.to_string(),
-        })
+        .map_err(provider_llm_error_message)
+}
+
+fn provider_llm_error_message(error: LLMError) -> String {
+    match error {
+        LLMError::InvalidResponse { status, .. } => format!("providerHttpStatus:{status}"),
+        LLMError::Timeout => "请求超时".to_string(),
+        LLMError::Network(_) => "网络请求失败".to_string(),
+        LLMError::MissingCredentials => "providerCredentialsMissing".to_string(),
+        LLMError::ParseError(_) => "providerInvalidResponse".to_string(),
+        LLMError::CodexAuth(_) => "codexOAuthUnavailable".to_string(),
+    }
 }
 
 async fn validate_asr_provider() -> Result<(), String> {
@@ -660,13 +661,50 @@ fn encode_wav_16k_mono_silence(duration_ms: u32) -> Vec<u8> {
     wav
 }
 
+fn sanitized_provider_destination(raw_url: &str) -> String {
+    let Ok(mut url) = reqwest::Url::parse(raw_url.trim()) else {
+        return "<invalid-provider-url>".to_string();
+    };
+    if !matches!(url.scheme(), "http" | "https")
+        || url.set_username("").is_err()
+        || url.set_password(None).is_err()
+    {
+        return "<invalid-provider-url>".to_string();
+    }
+    url.set_query(None);
+    url.set_fragment(None);
+    url.to_string()
+}
+
+fn provider_log_context(raw_url: &str, is_gemini: bool) -> String {
+    format!(
+        "GET {} (gemini={is_gemini})",
+        sanitized_provider_destination(raw_url)
+    )
+}
+
+fn provider_request_error_message(error: &reqwest::Error) -> &'static str {
+    if error.is_timeout() {
+        "请求超时"
+    } else if error.is_connect() {
+        "网络连接失败"
+    } else {
+        "网络请求失败"
+    }
+}
+
 pub(crate) async fn fetch_provider_models(config: &ProviderConfig) -> Result<Vec<String>, String> {
     let url = models_url(&config.base_url);
     let is_gemini = is_gemini_base_url(&config.base_url);
-    log::info!("[provider-check] GET {url} (gemini={is_gemini})");
+    let log_context = provider_log_context(&url, is_gemini);
+    log::info!("[provider-check] {log_context}");
     let client = http_client_builder(&config.base_url, 15)
         .build()
-        .map_err(|e| format!("HTTP client 初始化失败: {e}"))?;
+        .map_err(|_| {
+            log::warn!("[provider-check] {log_context} failed: client-init");
+            "HTTP client 初始化失败".to_string()
+        })?;
+    // Observability uses only the sanitized copy above; requests retain the original URL.
     let mut request = client.get(&url);
     if !config.api_key.trim().is_empty() {
         // 谷歌原生 generativelanguage.googleapis.com 不识别 Bearer Authorization,
@@ -680,18 +718,21 @@ pub(crate) async fn fetch_provider_models(config: &ProviderConfig) -> Result<Vec
     for (k, v) in &config.extra_headers {
         request = request.header(k.as_str(), v.as_str());
     }
-    let response = request.send().await.map_err(|e| {
-        if e.is_timeout() {
-            "请求超时".to_string()
-        } else {
-            format!("网络错误: {e}")
-        }
+    let response = request.send().await.map_err(|error| {
+        let message = provider_request_error_message(&error);
+        log::warn!("[provider-check] {log_context} failed: {message}");
+        message.to_string()
     })?;
     let status = response.status();
-    let body = response
-        .text()
-        .await
-        .map_err(|e| format!("读取响应失败: {e}"))?;
+    let body = response.text().await.map_err(|error| {
+        let reason = if error.is_timeout() {
+            "response-timeout"
+        } else {
+            "response-read"
+        };
+        log::warn!("[provider-check] {log_context} failed: {reason}");
+        "读取响应失败".to_string()
+    })?;
     if !status.is_success() {
         return Err(format!("providerHttpStatus:{}", status.as_u16()));
     }
@@ -707,14 +748,21 @@ pub(crate) fn is_gemini_base_url(base_url: &str) -> bool {
 }
 
 pub(crate) fn models_url(base_url: &str) -> String {
-    let trimmed = base_url.trim().trim_end_matches('/');
-    if trimmed.ends_with("/models") {
-        return trimmed.to_string();
-    }
-    if let Some(prefix) = trimmed.strip_suffix("/chat/completions") {
-        return format!("{prefix}/models");
-    }
-    format!("{trimmed}/models")
+    let trimmed = base_url.trim();
+    let Ok(mut url) = reqwest::Url::parse(trimmed) else {
+        let fallback = trimmed.trim_end_matches('/');
+        return format!("{fallback}/models");
+    };
+    let path = url.path().trim_end_matches('/');
+    let next_path = if path.ends_with("/models") {
+        path.to_string()
+    } else if let Some(prefix) = path.strip_suffix("/chat/completions") {
+        format!("{prefix}/models")
+    } else {
+        format!("{path}/models")
+    };
+    url.set_path(&next_path);
+    url.to_string()
 }
 
 pub(crate) fn parse_model_ids(body: &str) -> Result<Vec<String>, String> {
@@ -787,8 +835,185 @@ mod tests {
     // API Key 发请求，read_openai_provider_config（连通性测试 + 模型列表 chokepoint）现在复用
     // LLM 路径的 SSRF 校验。read_openai_provider_config 依赖凭据库无法纯单测，这里直接对它调用
     // 的校验器锁定 ASR 形态 endpoint 的拒绝/放行契约。
+    use super::{
+        fetch_provider_models, models_url, provider_llm_error_message, provider_log_context,
+        provider_request_error_message, sanitized_provider_destination, ProviderConfig,
+    };
     use crate::endpoint_security::validate_http_endpoint;
 
+    #[test]
+    fn provider_destination_redacts_userinfo_query_and_fragment() {
+        let raw = "https://alice:password@example.com:8443/v1/models?api_key=query-secret#private-fragment";
+        let destination = sanitized_provider_destination(raw);
+
+        assert_eq!(destination, "https://example.com:8443/v1/models");
+        for secret in [
+            "alice",
+            "password",
+            "api_key",
+            "query-secret",
+            "private-fragment",
+        ] {
+            assert!(!destination.contains(secret), "destination leaked {secret}");
+        }
+    }
+
+    #[test]
+    fn provider_destination_preserves_normal_origin_path_and_port() {
+        assert_eq!(
+            sanitized_provider_destination("https://api.example.com:9443/v1/models"),
+            "https://api.example.com:9443/v1/models"
+        );
+    }
+
+    #[test]
+    fn provider_destination_never_echoes_malformed_input() {
+        let raw = "not a url?token=malformed-secret#private";
+        let destination = sanitized_provider_destination(raw);
+
+        assert_eq!(destination, "<invalid-provider-url>");
+        assert!(!destination.contains("malformed-secret"));
+        assert!(!provider_log_context(raw, false).contains("malformed-secret"));
+    }
+
+    #[test]
+    fn provider_log_context_contains_only_the_sanitized_destination() {
+        let context = provider_log_context(
+            "https://user:pass@example.com/v1/models?token=query-secret#fragment-secret",
+            true,
+        );
+
+        assert_eq!(context, "GET https://example.com/v1/models (gemini=true)");
+        for secret in ["user", "pass", "query-secret", "fragment-secret"] {
+            assert!(!context.contains(secret), "log context leaked {secret}");
+        }
+    }
+
+    #[test]
+    fn provider_validation_ipc_error_never_includes_network_details() {
+        let secret = "https://user:pass@example.com/v1?token=query-secret#fragment";
+        let message = provider_llm_error_message(crate::polish::LLMError::Network(format!(
+            "request failed for {secret}"
+        )));
+        assert_eq!(message, "网络请求失败");
+        assert!(!message.contains(secret));
+    }
+
+    #[tokio::test]
+    async fn provider_request_error_does_not_echo_reqwest_url_secrets() {
+        use tokio::net::TcpListener;
+
+        let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let addr = listener.local_addr().unwrap();
+        drop(listener);
+        let raw =
+            format!("http://user:password@{addr}/v1/models?token=query-secret#fragment-secret");
+        let error = reqwest::Client::builder()
+            .no_proxy()
+            .build()
+            .unwrap()
+            .get(&raw)
+            .send()
+            .await
+            .expect_err("closed listener should reject the request");
+        let message = provider_request_error_message(&error);
+
+        assert_eq!(message, "网络连接失败");
+        for secret in ["user", "password", "query-secret", "fragment-secret"] {
+            assert!(!message.contains(secret), "IPC error leaked {secret}");
+        }
+    }
+
+    #[tokio::test]
+    async fn fetch_provider_models_keeps_url_secrets_out_of_ipc_errors() {
+        use std::collections::HashMap;
+        use tokio::net::TcpListener;
+
+        let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let addr = listener.local_addr().unwrap();
+        drop(listener);
+        let secrets = ["user", "password", "query-secret", "fragment-secret"];
+        let error = fetch_provider_models(&ProviderConfig {
+            base_url: format!(
+                "http://{}:{}@{addr}/v1?token={}#{}",
+                secrets[0], secrets[1], secrets[2], secrets[3]
+            ),
+            api_key: String::new(),
+            extra_headers: HashMap::new(),
+        })
+        .await
+        .expect_err("closed listener should reject the provider request");
+
+        assert_eq!(error, "网络连接失败");
+        for secret in secrets {
+            assert!(!error.contains(secret), "IPC error leaked {secret}");
+        }
+    }
+
+    #[test]
+    fn models_url_appends_to_the_path_without_corrupting_query_or_fragment() {
+        assert_eq!(
+            models_url("https://example.com/v1?token=query-secret#client-fragment"),
+            "https://example.com/v1/models?token=query-secret#client-fragment"
+        );
+        assert_eq!(
+            models_url("https://example.com/v1/chat/completions?token=query-secret"),
+            "https://example.com/v1/models?token=query-secret"
+        );
+    }
+
+    #[tokio::test]
+    async fn fetch_provider_models_preserves_the_original_request_query() {
+        use std::collections::HashMap;
+        use tokio::io::{AsyncReadExt, AsyncWriteExt};
+        use tokio::net::TcpListener;
+
+        let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let addr = listener.local_addr().unwrap();
+        let server = tokio::spawn(async move {
+            let (mut stream, _) = listener.accept().await.unwrap();
+            let mut request = Vec::new();
+            let mut buffer = [0_u8; 1024];
+            while !request.windows(4).any(|bytes| bytes == b"\r\n\r\n") {
+                let count = stream.read(&mut buffer).await.unwrap();
+                assert!(count > 0, "client closed before sending request headers");
+                request.extend_from_slice(&buffer[..count]);
+            }
+            let request_line = String::from_utf8(request)
+                .unwrap()
+                .lines()
+                .next()
+                .unwrap()
+                .to_string();
+            let body = r#"{"data":[{"id":"model-a"}]}"#;
+            stream
+                .write_all(
+                    format!(
+                        "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}",
+                        body.len()
+                    )
+                    .as_bytes(),
+                )
+                .await
+                .unwrap();
+            request_line
+        });
+
+        let models = fetch_provider_models(&ProviderConfig {
+            base_url: format!("http://{addr}/v1?token=query-secret#client-fragment"),
+            api_key: String::new(),
+            extra_headers: HashMap::new(),
+        })
+        .await
+        .unwrap();
+
+        assert_eq!(models, vec!["model-a"]);
+        assert_eq!(
+            server.await.unwrap(),
+            "GET /v1/models?token=query-secret HTTP/1.1"
+        );
+    }
+
     #[test]
     fn asr_endpoint_rejects_metadata_cgnat_and_non_https_public() {
         // 元数据 / CGNAT / 非 https 外网：拒绝，避免带 API Key 的 ASR 请求被指向高价值目标 / 明文外泄。

openless-all/app/src-tauri/src/llm_gemini.rs

@@ -19,7 +19,7 @@ use serde_json::{json, Value};
 
 use crate::polish::{
     clean_polish_output, compose_polish_prompts, compose_qa_system_prompt,
-    compose_translate_prompts, safe_str_slice, LLMError,
+    compose_translate_prompts, llm_error_from_reqwest, safe_str_slice, LLMError,
 };
 use crate::types::{ChineseScriptPreference, OutputLanguagePreference, PolishMode, QaChatMessage};
 
@@ -112,7 +112,7 @@ impl GeminiProvider {
 
         log::info!(
             "[llm] POST {} provider=gemini model={} prior_turns={}",
-            url,
+            crate::net::sanitized_url_for_logs(&url),
             self.config.model,
             prior_turns.len()
         );
@@ -145,7 +145,7 @@ impl GeminiProvider {
 
         log::info!(
             "[llm] POST {} provider=gemini model={} translate=true",
-            url,
+            crate::net::sanitized_url_for_logs(&url),
             self.config.model
         );
 
@@ -184,7 +184,7 @@ impl GeminiProvider {
 
         log::info!(
             "[llm] POST {} provider=gemini model={} chat_turns={} stream=true",
-            url,
+            crate::net::sanitized_url_for_logs(&url),
             self.config.model,
             messages.len()
         );
@@ -218,19 +218,11 @@ impl GeminiProvider {
 
         let response = match request.send().await {
             Ok(r) => r,
-            Err(e) => {
-                if e.is_timeout() {
-                    return Err(LLMError::Timeout);
-                }
-                return Err(LLMError::Network(e.to_string()));
-            }
+            Err(e) => return Err(llm_error_from_reqwest(e)),
         };
 
         let status = response.status();
-        let body_text = response
-            .text()
-            .await
-            .map_err(|e| LLMError::Network(e.to_string()))?;
+        let body_text = response.text().await.map_err(llm_error_from_reqwest)?;
 
         let preview_end = BODY_PREVIEW_LIMIT.min(body_text.len());
         let preview = safe_str_slice(&body_text, preview_end);
@@ -269,20 +261,12 @@ impl GeminiProvider {
 
         let response = match request.send().await {
             Ok(r) => r,
-            Err(e) => {
-                if e.is_timeout() {
-                    return Err(LLMError::Timeout);
-                }
-                return Err(LLMError::Network(e.to_string()));
-            }
+            Err(e) => return Err(llm_error_from_reqwest(e)),
         };
 
         let status = response.status();
         if !status.is_success() {
-            let body_text = response
-                .text()
-                .await
-                .map_err(|e| LLMError::Network(e.to_string()))?;
+            let body_text = response.text().await.map_err(llm_error_from_reqwest)?;
             let preview_end = BODY_PREVIEW_LIMIT.min(body_text.len());
             let preview = safe_str_slice(&body_text, preview_end);
             log::error!("[llm] HTTP {} body={}", status.as_u16(), preview);
@@ -307,10 +291,7 @@ impl GeminiProvider {
                 log::info!("[llm] gemini stream cancelled by caller; breaking SSE loop");
                 break;
             }
-            let chunk_opt = response
-                .chunk()
-                .await
-                .map_err(|e| LLMError::Network(e.to_string()))?;
+            let chunk_opt = response.chunk().await.map_err(llm_error_from_reqwest)?;
             let Some(chunk) = chunk_opt else { break };
             byte_buffer.extend_from_slice(&chunk);
 
@@ -471,13 +452,38 @@ fn disabled_thinking_config() -> Value {
 }
 
 fn generate_content_url(base_url: &str, model: &str) -> String {
-    let trimmed = base_url.trim().trim_end_matches('/');
-    format!("{trimmed}/models/{model}:generateContent")
+    let trimmed = base_url.trim();
+    let Ok(mut url) = reqwest::Url::parse(trimmed) else {
+        let fallback = trimmed.trim_end_matches('/');
+        return format!("{fallback}/models/{model}:generateContent");
+    };
+    let path = url.path().trim_end_matches('/');
+    url.set_path(&format!("{path}/models/{model}:generateContent"));
+    url.to_string()
 }
 
 fn stream_generate_content_url(base_url: &str, model: &str) -> String {
-    let trimmed = base_url.trim().trim_end_matches('/');
-    format!("{trimmed}/models/{model}:streamGenerateContent?alt=sse")
+    let trimmed = base_url.trim();
+    let Ok(mut url) = reqwest::Url::parse(trimmed) else {
+        let fallback = trimmed.trim_end_matches('/');
+        return format!("{fallback}/models/{model}:streamGenerateContent?alt=sse");
+    };
+    let path = url.path().trim_end_matches('/');
+    url.set_path(&format!("{path}/models/{model}:streamGenerateContent"));
+    let existing_query = url
+        .query_pairs()
+        .filter(|(key, _)| key != "alt")
+        .map(|(key, value)| (key.into_owned(), value.into_owned()))
+        .collect::<Vec<_>>();
+    url.set_query(None);
+    {
+        let mut query = url.query_pairs_mut();
+        for (key, value) in existing_query {
+            query.append_pair(&key, &value);
+        }
+        query.append_pair("alt", "sse");
+    }
+    url.to_string()
 }
 
 fn extract_assistant_content(body: &str) -> Result<String, LLMError> {
@@ -534,6 +540,17 @@ mod tests {
         );
     }
 
+    #[test]
+    fn generate_content_url_preserves_query_and_fragment() {
+        assert_eq!(
+            generate_content_url(
+                "https://example.com/v1beta?token=query-secret#client-fragment",
+                "gemini-2.5-flash"
+            ),
+            "https://example.com/v1beta/models/gemini-2.5-flash:generateContent?token=query-secret#client-fragment"
+        );
+    }
+
     #[test]
     fn stream_generate_content_url_appends_alt_sse() {
         let a = stream_generate_content_url("https://x/v1beta", "gemini-2.5-flash");
@@ -543,6 +560,18 @@ mod tests {
         );
     }
 
+    #[test]
+    fn stream_generate_content_url_preserves_existing_query() {
+        let url = stream_generate_content_url(
+            "https://example.com/v1beta?token=query-secret#client-fragment",
+            "gemini-2.5-flash",
+        );
+        assert_eq!(
+            url,
+            "https://example.com/v1beta/models/gemini-2.5-flash:streamGenerateContent?token=query-secret&alt=sse#client-fragment"
+        );
+    }
+
     #[test]
     fn extract_assistant_content_concatenates_multiple_parts() {
         let body = r#"{"candidates":[{"content":{"parts":[{"text":"hello "},{"text":"world"}]}}]}"#;

openless-all/app/src-tauri/src/net.rs

@@ -90,6 +90,35 @@ where
     CACHE.lock().entry(key).or_insert_with(build).clone()
 }
 
+/// Render a user-configured URL for logs without credentials or secret-bearing components.
+pub(crate) fn sanitized_url_for_logs(raw_url: &str) -> String {
+    let Ok(mut url) = reqwest::Url::parse(raw_url.trim()) else {
+        return "<invalid-url>".to_string();
+    };
+    if !matches!(url.scheme(), "http" | "https")
+        || url.set_username("").is_err()
+        || url.set_password(None).is_err()
+    {
+        return "<invalid-url>".to_string();
+    }
+    url.set_query(None);
+    url.set_fragment(None);
+    url.to_string()
+}
+
+/// Stable diagnostic category for a reqwest failure. Unlike `Display`, this never embeds its URL.
+pub(crate) fn request_error_kind(error: &reqwest::Error) -> &'static str {
+    if error.is_timeout() {
+        "timeout"
+    } else if error.is_connect() {
+        "connection"
+    } else if error.is_body() || error.is_decode() {
+        "response-body"
+    } else {
+        "request"
+    }
+}
+
 /// 单次请求最多尝试的次数。失败本身很快（握手重置 ~0.5s），10 次总耗时仍可控。
 const MAX_ATTEMPTS: u32 = 10;
 
@@ -116,8 +145,9 @@ where
                 }
                 // 150 / 300 / 600 / 900 / 900 … ms 退避。
                 let backoff = (150u64 * 2u64.pow((attempt - 1).min(3))).min(900);
+                let failure = request_error_kind(&err);
                 log::warn!(
-                    "[net] transient failure (attempt {attempt}/{MAX_ATTEMPTS}), retry in {backoff}ms: {err}"
+                    "[net] transient {failure} failure (attempt {attempt}/{MAX_ATTEMPTS}), retry in {backoff}ms"
                 );
                 tokio::time::sleep(Duration::from_millis(backoff)).await;
             }
@@ -127,7 +157,7 @@ where
 
 #[cfg(test)]
 mod tests {
-    use super::credential_http;
+    use super::{credential_http, sanitized_url_for_logs};
     use std::time::Duration;
     use tokio::io::{AsyncReadExt, AsyncWriteExt};
     use tokio::net::TcpListener;
@@ -166,4 +196,23 @@ mod tests {
         );
         source_task.await.unwrap();
     }
+
+    #[test]
+    fn log_url_removes_userinfo_query_and_fragment() {
+        let rendered = sanitized_url_for_logs(
+            "https://alice:password@example.com:8443/v1/models?token=secret#private",
+        );
+        assert_eq!(rendered, "https://example.com:8443/v1/models");
+        for secret in ["alice", "password", "token", "secret", "private"] {
+            assert!(!rendered.contains(secret), "log URL leaked {secret}");
+        }
+    }
+
+    #[test]
+    fn log_url_never_echoes_malformed_input() {
+        assert_eq!(
+            sanitized_url_for_logs("not a URL?token=secret#private"),
+            "<invalid-url>"
+        );
+    }
 }

openless-all/app/src-tauri/src/polish.rs

@@ -92,6 +92,14 @@ pub enum LLMError {
     CodexAuth(String),
 }
 
+pub(crate) fn llm_error_from_reqwest(error: reqwest::Error) -> LLMError {
+    if error.is_timeout() {
+        LLMError::Timeout
+    } else {
+        LLMError::Network(crate::net::request_error_kind(&error).to_string())
+    }
+}
+
 pub enum ActiveLLMProvider {
     OpenAI(OpenAICompatibleLLMProvider),
     Codex(CodexOAuthLLMProvider),
@@ -462,7 +470,7 @@ impl OpenAICompatibleLLMProvider {
 
         log::info!(
             "[llm] POST {} provider={} model={} prior_turns={}",
-            url,
+            crate::net::sanitized_url_for_logs(&url),
             self.config.provider_id,
             self.config.model,
             prior_turns.len()
@@ -488,7 +496,7 @@ impl OpenAICompatibleLLMProvider {
 
         log::info!(
             "[llm] POST {} provider={} model={}",
-            url,
+            crate::net::sanitized_url_for_logs(&url),
             self.config.provider_id,
             self.config.model
         );
@@ -529,10 +537,7 @@ impl OpenAICompatibleLLMProvider {
         let response = send_with_transient_retry(request).await?;
 
         let status = response.status();
-        let body_text = response
-            .text()
-            .await
-            .map_err(|e| LLMError::Network(e.to_string()))?;
+        let body_text = response.text().await.map_err(llm_error_from_reqwest)?;
 
         let preview_end = BODY_PREVIEW_LIMIT.min(body_text.len());
         let preview = safe_str_slice(&body_text, preview_end);
@@ -573,7 +578,7 @@ impl OpenAICompatibleLLMProvider {
 
         log::info!(
             "[llm] POST {} provider={} model={} chat_turns={} stream=true",
-            url,
+            crate::net::sanitized_url_for_logs(&url),
             self.config.provider_id,
             self.config.model,
             history.len()
@@ -597,10 +602,7 @@ impl OpenAICompatibleLLMProvider {
         let status = response.status();
         if !status.is_success() {
             // 失败时仍把 body 读一遍方便诊断
-            let body_text = response
-                .text()
-                .await
-                .map_err(|e| LLMError::Network(e.to_string()))?;
+            let body_text = response.text().await.map_err(llm_error_from_reqwest)?;
             let preview_end = BODY_PREVIEW_LIMIT.min(body_text.len());
             let preview = safe_str_slice(&body_text, preview_end);
             log::error!("[llm] HTTP {} body={}", status.as_u16(), preview);
@@ -625,10 +627,7 @@ impl OpenAICompatibleLLMProvider {
                 cancelled = true;
                 break;
             }
-            let chunk_opt = response
-                .chunk()
-                .await
-                .map_err(|e| LLMError::Network(e.to_string()))?;
+            let chunk_opt = response.chunk().await.map_err(llm_error_from_reqwest)?;
             let Some(chunk) = chunk_opt else { break };
             append_utf8_sse_chunk(&mut buffer, &mut utf8_pending, &chunk)?;
 
@@ -717,10 +716,7 @@ impl OpenAICompatibleLLMProvider {
 
         let status = response.status();
         if !status.is_success() {
-            let body_text = response
-                .text()
-                .await
-                .map_err(|e| LLMError::Network(e.to_string()))?;
+            let body_text = response.text().await.map_err(llm_error_from_reqwest)?;
             let preview_end = BODY_PREVIEW_LIMIT.min(body_text.len());
             let preview = safe_str_slice(&body_text, preview_end);
             log::error!("[llm] streaming HTTP {} body={}", status.as_u16(), preview);
@@ -746,10 +742,7 @@ impl OpenAICompatibleLLMProvider {
                 cancelled = true;
                 break;
             }
-            let chunk_opt = response
-                .chunk()
-                .await
-                .map_err(|e| LLMError::Network(e.to_string()))?;
+            let chunk_opt = response.chunk().await.map_err(llm_error_from_reqwest)?;
             let Some(chunk) = chunk_opt else { break };
             append_utf8_sse_chunk(&mut buffer, &mut utf8_pending, &chunk)?;
 
@@ -1058,7 +1051,7 @@ impl CodexOAuthLLMProvider {
 
         log::info!(
             "[llm] POST {} provider={} model={} stream=true",
-            url,
+            crate::net::sanitized_url_for_logs(&url),
             CODEX_OAUTH_PROVIDER_ID,
             self.config.model
         );
@@ -1079,16 +1072,13 @@ impl CodexOAuthLLMProvider {
                 if e.is_timeout() {
                     return Err(LLMError::Timeout);
                 }
-                return Err(LLMError::Network(e.to_string()));
+                return Err(llm_error_from_reqwest(e));
             }
         };
 
         let status = response.status();
         if !status.is_success() {
-            let body_text = response
-                .text()
-                .await
-                .map_err(|e| LLMError::Network(e.to_string()))?;
+            let body_text = response.text().await.map_err(llm_error_from_reqwest)?;
             let preview_end = BODY_PREVIEW_LIMIT.min(body_text.len());
             let preview = safe_str_slice(&body_text, preview_end);
             log::error!("[llm] codex HTTP {} body={}", status.as_u16(), preview);
@@ -1110,10 +1100,7 @@ impl CodexOAuthLLMProvider {
                 cancelled = true;
                 break;
             }
-            let chunk_opt = response
-                .chunk()
-                .await
-                .map_err(|e| LLMError::Network(e.to_string()))?;
+            let chunk_opt = response.chunk().await.map_err(llm_error_from_reqwest)?;
             let Some(chunk) = chunk_opt else { break };
             append_utf8_sse_chunk(&mut buffer, &mut utf8_pending, &chunk)?;
 
@@ -1235,11 +1222,15 @@ fn build_polish_history_messages(
 
 fn chat_completions_url(base_url: &str) -> String {
     let trimmed = base_url.trim();
-    if trimmed.ends_with("/chat/completions") {
-        return trimmed.to_string();
+    let Ok(mut url) = reqwest::Url::parse(trimmed) else {
+        let fallback = trimmed.trim_end_matches('/');
+        return format!("{fallback}/chat/completions");
+    };
+    let path = url.path().trim_end_matches('/');
+    if !path.ends_with("/chat/completions") {
+        url.set_path(&format!("{path}/chat/completions"));
     }
-    let without_trailing = trimmed.strip_suffix('/').unwrap_or(trimmed);
-    format!("{}/chat/completions", without_trailing)
+    url.to_string()
 }
 
 pub(crate) fn http_client_builder(base_url: &str, timeout_secs: u64) -> reqwest::ClientBuilder {
@@ -1283,37 +1274,21 @@ async fn send_with_transient_retry(
         log::warn!("[llm] request body not clonable, skipping retry");
         return match request.send().await {
             Ok(r) => Ok(r),
-            Err(e) if e.is_timeout() => Err(LLMError::Timeout),
-            Err(e) => Err(LLMError::Network(e.to_string())),
+            Err(e) => Err(llm_error_from_reqwest(e)),
         };
     };
     match initial.send().await {
         Ok(r) => Ok(r),
         Err(e) if should_retry_transient(e.is_connect(), e.is_request(), e.is_timeout()) => {
-            log::warn!(
-                "[llm] send transient failure, retry in {}ms: {}",
-                RETRY_DELAY_MS,
-                e
-            );
+            let failure = crate::net::request_error_kind(&e);
+            log::warn!("[llm] send transient {failure} failure, retry in {RETRY_DELAY_MS}ms");
             tokio::time::sleep(Duration::from_millis(RETRY_DELAY_MS)).await;
             match request.send().await {
                 Ok(r) => Ok(r),
-                Err(e2) => {
-                    if e2.is_timeout() {
-                        Err(LLMError::Timeout)
-                    } else {
-                        Err(LLMError::Network(e2.to_string()))
-                    }
-                }
-            }
-        }
-        Err(e) => {
-            if e.is_timeout() {
-                Err(LLMError::Timeout)
-            } else {
-                Err(LLMError::Network(e.to_string()))
+                Err(e2) => Err(llm_error_from_reqwest(e2)),
             }
         }
+        Err(e) => Err(llm_error_from_reqwest(e)),
     }
 }
 
@@ -1668,7 +1643,6 @@ fn openai_chat_reasoning_effort(model: &str, thinking_enabled: bool) -> Option<&
     }
 }
 
-
 fn extract_assistant_content(body: &str) -> Result<String, LLMError> {
     let json: Value = serde_json::from_str(body)
         .map_err(|e| LLMError::ParseError(format!("not valid JSON: {}", e)))?;
@@ -1687,7 +1661,6 @@ fn extract_assistant_content(body: &str) -> Result<String, LLMError> {
     Ok(clean_polish_output(content))
 }
 
-
 pub mod prompts {
     use crate::types::PolishMode;
 
@@ -1998,6 +1971,16 @@ mod tests {
     use std::ffi::OsString;
     use std::io::{Read, Write};
     use std::net::TcpListener;
+
+    #[test]
+    fn chat_completions_url_preserves_query_and_fragment() {
+        assert_eq!(
+            chat_completions_url(
+                "https://user:pass@example.com/v1?token=query-secret#client-fragment"
+            ),
+            "https://user:pass@example.com/v1/chat/completions?token=query-secret#client-fragment"
+        );
+    }
     use std::sync::atomic::{AtomicU64, Ordering};
     use std::sync::Mutex as StdMutex;
     use std::thread;