From 931b297321b49668ddbaed107366ef2af24bfdc0 Mon Sep 17 00:00:00 2001 From: Hammed Abass Date: Sun, 9 Aug 2026 18:44:58 +0200 Subject: [PATCH] test(core): harden secret custody recovery --- crates/vnidrop/src/secure_secret.rs | 27 +++++----- crates/vnidrop/src/tests/secure_secret.rs | 65 +++++++++++++++++++++-- 2 files changed, 73 insertions(+), 19 deletions(-) diff --git a/crates/vnidrop/src/secure_secret.rs b/crates/vnidrop/src/secure_secret.rs index f100d73..efe3225 100644 --- a/crates/vnidrop/src/secure_secret.rs +++ b/crates/vnidrop/src/secure_secret.rs @@ -12,6 +12,7 @@ use crate::{error::VnidropError, util::now_ms}; const SECRET_BYTES: usize = 32; const HANDLE_NAMESPACE: &str = "vnidrop"; +const HANDLE_VERSION: &str = "v1"; #[derive(Clone, PartialEq, Eq)] pub(crate) struct SecretMaterial(Vec); @@ -44,13 +45,13 @@ pub(crate) struct SecretHandle(String); impl SecretHandle { fn generate(kind: SecretKind) -> Self { Self(format!( - "{HANDLE_NAMESPACE}/{}/{}", + "{HANDLE_NAMESPACE}/{HANDLE_VERSION}/{}/{}", kind.as_str(), Uuid::new_v4() )) } - fn as_str(&self) -> &str { + pub(crate) fn as_str(&self) -> &str { &self.0 } } @@ -419,10 +420,7 @@ impl SecretCustody { for entry in metadata { if entry.state == SecretMetadataState::Disabled { - match self.store.delete(&entry.handle) { - Ok(()) | Err(SecureSecretStoreError::Missing) => {} - Err(error) => return Err(map_store_error(error)), - } + self.delete_if_present(&entry.handle)?; continue; } match self.store.get(&entry.handle) { @@ -431,10 +429,7 @@ impl SecretCustody { .is_err() { self.metadata.disable(&entry.handle).await?; - match self.store.delete(&entry.handle) { - Ok(()) | Err(SecureSecretStoreError::Missing) => {} - Err(error) => return Err(map_store_error(error)), - } + self.delete_if_present(&entry.handle)?; summary.disabled += 1; } else if entry.state == SecretMetadataState::Staged { self.metadata.activate(&entry.handle).await?; @@ -443,10 +438,7 @@ impl SecretCustody { } Err(SecureSecretStoreError::Missing | SecureSecretStoreError::Corrupted) => { self.metadata.disable(&entry.handle).await?; - match self.store.delete(&entry.handle) { - Ok(()) | Err(SecureSecretStoreError::Missing) => {} - Err(error) => return Err(map_store_error(error)), - } + self.delete_if_present(&entry.handle)?; summary.disabled += 1; } Err(error) => return Err(map_store_error(error)), @@ -462,6 +454,13 @@ impl SecretCustody { Ok(summary) } + fn delete_if_present(&self, handle: &SecretHandle) -> Result<(), VnidropError> { + match self.store.delete(handle) { + Ok(()) | Err(SecureSecretStoreError::Missing) => Ok(()), + Err(error) => Err(map_store_error(error)), + } + } + #[cfg(test)] pub(crate) fn crash_once_at(&self, point: CustodyCrashPoint) { *self.crash_point.lock().unwrap() = Some(point); diff --git a/crates/vnidrop/src/tests/secure_secret.rs b/crates/vnidrop/src/tests/secure_secret.rs index 51b9eec..9641340 100644 --- a/crates/vnidrop/src/tests/secure_secret.rs +++ b/crates/vnidrop/src/tests/secure_secret.rs @@ -1,4 +1,7 @@ -use std::sync::Arc; +use std::{ + io::{self, Write}, + sync::{Arc, Mutex}, +}; use data_encoding::HEXLOWER; use iroh::SecretKey; @@ -12,6 +15,22 @@ use crate::{ VnidropError, }; +#[derive(Clone, Default)] +struct CapturedOutput(Arc>>); + +struct CapturedWriter(CapturedOutput); + +impl Write for CapturedWriter { + fn write(&mut self, bytes: &[u8]) -> io::Result { + self.0 .0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } +} + #[tokio::test] async fn custody_maps_reference_store_failures_to_typed_core_errors() { let temp = tempfile::tempdir().unwrap(); @@ -59,7 +78,7 @@ async fn custody_maps_reference_store_failures_to_typed_core_errors() { #[tokio::test] async fn reconciliation_repairs_staged_metadata_and_disables_unusable_secrets() { let temp = tempfile::tempdir().unwrap(); - let repository = Repository::open(temp.path()).await.unwrap(); + let mut repository = Repository::open(temp.path()).await.unwrap(); let store = Arc::new(FaultInjectingSecretStore::default()); let custody = SecretCustody::new(repository.protected_secrets(), store.clone()); @@ -73,6 +92,8 @@ async fn reconciliation_repairs_staged_metadata_and_disables_unusable_secrets() .await .is_err()); drop(custody); + drop(repository); + repository = Repository::open(temp.path()).await.unwrap(); let (custody, summary) = SecretCustody::start(repository.protected_secrets(), store.clone()) .await .unwrap(); @@ -90,6 +111,8 @@ async fn reconciliation_repairs_staged_metadata_and_disables_unusable_secrets() .is_err()); let staged_handle = store.only_handle_for_test(); drop(custody); + drop(repository); + repository = Repository::open(temp.path()).await.unwrap(); let (custody, summary) = SecretCustody::start(repository.protected_secrets(), store.clone()) .await .unwrap(); @@ -100,7 +123,12 @@ async fn reconciliation_repairs_staged_metadata_and_disables_unusable_secrets() ); store.remove_for_test(&staged_handle); - let summary = custody.reconcile().await.unwrap(); + drop(custody); + drop(repository); + repository = Repository::open(temp.path()).await.unwrap(); + let (custody, summary) = SecretCustody::start(repository.protected_secrets(), store.clone()) + .await + .unwrap(); assert_eq!(summary.disabled, 1); assert!(matches!( custody.load(&staged_handle).await, @@ -116,7 +144,12 @@ async fn reconciliation_repairs_staged_metadata_and_disables_unusable_secrets() .await .unwrap(); store.corrupt_for_test(&corrupted); - let summary = custody.reconcile().await.unwrap(); + drop(custody); + drop(repository); + let repository = Repository::open(temp.path()).await.unwrap(); + let (custody, summary) = SecretCustody::start(repository.protected_secrets(), store.clone()) + .await + .unwrap(); assert_eq!(summary.disabled, 1); assert!(matches!( custody.load(&corrupted).await, @@ -144,7 +177,13 @@ async fn endpoint_migration_preserves_identity_across_crash_and_rejects_replacem "legacy key must survive before activation" ); - assert_eq!(custody.reconcile().await.unwrap().staged_activated, 0); + drop(custody); + drop(repository); + let repository = Repository::open(temp.path()).await.unwrap(); + let (custody, summary) = SecretCustody::start(repository.protected_secrets(), store.clone()) + .await + .unwrap(); + assert_eq!(summary.staged_activated, 0); let handle = custody .migrate_legacy_endpoint_identity(&legacy_path) .await @@ -196,11 +235,27 @@ async fn protected_material_is_absent_from_database_and_diagnostics() { .await .unwrap(); + assert!(handle + .as_str() + .starts_with("vnidrop/v1/relationship-grant/")); assert_eq!(format!("{material:?}"), "SecretMaterial(redacted)"); store.corrupt_for_test(&handle); let error = custody.load(&handle).await.unwrap_err().to_string(); assert!(!error.contains(&encoded)); + let captured = CapturedOutput::default(); + let writer_output = captured.clone(); + let subscriber = tracing_subscriber::fmt() + .without_time() + .with_writer(move || CapturedWriter(writer_output.clone())) + .finish(); + let _subscriber = tracing::subscriber::set_default(subscriber); + tracing::info!(material = ?material, error, "custody diagnostic"); + let diagnostics = String::from_utf8(captured.0.lock().unwrap().clone()).unwrap(); + assert!(!diagnostics.contains(&encoded)); + + assert!(repository.list_events(None, 500).await.unwrap().is_empty()); + let mut persisted = Vec::new(); for entry in std::fs::read_dir(temp.path()).unwrap() { let path = entry.unwrap().path();