diff --git a/crates/animus-mcp-oauth/src/config.rs b/crates/animus-mcp-oauth/src/config.rs index b67a95ad..092bf09d 100644 --- a/crates/animus-mcp-oauth/src/config.rs +++ b/crates/animus-mcp-oauth/src/config.rs @@ -52,11 +52,16 @@ pub struct ServerResolution { /// Build a keychain-backed [`SecretStore`] for `project_root`, mirroring the /// `animus secret` surface so OAuth tokens share the project's keychain /// 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()))?; let scope = resolve_keychain_scope(project_root, &scoped_root); - Ok(Arc::from(orchestrator_core::build_secret_store(&scope, scoped_root))) + Ok(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 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..1e814ffb 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,8 @@ 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 +288,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..6615eebb 100644 --- a/crates/orchestrator-core/src/lib.rs +++ b/crates/orchestrator-core/src/lib.rs @@ -96,7 +96,7 @@ 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..20df5fb2 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,96 @@ 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::{ENV_PASSPHRASE, ENV_USER_KEY}; + matches!(cfg.key_source.as_deref(), Some("user-key") | Some("user_key") | Some("passphrase")) + || cfg.key_file.is_some() + || std::env::var(ENV_USER_KEY).is_ok() + || std::env::var(ENV_PASSPHRASE).is_ok() +} + +/// 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 @@ -377,6 +473,7 @@ fn key_source_config(cfg: &protocol::SecretsConfig) -> KeySourceConfig { mod tests { use super::*; use crate::secret_keysource::{KeySourceConfig, KeySourceKind}; + use crate::secret_keysource::tests::env_lock; // A user-key store backed by a per-test key FILE, so tests need no shared // process env and never race each other. @@ -392,6 +489,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 +509,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 +519,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 +532,189 @@ 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, + "backend": "device" + } + }); + 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 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::{ENV_PASSPHRASE, ENV_USER_KEY}; + use crate::secret_keysource::tests::env_lock; + 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 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(); + 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"); + 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!(store.get("FOO").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::{ENV_PASSPHRASE, ENV_USER_KEY}; + use crate::secret_keysource::tests::env_lock; + 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"); + 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!(store.get("MIGRATE_KEY").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..8eb39bff 100644 --- a/crates/orchestrator-core/src/secret_keysource.rs +++ b/crates/orchestrator-core/src/secret_keysource.rs @@ -306,20 +306,59 @@ 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> { +/// 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. `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() || config.key_file.is_some() { + return Ok(Box::new(UserKeySource::resolve(config.key_file.as_deref())?)); + } + // Passphrase env var: also a headless-safe server source. The backend + // selector (`has_server_key_configured`) already counts this as a + // "server key" and picks the device backend; resolve to the matching + // key source here so the two paths stay in sync. + if std::env::var(ENV_PASSPHRASE).is_ok() { + return Ok(Box::new(PassphraseKeySource::resolve(None, 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() { @@ -352,6 +391,79 @@ 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_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_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..d0c837e5 100644 --- a/docs/architecture/secret-backends.md +++ b/docs/architecture/secret-backends.md @@ -64,12 +64,18 @@ trait KeySource { fn key(&self) -> Result>; fn id(&self) -> 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. +- `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 diff --git a/docs/reference/secrets.md b/docs/reference/secrets.md index e873e9f9..9fcb6b83 100644 --- a/docs/reference/secrets.md +++ b/docs/reference/secrets.md @@ -80,7 +80,7 @@ Select per machine in the **global** config (`~/.animus/config.json`, or `$ANIMU - **`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