From 1a17b7e182b640bfcc6758aaf84867485daa58b8 Mon Sep 17 00:00:00 2001 From: pmaxhogan Date: Fri, 26 Jun 2026 20:06:26 -0500 Subject: [PATCH] test: cover high-risk crypto, restore, and CLI paths Fill the genuine test gaps in the highest-risk crate (driven-crypto): - keystore.rs had ZERO tests (the only crypto module without any). Extract the keyring-result -> domain mapping into pure free fns (map_load_secret, map_delete_result) - matching the deliberate, CI-safe pattern the sibling driven-drive::token_store already uses (the keyring 4.1.2 mock store is not a declared dependency and a real round-trip is flaky on headless CI) - and unit-test them: NoEntry -> NotFound, wrong-length secret -> MalformedKey(n) (empty / short / long), a 32-byte secret -> the exact MasterKey, any other backend error -> Backend, and idempotent delete (Ok / NoEntry both Ok). - Add the end-to-end disaster-recovery chain to lib.rs suite_tests: master key -> wrap a per-source key -> persist the wrapped blob + write down the BIP39 phrase -> encrypt a file + path components -> drop the suite (model a wiped keychain) -> recover master from the phrase -> unwrap the SAME source key from the persisted blob -> decrypt the old ciphertext and filenames. This ties together recovery + key-wrapping + content STREAM + filename encryption (each only unit-tested in isolation before). Also assert a different (valid) phrase fails to unwrap the source key (DecryptFailed). No production behavior change - the keystore methods delegate to the new pure fns with identical semantics. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01MZQh3ZfwtZsM6c5qnTuWZP --- crates/driven-crypto/src/keystore.rs | 122 +++++++++++++++++++++++---- crates/driven-crypto/src/lib.rs | 77 +++++++++++++++++ 2 files changed, 184 insertions(+), 15 deletions(-) diff --git a/crates/driven-crypto/src/keystore.rs b/crates/driven-crypto/src/keystore.rs index c7df3036..33e1f30d 100644 --- a/crates/driven-crypto/src/keystore.rs +++ b/crates/driven-crypto/src/keystore.rs @@ -74,17 +74,7 @@ impl Keystore { /// blob is the wrong length, or [`KeystoreError::Backend`] on a backend /// failure. pub fn load_master_key(&self) -> Result { - let secret = match self.entry.get_secret() { - Ok(s) => Zeroizing::new(s), - Err(keyring::Error::NoEntry) => return Err(KeystoreError::NotFound), - Err(e) => return Err(KeystoreError::Backend(e)), - }; - if secret.len() != KEY_LEN { - return Err(KeystoreError::MalformedKey(secret.len())); - } - let mut bytes = [0u8; KEY_LEN]; - bytes.copy_from_slice(&secret); - Ok(MasterKey::from_bytes(bytes)) + map_load_secret(self.entry.get_secret()) } /// Deletes the master key entry (account removal / encryption opt-out). @@ -94,9 +84,111 @@ impl Keystore { /// Returns [`KeystoreError::Backend`] on a backend failure other than a /// missing entry. pub fn delete_master_key(&self) -> Result<(), KeystoreError> { - match self.entry.delete_credential() { - Ok(()) | Err(keyring::Error::NoEntry) => Ok(()), - Err(e) => Err(KeystoreError::Backend(e)), - } + map_delete_result(self.entry.delete_credential()) + } +} + +/// Maps a `keyring` `get_secret` result onto a loaded [`MasterKey`]. +/// +/// This is the keyring-result -> domain mapping that is Driven's own +/// responsibility (length validation, `NoEntry` -> recovery-phrase signal), +/// split out as a PURE free fn so it is unit-tested WITHOUT an OS keychain - +/// the same testability pattern as `driven-drive`'s +/// `token_store::map_load_result` (the 4.1.2 mock store is not a declared +/// dependency and a real round-trip would be flaky on headless CI). A missing +/// entry maps to [`KeystoreError::NotFound`]; a secret that is not exactly +/// [`KEY_LEN`] bytes maps to [`KeystoreError::MalformedKey`]; any other +/// backend error maps to [`KeystoreError::Backend`]. The retrieved bytes are +/// held in a [`Zeroizing`] buffer and scrubbed after the copy into the key. +fn map_load_secret(result: keyring::Result>) -> Result { + let secret = match result { + Ok(s) => Zeroizing::new(s), + Err(keyring::Error::NoEntry) => return Err(KeystoreError::NotFound), + Err(e) => return Err(KeystoreError::Backend(e)), + }; + if secret.len() != KEY_LEN { + return Err(KeystoreError::MalformedKey(secret.len())); + } + let mut bytes = [0u8; KEY_LEN]; + bytes.copy_from_slice(&secret); + Ok(MasterKey::from_bytes(bytes)) +} + +/// Maps a `keyring` `delete_credential` result onto the idempotent-delete +/// domain result (pure, OS-keychain-free; mirrors `driven-drive`'s +/// `token_store::map_delete_result`). A missing entry is NOT an error +/// (delete is idempotent); any other backend failure maps to +/// [`KeystoreError::Backend`]. +fn map_delete_result(result: keyring::Result<()>) -> Result<(), KeystoreError> { + match result { + Ok(()) | Err(keyring::Error::NoEntry) => Ok(()), + Err(e) => Err(KeystoreError::Backend(e)), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn load_maps_no_entry_to_not_found() { + // A missing keychain entry is the first-run / wiped-keychain signal the + // recovery-phrase flow keys off - it must NOT be a generic backend error. + assert!(matches!( + map_load_secret(Err(keyring::Error::NoEntry)), + Err(KeystoreError::NotFound) + )); + } + + #[test] + fn load_maps_correct_length_secret_to_master_key() { + // A well-formed 32-byte secret reconstructs the master key byte-for-byte. + let raw = [7u8; KEY_LEN]; + let key = map_load_secret(Ok(raw.to_vec())).unwrap(); + assert_eq!(key.as_bytes(), &raw); + } + + #[test] + fn load_rejects_wrong_length_secret_as_malformed() { + // A foreign / corrupt write of the wrong length must surface MalformedKey + // carrying the observed length - never be silently truncated or panic. + assert!(matches!( + map_load_secret(Ok(vec![0u8; 16])), + Err(KeystoreError::MalformedKey(16)) + )); + assert!(matches!( + map_load_secret(Ok(Vec::new())), + Err(KeystoreError::MalformedKey(0)) + )); + assert!(matches!( + map_load_secret(Ok(vec![0u8; KEY_LEN + 1])), + Err(KeystoreError::MalformedKey(n)) if n == KEY_LEN + 1 + )); + } + + #[test] + fn load_maps_other_backend_error() { + // A real backend failure (anything but NoEntry) is preserved as Backend. + let r = map_load_secret(Err(keyring::Error::Invalid( + "service".to_string(), + "boom".to_string(), + ))); + assert!(matches!(r, Err(KeystoreError::Backend(_)))); + } + + #[test] + fn delete_is_idempotent_for_missing_entry() { + // Both a successful delete and a no-such-entry delete are Ok (idempotent). + assert!(map_delete_result(Ok(())).is_ok()); + assert!(map_delete_result(Err(keyring::Error::NoEntry)).is_ok()); + } + + #[test] + fn delete_surfaces_other_backend_error() { + let r = map_delete_result(Err(keyring::Error::Invalid( + "service".to_string(), + "boom".to_string(), + ))); + assert!(matches!(r, Err(KeystoreError::Backend(_)))); } } diff --git a/crates/driven-crypto/src/lib.rs b/crates/driven-crypto/src/lib.rs index e725cb92..86dcbb71 100644 --- a/crates/driven-crypto/src/lib.rs +++ b/crates/driven-crypto/src/lib.rs @@ -272,4 +272,81 @@ mod suite_tests { Err(CryptoError::Protocol(_)) )); } + + #[test] + fn full_keychain_loss_recovery_decrypts_old_ciphertext() { + // The end-to-end disaster-recovery promise (DESIGN s7.3): a user whose OS + // keychain is wiped (machine reformat) can paste back their 24-word BIP39 + // phrase and STILL decrypt everything previously uploaded. This ties + // together the four pieces each unit-tested in isolation - master-key + // recovery phrase, master-wraps-source, content STREAM, and filename + // encryption - in the exact order the recovery flow exercises them. + // + // 1. Original install: a master key wraps a fresh per-source key; the + // wrapped blob is what persists in SQLite (`wrapped_source_key`), the + // master key is what lived ONLY in the now-lost keychain. + let master = MasterKey::generate(); + let (source_key, wrapped) = master.wrap_new_source_key().unwrap(); + let wrapped_blob = wrapped.to_bytes(); // the on-disk form + let phrase = master_key_to_phrase(&master).unwrap(); // what the user wrote down + + // 2. Encrypt a file + its path under the original source key, capturing the + // header, ciphertext chunks, and the encrypted folder/leaf names. + let suite = DrivenCryptoSuite::new(source_key); + let dir_name = suite.encrypt_filename("Taxes", &[]).unwrap(); + let leaf_name = suite + .encrypt_filename("2023-return.pdf", dir_name.as_bytes()) + .unwrap(); + let mut enc = suite.content_encryptor(); + let header = enc.header(); + let c0 = enc.encrypt_chunk(b"page one of the return").unwrap(); + let (c1, _md5) = enc.finalize_last(b"and the final page").unwrap(); + // Drop the original suite + source key, modelling the wiped keychain: from + // here on ONLY the phrase and the on-disk wrapped blob exist. + drop(suite); + + // 3. Recover: phrase -> master key -> unwrap the SAME source key from the + // persisted blob -> rebuild the suite. + let recovered_master = phrase_to_master_key(&phrase).unwrap(); + let restored_wrapped = WrappedSourceKey::from_bytes(&wrapped_blob).unwrap(); + let recovered_source = recovered_master + .unwrap_source_key(&restored_wrapped) + .unwrap(); + let recovered_suite = DrivenCryptoSuite::new(recovered_source); + + // 4. The recovered suite decrypts both the plaintext path components and + // the file content that the lost-key suite produced. + assert_eq!( + recovered_suite.decrypt_filename(&dir_name, &[]).unwrap(), + "Taxes" + ); + assert_eq!( + recovered_suite + .decrypt_filename(&leaf_name, dir_name.as_bytes()) + .unwrap(), + "2023-return.pdf" + ); + let mut dec = recovered_suite.content_decryptor(&header).unwrap(); + let mut out = Vec::new(); + out.extend_from_slice(&dec.decrypt_chunk(&c0).unwrap()); + out.extend_from_slice(&dec.decrypt_last(&c1).unwrap()); + assert_eq!(out, b"page one of the returnand the final page"); + } + + #[test] + fn wrong_recovery_phrase_cannot_unwrap_the_source_key() { + // A DIFFERENT (valid) recovery phrase reconstructs a different master key, + // which must FAIL to unwrap the source key (AEAD tag mismatch) - so a + // mistyped-but-checksum-valid phrase can never silently yield garbage. + let master = MasterKey::generate(); + let (_source_key, wrapped) = master.wrap_new_source_key().unwrap(); + + let other_master = MasterKey::generate(); + let other_phrase = master_key_to_phrase(&other_master).unwrap(); + let wrong = phrase_to_master_key(&other_phrase).unwrap(); + assert!(matches!( + wrong.unwrap_source_key(&wrapped), + Err(CryptoError::DecryptFailed) + )); + } }