gitops/optimclaw
1
0
Fork 0
mirror of https://github.com/outbackdingo/optimclaw.git synced 2026-08-26 23:50:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(llm): address security and correctness issues in custom LLM provider

Browse Source
This commit is contained in:
italic-jinxin
2026-03-23 16:16:03 +08:00
parent 06ea64eee9
commit bc6494527b
4 changed files with 105 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/channels/web/handlers/settings.rs
+53 -1
View File
@@ -72,6 +72,7 @@ pub async fn settings_set_handler(
// Guard: cannot remove a custom provider that is currently active.
if key == "llm_custom_providers" {
guard_active_provider_not_removed(store, &state.user_id, &body.value).await?;
validate_custom_providers_adapters(&body.value)?;
}
store
@@ -85,6 +86,25 @@ pub async fn settings_set_handler(
Ok(StatusCode::NO_CONTENT)
}
const VALID_ADAPTERS: &[&str] = &["open_ai_completions", "anthropic", "ollama"];
/// Returns `Err(422)` if any provider in the incoming list has an unrecognised adapter.
fn validate_custom_providers_adapters(value: &serde_json::Value) -> Result<(), StatusCode> {
let providers = match value.as_array() {
Some(arr) => arr,
None => return Ok(()),
};
for p in providers {
if let Some(adapter) = p.get("adapter").and_then(|v| v.as_str())
&& !VALID_ADAPTERS.contains(&adapter)
{
tracing::warn!(adapter = %adapter, "Rejected unknown LLM adapter");
return Err(StatusCode::UNPROCESSABLE_ENTITY);
}
}
Ok(())
}
/// Returns `Err(409)` if the active `llm_backend` is a custom provider that
/// would be removed by the incoming update to `llm_custom_providers`.
async fn guard_active_provider_not_removed(
@@ -147,6 +167,14 @@ pub async fn settings_delete_handler(
.store
.as_ref()
.ok_or(StatusCode::SERVICE_UNAVAILABLE)?;
// Guard: deleting llm_custom_providers is equivalent to setting it to [].
// Reject if the active backend is a custom provider that would be removed.
if key == "llm_custom_providers" {
guard_active_provider_not_removed(store, &state.user_id, &serde_json::Value::Array(vec![]))
.await?;
}
store
.delete_setting(&state.user_id, &key)
.await
@@ -165,11 +193,35 @@ pub async fn settings_export_handler(
.store
.as_ref()
.ok_or(StatusCode::SERVICE_UNAVAILABLE)?;
let settings = store.get_all_settings(&state.user_id).await.map_err(|e| {
let mut settings = store.get_all_settings(&state.user_id).await.map_err(|e| {
tracing::error!("Failed to export settings: {}", e);
StatusCode::INTERNAL_SERVER_ERROR
})?;
// Redact API keys — never expose secrets over the export endpoint.
if let Some(val) = settings.get_mut("llm_custom_providers")
&& let Some(providers) = val.as_array_mut()
{
for p in providers.iter_mut() {
if let Some(obj) = p.as_object_mut()
&& obj.contains_key("api_key")
{
obj.insert("api_key".to_string(), serde_json::Value::Null);
}
}
}
if let Some(val) = settings.get_mut("llm_builtin_overrides")
&& let Some(obj) = val.as_object_mut()
{
for override_val in obj.values_mut() {
if let Some(provider_obj) = override_val.as_object_mut()
&& provider_obj.contains_key("api_key")
{
provider_obj.insert("api_key".to_string(), serde_json::Value::Null);
}
}
}
Ok(Json(SettingsExportResponse { settings }))
}
src/channels/web/server.rs
+16
View File
@@ -51,6 +51,7 @@ use crate::channels::web::log_layer::LogBroadcaster;
use crate::channels::web::sse::SseManager;
use crate::channels::web::types::*;
use crate::channels::web::util::{build_turns_from_db_messages, truncate_preview};
use crate::config::helpers::validate_base_url;
use crate::db::Database;
use crate::extensions::ExtensionManager;
use crate::orchestrator::job_manager::ContainerJobManager;
@@ -2524,6 +2525,13 @@ async fn llm_test_connection_handler(
}
async fn test_provider_connection(req: TestConnectionRequest) -> TestConnectionResponse {
if let Err(e) = validate_base_url(&req.base_url, "base_url") {
return TestConnectionResponse {
ok: false,
message: format!("Invalid base URL: {e}"),
};
}
let client = match reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(10))
.build()
@@ -2662,6 +2670,14 @@ async fn llm_list_models_handler(Json(body): Json<ListModelsRequest>) -> Json<Li
}
async fn fetch_provider_models(req: ListModelsRequest) -> ListModelsResponse {
if let Err(e) = validate_base_url(&req.base_url, "base_url") {
return ListModelsResponse {
ok: false,
models: vec![],
message: format!("Invalid base URL: {e}"),
};
}
let client = match reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(15))
.build()
src/channels/web/static/app.js
+15 -16
View File
@@ -6084,6 +6084,9 @@ document.addEventListener('keydown', function(e) {
if (e.key === 'Escape' && document.getElementById('confirm-modal').style.display === 'flex') {
closeConfirmModal();
}
if (e.key === 'Escape' && document.getElementById('provider-dialog').style.display === 'flex') {
resetProviderForm();
}
});
// --- Settings Import/Export ---
@@ -6195,8 +6198,8 @@ function apiFetchVoid(path, options) {
// Fields: id, name, adapter, base_url, builtin, default_model, api_key_required, can_list_models
// nearai/bedrock use special auth flows — no Configure button (api_key_required=false, can_list_models=false)
const BUILTIN_PROVIDERS = [
{ id: 'nearai', name: 'NEAR AI', adapter: 'nearai', base_url: 'https://private-chat-stg.near.ai/v1', builtin: true, default_model: 'zai-org/GLM-5-FP8', api_key_required: true, can_list_models: true },
{ id: 'openai', name: 'OpenAI', adapter: 'open_ai_completions', base_url: 'https://api.openai.com/v1', builtin: true, default_model: 'gpt-5-mini', api_key_required: true, can_list_models: true },
{ id: 'nearai', name: 'NEAR AI', adapter: 'nearai', base_url: 'https://cloud-api.near.ai', builtin: true, default_model: 'zai-org/GLM-5-FP8', api_key_required: true, can_list_models: true },
{ id: 'openai', name: 'OpenAI', adapter: 'open_ai_completions', base_url: 'https://api.openai.com/v1', builtin: true, default_model: 'gpt-4o-mini', api_key_required: true, can_list_models: true },
{ id: 'anthropic', name: 'Anthropic', adapter: 'anthropic', base_url: 'https://api.anthropic.com', builtin: true, default_model: 'claude-sonnet-4-20250514', api_key_required: true, can_list_models: true },
{ id: 'ollama', name: 'Ollama', adapter: 'ollama', base_url: 'http://localhost:11434', builtin: true, default_model: 'llama3', api_key_required: false, can_list_models: true },
{ id: 'openai_compatible', name: 'OpenAI Compatible', adapter: 'open_ai_completions', base_url: '', builtin: true, default_model: 'default', api_key_required: false, can_list_models: false },
@@ -6296,37 +6299,37 @@ function renderProviders() {
? '<span class="provider-badge provider-badge-builtin">' + I18n.t('config.builtin') + '</span>'
: '';
const deleteBtn = !p.builtin && !isActive
? '<button class="provider-action-btn provider-delete-btn" data-action="delete-custom-provider" data-id="' + escHtml(p.id) + '">' + I18n.t('common.delete') + '</button>'
? '<button class="provider-action-btn provider-delete-btn" data-action="delete-custom-provider" data-id="' + escapeHtml(p.id) + '">' + I18n.t('common.delete') + '</button>'
: '';
const editBtn = !p.builtin
? '<button class="provider-action-btn" data-action="edit-custom-provider" data-id="' + escHtml(p.id) + '">' + I18n.t('common.edit') + '</button>'
? '<button class="provider-action-btn" data-action="edit-custom-provider" data-id="' + escapeHtml(p.id) + '">' + I18n.t('common.edit') + '</button>'
: '';
// Show Configure for built-in providers that support it (not bedrock — uses AWS credential chain)
const configureBtn = p.builtin && p.id !== 'bedrock'
? '<button class="provider-action-btn" data-action="configure-builtin-provider" data-id="' + escHtml(p.id) + '">' + I18n.t('config.configureProvider') + '</button>'
? '<button class="provider-action-btn" data-action="configure-builtin-provider" data-id="' + escapeHtml(p.id) + '">' + I18n.t('config.configureProvider') + '</button>'
: '';
const useBtn = !isActive
? '<button class="provider-action-btn" data-action="set-active-provider" data-id="' + escHtml(p.id) + '">' + I18n.t('config.useProvider') + '</button>'
? '<button class="provider-action-btn" data-action="set-active-provider" data-id="' + escapeHtml(p.id) + '">' + I18n.t('config.useProvider') + '</button>'
: '';
const baseUrlText = p.base_url
? '<span class="provider-url">' + escHtml(p.base_url) + '</span>'
? '<span class="provider-url">' + escapeHtml(p.base_url) + '</span>'
: '';
// Show configured model: for active provider use _selectedModel, for others check _builtinOverrides
const displayModel = isActive
? _selectedModel
: (p.builtin && _builtinOverrides[p.id] ? (_builtinOverrides[p.id].model || '') : '');
const modelText = displayModel
? '<span class="provider-current-model">' + escHtml(I18n.t('config.currentModel', { model: displayModel })) + '</span>'
? '<span class="provider-current-model">' + escapeHtml(I18n.t('config.currentModel', { model: displayModel })) + '</span>'
: '';
return '<div class="provider-card' + (isActive ? ' provider-card-active' : '') + '">'
+ '<div class="provider-card-header">'
+ '<span class="provider-name">' + escHtml(p.name || p.id) + '</span>'
+ '<span class="provider-id-label">' + escHtml(p.id) + '</span>'
+ '<span class="provider-name">' + escapeHtml(p.name || p.id) + '</span>'
+ '<span class="provider-id-label">' + escapeHtml(p.id) + '</span>'
+ activeBadge + builtinBadge
+ '</div>'
+ '<div class="provider-card-meta">'
+ '<span class="provider-adapter">' + escHtml(adapterLabel) + '</span>'
+ '<span class="provider-adapter">' + escapeHtml(adapterLabel) + '</span>'
+ baseUrlText
+ modelText
+ '</div>'
@@ -6337,10 +6340,6 @@ function renderProviders() {
}).join('');
}
function escHtml(s) {
return String(s).replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;');
}
function setActiveProvider(id) {
const provider = [...BUILTIN_PROVIDERS, ..._customProviders].find((p) => p.id === id);
// Restore the last-configured model for this provider, falling back to the provider's default
@@ -6678,7 +6677,7 @@ document.getElementById('fetch-models-btn').addEventListener('click', () => {
if (data.ok && data.models && data.models.length > 0) {
const currentModel = document.getElementById('provider-model').value;
select.innerHTML = data.models
.map((m) => `<option value="${escHtml(m)}"${m === currentModel ? ' selected' : ''}>${escHtml(m)}</option>`)
.map((m) => `<option value="${escapeHtml(m)}"${m === currentModel ? ' selected' : ''}>${escapeHtml(m)}</option>`)
.join('');
select.style.display = '';
btn.style.display = 'none';
src/config/llm.rs
+21 -6
View File
@@ -75,6 +75,17 @@ impl LlmConfig {
custom_providers_count = settings.llm_custom_providers.len(),
"Resolving LLM backend"
);
// 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")
{
tracing::warn!(
db_value = %backend,
env_value = %env_val,
"LLM_BACKEND env var is set but DB setting takes priority. \
Unset llm_backend in the DB (via settings UI) to use the env var."
);
}
// Validate the backend is known
let backend_lower = backend.to_lowercase();
@@ -1353,8 +1364,16 @@ mod tests {
#[test]
fn db_llm_backend_takes_priority_over_env_var() {
let _guard = ENV_MUTEX.lock().expect("env mutex poisoned");
// SAFETY: Under ENV_MUTEX.
let _guard = lock_env();
// SAFETY: Under ENV_MUTEX. RAII guard removes LLM_BACKEND on drop so
// a panicking assertion cannot leak the env var to other tests.
struct RemoveOnDrop(&'static str);
impl Drop for RemoveOnDrop {
fn drop(&mut self) {
unsafe { std::env::remove_var(self.0) };
}
}
let _cleanup = RemoveOnDrop("LLM_BACKEND");
unsafe {
std::env::set_var("LLM_BACKEND", "nearai");
std::env::remove_var("LLM_MODEL");
@@ -1379,10 +1398,6 @@ mod tests {
cfg.backend, "myprovider",
"DB setting should override LLM_BACKEND env var"
);
// SAFETY: Under ENV_MUTEX.
unsafe {
std::env::remove_var("LLM_BACKEND");
}
}
// ── OpenAI Codex tests ──────────────────────────────────────────