gitops/optimclaw
1
0
Fork 0
mirror of https://github.com/outbackdingo/optimclaw.git synced 2026-08-25 14:53:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: address review feedback on provider config priority

Browse Source
This commit is contained in:
italic-jinxin
2026-03-24 20:52:12 +08:00
parent f13226d011
commit 08ad984701
5 changed files with 128 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/channels/web/handlers/settings.rs
+6 -3
View File
@@ -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);
}
src/channels/web/server.rs
+22 -22
View File
@@ -2603,7 +2603,10 @@ struct TestConnectionRequest {
base_url: String,
#[serde(default)]
api_key: Option<String>,
/// 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<String>,
}
@@ -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")
src/channels/web/static/app.js
+3 -3
View File
@@ -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');
});
});
src/config/llm.rs
+91 -11
View File
@@ -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();
src/settings.rs
+6 -5
View File
@@ -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\