From 84315840f86974e691857c4302930fe924d948a9 Mon Sep 17 00:00:00 2001 From: whit3rabbit Date: Sat, 4 Apr 2026 13:33:23 -0500 Subject: [PATCH] fix: harden security, fix expires_at TTL, improve translation accuracy - Wrap admin token in Zeroizing so memory is wiped on drop - Use SSRF-safe HTTP client for Langfuse and webhook dispatcher - Wire up webhook dispatcher at startup (was previously un-started) - Fix batch expires_at: was using now instead of now+24h - Extract epoch_secs() helper; replace 4 inline SystemTime::now() blocks - Gemini tool_choice {type:tool}: use ANY+allowedFunctionNames instead of AUTO - Map Anthropic thinking budget_tokens to OpenAI reasoning_effort - Preserve temperature/top_p for GA o-series models (o1/o3/o3-mini/o4-mini); only strip for o1-preview and o1-mini which reject those params - Azure simple config: always route through default_base_url; guard against double-appending deployment path when user provides a full URL Co-Authored-By: Claude Sonnet 4.6 --- crates/batch_engine/src/db.rs | 19 +++-- crates/batch_engine/src/engine.rs | 7 +- crates/batch_engine/src/queue/sqlite.rs | 20 +---- crates/batch_engine/src/webhook/sqlite.rs | 20 +---- crates/proxy/src/admin/auth.rs | 5 +- crates/proxy/src/admin/routes.rs | 7 +- crates/proxy/src/admin/ws.rs | 4 +- crates/proxy/src/config/simple.rs | 21 +++-- crates/proxy/src/integrations/langfuse.rs | 10 ++- crates/proxy/src/main.rs | 23 ++++-- crates/proxy/src/server/middleware.rs | 10 ++- crates/proxy/tests/virtual_keys.rs | 8 +- crates/translator/src/gemini/request.rs | 5 +- .../src/mapping/gemini_message_map.rs | 24 +++--- crates/translator/src/mapping/message_map.rs | 81 +++++++++++++++---- 15 files changed, 165 insertions(+), 99 deletions(-) diff --git a/crates/batch_engine/src/db.rs b/crates/batch_engine/src/db.rs index 4851985..237d9ff 100644 --- a/crates/batch_engine/src/db.rs +++ b/crates/batch_engine/src/db.rs @@ -28,13 +28,22 @@ pub fn format_epoch_iso8601(secs: u64) -> String { ) } +/// Current epoch seconds. +pub fn epoch_secs() -> u64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system clock before Unix epoch") + .as_secs() +} + /// ISO 8601 timestamp for "now" in UTC. pub fn now_iso8601() -> String { - let secs = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs(); - format_epoch_iso8601(secs) + format_epoch_iso8601(epoch_secs()) +} + +/// ISO 8601 timestamp for a given epoch + `hours` offset. +pub fn epoch_plus_hours_iso8601(epoch: u64, hours: u64) -> String { + format_epoch_iso8601(epoch + hours * 3600) } /// Initialize all batch_engine tables. diff --git a/crates/batch_engine/src/engine.rs b/crates/batch_engine/src/engine.rs index 5f91c57..a912744 100644 --- a/crates/batch_engine/src/engine.rs +++ b/crates/batch_engine/src/engine.rs @@ -2,7 +2,6 @@ //! BatchEngine: the main entry point for batch operations. //! Thin facade over JobQueue, FileStore, and WebhookQueue. -use crate::db::now_iso8601; use crate::error::EngineError; use crate::file_store::FileStore; use crate::job::*; @@ -29,7 +28,8 @@ impl BatchEngine { const DEFAULT_MAX_RETRIES: u8 = 3; - let now = now_iso8601(); + let epoch = crate::db::epoch_secs(); + let now = crate::db::format_epoch_iso8601(epoch); let batch_id = BatchId::new(); let total = submission.items.len() as u32; @@ -49,7 +49,8 @@ impl BatchEngine { created_at: now.clone(), started_at: None, completed_at: None, - expires_at: now.clone(), // TODO: add 24h + // 24h TTL matches the Anthropic batch API contract (results expire after 24h). + expires_at: crate::db::epoch_plus_hours_iso8601(epoch, 24), }; let items: Vec = submission diff --git a/crates/batch_engine/src/queue/sqlite.rs b/crates/batch_engine/src/queue/sqlite.rs index 092224f..3e22376 100644 --- a/crates/batch_engine/src/queue/sqlite.rs +++ b/crates/batch_engine/src/queue/sqlite.rs @@ -2,7 +2,7 @@ //! SQLite-backed JobQueue implementation. use super::{JobQueue, LeasedItem}; -use crate::db::{format_epoch_iso8601, now_iso8601}; +use crate::db::{epoch_secs, format_epoch_iso8601, now_iso8601}; use crate::error::QueueError; use crate::job::*; use async_trait::async_trait; @@ -193,14 +193,7 @@ impl JobQueue for SqliteQueue { let lease_id = format!("lease_{}", uuid::Uuid::new_v4()); let now = now_iso8601(); // Lease for 120 seconds. - let lease_expires = { - let secs = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() - + 120; - format_epoch_iso8601(secs) - }; + let lease_expires = format_epoch_iso8601(epoch_secs() + 120); let result = conn.query_row( "UPDATE batch_item @@ -303,14 +296,7 @@ impl JobQueue for SqliteQueue { let db = self.db.clone(); let id = id.0.clone(); let error = error.to_string(); - let retry_at = { - let secs = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() - + delay.as_secs(); - format_epoch_iso8601(secs) - }; + let retry_at = format_epoch_iso8601(epoch_secs() + delay.as_secs()); tokio::task::spawn_blocking(move || { let conn = db.blocking_lock(); diff --git a/crates/batch_engine/src/webhook/sqlite.rs b/crates/batch_engine/src/webhook/sqlite.rs index 602ed65..82162b2 100644 --- a/crates/batch_engine/src/webhook/sqlite.rs +++ b/crates/batch_engine/src/webhook/sqlite.rs @@ -2,7 +2,7 @@ //! SQLite-backed webhook delivery queue. use super::{LeasedDelivery, WebhookDelivery, WebhookQueue}; -use crate::db::{format_epoch_iso8601, now_iso8601}; +use crate::db::{epoch_secs, format_epoch_iso8601, now_iso8601}; use crate::error::QueueError; use async_trait::async_trait; use rusqlite::{params, Connection}; @@ -59,14 +59,7 @@ impl WebhookQueue for SqliteWebhookQueue { let conn = db.blocking_lock(); let lease_id = format!("whl_{}", uuid::Uuid::new_v4()); let now = now_iso8601(); - let lease_expires = { - let secs = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() - + 60; - format_epoch_iso8601(secs) - }; + let lease_expires = format_epoch_iso8601(epoch_secs() + 60); let result = conn.query_row( "UPDATE webhook_delivery @@ -133,14 +126,7 @@ impl WebhookQueue for SqliteWebhookQueue { async fn schedule_retry(&self, delivery_id: &str, delay: Duration) -> Result<(), QueueError> { let db = self.db.clone(); let id = delivery_id.to_string(); - let retry_at = { - let secs = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap() - .as_secs() - + delay.as_secs(); - format_epoch_iso8601(secs) - }; + let retry_at = format_epoch_iso8601(epoch_secs() + delay.as_secs()); tokio::task::spawn_blocking(move || { let conn = db.blocking_lock(); conn.execute( diff --git a/crates/proxy/src/admin/auth.rs b/crates/proxy/src/admin/auth.rs index f46811b..5652814 100644 --- a/crates/proxy/src/admin/auth.rs +++ b/crates/proxy/src/admin/auth.rs @@ -8,6 +8,7 @@ use axum::{ response::{IntoResponse, Response}, }; use std::sync::Arc; +use zeroize::Zeroizing; use subtle::ConstantTimeEq; /// Constant-time string comparison to prevent timing side-channels. @@ -43,7 +44,7 @@ pub fn validate_csrf_tokens(from_header: &str, from_cookie: &str) -> bool { /// Axum middleware that validates the admin bearer token. pub async fn validate_admin_token( - token: axum::extract::State>, + token: axum::extract::State>>, req: Request, next: Next, ) -> Response { @@ -130,7 +131,7 @@ mod tests { use tower::ServiceExt; fn test_app(token: &str) -> Router { - let token = Arc::new(token.to_string()); + let token = Arc::new(Zeroizing::new(token.to_string())); Router::new() .route("/protected", get(|| async { "ok" })) .layer(middleware::from_fn_with_state( diff --git a/crates/proxy/src/admin/routes.rs b/crates/proxy/src/admin/routes.rs index a3ebbc0..8e0b490 100644 --- a/crates/proxy/src/admin/routes.rs +++ b/crates/proxy/src/admin/routes.rs @@ -230,6 +230,7 @@ pub async fn validate_csrf( ) -> axum::response::Response { let method = req.method().clone(); + // PATCH is included: partial updates mutate state just like PUT/DELETE. if matches!( method, axum::http::Method::POST @@ -330,7 +331,7 @@ async fn get_csrf_token(State(shared): State) -> axum::response::Re /// Build the admin router. /// Token is used for auth middleware on all routes except /admin/health. -pub fn admin_router(shared: SharedState, token: Arc) -> Router { +pub fn admin_router(shared: SharedState, token: Arc>) -> Router { // Public routes (no auth). // /admin/csrf-token is public so the SPA can fetch a token before and after login. // Rate-limited to prevent unauthenticated flooding of the CSRF token map. @@ -1843,7 +1844,7 @@ mod tests { // Raise rate limit so parallel unit tests don't interfere. set_admin_rpm(10_000); let shared = crate::admin::state::SharedState::new_for_test(); - let token = Arc::new("test-token".to_string()); + let token = Arc::new(zeroize::Zeroizing::new("test-token".to_string())); admin_router(shared, token) } @@ -1969,7 +1970,7 @@ mod tests { let token_str = "a".repeat(64); // Pre-register the token as server-issued so validate_csrf can find it. shared.issued_csrf_tokens.insert(token_str.clone(), ()); - let app = admin_router(shared, Arc::new("test-token".to_string())); + let app = admin_router(shared, Arc::new(zeroize::Zeroizing::new("test-token".to_string()))); let req = Request::post("/admin/api/keys") .header("host", "localhost:9090") .header("authorization", "Bearer test-token") diff --git a/crates/proxy/src/admin/ws.rs b/crates/proxy/src/admin/ws.rs index f7f5bc0..e0de0a3 100644 --- a/crates/proxy/src/admin/ws.rs +++ b/crates/proxy/src/admin/ws.rs @@ -15,7 +15,7 @@ use std::sync::Arc; /// Auth via the first WebSocket message to avoid leaking the token in URLs/logs. /// The client must send `{"token": ""}` as its first message. pub(crate) async fn ws_handler( - State((shared, expected_token)): State<(SharedState, Arc)>, + State((shared, expected_token)): State<(SharedState, Arc>)>, ws: WebSocketUpgrade, ) -> impl IntoResponse { ws.on_upgrade(move |socket| handle_ws(socket, shared, expected_token)) @@ -23,7 +23,7 @@ pub(crate) async fn ws_handler( } /// Authenticate via the first WebSocket message, then stream events. -async fn handle_ws(mut socket: WebSocket, shared: SharedState, expected_token: Arc) { +async fn handle_ws(mut socket: WebSocket, shared: SharedState, expected_token: Arc>) { // Wait for the first message containing the auth token. let authenticated = tokio::time::timeout(std::time::Duration::from_secs(5), socket.recv()).await; diff --git a/crates/proxy/src/config/simple.rs b/crates/proxy/src/config/simple.rs index 560f60c..74ecaa8 100644 --- a/crates/proxy/src/config/simple.rs +++ b/crates/proxy/src/config/simple.rs @@ -229,8 +229,8 @@ pub fn parse_simple_yaml(yaml: &str) -> SimpleParsed { .api_key .clone() .unwrap_or_else(|| default_api_key_for_provider(&norm.provider, &kind)); - let base_url = if norm.api_base.is_some() && kind == BackendKind::AzureOpenAI { - // Azure: build the full deployment URL from api_base + deployment + api_version + let base_url = if kind == BackendKind::AzureOpenAI { + // Azure always builds a full deployment URL (api_base or env var + deployment + version). default_base_url(&kind, &norm) } else { norm.api_base @@ -556,12 +556,17 @@ fn default_base_url(kind: &BackendKind, entry: &NormalizedEntry) -> String { "api_base field (or AZURE_OPENAI_ENDPOINT env var) required for azure provider", ) }); - let dep = entry.deployment.as_deref().unwrap_or("chat"); - let version = entry.api_version.as_deref().unwrap_or("2024-10-21"); - format!( - "{}/openai/deployments/{dep}/chat/completions?api-version={version}", - endpoint.trim_end_matches('/') - ) + // Guard against double-appending if user provided a full deployment URL. + if endpoint.contains("/openai/deployments/") { + endpoint + } else { + let dep = entry.deployment.as_deref().unwrap_or("chat"); + let version = entry.api_version.as_deref().unwrap_or("2024-10-21"); + format!( + "{}/openai/deployments/{dep}/chat/completions?api-version={version}", + endpoint.trim_end_matches('/') + ) + } } BackendKind::Bedrock => { // Bedrock doesn't use a URL — the region string is stored in base_url diff --git a/crates/proxy/src/integrations/langfuse.rs b/crates/proxy/src/integrations/langfuse.rs index d8a90a0..4c4b840 100644 --- a/crates/proxy/src/integrations/langfuse.rs +++ b/crates/proxy/src/integrations/langfuse.rs @@ -33,10 +33,12 @@ impl LangfuseClient { tracing::error!(host = %host, error = %e, "LANGFUSE_HOST rejected (SSRF protection)"); return None; } - let client = reqwest::Client::builder() - .timeout(std::time::Duration::from_secs(5)) - .build() - .expect("langfuse http client"); + let client = anyllm_client::http::build_http_client(&anyllm_client::http::HttpClientConfig { + ssrf_protection: true, + connect_timeout: Some(std::time::Duration::from_secs(5)), + read_timeout: Some(std::time::Duration::from_secs(5)), + ..Default::default() + }); Some(Arc::new(Self { public_key, secret_key, diff --git a/crates/proxy/src/main.rs b/crates/proxy/src/main.rs index e3da187..517e2df 100644 --- a/crates/proxy/src/main.rs +++ b/crates/proxy/src/main.rs @@ -497,7 +497,7 @@ async fn async_main(args: Vec) { } token }); - let admin_token = Arc::new(admin_token); + let admin_token = Arc::new(zeroize::Zeroizing::new(admin_token)); // Spawn periodic tasks: log retention and metrics snapshot broadcast. let retention_days: u32 = std::env::var("ADMIN_LOG_RETENTION_DAYS") @@ -610,14 +610,27 @@ async fn async_main(args: Vec) { .map(|s| s.trim().to_string()) .filter(|s| !s.is_empty()) .collect(); + let webhook_queue = std::sync::Arc::new( + anyllm_batch_engine::webhook::sqlite::SqliteWebhookQueue::new(batch_db.clone()), + ); + let webhook_client = + anyllm_client::http::build_http_client(&anyllm_client::http::HttpClientConfig { + ssrf_protection: true, + connect_timeout: Some(std::time::Duration::from_secs(10)), + read_timeout: Some(std::time::Duration::from_secs(30)), + ..Default::default() + }); + let _webhook_handle = anyllm_batch_engine::webhook::dispatcher::start_dispatcher( + webhook_queue.clone(), + webhook_client, + anyllm_batch_engine::webhook::dispatcher::WebhookConfig::default(), + ); Some(std::sync::Arc::new(anyllm_batch_engine::BatchEngine { queue: std::sync::Arc::new(anyllm_batch_engine::queue::sqlite::SqliteQueue::new( batch_db.clone(), )), - file_store: anyllm_batch_engine::file_store::FileStore::new(batch_db.clone()), - webhook_queue: std::sync::Arc::new( - anyllm_batch_engine::webhook::sqlite::SqliteWebhookQueue::new(batch_db), - ), + file_store: anyllm_batch_engine::file_store::FileStore::new(batch_db), + webhook_queue, global_webhook_urls, webhook_signing_secret: std::env::var("BATCH_WEBHOOK_SIGNING_SECRET").ok(), })) diff --git a/crates/proxy/src/server/middleware.rs b/crates/proxy/src/server/middleware.rs index db9fb13..f497dd2 100644 --- a/crates/proxy/src/server/middleware.rs +++ b/crates/proxy/src/server/middleware.rs @@ -555,8 +555,14 @@ pub fn ip_allowlist_active() -> bool { /// Applied before auth so blocked IPs never reach authentication. pub async fn check_ip_allowlist(request: Request, next: Next) -> Result { // Extract client IP from X-Forwarded-For (if trusted) or connection info. - // TRUSTED_PROXY_DEPTH controls which entry to pick: depth=1 (default) takes - // the rightmost (single proxy), depth=2 takes the second-from-right (two hops), etc. + // + // XFF spoofing: attacker-controlled headers appear at the *left* of the list. + // Each hop's proxy appends the IP it received from, so the rightmost entry is + // added by our immediate (trusted) upstream. We iterate right-to-left with + // rsplit and skip (depth-1) entries to skip past our own trusted proxies. + // TRUSTED_PROXY_DEPTH=1 (default) selects the rightmost entry; depth=2 + // selects the second-from-right for a two-hop CDN -> LB topology, etc. + // Using .last() would ignore depth; using .first() would trust the attacker. let client_ip = if *TRUST_PROXY_HEADERS { let depth = *TRUSTED_PROXY_DEPTH; request diff --git a/crates/proxy/tests/virtual_keys.rs b/crates/proxy/tests/virtual_keys.rs index 41852d9..1afc9ab 100644 --- a/crates/proxy/tests/virtual_keys.rs +++ b/crates/proxy/tests/virtual_keys.rs @@ -79,7 +79,7 @@ fn test_admin_router() -> (Router, admin::state::SharedState) { state .issued_csrf_tokens .insert(TEST_CSRF_TOKEN.to_string(), ()); - let token = Arc::new("test-admin-token".to_string()); + let token = Arc::new(zeroize::Zeroizing::new("test-admin-token".to_string())); let router = admin::routes::admin_router(state.clone(), token) // ConnectInfo extractor requires the service to be wrapped with // into_make_service_with_connect_info in production. In tests we use @@ -485,7 +485,7 @@ async fn virtual_key_auth_and_revocation_lifecycle() { // Admin server uses shared VK map so create/revoke affect the same DashMap // the middleware checks. let state = shared_state(); - let admin_app = admin::routes::admin_router(state, Arc::new("admin-token".to_string())); + let admin_app = admin::routes::admin_router(state, Arc::new(zeroize::Zeroizing::new("admin-token".to_string()))); let admin_listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let admin_port = admin_listener.local_addr().unwrap().port(); let admin_url = format!("http://127.0.0.1:{admin_port}"); @@ -569,7 +569,7 @@ async fn rpm_limit_returns_429_after_exceeded() { let proxy_url = spawn_proxy_with_shared_vk(openai_config_with_base(&mock)).await; let state = shared_state(); - let admin_app = admin::routes::admin_router(state, Arc::new("admin-token2".to_string())); + let admin_app = admin::routes::admin_router(state, Arc::new(zeroize::Zeroizing::new("admin-token2".to_string()))); let admin_listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let admin_port = admin_listener.local_addr().unwrap().port(); let admin_url = format!("http://127.0.0.1:{admin_port}"); @@ -694,7 +694,7 @@ async fn spawn_test_servers(admin_token: &str) -> (String, String, u16) { let proxy_url = spawn_proxy_with_shared_vk(openai_config_with_base(&mock)).await; let state = shared_state(); - let admin_app = admin::routes::admin_router(state, Arc::new(admin_token.to_string())); + let admin_app = admin::routes::admin_router(state, Arc::new(zeroize::Zeroizing::new(admin_token.to_string()))); let admin_listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let admin_port = admin_listener.local_addr().unwrap().port(); let admin_url = format!("http://127.0.0.1:{admin_port}"); diff --git a/crates/translator/src/gemini/request.rs b/crates/translator/src/gemini/request.rs index 9724e09..273d8fd 100644 --- a/crates/translator/src/gemini/request.rs +++ b/crates/translator/src/gemini/request.rs @@ -195,9 +195,12 @@ pub struct ToolConfig { } /// Function calling mode: AUTO, NONE, or ANY. +/// When mode is ANY with `allowed_function_names`, only those functions may be called. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FunctionCallingConfig { pub mode: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub allowed_function_names: Option>, } /// Per-category safety threshold. @@ -380,7 +383,7 @@ mod tests { #[test] fn tool_config_serializes_correctly() { let tc = ToolConfig { - function_calling_config: FunctionCallingConfig { mode: "ANY".into() }, + function_calling_config: FunctionCallingConfig { mode: "ANY".into(), allowed_function_names: None }, }; let j = serde_json::to_value(&tc).unwrap(); assert_eq!(j["functionCallingConfig"]["mode"], "ANY"); diff --git a/crates/translator/src/mapping/gemini_message_map.rs b/crates/translator/src/mapping/gemini_message_map.rs index 7ba9d0a..e2ac284 100644 --- a/crates/translator/src/mapping/gemini_message_map.rs +++ b/crates/translator/src/mapping/gemini_message_map.rs @@ -75,16 +75,19 @@ pub fn anthropic_to_gemini_request( // Tool config let tool_config = req.tool_choice.as_ref().map(|tc| { - let mode = match tc { - anthropic::ToolChoice::Auto { .. } => "AUTO", - anthropic::ToolChoice::Any { .. } => "ANY", - anthropic::ToolChoice::None => "NONE", - // Gemini has no forced-specific-tool mode; fall back to AUTO. - anthropic::ToolChoice::Tool { .. } => "AUTO", + let (mode, allowed) = match tc { + anthropic::ToolChoice::Auto { .. } => ("AUTO", None), + anthropic::ToolChoice::Any { .. } => ("ANY", None), + anthropic::ToolChoice::None => ("NONE", None), + // Gemini ANY + allowedFunctionNames restricts to a specific tool. + anthropic::ToolChoice::Tool { name, .. } => { + ("ANY", Some(vec![name.clone()])) + } }; gemini::ToolConfig { function_calling_config: gemini::FunctionCallingConfig { mode: mode.to_string(), + allowed_function_names: allowed, }, } }); @@ -698,16 +701,17 @@ mod tests { } #[test] - fn tool_choice_specific_tool_maps_to_auto() { + fn tool_choice_specific_tool_maps_to_any_with_allowed_names() { let mut req = make_request(vec![user_text("test")]); req.tool_choice = Some(anthropic::ToolChoice::Tool { name: "get_weather".into(), }); let gem = anthropic_to_gemini_request(&req); - // Gemini has no forced-tool mode, so we fall back to AUTO. + let fc = gem.tool_config.unwrap().function_calling_config; + assert_eq!(fc.mode, "ANY"); assert_eq!( - gem.tool_config.unwrap().function_calling_config.mode, - "AUTO" + fc.allowed_function_names, + Some(vec!["get_weather".to_string()]) ); } diff --git a/crates/translator/src/mapping/message_map.rs b/crates/translator/src/mapping/message_map.rs index 6d32c88..6c6caac 100644 --- a/crates/translator/src/mapping/message_map.rs +++ b/crates/translator/src/mapping/message_map.rs @@ -119,11 +119,22 @@ pub fn anthropic_to_openai_request( if req.top_k.is_some() { tracing::warn!("top_k parameter dropped: no OpenAI equivalent"); } - if req.thinking.is_some() { - // Thinking config (budget_tokens) has no standard OpenAI equivalent. - // Thinking content blocks in messages ARE mapped to reasoning_content. - tracing::warn!("thinking config stripped: no standard OpenAI equivalent (thinking blocks in messages are preserved as reasoning_content)"); - } + // Map Anthropic thinking config to OpenAI reasoning_effort (via extra). + // Thinking content blocks in messages are separately mapped to reasoning_content. + let reasoning_effort = match &req.thinking { + Some(crate::anthropic::ThinkingConfig::Enabled { budget_tokens }) => { + let effort = if *budget_tokens < 4_000 { + "low" + } else if *budget_tokens < 16_000 { + "medium" + } else { + "high" + }; + tracing::info!(budget_tokens, reasoning_effort = effort, "thinking config mapped to reasoning_effort"); + Some(effort) + } + _ => None, + }; // OpenAI caps stop sequences at 4; empty array is invalid (requires 1-4 elements) let stop = req.stop_sequences.as_ref().and_then(|seqs| { @@ -179,6 +190,14 @@ pub fn anthropic_to_openai_request( extra: req.extra.clone(), }; + // Inject reasoning_effort if derived from thinking config and not already set. + if let Some(effort) = reasoning_effort { + oai_req + .extra + .entry("reasoning_effort") + .or_insert_with(|| serde_json::Value::String(effort.to_owned())); + } + // Anthropic API returns single completions only; strip n to avoid // wasting tokens on choices that get discarded (only choices[0] is used). if let Some(n_val) = oai_req.extra.remove("n") { @@ -187,14 +206,16 @@ pub fn anthropic_to_openai_request( } } - // o-series reasoning models (o1, o3, o4-mini, etc.) reject requests - // with both max_tokens and max_completion_tokens, require the - // "developer" role instead of "system", and reject non-default - // temperature/top_p values. + // o-series reasoning models require "developer" role instead of "system" + // and reject max_tokens (use max_completion_tokens instead). + // GA models (o1, o3, o3-mini) support temperature/top_p; + // preview/early models (o1-preview, o1-mini) do not. if is_o_series_model(&oai_req.model) { oai_req.max_tokens = None; - oai_req.temperature = None; - oai_req.top_p = None; + if is_o_series_no_temperature(&oai_req.model) { + oai_req.temperature = None; + oai_req.top_p = None; + } for msg in &mut oai_req.messages { if msg.role == openai::ChatRole::System { msg.role = openai::ChatRole::Developer; @@ -260,6 +281,13 @@ fn is_o_series_model(model: &str) -> bool { after_digits == bytes.len() || bytes[after_digits] == b'-' } +/// Returns true for o-series models that do NOT support temperature/top_p. +/// GA models (o1, o3, o3-mini, o4-mini) gained temperature support; +/// only the early preview/mini variants (o1-preview, o1-mini) still reject it. +fn is_o_series_no_temperature(model: &str) -> bool { + model.eq_ignore_ascii_case("o1-preview") || model.eq_ignore_ascii_case("o1-mini") +} + /// Convert a single Anthropic InputMessage into one or more OpenAI ChatMessages. /// An assistant message with tool_use blocks produces tool_calls. /// A user message with tool_result blocks produces OpenAI tool-role messages. @@ -2222,22 +2250,43 @@ mod tests { } #[test] - fn o_series_strips_temperature() { + fn o_series_ga_preserves_temperature() { + // GA models (o1, o3, o3-mini) support temperature. let mut req = make_request("o3-mini", None); req.temperature = Some(0.7); let oai = anthropic_to_openai_request(&req); assert!( - oai.temperature.is_none(), - "o-series should strip temperature" + oai.temperature.is_some(), + "GA o-series should preserve temperature" ); } #[test] - fn o_series_strips_top_p() { + fn o_series_preview_strips_temperature() { + // Early preview models (o1-preview, o1-mini) do not support temperature. + let mut req = make_request("o1-preview", None); + req.temperature = Some(0.7); + let oai = anthropic_to_openai_request(&req); + assert!( + oai.temperature.is_none(), + "o1-preview should strip temperature" + ); + } + + #[test] + fn o_series_preview_strips_top_p() { let mut req = make_request("o1-preview", None); req.top_p = Some(0.9); let oai = anthropic_to_openai_request(&req); - assert!(oai.top_p.is_none(), "o-series should strip top_p"); + assert!(oai.top_p.is_none(), "o1-preview should strip top_p"); + } + + #[test] + fn o1_mini_strips_top_p() { + let mut req = make_request("o1-mini", None); + req.top_p = Some(0.9); + let oai = anthropic_to_openai_request(&req); + assert!(oai.top_p.is_none(), "o1-mini should strip top_p"); } #[test]