mirror of
https://github.com/block/buzz.git
synced 2026-10-30 22:11:36 +00:00
Keep boot reconcile unit tests off the live OS keyring
The defect-(2) fix routed boot reconcile through storage::hydrate_keys, which resolves the ambient agent_secret_store(). From a unit test on macOS that reaches the real Keychain: the ACL prompt for the shared buzz-desktop-dev service blocks a headless test binary forever, so 'cargo test --workspace' (the desktop-tauri-test gate) never terminates on any machine that has run the dev desktop. Ubuntu CI has no keychain, which is why it stayed green. Thread the existing KeyStore seam through the reconcile core instead: reconcile_agents_in_dir_with takes Option<&impl KeyStore>, the production wrapper passes agent_secret_store(), and the test helper passes None so no reconcile unit test can ever reach the OS keyring. New test injects the FakeKeyStore to pin the hydration behavior itself: keyring-resident nsec -> first 30179 published and decrypts back to the same key; None store -> empty-nsec skip, no 30179. That test fails if the hydrate call is dropped. No production behavior change: reconcile_agents_in_dir_at resolves the same store hydrate_keys did, and hydrate_keys_with is the same code path it already wrapped. Co-authored-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -72,13 +72,40 @@ pub(crate) fn reconcile_agents_to_events(
|
||||
/// Returns the number of agents (re)written to the retention store.
|
||||
#[cfg(test)]
|
||||
pub(crate) fn reconcile_agents_in_dir(base_dir: &Path, keys: &nostr::Keys) -> Result<u32, String> {
|
||||
reconcile_agents_in_dir_at(base_dir, keys, &base_dir.join("retention.db"))
|
||||
// No key store: unit tests must never touch the live OS keyring (a macOS
|
||||
// Keychain ACL prompt blocks a headless test binary forever). Tests that
|
||||
// exercise key hydration inject a fake via
|
||||
// [`reconcile_agents_in_dir_with`].
|
||||
reconcile_agents_in_dir_with(
|
||||
base_dir,
|
||||
keys,
|
||||
&base_dir.join("retention.db"),
|
||||
None::<&crate::secret_store::SecretStore>,
|
||||
)
|
||||
}
|
||||
|
||||
/// Production entry: hydrates keys from the real agent secret store (`None`
|
||||
/// on builds without a keyring backend, where keys stay inline in the JSON).
|
||||
fn reconcile_agents_in_dir_at(
|
||||
base_dir: &Path,
|
||||
keys: &nostr::Keys,
|
||||
db_path: &Path,
|
||||
) -> Result<u32, String> {
|
||||
reconcile_agents_in_dir_with(
|
||||
base_dir,
|
||||
keys,
|
||||
db_path,
|
||||
super::storage::agent_secret_store(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Testable core, generic over the [`super::storage::KeyStore`] seam so unit
|
||||
/// tests never reach the live OS keyring.
|
||||
fn reconcile_agents_in_dir_with(
|
||||
base_dir: &Path,
|
||||
keys: &nostr::Keys,
|
||||
db_path: &Path,
|
||||
store: Option<&impl super::storage::KeyStore>,
|
||||
) -> Result<u32, String> {
|
||||
let store_path = base_dir.join("managed-agents.json");
|
||||
if !store_path.exists() {
|
||||
@@ -101,7 +128,9 @@ fn reconcile_agents_in_dir_at(
|
||||
// default `system-keyring` build lives in the keyring and NOT in the JSON.
|
||||
// Without this, `retain_private_agent_record`'s empty-nsec skip fires for
|
||||
// every untouched agent and boot reconcile publishes zero 30179s.
|
||||
super::storage::hydrate_keys(&mut records);
|
||||
if let Some(store) = store {
|
||||
super::storage::hydrate_keys_with(store, &mut records);
|
||||
}
|
||||
|
||||
let conn =
|
||||
open_retention_db(db_path).map_err(|e| format!("failed to open retention db: {e}"))?;
|
||||
|
||||
@@ -195,6 +195,69 @@ fn missing_store_is_noop() {
|
||||
assert_eq!(reconcile_agents_in_dir(dir.path(), &keys).unwrap(), 0);
|
||||
}
|
||||
|
||||
/// Boot reconcile on a default `system-keyring` build reads the agent nsec
|
||||
/// from the keyring, not the JSON. Hydration through the [`KeyStore`] seam
|
||||
/// must fill it in so the first 30179 gets published; without it, the
|
||||
/// empty-nsec skip fires and only the 30177 lands. The `None`-store control
|
||||
/// pins the other side: no store, no 30179 — which is also why the plain
|
||||
/// [`reconcile_agents_in_dir`] test helper can never hit the live OS keyring
|
||||
/// (a macOS Keychain ACL prompt blocks a headless test binary forever).
|
||||
#[test]
|
||||
fn keyring_resident_nsec_is_hydrated_for_private_config() {
|
||||
use crate::managed_agents::storage::{agent_keyring_name, tests::FakeKeyStore};
|
||||
use nostr::ToBech32;
|
||||
|
||||
let owner_keys = nostr::Keys::generate();
|
||||
let agent_keys = nostr::Keys::generate();
|
||||
let pubkey = agent_keys.public_key().to_hex();
|
||||
let nsec = agent_keys.secret_key().to_bech32().unwrap();
|
||||
// JSON carries an empty nsec — keyring-resident, as on a default build.
|
||||
let record = sample_record(&pubkey, "keyring-agent");
|
||||
assert!(record.private_key_nsec.is_empty());
|
||||
|
||||
// Control: no key store → empty-nsec skip → no 30179 head.
|
||||
let dir = TempDir::new().unwrap();
|
||||
write_store(&dir, std::slice::from_ref(&record));
|
||||
let db_path = dir.path().join("retention.db");
|
||||
reconcile_agents_in_dir_with(
|
||||
dir.path(),
|
||||
&owner_keys,
|
||||
&db_path,
|
||||
None::<&crate::secret_store::SecretStore>,
|
||||
)
|
||||
.unwrap();
|
||||
let conn = open_retention_db(&db_path).unwrap();
|
||||
assert!(get_retained_event(
|
||||
&conn,
|
||||
KIND_PRIVATE_MANAGED_AGENT,
|
||||
&owner_keys.public_key().to_hex(),
|
||||
&pubkey,
|
||||
)
|
||||
.unwrap()
|
||||
.is_none());
|
||||
drop(conn);
|
||||
|
||||
// With the key in the (fake) keyring, hydration fills the nsec and the
|
||||
// first 30179 is retained — and decrypts back to that exact key.
|
||||
let dir = TempDir::new().unwrap();
|
||||
write_store(&dir, &[record]);
|
||||
let db_path = dir.path().join("retention.db");
|
||||
let store = FakeKeyStore::reachable().with_key(&agent_keyring_name(&pubkey), &nsec);
|
||||
reconcile_agents_in_dir_with(dir.path(), &owner_keys, &db_path, Some(&store)).unwrap();
|
||||
let conn = open_retention_db(&db_path).unwrap();
|
||||
let row = get_retained_event(
|
||||
&conn,
|
||||
KIND_PRIVATE_MANAGED_AGENT,
|
||||
&owner_keys.public_key().to_hex(),
|
||||
&pubkey,
|
||||
)
|
||||
.unwrap()
|
||||
.expect("hydrated nsec must publish the first 30179");
|
||||
let event = nostr::Event::from_json(&row.raw_event).unwrap();
|
||||
let (_, payload) = private_managed_agent::validate_and_decrypt(&event, &owner_keys).unwrap();
|
||||
assert_eq!(payload.identity.private_key_nsec, nsec);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fresh_record_is_retained_pending() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
|
||||
@@ -15,7 +15,7 @@ use crate::secret_store::{KeyringProbe, SecretStore};
|
||||
|
||||
/// Keyring key name for an agent's nsec, namespaced from the human identity
|
||||
/// key (`"identity"`) which shares the service.
|
||||
fn agent_keyring_name(pubkey: &str) -> String {
|
||||
pub(super) fn agent_keyring_name(pubkey: &str) -> String {
|
||||
format!("agent:{pubkey}")
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ fn agent_keyring_name(pubkey: &str) -> String {
|
||||
/// `SecretStore::shared` so identity and agent callers share one instance —
|
||||
/// and therefore one in-memory cache and one mutex — preventing last-writer-wins
|
||||
/// races on concurrent blob writes.
|
||||
fn agent_secret_store() -> Option<&'static SecretStore> {
|
||||
pub(super) fn agent_secret_store() -> Option<&'static SecretStore> {
|
||||
if cfg!(feature = "system-keyring") {
|
||||
Some(SecretStore::shared(keyring_service()))
|
||||
} else {
|
||||
@@ -131,7 +131,7 @@ fn newest_agent_log_in_dir(dir: &Path, pubkey: &str) -> Option<PathBuf> {
|
||||
/// The keyring operations the migration chokepoint needs. Abstracted so the
|
||||
/// migrate-and-strip decision logic ([`migrate_inline_key`]) can be unit-tested
|
||||
/// against a fake without touching the live OS keyring.
|
||||
trait KeyStore {
|
||||
pub(super) trait KeyStore {
|
||||
fn probe(&self, name: &str) -> KeyringProbe;
|
||||
/// Read a key. `Ok(None)` is "no such entry" (absent); `Err` is a backend
|
||||
/// failure (keyring unreachable) — the caller MUST NOT collapse the two.
|
||||
@@ -317,7 +317,7 @@ pub(crate) fn hydrate_keys(records: &mut [ManagedAgentRecord]) {
|
||||
/// to spawn an agent whose key could not be read (see the empty-key bail in
|
||||
/// `spawn_agent_child`). Empty here never means "fine" — it means "no usable
|
||||
/// key this boot."
|
||||
fn hydrate_keys_with(store: &impl KeyStore, records: &mut [ManagedAgentRecord]) {
|
||||
pub(super) fn hydrate_keys_with(store: &impl KeyStore, records: &mut [ManagedAgentRecord]) {
|
||||
for record in records.iter_mut() {
|
||||
// A key-less definition (no pubkey yet — unified agent model) has no
|
||||
// keyring entry by construction; keys are minted on first start.
|
||||
@@ -906,4 +906,4 @@ pub fn meaningful_agent_error_from_log(path: &Path) -> Option<AgentLogError> {
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "storage_tests.rs"]
|
||||
mod tests;
|
||||
pub(super) mod tests;
|
||||
|
||||
@@ -19,7 +19,11 @@ use super::{
|
||||
/// In-memory [`KeyStore`] for testing the migrate decision without the OS
|
||||
/// keyring. `reachable=false` simulates a backend outage; `fail_verify`
|
||||
/// simulates a write whose read-back does not confirm.
|
||||
struct FakeKeyStore {
|
||||
///
|
||||
/// `pub(in crate::managed_agents)` so sibling test modules (e.g.
|
||||
/// `reconcile::tests`) can inject it through the same seam instead of
|
||||
/// touching the live keyring.
|
||||
pub(in crate::managed_agents) struct FakeKeyStore {
|
||||
reachable: bool,
|
||||
fail_verify: bool,
|
||||
stored: RefCell<HashMap<String, String>>,
|
||||
@@ -28,7 +32,7 @@ struct FakeKeyStore {
|
||||
}
|
||||
|
||||
impl FakeKeyStore {
|
||||
fn reachable() -> Self {
|
||||
pub(in crate::managed_agents) fn reachable() -> Self {
|
||||
Self {
|
||||
reachable: true,
|
||||
fail_verify: false,
|
||||
@@ -56,7 +60,7 @@ impl FakeKeyStore {
|
||||
}
|
||||
}
|
||||
/// Seed a key as already present in the keyring.
|
||||
fn with_key(self, name: &str, value: &str) -> Self {
|
||||
pub(in crate::managed_agents) fn with_key(self, name: &str, value: &str) -> Self {
|
||||
self.stored
|
||||
.borrow_mut()
|
||||
.insert(name.to_string(), value.to_string());
|
||||
|
||||
Reference in New Issue
Block a user