mirror of
https://github.com/block/buzz.git
synced 2026-10-31 14:31:02 +00:00
feat(desktop): project custom-harness env secrets into the keyring
Custom harness definitions carried their `env` map (which can hold provider secrets like ANTHROPIC_API_KEY) in plaintext on disk. Mirror the agent-store secret seam: add an `env_ref` generation pointer, strip `env` into the OS keyring under `harness:<id>:env:<gen>` on save, hydrate it back on load, and write the stripped JSON 0o600. A boot migration lifts pre-existing inline env, verifies each projected generation reads back, then scrubs plaintext-bearing backup/temp artifacts. Keyless builds and keyring outages keep env inline as an authoritative fallback. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
Hayt
@@ -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<String, String>,
|
||||
/// Generation reference for the keyring-projected `env` map, set when the
|
||||
/// inline `env` was stripped into the keyring under `harness:<id>:env:<gen>`.
|
||||
/// 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<String>,
|
||||
/// 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:<id>:env:<gen>` 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<S: ProjectionStore>(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<S: ProjectionStore>(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<S: ProjectionStore>(
|
||||
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<HarnessDefinition> {
|
||||
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<S: ProjectionStore>(
|
||||
store: Option<&S>,
|
||||
dir: &Path,
|
||||
) -> Vec<HarnessDefinition> {
|
||||
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<HarnessDefinition> {
|
||||
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<std::path::PathBuf>,
|
||||
}
|
||||
|
||||
/// Write a harness definition to `dir/<id>.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 `<target>.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 `<dir>/<old_id>.json` after the
|
||||
/// new file is committed (non-fatal if NotFound).
|
||||
/// Write a harness definition to `dir/<id>.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<SaveOutcome, String> {
|
||||
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/<id>.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 `<target>.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 `<dir>/<old_id>.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<S: ProjectionStore>(
|
||||
store: Option<&S>,
|
||||
dir: &Path,
|
||||
definition: &HarnessDefinition,
|
||||
rename_old_id: Option<&str>,
|
||||
) -> Result<SaveOutcome, String> {
|
||||
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}"))?;
|
||||
|
||||
|
||||
@@ -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<HashMap<String, String>>,
|
||||
}
|
||||
|
||||
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<Option<String>, String> {
|
||||
Ok(self.data.borrow().get(key).cloned())
|
||||
}
|
||||
fn load_all(&self) -> Result<Option<HashMap<String, String>>, String> {
|
||||
Ok(Some(self.data.borrow().clone()))
|
||||
}
|
||||
fn store_batch(&self, entries: &HashMap<String, String>) -> 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<Option<String>, String> {
|
||||
Ok(None)
|
||||
}
|
||||
fn load_all(&self) -> Result<Option<HashMap<String, String>>, String> {
|
||||
Ok(Some(HashMap::new()))
|
||||
}
|
||||
fn store_batch(&self, _entries: &HashMap<String, String>) -> 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::<FakeProjectionStore>(None, dir.path(), &def, None).unwrap();
|
||||
|
||||
let loaded = load_custom_harnesses_with::<FakeProjectionStore>(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(),
|
||||
};
|
||||
|
||||
@@ -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(),
|
||||
},
|
||||
|
||||
@@ -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(),
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
}]);
|
||||
|
||||
@@ -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;
|
||||
|
||||
268
desktop/src-tauri/src/migration/migration_harness_secrets.rs
Normal file
268
desktop/src-tauri/src/migration/migration_harness_secrets.rs
Normal file
@@ -0,0 +1,268 @@
|
||||
//! Boot-time secret extraction for custom harness definitions.
|
||||
//!
|
||||
//! Custom harnesses (`<app-data>/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:<id>:env:<gen>` 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<S: ProjectionStore>(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<S: ProjectionStore>(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<String> {
|
||||
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<std::path::PathBuf> {
|
||||
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<HarnessDefinition> {
|
||||
let content = std::fs::read_to_string(path).ok()?;
|
||||
serde_json::from_str(&content).ok()
|
||||
}
|
||||
|
||||
/// True when `fname` is a harness backup: `<id>.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;
|
||||
@@ -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<HashMap<String, String>>,
|
||||
}
|
||||
|
||||
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<Option<String>, String> {
|
||||
Ok(self.data.borrow().get(key).cloned())
|
||||
}
|
||||
fn load_all(&self) -> Result<Option<HashMap<String, String>>, String> {
|
||||
Ok(Some(self.data.borrow().clone()))
|
||||
}
|
||||
fn store_batch(&self, entries: &HashMap<String, String>) -> 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<String, String> {
|
||||
pairs
|
||||
.iter()
|
||||
.map(|(k, v)| (k.to_string(), v.to_string()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn harness_def(
|
||||
id: &str,
|
||||
env: BTreeMap<String, String>,
|
||||
env_ref: Option<String>,
|
||||
) -> 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/<file_stem>.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
|
||||
// <id>.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()));
|
||||
}
|
||||
Reference in New Issue
Block a user