diff --git a/src-tauri/src/app/settings_commands.rs b/src-tauri/src/app/settings_commands.rs index 53e184e1..bd667595 100644 --- a/src-tauri/src/app/settings_commands.rs +++ b/src-tauri/src/app/settings_commands.rs @@ -18,6 +18,10 @@ macro_rules! settings_commands { $crate::settings::commands::settings_set_api_key, $crate::settings::commands::settings_get_api_key_exists, $crate::settings::commands::settings_delete_api_key, + // Secure auth secret commands (keyring-based storage) + $crate::settings::commands::settings_set_auth_secret, + $crate::settings::commands::settings_get_auth_secret, + $crate::settings::commands::settings_delete_auth_secret, // Settings Sync commands $crate::settings_sync::commands::sync_push, $crate::settings_sync::commands::sync_pull, diff --git a/src-tauri/src/app/workspace_commands.rs b/src-tauri/src/app/workspace_commands.rs index 7cab4830..c338b447 100644 --- a/src-tauri/src/app/workspace_commands.rs +++ b/src-tauri/src/app/workspace_commands.rs @@ -170,4 +170,4 @@ macro_rules! workspace_commands { $crate::batch::batch_cache_clear, ]) }; -} \ No newline at end of file +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 7fe81137..7755e365 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -48,9 +48,9 @@ mod sandbox; mod search; mod settings; mod settings_sync; -mod startup_timing; #[cfg(feature = "remote-ssh")] mod ssh_terminal; +mod startup_timing; mod system_specs; mod tasks; mod terminal; diff --git a/src-tauri/src/settings/commands.rs b/src-tauri/src/settings/commands.rs index 8a5bea8f..d3f3b699 100644 --- a/src-tauri/src/settings/commands.rs +++ b/src-tauri/src/settings/commands.rs @@ -11,6 +11,7 @@ use std::collections::HashMap; use std::fs; +use secrecy::ExposeSecret; use tauri::{AppHandle, Manager}; use tracing::info; @@ -23,6 +24,57 @@ use super::types::{ WorkbenchSettings, ZenModeSettings, }; +const ALLOWED_API_KEY_NAMES: &[&str] = &[ + "anthropic_api_key", + "google_api_key", + "openai_api_key", + "openrouter_api_key", + "proxy_authorization", + "supermaven_api_key", +]; + +const ALLOWED_AUTH_SECRET_NAMES: &[&str] = &[ + "codespaces_auth_state", + "copilot_api_token", + "copilot_oauth_token", +]; + +const MAX_API_KEY_LENGTH: usize = 512; +const MAX_AUTH_SECRET_LENGTH: usize = 8 * 1024; + +fn validate_allowed_key(key_name: &str, allowed_keys: &[&str], kind: &str) -> Result<(), String> { + if allowed_keys.contains(&key_name) { + Ok(()) + } else { + Err(format!("Unsupported {} key: {}", kind, key_name)) + } +} + +fn validate_api_key_name(key_name: &str) -> Result<(), String> { + validate_allowed_key(key_name, ALLOWED_API_KEY_NAMES, "API") +} + +fn validate_auth_secret_name(key_name: &str) -> Result<(), String> { + validate_allowed_key(key_name, ALLOWED_AUTH_SECRET_NAMES, "auth secret") +} + +fn validate_secret_value<'a>( + value: &'a str, + kind: &str, + max_length: usize, +) -> Result<&'a str, String> { + let trimmed = value.trim(); + if trimmed.is_empty() { + return Err(format!("{kind} cannot be empty")); + } + + if trimmed.len() > max_length { + return Err(format!("{kind} exceeds maximum length")); + } + + Ok(trimmed) +} + /// Load settings from disk #[tauri::command] pub async fn settings_load(app: AppHandle) -> Result { @@ -362,6 +414,14 @@ pub async fn settings_set_extension( // === Secure API key commands === +fn set_api_key_presence(settings: &mut CortexSettings, key_name: &str, exists: bool) { + match key_name { + "supermaven_api_key" => settings.ai.has_supermaven_api_key = exists, + "proxy_authorization" => settings.http.has_proxy_authorization = exists, + _ => {} + } +} + /// Store an API key securely in the keyring #[tauri::command] pub async fn settings_set_api_key( @@ -369,16 +429,14 @@ pub async fn settings_set_api_key( key_name: String, api_key: String, ) -> Result<(), String> { - SecureApiKeyStore::set_api_key(&key_name, &api_key)?; + validate_api_key_name(&key_name)?; + let api_key = validate_secret_value(&api_key, "API key", MAX_API_KEY_LENGTH)?; + SecureApiKeyStore::set_api_key(&key_name, api_key)?; // Update settings to reflect that key exists let settings_state = app.state::(); if let Ok(mut settings) = settings_state.0.lock() { - match key_name.as_str() { - "supermaven_api_key" => settings.ai.has_supermaven_api_key = true, - "proxy_authorization" => settings.http.has_proxy_authorization = true, - _ => {} - } + set_api_key_presence(&mut settings, &key_name, true); } Ok(()) @@ -387,25 +445,108 @@ pub async fn settings_set_api_key( /// Get an API key from the keyring (returns redacted version for UI) #[tauri::command] pub async fn settings_get_api_key_exists(key_name: String) -> Result { + validate_api_key_name(&key_name)?; SecureApiKeyStore::has_api_key(&key_name) } /// Delete an API key from the keyring #[tauri::command] pub async fn settings_delete_api_key(app: AppHandle, key_name: String) -> Result { + validate_api_key_name(&key_name)?; let deleted = SecureApiKeyStore::delete_api_key(&key_name)?; // Update settings to reflect that key is gone if deleted { let settings_state = app.state::(); if let Ok(mut settings) = settings_state.0.lock() { - match key_name.as_str() { - "supermaven_api_key" => settings.ai.has_supermaven_api_key = false, - "proxy_authorization" => settings.http.has_proxy_authorization = false, - _ => {} - } - }; + set_api_key_presence(&mut settings, &key_name, false); + } } Ok(deleted) } + +/// Store an auth secret securely in the keyring +#[tauri::command] +pub async fn settings_set_auth_secret(key_name: String, value: String) -> Result<(), String> { + validate_auth_secret_name(&key_name)?; + let value = validate_secret_value(&value, "Auth secret", MAX_AUTH_SECRET_LENGTH)?; + SecureApiKeyStore::set_secret(&key_name, value) +} + +/// Retrieve an auth secret from the keyring +#[tauri::command] +pub async fn settings_get_auth_secret(key_name: String) -> Result, String> { + validate_auth_secret_name(&key_name)?; + Ok(SecureApiKeyStore::get_secret(&key_name)?.map(|secret| secret.expose_secret().to_string())) +} + +/// Delete an auth secret from the keyring +#[tauri::command] +pub async fn settings_delete_auth_secret(key_name: String) -> Result { + validate_auth_secret_name(&key_name)?; + SecureApiKeyStore::delete_secret(&key_name) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn api_key_allowlist_accepts_known_provider_keys() { + for key_name in ALLOWED_API_KEY_NAMES { + assert!( + validate_api_key_name(key_name).is_ok(), + "expected {key_name} to be allowed" + ); + } + } + + #[test] + fn api_key_allowlist_rejects_auth_secret_keys() { + assert!(validate_api_key_name("copilot_oauth_token").is_err()); + assert!(validate_api_key_name("codespaces_auth_state").is_err()); + } + + #[test] + fn auth_secret_allowlist_accepts_known_auth_keys() { + for key_name in ALLOWED_AUTH_SECRET_NAMES { + assert!( + validate_auth_secret_name(key_name).is_ok(), + "expected {key_name} to be allowed" + ); + } + } + + #[test] + fn auth_secret_allowlist_rejects_api_key_names() { + assert!(validate_auth_secret_name("openai_api_key").is_err()); + assert!(validate_auth_secret_name("proxy_authorization").is_err()); + } + + #[test] + fn validate_secret_value_trims_valid_input() { + assert!(matches!( + validate_secret_value(" secret ", "Auth secret", MAX_AUTH_SECRET_LENGTH), + Ok("secret") + )); + } + + #[test] + fn validate_secret_value_rejects_empty_input() { + assert!(matches!( + validate_secret_value(" ", "Auth secret", MAX_AUTH_SECRET_LENGTH), + Err(ref error) if error == "Auth secret cannot be empty" + )); + } + + #[test] + fn validate_secret_value_rejects_oversized_input() { + let oversized = "a".repeat(MAX_AUTH_SECRET_LENGTH + 1); + + assert!(matches!( + validate_secret_value(&oversized, "Auth secret", MAX_AUTH_SECRET_LENGTH), + Err(ref error) if error == "Auth secret exceeds maximum length" + )); + } +} diff --git a/src-tauri/src/settings/secure_store.rs b/src-tauri/src/settings/secure_store.rs index 47cd0526..e09de1fc 100644 --- a/src-tauri/src/settings/secure_store.rs +++ b/src-tauri/src/settings/secure_store.rs @@ -12,49 +12,69 @@ use super::KEYRING_SERVICE; pub struct SecureApiKeyStore; impl SecureApiKeyStore { - /// Get keyring entry for an API key + /// Get keyring entry for a secure secret fn get_entry(key_name: &str) -> Result { keyring::Entry::new(KEYRING_SERVICE, key_name) .map_err(|e| format!("Failed to access keyring: {e}")) } - /// Store an API key securely in the keyring - pub fn set_api_key(key_name: &str, api_key: &str) -> Result<(), String> { + /// Store a secret securely in the keyring + pub fn set_secret(key_name: &str, secret: &str) -> Result<(), String> { let entry = Self::get_entry(key_name)?; entry - .set_password(api_key) - .map_err(|e| format!("Failed to store API key: {e}")) + .set_password(secret) + .map_err(|e| format!("Failed to store secret: {e}")) } - /// Retrieve an API key from the keyring - pub fn get_api_key(key_name: &str) -> Result, String> { + /// Retrieve a secret from the keyring + pub fn get_secret(key_name: &str) -> Result, String> { let entry = Self::get_entry(key_name)?; match entry.get_password() { - Ok(key) => Ok(Some(SecretString::from(key))), + Ok(secret) => Ok(Some(SecretString::from(secret))), Err(keyring::Error::NoEntry) => Ok(None), - Err(e) => Err(format!("Failed to retrieve API key: {e}")), + Err(e) => Err(format!("Failed to retrieve secret: {e}")), } } - /// Delete an API key from the keyring - pub fn delete_api_key(key_name: &str) -> Result { + /// Delete a secret from the keyring + pub fn delete_secret(key_name: &str) -> Result { let entry = Self::get_entry(key_name)?; match entry.delete_credential() { Ok(()) => Ok(true), Err(keyring::Error::NoEntry) => Ok(false), - Err(e) => Err(format!("Failed to delete API key: {e}")), + Err(e) => Err(format!("Failed to delete secret: {e}")), } } - /// Check if an API key exists - pub fn has_api_key(key_name: &str) -> Result { + /// Check if a secret exists + pub fn has_secret(key_name: &str) -> Result { let entry = Self::get_entry(key_name)?; match entry.get_password() { Ok(_) => Ok(true), Err(keyring::Error::NoEntry) => Ok(false), - Err(e) => Err(format!("Failed to check API key: {e}")), + Err(e) => Err(format!("Failed to check secret: {e}")), } } + + /// Store an API key securely in the keyring + pub fn set_api_key(key_name: &str, api_key: &str) -> Result<(), String> { + Self::set_secret(key_name, api_key) + } + + /// Retrieve an API key from the keyring + pub fn get_api_key(key_name: &str) -> Result, String> { + Self::get_secret(key_name) + } + + /// Delete an API key from the keyring + pub fn delete_api_key(key_name: &str) -> Result { + Self::delete_secret(key_name) + } + + /// Check if an API key exists + pub fn has_api_key(key_name: &str) -> Result { + Self::has_secret(key_name) + } } /// Get API key for internal use (returns actual value as SecretString) diff --git a/src-tauri/src/workspace/mod.rs b/src-tauri/src/workspace/mod.rs index f3cd1648..3527cef0 100644 --- a/src-tauri/src/workspace/mod.rs +++ b/src-tauri/src/workspace/mod.rs @@ -10,4 +10,4 @@ pub mod multi_root; pub mod types; pub use commands::*; -pub use core::*; \ No newline at end of file +pub use core::*; diff --git a/src/utils/authSecretStorage.ts b/src/utils/authSecretStorage.ts new file mode 100644 index 00000000..27f5bb8e --- /dev/null +++ b/src/utils/authSecretStorage.ts @@ -0,0 +1,113 @@ +import { invoke } from "@tauri-apps/api/core"; +import { safeGetItem, safeRemoveItem } from "./safeStorage"; + +interface SecureJsonOptions { + keyName: string; + legacyKey?: string; + loggerScope: string; + validate?: (value: unknown) => value is T; +} + +interface SaveSecureJsonOptions extends SecureJsonOptions { + value: T; +} + +function logError(scope: string, message: string, error: unknown): void { + console.warn(`[${scope}] ${message}`, error); +} + +function parseStoredJson( + raw: string, + { loggerScope, validate }: SecureJsonOptions +): T | null { + try { + const parsed = JSON.parse(raw); + if (validate && !validate(parsed)) { + console.warn(`[${loggerScope}] Ignoring invalid secure auth payload`); + return null; + } + return parsed as T; + } catch (error) { + logError(loggerScope, "Failed to parse secure auth payload", error); + return null; + } +} + +async function setSecureSecret( + keyName: string, + value: string, + loggerScope: string +): Promise { + try { + await invoke("settings_set_auth_secret", { keyName, value }); + return true; + } catch (error) { + logError(loggerScope, `Failed to store secure auth secret for ${keyName}`, error); + return false; + } +} + +export async function loadSecureJson(options: SecureJsonOptions): Promise { + const { keyName, legacyKey, loggerScope } = options; + + try { + const stored = await invoke("settings_get_auth_secret", { keyName }); + if (typeof stored === "string") { + const parsed = parseStoredJson(stored, options); + if (parsed === null) { + await deleteSecureSecret({ keyName, loggerScope }); + } + if (legacyKey) { + safeRemoveItem(legacyKey); + } + return parsed; + } + } catch (error) { + logError(loggerScope, `Failed to load secure auth secret for ${keyName}`, error); + } + + if (!legacyKey) { + return null; + } + + const legacyValue = safeGetItem(legacyKey); + if (!legacyValue) { + return null; + } + + const parsed = parseStoredJson(legacyValue, options); + safeRemoveItem(legacyKey); + + if (parsed !== null) { + await setSecureSecret(keyName, JSON.stringify(parsed), loggerScope); + } + + return parsed; +} + +export async function saveSecureJson(options: SaveSecureJsonOptions): Promise { + const { keyName, legacyKey, loggerScope, value } = options; + + if (legacyKey) { + safeRemoveItem(legacyKey); + } + + const serialized = JSON.stringify(value); + await setSecureSecret(keyName, serialized, loggerScope); +} + +export async function deleteSecureSecret( + options: Pick, "keyName" | "legacyKey" | "loggerScope"> +): Promise { + const { keyName, legacyKey, loggerScope } = options; + + if (legacyKey) { + safeRemoveItem(legacyKey); + } + + try { + await invoke("settings_delete_auth_secret", { keyName }); + } catch (error) { + logError(loggerScope, `Failed to delete secure auth secret for ${keyName}`, error); + } +}