diff --git a/desktop/src-tauri/src/managed_agents/custom_harnesses.rs b/desktop/src-tauri/src/managed_agents/custom_harnesses.rs index 3329674f65..5557d975a1 100644 --- a/desktop/src-tauri/src/managed_agents/custom_harnesses.rs +++ b/desktop/src-tauri/src/managed_agents/custom_harnesses.rs @@ -18,6 +18,12 @@ use std::path::Path; use serde::{Deserialize, Serialize}; +use crate::managed_agents::secret_projection::{ + cancel_gc_candidacy, deserialize_env_map, load_secret, serialize_env_map, write_secret, + ProjectionStore, WriteOutcome, +}; +use crate::managed_agents::secret_seam::{migrate_inline_field, FieldMigration}; + /// Regex-equivalent predicate for a valid harness ID. /// /// IDs must match `[a-z0-9_][a-z0-9_-]*` — lowercase alphanumeric plus @@ -59,8 +65,20 @@ pub(crate) struct HarnessDefinition { /// Environment variables injected at spawn time. Definition env is applied /// first and LOSES on conflict with Buzz-injected vars — `BUZZ_MANAGED_AGENT` /// is always authoritative and cannot be overridden here. + /// + /// Secret-projection contract (mirrors `ManagedAgentRecord.env_vars`): on a + /// keyring-backed save this map is stripped to the OS keyring and left empty + /// on disk, with [`env_ref`](Self::env_ref) pointing at the generation. When + /// non-empty on disk it is the authoritative inline-fallback state (keyring + /// unavailable) and wins over any stale `env_ref` on hydration. #[serde(default)] pub env: BTreeMap, + /// Generation reference for the keyring-projected `env` map, set when the + /// inline `env` was stripped into the keyring under `harness::env:`. + /// Non-secret pointer only. Absent when the definition carries no env, or on + /// a keyless build where env stays inline in the `0o600` JSON. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub env_ref: Option, /// Link to external docs for manual install/setup instructions. #[serde(default)] pub install_instructions_url: String, @@ -69,7 +87,133 @@ pub(crate) struct HarnessDefinition { pub install_hint: String, } -/// Scan `dir` for `*.json` files and deserialize each into a `HarnessDefinition`. +/// Keyring blob coordinate for a harness definition's projected `env` map. +/// +/// The `harness:` namespace is deliberately distinct from the agent-store +/// namespaces (`global:env:` / `agent:…` / `definition:…`) that +/// [`crate::managed_agents::secret_projection::is_projection_key`] matches, so +/// the agent-store GC sweeps never touch harness generations. Harness env edits +/// are rare, so the un-reclaimed generations are a bounded, encrypted-at-rest +/// cost documented as a known limitation. +pub(crate) fn harness_env_key(id: &str, gen: &str) -> String { + format!("harness:{id}:env:{gen}") +} + +// ── Env secret projection (mirrors the agent-store `secret_seam`) ──────────── +// +// A harness definition carries a single `env` map that can hold provider +// secrets (e.g. `ANTHROPIC_API_KEY`). These three helpers move it between the +// on-disk JSON and the OS keyring under the `harness::env:` coordinate. +// Each is generic over [`ProjectionStore`] so tests drive a fake and never the +// live OS keyring (the default `system-keyring` feature makes the live store +// real under `cargo test`). + +/// Save-path strip: project a non-empty inline `env` into the keyring under a +/// fresh generation and clear it on disk; an empty inline `env` clears the ref +/// (a UI save carries the user's full intent — empty means "no env"). On a +/// keyring write failure the value stays inline (`0o600` fallback) with the ref +/// cleared. Mutates `def` in place. +/// +/// Unlike the agent seam there is no `secrets_unavailable` guard: a harness save +/// is always a fresh, complete UI submission, never a re-save of a partially +/// hydrated record, so empty inline is unambiguously a user-clear here. +fn strip_harness_env(store: &S, def: &mut HarnessDefinition) { + let id = def.id.clone(); + let inline_env = if !def.env.is_empty() { + serialize_env_map(&def.env).ok() + } else { + None + }; + match write_secret( + store, + |gen| harness_env_key(&id, gen), + inline_env.as_deref(), + &format!("harness:{id} env"), + ) { + WriteOutcome::Persisted { gen } => { + cancel_gc_candidacy(store, &harness_env_key(&id, &gen)); + def.env.clear(); + def.env_ref = Some(gen); + } + WriteOutcome::KeptInline { .. } | WriteOutcome::Nothing => { + def.env_ref = None; + } + } +} + +/// Load-path hydrate: fill an empty on-disk `env` from the keyring generation +/// named by `env_ref`. A non-empty inline `env` is the authoritative +/// keyring-unavailable fallback and wins over any ref. A hydration failure is +/// logged and leaves `env` empty — harness env is optional (unlike an agent +/// nsec), so the harness still resolves rather than failing the whole load. +fn hydrate_harness_env(store: &S, def: &mut HarnessDefinition) { + if !def.env.is_empty() { + return; // inline fallback is authoritative + } + let id = def.id.clone(); + match load_secret( + store, + def.env_ref.as_deref(), + |gen| harness_env_key(&id, gen), + &format!("harness:{id} env"), + ) { + Ok(Some(s)) => match deserialize_env_map(&s) { + Ok(map) => def.env = map, + Err(e) => { + tracing::warn!("custom_harnesses: harness {id} env deserialize failed: {e}") + } + }, + Ok(None) => {} + Err(e) => tracing::warn!("custom_harnesses: {e}"), + } +} + +/// Boot-migration transition: W1-safe field migration of one definition's +/// `env`. An absent inline value here means "already projected on a prior +/// launch," so an existing ref is preserved rather than cleared (the +/// distinction from [`strip_harness_env`], which reads empty as a user-clear). +/// Returns `true` when the ref changed and the file must be rewritten. +/// +/// Shares the single W1-safe seam +/// ([`crate::managed_agents::secret_seam::migrate_inline_field`]) with the agent +/// tiers so the two surfaces cannot diverge on the empty-inline semantics. +pub(crate) fn migrate_harness_env( + store: &S, + def: &mut HarnessDefinition, +) -> bool { + let id = def.id.clone(); + let inline_env = if !def.env.is_empty() { + serialize_env_map(&def.env).ok() + } else { + None + }; + match migrate_inline_field( + store, + |gen| harness_env_key(&id, gen), + inline_env.as_deref(), + def.env_ref.as_deref(), + &format!("harness:{id} env"), + ) { + FieldMigration::Projected { gen } => { + def.env.clear(); + def.env_ref = Some(gen); + true + } + FieldMigration::Preserved | FieldMigration::Cleared => false, + } +} + +/// Resolve the live secret-projection store, or `None` on a keyless build. +/// +/// Consumes the agent-store resolver so harness env shares the exact same +/// `SecretStore` instance (one cache, one mutex) as every other projected +/// secret — never a second handle that could race on the blob. +fn live_projection_store() -> Option<&'static crate::secret_store::SecretStore> { + crate::managed_agents::storage::agent_secret_store_pub() +} + +/// Scan `dir` for `*.json` files and deserialize each into a `HarnessDefinition`, +/// hydrating each definition's `env` from the live keyring. /// /// Errors per file are logged with `tracing::warn` and skipped — a single /// malformed file never fails discovery for the rest. Returns only @@ -86,6 +230,28 @@ pub(crate) struct HarnessDefinition { /// call** — this function performs no caching, mirroring goose's /// `refresh_custom_providers()` pattern. pub(crate) fn load_custom_harnesses(dir: &Path) -> Vec { + load_custom_harnesses_with(live_projection_store(), dir) +} + +/// Testable core of [`load_custom_harnesses`], generic over the projection +/// store so tests drive a [`ProjectionStore`] fake instead of the live keyring. +/// Scans + validates the directory, then hydrates each definition's `env`. +pub(crate) fn load_custom_harnesses_with( + store: Option<&S>, + dir: &Path, +) -> Vec { + let mut definitions = load_custom_harnesses_from_disk(dir); + if let Some(store) = store { + for def in &mut definitions { + hydrate_harness_env(store, def); + } + } + definitions +} + +/// Raw directory scan + per-file validation, WITHOUT env hydration. The +/// store-independent half of [`load_custom_harnesses_with`]. +fn load_custom_harnesses_from_disk(dir: &Path) -> Vec { let entries = match std::fs::read_dir(dir) { Ok(e) => e, Err(err) if err.kind() == std::io::ErrorKind::NotFound => return vec![], @@ -397,18 +563,9 @@ pub(crate) struct SaveOutcome { pub removed_old_path: Option, } -/// Write a harness definition to `dir/.json` using a backup-swap strategy -/// that is safe on all platforms (including Windows where `fs::rename` over an -/// existing file fails with "access denied"): -/// -/// 1. Serialize the definition and write it to a unique temp file via -/// `atomic_write_file`. -/// 2. If the target already exists, rename it to `.bak` (the backup). -/// 3. `commit()` the temp file (renames temp → target). -/// * On success: delete `.bak` (best-effort; a stale `.bak` is harmless). -/// * On failure: restore `.bak` → target so the original is never lost. -/// 4. If `rename_old_id` is `Some`, remove `/.json` after the -/// new file is committed (non-fatal if NotFound). +/// Write a harness definition to `dir/.json`, projecting its `env` secrets +/// into the OS keyring first. The resolved live [`ProjectionStore`] is used; +/// see [`save_custom_harness_to_dir_with`] for the store-injected core. /// /// The caller is responsible for full validation (id, env, etc.) BEFORE /// calling this function — no validation is performed here. @@ -416,19 +573,65 @@ pub(crate) fn save_custom_harness_to_dir( dir: &Path, definition: &HarnessDefinition, rename_old_id: Option<&str>, +) -> Result { + save_custom_harness_to_dir_with(live_projection_store(), dir, definition, rename_old_id) +} + +/// Store-injected core of [`save_custom_harness_to_dir`]: strip `env` secrets +/// into `store`, then write the stripped definition to `dir/.json` using a +/// backup-swap strategy that is safe on all platforms (including Windows where +/// `fs::rename` over an existing file fails with "access denied"): +/// +/// 1. Serialize the STRIPPED definition and write it to a unique temp file via +/// `atomic_write_file`, created `0o600` before any bytes hit disk (the JSON +/// carries inline env in the keyless / keyring-unavailable fallback). +/// 2. If the target already exists, rename it to `.bak` (the backup). +/// 3. `commit()` the temp file (renames temp → target). +/// * On success: delete `.bak` (best-effort; a stale `.bak` is harmless). +/// * On failure: restore `.bak` → target so the original is never lost. +/// 4. If `rename_old_id` is `Some`, remove `/.json` after the +/// new file is committed (non-fatal if NotFound). +/// +/// `definition` is NOT mutated — a clone is stripped so the caller keeps the +/// full `env` for the returned catalog entry (edit round-trip). On a keyless +/// build (`store` is `None`) the `env` stays inline in the `0o600` JSON. +/// +/// No cross-process secret-transaction lock is taken here: the `harness:` +/// keyring namespace is deliberately excluded from the agent-store GC +/// ([`crate::managed_agents::secret_projection::is_projection_key`]), so no +/// concurrent sweep can ever delete a harness generation between its write and +/// its JSON commit — the window that lock exists to close does not exist here. +pub(crate) fn save_custom_harness_to_dir_with( + store: Option<&S>, + dir: &Path, + definition: &HarnessDefinition, + rename_old_id: Option<&str>, ) -> Result { use atomic_write_file::AtomicWriteFile; use std::io::Write; - let json = serde_json::to_string_pretty(definition) + // Strip env → keyring on a clone. The keyring write happens BEFORE the + // atomic JSON commit (the commit point), mirroring the agent-store seam. + let mut to_write = definition.clone(); + if let Some(store) = store { + strip_harness_env(store, &mut to_write); + } + + let json = serde_json::to_string_pretty(&to_write) .map_err(|e| format!("failed to serialize harness definition: {e}"))?; - let target_path = dir.join(format!("{}.json", definition.id)); - let bak_path = dir.join(format!("{}.json.bak", definition.id)); + let target_path = dir.join(format!("{}.json", to_write.id)); + let bak_path = dir.join(format!("{}.json.bak", to_write.id)); - // Stage write to temp file. + // Stage write to temp file, owner-only before any bytes hit disk. let mut file = AtomicWriteFile::open(&target_path) .map_err(|e| format!("failed to open {}: {e}", target_path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + file.set_permissions(std::fs::Permissions::from_mode(0o600)) + .map_err(|e| format!("failed to set {} permissions: {e}", target_path.display()))?; + } file.write_all(json.as_bytes()) .map_err(|e| format!("failed to write harness definition: {e}"))?; diff --git a/desktop/src-tauri/src/managed_agents/custom_harnesses_tests.rs b/desktop/src-tauri/src/managed_agents/custom_harnesses_tests.rs index 09273f036a..29e5bc7c6c 100644 --- a/desktop/src-tauri/src/managed_agents/custom_harnesses_tests.rs +++ b/desktop/src-tauri/src/managed_agents/custom_harnesses_tests.rs @@ -257,6 +257,7 @@ fn make_def(id: &str, label: &str) -> HarnessDefinition { command: format!("{id}-bin"), args: vec![], env: BTreeMap::new(), + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), } @@ -355,30 +356,246 @@ fn save_to_dir_rename_nonexistent_old_id_is_non_fatal() { assert!(load_custom_harnesses(dir.path()).len() == 1); } -#[test] -fn save_to_dir_roundtrip_with_env_preserves_values() { - let dir = tempfile::tempdir().unwrap(); +// ── Env secret projection (save strips → keyring; load hydrates) ───────── +// +// These drive the store-injected `*_with` seams against an in-memory fake so +// they are deterministic and never touch the live OS keyring (the default +// `system-keyring` feature makes the live store real under `cargo test`). +// The fake mirrors `secret_seam_tests::FakeProjectionStore`. + +use crate::managed_agents::secret_projection::{deserialize_env_map, ProjectionStore}; +use std::cell::RefCell; +use std::collections::HashMap; + +/// In-memory projection store: every write succeeds and is recoverable. +struct FakeProjectionStore { + data: RefCell>, +} + +impl FakeProjectionStore { + fn new() -> Self { + Self { + data: RefCell::new(HashMap::new()), + } + } + fn len(&self) -> usize { + self.data.borrow().len() + } +} + +impl ProjectionStore for FakeProjectionStore { + fn write_and_verify(&self, key: &str, value: &str) -> Result<(), String> { + self.data + .borrow_mut() + .insert(key.to_string(), value.to_string()); + Ok(()) + } + fn load_key(&self, key: &str) -> Result, String> { + Ok(self.data.borrow().get(key).cloned()) + } + fn load_all(&self) -> Result>, String> { + Ok(Some(self.data.borrow().clone())) + } + fn store_batch(&self, entries: &HashMap) -> Result<(), String> { + for (k, v) in entries { + self.data.borrow_mut().insert(k.clone(), v.clone()); + } + Ok(()) + } + fn remove_batch(&self, keys: &[&str]) -> Result<(), String> { + for k in keys { + self.data.borrow_mut().remove(*k); + } + Ok(()) + } +} + +/// Projection store whose writes always fail — models a keyring outage that +/// forces the inline `0o600` fallback (`WriteOutcome::KeptInline`). +struct FailingWriteStore; + +impl ProjectionStore for FailingWriteStore { + fn write_and_verify(&self, _key: &str, _value: &str) -> Result<(), String> { + Err("simulated keyring write failure".to_string()) + } + fn load_key(&self, _key: &str) -> Result, String> { + Ok(None) + } + fn load_all(&self) -> Result>, String> { + Ok(Some(HashMap::new())) + } + fn store_batch(&self, _entries: &HashMap) -> Result<(), String> { + Ok(()) + } + fn remove_batch(&self, _keys: &[&str]) -> Result<(), String> { + Ok(()) + } +} + +fn env_harness_def() -> HarnessDefinition { let mut env = BTreeMap::new(); - env.insert("MY_KEY".to_string(), "my_value".to_string()); - let def = HarnessDefinition { + env.insert("ANTHROPIC_API_KEY".to_string(), "sk-ant-secret".to_string()); + HarnessDefinition { id: "env-harness".to_string(), label: "Env Harness".to_string(), command: "env-bin".to_string(), args: vec!["--flag".to_string()], env, + env_ref: None, install_instructions_url: "https://example.com".to_string(), install_hint: "Install from example.com".to_string(), - }; + } +} - save_custom_harness_to_dir(dir.path(), &def, None).unwrap(); +#[test] +fn save_projects_env_to_keyring_and_leaves_no_plaintext_on_disk() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + let def = env_harness_def(); - let loaded = load_custom_harnesses(dir.path()); + let outcome = save_custom_harness_to_dir_with(Some(&store), dir.path(), &def, None).unwrap(); + + let raw = fs::read_to_string(&outcome.target_path).unwrap(); + assert!( + !raw.contains("sk-ant-secret"), + "the secret value must never be written to disk in plaintext" + ); + // The on-disk record carries an env_ref and an empty inline env. + let on_disk: HarnessDefinition = serde_json::from_str(&raw).unwrap(); + assert!( + on_disk.env.is_empty(), + "inline env must be stripped on disk" + ); + let gen = on_disk + .env_ref + .expect("env_ref must point at the projected gen"); + // The keyring fake holds the env verbatim under the harness coordinate. + let stored = store + .load_key(&harness_env_key("env-harness", &gen)) + .unwrap() + .expect("keyring must hold the projected env"); + assert_eq!( + deserialize_env_map(&stored) + .unwrap() + .get("ANTHROPIC_API_KEY"), + Some(&"sk-ant-secret".to_string()) + ); +} + +#[test] +fn save_then_load_with_same_fake_roundtrips_env_values() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + let def = env_harness_def(); + + save_custom_harness_to_dir_with(Some(&store), dir.path(), &def, None).unwrap(); + + let loaded = load_custom_harnesses_with(Some(&store), dir.path()); assert_eq!(loaded.len(), 1); assert_eq!(loaded[0].args, vec!["--flag"]); assert_eq!( - loaded[0].env.get("MY_KEY").map(String::as_str), - Some("my_value"), - "env must round-trip through save_custom_harness_to_dir" + loaded[0].env.get("ANTHROPIC_API_KEY").map(String::as_str), + Some("sk-ant-secret"), + "env must hydrate back from the keyring on load" + ); +} + +#[test] +fn save_does_not_mutate_the_caller_definition() { + // The catalog entry the UI keeps after a save must retain the full env for + // the edit round-trip — the seam strips a clone, never the caller's def. + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + let def = env_harness_def(); + + save_custom_harness_to_dir_with(Some(&store), dir.path(), &def, None).unwrap(); + + assert_eq!( + def.env.get("ANTHROPIC_API_KEY").map(String::as_str), + Some("sk-ant-secret"), + "caller's definition must keep its full env after save" + ); + assert!(def.env_ref.is_none(), "caller's def must be untouched"); +} + +#[test] +fn save_empty_env_writes_no_ref_and_no_keyring_entry() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + let def = make_def("bare", "Bare"); // env is empty + + let outcome = save_custom_harness_to_dir_with(Some(&store), dir.path(), &def, None).unwrap(); + + assert_eq!( + store.len(), + 0, + "an empty env must not mint a keyring generation" + ); + let on_disk: HarnessDefinition = + serde_json::from_str(&fs::read_to_string(&outcome.target_path).unwrap()).unwrap(); + assert!(on_disk.env.is_empty()); + assert!(on_disk.env_ref.is_none(), "no env means no env_ref on disk"); +} + +#[test] +fn save_keyring_write_failure_keeps_env_inline_with_ref_cleared() { + // A keyring outage must fall back to the inline `0o600` JSON so the harness + // still resolves — with env_ref cleared (inline is authoritative). + let store = FailingWriteStore; + let dir = tempfile::tempdir().unwrap(); + let def = env_harness_def(); + + let outcome = save_custom_harness_to_dir_with(Some(&store), dir.path(), &def, None).unwrap(); + + let on_disk: HarnessDefinition = + serde_json::from_str(&fs::read_to_string(&outcome.target_path).unwrap()).unwrap(); + assert_eq!( + on_disk.env.get("ANTHROPIC_API_KEY").map(String::as_str), + Some("sk-ant-secret"), + "on a keyring write failure the env stays inline as the fallback" + ); + assert!( + on_disk.env_ref.is_none(), + "the ref must be cleared so inline wins on the next hydrate" + ); +} + +/// Inline env survives even without a store (keyless build): the env is written +/// inline to the `0o600` JSON and hydrates straight from disk. +#[test] +fn save_without_store_keeps_env_inline_and_roundtrips() { + let dir = tempfile::tempdir().unwrap(); + let def = env_harness_def(); + + save_custom_harness_to_dir_with::(None, dir.path(), &def, None).unwrap(); + + let loaded = load_custom_harnesses_with::(None, dir.path()); + assert_eq!(loaded.len(), 1); + assert_eq!( + loaded[0].env.get("ANTHROPIC_API_KEY").map(String::as_str), + Some("sk-ant-secret"), + "keyless build must keep env inline and round-trip it" + ); +} + +#[cfg(unix)] +#[test] +fn save_written_file_is_owner_only_0o600() { + use std::os::unix::fs::PermissionsExt; + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + let def = env_harness_def(); + + let outcome = save_custom_harness_to_dir_with(Some(&store), dir.path(), &def, None).unwrap(); + + let mode = fs::metadata(&outcome.target_path) + .unwrap() + .permissions() + .mode(); + assert_eq!( + mode & 0o777, + 0o600, + "harness file must be created owner-only before any bytes hit disk" ); } @@ -397,6 +614,7 @@ fn validate_rejects_malformed_key_with_equals_sign() { command: "bad-bin".to_string(), args: vec![], env, + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), }; @@ -426,6 +644,7 @@ fn validate_rejects_reserved_key_buzz_managed_agent() { command: "bad-bin".to_string(), args: vec![], env, + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), }; @@ -447,6 +666,7 @@ fn validate_rejects_reserved_key_case_insensitive() { command: "ci-bin".to_string(), args: vec![], env, + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), }; @@ -468,6 +688,7 @@ fn validate_rejects_nul_byte_in_value() { command: "nul-bin".to_string(), args: vec![], env, + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), }; @@ -490,6 +711,7 @@ fn validate_rejects_value_over_per_value_size_limit() { command: "big-bin".to_string(), args: vec![], env, + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), }; @@ -511,6 +733,7 @@ fn validate_accepts_well_formed_env() { command: "good-bin".to_string(), args: vec![], env, + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), }; diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index d86e5f33f0..bf3d0b108b 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -192,6 +192,7 @@ pub(crate) fn preset_harness_definitions( command: preset.command.to_string(), args: preset.args.iter().map(|arg| arg.to_string()).collect(), env: Default::default(), + env_ref: None, install_instructions_url: preset.install_instructions_url.to_string(), install_hint: preset.install_hint.to_string(), }, diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 9dd8e88db1..e6e2cfe57e 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -1608,6 +1608,7 @@ fn deleted_harness_summary_display_and_spawn_sentence_agree() { command: "doomed-bin".to_string(), args: vec![], env: Default::default(), + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), }; @@ -1752,6 +1753,7 @@ fn harness_def( command: command.to_string(), args: vec![], env: Default::default(), + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), } diff --git a/desktop/src-tauri/src/managed_agents/persona_events/stale_pin_tests.rs b/desktop/src-tauri/src/managed_agents/persona_events/stale_pin_tests.rs index 2bd7ba3d1c..5c876bfb0b 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events/stale_pin_tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events/stale_pin_tests.rs @@ -128,6 +128,7 @@ fn apply_persona_snapshot_goose_to_custom_harness_drops_stale_goose_pin() { command: "my-custom-bin".to_string(), args: vec![], env: BTreeMap::new(), + env_ref: None, install_instructions_url: String::new(), install_hint: String::new(), }]); diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 6286c36e98..09f01dc713 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -192,6 +192,7 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { reconcile_databricks_v1_to_v2(app); materialize_agent_runtimes(app); migrate_inline_secrets_to_keyring(app); // keyring extraction — after all raw-JSON migrations + migrate_harness_secrets_to_keyring(app); // custom-harness env extraction — same phase #[cfg(debug_assertions)] // dev-build only; runs after extraction so keys exist in the source if is_dev { crate::managed_agents::migrate_agent_secrets_to_dev_service(app); @@ -1279,6 +1280,8 @@ mod migration_secrets; use migration_secrets::migrate_inline_secrets_to_keyring; mod databricks_reconcile; use databricks_reconcile::reconcile_databricks_v1_to_v2; +mod migration_harness_secrets; +use migration_harness_secrets::migrate_harness_secrets_to_keyring; #[cfg(test)] #[path = "migration_avatar_tests.rs"] mod avatar_tests; diff --git a/desktop/src-tauri/src/migration/migration_harness_secrets.rs b/desktop/src-tauri/src/migration/migration_harness_secrets.rs new file mode 100644 index 0000000000..f02040fe8a --- /dev/null +++ b/desktop/src-tauri/src/migration/migration_harness_secrets.rs @@ -0,0 +1,268 @@ +//! Boot-time secret extraction for custom harness definitions. +//! +//! Custom harnesses (`/custom_harnesses/*.json`) carry an `env` map +//! that can hold provider secrets (e.g. `ANTHROPIC_API_KEY`). Before the +//! generation-reference projection this map was written plaintext to disk. This +//! migration lifts each inline `env` into the OS keyring under the +//! `harness::env:` coordinate, rewrites the file stripped at `0o600`, +//! and — once every projected generation reads back cleanly — scrubs the +//! plaintext-bearing backup/temp artifacts the save path can leave behind. +//! +//! Runs at boot alongside [`super::migration_secrets`], BEFORE +//! `warm_harness_registry_from_dir` hydrates the spawn registry. +//! +//! Idempotent across launches: extraction goes through the W1-safe field +//! migration seam ([`crate::managed_agents::custom_harnesses::migrate_harness_env`]), +//! which projects only a non-empty inline value and otherwise preserves an +//! existing ref. A second launch re-reads already-projected files (empty inline +//! + live ref) and rewrites nothing. + +use std::path::Path; + +use tauri::Manager; + +use crate::managed_agents::custom_harnesses::{ + harness_env_key, migrate_harness_env, HarnessDefinition, +}; +use crate::managed_agents::secret_projection::{load_secret, ProjectionStore}; + +/// Extract inline harness `env` secrets into the keyring, then scrub plaintext +/// artifacts once the projection is verified readable. +/// +/// Non-fatal throughout: a keyless build, an unresolvable app-data dir, or an +/// unreadable directory each short-circuit without disturbing the rest of boot. +pub(super) fn migrate_harness_secrets_to_keyring(app: &tauri::AppHandle) { + let Some(store) = crate::managed_agents::storage::agent_secret_store_pub() else { + return; // keyless build — env stays inline in the 0o600 JSON + }; + let Ok(data_dir) = app.path().app_data_dir() else { + return; + }; + let dir = data_dir.join("custom_harnesses"); + if !dir.exists() { + return; + } + migrate_harness_secrets_in_dir(store, &dir); +} + +/// Store-injected core of [`migrate_harness_secrets_to_keyring`], over a +/// concrete `dir`, so the full extract → verify → scrub flow is testable +/// against a [`ProjectionStore`] fake and a tempdir without an `AppHandle`. +fn migrate_harness_secrets_in_dir(store: &S, dir: &Path) { + // Phase 1: extract inline env → keyring, rewriting each changed file in + // place (stripped, 0o600). Rewrite the ORIGINAL path, never an id-derived + // one: a hand-authored file's name may differ from its `id`, and relocating + // it would orphan the original and duplicate the entry. + for path in harness_json_files(dir) { + let Some(mut def) = read_definition(&path) else { + continue; // unparseable live file — the loader already skips it + }; + if migrate_harness_env(store, &mut def) { + match serde_json::to_string_pretty(&def) { + Ok(json) => { + if let Err(e) = crate::managed_agents::storage::atomic_write_json_restricted( + &path, + json.as_bytes(), + ) { + eprintln!( + "buzz-desktop: harness-migration: failed to rewrite {}: {e}", + path.display() + ); + } + } + Err(e) => eprintln!( + "buzz-desktop: harness-migration: failed to serialize {}: {e}", + path.display() + ), + } + } + } + + // Phase 2: scrub plaintext artifacts — ONLY when every live file's projected + // env reads back cleanly. A dangling ref means the keyring copy is + // unavailable, so a backup could be the last recoverable plaintext; leave it. + if extraction_verified(store, dir) { + cleanup_harness_artifacts(dir); + } +} + +/// Returns `true` when every live `*.json` file's `env_ref` (if any) hydrates +/// from the keyring without error. A single failure returns `false` so the +/// caller skips the plaintext-artifact scrub this boot. +fn extraction_verified(store: &S, dir: &Path) -> bool { + for path in harness_json_files(dir) { + let Some(def) = read_definition(&path) else { + continue; + }; + // A non-empty inline env is the authoritative fallback (keyring write + // failed this boot) — it is not projected, so nothing to verify. + if !def.env.is_empty() { + continue; + } + let id = def.id.clone(); + match load_secret( + store, + def.env_ref.as_deref(), + |gen| harness_env_key(&id, gen), + &format!("harness:{id} env"), + ) { + Ok(_) => {} + Err(e) => { + eprintln!( + "buzz-desktop: harness-migration: extraction-verify failed for {}: {e} — \ + skipping artifact scrub this boot", + path.display() + ); + return false; + } + } + } + true +} + +/// Scrub plaintext-bearing artifacts in the custom-harness dir: `*.json.bak` +/// backups (from the save path's backup-swap) and atomic-write temp siblings. +/// +/// - Parseable backup → strip `env` and rewrite at `0o600` (non-secret fields +/// survive for manual recovery). +/// - Unparseable backup or atomic-write temp → delete (may carry plaintext). +/// - Live `*.json` files are NOT touched here — Phase 1 already stripped them. +/// - Never follows a symlink that escapes the dir. +fn cleanup_harness_artifacts(dir: &Path) { + let canonical_dir = match std::fs::canonicalize(dir) { + Ok(p) => p, + Err(e) => { + eprintln!( + "buzz-desktop: harness-migration: cannot canonicalize {}: {e}", + dir.display() + ); + return; + } + }; + let entries = match std::fs::read_dir(&canonical_dir) { + Ok(e) => e, + Err(e) => { + eprintln!( + "buzz-desktop: harness-migration: cannot read {}: {e}", + canonical_dir.display() + ); + return; + } + }; + + for entry in entries.flatten() { + let path = entry.path(); + let real_path = match std::fs::canonicalize(&path) { + Ok(p) => p, + Err(_) => path.clone(), + }; + if !real_path.starts_with(&canonical_dir) { + eprintln!( + "buzz-desktop: harness-migration: skipping symlink escape: {}", + path.display() + ); + continue; + } + let meta = match std::fs::symlink_metadata(&path) { + Ok(m) => m, + Err(_) => continue, + }; + if !meta.is_file() { + continue; + } + let fname = match path.file_name().and_then(|n| n.to_str()) { + Some(s) => s.to_string(), + None => continue, + }; + + if is_atomic_write_temp(&fname) { + if let Err(e) = std::fs::remove_file(&real_path) { + eprintln!("buzz-desktop: harness-migration: cannot remove temp {fname}: {e}"); + } + continue; + } + if is_harness_backup(&fname) { + scrub_backup_file(&real_path, &fname); + } + } +} + +/// Strip the `env` field from a parseable harness backup and rewrite it at +/// `0o600`; delete the file when it cannot be parsed (may carry plaintext). +fn scrub_backup_file(path: &Path, fname: &str) { + let content = match std::fs::read_to_string(path) { + Ok(s) => s, + Err(e) => { + eprintln!("buzz-desktop: harness-migration: cannot read backup {fname}: {e}"); + return; + } + }; + match strip_env_from_json(&content) { + Some(clean) => { + if let Err(e) = + crate::managed_agents::storage::atomic_write_json_restricted(path, clean.as_bytes()) + { + eprintln!( + "buzz-desktop: harness-migration: cannot write scrubbed backup {fname}: {e}" + ); + } + } + None => { + if let Err(e) = std::fs::remove_file(path) { + eprintln!("buzz-desktop: harness-migration: cannot remove unparseable backup {fname}: {e}"); + } + } + } +} + +/// Remove the `env` object from a harness JSON document, returning the +/// re-serialized string. `None` when the content is not a JSON object. +fn strip_env_from_json(content: &str) -> Option { + use serde_json::Value; + let mut v: Value = serde_json::from_str(content).ok()?; + let Value::Object(map) = &mut v else { + return None; + }; + map.remove("env"); + serde_json::to_string_pretty(&v).ok() +} + +/// Enumerate the live `*.json` files in `dir` (excludes `.json.bak`, temps, and +/// any non-`.json` sibling). Returns an empty vec on an unreadable directory. +fn harness_json_files(dir: &Path) -> Vec { + let entries = match std::fs::read_dir(dir) { + Ok(e) => e, + Err(_) => return Vec::new(), + }; + entries + .flatten() + .map(|e| e.path()) + .filter(|p| p.extension().and_then(|e| e.to_str()) == Some("json")) + .collect() +} + +/// Parse one harness file with a raw (NON-hydrating) serde read. Warnings are +/// swallowed — a malformed live file is left in place for the user to fix, +/// exactly as the loader treats it. +fn read_definition(path: &Path) -> Option { + let content = std::fs::read_to_string(path).ok()?; + serde_json::from_str(&content).ok() +} + +/// True when `fname` is a harness backup: `.json.bak`. +fn is_harness_backup(fname: &str) -> bool { + match fname.split_once(".json") { + Some((_, after)) => after.contains(".bak"), + None => false, + } +} + +/// True when `fname` looks like an atomic-write-file temp: no extension, at +/// least 8 chars, all ASCII hex digits. +fn is_atomic_write_temp(fname: &str) -> bool { + !fname.contains('.') && fname.len() >= 8 && fname.chars().all(|c| c.is_ascii_hexdigit()) +} + +#[cfg(test)] +#[path = "migration_harness_secrets_tests.rs"] +mod tests; diff --git a/desktop/src-tauri/src/migration/migration_harness_secrets_tests.rs b/desktop/src-tauri/src/migration/migration_harness_secrets_tests.rs new file mode 100644 index 0000000000..c623cf8a13 --- /dev/null +++ b/desktop/src-tauri/src/migration/migration_harness_secrets_tests.rs @@ -0,0 +1,471 @@ +//! Tests for boot-time custom-harness secret extraction. +//! +//! The module under test is generic over [`ProjectionStore`], so the full +//! extract → verify → scrub flow runs against a fake keyring and a tempdir +//! with no `AppHandle` — the precedent `migration_secrets` could only unit-test +//! its helpers because its extraction was not store-injected. +//! +//! Load-bearing properties proven here: +//! * inline `env` is projected to the keyring and stripped from disk (no +//! plaintext survives), recoverable through the same coordinate hydrate reads; +//! * the plaintext-artifact scrub runs ONLY when every projected ref reads +//! back cleanly — a dangling ref preserves the backup as last-recoverable +//! plaintext, and a later boot scrubs it once the ref resolves; +//! * extraction is idempotent across launches (a re-read projected record +//! mints no new generation). + +use super::*; +use crate::managed_agents::secret_projection::{deserialize_env_map, serialize_env_map}; +use std::cell::RefCell; +use std::collections::{BTreeMap, HashMap}; +use std::path::PathBuf; + +// ── Fake store (mirrors secret_seam_tests::FakeProjectionStore) ──────────── + +struct FakeProjectionStore { + data: RefCell>, +} + +impl FakeProjectionStore { + fn new() -> Self { + Self { + data: RefCell::new(HashMap::new()), + } + } + fn with_entry(self, key: &str, value: &str) -> Self { + self.data + .borrow_mut() + .insert(key.to_string(), value.to_string()); + self + } + fn insert(&self, key: &str, value: &str) { + self.data + .borrow_mut() + .insert(key.to_string(), value.to_string()); + } + fn contains(&self, key: &str) -> bool { + self.data.borrow().contains_key(key) + } + fn len(&self) -> usize { + self.data.borrow().len() + } +} + +impl ProjectionStore for FakeProjectionStore { + fn write_and_verify(&self, key: &str, value: &str) -> Result<(), String> { + self.data + .borrow_mut() + .insert(key.to_string(), value.to_string()); + Ok(()) + } + fn load_key(&self, key: &str) -> Result, String> { + Ok(self.data.borrow().get(key).cloned()) + } + fn load_all(&self) -> Result>, String> { + Ok(Some(self.data.borrow().clone())) + } + fn store_batch(&self, entries: &HashMap) -> Result<(), String> { + for (k, v) in entries { + self.data.borrow_mut().insert(k.clone(), v.clone()); + } + Ok(()) + } + fn remove_batch(&self, keys: &[&str]) -> Result<(), String> { + for k in keys { + self.data.borrow_mut().remove(*k); + } + Ok(()) + } +} + +// ── Builders ─────────────────────────────────────────────────────────────── + +fn env_of(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() +} + +fn harness_def( + id: &str, + env: BTreeMap, + env_ref: Option, +) -> HarnessDefinition { + HarnessDefinition { + id: id.to_string(), + label: format!("{id} label"), + command: format!("{id}-bin"), + args: vec![], + env, + env_ref, + install_instructions_url: String::new(), + install_hint: String::new(), + } +} + +/// Write `def` to `dir/.json` and return the path. The stem defaults +/// to the id but may differ to exercise hand-authored filenames. +fn write_harness_file(dir: &Path, file_stem: &str, def: &HarnessDefinition) -> PathBuf { + let path = dir.join(format!("{file_stem}.json")); + std::fs::write(&path, serde_json::to_string_pretty(def).unwrap()).unwrap(); + path +} + +fn read_back(path: &Path) -> HarnessDefinition { + serde_json::from_str(&std::fs::read_to_string(path).unwrap()).unwrap() +} + +// ── Phase 1: extraction ────────────────────────────────────────────────────── + +#[test] +fn test_inline_env_projected_and_file_stripped_of_plaintext() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + let env = env_of(&[("ANTHROPIC_API_KEY", "sk-ant-secret")]); + let path = write_harness_file( + dir.path(), + "env-harness", + &harness_def("env-harness", env.clone(), None), + ); + + migrate_harness_secrets_in_dir(&store, dir.path()); + + // Disk: env stripped, ref set, no plaintext byte left behind. + let on_disk = read_back(&path); + assert!( + on_disk.env.is_empty(), + "inline env must be stripped from disk" + ); + let gen = on_disk + .env_ref + .expect("env_ref must be set after projection"); + let raw = std::fs::read_to_string(&path).unwrap(); + assert!( + !raw.contains("sk-ant-secret"), + "plaintext secret must not survive on disk" + ); + + // Keyring: the coordinate hydrate reads holds the projected env verbatim. + let key = harness_env_key("env-harness", &gen); + let stored = store + .load_key(&key) + .unwrap() + .expect("keyring must hold the projected env"); + assert_eq!(deserialize_env_map(&stored).unwrap(), env); +} + +#[test] +fn test_empty_env_is_noop_no_projection_no_rewrite() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + let path = write_harness_file( + dir.path(), + "bare", + &harness_def("bare", BTreeMap::new(), None), + ); + let before = std::fs::read_to_string(&path).unwrap(); + + migrate_harness_secrets_in_dir(&store, dir.path()); + + assert_eq!(store.len(), 0, "empty env must not mint a generation"); + assert_eq!( + std::fs::read_to_string(&path).unwrap(), + before, + "a no-env harness file must be left byte-for-byte unchanged" + ); +} + +#[test] +fn test_rewrites_original_path_never_id_derived_name() { + // A hand-authored filename may differ from the harness id; relocating to + // .json would orphan the original and duplicate the entry. + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + let env = env_of(&[("K", "v")]); + let authored = write_harness_file( + dir.path(), + "hand-authored", + &harness_def("realid", env, None), + ); + + migrate_harness_secrets_in_dir(&store, dir.path()); + + assert!( + authored.exists(), + "the original hand-authored file must be rewritten in place" + ); + assert!( + !dir.path().join("realid.json").exists(), + "migration must not relocate to an id-derived filename" + ); + assert!( + read_back(&authored).env_ref.is_some(), + "the original file must carry the new ref" + ); +} + +// ── Phase 2: plaintext-artifact scrub, gated on verified extraction ────────── + +#[test] +fn test_parseable_backup_scrubbed_when_extraction_verified() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + write_harness_file( + dir.path(), + "h", + &harness_def("h", env_of(&[("K", "v")]), None), + ); + // A backup left by the save path still carries plaintext env. + let bak = dir.path().join("h.json.bak"); + std::fs::write( + &bak, + r#"{"id":"h","label":"h","command":"h-bin","env":{"ANTHROPIC_API_KEY":"sk-plaintext"}}"#, + ) + .unwrap(); + + migrate_harness_secrets_in_dir(&store, dir.path()); + + assert!( + bak.exists(), + "a parseable backup must survive as a scrubbed file" + ); + let scrubbed = std::fs::read_to_string(&bak).unwrap(); + assert!( + !scrubbed.contains("sk-plaintext"), + "backup plaintext env must be scrubbed" + ); + assert!( + scrubbed.contains("\"command\""), + "non-secret backup fields must survive" + ); +} + +#[test] +fn test_unparseable_backup_deleted_when_verified() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + write_harness_file( + dir.path(), + "h", + &harness_def("h", env_of(&[("K", "v")]), None), + ); + let bak = dir.path().join("h.json.bak"); + std::fs::write(&bak, b"not valid json holding sk-plaintext").unwrap(); + + migrate_harness_secrets_in_dir(&store, dir.path()); + + assert!( + !bak.exists(), + "an unparseable backup may hold plaintext and must be deleted" + ); +} + +#[test] +fn test_atomic_write_temp_deleted_when_verified() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + write_harness_file( + dir.path(), + "h", + &harness_def("h", env_of(&[("K", "v")]), None), + ); + let temp = dir.path().join("deadbeefcafe0123"); + std::fs::write(&temp, b"interrupted write with sk-plaintext").unwrap(); + + migrate_harness_secrets_in_dir(&store, dir.path()); + + assert!( + !temp.exists(), + "an atomic-write temp sibling must be deleted" + ); +} + +#[test] +fn test_scrub_skipped_when_ref_unreachable_backup_survives_intact() { + // A live file already projected on a prior boot (empty inline + ref) whose + // keyring entry is now unreachable: the backup may be the last recoverable + // plaintext, so it must NOT be scrubbed this boot. + let store = FakeProjectionStore::new(); // does NOT hold the referenced gen + let dir = tempfile::tempdir().unwrap(); + write_harness_file( + dir.path(), + "h", + &harness_def("h", BTreeMap::new(), Some("lostgen".to_string())), + ); + let bak = dir.path().join("h.json.bak"); + std::fs::write( + &bak, + r#"{"id":"h","label":"h","command":"h-bin","env":{"ANTHROPIC_API_KEY":"sk-plaintext"}}"#, + ) + .unwrap(); + + migrate_harness_secrets_in_dir(&store, dir.path()); + + assert!( + bak.exists(), + "a dangling ref must leave the plaintext backup in place" + ); + assert!( + std::fs::read_to_string(&bak) + .unwrap() + .contains("sk-plaintext"), + "the backup must retain its plaintext while the ref is unrecoverable" + ); +} + +#[test] +fn test_scrub_retried_once_ref_becomes_reachable() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + write_harness_file( + dir.path(), + "h", + &harness_def("h", BTreeMap::new(), Some("g1".to_string())), + ); + let bak = dir.path().join("h.json.bak"); + std::fs::write( + &bak, + r#"{"id":"h","label":"h","command":"h-bin","env":{"ANTHROPIC_API_KEY":"sk-plaintext"}}"#, + ) + .unwrap(); + + // Boot 1: keyring unreachable for g1 → scrub skipped, backup preserved. + migrate_harness_secrets_in_dir(&store, dir.path()); + assert!( + std::fs::read_to_string(&bak) + .unwrap() + .contains("sk-plaintext"), + "scrub must be skipped while the ref is unreachable" + ); + + // Keyring recovers the entry; Boot 2: verify passes → scrub runs. + store.insert( + &harness_env_key("h", "g1"), + &serialize_env_map(&env_of(&[("K", "v")])).unwrap(), + ); + migrate_harness_secrets_in_dir(&store, dir.path()); + assert!( + !std::fs::read_to_string(&bak) + .unwrap() + .contains("sk-plaintext"), + "scrub must run on the boot where the ref finally resolves" + ); +} + +// ── Idempotence ────────────────────────────────────────────────────────────── + +#[test] +fn test_two_launches_mint_no_second_generation() { + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + let path = write_harness_file( + dir.path(), + "h", + &harness_def("h", env_of(&[("K", "v")]), None), + ); + + migrate_harness_secrets_in_dir(&store, dir.path()); + let gen1 = read_back(&path).env_ref.expect("gen after first launch"); + let keys_after_1 = store.len(); + + migrate_harness_secrets_in_dir(&store, dir.path()); + let gen2 = read_back(&path).env_ref.expect("gen after second launch"); + + assert_eq!( + gen1, gen2, + "a re-read projected record must keep its generation" + ); + assert_eq!( + store.len(), + keys_after_1, + "second launch must not mint a new generation" + ); +} + +// ── Helper predicates and JSON strip ───────────────────────────────────────── + +#[test] +fn test_is_harness_backup_matches_bak_variants() { + assert!(is_harness_backup("h.json.bak")); + assert!(is_harness_backup("h.json.bak-20260813-000000")); + assert!(is_harness_backup("h.json.bak.20260813")); + assert!(!is_harness_backup("h.json")); + assert!(!is_harness_backup("h.json.tmp")); +} + +#[test] +fn test_is_atomic_write_temp_matches_hex_names_only() { + assert!(is_atomic_write_temp("deadbeefcafe0123")); + assert!(!is_atomic_write_temp("h.json")); // has extension + assert!(!is_atomic_write_temp("abc")); // too short + assert!(!is_atomic_write_temp("not-hex-name")); // non-hex +} + +#[test] +fn test_strip_env_from_json_removes_only_env() { + let clean = + strip_env_from_json(r#"{"id":"h","command":"h-bin","env":{"K":"secret"}}"#).unwrap(); + let v: serde_json::Value = serde_json::from_str(&clean).unwrap(); + assert!(v.get("env").is_none(), "env must be removed"); + assert_eq!(v.get("command").and_then(|c| c.as_str()), Some("h-bin")); + assert_eq!(v.get("id").and_then(|c| c.as_str()), Some("h")); +} + +#[test] +fn test_strip_env_from_json_returns_none_on_non_object() { + assert!(strip_env_from_json("not json").is_none()); + assert!(strip_env_from_json("[1,2,3]").is_none()); +} + +#[test] +fn test_extraction_verified_true_when_no_projected_refs() { + // A directory of purely inline-fallback (non-empty env, no ref) files has + // nothing to hydrate, so verification passes and the scrub may proceed. + let store = FakeProjectionStore::new(); + let dir = tempfile::tempdir().unwrap(); + write_harness_file( + dir.path(), + "a", + &harness_def("a", env_of(&[("K", "v")]), None), + ); + assert!(extraction_verified(&store, dir.path())); +} + +#[cfg(unix)] +#[test] +fn test_cleanup_skips_symlink_escaping_dir() { + use std::os::unix::fs::symlink; + let dir = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + let target = outside.path().join("secret.txt"); + std::fs::write(&target, b"outside-secret").unwrap(); + + // A backup-shaped symlink pointing outside the harness dir. + symlink(&target, dir.path().join("escape.json.bak")).unwrap(); + + cleanup_harness_artifacts(dir.path()); + + assert!( + target.exists(), + "cleanup must not follow a symlink escaping the dir" + ); + assert_eq!(std::fs::read_to_string(&target).unwrap(), "outside-secret"); +} + +// A pre-populated fake keeps the with_entry builder exercised for future use. +#[test] +fn test_verify_passes_for_reachable_pre_projected_ref() { + let dir = tempfile::tempdir().unwrap(); + write_harness_file( + dir.path(), + "h", + &harness_def("h", BTreeMap::new(), Some("g1".to_string())), + ); + let store = FakeProjectionStore::new().with_entry( + &harness_env_key("h", "g1"), + &serialize_env_map(&env_of(&[("K", "v")])).unwrap(), + ); + assert!(store.contains(&harness_env_key("h", "g1"))); + assert!(extraction_verified(&store, dir.path())); +}