diff --git a/src/channels/web/handlers/settings.rs b/src/channels/web/handlers/settings.rs index 43f25a0c..6d980b25 100644 --- a/src/channels/web/handlers/settings.rs +++ b/src/channels/web/handlers/settings.rs @@ -99,9 +99,12 @@ fn validate_custom_providers_adapters(value: &serde_json::Value) -> Result<(), S None => return Ok(()), }; for p in providers { - if let Some(adapter) = p.get("adapter").and_then(|v| v.as_str()) - && !VALID_ADAPTERS.contains(&adapter) - { + let adapter = p.get("adapter").and_then(|v| v.as_str()).unwrap_or(""); + if adapter.is_empty() { + tracing::warn!("Rejected custom provider with missing adapter field"); + return Err(StatusCode::UNPROCESSABLE_ENTITY); + } + if !VALID_ADAPTERS.contains(&adapter) { tracing::warn!(adapter = %adapter, "Rejected unknown LLM adapter"); return Err(StatusCode::UNPROCESSABLE_ENTITY); } diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index 586bc366..d5a4461e 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -2603,7 +2603,10 @@ struct TestConnectionRequest { base_url: String, #[serde(default)] api_key: Option, + /// Accepted for backward compatibility with frontends that still send it, + /// but no longer used since test_connection switched to `GET /models`. #[serde(default)] + #[allow(dead_code)] model: Option, } @@ -2619,6 +2622,16 @@ async fn llm_test_connection_handler( Json(test_provider_connection(body).await) } +/// Check if a base URL belongs to a NEAR AI private endpoint by verifying the +/// hostname ends with `.near.ai` and contains "private". This prevents an +/// attacker from crafting `https://evil.com/private/...` to match. +fn is_nearai_private_endpoint(base_url: &str) -> bool { + url::Url::parse(base_url) + .ok() + .and_then(|u| u.host_str().map(|h| h.to_lowercase())) + .is_some_and(|host| host.ends_with(".near.ai") && host.contains("private")) +} + async fn test_provider_connection(req: TestConnectionRequest) -> TestConnectionResponse { if let Err(e) = validate_base_url(&req.base_url, "base_url") { return TestConnectionResponse { @@ -2643,7 +2656,7 @@ async fn test_provider_connection(req: TestConnectionRequest) -> TestConnectionR let base = req.base_url.trim_end_matches('/'); match req.adapter.as_str() { - "nearai" if base.contains("private") => { + "nearai" if is_nearai_private_endpoint(base) => { // NEAR AI private endpoints use /health for connectivity checks. let url = format!("{base}/health"); let mut builder = client.get(&url); @@ -2692,37 +2705,24 @@ async fn test_provider_connection(req: TestConnectionRequest) -> TestConnectionR } } "anthropic" => { + // Use GET /v1/models to verify connectivity + auth without consuming tokens. let anthropic_base = if base.ends_with("/v1") || base.contains("/v1/") { base.to_string() } else { format!("{base}/v1") }; - let url = format!("{anthropic_base}/messages"); - let model = req.model.as_deref().unwrap_or("claude-3-haiku-20240307"); - let payload = serde_json::json!({ - "model": model, - "max_tokens": 1, - "messages": [{"role": "user", "content": "hi"}] - }); - let mut builder = client - .post(&url) - .header("anthropic-version", "2023-06-01") - .json(&payload); + let url = format!("{anthropic_base}/models"); + let mut builder = client.get(&url).header("anthropic-version", "2023-06-01"); if let Some(key) = req.api_key.as_deref().filter(|k| !k.is_empty()) { builder = builder.header("x-api-key", key); } interpret_chat_response(builder.send().await) } _ => { - // OpenAI-compatible - let url = format!("{base}/chat/completions"); - let model = req.model.as_deref().unwrap_or("gpt-4o-mini"); - let payload = serde_json::json!({ - "model": model, - "messages": [{"role": "user", "content": "hi"}], - "max_tokens": 1 - }); - let mut builder = client.post(&url).json(&payload); + // OpenAI-compatible: use GET /models to verify connectivity + auth + // without consuming tokens (no chat/completions call). + let url = format!("{base}/models"); + let mut builder = client.get(&url); if let Some(key) = req.api_key.as_deref().filter(|k| !k.is_empty()) { builder = builder.header("Authorization", format!("Bearer {key}")); } @@ -2865,7 +2865,7 @@ async fn fetch_provider_models(req: ListModelsRequest) -> ListModelsResponse { _ => { // OpenAI-compatible, Anthropic, and NEAR AI all support GET /models. // NEAR AI private endpoints and Anthropic need a /v1 prefix. - let effective_base = if (req.adapter == "nearai" && base.contains("private")) + let effective_base = if (req.adapter == "nearai" && is_nearai_private_endpoint(base)) || (req.adapter == "anthropic" && !base.ends_with("/v1") && !base.contains("/v1/")) { format!("{base}/v1") diff --git a/src/channels/web/static/app.js b/src/channels/web/static/app.js index 972aae45..9098afe6 100644 --- a/src/channels/web/static/app.js +++ b/src/channels/web/static/app.js @@ -6601,7 +6601,7 @@ document.getElementById('save-provider-btn').addEventListener('click', () => { return; } - if (!/^[a-z0-9_-]+$/.test(id)) { + if (!/^[a-z0-9-]+$/.test(id)) { showToast(I18n.t('config.providerIdInvalid'), 'error'); return; } @@ -6682,7 +6682,7 @@ document.getElementById('fetch-models-btn').addEventListener('click', () => { const btn = document.getElementById('fetch-models-btn'); btn.disabled = true; - btn.textContent = 'Fetching…'; + btn.textContent = I18n.t('config.fetchingModels'); apiFetch('/api/llm/list_models', { method: 'POST', @@ -6705,7 +6705,7 @@ document.getElementById('fetch-models-btn').addEventListener('click', () => { .catch((e) => showToast(e.message, 'error')) .finally(() => { btn.disabled = false; - btn.textContent = '↻ Fetch available models'; + btn.textContent = I18n.t('config.fetchModels'); }); }); diff --git a/src/config/llm.rs b/src/config/llm.rs index 7fceeda4..aaa73c9c 100644 --- a/src/config/llm.rs +++ b/src/config/llm.rs @@ -79,7 +79,8 @@ impl LlmConfig { ); // Warn operators when a DB-persisted value silently overrides LLM_BACKEND. if backend_source == "db:llm_backend" - && let Ok(Some(env_val)) = optional_env("LLM_BACKEND") + && let Ok(env_val) = std::env::var("LLM_BACKEND") + && !env_val.is_empty() { tracing::warn!( db_value = %backend, @@ -143,7 +144,13 @@ impl LlmConfig { .map(SecretString::from) }); let nearai = NearAiConfig { - model: Self::resolve_model("NEARAI_MODEL", settings, crate::llm::DEFAULT_MODEL)?, + // Model priority: selected_model (DB) > builtin_overrides (DB) > env > default + model: settings + .selected_model + .clone() + .or_else(|| nearai_override.and_then(|o| o.model.clone())) + .or_else(|| optional_env("NEARAI_MODEL").ok().flatten()) + .unwrap_or_else(|| crate::llm::DEFAULT_MODEL.to_string()), cheap_model: optional_env("NEARAI_CHEAP_MODEL")?, base_url: { let url = nearai_override @@ -347,6 +354,11 @@ impl LlmConfig { let base_url = custom.base_url.clone().unwrap_or_default(); if base_url.is_empty() { tracing::warn!(id = %custom.id, "Custom provider has no base_url configured — requests will fail"); + } else { + validate_base_url( + &base_url, + &format!("custom provider '{}' base_url", custom.id), + )?; } let model = settings @@ -665,7 +677,7 @@ pub fn default_session_path() -> PathBuf { #[cfg(test)] mod tests { use super::*; - use crate::config::helpers::{ENV_MUTEX, lock_env}; + use crate::config::helpers::lock_env; use crate::settings::Settings; use crate::testing::credentials::*; @@ -1383,7 +1395,7 @@ mod tests { #[test] fn custom_provider_resolves_when_backend_matches_id() { - let _guard = ENV_MUTEX.lock().expect("env mutex poisoned"); + let _guard = lock_env(); // SAFETY: Under ENV_MUTEX. unsafe { std::env::remove_var("LLM_BACKEND"); @@ -1396,7 +1408,7 @@ mod tests { id: "myprovider".to_string(), name: "My Provider".to_string(), adapter: "open_ai_completions".to_string(), - base_url: Some("https://api.example.com/v1".to_string()), + base_url: Some("http://localhost:9090/v1".to_string()), default_model: Some("my-model".to_string()), api_key: Some("sk-test".to_string()), builtin: false, @@ -1408,7 +1420,7 @@ mod tests { assert_eq!(cfg.backend, "myprovider"); let provider = cfg.provider.expect("provider config should be present"); assert_eq!(provider.provider_id, "myprovider"); - assert_eq!(provider.base_url, "https://api.example.com/v1"); + assert_eq!(provider.base_url, "http://localhost:9090/v1"); assert_eq!(provider.model, "my-model"); assert_eq!( provider.protocol, @@ -1439,7 +1451,7 @@ mod tests { id: "myprovider".to_string(), name: "My Provider".to_string(), adapter: "open_ai_completions".to_string(), - base_url: Some("https://api.example.com/v1".to_string()), + base_url: Some("http://localhost:9090/v1".to_string()), default_model: Some("my-model".to_string()), api_key: None, builtin: false, @@ -1468,7 +1480,7 @@ mod tests { #[test] fn builtin_override_model_used_when_no_selected_model() { - let _guard = ENV_MUTEX.lock().expect("env mutex poisoned"); + let _guard = lock_env(); // SAFETY: Under ENV_MUTEX. unsafe { std::env::remove_var("LLM_BACKEND"); @@ -1520,7 +1532,7 @@ mod tests { #[test] fn selected_model_takes_priority_over_builtin_override_model() { - let _guard = ENV_MUTEX.lock().expect("env mutex poisoned"); + let _guard = lock_env(); // SAFETY: Under ENV_MUTEX. unsafe { std::env::remove_var("LLM_BACKEND"); @@ -1577,7 +1589,7 @@ mod tests { #[test] fn builtin_override_api_key_used_when_no_env_var() { - let _guard = ENV_MUTEX.lock().expect("env mutex poisoned"); + let _guard = lock_env(); // SAFETY: Under ENV_MUTEX. unsafe { std::env::remove_var("LLM_BACKEND"); @@ -1810,7 +1822,7 @@ mod tests { id: "myprovider".to_string(), name: "My Provider".to_string(), adapter: "open_ai_completions".to_string(), - base_url: Some("https://api.example.com/v1".to_string()), + base_url: Some("http://localhost:9090/v1".to_string()), default_model: Some("default-model".to_string()), api_key: None, builtin: false, @@ -1886,6 +1898,74 @@ mod tests { } } + #[test] + fn nearai_override_model_wins_over_env() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::set_var("NEARAI_MODEL", "model-from-env"); + } + + let mut overrides = std::collections::HashMap::new(); + overrides.insert( + "nearai".to_string(), + crate::settings::LlmBuiltinOverride { + api_key: None, + model: Some("model-from-db-override".to_string()), + base_url: None, + }, + ); + let settings = Settings { + llm_backend: Some("nearai".to_string()), + llm_builtin_overrides: overrides, + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + assert_eq!( + cfg.nearai.model, "model-from-db-override", + "DB builtin_overrides model must take priority over NEARAI_MODEL env var" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("NEARAI_MODEL"); + } + } + + #[test] + fn nearai_selected_model_wins_over_override_model() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::remove_var("NEARAI_MODEL"); + } + + let mut overrides = std::collections::HashMap::new(); + overrides.insert( + "nearai".to_string(), + crate::settings::LlmBuiltinOverride { + api_key: None, + model: Some("model-from-override".to_string()), + base_url: None, + }, + ); + let settings = Settings { + llm_backend: Some("nearai".to_string()), + selected_model: Some("model-from-selected".to_string()), + llm_builtin_overrides: overrides, + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + assert_eq!( + cfg.nearai.model, "model-from-selected", + "selected_model must take priority over builtin_overrides model" + ); + } + #[test] fn nearai_override_base_url_wins_over_env() { let _guard = lock_env(); diff --git a/src/settings.rs b/src/settings.rs index a159f914..f0039d8e 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -1,10 +1,11 @@ //! User settings persistence. //! //! Stores user preferences in `~/.ironclaw` (JSON/TOML) and, for some values, -//! in the database. At runtime, settings are resolved using the following -//! precedence: database > environment variables > on-disk config > built-in -//! defaults. In particular, LLM backend and related settings prefer DB values -//! over environment variables. +//! in the database. Precedence between database values, environment variables, +//! on-disk config, and built-in defaults is determined on a per-setting basis +//! by the corresponding resolver. LLM provider settings (backend, model, +//! api_key, base_url) prefer DB values over environment variables, as +//! documented on their respective types. use std::collections::HashMap; use std::path::PathBuf; @@ -901,7 +902,7 @@ impl Settings { let content = format!( "# IronClaw configuration file.\n\ #\n\ - # Priority: env var > this file > database settings > defaults.\n\ + # Priority: database settings > env var > this file > defaults.\n\ # Uncomment and edit values to override defaults.\n\ # Run `ironclaw config init` to regenerate this file.\n\ #\n\