From 07b2e6a141d0f110954b1f79d58dd79c7cc500e7 Mon Sep 17 00:00:00 2001 From: TRIP <1933142963@qq.com> Date: Wed, 15 Jul 2026 15:18:10 +0800 Subject: [PATCH] fix(security): redact provider URL log secrets (#831) Co-authored-by: sim --- .../app/src-tauri/src/commands/providers.rs | 289 ++++++++++++++++-- openless-all/app/src-tauri/src/llm_gemini.rs | 93 ++++-- openless-all/app/src-tauri/src/net.rs | 53 +++- openless-all/app/src-tauri/src/polish.rs | 103 +++---- 4 files changed, 412 insertions(+), 126 deletions(-) diff --git a/openless-all/app/src-tauri/src/commands/providers.rs b/openless-all/app/src-tauri/src/commands/providers.rs index f589cce2..a8b35f74 100644 --- a/openless-all/app/src-tauri/src/commands/providers.rs +++ b/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 { wav } +fn sanitized_provider_destination(raw_url: &str) -> String { + let Ok(mut url) = reqwest::Url::parse(raw_url.trim()) else { + return "".to_string(); + }; + if !matches!(url.scheme(), "http" | "https") + || url.set_username("").is_err() + || url.set_password(None).is_err() + { + return "".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, 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 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, 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, ""); + 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 请求被指向高价值目标 / 明文外泄。 diff --git a/openless-all/app/src-tauri/src/llm_gemini.rs b/openless-all/app/src-tauri/src/llm_gemini.rs index 9be778ab..07124b5c 100644 --- a/openless-all/app/src-tauri/src/llm_gemini.rs +++ b/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::>(); + 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 { @@ -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"}]}}]}"#; diff --git a/openless-all/app/src-tauri/src/net.rs b/openless-all/app/src-tauri/src/net.rs index 45be25af..bc3421ab 100644 --- a/openless-all/app/src-tauri/src/net.rs +++ b/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 "".to_string(); + }; + if !matches!(url.scheme(), "http" | "https") + || url.set_username("").is_err() + || url.set_password(None).is_err() + { + return "".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"), + "" + ); + } } diff --git a/openless-all/app/src-tauri/src/polish.rs b/openless-all/app/src-tauri/src/polish.rs index 0b5056fe..3f552061 100644 --- a/openless-all/app/src-tauri/src/polish.rs +++ b/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 { 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 { 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;