mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 15:40:18 +00:00
fix(security): store LLM API keys in encrypted secrets store instead of plaintext
This commit is contained in:
+20
-3
@@ -229,18 +229,35 @@ impl AppBuilder {
|
||||
let store = crate::secrets::create_secrets_store(crypto, handles);
|
||||
|
||||
if let Some(ref secrets) = store {
|
||||
// Migrate any plaintext API keys from the settings table to the
|
||||
// encrypted secrets store. Idempotent — safe to run on every startup.
|
||||
if let Some(ref db) = self.db {
|
||||
crate::config::migrate_plaintext_llm_keys(
|
||||
db.as_ref(),
|
||||
secrets.as_ref(),
|
||||
&self.config.owner_id,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
// Inject LLM API keys from encrypted storage
|
||||
crate::config::inject_llm_keys_from_secrets(secrets.as_ref(), &self.config.owner_id)
|
||||
.await;
|
||||
|
||||
// Re-resolve only the LLM config with newly available keys.
|
||||
let store: Option<&(dyn crate::db::SettingsStore + Sync)> =
|
||||
// Re-resolve only the LLM config with newly available keys,
|
||||
// including keys hydrated from the secrets store.
|
||||
let settings_store: Option<&(dyn crate::db::SettingsStore + Sync)> =
|
||||
self.db.as_ref().map(|db| db.as_ref() as _);
|
||||
let toml_path = self.toml_path.as_deref();
|
||||
let owner_id = self.config.owner_id.clone();
|
||||
if let Err(e) = self
|
||||
.config
|
||||
.re_resolve_llm(store, &owner_id, toml_path)
|
||||
.re_resolve_llm_with_secrets(
|
||||
settings_store,
|
||||
&owner_id,
|
||||
toml_path,
|
||||
Some(secrets.as_ref()),
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::warn!("Failed to re-resolve LLM config after secret injection: {e}");
|
||||
|
||||
@@ -7,10 +7,15 @@ use axum::{
|
||||
extract::{Path, State},
|
||||
http::StatusCode,
|
||||
};
|
||||
use secrecy::SecretString;
|
||||
|
||||
use crate::channels::web::auth::AuthenticatedUser;
|
||||
use crate::channels::web::server::GatewayState;
|
||||
use crate::channels::web::types::*;
|
||||
use crate::secrets::{CreateSecretParams, SecretsStore};
|
||||
|
||||
/// Sentinel value the frontend sends to mean "key is unchanged, don't touch it".
|
||||
const API_KEY_UNCHANGED: &str = "••••••••";
|
||||
|
||||
pub async fn settings_list_handler(
|
||||
State(state): State<Arc<GatewayState>>,
|
||||
@@ -79,8 +84,20 @@ pub async fn settings_set_handler(
|
||||
validate_custom_providers_adapters(&body.value)?;
|
||||
}
|
||||
|
||||
// Extract API keys from LLM settings and vault them in the secrets store.
|
||||
// The sanitized value has api_key fields removed (stored encrypted instead).
|
||||
let sanitized_value = match key.as_str() {
|
||||
"llm_builtin_overrides" => {
|
||||
extract_builtin_override_keys(&state, &user.user_id, &body.value).await?
|
||||
}
|
||||
"llm_custom_providers" => {
|
||||
extract_custom_provider_keys(&state, &user.user_id, &body.value).await?
|
||||
}
|
||||
_ => body.value.clone(),
|
||||
};
|
||||
|
||||
store
|
||||
.set_setting(&user.user_id, &key, &body.value)
|
||||
.set_setting(&user.user_id, &key, &sanitized_value)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!("Failed to set setting '{}': {}", key, e);
|
||||
@@ -202,11 +219,16 @@ pub async fn settings_export_handler(
|
||||
.store
|
||||
.as_ref()
|
||||
.ok_or(StatusCode::SERVICE_UNAVAILABLE)?;
|
||||
let settings = store.get_all_settings(&user.user_id).await.map_err(|e| {
|
||||
let mut settings = store.get_all_settings(&user.user_id).await.map_err(|e| {
|
||||
tracing::error!("Failed to export settings: {}", e);
|
||||
StatusCode::INTERNAL_SERVER_ERROR
|
||||
})?;
|
||||
|
||||
// Indicate key presence from secrets store without exposing values.
|
||||
annotate_secret_key_presence(&state, &user.user_id, &mut settings).await;
|
||||
|
||||
mask_settings_api_keys(&mut settings);
|
||||
|
||||
Ok(Json(SettingsExportResponse { settings }))
|
||||
}
|
||||
|
||||
@@ -229,3 +251,462 @@ pub async fn settings_import_handler(
|
||||
|
||||
Ok(StatusCode::NO_CONTENT)
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// LLM API key vaulting helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// Canonical secret name for a built-in provider's API key.
|
||||
fn builtin_secret_name(provider_id: &str) -> String {
|
||||
format!("llm_builtin_{}_api_key", provider_id)
|
||||
}
|
||||
|
||||
/// Canonical secret name for a custom provider's API key.
|
||||
fn custom_secret_name(provider_id: &str) -> String {
|
||||
format!("llm_custom_{}_api_key", provider_id)
|
||||
}
|
||||
|
||||
/// Extract API keys from builtin overrides, store in secrets, return sanitized JSON.
|
||||
async fn extract_builtin_override_keys(
|
||||
state: &GatewayState,
|
||||
user_id: &str,
|
||||
value: &serde_json::Value,
|
||||
) -> Result<serde_json::Value, StatusCode> {
|
||||
let secrets = match state.secrets_store.as_ref() {
|
||||
Some(s) => s,
|
||||
None => return Ok(value.clone()),
|
||||
};
|
||||
|
||||
let obj = match value.as_object() {
|
||||
Some(o) => o,
|
||||
None => return Ok(value.clone()),
|
||||
};
|
||||
|
||||
let mut sanitized = obj.clone();
|
||||
|
||||
for (provider_id, override_val) in obj {
|
||||
if let Some(api_key) = override_val.get("api_key").and_then(|v| v.as_str()) {
|
||||
if api_key == API_KEY_UNCHANGED || api_key.is_empty() {
|
||||
// Unchanged or empty — remove from settings, keep existing secret.
|
||||
if let Some(o) = sanitized
|
||||
.get_mut(provider_id)
|
||||
.and_then(|v| v.as_object_mut())
|
||||
{
|
||||
o.remove("api_key");
|
||||
}
|
||||
continue;
|
||||
}
|
||||
vault_secret(
|
||||
secrets.as_ref(),
|
||||
user_id,
|
||||
&builtin_secret_name(provider_id),
|
||||
api_key,
|
||||
provider_id,
|
||||
)
|
||||
.await?;
|
||||
if let Some(o) = sanitized
|
||||
.get_mut(provider_id)
|
||||
.and_then(|v| v.as_object_mut())
|
||||
{
|
||||
o.remove("api_key");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(serde_json::Value::Object(sanitized))
|
||||
}
|
||||
|
||||
/// Extract API keys from custom providers, store in secrets, return sanitized JSON.
|
||||
async fn extract_custom_provider_keys(
|
||||
state: &GatewayState,
|
||||
user_id: &str,
|
||||
value: &serde_json::Value,
|
||||
) -> Result<serde_json::Value, StatusCode> {
|
||||
let secrets = match state.secrets_store.as_ref() {
|
||||
Some(s) => s,
|
||||
None => return Ok(value.clone()),
|
||||
};
|
||||
|
||||
let arr = match value.as_array() {
|
||||
Some(a) => a,
|
||||
None => return Ok(value.clone()),
|
||||
};
|
||||
|
||||
let mut sanitized = arr.clone();
|
||||
|
||||
for (idx, provider_val) in arr.iter().enumerate() {
|
||||
let provider_id = provider_val
|
||||
.get("id")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("");
|
||||
if provider_id.is_empty() {
|
||||
continue;
|
||||
}
|
||||
|
||||
if let Some(api_key) = provider_val.get("api_key").and_then(|v| v.as_str()) {
|
||||
if api_key == API_KEY_UNCHANGED || api_key.is_empty() {
|
||||
if let Some(o) = sanitized[idx].as_object_mut() {
|
||||
o.remove("api_key");
|
||||
}
|
||||
continue;
|
||||
}
|
||||
vault_secret(
|
||||
secrets.as_ref(),
|
||||
user_id,
|
||||
&custom_secret_name(provider_id),
|
||||
api_key,
|
||||
provider_id,
|
||||
)
|
||||
.await?;
|
||||
if let Some(o) = sanitized[idx].as_object_mut() {
|
||||
o.remove("api_key");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(serde_json::Value::Array(sanitized))
|
||||
}
|
||||
|
||||
/// Encrypt and store an API key in the secrets store.
|
||||
async fn vault_secret(
|
||||
secrets: &(dyn SecretsStore + Send + Sync),
|
||||
user_id: &str,
|
||||
secret_name: &str,
|
||||
api_key: &str,
|
||||
provider_id: &str,
|
||||
) -> Result<(), StatusCode> {
|
||||
secrets
|
||||
.create(
|
||||
user_id,
|
||||
CreateSecretParams {
|
||||
name: secret_name.to_string(),
|
||||
value: SecretString::from(api_key.to_string()),
|
||||
provider: Some(provider_id.to_string()),
|
||||
expires_at: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!(
|
||||
"Failed to store secret '{}' for provider '{}': {}",
|
||||
secret_name,
|
||||
provider_id,
|
||||
e
|
||||
);
|
||||
StatusCode::INTERNAL_SERVER_ERROR
|
||||
})?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Mask plaintext API keys in settings values before returning to the frontend.
|
||||
///
|
||||
/// Any `api_key` field still present in the settings JSON (legacy plaintext)
|
||||
/// is replaced with the sentinel so the frontend shows "key configured".
|
||||
fn mask_settings_api_keys(settings: &mut std::collections::HashMap<String, serde_json::Value>) {
|
||||
if let Some(obj) = settings
|
||||
.get_mut("llm_builtin_overrides")
|
||||
.and_then(|v| v.as_object_mut())
|
||||
{
|
||||
for override_val in obj.values_mut() {
|
||||
if let Some(o) = override_val.as_object_mut()
|
||||
&& o.contains_key("api_key")
|
||||
{
|
||||
o.insert(
|
||||
"api_key".to_string(),
|
||||
serde_json::Value::String(API_KEY_UNCHANGED.to_string()),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(arr) = settings
|
||||
.get_mut("llm_custom_providers")
|
||||
.and_then(|v| v.as_array_mut())
|
||||
{
|
||||
for provider_val in arr.iter_mut() {
|
||||
if let Some(o) = provider_val.as_object_mut()
|
||||
&& o.contains_key("api_key")
|
||||
{
|
||||
o.insert(
|
||||
"api_key".to_string(),
|
||||
serde_json::Value::String(API_KEY_UNCHANGED.to_string()),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Check the secrets store for vaulted API keys and annotate the settings map.
|
||||
///
|
||||
/// For builtin overrides and custom providers whose API key was stripped from
|
||||
/// settings (stored in secrets), this adds `api_key: "••••••••"` so the
|
||||
/// frontend knows a key is configured without seeing the actual value.
|
||||
async fn annotate_secret_key_presence(
|
||||
state: &GatewayState,
|
||||
user_id: &str,
|
||||
settings: &mut std::collections::HashMap<String, serde_json::Value>,
|
||||
) {
|
||||
let secrets = match state.secrets_store.as_ref() {
|
||||
Some(s) => s,
|
||||
None => return,
|
||||
};
|
||||
|
||||
// Annotate builtin overrides
|
||||
if let Some(obj) = settings
|
||||
.get_mut("llm_builtin_overrides")
|
||||
.and_then(|v| v.as_object_mut())
|
||||
{
|
||||
let provider_ids: Vec<String> = obj.keys().cloned().collect();
|
||||
for provider_id in provider_ids {
|
||||
let has_key_in_settings = obj
|
||||
.get(&provider_id)
|
||||
.and_then(|v| v.get("api_key"))
|
||||
.is_some();
|
||||
if has_key_in_settings {
|
||||
continue; // Will be masked by mask_settings_api_keys
|
||||
}
|
||||
let secret_name = builtin_secret_name(&provider_id);
|
||||
if secrets.exists(user_id, &secret_name).await.unwrap_or(false)
|
||||
&& let Some(o) = obj.get_mut(&provider_id).and_then(|v| v.as_object_mut())
|
||||
{
|
||||
o.insert(
|
||||
"api_key".to_string(),
|
||||
serde_json::Value::String(API_KEY_UNCHANGED.to_string()),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Annotate custom providers
|
||||
if let Some(arr) = settings
|
||||
.get_mut("llm_custom_providers")
|
||||
.and_then(|v| v.as_array_mut())
|
||||
{
|
||||
for provider_val in arr.iter_mut() {
|
||||
let provider_id = provider_val
|
||||
.get("id")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("")
|
||||
.to_string();
|
||||
if provider_id.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let has_key_in_settings = provider_val.get("api_key").is_some();
|
||||
if has_key_in_settings {
|
||||
continue;
|
||||
}
|
||||
let secret_name = custom_secret_name(&provider_id);
|
||||
if secrets.exists(user_id, &secret_name).await.unwrap_or(false)
|
||||
&& let Some(o) = provider_val.as_object_mut()
|
||||
{
|
||||
o.insert(
|
||||
"api_key".to_string(),
|
||||
serde_json::Value::String(API_KEY_UNCHANGED.to_string()),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::collections::HashMap;
|
||||
|
||||
#[test]
|
||||
fn test_mask_settings_api_keys_builtin_overrides() {
|
||||
let mut settings = HashMap::new();
|
||||
settings.insert(
|
||||
"llm_builtin_overrides".to_string(),
|
||||
serde_json::json!({
|
||||
"openai": { "api_key": "sk-secret-123", "model": "gpt-4" },
|
||||
"anthropic": { "model": "claude-3" }
|
||||
}),
|
||||
);
|
||||
|
||||
mask_settings_api_keys(&mut settings);
|
||||
|
||||
let overrides = settings["llm_builtin_overrides"].as_object().unwrap();
|
||||
assert_eq!(
|
||||
overrides["openai"]["api_key"].as_str().unwrap(),
|
||||
API_KEY_UNCHANGED,
|
||||
);
|
||||
assert_eq!(overrides["openai"]["model"].as_str().unwrap(), "gpt-4");
|
||||
assert!(overrides["anthropic"].get("api_key").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mask_settings_api_keys_custom_providers() {
|
||||
let mut settings = HashMap::new();
|
||||
settings.insert(
|
||||
"llm_custom_providers".to_string(),
|
||||
serde_json::json!([
|
||||
{ "id": "my-llm", "api_key": "secret-key", "adapter": "open_ai_completions" },
|
||||
{ "id": "no-key", "adapter": "ollama" }
|
||||
]),
|
||||
);
|
||||
|
||||
mask_settings_api_keys(&mut settings);
|
||||
|
||||
let providers = settings["llm_custom_providers"].as_array().unwrap();
|
||||
assert_eq!(providers[0]["api_key"].as_str().unwrap(), API_KEY_UNCHANGED,);
|
||||
assert!(providers[1].get("api_key").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_mask_settings_no_llm_keys_is_noop() {
|
||||
let mut settings = HashMap::new();
|
||||
settings.insert("some_other_setting".to_string(), serde_json::json!("value"));
|
||||
|
||||
mask_settings_api_keys(&mut settings);
|
||||
|
||||
assert_eq!(settings["some_other_setting"].as_str().unwrap(), "value");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_builtin_secret_name_format() {
|
||||
assert_eq!(builtin_secret_name("openai"), "llm_builtin_openai_api_key");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_custom_secret_name_format() {
|
||||
assert_eq!(custom_secret_name("my-groq"), "llm_custom_my-groq_api_key");
|
||||
}
|
||||
|
||||
fn test_secrets_store() -> Arc<dyn SecretsStore + Send + Sync> {
|
||||
let crypto = Arc::new(
|
||||
crate::secrets::SecretsCrypto::new(secrecy::SecretString::from(
|
||||
crate::secrets::keychain::generate_master_key_hex(),
|
||||
))
|
||||
.unwrap(),
|
||||
);
|
||||
Arc::new(crate::secrets::InMemorySecretsStore::new(crypto))
|
||||
}
|
||||
|
||||
fn test_gateway_state(secrets: Arc<dyn SecretsStore + Send + Sync>) -> GatewayState {
|
||||
GatewayState {
|
||||
msg_tx: tokio::sync::RwLock::new(None),
|
||||
sse: Arc::new(crate::channels::web::sse::SseManager::new()),
|
||||
workspace: None,
|
||||
workspace_pool: None,
|
||||
session_manager: None,
|
||||
log_broadcaster: None,
|
||||
log_level_handle: None,
|
||||
extension_manager: None,
|
||||
tool_registry: None,
|
||||
store: None,
|
||||
job_manager: None,
|
||||
prompt_queue: None,
|
||||
scheduler: None,
|
||||
default_user_id: "test".to_string(),
|
||||
shutdown_tx: tokio::sync::RwLock::new(None),
|
||||
ws_tracker: None,
|
||||
llm_provider: None,
|
||||
skill_registry: None,
|
||||
skill_catalog: None,
|
||||
chat_rate_limiter: crate::channels::web::server::PerUserRateLimiter::new(30, 60),
|
||||
oauth_rate_limiter: crate::channels::web::server::RateLimiter::new(10, 60),
|
||||
webhook_rate_limiter: crate::channels::web::server::RateLimiter::new(10, 60),
|
||||
registry_entries: Vec::new(),
|
||||
cost_guard: None,
|
||||
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
|
||||
startup_time: std::time::Instant::now(),
|
||||
active_config: crate::channels::web::server::ActiveConfigSnapshot::default(),
|
||||
secrets_store: Some(secrets),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_extract_builtin_keys_vaults_and_strips() {
|
||||
let secrets = test_secrets_store();
|
||||
let state = test_gateway_state(Arc::clone(&secrets));
|
||||
|
||||
let input = serde_json::json!({
|
||||
"openai": { "api_key": "sk-test-key", "model": "gpt-4" },
|
||||
"anthropic": { "model": "claude-3" }
|
||||
});
|
||||
|
||||
let result = extract_builtin_override_keys(&state, "test", &input)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let obj = result.as_object().unwrap();
|
||||
assert!(
|
||||
obj["openai"].get("api_key").is_none(),
|
||||
"api_key should be stripped"
|
||||
);
|
||||
assert_eq!(obj["openai"]["model"].as_str().unwrap(), "gpt-4");
|
||||
assert_eq!(obj["anthropic"]["model"].as_str().unwrap(), "claude-3");
|
||||
|
||||
let decrypted = secrets
|
||||
.get_decrypted("test", "llm_builtin_openai_api_key")
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(decrypted.expose(), "sk-test-key");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_extract_custom_keys_vaults_and_strips() {
|
||||
let secrets = test_secrets_store();
|
||||
let state = test_gateway_state(Arc::clone(&secrets));
|
||||
|
||||
let input = serde_json::json!([
|
||||
{ "id": "my-llm", "api_key": "gsk-custom-key", "adapter": "open_ai_completions" },
|
||||
{ "id": "local", "adapter": "ollama" }
|
||||
]);
|
||||
|
||||
let result = extract_custom_provider_keys(&state, "test", &input)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let arr = result.as_array().unwrap();
|
||||
assert!(
|
||||
arr[0].get("api_key").is_none(),
|
||||
"api_key should be stripped"
|
||||
);
|
||||
assert_eq!(arr[0]["id"].as_str().unwrap(), "my-llm");
|
||||
assert!(arr[1].get("api_key").is_none());
|
||||
|
||||
let decrypted = secrets
|
||||
.get_decrypted("test", "llm_custom_my-llm_api_key")
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(decrypted.expose(), "gsk-custom-key");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_unchanged_sentinel_preserves_existing_secret() {
|
||||
let secrets = test_secrets_store();
|
||||
|
||||
secrets
|
||||
.create(
|
||||
"test",
|
||||
CreateSecretParams {
|
||||
name: "llm_builtin_openai_api_key".to_string(),
|
||||
value: SecretString::from("sk-original".to_string()),
|
||||
provider: Some("openai".to_string()),
|
||||
expires_at: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let state = test_gateway_state(Arc::clone(&secrets));
|
||||
|
||||
let input = serde_json::json!({
|
||||
"openai": { "api_key": "••••••••", "model": "gpt-4" }
|
||||
});
|
||||
|
||||
let result = extract_builtin_override_keys(&state, "test", &input)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(result["openai"].get("api_key").is_none());
|
||||
|
||||
let decrypted = secrets
|
||||
.get_decrypted("test", "llm_builtin_openai_api_key")
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(decrypted.expose(), "sk-original");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -112,6 +112,7 @@ impl GatewayChannel {
|
||||
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
|
||||
startup_time: std::time::Instant::now(),
|
||||
active_config: server::ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
});
|
||||
|
||||
Self {
|
||||
@@ -151,6 +152,7 @@ impl GatewayChannel {
|
||||
startup_time: std::time::Instant::now(),
|
||||
webhook_rate_limiter: server::RateLimiter::new(10, 60),
|
||||
active_config: server::ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
});
|
||||
|
||||
Self {
|
||||
@@ -191,6 +193,7 @@ impl GatewayChannel {
|
||||
routine_engine: Arc::clone(&self.state.routine_engine),
|
||||
startup_time: self.state.startup_time,
|
||||
active_config: self.state.active_config.clone(),
|
||||
secrets_store: self.state.secrets_store.clone(),
|
||||
};
|
||||
mutate(&mut new_state);
|
||||
self.state = Arc::new(new_state);
|
||||
@@ -308,6 +311,15 @@ impl GatewayChannel {
|
||||
self
|
||||
}
|
||||
|
||||
/// Inject the secrets store for encrypting LLM API keys in settings handlers.
|
||||
pub fn with_secrets_store(
|
||||
mut self,
|
||||
ss: Arc<dyn crate::secrets::SecretsStore + Send + Sync>,
|
||||
) -> Self {
|
||||
self.rebuild_state(|s| s.secrets_store = Some(ss));
|
||||
self
|
||||
}
|
||||
|
||||
/// Inject the per-user workspace pool for multi-user mode.
|
||||
pub fn with_workspace_pool(mut self, pool: Arc<server::WorkspacePool>) -> Self {
|
||||
self.rebuild_state(|s| s.workspace_pool = Some(pool));
|
||||
|
||||
@@ -381,6 +381,8 @@ pub struct GatewayState {
|
||||
pub startup_time: std::time::Instant,
|
||||
/// Snapshot of active (resolved) configuration for the frontend.
|
||||
pub active_config: ActiveConfigSnapshot,
|
||||
/// Secrets store for encrypting LLM API keys (and future sensitive settings).
|
||||
pub secrets_store: Option<Arc<dyn crate::secrets::SecretsStore + Send + Sync>>,
|
||||
}
|
||||
|
||||
/// Start the gateway HTTP server.
|
||||
@@ -2955,9 +2957,11 @@ async fn llm_env_defaults_handler() -> Json<serde_json::Value> {
|
||||
// NEAR AI is a special case (not in the registry)
|
||||
{
|
||||
let mut entry = serde_json::Map::new();
|
||||
if let Some(key) = read_env("NEARAI_API_KEY") {
|
||||
entry.insert("api_key".to_string(), serde_json::Value::String(key));
|
||||
}
|
||||
// Only expose presence of API key, never the value itself.
|
||||
entry.insert(
|
||||
"has_api_key".to_string(),
|
||||
serde_json::Value::Bool(read_env("NEARAI_API_KEY").is_some()),
|
||||
);
|
||||
if let Some(model) = read_env("NEARAI_MODEL") {
|
||||
entry.insert("model".to_string(), serde_json::Value::String(model));
|
||||
}
|
||||
@@ -2971,10 +2975,11 @@ async fn llm_env_defaults_handler() -> Json<serde_json::Value> {
|
||||
for def in registry.all() {
|
||||
let mut entry = serde_json::Map::new();
|
||||
|
||||
if let Some(ref api_key_env) = def.api_key_env
|
||||
&& let Some(key) = read_env(api_key_env)
|
||||
{
|
||||
entry.insert("api_key".to_string(), serde_json::Value::String(key));
|
||||
if let Some(ref api_key_env) = def.api_key_env {
|
||||
entry.insert(
|
||||
"has_api_key".to_string(),
|
||||
serde_json::Value::Bool(read_env(api_key_env).is_some()),
|
||||
);
|
||||
}
|
||||
|
||||
if let Some(model) = read_env(&def.model_env) {
|
||||
@@ -3257,9 +3262,14 @@ mod tests {
|
||||
.get("nearai")
|
||||
.and_then(|v| v.as_object())
|
||||
.expect("nearai entry");
|
||||
// API key should NOT be exposed — only has_api_key presence flag.
|
||||
assert_eq!(
|
||||
nearai.get("api_key").and_then(|v| v.as_str()),
|
||||
Some("test-key-123")
|
||||
nearai.get("has_api_key").and_then(|v| v.as_bool()),
|
||||
Some(true)
|
||||
);
|
||||
assert!(
|
||||
nearai.get("api_key").is_none(),
|
||||
"raw api_key must never be returned"
|
||||
);
|
||||
assert_eq!(
|
||||
nearai.get("model").and_then(|v| v.as_str()),
|
||||
@@ -3326,6 +3336,7 @@ mod tests {
|
||||
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
|
||||
startup_time: std::time::Instant::now(),
|
||||
active_config: ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -6374,7 +6374,14 @@ function editCustomProvider(id) {
|
||||
idField.style.opacity = '0.6';
|
||||
document.getElementById('provider-adapter').value = p.adapter || 'open_ai_completions';
|
||||
document.getElementById('provider-base-url').value = p.base_url || '';
|
||||
document.getElementById('provider-api-key').value = p.api_key || '';
|
||||
const editApiKeyInput = document.getElementById('provider-api-key');
|
||||
if (p.api_key === '••••••••') {
|
||||
editApiKeyInput.value = '';
|
||||
editApiKeyInput.placeholder = 'Key configured (leave blank to keep)';
|
||||
} else {
|
||||
editApiKeyInput.value = '';
|
||||
editApiKeyInput.placeholder = 'Enter API key';
|
||||
}
|
||||
document.getElementById('provider-model').value = p.default_model || '';
|
||||
openProviderDialog(true);
|
||||
document.getElementById('provider-name').focus();
|
||||
@@ -6404,8 +6411,16 @@ function configureBuiltinProvider(id) {
|
||||
document.getElementById('provider-api-key-row').style.display = p.api_key_required !== false ? '' : 'none';
|
||||
document.getElementById('fetch-models-btn').style.display = p.can_list_models ? '' : 'none';
|
||||
const apiKeyInput = document.getElementById('provider-api-key');
|
||||
apiKeyInput.value = override.api_key || envDef.api_key || '';
|
||||
apiKeyInput.placeholder = '';
|
||||
const hasDbKey = override.api_key === '••••••••';
|
||||
const hasEnvKey = envDef.has_api_key === true;
|
||||
apiKeyInput.value = '';
|
||||
if (hasDbKey) {
|
||||
apiKeyInput.placeholder = 'Key configured (leave blank to keep)';
|
||||
} else if (hasEnvKey) {
|
||||
apiKeyInput.placeholder = 'Key set via environment variable';
|
||||
} else {
|
||||
apiKeyInput.placeholder = 'Enter API key';
|
||||
}
|
||||
document.getElementById('provider-model').value = override.model || envDef.model || p.default_model || '';
|
||||
openProviderDialog(true);
|
||||
document.getElementById('provider-model').focus();
|
||||
@@ -6495,8 +6510,15 @@ document.getElementById('save-provider-btn').addEventListener('click', () => {
|
||||
const model = document.getElementById('provider-model').value.trim();
|
||||
const baseUrl = document.getElementById('provider-base-url').value.trim();
|
||||
const id = _configuringBuiltinId;
|
||||
const prevOverride = _builtinOverrides[id] || {};
|
||||
const hadKey = prevOverride.api_key === '••••••••';
|
||||
const override = {};
|
||||
if (apiKey) override.api_key = apiKey;
|
||||
if (apiKey) {
|
||||
override.api_key = apiKey; // New key entered — backend will encrypt it
|
||||
} else if (hadKey) {
|
||||
override.api_key = '••••••••'; // Sentinel: keep existing encrypted key
|
||||
}
|
||||
// If neither — key is cleared (no key configured)
|
||||
if (model) override.model = model;
|
||||
if (baseUrl) override.base_url = baseUrl;
|
||||
const prev = _builtinOverrides[id];
|
||||
@@ -6543,7 +6565,16 @@ document.getElementById('save-provider-btn').addEventListener('click', () => {
|
||||
const idx = _customProviders.findIndex((p) => p.id === _editingProviderId);
|
||||
if (idx === -1) return;
|
||||
const original = _customProviders[idx];
|
||||
_customProviders[idx] = { ...original, name, adapter, base_url: baseUrl, default_model: model || undefined, api_key: apiKey || undefined };
|
||||
const hadCustomKey = original.api_key === '••••••••';
|
||||
let effectiveApiKey;
|
||||
if (apiKey) {
|
||||
effectiveApiKey = apiKey; // New key — backend will encrypt it
|
||||
} else if (hadCustomKey) {
|
||||
effectiveApiKey = '••••••••'; // Sentinel: keep existing encrypted key
|
||||
} else {
|
||||
effectiveApiKey = undefined; // No key
|
||||
}
|
||||
_customProviders[idx] = { ...original, name, adapter, base_url: baseUrl, default_model: model || undefined, api_key: effectiveApiKey };
|
||||
const isActive = _editingProviderId === _activeLlmBackend;
|
||||
const modelUpdate = () => {
|
||||
if (!isActive) return Promise.resolve();
|
||||
|
||||
@@ -91,6 +91,7 @@ impl TestGatewayBuilder {
|
||||
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
|
||||
startup_time: std::time::Instant::now(),
|
||||
active_config: crate::channels::web::server::ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -79,6 +79,7 @@ fn build_state(
|
||||
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
|
||||
startup_time: std::time::Instant::now(),
|
||||
active_config: ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -534,6 +534,7 @@ mod tests {
|
||||
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
|
||||
startup_time: std::time::Instant::now(),
|
||||
active_config: crate::channels::web::server::ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+179
-1
@@ -298,7 +298,19 @@ impl Config {
|
||||
user_id: &str,
|
||||
toml_path: Option<&std::path::Path>,
|
||||
) -> Result<(), ConfigError> {
|
||||
let settings = if let Some(store) = store {
|
||||
self.re_resolve_llm_with_secrets(store, user_id, toml_path, None)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Re-resolve LLM config, hydrating API keys from the secrets store.
|
||||
pub async fn re_resolve_llm_with_secrets(
|
||||
&mut self,
|
||||
store: Option<&(dyn crate::db::SettingsStore + Sync)>,
|
||||
user_id: &str,
|
||||
toml_path: Option<&std::path::Path>,
|
||||
secrets: Option<&(dyn crate::secrets::SecretsStore + Send + Sync)>,
|
||||
) -> Result<(), ConfigError> {
|
||||
let mut settings = if let Some(store) = store {
|
||||
// TOML as base, then DB on top (DB wins).
|
||||
let mut s = Settings::default();
|
||||
Self::apply_toml_overlay(&mut s, toml_path)?;
|
||||
@@ -310,6 +322,14 @@ impl Config {
|
||||
} else {
|
||||
Settings::default()
|
||||
};
|
||||
|
||||
// Hydrate API keys from encrypted secrets store into the settings
|
||||
// struct so that LlmConfig::resolve() sees them without any changes
|
||||
// to its synchronous resolution logic.
|
||||
if let Some(secrets) = secrets {
|
||||
hydrate_llm_keys_from_secrets(&mut settings, secrets, user_id).await;
|
||||
}
|
||||
|
||||
self.llm = LlmConfig::resolve(&settings)?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -512,3 +532,161 @@ fn inject_os_credential_store_tokens(injected: &mut HashMap<String, String>) {
|
||||
tracing::debug!("Refreshed ANTHROPIC_OAUTH_TOKEN from OS credential store");
|
||||
}
|
||||
}
|
||||
|
||||
/// Hydrate LLM API keys from the secrets store into the settings struct.
|
||||
///
|
||||
/// Called after loading settings from DB but before `LlmConfig::resolve()`.
|
||||
/// Populates `api_key` fields that were stripped from settings during the
|
||||
/// write path and stored encrypted in the secrets store instead.
|
||||
pub async fn hydrate_llm_keys_from_secrets(
|
||||
settings: &mut Settings,
|
||||
secrets: &(dyn crate::secrets::SecretsStore + Send + Sync),
|
||||
user_id: &str,
|
||||
) {
|
||||
// Hydrate builtin overrides
|
||||
for (provider_id, override_val) in settings.llm_builtin_overrides.iter_mut() {
|
||||
if override_val.api_key.is_some() {
|
||||
continue; // Already has a key (legacy plaintext or TOML)
|
||||
}
|
||||
let secret_name = format!("llm_builtin_{}_api_key", provider_id);
|
||||
if let Ok(decrypted) = secrets.get_decrypted(user_id, &secret_name).await {
|
||||
override_val.api_key = Some(decrypted.expose().to_string());
|
||||
}
|
||||
}
|
||||
|
||||
// Hydrate custom providers
|
||||
for provider in settings.llm_custom_providers.iter_mut() {
|
||||
if provider.api_key.is_some() {
|
||||
continue;
|
||||
}
|
||||
let secret_name = format!("llm_custom_{}_api_key", provider.id);
|
||||
if let Ok(decrypted) = secrets.get_decrypted(user_id, &secret_name).await {
|
||||
provider.api_key = Some(decrypted.expose().to_string());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Migrate plaintext API keys from the settings table to the encrypted secrets store.
|
||||
///
|
||||
/// Idempotent: skips keys that are already in the secrets store.
|
||||
/// After migration, strips plaintext keys from the settings table.
|
||||
pub async fn migrate_plaintext_llm_keys(
|
||||
settings_store: &(dyn crate::db::SettingsStore + Sync),
|
||||
secrets: &(dyn crate::secrets::SecretsStore + Send + Sync),
|
||||
user_id: &str,
|
||||
) {
|
||||
let settings_map = match settings_store.get_all_settings(user_id).await {
|
||||
Ok(m) => m,
|
||||
Err(_) => return,
|
||||
};
|
||||
|
||||
let mut migrated = 0u32;
|
||||
|
||||
// Migrate builtin overrides
|
||||
if let Some(obj) = settings_map
|
||||
.get("llm_builtin_overrides")
|
||||
.and_then(|v| v.as_object())
|
||||
{
|
||||
let mut sanitized = obj.clone();
|
||||
for (provider_id, override_val) in obj {
|
||||
if let Some(api_key) = override_val.get("api_key").and_then(|v| v.as_str()) {
|
||||
if api_key.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let secret_name = format!("llm_builtin_{}_api_key", provider_id);
|
||||
if !secrets.exists(user_id, &secret_name).await.unwrap_or(false)
|
||||
&& let Err(e) = secrets
|
||||
.create(
|
||||
user_id,
|
||||
crate::secrets::CreateSecretParams {
|
||||
name: secret_name.clone(),
|
||||
value: secrecy::SecretString::from(api_key.to_string()),
|
||||
provider: Some(provider_id.clone()),
|
||||
expires_at: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::warn!("Failed to migrate key for builtin '{}': {}", provider_id, e);
|
||||
continue;
|
||||
}
|
||||
if let Some(o) = sanitized
|
||||
.get_mut(provider_id)
|
||||
.and_then(|v| v.as_object_mut())
|
||||
{
|
||||
o.remove("api_key");
|
||||
}
|
||||
migrated += 1;
|
||||
}
|
||||
}
|
||||
if migrated > 0 {
|
||||
let _ = settings_store
|
||||
.set_setting(
|
||||
user_id,
|
||||
"llm_builtin_overrides",
|
||||
&serde_json::Value::Object(sanitized),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
// Migrate custom providers
|
||||
let before = migrated;
|
||||
if let Some(arr) = settings_map
|
||||
.get("llm_custom_providers")
|
||||
.and_then(|v| v.as_array())
|
||||
{
|
||||
let mut sanitized = arr.clone();
|
||||
for (idx, provider_val) in arr.iter().enumerate() {
|
||||
let provider_id = provider_val
|
||||
.get("id")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("");
|
||||
if provider_id.is_empty() {
|
||||
continue;
|
||||
}
|
||||
if let Some(api_key) = provider_val.get("api_key").and_then(|v| v.as_str()) {
|
||||
if api_key.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let secret_name = format!("llm_custom_{}_api_key", provider_id);
|
||||
if !secrets.exists(user_id, &secret_name).await.unwrap_or(false)
|
||||
&& let Err(e) = secrets
|
||||
.create(
|
||||
user_id,
|
||||
crate::secrets::CreateSecretParams {
|
||||
name: secret_name.clone(),
|
||||
value: secrecy::SecretString::from(api_key.to_string()),
|
||||
provider: Some(provider_id.to_string()),
|
||||
expires_at: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::warn!("Failed to migrate key for custom '{}': {}", provider_id, e);
|
||||
continue;
|
||||
}
|
||||
if let Some(o) = sanitized[idx].as_object_mut() {
|
||||
o.remove("api_key");
|
||||
}
|
||||
migrated += 1;
|
||||
}
|
||||
}
|
||||
if migrated > before {
|
||||
let _ = settings_store
|
||||
.set_setting(
|
||||
user_id,
|
||||
"llm_custom_providers",
|
||||
&serde_json::Value::Array(sanitized),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
if migrated > 0 {
|
||||
tracing::info!(
|
||||
"Migrated {} plaintext LLM API key(s) to encrypted secrets store",
|
||||
migrated
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -650,6 +650,9 @@ async fn async_main() -> anyhow::Result<()> {
|
||||
if let Some(ref d) = components.db {
|
||||
gw = gw.with_store(Arc::clone(d));
|
||||
}
|
||||
if let Some(ref ss) = components.secrets_store {
|
||||
gw = gw.with_secrets_store(Arc::clone(ss));
|
||||
}
|
||||
if let Some(ref jm) = container_job_manager {
|
||||
gw = gw.with_job_manager(Arc::clone(jm));
|
||||
}
|
||||
|
||||
@@ -551,6 +551,7 @@ fn gateway_state_has_multi_tenant_fields() {
|
||||
startup_time: std::time::Instant::now(),
|
||||
webhook_rate_limiter: RateLimiter::new(10, 60),
|
||||
active_config: Default::default(),
|
||||
secrets_store: None,
|
||||
};
|
||||
|
||||
assert_eq!(state.default_user_id, "fallback");
|
||||
@@ -902,6 +903,7 @@ async fn start_multi_user_server_with_db() -> (
|
||||
startup_time: std::time::Instant::now(),
|
||||
webhook_rate_limiter: RateLimiter::new(10, 60),
|
||||
active_config: Default::default(),
|
||||
secrets_store: None,
|
||||
});
|
||||
|
||||
let addr: SocketAddr = "127.0.0.1:0".parse().unwrap();
|
||||
|
||||
@@ -217,6 +217,7 @@ async fn start_test_server_with_provider(
|
||||
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
|
||||
startup_time: std::time::Instant::now(),
|
||||
active_config: ironclaw::channels::web::server::ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
});
|
||||
|
||||
let auth = ironclaw::channels::web::auth::MultiAuthState::single(
|
||||
@@ -715,6 +716,7 @@ async fn test_no_llm_provider_returns_503() {
|
||||
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
|
||||
startup_time: std::time::Instant::now(),
|
||||
active_config: ironclaw::channels::web::server::ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
});
|
||||
|
||||
let auth = ironclaw::channels::web::auth::MultiAuthState::single(
|
||||
|
||||
@@ -240,6 +240,7 @@ impl GatewayWorkflowHarness {
|
||||
routine_engine: Arc::clone(&routine_slot),
|
||||
startup_time: Instant::now(),
|
||||
active_config: ironclaw::channels::web::server::ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
});
|
||||
|
||||
let mut agent = Agent::new(
|
||||
|
||||
@@ -65,6 +65,7 @@ async fn start_test_server() -> (
|
||||
routine_engine: Arc::new(tokio::sync::RwLock::new(None)),
|
||||
startup_time: std::time::Instant::now(),
|
||||
active_config: ironclaw::channels::web::server::ActiveConfigSnapshot::default(),
|
||||
secrets_store: None,
|
||||
});
|
||||
|
||||
let auth = ironclaw::channels::web::auth::MultiAuthState::single(
|
||||
|
||||
Reference in New Issue
Block a user