diff --git a/crates/animus-mcp-oauth/src/config.rs b/crates/animus-mcp-oauth/src/config.rs index b67a95ad..cb09d1c3 100644 --- a/crates/animus-mcp-oauth/src/config.rs +++ b/crates/animus-mcp-oauth/src/config.rs @@ -49,14 +49,24 @@ pub struct ServerResolution { pub broker_oauth: Option, } -/// Build a keychain-backed [`SecretStore`] for `project_root`, mirroring the -/// `animus secret` surface so OAuth tokens share the project's keychain +/// Build the configured [`SecretStore`] for `project_root`, mirroring the +/// `animus secret` surface so OAuth tokens share the project's secret-store /// scope. +/// +/// Consults both the global `~/.animus/config.json` and the project-level +/// `.animus/config.json` for the `secrets` configuration block so that +/// per-project key-source overrides (e.g. `key_source = user-key`) are +/// honored by `mcp auth --complete` and every other OAuth code path. pub fn build_secret_store(project_root: &Path) -> Result, ServerResolutionError> { let scoped_root = scoped_state_root(project_root) .ok_or_else(|| ServerResolutionError::NoScopedRoot(project_root.display().to_string()))?; + Ok(build_secret_store_at(project_root, scoped_root)) +} + +fn build_secret_store_at(project_root: &Path, scoped_root: impl Into) -> Arc { + let scoped_root = scoped_root.into(); let scope = resolve_keychain_scope(project_root, &scoped_root); - Ok(Arc::from(orchestrator_core::build_secret_store(&scope, scoped_root))) + Arc::from(orchestrator_core::build_secret_store_for_project(&scope, scoped_root, project_root)) } /// Pick the keychain service-scope string from the adopted scoped state @@ -188,3 +198,115 @@ fn finalize( }), } } + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::{Mutex, OnceLock}; + + fn env_lock() -> &'static Mutex<()> { + static ENV_LOCK: OnceLock> = OnceLock::new(); + ENV_LOCK.get_or_init(|| Mutex::new(())) + } + + struct EnvVarGuard { + name: &'static str, + previous: Option, + } + + impl EnvVarGuard { + fn remove(name: &'static str) -> Self { + let previous = std::env::var(name).ok(); + std::env::remove_var(name); + Self { name, previous } + } + } + + impl Drop for EnvVarGuard { + fn drop(&mut self) { + match &self.previous { + Some(value) => std::env::set_var(self.name, value), + None => std::env::remove_var(self.name), + } + } + } + + fn assert_oauth_store_uses_project_key_file(key_source: Option<&str>, backend: Option<&str>) { + // Secret-key environment variables are process-global. Keep removal, + // store construction, and restoration in one serialized window so + // these tests cannot borrow or overwrite another test's key. + let _env_guard = env_lock().lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let project_root = tmp.path().join("project"); + let animus_dir = project_root.join(".animus"); + std::fs::create_dir_all(&animus_dir).unwrap(); + + let key_file = tmp.path().join("server.key"); + std::fs::write(&key_file, "a5".repeat(32)).unwrap(); + let config = serde_json::json!({ + "secrets": { + "backend": backend, + "key_source": key_source, + "key_file": key_file + } + }); + std::fs::write(animus_dir.join("config.json"), serde_json::to_vec(&config).unwrap()).unwrap(); + + let _key_guard = EnvVarGuard::remove("ANIMUS_SECRET_KEY"); + let store = build_secret_store_at(&project_root, tmp.path().join("state")); + assert_eq!( + store.backend_label(), + "device-encrypted store", + "a project key_file must select the headless-safe device backend" + ); + store.set("oauth_test", "token") + .expect("OAuth secret write must use the project-configured key file"); + drop(store); + + // Rebuild the store as the separate `mcp auth --complete` invocation + // does. This verifies that config is consulted on every path, not just + // that one in-memory store can read back its own write. + let reopened = build_secret_store_at(&project_root, tmp.path().join("state")); + assert_eq!( + reopened.backend_label(), + "device-encrypted store", + "OAuth completion must reselect the project-configured device backend" + ); + let stored = reopened.get("oauth_test"); + + assert_eq!( + stored.expect("OAuth secret operations must use the project-configured key file").as_deref(), + Some("token") + ); + } + + #[test] + fn oauth_secret_store_honors_project_user_key_without_env_key() { + assert_oauth_store_uses_project_key_file(Some("user-key"), Some("device")); + } + + #[test] + fn oauth_secret_store_auto_backend_honors_project_user_key_without_env_key() { + assert_oauth_store_uses_project_key_file(Some("user-key"), Some("auto")); + } + + #[test] + fn oauth_secret_store_auto_uses_project_key_file_without_env_key() { + assert_oauth_store_uses_project_key_file(Some("auto"), Some("auto")); + } + + #[test] + fn oauth_secret_store_device_backend_auto_source_uses_project_key_file_without_env_key() { + assert_oauth_store_uses_project_key_file(Some("auto"), Some("device")); + } + + #[test] + fn oauth_secret_store_device_backend_default_source_uses_project_key_file_without_env_key() { + assert_oauth_store_uses_project_key_file(None, Some("device")); + } + + #[test] + fn oauth_secret_store_default_auto_uses_project_key_file_without_env_key() { + assert_oauth_store_uses_project_key_file(None, None); + } +} diff --git a/crates/orchestrator-cli/src/services/operations/ops_doctor/checks_api_keys.rs b/crates/orchestrator-cli/src/services/operations/ops_doctor/checks_api_keys.rs index 9f3f6976..96d8cf14 100644 --- a/crates/orchestrator-cli/src/services/operations/ops_doctor/checks_api_keys.rs +++ b/crates/orchestrator-cli/src/services/operations/ops_doctor/checks_api_keys.rs @@ -73,7 +73,7 @@ fn keychain_store(project_root: &Path) -> Option> { .and_then(|s| s.to_str()) .map(|s| s.to_string()) .unwrap_or_else(|| repository_scope_for_path(project_root)); - Some(orchestrator_core::build_secret_store(&scope, scoped_root)) + Some(orchestrator_core::build_secret_store_for_project(&scope, scoped_root, project_root)) } /// True when `name` resolves to a non-empty value in the project secret store. diff --git a/crates/orchestrator-cli/src/services/operations/ops_secret.rs b/crates/orchestrator-cli/src/services/operations/ops_secret.rs index 86e0b6ba..e057c4b5 100644 --- a/crates/orchestrator-cli/src/services/operations/ops_secret.rs +++ b/crates/orchestrator-cli/src/services/operations/ops_secret.rs @@ -46,8 +46,10 @@ pub(crate) async fn handle_secret( } /// Copy every secret between backends (keyring <-> device store). Builds both -/// ends explicitly via `build_backend` (independent of the configured default), -/// verifies each copy, and only clears the source when `--remove-source` is set. +/// ends explicitly via `build_backend_for_project` (independent of the configured +/// default), verifies each copy, and only clears the source when `--remove-source` +/// is set. Using the project-aware builder ensures `key_source`/`key_file` from +/// `.animus/config.json` are honored when the device backend is the source or target. fn handle_migrate( args: SecretMigrateArgs, project_root: &Path, @@ -61,8 +63,10 @@ fn handle_migrate( "keyring" => ("device", "keyring"), other => return Err(anyhow!("unknown migrate target '{other}' (expected: device or keyring)")), }; - let source = orchestrator_core::build_backend(&scope, scoped_root.to_path_buf(), source_name); - let target = orchestrator_core::build_backend(&scope, scoped_root.to_path_buf(), target_name); + let source = + orchestrator_core::build_backend_for_project(&scope, scoped_root.to_path_buf(), source_name, project_root); + let target = + orchestrator_core::build_backend_for_project(&scope, scoped_root.to_path_buf(), target_name, project_root); let MigrationOutcome { migrated, remove_failures } = migrate_secrets(source.as_ref(), target.as_ref(), args.remove_source)?; @@ -286,7 +290,7 @@ fn build_store(project_root: &Path) -> Result> { let scoped_root = scoped_state_root(project_root) .ok_or_else(|| anyhow!("could not resolve scoped state root for project at {}", project_root.display()))?; let scope = resolve_keychain_scope(project_root, &scoped_root); - Ok(orchestrator_core::build_secret_store(&scope, scoped_root)) + Ok(orchestrator_core::build_secret_store_for_project(&scope, scoped_root, project_root)) } /// Pick the keychain service-scope string from the adopted scoped state diff --git a/crates/orchestrator-core/src/lib.rs b/crates/orchestrator-core/src/lib.rs index 4a1ca3d0..0db8e143 100644 --- a/crates/orchestrator-core/src/lib.rs +++ b/crates/orchestrator-core/src/lib.rs @@ -96,7 +96,10 @@ pub use runtime_contract::{ cli_tool_executable, cli_tool_read_only_flag, cli_tool_response_schema_flag, CliCapabilities, CliSessionResumeMode, CliSessionResumePlan, }; -pub use secret_device_store::{build_backend, build_secret_store, DeviceEncryptedSecretStore}; +pub use secret_device_store::{ + build_backend, build_backend_for_project, build_secret_store, build_secret_store_for_project, + DeviceEncryptedSecretStore, +}; pub use secret_keysource::{KeySource, KeySourceConfig, KeySourceKind}; pub use secret_store::{ enforce_injection_cap, index_path as secrets_index_path, keychain_service_name, diff --git a/crates/orchestrator-core/src/secret_device_store.rs b/crates/orchestrator-core/src/secret_device_store.rs index e86e26f6..02288770 100644 --- a/crates/orchestrator-core/src/secret_device_store.rs +++ b/crates/orchestrator-core/src/secret_device_store.rs @@ -319,27 +319,33 @@ fn restrict_dir(_path: &Path) {} /// Build the configured [`SecretStore`] for a repo scope. Reads the global /// `secrets` config to choose the backend. Conservative default: the OS keyring /// (existing installs are unchanged). Uses the device-encrypted store when -/// `backend = device`, or when an encrypted store already exists for this scope -/// (a migrated install keeps using it). This is the single seam the rest of the -/// codebase constructs through, replacing direct `KeyringSecretStore::new`. +/// `backend = device`, when an encrypted store already exists for this scope +/// (a migrated install keeps using it), or when a server key source is +/// configured/injected (headless install — avoids the keyring-unavailable error). +/// This is the single seam the rest of the codebase constructs through. pub fn build_secret_store(repo_scope: &str, scoped_root: impl Into) -> Box { - let scoped_root = scoped_root.into(); let cfg = protocol::Config::load_global_if_exists().and_then(|c| c.secrets).unwrap_or_default(); - let resolved = match cfg.backend.as_deref().unwrap_or("auto") { - "device" => "device", - "keyring" | "env" => "keyring", - // auto: keep using the device store once one exists (post-migration), - // otherwise stay on the keyring so existing secrets are never stranded. - _ => { - let device = DeviceEncryptedSecretStore::new(scoped_root.clone(), key_source_config(&cfg)); - if device.path().exists() { - "device" - } else { - "keyring" - } - } - }; - build_backend(repo_scope, scoped_root, resolved) + build_with_cfg(repo_scope, scoped_root.into(), &cfg) +} + +/// Build the configured [`SecretStore`], consulting both the global config +/// (`~/.animus/config.json`) and the project-level `.animus/config.json`. The +/// project config's `secrets` block takes precedence over the global config's, +/// so per-project deployments can override the key source without touching the +/// global config. +/// +/// Use this instead of [`build_secret_store`] in surfaces that have access to +/// the project root (e.g. `mcp auth --complete`) so that writing `key_source` +/// into the project-level config is honored end-to-end. +pub fn build_secret_store_for_project( + repo_scope: &str, + scoped_root: impl Into, + project_root: &Path, +) -> Box { + let global = protocol::Config::load_global_if_exists().and_then(|c| c.secrets).unwrap_or_default(); + let project = load_project_secrets_config(project_root); + let cfg = merge_secrets_config(global, project); + build_with_cfg(repo_scope, scoped_root.into(), &cfg) } /// Build a SPECIFIC backend by name (`"device"` or anything else → keyring), @@ -355,6 +361,104 @@ pub fn build_backend(repo_scope: &str, scoped_root: impl Into, backend: } } +/// Same as [`build_backend`] but also loads the project-level `.animus/config.json` +/// so `key_source`/`key_file` set in the project config are honored when explicitly +/// building the device backend (e.g. for `animus secret migrate`). The project +/// config's `secrets` block wins field-by-field over the global config. +pub fn build_backend_for_project( + repo_scope: &str, + scoped_root: impl Into, + backend: &str, + project_root: &Path, +) -> Box { + let scoped_root = scoped_root.into(); + if backend == "device" { + let global = protocol::Config::load_global_if_exists().and_then(|c| c.secrets).unwrap_or_default(); + let project = load_project_secrets_config(project_root); + let cfg = merge_secrets_config(global, project); + Box::new(DeviceEncryptedSecretStore::new(scoped_root, key_source_config(&cfg))) + } else { + Box::new(crate::secret_store::KeyringSecretStore::new(repo_scope, scoped_root)) + } +} + +/// Core builder: choose backend from `cfg` and construct the store. +fn build_with_cfg(repo_scope: &str, scoped_root: PathBuf, cfg: &protocol::SecretsConfig) -> Box { + let backend = resolve_auto_backend(cfg, &scoped_root); + if backend == "device" { + Box::new(DeviceEncryptedSecretStore::new(scoped_root, key_source_config(cfg))) + } else { + Box::new(crate::secret_store::KeyringSecretStore::new(repo_scope, scoped_root)) + } +} + +/// Resolve which storage backend to use given a [`protocol::SecretsConfig`]. +/// +/// `auto` rules (applied in order): +/// 1. An operator-configured or env-injected server key source (`user-key` / +/// `passphrase` / `ANIMUS_SECRET_KEY` / `ANIMUS_SECRET_PASSPHRASE`) → +/// `device`. The operator has signaled they want device-encrypted storage; +/// on headless hosts this avoids the OS-keyring-unavailable hard error. +/// 2. A device-encrypted store already exists for this scope → `device`. +/// Post-migration installs continue using the device store. +/// 3. Fall back to `keyring` (existing desktop installs are unchanged). +fn resolve_auto_backend(cfg: &protocol::SecretsConfig, scoped_root: &Path) -> &'static str { + match cfg.backend.as_deref().unwrap_or("auto") { + "device" => "device", + "keyring" | "env" => "keyring", + _ => { + if has_server_key_configured(cfg) { + return "device"; + } + let device = DeviceEncryptedSecretStore::new(scoped_root.to_path_buf(), key_source_config(cfg)); + if device.path().exists() { + "device" + } else { + "keyring" + } + } + } +} + +/// True when a server-appropriate key source is available: explicitly configured +/// via `key_source`, a `key_file` path (honored by `auto` and `user-key`), or +/// injected via the corresponding env var. +fn has_server_key_configured(cfg: &protocol::SecretsConfig) -> bool { + use crate::secret_keysource::{KeySourceKind, ENV_PASSPHRASE, ENV_USER_KEY}; + let configured_source = cfg.key_source + .as_deref() + .and_then(|source| KeySourceKind::parse(source).ok()); + configured_source.is_some_and(|source| matches!(source, KeySourceKind::UserKey | KeySourceKind::Passphrase)) + || (cfg.key_file.as_deref().is_some_and(|path| !path.trim().is_empty()) + && matches!(configured_source, None | Some(KeySourceKind::Auto | KeySourceKind::UserKey))) + || std::env::var(ENV_USER_KEY).is_ok_and(|raw| !raw.trim().is_empty()) + || std::env::var(ENV_PASSPHRASE).is_ok_and(|raw| !raw.trim().is_empty()) +} + +/// Read the project-level `.animus/config.json` and return its `secrets` block. +/// Returns `None` when the file is absent or unparseable (side-effect-free). +fn load_project_secrets_config(project_root: &Path) -> Option { + let path = project_root.join(".animus").join("config.json"); + if !path.exists() { + return None; + } + let content = std::fs::read_to_string(&path).ok()?; + serde_json::from_str::(&content).ok()?.secrets +} + +/// Merge two [`protocol::SecretsConfig`] values; `project` wins field-by-field. +fn merge_secrets_config( + global: protocol::SecretsConfig, + project: Option, +) -> protocol::SecretsConfig { + let Some(proj) = project else { return global }; + protocol::SecretsConfig { + backend: proj.backend.or(global.backend), + key_source: proj.key_source.or(global.key_source), + key_file: proj.key_file.or(global.key_file), + } +} + fn key_source_config(cfg: &protocol::SecretsConfig) -> KeySourceConfig { let kind = cfg .key_source @@ -363,7 +467,10 @@ fn key_source_config(cfg: &protocol::SecretsConfig) -> KeySourceConfig { .unwrap_or(crate::secret_keysource::KeySourceKind::Auto); KeySourceConfig { kind_override: Some(kind), - key_file: cfg.key_file.as_ref().map(PathBuf::from), + // Treat an empty JSON string as absent. Otherwise `auto` selects the + // device backend and later attempts to read an empty path as a key + // file, obscuring the actionable "no key provided" error. + key_file: cfg.key_file.as_deref().filter(|path| !path.trim().is_empty()).map(PathBuf::from), // `passphrase` is env-driven for both the CLI and the daemon: the key // source reads ANIMUS_SECRET_PASSPHRASE at resolve time (and errors with // that instruction when unset), so there is no in-process passphrase to @@ -376,6 +483,7 @@ fn key_source_config(cfg: &protocol::SecretsConfig) -> KeySourceConfig { #[cfg(test)] mod tests { use super::*; + use crate::secret_keysource::tests::env_lock; use crate::secret_keysource::{KeySourceConfig, KeySourceKind}; // A user-key store backed by a per-test key FILE, so tests need no shared @@ -392,6 +500,9 @@ mod tests { #[test] fn round_trip_set_get_list_delete() { + // UserKeySource::resolve checks ANIMUS_SECRET_KEY first; hold env_lock + // so tests that mutate the var cannot race this key-file-based test. + let _guard = env_lock().lock().unwrap(); let tmp = tempfile::tempdir().unwrap(); let s = store(tmp.path()); assert_eq!(s.get("API_KEY").unwrap(), None); @@ -409,6 +520,7 @@ mod tests { #[test] fn file_is_not_plaintext() { + let _guard = env_lock().lock().unwrap(); let tmp = tempfile::tempdir().unwrap(); let s = store(tmp.path()); s.set("API_KEY", "PLAINTEXT_NEEDLE").unwrap(); @@ -418,6 +530,7 @@ mod tests { #[test] fn tamper_fails_closed() { + let _guard = env_lock().lock().unwrap(); let tmp = tempfile::tempdir().unwrap(); let s = store(tmp.path()); s.set("API_KEY", "v").unwrap(); @@ -430,10 +543,275 @@ mod tests { #[test] fn wrong_device_key_cannot_decrypt() { + let _guard = env_lock().lock().unwrap(); let tmp = tempfile::tempdir().unwrap(); store_with_key(tmp.path(), "right.key", [3u8; KEY_LEN]).set("API_KEY", "v").unwrap(); // Simulate the file moved to a machine with a different key. let wrong = store_with_key(tmp.path(), "wrong.key", [9u8; KEY_LEN]); assert!(wrong.get("API_KEY").is_err(), "a different device/user key must not decrypt the store"); } + + // --- build_secret_store_for_project / merge / resolve_auto_backend --- + + fn write_project_secrets_config(project_root: &Path, key_source: Option<&str>, key_file: Option<&str>) { + let animus_dir = project_root.join(".animus"); + std::fs::create_dir_all(&animus_dir).unwrap(); + let cfg = serde_json::json!({ + "secrets": { + "key_source": key_source, + "key_file": key_file + } + }); + std::fs::write(animus_dir.join("config.json"), serde_json::to_string_pretty(&cfg).unwrap()).unwrap(); + } + + #[test] + fn merge_secrets_config_project_wins_field_by_field() { + let global = protocol::SecretsConfig { + backend: Some("keyring".to_string()), + key_source: Some("device-id".to_string()), + key_file: Some("/global/key".to_string()), + }; + let project = + Some(protocol::SecretsConfig { backend: None, key_source: Some("user-key".to_string()), key_file: None }); + let merged = merge_secrets_config(global, project); + // project key_source wins; global backend/key_file kept where project has None + assert_eq!(merged.key_source.as_deref(), Some("user-key")); + assert_eq!(merged.backend.as_deref(), Some("keyring")); + assert_eq!(merged.key_file.as_deref(), Some("/global/key")); + } + + #[test] + fn merge_secrets_config_no_project_returns_global() { + let global = protocol::SecretsConfig { + backend: Some("device".to_string()), + key_source: Some("user-key".to_string()), + key_file: Some("/k".to_string()), + }; + let merged = merge_secrets_config(global.clone(), None); + assert_eq!(merged, global); + } + + #[test] + fn has_server_key_configured_env_user_key() { + use crate::secret_keysource::ENV_USER_KEY; + let cfg = protocol::SecretsConfig::default(); + let _guard = env_lock().lock().unwrap(); + let prev = std::env::var(ENV_USER_KEY).ok(); + // Use a valid 32-byte hex key so other tests do not see an invalid value + // if this key somehow outlives its lock window. + std::env::set_var(ENV_USER_KEY, hex::encode([0xEEu8; KEY_LEN])); + let result = has_server_key_configured(&cfg); + match &prev { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + assert!(result, "has_server_key_configured must be true when ANIMUS_SECRET_KEY is set"); + } + + #[test] + fn has_server_key_configured_via_config_key_source() { + let cfg = protocol::SecretsConfig { key_source: Some("user-key".to_string()), ..Default::default() }; + assert!(has_server_key_configured(&cfg)); + let cfg_alias = protocol::SecretsConfig { key_source: Some("userkey".to_string()), ..Default::default() }; + assert!(has_server_key_configured(&cfg_alias)); + let cfg2 = protocol::SecretsConfig { key_source: Some("passphrase".to_string()), ..Default::default() }; + assert!(has_server_key_configured(&cfg2)); + let cfg3 = protocol::SecretsConfig { key_source: Some("device-id".to_string()), ..Default::default() }; + use crate::secret_keysource::tests::env_lock; + use crate::secret_keysource::{ENV_PASSPHRASE, ENV_USER_KEY}; + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::remove_var(ENV_USER_KEY); + std::env::remove_var(ENV_PASSPHRASE); + let result = has_server_key_configured(&cfg3); + if let Some(v) = prev_key { + std::env::set_var(ENV_USER_KEY, v) + } + if let Some(v) = prev_pass { + std::env::set_var(ENV_PASSPHRASE, v) + } + assert!(!result, "device-id key source must not count as a server key"); + } + + #[test] + fn has_server_key_configured_with_key_file() { + use crate::secret_keysource::{ENV_PASSPHRASE, ENV_USER_KEY}; + let cfg = + protocol::SecretsConfig { key_file: Some("/srv/animus/secret.key".to_string()), ..Default::default() }; + // Remove env vars so only key_file drives the result. + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::remove_var(ENV_USER_KEY); + std::env::remove_var(ENV_PASSPHRASE); + let result = has_server_key_configured(&cfg); + if let Some(v) = prev_key { + std::env::set_var(ENV_USER_KEY, v) + } + if let Some(v) = prev_pass { + std::env::set_var(ENV_PASSPHRASE, v) + } + assert!(result, "key_file in secrets config must count as a server key source"); + } + + #[test] + fn empty_key_file_is_not_a_server_key() { + use crate::secret_keysource::{ENV_PASSPHRASE, ENV_USER_KEY}; + let cfg = protocol::SecretsConfig { key_file: Some(" ".to_string()), ..Default::default() }; + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::remove_var(ENV_USER_KEY); + std::env::remove_var(ENV_PASSPHRASE); + + let configured = has_server_key_configured(&cfg); + let resolved = key_source_config(&cfg); + + match prev_key { + Some(value) => std::env::set_var(ENV_USER_KEY, value), + None => std::env::remove_var(ENV_USER_KEY), + } + match prev_pass { + Some(value) => std::env::set_var(ENV_PASSPHRASE, value), + None => std::env::remove_var(ENV_PASSPHRASE), + } + assert!(!configured, "an empty key_file must not select the device backend"); + assert!(resolved.key_file.is_none(), "an empty key_file must be normalized to absent"); + } + + #[test] + fn key_file_does_not_override_explicit_device_id_source() { + use crate::secret_keysource::{ENV_PASSPHRASE, ENV_USER_KEY}; + let cfg = protocol::SecretsConfig { + key_source: Some("device-id".to_string()), + key_file: Some("/srv/animus/secret.key".to_string()), + ..Default::default() + }; + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::remove_var(ENV_USER_KEY); + std::env::remove_var(ENV_PASSPHRASE); + let result = has_server_key_configured(&cfg); + match prev_key { + Some(value) => std::env::set_var(ENV_USER_KEY, value), + None => std::env::remove_var(ENV_USER_KEY), + } + match prev_pass { + Some(value) => std::env::set_var(ENV_PASSPHRASE, value), + None => std::env::remove_var(ENV_PASSPHRASE), + } + assert!(!result, "key_file is ignored when device-id is explicitly selected"); + } + + #[test] + fn empty_passphrase_env_is_not_a_server_key() { + use crate::secret_keysource::{ENV_PASSPHRASE, ENV_USER_KEY}; + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::remove_var(ENV_USER_KEY); + std::env::set_var(ENV_PASSPHRASE, " "); + let result = has_server_key_configured(&protocol::SecretsConfig::default()); + match prev_key { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + match prev_pass { + Some(v) => std::env::set_var(ENV_PASSPHRASE, v), + None => std::env::remove_var(ENV_PASSPHRASE), + } + assert!(!result, "empty ANIMUS_SECRET_PASSPHRASE must be treated as unset"); + } + + #[test] + fn build_secret_store_for_project_reads_project_config() { + crate::test_env::stable_test_home(); + let _guard = env_lock().lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let project_dir = tmp.path().join("project"); + std::fs::create_dir_all(&project_dir).unwrap(); + let key = [0xABu8; KEY_LEN]; + let key_file = tmp.path().join("server.key"); + std::fs::write(&key_file, hex::encode(key)).unwrap(); + // Do not force `backend = device`: this exercises the production + // headless path where configured server key material makes `auto` + // select the device-encrypted store. + write_project_secrets_config(&project_dir, Some("user-key"), Some(key_file.to_str().unwrap())); + let scope = "test-project-scope"; + let scoped_root = tmp.path().join("state"); + std::fs::create_dir_all(&scoped_root).unwrap(); + // Ensure ANIMUS_SECRET_KEY is not set so the key file is used. + use crate::secret_keysource::ENV_USER_KEY; + let prev = std::env::var(ENV_USER_KEY).ok(); + std::env::remove_var(ENV_USER_KEY); + let store = build_secret_store_for_project(scope, scoped_root, &project_dir); + let set_result = store.set("FOO", "bar"); + let get_result = store.get("FOO"); + if let Some(v) = prev { + std::env::set_var(ENV_USER_KEY, v) + } + set_result.expect("project-config-sourced store must accept writes"); + assert_eq!(get_result.unwrap().as_deref(), Some("bar")); + } + + #[test] + fn resolve_auto_backend_uses_device_when_server_key_in_cfg() { + let cfg = protocol::SecretsConfig { backend: None, key_source: Some("user-key".to_string()), key_file: None }; + let dir = tempfile::tempdir().unwrap(); + // No pre-existing device store — but server key is configured. + assert_eq!(resolve_auto_backend(&cfg, dir.path()), "device"); + } + + #[test] + fn resolve_auto_backend_falls_back_to_keyring_without_server_key() { + use crate::secret_keysource::tests::env_lock; + use crate::secret_keysource::{ENV_PASSPHRASE, ENV_USER_KEY}; + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::remove_var(ENV_USER_KEY); + std::env::remove_var(ENV_PASSPHRASE); + let cfg = protocol::SecretsConfig::default(); + let dir = tempfile::tempdir().unwrap(); + let result = resolve_auto_backend(&cfg, dir.path()); + if let Some(v) = prev_key { + std::env::set_var(ENV_USER_KEY, v) + } + if let Some(v) = prev_pass { + std::env::set_var(ENV_PASSPHRASE, v) + } + assert_eq!(result, "keyring", "auto without a server key and no existing store must fall back to keyring"); + } + + #[test] + fn build_backend_for_project_honors_project_key_source() { + crate::test_env::stable_test_home(); + let _guard = env_lock().lock().unwrap(); + use crate::secret_keysource::ENV_USER_KEY; + let tmp = tempfile::tempdir().unwrap(); + let project_dir = tmp.path().join("project"); + std::fs::create_dir_all(&project_dir).unwrap(); + let key = [0xCDu8; KEY_LEN]; + let key_file = tmp.path().join("migrate.key"); + std::fs::write(&key_file, hex::encode(key)).unwrap(); + // Write project config with user-key source and a key_file. + write_project_secrets_config(&project_dir, Some("user-key"), Some(key_file.to_str().unwrap())); + let scope = "test-migrate-scope"; + let scoped_root = tmp.path().join("state"); + std::fs::create_dir_all(&scoped_root).unwrap(); + // Remove env var so only the key_file drives the device store key. + let prev = std::env::var(ENV_USER_KEY).ok(); + std::env::remove_var(ENV_USER_KEY); + let store = build_backend_for_project(scope, scoped_root, "device", &project_dir); + let set_result = store.set("MIGRATE_KEY", "value"); + let get_result = store.get("MIGRATE_KEY"); + if let Some(v) = prev { + std::env::set_var(ENV_USER_KEY, v) + } + set_result.expect("build_backend_for_project must honor project key_file for the device backend"); + assert_eq!(get_result.unwrap().as_deref(), Some("value")); + } } diff --git a/crates/orchestrator-core/src/secret_keysource.rs b/crates/orchestrator-core/src/secret_keysource.rs index f502a140..2dbf7bd0 100644 --- a/crates/orchestrator-core/src/secret_keysource.rs +++ b/crates/orchestrator-core/src/secret_keysource.rs @@ -99,15 +99,17 @@ impl UserKeySource { /// Resolve from the env var first, then the configured key file. pub fn resolve(key_file: Option<&Path>) -> Result { if let Ok(raw) = std::env::var(ENV_USER_KEY) { - return Ok(Self { key: parse_raw_key(raw.trim())? }); + if !raw.trim().is_empty() { + return Ok(Self { key: parse_raw_key(raw.trim())? }); + } } if let Some(path) = key_file { let raw = std::fs::read_to_string(path) - .with_context(|| format!("reading secret_key_file at {}", path.display()))?; + .with_context(|| format!("reading secrets.key_file at {}", path.display()))?; return Ok(Self { key: parse_raw_key(raw.trim())? }); } bail!( - "secret_key_source = user-key but no key provided: set {ENV_USER_KEY} (hex or base64, 32 bytes) or secret_key_file" + "secret_key_source = user-key but no key provided: set {ENV_USER_KEY} (hex or base64, 32 bytes) or secrets.key_file" ) } } @@ -306,20 +308,63 @@ pub fn resolve_key_source(config: &KeySourceConfig, salt: &[u8]) -> Result Ok(Box::new(DeviceIdKeySource::resolve(salt)?)), - KeySourceKind::Auto => resolve_auto(salt), + KeySourceKind::Auto => resolve_auto(config, salt), } } -/// `auto`: prefer an OS hardware-backed key, fall back to `device-id`. Hardware -/// providers (Secure Enclave / DPAPI / TPM) are wired in per platform; until a -/// platform's provider lands, `auto` resolves to `device-id` there. -fn resolve_auto(salt: &[u8]) -> Result> { +/// `auto`: prefer operator-supplied server key material, then fall back to +/// `device-id`. Hardware providers (Secure Enclave / DPAPI / TPM) can be wired +/// in per platform; until a platform's provider lands, `auto` resolves per the +/// following priority: +/// +/// 1. `ANIMUS_SECRET_KEY` env var → `user-key` (runtime-injected key; highest priority) +/// 2. `key_file` from `config` → `user-key` (operator-configured file; headless-safe) +/// 3. configured passphrase or `ANIMUS_SECRET_PASSPHRASE` env var → `passphrase` +/// (Argon2id KDF; headless-safe) +/// 4. `device-id` (fallback; interactive hosts only — binding, not on-device-secret-safe) +/// +/// Steps 1–3 let headless/server deployments work without setting +/// `secret_key_source` explicitly: they just supply the key material (via env +/// or file) and `auto` does the right thing. This avoids the keyring-unavailable +/// hard error and prevents the device-id redeploy wipe caused by a new machine-id. +/// +/// The priority here MUST mirror `has_server_key_configured` in +/// `secret_device_store` — that function picks the `device` backend for the +/// same set of conditions; if a condition triggers backend=device but this +/// function falls through to `device-id`, the store will be sealed with the +/// wrong key and reads will fail. +fn resolve_auto(config: &KeySourceConfig, salt: &[u8]) -> Result> { + // Prefer operator-supplied key: env var wins over key_file so runtime + // injection (e.g. Docker secrets via envFrom) takes precedence over a + // file configured in the project/global config. If only key_file is set, + // UserKeySource::resolve will still try the env first then the file. + if std::env::var(ENV_USER_KEY).is_ok_and(|raw| !raw.trim().is_empty()) || config.key_file.is_some() { + return Ok(Box::new(UserKeySource::resolve(config.key_file.as_deref())?)); + } + // An in-process or env-injected passphrase is also a headless-safe server + // source. Prefer the in-process value when the caller supplied one, just + // as explicit user-key material takes precedence over its fallback. + if config.passphrase.is_some() || std::env::var(ENV_PASSPHRASE).is_ok_and(|raw| !raw.trim().is_empty()) { + return Ok(Box::new(PassphraseKeySource::resolve( + config.passphrase.as_ref().map(|passphrase| passphrase.as_str()), + salt, + )?)); + } Ok(Box::new(DeviceIdKeySource::resolve(salt)?)) } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; + use std::sync::{Mutex, OnceLock}; + + /// Serialize all tests that mutate process-wide env vars so they cannot + /// race each other. Any test that calls `set_var`/`remove_var` must hold + /// this lock for the duration of the mutation + observation window. + pub(crate) fn env_lock() -> &'static Mutex<()> { + static ENV_LOCK: OnceLock> = OnceLock::new(); + ENV_LOCK.get_or_init(|| Mutex::new(())) + } #[test] fn key_source_kind_parse_round_trips() { @@ -341,6 +386,22 @@ mod tests { assert!(parse_raw_key("").is_err()); } + #[test] + fn user_key_error_names_the_public_config_field() { + let _guard = env_lock().lock().unwrap(); + let previous = std::env::var(ENV_USER_KEY).ok(); + std::env::remove_var(ENV_USER_KEY); + let error = match UserKeySource::resolve(None) { + Ok(_) => panic!("user-key resolution unexpectedly succeeded without key material"), + Err(error) => error.to_string(), + }; + match previous { + Some(value) => std::env::set_var(ENV_USER_KEY, value), + None => std::env::remove_var(ENV_USER_KEY), + } + assert!(error.contains("secrets.key_file"), "unexpected error: {error}"); + } + #[test] fn passphrase_is_deterministic_per_salt_and_varies_by_salt() { let salt_a = [1u8; 16]; @@ -352,6 +413,189 @@ mod tests { assert_ne!(*a1.key().unwrap(), *b.key().unwrap(), "different salt must derive a different key"); } + #[test] + fn resolve_auto_uses_user_key_when_env_is_set() { + use base64::Engine; + let raw = [0x42u8; KEY_LEN]; + let b64 = base64::engine::general_purpose::STANDARD.encode(raw); + let _guard = env_lock().lock().unwrap(); + let prev = std::env::var(ENV_USER_KEY).ok(); + std::env::set_var(ENV_USER_KEY, &b64); + let salt = [0u8; 16]; + let result = resolve_auto(&KeySourceConfig::default(), &salt); + match &prev { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + let src = result.expect("resolve_auto with ANIMUS_SECRET_KEY set should succeed"); + assert_eq!(src.id(), "user-key", "auto must resolve to user-key when ANIMUS_SECRET_KEY is set"); + assert_eq!(*src.key().unwrap(), raw); + } + + #[test] + fn resolve_auto_uses_user_key_when_key_file_configured() { + let raw = [0x55u8; KEY_LEN]; + let tmp = tempfile::tempdir().unwrap(); + let key_file = tmp.path().join("server.key"); + std::fs::write(&key_file, hex::encode(raw)).unwrap(); + let _guard = env_lock().lock().unwrap(); + let prev = std::env::var(ENV_USER_KEY).ok(); + std::env::remove_var(ENV_USER_KEY); + let config = KeySourceConfig { kind_override: None, key_file: Some(key_file), passphrase: None }; + let salt = [0u8; 16]; + let result = resolve_auto(&config, &salt); + match &prev { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + let src = result.expect("resolve_auto with key_file configured should succeed"); + assert_eq!(src.id(), "user-key", "auto must resolve to user-key when key_file is configured"); + assert_eq!(*src.key().unwrap(), raw); + } + + #[test] + fn resolve_auto_prefers_env_user_key_over_configured_key_file() { + let env_key = [0x66u8; KEY_LEN]; + let file_key = [0x77u8; KEY_LEN]; + let tmp = tempfile::tempdir().unwrap(); + let key_file = tmp.path().join("server.key"); + std::fs::write(&key_file, hex::encode(file_key)).unwrap(); + let _guard = env_lock().lock().unwrap(); + let prev = std::env::var(ENV_USER_KEY).ok(); + std::env::set_var(ENV_USER_KEY, hex::encode(env_key)); + let config = KeySourceConfig { kind_override: None, key_file: Some(key_file), passphrase: None }; + let salt = [0u8; 16]; + let result = resolve_auto(&config, &salt); + match &prev { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + let src = result.expect("resolve_auto with both server key sources should succeed"); + assert_eq!(src.id(), "user-key"); + assert_eq!(*src.key().unwrap(), env_key, "ANIMUS_SECRET_KEY must override the configured key file"); + } + + #[test] + fn resolve_auto_ignores_empty_env_user_key_when_key_file_configured() { + let file_key = [0x78u8; KEY_LEN]; + let tmp = tempfile::tempdir().unwrap(); + let key_file = tmp.path().join("server.key"); + std::fs::write(&key_file, hex::encode(file_key)).unwrap(); + let _guard = env_lock().lock().unwrap(); + let prev = std::env::var(ENV_USER_KEY).ok(); + std::env::set_var(ENV_USER_KEY, " "); + let config = KeySourceConfig { kind_override: None, key_file: Some(key_file), passphrase: None }; + let salt = [0u8; 16]; + let result = resolve_auto(&config, &salt); + match &prev { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + let src = result.expect("empty ANIMUS_SECRET_KEY must not mask a configured key file"); + assert_eq!(src.id(), "user-key"); + assert_eq!(*src.key().unwrap(), file_key); + } + + #[test] + fn resolve_auto_uses_passphrase_when_passphrase_env_is_set() { + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::remove_var(ENV_USER_KEY); + std::env::set_var(ENV_PASSPHRASE, "headless-passphrase"); + let salt = [0xAAu8; 16]; + let result = resolve_auto(&KeySourceConfig::default(), &salt); + match &prev_key { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + match &prev_pass { + Some(v) => std::env::set_var(ENV_PASSPHRASE, v), + None => std::env::remove_var(ENV_PASSPHRASE), + } + let src = result.expect("resolve_auto with ANIMUS_SECRET_PASSPHRASE set should succeed"); + assert_eq!(src.id(), "passphrase", "auto must resolve to passphrase when ANIMUS_SECRET_PASSPHRASE is set"); + } + + #[test] + fn resolve_auto_ignores_empty_passphrase_env() { + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::remove_var(ENV_USER_KEY); + std::env::set_var(ENV_PASSPHRASE, " "); + let salt = [0xACu8; 16]; + let result = resolve_auto(&KeySourceConfig::default(), &salt); + match &prev_key { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + match &prev_pass { + Some(v) => std::env::set_var(ENV_PASSPHRASE, v), + None => std::env::remove_var(ENV_PASSPHRASE), + } + match result { + Ok(src) => assert_eq!(src.id(), "device-id"), + Err(err) => { + let message = format!("{err:#}"); + assert!( + message.contains("machine id") || message.contains("machine-id"), + "empty passphrase must fall through to device-id, got: {message}" + ); + } + } + } + + #[test] + fn resolve_auto_uses_configured_passphrase_without_env() { + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::remove_var(ENV_USER_KEY); + std::env::remove_var(ENV_PASSPHRASE); + let config = KeySourceConfig { + kind_override: None, + key_file: None, + passphrase: Some(Zeroizing::new("configured-headless-passphrase".to_string())), + }; + let salt = [0xABu8; 16]; + let result = resolve_auto(&config, &salt); + match &prev_key { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + match &prev_pass { + Some(v) => std::env::set_var(ENV_PASSPHRASE, v), + None => std::env::remove_var(ENV_PASSPHRASE), + } + let src = result.expect("resolve_auto with a configured passphrase should succeed"); + assert_eq!(src.id(), "passphrase"); + } + + #[test] + fn resolve_auto_user_key_wins_over_passphrase() { + use base64::Engine; + let raw = [0x99u8; KEY_LEN]; + let b64 = base64::engine::general_purpose::STANDARD.encode(raw); + let _guard = env_lock().lock().unwrap(); + let prev_key = std::env::var(ENV_USER_KEY).ok(); + let prev_pass = std::env::var(ENV_PASSPHRASE).ok(); + std::env::set_var(ENV_USER_KEY, &b64); + std::env::set_var(ENV_PASSPHRASE, "also-set"); + let salt = [0u8; 16]; + let result = resolve_auto(&KeySourceConfig::default(), &salt); + match &prev_key { + Some(v) => std::env::set_var(ENV_USER_KEY, v), + None => std::env::remove_var(ENV_USER_KEY), + } + match &prev_pass { + Some(v) => std::env::set_var(ENV_PASSPHRASE, v), + None => std::env::remove_var(ENV_PASSPHRASE), + } + let src = result.expect("resolve_auto with both env vars set should succeed"); + assert_eq!(src.id(), "user-key", "user-key env must take priority over passphrase env"); + } + #[test] fn device_id_is_deterministic_and_binds_to_machine_material() { let salt = [9u8; 16]; diff --git a/crates/orchestrator-daemon-runtime/src/quotas.rs b/crates/orchestrator-daemon-runtime/src/quotas.rs index 10a65ed3..a27e3f1a 100644 --- a/crates/orchestrator-daemon-runtime/src/quotas.rs +++ b/crates/orchestrator-daemon-runtime/src/quotas.rs @@ -253,7 +253,7 @@ pub fn install_keychain_secret_provider_for(project_root: &std::path::Path) -> b return false; }; let scope = scope_label_for_scoped_root(project_root, &scoped_root); - let store = orchestrator_core::build_secret_store(&scope, scoped_root); + let store = orchestrator_core::build_secret_store_for_project(&scope, scoped_root, project_root); orchestrator_plugin_host::install_secret_snapshot_provider(std::sync::Arc::new( KeychainSecretSnapshotProvider::new(store), )) diff --git a/docs/architecture/secret-backends.md b/docs/architecture/secret-backends.md index cee19989..0fa66f2f 100644 --- a/docs/architecture/secret-backends.md +++ b/docs/architecture/secret-backends.md @@ -32,8 +32,11 @@ everywhere, and binds the ciphertext to the device. `SecretStore` (set/get/delete/list_keys/snapshot_for_spawn) stays the seam. - New backend `DeviceEncryptedSecretStore: SecretStore`. -- A factory `build_secret_store(repo_scope, scoped_root) -> Box` - reads config and returns the configured backend. It replaces the ~5 direct +- The factories `build_secret_store(repo_scope, scoped_root)` and + `build_secret_store_for_project(repo_scope, scoped_root, project_root)` read + config and return the configured backend. Project-aware call sites use the + latter so the global and project `secrets` blocks are merged before backend + and key-source selection. They replace the ~5 direct `KeyringSecretStore::new(&scope, scoped_root)` construction sites (orchestrator-daemon-runtime/quotas.rs ×2, animus-mcp-oauth/config.rs, orchestrator-cli ops_secret.rs, ops_doctor/checks_api_keys.rs). @@ -62,14 +65,20 @@ AEAD-sealed JSON map `{key: value}`: trait KeySource { fn key(&self) -> Result>; fn id(&self) -> &str; } ``` -Selected by config `secret_key_source`: - -- `auto` (default): resolves to `device-id` today. The hardware key sources - (Secure Enclave / DPAPI / TPM) are deferred — see "Platform support" below — - so `auto` does not currently reach for an OS-hardware key. +Selected by config `secrets.key_source`: + +- `auto` (default): resolves in priority order — (1) `ANIMUS_SECRET_KEY` env var + → `user-key`; (2) `key_file` configured in the `secrets` block → `user-key`; + (3) `ANIMUS_SECRET_PASSPHRASE` env var → `passphrase`; (4) `device-id` fallback + (interactive hosts). Steps 1–3 make headless/server deployments work without + setting `key_source` explicitly: supplying the key via env or file is enough, and + `auto` selects the right source automatically. This avoids the + keyring-unavailable hard error and prevents the device-id redeploy wipe caused by + a new machine-id on container rebuild. The hardware key sources (Secure Enclave / + DPAPI / TPM) are deferred — see "Platform support" below. - `user-key`: operator-supplied 32-byte key from `ANIMUS_SECRET_KEY` - (hex or base64) or a `secret_key_file` path. For headless/server with a - deploy-injected key (systemd `LoadCredential`, mounted secret, external KMS). + (hex or base64) or a `key_file` path. For headless/server with a deploy-injected + key (systemd `LoadCredential`, mounted secret, external KMS). - `passphrase`: `Argon2id(passphrase, salt)`. The passphrase arrives via `ANIMUS_SECRET_PASSPHRASE` for both the CLI and the daemon — env-driven and script-safe, with no TTY-only path that would break under automation. In @@ -101,11 +110,13 @@ already cover every target: ## Config (protocol::Config) -- `secret_backend: auto | keyring | device | env` — `auto` keeps existing - keyring installs on keyring (don't silently strand secrets), uses `device` for - fresh installs / where keyring is absent. -- `secret_key_source: auto | user-key | passphrase | device-id`. -- `secret_key_file: `. +The settings live in the `secrets` object in global or project `config.json`: + +- `secrets.backend: auto | keyring | device | env` — `auto` keeps existing + keyring installs on keyring (don't silently strand secrets) and selects + `device` when server key material is configured. +- `secrets.key_source: auto | user-key | passphrase | device-id`. +- `secrets.key_file: `. `animus secret migrate` moves secrets between keyring and the device store (idempotent, non-destructive by default). `env` always wins (unchanged). diff --git a/docs/reference/secrets.md b/docs/reference/secrets.md index e873e9f9..eca00bce 100644 --- a/docs/reference/secrets.md +++ b/docs/reference/secrets.md @@ -59,28 +59,36 @@ The index stores KEY names only — every actual value lives in the OS keychain. Two backends sit behind the same `animus secret` surface. The default is unchanged (OS keyring); the device-encrypted store is opt-in and exists for hosts where the keyring is awkward (a macOS binary whose signature changes re-prompts on every keychain access) or absent (a headless Linux server with no session keyring). -Select per machine in the **global** config (`~/.animus/config.json`, or `$ANIMUS_CONFIG_DIR`): +Configure this globally in `~/.animus/config.json` (or `$ANIMUS_CONFIG_DIR`), +or per project in `/.animus/config.json`. Project settings override +global settings field by field, and are honored by both `animus secret` and MCP +OAuth operations, including `animus mcp auth --complete`: ```jsonc { "secrets": { "backend": "device", // auto (default) | keyring | device | env "key_source": "device-id", // auto | user-key | passphrase | device-id - "key_file": "/path/to/key" // only for key_source = user-key + "key_file": "/path/to/key" // used by key_source = auto or user-key } } ``` - **`keyring`** — the OS keychain (macOS Keychain / libsecret / Windows Credential Manager). Default. - **`device`** — secrets live AEAD-sealed (ChaCha20-Poly1305) in `~/.animus//secrets/secrets.enc.v1` (`0600`). A random master key seals the data and is itself wrapped under a **key source**. No keychain, no prompts, and the file is useless if copied off the device. -- **`auto`** — keeps existing keyring installs on the keyring (never strands secrets); uses the device store once one exists for the scope. +- **`auto`** — keeps existing keyring installs on the keyring (never strands + secrets); selects the device store when a server key is supplied through + `ANIMUS_SECRET_KEY`, `ANIMUS_SECRET_PASSPHRASE`, or `key_file`, and continues + using the device store once one exists for the scope. Setting `key_file` + is sufficient; `key_source: "user-key"` is optional when `backend` and + `key_source` remain `auto`. ### Key sources (what wraps the device store's master key) - **`device-id`** — `HKDF(machine-id + per-install salt)`. The machine id never travels with the file, so an off-device copy can't decrypt. Cross-platform, no prompt. Default fallback. - **`user-key`** — an operator-supplied 32-byte key from `ANIMUS_SECRET_KEY` (hex/base64) or `key_file`. For headless/server with a deploy-injected key (systemd `LoadCredential`, mounted secret, external KMS). - **`passphrase`** — `Argon2id` over a passphrase read from `ANIMUS_SECRET_PASSPHRASE`. Env-driven for both the CLI and the daemon (so the mode is script-safe and behaves identically everywhere); in exposure terms a non-interactive passphrase is equivalent to `user-key`. The store errors with that variable name when it is unset. -- **`auto`** — resolves to `device-id` today. The OS hardware-backed sources (Secure Enclave / DPAPI / TPM) are deferred (see the architecture doc for the per-platform reasons), so `auto` is currently equivalent to `device-id`. +- **`auto`** — resolves in priority order: (1) `ANIMUS_SECRET_KEY` env var → `user-key`; (2) `key_file` configured in the `secrets` block → `user-key`; (3) `ANIMUS_SECRET_PASSPHRASE` env var → `passphrase`; (4) `device-id` fallback. Steps 1–3 let headless/server deployments work without setting `key_source` explicitly — just supply the key material via env or file and `auto` selects the right source. The OS hardware-backed sources (Secure Enclave / DPAPI / TPM) are deferred. ### Moving between backends @@ -149,7 +157,9 @@ What the design does NOT defend against: ## Headless and CI -CI containers typically have no D-Bus session and no macOS Keychain. The recommended pattern is to keep using process env on the CI box: +CI containers and servers typically have no D-Bus session or macOS Keychain. +For ephemeral CI jobs, keep passing application credentials through the process +environment: ```sh LINEAR_API_TOKEN=$LINEAR_API_TOKEN \ @@ -157,7 +167,29 @@ OPENAI_API_KEY=$OPENAI_API_KEY \ animus daemon start ``` -The keychain layer is a no-op in this configuration — the index file is empty, the spawn-path merge is empty, and behavior is byte-identical to pre-v0.5.8. +For a long-running server that must persist Animus secrets or MCP OAuth tokens, +configure a durable `key_file` in the `secrets` block or inject the same +32-byte `ANIMUS_SECRET_KEY` on every start. With the default `auto` settings, +either source selects the device-encrypted backend without requiring a desktop +keyring. Back up the key separately from the encrypted store: losing or changing +it makes the stored secrets unrecoverable. + +For example, a project-level `.animus/config.json` can select a key file without +requiring `ANIMUS_SECRET_KEY` in the environment: + +```json +{ + "secrets": { + "key_source": "user-key", + "key_file": "/run/secrets/animus-secret-key" + } +} +``` + +The file must contain exactly 32 bytes encoded as 64 hexadecimal characters or +base64. `ANIMUS_SECRET_KEY` accepts the same formats and takes precedence over +`key_file` when both are present. Both the CLI secret commands and MCP OAuth +operations honor the project-level `secrets` block. ## Audit log @@ -202,7 +234,6 @@ The following are non-goals and will land (or not) in v0.6+: - Encrypted file backends (age, sops, similar) — only OS keychain for v0.5.8. - External secret brokers (1Password / HashiCorp Vault / Doppler) — defer until the plugin role for "secret broker" is designed. - Cross-machine sync — the keychain is local-only by design. -- Headless / Linux-no-D-Bus mode — for now CI uses process env directly. - TUI for managing secrets. [RBAC]: ./security.md