commit 85e32e4dcd77d76385184ad09283561628f686e2 parent 2d7a2a46391fc0204e6c6b4bd862058f2e0b50a5 Author: triesap <tyson@radroots.org> Date: Fri, 28 Aug 2026 01:23:52 +0000 refactor(secrets): make credential mutations outcome-aware Bind create-only native credential writes to durable UUIDv7 operations, add bounded cancellation-aware worker phases, and require joined shutdown. Diffstat:
14 files changed, 967 insertions(+), 439 deletions(-)
diff --git a/AGENTS.md b/AGENTS.md @@ -129,7 +129,13 @@ substitute. close is reported. `SecretStore` is an object-safe asynchronous application port. Every caller awaits it, Tokio workers await one-shot results, and only the dedicated credential thread may drive the blocking platform adapter. - Authoritative locks fail closed on poison. + Authoritative locks fail closed on poison. The worker queue remains fixed at + eight, mutations carry the canonical UUIDv7 durable request identity, queued + cancellation has no effect, started caller loss is recovery-required, and + shutdown succeeds only after join within the fixed 30-second bound. Native + creation is create-only (`SecKeychainAddGenericPassword` on macOS and Secret + Service `replace=false` on Linux); exact same-operation replay verifies the + complete zeroizing credential envelope before it is accepted as idempotent. - Services-hardening changes use the approved target-state contracts. Do not add compatibility aliases, dual reads, dual writes, or fallback behavior for prototype surfaces removed by the clean-slate refactor. diff --git a/README.md b/README.md @@ -85,7 +85,20 @@ close call resumes the same shutdown. Operating-system keyring calls run through an object-safe asynchronous application port and a bounded supervised worker rather than directly on an async runtime worker. Callers await one-shot responses; the dedicated operating-system thread alone drives the blocking -platform adapter. +platform adapter. Its request queue is fixed at eight entries and credential +mutations carry the caller's canonical UUIDv7 durable request identity. Work +cancelled while still queued has no credential effect; caller loss after work +starts is an unknown outcome reconciled from the durable operation journal. +Shutdown has a fixed 30-second wait and reports success only after the worker +thread is joined; a timeout remains recovery-required and a later close resumes +the same drain. + +Credential creation is native and atomic: macOS uses create-only Keychain +insertion, while Linux uses Secret Service creation with replacement disabled. +The stored zeroizing envelope binds the creating durable operation to the +secret. Exact same-operation replay is idempotent only when the complete +envelope matches; another operation conflicts and never overwrites the existing +credential. No compatibility path reads the former plaintext credential shape. ## Project documentation diff --git a/core/Cargo.lock b/core/Cargo.lock @@ -39,15 +39,6 @@ dependencies = [ ] [[package]] -name = "aho-corasick" -version = "1.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c982642fa9e8606056828ee9a8505737230110bb1099153c79efe865c59d12ba" -dependencies = [ - "memchr", -] - -[[package]] name = "allocator-api2" version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -66,17 +57,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" [[package]] -name = "apple-native-keyring-store" -version = "1.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b350bfd03649e07aa05c0a81b3e15934374e585c98204a57e20b9d49f49bb9a" -dependencies = [ - "keyring-core", - "log", - "security-framework", -] - -[[package]] name = "arrayvec" version = "0.7.8" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -148,79 +128,6 @@ dependencies = [ ] [[package]] -name = "async-channel" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "924ed96dd52d1b75e9c1a3e6275715fd320f5f9439fb5a4a11fa51f4221158d2" -dependencies = [ - "concurrent-queue", - "event-listener-strategy", - "futures-core", - "pin-project-lite", -] - -[[package]] -name = "async-executor" -version = "1.14.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c96bf972d85afc50bf5ab8fe2d54d1586b4e0b46c97c50a0c9e71e2f7bcd812a" -dependencies = [ - "async-task", - "concurrent-queue", - "fastrand", - "futures-lite", - "pin-project-lite", - "slab", -] - -[[package]] -name = "async-io" -version = "2.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "456b8a8feb6f42d237746d4b3e9a178494627745c3c56c6ea55d92ba50d026fc" -dependencies = [ - "autocfg", - "cfg-if", - "concurrent-queue", - "futures-io", - "futures-lite", - "parking", - "polling", - "rustix", - "slab", - "windows-sys 0.61.2", -] - -[[package]] -name = "async-lock" -version = "3.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "290f7f2596bd5b78a9fec8088ccd89180d7f9f55b94b0576823bbbdc72ee8311" -dependencies = [ - "event-listener", - "event-listener-strategy", - "pin-project-lite", -] - -[[package]] -name = "async-process" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc50921ec0055cdd8a16de48773bfeec5c972598674347252c0399676be7da75" -dependencies = [ - "async-channel", - "async-io", - "async-lock", - "async-signal", - "async-task", - "blocking", - "cfg-if", - "event-listener", - "futures-lite", - "rustix", -] - -[[package]] name = "async-recursion" version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -232,30 +139,6 @@ dependencies = [ ] [[package]] -name = "async-signal" -version = "0.2.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52b5aaafa020cf5053a01f2a60e8ff5dccf550f0f77ec54a4e47285ac2bab485" -dependencies = [ - "async-io", - "async-lock", - "atomic-waker", - "cfg-if", - "futures-core", - "futures-io", - "rustix", - "signal-hook-registry", - "slab", - "windows-sys 0.61.2", -] - -[[package]] -name = "async-task" -version = "4.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b75356056920673b02621b35afd0f7dda9306d03c79a30f5c56c44cf256e3de" - -[[package]] name = "async-trait" version = "0.1.92" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -313,12 +196,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ef49f5882e4b6afaac09ad239a4f8c70a24b8f2b0897edb1f706008efd109cf4" [[package]] -name = "atomic-waker" -version = "1.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" - -[[package]] name = "autocfg" version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -445,31 +322,12 @@ dependencies = [ ] [[package]] -name = "blocking" -version = "1.6.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e83f8d02be6967315521be875afa792a316e28d57b5a2d401897e2a7921b7f21" -dependencies = [ - "async-channel", - "async-task", - "futures-io", - "futures-lite", - "piper", -] - -[[package]] name = "bumpalo" version = "3.20.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" [[package]] -name = "byteorder" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" - -[[package]] name = "bytes" version = "1.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -608,15 +466,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" [[package]] -name = "concurrent-queue" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ca0197aee26d1ae37445ee532fefce43251d24cc7c166799f4d46817f1d3973" -dependencies = [ - "crossbeam-utils", -] - -[[package]] name = "const-oid" version = "0.9.6" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -1222,10 +1071,12 @@ dependencies = [ "harvestcircle_domain", "harvestcircle_nostr", "harvestcircle_product", - "keyring", "radroots_runtime_paths", "radroots_service_sqlite", "radroots_storage", + "secret-service", + "security-framework", + "security-framework-sys", "sqlx", "tempfile", "tokio", @@ -1289,12 +1140,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" [[package]] -name = "hermit-abi" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" - -[[package]] name = "hex" version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -1544,27 +1389,6 @@ dependencies = [ ] [[package]] -name = "keyring" -version = "4.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72585bb6cc9bc370d1d545b7e23fcce71dfd4461c5e15275e3cf51bdfd9a980a" -dependencies = [ - "apple-native-keyring-store", - "keyring-core", - "windows-native-keyring-store", - "zbus-secret-service-keyring-store", -] - -[[package]] -name = "keyring-core" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb1e621458ca9c51aa110bd0339d4751a056b9576bf1253aee1aa560dda0fc9d" -dependencies = [ - "log", -] - -[[package]] name = "libc" version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -2025,17 +1849,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] -name = "piper" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c835479a4443ded371d6c535cbfd8d31ad92c5d23ae9770a61bc155e4992a3c1" -dependencies = [ - "atomic-waker", - "fastrand", - "futures-io", -] - -[[package]] name = "pkg-config" version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -2048,20 +1861,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4596b6d070b27117e987119b4dac604f3c58cfb0b191112e24771b2faeac1a6" [[package]] -name = "polling" -version = "3.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d0e4f59085d47d8241c88ead0f274e8a0cb551f3625263c05eb8dd897c34218" -dependencies = [ - "cfg-if", - "concurrent-queue", - "hermit-abi", - "pin-project-lite", - "rustix", - "windows-sys 0.61.2", -] - -[[package]] name = "poly1305" version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -2379,35 +2178,6 @@ dependencies = [ ] [[package]] -name = "regex" -version = "1.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f020237b6c8eed93db2e2cb53c00c60a8e1bc73da7d073199a1180401450218d" -dependencies = [ - "aho-corasick", - "memchr", - "regex-automata", - "regex-syntax", -] - -[[package]] -name = "regex-automata" -version = "0.4.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ad8553b9b26413251cbf30e620595c7a41b3887f03da04579c0e6b0d6a06b4b2" -dependencies = [ - "aho-corasick", - "memchr", - "regex-syntax", -] - -[[package]] -name = "regex-syntax" -version = "0.8.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" - -[[package]] name = "ring" version = "0.17.14" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -2731,16 +2501,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" [[package]] -name = "signal-hook-registry" -version = "1.4.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c4db69cba1110affc0e9f7bcd48bbf87b3f4fc7c61fc9155afd4c469eb3d6c1b" -dependencies = [ - "errno", - "libc", -] - -[[package]] name = "siphasher" version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -3590,19 +3350,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" [[package]] -name = "windows-native-keyring-store" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "063426e76fdec7438d56bb777f67e318a84a25c707b07e575cb8b78e10c028f8" -dependencies = [ - "byteorder", - "keyring-core", - "regex", - "windows-sys 0.61.2", - "zeroize", -] - -[[package]] name = "windows-sys" version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -3744,14 +3491,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5db4be7c075cb421e4b7ee645541604239bd243ba7c357511f4ff3a74b555907" dependencies = [ "async-broadcast", - "async-executor", - "async-io", - "async-lock", - "async-process", "async-recursion", - "async-task", "async-trait", - "blocking", "enumflags2", "event-listener", "futures-core", @@ -3773,17 +3514,6 @@ dependencies = [ ] [[package]] -name = "zbus-secret-service-keyring-store" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ccede190ba363386a24e8021c7f3848393976609ec9f5d1f8c6c09ef37075b4" -dependencies = [ - "keyring-core", - "secret-service", - "zbus", -] - -[[package]] name = "zbus_macros" version = "5.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" diff --git a/core/compatibility/harvestcircle-storage-api-v1.txt b/core/compatibility/harvestcircle-storage-api-v1.txt @@ -55,9 +55,9 @@ pub fn harvestcircle_storage::HarvestCircleStorageContract::fmt(&self, &mut core pub struct harvestcircle_storage::OsKeyringSecretStore impl harvestcircle_application::secrets::SecretStore for harvestcircle_storage::OsKeyringSecretStore pub fn harvestcircle_storage::OsKeyringSecretStore::contains(&self, harvestcircle_domain::key::PublicKey) -> harvestcircle_application::ports::BoxFuture<'_, core::result::Result<bool, harvestcircle_domain::error::SafeError>> -pub fn harvestcircle_storage::OsKeyringSecretStore::delete(&self, harvestcircle_domain::key::PublicKey) -> harvestcircle_application::ports::BoxFuture<'_, core::result::Result<(), harvestcircle_domain::error::SafeError>> +pub fn harvestcircle_storage::OsKeyringSecretStore::delete<'a>(&'a self, &'a harvestcircle_application::ports::DurableRequestId, harvestcircle_domain::key::PublicKey) -> harvestcircle_application::ports::BoxFuture<'a, core::result::Result<(), harvestcircle_domain::error::SafeError>> pub fn harvestcircle_storage::OsKeyringSecretStore::load(&self, harvestcircle_domain::key::PublicKey) -> harvestcircle_application::ports::BoxFuture<'_, core::result::Result<harvestcircle_domain::key::SecretKeyInput, harvestcircle_domain::error::SafeError>> -pub fn harvestcircle_storage::OsKeyringSecretStore::put(&self, harvestcircle_domain::key::PublicKey, harvestcircle_domain::key::SecretKeyInput) -> harvestcircle_application::ports::BoxFuture<'_, core::result::Result<(), harvestcircle_domain::error::SafeError>> +pub fn harvestcircle_storage::OsKeyringSecretStore::put<'a>(&'a self, &'a harvestcircle_application::ports::DurableRequestId, harvestcircle_domain::key::PublicKey, harvestcircle_domain::key::SecretKeyInput) -> harvestcircle_application::ports::BoxFuture<'a, core::result::Result<(), harvestcircle_domain::error::SafeError>> pub struct harvestcircle_storage::VerifiedHarvestCircleBackup impl core::fmt::Debug for harvestcircle_storage::VerifiedHarvestCircleBackup pub fn harvestcircle_storage::VerifiedHarvestCircleBackup::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result diff --git a/core/crates/harvestcircle_application/src/identities.rs b/core/crates/harvestcircle_application/src/identities.rs @@ -262,7 +262,9 @@ impl AppCore { }; } } - secrets.put(identity.public_key(), secret).await?; + secrets + .put(request_id, identity.public_key(), secret) + .await?; operations .advance_durable_operation( request_id, @@ -372,7 +374,8 @@ impl AppCore { } let identity = ®istry[index]; let local_keyring = local_keyring_binding(identity)?; - match secrets.delete(public_key).await { + let request_id = DurableRequestId::new_v7(); + match secrets.delete(&request_id, public_key).await { Ok(()) => {} Err(error) if error.code() == SafeErrorCode::CredentialMissing @@ -468,7 +471,7 @@ impl AppCore { { self.sign_out()?; } - match secrets.delete(public_key).await { + match secrets.delete(request_id, public_key).await { Ok(()) => {} Err(error) if error.code() == SafeErrorCode::CredentialMissing @@ -677,7 +680,8 @@ impl AppCore { let operation = journal .begin_operation(kind, public_key, clock.now()) .await?; - if let Err(error) = secrets.put(public_key, secret).await { + let request_id = DurableRequestId::new_v7(); + if let Err(error) = secrets.put(&request_id, public_key, secret).await { let _ = journal.finalize_operation(operation).await; return Err(error); } @@ -771,7 +775,8 @@ async fn compensate_identity_write( identities.remove_identity(public_key).await }; let selection_rollback = app_state.save_selected_identity(previous_selection).await; - let credential_rollback = secrets.delete(public_key).await; + let request_id = DurableRequestId::new_v7(); + let credential_rollback = secrets.delete(&request_id, public_key).await; if metadata_rollback.is_err() || selection_rollback.is_err() || credential_rollback.is_err() { let _ = journal .update_operation( @@ -1034,9 +1039,10 @@ mod tests { use super::InMemoryIdentityRepository; use crate::{ AppCore, AppStateRepository, BoxFuture, Clock, DurableOperationKind, DurableOperationPhase, - FailureSecretStore, IdentityOperationPhase, IdentityRepository, InMemoryOperationJournal, - InMemorySecretStore, OperationJournal, ProfileRefreshStatus, ProfileRepository, - RelayConfiguration, SecretStore, SecretStoreOperation, SessionState, StateTransition, + DurableRequestId, FailureSecretStore, IdentityOperationPhase, IdentityRepository, + InMemoryOperationJournal, InMemorySecretStore, OperationJournal, ProfileRefreshStatus, + ProfileRepository, RelayConfiguration, SecretStore, SecretStoreOperation, SessionState, + StateTransition, recovery::tests::{TestDurableRepository, operation as durable_operation}, }; @@ -1711,7 +1717,7 @@ mod tests { .expect("key material"); let (public_key, _npub, secret) = material.into_parts(); secrets - .put(public_key, secret) + .put(&DurableRequestId::new_v7(), public_key, secret) .await .expect("orphan credential"); @@ -1938,7 +1944,10 @@ mod tests { .save_selected_identity(Some(public_key)) .await .expect("selection"); - secrets.put(public_key, secret).await.expect("credential"); + secrets + .put(&DurableRequestId::new_v7(), public_key, secret) + .await + .expect("credential"); core.apply_transition(StateTransition::BootstrapRegistry { identities: vec![identity], selected: Some(public_key), diff --git a/core/crates/harvestcircle_application/src/recovery.rs b/core/crates/harvestcircle_application/src/recovery.rs @@ -8,7 +8,7 @@ use crate::{ SecretStore, }; #[cfg(test)] -use crate::{IdentityOperationKind, IdentityOperationPhase, OperationJournal}; +use crate::{DurableRequestId, IdentityOperationKind, IdentityOperationPhase, OperationJournal}; impl AppCore { /// Reconciles durable request operations before public state is restored. @@ -91,7 +91,7 @@ async fn recover_durable_removal( let mut phase = operation.phase(); if phase == DurableOperationPhase::IntentRecorded { if secrets.contains(identity).await? { - secrets.delete(identity).await?; + secrets.delete(operation.request_id(), identity).await?; } operations .advance_durable_operation( @@ -161,7 +161,7 @@ async fn recover_durable_addition( match operation.phase() { DurableOperationPhase::IntentRecorded => { if secrets.contains(identity).await? { - secrets.delete(identity).await?; + secrets.delete(operation.request_id(), identity).await?; } operations .finalize_durable_operation( @@ -286,7 +286,9 @@ async fn compensate_durable_addition( clock: &(impl Clock + ?Sized), ) -> Result<(), SafeError> { if secrets.contains(operation.identity()).await? { - secrets.delete(operation.identity()).await?; + secrets + .delete(operation.request_id(), operation.identity()) + .await?; } if let Some(availability) = operation.prior().binding_availability() { if let Some(previous) = identities.find_identity(operation.identity()).await? { @@ -346,7 +348,8 @@ async fn recover_removal( ) -> Result<(), SafeError> { let public_key = operation.subject(); if operation.phase() == IdentityOperationPhase::IntentRecorded { - match secrets.delete(public_key).await { + let request_id = DurableRequestId::new_v7(); + match secrets.delete(&request_id, public_key).await { Ok(()) => {} Err(error) if error.code() == harvestcircle_domain::SafeErrorCode::CredentialMissing => {} @@ -401,7 +404,8 @@ async fn recover_addition( IdentityOperationPhase::CredentialWritten | IdentityOperationPhase::CompensationPending if !has_metadata => { - match secrets.delete(operation.subject()).await { + let request_id = DurableRequestId::new_v7(); + match secrets.delete(&request_id, operation.subject()).await { Ok(()) => {} Err(error) if error.code() == harvestcircle_domain::SafeErrorCode::CredentialMissing => {} @@ -671,7 +675,10 @@ pub(crate) mod tests { ] { let (core, identities, secrets, _journal, public_key) = seeded().await; if phase != DurableOperationPhase::IntentRecorded { - secrets.delete(public_key).await.expect("delete credential"); + secrets + .delete(&DurableRequestId::new_v7(), public_key) + .await + .expect("delete credential"); } if matches!( phase, @@ -695,7 +702,10 @@ pub(crate) mod tests { } let (core, identities, secrets, _journal, public_key) = seeded().await; - secrets.delete(public_key).await.expect("delete credential"); + secrets + .delete(&DurableRequestId::new_v7(), public_key) + .await + .expect("delete credential"); identities .remove_identity(public_key) .await @@ -757,7 +767,10 @@ pub(crate) mod tests { .expect("remove metadata"); } if !retain_secret { - secrets.delete(public_key).await.expect("delete credential"); + secrets + .delete(&DurableRequestId::new_v7(), public_key) + .await + .expect("delete credential"); } let recovered = run_durable( &core, @@ -798,7 +811,10 @@ pub(crate) mod tests { .remove_identity(public_key) .await .expect("remove metadata"); - secrets.delete(public_key).await.expect("delete credential"); + secrets + .delete(&DurableRequestId::new_v7(), public_key) + .await + .expect("delete credential"); let recovered = run_durable( &core, &identities, @@ -819,7 +835,10 @@ pub(crate) mod tests { for credential_present in [true, false] { let (core, identities, secrets, journal, public_key) = seeded().await; if !credential_present { - secrets.delete(public_key).await.expect("delete credential"); + secrets + .delete(&DurableRequestId::new_v7(), public_key) + .await + .expect("delete credential"); identities .save_selected_identity(None) .await @@ -854,7 +873,10 @@ pub(crate) mod tests { .expect("remove metadata"); } if !credential_present { - secrets.delete(public_key).await.expect("delete credential"); + secrets + .delete(&DurableRequestId::new_v7(), public_key) + .await + .expect("delete credential"); } let id = journal .begin_operation(kind, public_key, FixedClock.now()) @@ -889,6 +911,7 @@ pub(crate) mod tests { let secrets = FailureSecretStore::default(); secrets .put( + &DurableRequestId::new_v7(), public_key, SecretKeyInput::parse( "7e7e9c42a91bfef19fa7ea99d52d8afdb67d893a8fefba1f5cb9793f2107f6d7".to_owned(), diff --git a/core/crates/harvestcircle_application/src/secrets.rs b/core/crates/harvestcircle_application/src/secrets.rs @@ -4,7 +4,7 @@ use std::sync::{Mutex, MutexGuard}; use harvestcircle_domain::{PublicKey, SafeError, SafeErrorCode, SafeMessage, SecretKeyInput}; use secrecy::{ExposeSecret, SecretString}; -use crate::BoxFuture; +use crate::{BoxFuture, DurableRequestId}; pub trait SecretStore: Send + Sync { /// Stores a credential under its canonical public key without overwriting. @@ -12,11 +12,12 @@ pub trait SecretStore: Send + Sync { /// # Errors /// /// Returns a safe duplicate or keyring error without exposing the credential. - fn put( - &self, + fn put<'a>( + &'a self, + request_id: &'a DurableRequestId, public_key: PublicKey, secret: SecretKeyInput, - ) -> BoxFuture<'_, Result<(), SafeError>>; + ) -> BoxFuture<'a, Result<(), SafeError>>; /// Loads a credential into a non-cloneable redacted boundary value. /// /// # Errors @@ -34,7 +35,11 @@ pub trait SecretStore: Send + Sync { /// # Errors /// /// Returns a safe missing-credential or keyring error. - fn delete(&self, public_key: PublicKey) -> BoxFuture<'_, Result<(), SafeError>>; + fn delete<'a>( + &'a self, + request_id: &'a DurableRequestId, + public_key: PublicKey, + ) -> BoxFuture<'a, Result<(), SafeError>>; } #[derive(Default)] @@ -114,16 +119,17 @@ impl FailureSecretStore { } impl SecretStore for FailureSecretStore { - fn put( - &self, + fn put<'a>( + &'a self, + request_id: &'a DurableRequestId, public_key: PublicKey, secret: SecretKeyInput, - ) -> BoxFuture<'_, Result<(), SafeError>> { + ) -> BoxFuture<'a, Result<(), SafeError>> { Box::pin(async move { if self.record_and_should_fail(SecretStoreOperation::Put, public_key) { return Err(keyring_unavailable()); } - self.inner.put(public_key, secret).await + self.inner.put(request_id, public_key, secret).await }) } @@ -145,12 +151,16 @@ impl SecretStore for FailureSecretStore { }) } - fn delete(&self, public_key: PublicKey) -> BoxFuture<'_, Result<(), SafeError>> { + fn delete<'a>( + &'a self, + request_id: &'a DurableRequestId, + public_key: PublicKey, + ) -> BoxFuture<'a, Result<(), SafeError>> { Box::pin(async move { if self.record_and_should_fail(SecretStoreOperation::Delete, public_key) { return Err(keyring_unavailable()); } - self.inner.delete(public_key).await + self.inner.delete(request_id, public_key).await }) } } @@ -162,11 +172,12 @@ impl InMemorySecretStore { } impl SecretStore for InMemorySecretStore { - fn put( - &self, + fn put<'a>( + &'a self, + _request_id: &'a DurableRequestId, public_key: PublicKey, secret: SecretKeyInput, - ) -> BoxFuture<'_, Result<(), SafeError>> { + ) -> BoxFuture<'a, Result<(), SafeError>> { Box::pin(async move { let mut credentials = self.credentials()?; if credentials.contains_key(&public_key) { @@ -193,7 +204,11 @@ impl SecretStore for InMemorySecretStore { Box::pin(async move { Ok(self.credentials()?.contains_key(&public_key)) }) } - fn delete(&self, public_key: PublicKey) -> BoxFuture<'_, Result<(), SafeError>> { + fn delete<'a>( + &'a self, + _request_id: &'a DurableRequestId, + public_key: PublicKey, + ) -> BoxFuture<'a, Result<(), SafeError>> { Box::pin(async move { self.credentials()? .remove(&public_key) @@ -226,12 +241,18 @@ const fn keyring_unavailable() -> SafeError { #[cfg(test)] mod tests { + use crate::DurableRequestId; + use harvestcircle_domain::{PublicKey, SafeErrorCode, SecretKeyInput}; use super::{FailureSecretStore, InMemorySecretStore, SecretStore, SecretStoreOperation}; const SECRET: &str = "7e7e9c42a91bfef19fa7ea99d52d8afdb67d893a8fefba1f5cb9793f2107f6d7"; + fn request_id() -> DurableRequestId { + DurableRequestId::parse("01890f3e-7b1c-7000-8000-000000000301").expect("request") + } + #[tokio::test] async fn secret_store_puts_loads_checks_and_deletes_redacted_credentials() { let store = InMemorySecretStore::default(); @@ -239,6 +260,7 @@ mod tests { assert!(!store.contains(public_key).await.expect("contains")); store .put( + &request_id(), public_key, SecretKeyInput::parse(SECRET.to_owned()).expect("secret"), ) @@ -247,7 +269,10 @@ mod tests { assert!(store.contains(public_key).await.expect("contains")); let loaded = store.load(public_key).await.expect("load"); assert_eq!(loaded.with_exposed_secret(str::len), 64); - store.delete(public_key).await.expect("delete"); + store + .delete(&request_id(), public_key) + .await + .expect("delete"); assert!(!store.contains(public_key).await.expect("contains")); } @@ -261,6 +286,7 @@ mod tests { assert_eq!(missing.code(), SafeErrorCode::CredentialMissing); store .put( + &request_id(), public_key, SecretKeyInput::parse(SECRET.to_owned()).expect("secret"), ) @@ -268,14 +294,21 @@ mod tests { .expect("put"); let duplicate = store .put( + &request_id(), public_key, SecretKeyInput::parse(SECRET.to_owned()).expect("secret"), ) .await .expect_err("duplicate"); assert_eq!(duplicate.code(), SafeErrorCode::IdentityAlreadyExists); - store.delete(public_key).await.expect("delete"); - let missing = store.delete(public_key).await.expect_err("missing delete"); + store + .delete(&request_id(), public_key) + .await + .expect("delete"); + let missing = store + .delete(&request_id(), public_key) + .await + .expect_err("missing delete"); assert_eq!(missing.code(), SafeErrorCode::CredentialMissing); } @@ -286,6 +319,7 @@ mod tests { store.fail_next(SecretStoreOperation::Put); let error = store .put( + &request_id(), public_key, SecretKeyInput::parse(SECRET.to_owned()).expect("secret"), ) @@ -296,6 +330,7 @@ mod tests { store .put( + &request_id(), public_key, SecretKeyInput::parse(SECRET.to_owned()).expect("secret"), ) @@ -310,7 +345,7 @@ mod tests { let error = match operation { SecretStoreOperation::Load => store.load(public_key).await.map(|_| ()), SecretStoreOperation::Contains => store.contains(public_key).await.map(|_| ()), - SecretStoreOperation::Delete => store.delete(public_key).await, + SecretStoreOperation::Delete => store.delete(&request_id(), public_key).await, SecretStoreOperation::Put => unreachable!("put tested separately"), } .expect_err("injected failure"); @@ -330,6 +365,7 @@ mod tests { let public_key = PublicKey::from_bytes([7; 32]).expect("valid public key"); store .put( + &request_id(), public_key, SecretKeyInput::parse(SECRET.to_owned()).expect("secret"), ) diff --git a/core/crates/harvestcircle_application/src/session.rs b/core/crates/harvestcircle_application/src/session.rs @@ -92,7 +92,7 @@ mod tests { use harvestcircle_domain::{PublicKey, SafeError, SecretKeyInput, UnixTimestamp}; use crate::{ - AppCore, BoxFuture, CachedProfile, Clock, InMemoryIdentityRepository, + AppCore, BoxFuture, CachedProfile, Clock, DurableRequestId, InMemoryIdentityRepository, InMemoryOperationJournal, InMemorySecretStore, ProfileRefreshStatus, ProfileRepository, RelayConfiguration, SecretStore, SessionState, }; @@ -203,7 +203,7 @@ mod tests { assert_eq!(registered.last_used_at(), Some(FixedClock.now())); secrets - .delete(second) + .delete(&DurableRequestId::new_v7(), second) .await .expect("remove second credential"); let error = core @@ -223,6 +223,7 @@ mod tests { ); secrets .put( + &DurableRequestId::new_v7(), second, input("7e7e9c42a91bfef19fa7ea99d52d8afdb67d893a8fefba1f5cb9793f2107f6d7"), ) diff --git a/core/crates/harvestcircle_ffi/src/keyring_worker.rs b/core/crates/harvestcircle_ffi/src/keyring_worker.rs @@ -1,27 +1,73 @@ +use std::sync::atomic::{AtomicU8, Ordering}; use std::sync::{Arc, Mutex}; use std::thread::JoinHandle; +use std::time::Duration; -use harvestcircle_application::{BoxFuture, SecretStore}; +use harvestcircle_application::{BoxFuture, DurableRequestId, SecretStore}; use harvestcircle_domain::{PublicKey, SafeError, SafeErrorCode, SafeMessage, SecretKeyInput}; use tokio::sync::{oneshot, watch}; const KEYRING_QUEUE_CAPACITY: usize = 8; +const KEYRING_SHUTDOWN_DEADLINE: Duration = Duration::from_secs(30); +const OPERATION_QUEUED: u8 = 0; +const OPERATION_STARTED: u8 = 1; +const OPERATION_COMPLETED: u8 = 2; +const OPERATION_CANCELLED: u8 = 3; enum Request { Put( + DurableRequestId, PublicKey, SecretKeyInput, + Arc<AtomicU8>, oneshot::Sender<Result<(), SafeError>>, ), Load( PublicKey, oneshot::Sender<Result<SecretKeyInput, SafeError>>, ), - Contains(PublicKey, oneshot::Sender<Result<bool, SafeError>>), - Delete(PublicKey, oneshot::Sender<Result<(), SafeError>>), + Contains( + PublicKey, + Arc<AtomicU8>, + oneshot::Sender<Result<bool, SafeError>>, + ), + Delete( + DurableRequestId, + PublicKey, + Arc<AtomicU8>, + oneshot::Sender<Result<(), SafeError>>, + ), Close, } +struct CancellationGuard { + phase: Arc<AtomicU8>, + armed: bool, +} + +impl CancellationGuard { + fn new(phase: Arc<AtomicU8>) -> Self { + Self { phase, armed: true } + } + + fn disarm(&mut self) { + self.armed = false; + } +} + +impl Drop for CancellationGuard { + fn drop(&mut self) { + if self.armed { + let _ = self.phase.compare_exchange( + OPERATION_QUEUED, + OPERATION_CANCELLED, + Ordering::AcqRel, + Ordering::Acquire, + ); + } + } +} + pub(crate) struct BoundedKeyringWorker { sender: Mutex<Option<std::sync::mpsc::SyncSender<Request>>>, completion: watch::Receiver<bool>, @@ -41,22 +87,35 @@ impl BoundedKeyringWorker { .spawn(move || { while let Ok(request) = receiver.recv() { match request { - Request::Put(public_key, secret, response) => { - let _ = response.send( - runtime.block_on(async { store.put(public_key, secret).await }), - ); + Request::Put(request_id, public_key, secret, phase, response) => { + if start_operation(&phase) { + let result = runtime.block_on(async { + store.put(&request_id, public_key, secret).await + }); + finish_operation(&phase); + let _ = response.send(result); + } } Request::Load(public_key, response) => { let _ = response .send(runtime.block_on(async { store.load(public_key).await })); } - Request::Contains(public_key, response) => { - let _ = response - .send(runtime.block_on(async { store.contains(public_key).await })); + Request::Contains(public_key, phase, response) => { + if start_operation(&phase) { + let result = + runtime.block_on(async { store.contains(public_key).await }); + finish_operation(&phase); + let _ = response.send(result); + } } - Request::Delete(public_key, response) => { - let _ = response - .send(runtime.block_on(async { store.delete(public_key).await })); + Request::Delete(request_id, public_key, phase, response) => { + if start_operation(&phase) { + let result = runtime.block_on(async { + store.delete(&request_id, public_key).await + }); + finish_operation(&phase); + let _ = response.send(result); + } } Request::Close => break, } @@ -73,41 +132,84 @@ impl BoundedKeyringWorker { async fn submit<T>( &self, - request: impl FnOnce(oneshot::Sender<T>) -> Request, + request: impl FnOnce(Arc<AtomicU8>, oneshot::Sender<T>) -> Request, ) -> Result<T, SafeError> { let (response_sender, response_receiver) = oneshot::channel(); + let phase = Arc::new(AtomicU8::new(OPERATION_QUEUED)); + let mut cancellation = CancellationGuard::new(Arc::clone(&phase)); { let sender_guard = self.sender.lock().map_err(|_| worker_unavailable())?; let Some(sender) = sender_guard.as_ref() else { return Err(worker_unavailable()); }; sender - .try_send(request(response_sender)) + .try_send(request(Arc::clone(&phase), response_sender)) .map_err(|_| worker_unavailable())?; } - response_receiver.await.map_err(|_| worker_unavailable()) + let response = response_receiver.await; + cancellation.disarm(); + response.map_err(|_| match phase.load(Ordering::Acquire) { + OPERATION_STARTED | OPERATION_COMPLETED => recovery_required(), + _ => worker_unavailable(), + }) } pub(crate) async fn close(&self) -> Result<(), SafeError> { + self.close_with_deadline(KEYRING_SHUTDOWN_DEADLINE).await + } + + async fn close_with_deadline(&self, deadline: Duration) -> Result<(), SafeError> { let sender = self.sender.lock().map_err(|_| worker_unavailable())?.take(); if let Some(sender) = sender { signal_close(sender); } let mut completion = self.completion.clone(); - while !*completion.borrow() { - completion - .changed() - .await - .map_err(|_| worker_unavailable())?; - } + tokio::time::timeout(deadline, async { + while !*completion.borrow() { + completion + .changed() + .await + .map_err(|_| worker_unavailable())?; + } + loop { + let finished = self + .thread + .lock() + .map_err(|_| worker_unavailable())? + .as_ref() + .is_none_or(JoinHandle::is_finished); + if finished { + break; + } + tokio::task::yield_now().await; + } + Ok::<(), SafeError>(()) + }) + .await + .map_err(|_| recovery_required())??; let thread = self.thread.lock().map_err(|_| worker_unavailable())?.take(); if let Some(thread) = thread { - thread.join().map_err(|_| worker_unavailable())?; + thread.join().map_err(|_| recovery_required())?; } Ok(()) } } +fn start_operation(phase: &AtomicU8) -> bool { + phase + .compare_exchange( + OPERATION_QUEUED, + OPERATION_STARTED, + Ordering::AcqRel, + Ordering::Acquire, + ) + .is_ok() +} + +fn finish_operation(phase: &AtomicU8) { + phase.store(OPERATION_COMPLETED, Ordering::Release); +} + fn signal_close(sender: std::sync::mpsc::SyncSender<Request>) { match sender.try_send(Request::Close) { Ok(()) @@ -122,35 +224,44 @@ fn signal_close(sender: std::sync::mpsc::SyncSender<Request>) { } impl SecretStore for BoundedKeyringWorker { - fn put( - &self, + fn put<'a>( + &'a self, + request_id: &'a DurableRequestId, public_key: PublicKey, secret: SecretKeyInput, - ) -> BoxFuture<'_, Result<(), SafeError>> { + ) -> BoxFuture<'a, Result<(), SafeError>> { Box::pin(async move { - self.submit(|response| Request::Put(public_key, secret, response)) - .await? + self.submit(|phase, response| { + Request::Put(request_id.clone(), public_key, secret, phase, response) + }) + .await? }) } fn load(&self, public_key: PublicKey) -> BoxFuture<'_, Result<SecretKeyInput, SafeError>> { Box::pin(async move { - self.submit(|response| Request::Load(public_key, response)) + self.submit(|_phase, response| Request::Load(public_key, response)) .await? }) } fn contains(&self, public_key: PublicKey) -> BoxFuture<'_, Result<bool, SafeError>> { Box::pin(async move { - self.submit(|response| Request::Contains(public_key, response)) + self.submit(|phase, response| Request::Contains(public_key, phase, response)) .await? }) } - fn delete(&self, public_key: PublicKey) -> BoxFuture<'_, Result<(), SafeError>> { + fn delete<'a>( + &'a self, + request_id: &'a DurableRequestId, + public_key: PublicKey, + ) -> BoxFuture<'a, Result<(), SafeError>> { Box::pin(async move { - self.submit(|response| Request::Delete(public_key, response)) - .await? + self.submit(|phase, response| { + Request::Delete(request_id.clone(), public_key, phase, response) + }) + .await? }) } } @@ -172,32 +283,148 @@ const fn worker_unavailable() -> SafeError { ) } +const fn recovery_required() -> SafeError { + SafeError::new( + SafeErrorCode::PendingOperationRecoveryRequired, + SafeMessage::new("Credential operation recovery is required."), + ) +} + #[cfg(test)] mod tests { + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, AtomicU8, AtomicUsize, Ordering}; use std::time::Duration; - use harvestcircle_application::{BoxFuture, InMemorySecretStore, SecretStore}; - use harvestcircle_domain::{PublicKey, SafeError, SecretKeyInput}; + use harvestcircle_application::{ + BoxFuture, DurableRequestId, InMemorySecretStore, SecretStore, + }; + use harvestcircle_domain::{PublicKey, SafeError, SafeErrorCode, SecretKeyInput}; use tokio::sync::oneshot; - use super::{BoundedKeyringWorker, Request, signal_close}; + use super::{ + BoundedKeyringWorker, CancellationGuard, KEYRING_QUEUE_CAPACITY, OPERATION_CANCELLED, + OPERATION_COMPLETED, OPERATION_QUEUED, OPERATION_STARTED, Request, finish_operation, + signal_close, start_operation, + }; fn public_key() -> PublicKey { PublicKey::from_hex("7e7e9c42a91bfef19fa7ea99d52d8afdb67d893a8fefba1f5cb9793f2107f6d7") .expect("public key") } + fn request_id() -> DurableRequestId { + DurableRequestId::parse("01890f3e-7b1c-7000-8000-000000000249").expect("request") + } + + fn alternate_request_id() -> DurableRequestId { + DurableRequestId::parse("01890f3e-7b1c-7000-8000-000000000250").expect("request") + } + + fn secret() -> SecretKeyInput { + SecretKeyInput::parse( + "0000000000000000000000000000000000000000000000000000000000000001".to_owned(), + ) + .expect("secret") + } + + struct BlockingPutState { + inner: InMemorySecretStore, + block_next_put: AtomicBool, + put_started: AtomicBool, + release_put: AtomicBool, + put_calls: AtomicUsize, + } + + #[derive(Clone)] + struct BlockingPutStore { + state: Arc<BlockingPutState>, + } + + impl BlockingPutStore { + fn new() -> Self { + Self { + state: Arc::new(BlockingPutState { + inner: InMemorySecretStore::default(), + block_next_put: AtomicBool::new(true), + put_started: AtomicBool::new(false), + release_put: AtomicBool::new(false), + put_calls: AtomicUsize::new(0), + }), + } + } + + async fn wait_until_started(&self) { + while !self.state.put_started.load(Ordering::Acquire) { + tokio::task::yield_now().await; + } + } + + fn release(&self) { + self.state.release_put.store(true, Ordering::Release); + } + + fn put_calls(&self) -> usize { + self.state.put_calls.load(Ordering::Acquire) + } + + async fn contains_direct(&self, public_key: PublicKey) -> bool { + self.state + .inner + .contains(public_key) + .await + .expect("contains") + } + } + + impl SecretStore for BlockingPutStore { + fn put<'a>( + &'a self, + request_id: &'a DurableRequestId, + public_key: PublicKey, + secret: SecretKeyInput, + ) -> BoxFuture<'a, Result<(), SafeError>> { + Box::pin(async move { + self.state.put_calls.fetch_add(1, Ordering::AcqRel); + if self.state.block_next_put.swap(false, Ordering::AcqRel) { + self.state.put_started.store(true, Ordering::Release); + while !self.state.release_put.load(Ordering::Acquire) { + std::thread::yield_now(); + } + } + self.state.inner.put(request_id, public_key, secret).await + }) + } + + fn load(&self, public_key: PublicKey) -> BoxFuture<'_, Result<SecretKeyInput, SafeError>> { + self.state.inner.load(public_key) + } + + fn contains(&self, public_key: PublicKey) -> BoxFuture<'_, Result<bool, SafeError>> { + self.state.inner.contains(public_key) + } + + fn delete<'a>( + &'a self, + request_id: &'a DurableRequestId, + public_key: PublicKey, + ) -> BoxFuture<'a, Result<(), SafeError>> { + self.state.inner.delete(request_id, public_key) + } + } + struct SlowContainsStore { inner: InMemorySecretStore, } impl SecretStore for SlowContainsStore { - fn put( - &self, + fn put<'a>( + &'a self, + request_id: &'a DurableRequestId, public_key: PublicKey, secret: SecretKeyInput, - ) -> BoxFuture<'_, Result<(), SafeError>> { - self.inner.put(public_key, secret) + ) -> BoxFuture<'a, Result<(), SafeError>> { + self.inner.put(request_id, public_key, secret) } fn load(&self, public_key: PublicKey) -> BoxFuture<'_, Result<SecretKeyInput, SafeError>> { @@ -211,27 +438,133 @@ mod tests { }) } - fn delete(&self, public_key: PublicKey) -> BoxFuture<'_, Result<(), SafeError>> { - self.inner.delete(public_key) + fn delete<'a>( + &'a self, + request_id: &'a DurableRequestId, + public_key: PublicKey, + ) -> BoxFuture<'a, Result<(), SafeError>> { + self.inner.delete(request_id, public_key) } } #[tokio::test] async fn worker_round_trips_without_exposing_secret_material() { let worker = BoundedKeyringWorker::new(InMemorySecretStore::default()).expect("worker"); - let secret = SecretKeyInput::parse( - "0000000000000000000000000000000000000000000000000000000000000001".to_owned(), - ) - .expect("secret"); - worker.put(public_key(), secret).await.expect("put"); + worker + .put(&request_id(), public_key(), secret()) + .await + .expect("put"); assert!(worker.contains(public_key()).await.expect("contains")); let loaded = worker.load(public_key()).await.expect("load"); assert_eq!(loaded.with_exposed_secret(str::len), 64); - worker.delete(public_key()).await.expect("delete"); + worker + .delete(&request_id(), public_key()) + .await + .expect("delete"); worker.close().await.expect("close"); assert!(worker.contains(public_key()).await.is_err()); } + #[test] + fn operation_phases_are_closed_and_cancel_only_queued_work() { + let cancelled = Arc::new(AtomicU8::new(OPERATION_QUEUED)); + drop(CancellationGuard::new(Arc::clone(&cancelled))); + assert_eq!(cancelled.load(Ordering::Acquire), OPERATION_CANCELLED); + assert!(!start_operation(&cancelled)); + + let completed = AtomicU8::new(OPERATION_QUEUED); + assert!(start_operation(&completed)); + assert_eq!(completed.load(Ordering::Acquire), OPERATION_STARTED); + finish_operation(&completed); + assert_eq!(completed.load(Ordering::Acquire), OPERATION_COMPLETED); + + let started = Arc::new(AtomicU8::new(OPERATION_STARTED)); + drop(CancellationGuard::new(Arc::clone(&started))); + assert_eq!(started.load(Ordering::Acquire), OPERATION_STARTED); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn cancellation_before_start_has_no_credential_effect() { + let store = BlockingPutStore::new(); + let worker = BoundedKeyringWorker::new(store.clone()).expect("worker"); + let first_worker = Arc::clone(&worker); + let first = tokio::spawn(async move { + first_worker + .put(&request_id(), public_key(), secret()) + .await + }); + store.wait_until_started().await; + + let queued_worker = Arc::clone(&worker); + let queued = tokio::spawn(async move { + queued_worker + .put(&alternate_request_id(), public_key(), secret()) + .await + }); + tokio::task::yield_now().await; + queued.abort(); + assert!(queued.await.expect_err("cancelled task").is_cancelled()); + + store.release(); + first.await.expect("first task").expect("first put"); + worker.close().await.expect("close"); + assert_eq!(store.put_calls(), 1); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn caller_loss_after_start_preserves_unknown_outcome_for_recovery() { + let store = BlockingPutStore::new(); + let worker = BoundedKeyringWorker::new(store.clone()).expect("worker"); + let operation_worker = Arc::clone(&worker); + let operation = tokio::spawn(async move { + operation_worker + .put(&request_id(), public_key(), secret()) + .await + }); + store.wait_until_started().await; + operation.abort(); + assert!(operation.await.expect_err("cancelled task").is_cancelled()); + + store.release(); + while !store.contains_direct(public_key()).await { + tokio::task::yield_now().await; + } + worker.close().await.expect("close"); + assert_eq!(store.put_calls(), 1); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn shutdown_timeout_is_recovery_required_and_retry_joins_thread() { + let store = BlockingPutStore::new(); + let worker = BoundedKeyringWorker::new(store.clone()).expect("worker"); + let operation_worker = Arc::clone(&worker); + let operation = tokio::spawn(async move { + operation_worker + .put(&request_id(), public_key(), secret()) + .await + }); + store.wait_until_started().await; + + let timeout = worker + .close_with_deadline(Duration::from_millis(1)) + .await + .expect_err("blocked worker must time out"); + assert_eq!( + timeout.code(), + SafeErrorCode::PendingOperationRecoveryRequired + ); + assert!(worker.thread.lock().expect("thread").is_some()); + + store.release(); + operation.await.expect("operation task").expect("put"); + worker + .close_with_deadline(Duration::from_secs(1)) + .await + .expect("retry close"); + assert!(worker.thread.lock().expect("thread").is_none()); + assert!(*worker.completion.borrow()); + } + #[tokio::test(flavor = "current_thread")] async fn response_waiting_never_blocks_the_tokio_runtime_thread() { let worker = BoundedKeyringWorker::new(SlowContainsStore { @@ -253,17 +586,31 @@ mod tests { #[test] fn close_signal_never_blocks_on_a_full_bounded_queue() { - let (sender, receiver) = std::sync::mpsc::sync_channel(1); - let (response, _response_receiver) = oneshot::channel(); - assert!( - sender - .try_send(Request::Contains(public_key(), response)) - .is_ok() - ); + let (sender, receiver) = std::sync::mpsc::sync_channel(KEYRING_QUEUE_CAPACITY); + for _ in 0..KEYRING_QUEUE_CAPACITY { + let (response, _response_receiver) = oneshot::channel(); + let phase = Arc::new(AtomicU8::new(OPERATION_QUEUED)); + assert!( + sender + .try_send(Request::Contains(public_key(), phase, response)) + .is_ok() + ); + } + let (overflow_response, _overflow_receiver) = oneshot::channel(); + assert!(matches!( + sender.try_send(Request::Contains( + public_key(), + Arc::new(AtomicU8::new(OPERATION_QUEUED)), + overflow_response, + )), + Err(std::sync::mpsc::TrySendError::Full(_)) + )); signal_close(sender); - assert!(matches!(receiver.recv(), Ok(Request::Contains(_, _)))); + for _ in 0..KEYRING_QUEUE_CAPACITY { + assert!(matches!(receiver.recv(), Ok(Request::Contains(_, _, _)))); + } assert!(receiver.recv().is_err()); } } diff --git a/core/crates/harvestcircle_runtime/src/runtime_actor.rs b/core/crates/harvestcircle_runtime/src/runtime_actor.rs @@ -1604,11 +1604,12 @@ mod tests { } impl SecretStore for BlockingSecretStore { - fn put( - &self, + fn put<'a>( + &'a self, + request_id: &'a harvestcircle_application::DurableRequestId, public_key: PublicKey, secret: SecretKeyInput, - ) -> BoxFuture<'_, Result<(), SafeError>> { + ) -> BoxFuture<'a, Result<(), SafeError>> { Box::pin(async move { if self.block_next_put.swap(false, Ordering::AcqRel) { self.put_started.store(true, Ordering::Release); @@ -1620,7 +1621,7 @@ mod tests { notified.await; } } - self.inner.put(public_key, secret).await + self.inner.put(request_id, public_key, secret).await }) } @@ -1632,8 +1633,12 @@ mod tests { self.inner.contains(public_key) } - fn delete(&self, public_key: PublicKey) -> BoxFuture<'_, Result<(), SafeError>> { - self.inner.delete(public_key) + fn delete<'a>( + &'a self, + request_id: &'a harvestcircle_application::DurableRequestId, + public_key: PublicKey, + ) -> BoxFuture<'a, Result<(), SafeError>> { + self.inner.delete(request_id, public_key) } } diff --git a/core/crates/harvestcircle_storage/Cargo.toml b/core/crates/harvestcircle_storage/Cargo.toml @@ -12,7 +12,6 @@ publish = false include = ["src/**", "tests/**", "Cargo.toml"] [dependencies] -keyring = "=4.1.6" harvestcircle_application.workspace = true harvestcircle_domain.workspace = true harvestcircle_product.workspace = true @@ -23,6 +22,13 @@ sqlx.workspace = true getrandom.workspace = true zeroize = "=1.9.0" +[target.'cfg(target_os = "linux")'.dependencies] +secret-service = { version = "=5.1.0", features = ["crypto-rust"] } + +[target.'cfg(target_os = "macos")'.dependencies] +security-framework = "=3.7.0" +security-framework-sys = "=2.17.0" + [dev-dependencies] harvestcircle_nostr.workspace = true tempfile = "=3.23.0" diff --git a/core/crates/harvestcircle_storage/src/os_keyring.rs b/core/crates/harvestcircle_storage/src/os_keyring.rs @@ -1,23 +1,38 @@ use std::sync::{Mutex, MutexGuard}; -use harvestcircle_application::{BoxFuture, SecretStore}; +use harvestcircle_application::{BoxFuture, DurableRequestId, SecretStore}; use harvestcircle_domain::{PublicKey, SafeError, SafeErrorCode, SafeMessage, SecretKeyInput}; use harvestcircle_product::KEYRING_SERVICE; -use keyring::{Entry, Error as KeyringError}; use zeroize::Zeroizing; pub const CREDENTIAL_SERVICE: &str = KEYRING_SERVICE; +const CREDENTIAL_ENVELOPE_DOMAIN: &[u8] = b"harvestcircle.credential.v1\0"; +#[cfg(target_os = "linux")] +const CREDENTIAL_OPERATION_ATTRIBUTE: &str = "harvestcircle-operation"; +#[cfg(target_os = "linux")] +const CREDENTIAL_ACCOUNT_ATTRIBUTE: &str = "account"; +#[cfg(target_os = "linux")] +const CREDENTIAL_SERVICE_ATTRIBUTE: &str = "service"; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum CreateError { + Existing, + Unavailable, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ReadError { + Missing, + Unavailable, +} + #[derive(Default)] pub struct OsKeyringSecretStore { operation_lock: Mutex<()>, } impl OsKeyringSecretStore { - fn entry(public_key: PublicKey) -> Result<Entry, SafeError> { - Entry::new(CREDENTIAL_SERVICE, &public_key.to_hex()).map_err(|_| keyring_unavailable()) - } - fn operation(&self) -> Result<MutexGuard<'_, ()>, SafeError> { self.operation_lock .lock() @@ -26,67 +41,298 @@ impl OsKeyringSecretStore { } impl SecretStore for OsKeyringSecretStore { - fn put( - &self, + fn put<'a>( + &'a self, + request_id: &'a DurableRequestId, public_key: PublicKey, secret: SecretKeyInput, - ) -> BoxFuture<'_, Result<(), SafeError>> { + ) -> BoxFuture<'a, Result<(), SafeError>> { Box::pin(async move { let _operation = self.operation()?; - let entry = Self::entry(public_key)?; - match entry.get_password() { - Ok(password) => { - drop(Zeroizing::new(password)); - return Err(credential_exists()); + let account = public_key.to_hex(); + let encoded = encode_credential(request_id, &secret); + match platform_create(&account, request_id, encoded.as_slice()) { + Ok(()) => Ok(()), + Err(CreateError::Existing) => { + let existing = Zeroizing::new(platform_read(&account).map_err(map_read_error)?); + verify_existing_replay(request_id, &secret, existing.as_slice()) } - Err(KeyringError::NoEntry) => {} - Err(_) => return Err(keyring_unavailable()), + Err(CreateError::Unavailable) => Err(keyring_unavailable()), } - secret - .with_exposed_secret(|value| entry.set_password(value)) - .map_err(|_| keyring_unavailable()) }) } fn load(&self, public_key: PublicKey) -> BoxFuture<'_, Result<SecretKeyInput, SafeError>> { Box::pin(async move { let _operation = self.operation()?; - let password = Self::entry(public_key)? - .get_password() - .map_err(|error| map_read_error(&error))?; - SecretKeyInput::parse(password) + let account = public_key.to_hex(); + let encoded = Zeroizing::new(platform_read(&account).map_err(map_read_error)?); + decode_credential(encoded.as_slice()).map(|(_, secret)| secret) }) } fn contains(&self, public_key: PublicKey) -> BoxFuture<'_, Result<bool, SafeError>> { Box::pin(async move { let _operation = self.operation()?; - match Self::entry(public_key)?.get_password() { - Ok(password) => { - drop(Zeroizing::new(password)); + let account = public_key.to_hex(); + match platform_read(&account) { + Ok(encoded) => { + let encoded = Zeroizing::new(encoded); + decode_credential(encoded.as_slice())?; Ok(true) } - Err(KeyringError::NoEntry) => Ok(false), - Err(_) => Err(keyring_unavailable()), + Err(ReadError::Missing) => Ok(false), + Err(ReadError::Unavailable) => Err(keyring_unavailable()), } }) } - fn delete(&self, public_key: PublicKey) -> BoxFuture<'_, Result<(), SafeError>> { + fn delete<'a>( + &'a self, + _request_id: &'a DurableRequestId, + public_key: PublicKey, + ) -> BoxFuture<'a, Result<(), SafeError>> { Box::pin(async move { let _operation = self.operation()?; - Self::entry(public_key)? - .delete_credential() - .map_err(|error| map_read_error(&error)) + let account = public_key.to_hex(); + platform_delete(&account).map_err(map_read_error) }) } } -const fn map_read_error(error: &KeyringError) -> SafeError { +fn encode_credential(request_id: &DurableRequestId, secret: &SecretKeyInput) -> Zeroizing<Vec<u8>> { + secret.with_exposed_secret(|value| { + let mut encoded = Zeroizing::new(Vec::with_capacity( + CREDENTIAL_ENVELOPE_DOMAIN.len() + 36 + 1 + value.len(), + )); + encoded.extend_from_slice(CREDENTIAL_ENVELOPE_DOMAIN); + encoded.extend_from_slice(request_id.as_str().as_bytes()); + encoded.push(0); + encoded.extend_from_slice(value.as_bytes()); + encoded + }) +} + +fn decode_credential(encoded: &[u8]) -> Result<(DurableRequestId, SecretKeyInput), SafeError> { + let request_start = CREDENTIAL_ENVELOPE_DOMAIN.len(); + let request_end = request_start + 36; + let secret_start = request_end + 1; + if encoded.len() != secret_start + 64 + || !encoded.starts_with(CREDENTIAL_ENVELOPE_DOMAIN) + || encoded.get(request_end) != Some(&0) + { + return Err(keyring_unavailable()); + } + let request = std::str::from_utf8(&encoded[request_start..request_end]) + .map_err(|_| keyring_unavailable())?; + let request = DurableRequestId::parse(request).map_err(|_| keyring_unavailable())?; + let secret = SecretKeyInput::parse_bytes(encoded[secret_start..].to_vec()) + .map_err(|_| keyring_unavailable())?; + Ok((request, secret)) +} + +fn verify_existing_replay( + request_id: &DurableRequestId, + secret: &SecretKeyInput, + encoded: &[u8], +) -> Result<(), SafeError> { + let (existing_request, existing_secret) = decode_credential(encoded)?; + if existing_request == *request_id + && existing_secret + .with_exposed_secret(|value| secret.with_exposed_secret(|expected| value == expected)) + { + Ok(()) + } else { + Err(credential_exists()) + } +} + +const fn map_read_error(error: ReadError) -> SafeError { match error { - KeyringError::NoEntry => credential_missing(), - _ => keyring_unavailable(), + ReadError::Missing => credential_missing(), + ReadError::Unavailable => keyring_unavailable(), + } +} + +#[cfg(target_os = "macos")] +fn platform_create( + account: &str, + _request_id: &DurableRequestId, + secret: &[u8], +) -> Result<(), CreateError> { + use security_framework::os::macos::keychain::SecKeychain; + use security_framework_sys::base::errSecDuplicateItem; + + let keychain = SecKeychain::default().map_err(|_| CreateError::Unavailable)?; + keychain + .add_generic_password(CREDENTIAL_SERVICE, account, secret) + .map_err(|error| { + if error.code() == errSecDuplicateItem { + CreateError::Existing + } else { + CreateError::Unavailable + } + }) +} + +#[cfg(target_os = "macos")] +fn platform_read(account: &str) -> Result<Vec<u8>, ReadError> { + use security_framework::os::macos::keychain::SecKeychain; + use security_framework_sys::base::errSecItemNotFound; + + let keychain = SecKeychain::default().map_err(|_| ReadError::Unavailable)?; + keychain + .find_generic_password(CREDENTIAL_SERVICE, account) + .map(|(password, _item)| password.as_ref().to_vec()) + .map_err(|error| { + if error.code() == errSecItemNotFound { + ReadError::Missing + } else { + ReadError::Unavailable + } + }) +} + +#[cfg(target_os = "macos")] +fn platform_delete(account: &str) -> Result<(), ReadError> { + use security_framework::item::{ItemClass, ItemSearchOptions}; + use security_framework_sys::base::errSecItemNotFound; + + let mut query = ItemSearchOptions::new(); + query + .class(ItemClass::generic_password()) + .service(CREDENTIAL_SERVICE) + .account(account); + match query.delete() { + Ok(()) => Ok(()), + Err(error) if error.code() == errSecItemNotFound => Err(ReadError::Missing), + Err(_) => Err(ReadError::Unavailable), + } +} + +#[cfg(target_os = "linux")] +fn linux_service() -> Result<secret_service::blocking::SecretService<'static>, ReadError> { + use secret_service::EncryptionType; + use secret_service::blocking::SecretService; + + SecretService::connect(EncryptionType::Dh).map_err(|_| ReadError::Unavailable) +} + +#[cfg(target_os = "linux")] +fn linux_items<'a>( + service: &'a secret_service::blocking::SecretService<'a>, + account: &'a str, +) -> Result<Vec<secret_service::blocking::Item<'a>>, ReadError> { + let attributes = std::collections::HashMap::from([ + (CREDENTIAL_SERVICE_ATTRIBUTE, CREDENTIAL_SERVICE), + (CREDENTIAL_ACCOUNT_ATTRIBUTE, account), + ]); + let mut result = service + .search_items(attributes) + .map_err(|_| ReadError::Unavailable)?; + if !result.locked.is_empty() { + let locked = result.locked.iter().collect::<Vec<_>>(); + service + .unlock_all(&locked) + .map_err(|_| ReadError::Unavailable)?; + result.unlocked.append(&mut result.locked); + } + Ok(result.unlocked) +} + +#[cfg(target_os = "linux")] +fn platform_create( + account: &str, + request_id: &DurableRequestId, + secret: &[u8], +) -> Result<(), CreateError> { + let service = linux_service().map_err(|_| CreateError::Unavailable)?; + let existing = linux_items(&service, account).map_err(|_| CreateError::Unavailable)?; + if !existing.is_empty() { + return Err(CreateError::Existing); + } + + let collection = service + .get_default_collection() + .map_err(|_| CreateError::Unavailable)?; + collection + .ensure_unlocked() + .map_err(|_| CreateError::Unavailable)?; + let attributes = std::collections::HashMap::from([ + (CREDENTIAL_SERVICE_ATTRIBUTE, CREDENTIAL_SERVICE), + (CREDENTIAL_ACCOUNT_ATTRIBUTE, account), + (CREDENTIAL_OPERATION_ATTRIBUTE, request_id.as_str()), + ]); + let created = collection + .create_item( + "HarvestCircle Nostr identity", + attributes, + secret, + false, + "application/octet-stream", + ) + .map_err(|_| CreateError::Unavailable)?; + + let all = linux_items(&service, account).map_err(|_| CreateError::Unavailable)?; + if all.len() == 1 && all.first() == Some(&created) { + Ok(()) + } else { + let _ = created.delete(); + Err(CreateError::Existing) + } +} + +#[cfg(target_os = "linux")] +fn platform_read(account: &str) -> Result<Vec<u8>, ReadError> { + let service = linux_service()?; + let mut items = linux_items(&service, account)?; + if items.is_empty() { + return Err(ReadError::Missing); + } + if items.len() != 1 { + return Err(ReadError::Unavailable); + } + items + .pop() + .expect("single item checked") + .get_secret() + .map_err(|_| ReadError::Unavailable) +} + +#[cfg(target_os = "linux")] +fn platform_delete(account: &str) -> Result<(), ReadError> { + let service = linux_service()?; + let mut items = linux_items(&service, account)?; + if items.is_empty() { + return Err(ReadError::Missing); + } + if items.len() != 1 { + return Err(ReadError::Unavailable); } + items + .pop() + .expect("single item checked") + .delete() + .map_err(|_| ReadError::Unavailable) +} + +#[cfg(not(any(target_os = "linux", target_os = "macos")))] +fn platform_create( + _account: &str, + _request_id: &DurableRequestId, + _secret: &[u8], +) -> Result<(), CreateError> { + Err(CreateError::Unavailable) +} + +#[cfg(not(any(target_os = "linux", target_os = "macos")))] +fn platform_read(_account: &str) -> Result<Vec<u8>, ReadError> { + Err(ReadError::Unavailable) +} + +#[cfg(not(any(target_os = "linux", target_os = "macos")))] +fn platform_delete(_account: &str) -> Result<(), ReadError> { + Err(ReadError::Unavailable) } const fn credential_exists() -> SafeError { @@ -112,19 +358,98 @@ const fn keyring_unavailable() -> SafeError { #[cfg(test)] mod tests { - use harvestcircle_application::SecretStore; + use harvestcircle_application::{DurableRequestId, SecretStore}; use harvestcircle_domain::{PublicKey, SafeErrorCode, SecretKeyInput}; - use super::{CREDENTIAL_SERVICE, OsKeyringSecretStore}; + use super::{ + CREDENTIAL_ENVELOPE_DOMAIN, CREDENTIAL_SERVICE, OsKeyringSecretStore, decode_credential, + encode_credential, verify_existing_replay, + }; + + const SECRET: &str = "0000000000000000000000000000000000000000000000000000000000000001"; + + fn request_id() -> DurableRequestId { + DurableRequestId::parse("01890f3e-7b1c-7000-8000-000000000249").expect("request") + } + + fn public_key() -> PublicKey { + PublicKey::from_hex("7e7e9c42a91bfef19fa7ea99d52d8afdb67d893a8fefba1f5cb9793f2107f6d7") + .expect("valid public key") + } + + #[test] + fn credential_envelope_binds_uuidv7_operation_and_secret() { + let secret = SecretKeyInput::parse(SECRET.to_owned()).expect("secret"); + let encoded = encode_credential(&request_id(), &secret); + assert_eq!( + encoded.len(), + CREDENTIAL_ENVELOPE_DOMAIN.len() + 36 + 1 + 64 + ); + + let (operation, decoded) = decode_credential(encoded.as_slice()).expect("decode"); + assert_eq!(operation, request_id()); + assert!(decoded.with_exposed_secret(|value| value == SECRET)); + } + + #[test] + fn malformed_credential_envelopes_fail_closed() { + let secret = SecretKeyInput::parse(SECRET.to_owned()).expect("secret"); + let encoded = encode_credential(&request_id(), &secret); + for candidate in [ + encoded[..encoded.len() - 1].to_vec(), + { + let mut value = encoded.to_vec(); + value[0] ^= 1; + value + }, + { + let mut value = encoded.to_vec(); + value[CREDENTIAL_ENVELOPE_DOMAIN.len() + 14] = b'4'; + value + }, + { + let mut value = encoded.to_vec(); + value[CREDENTIAL_ENVELOPE_DOMAIN.len() + 36] = b'x'; + value + }, + ] { + let error = match decode_credential(&candidate) { + Err(error) => error, + Ok(_) => panic!("malformed envelope was accepted"), + }; + assert_eq!(error.code(), SafeErrorCode::KeyringUnavailable); + } + } + + #[test] + fn only_exact_same_operation_replay_is_idempotent() { + let secret = SecretKeyInput::parse(SECRET.to_owned()).expect("secret"); + let encoded = encode_credential(&request_id(), &secret); + assert!(verify_existing_replay(&request_id(), &secret, &encoded).is_ok()); + + let another_request = + DurableRequestId::parse("01890f3e-7b1c-7000-8000-000000000250").expect("request"); + let request_conflict = verify_existing_replay(&another_request, &secret, &encoded) + .expect_err("another operation must conflict"); + assert_eq!( + request_conflict.code(), + SafeErrorCode::IdentityAlreadyExists + ); + + let another_secret = SecretKeyInput::parse( + "0000000000000000000000000000000000000000000000000000000000000002".to_owned(), + ) + .expect("secret"); + let secret_conflict = verify_existing_replay(&request_id(), &another_secret, &encoded) + .expect_err("same operation cannot change the secret"); + assert_eq!(secret_conflict.code(), SafeErrorCode::IdentityAlreadyExists); + } #[test] fn keyring_coordinates_are_stable_and_public() { - let public_key = - PublicKey::from_hex("7e7e9c42a91bfef19fa7ea99d52d8afdb67d893a8fefba1f5cb9793f2107f6d7") - .expect("valid public key"); assert_eq!(CREDENTIAL_SERVICE, "org.harvestcircle.desktop.nostr"); assert_eq!( - public_key.to_hex(), + public_key().to_hex(), "7e7e9c42a91bfef19fa7ea99d52d8afdb67d893a8fefba1f5cb9793f2107f6d7" ); } @@ -138,11 +463,8 @@ mod tests { })); assert!(panic.is_err()); - let public_key = - PublicKey::from_hex("7e7e9c42a91bfef19fa7ea99d52d8afdb67d893a8fefba1f5cb9793f2107f6d7") - .expect("valid public key"); let error = store - .contains(public_key) + .contains(public_key()) .await .expect_err("poison must reject"); assert_eq!(error.code(), SafeErrorCode::KeyringUnavailable); @@ -152,20 +474,27 @@ mod tests { #[ignore = "mutates the current user's operating-system credential store"] async fn real_keyring_smoke_round_trips_and_deletes() { let store = OsKeyringSecretStore::default(); - let public_key = - PublicKey::from_hex("7e7e9c42a91bfef19fa7ea99d52d8afdb67d893a8fefba1f5cb9793f2107f6d7") - .expect("valid public key"); - let _ = store.delete(public_key).await; + let request_id = request_id(); + let _ = store.delete(&request_id, public_key()).await; store .put( - public_key, - SecretKeyInput::parse("11".repeat(32)).expect("secret"), + &request_id, + public_key(), + SecretKeyInput::parse(SECRET.to_owned()).expect("secret"), ) .await .expect("keyring put"); - assert!(store.contains(public_key).await.expect("keyring contains")); - let loaded = store.load(public_key).await.expect("keyring load"); - assert_eq!(loaded.with_exposed_secret(str::len), 64); - store.delete(public_key).await.expect("keyring delete"); + assert!( + store + .contains(public_key()) + .await + .expect("keyring contains") + ); + let loaded = store.load(public_key()).await.expect("keyring load"); + assert!(loaded.with_exposed_secret(|value| value == SECRET)); + store + .delete(&request_id, public_key()) + .await + .expect("keyring delete"); } } diff --git a/core/crates/harvestcircle_storage/tests/package_boundary.rs b/core/crates/harvestcircle_storage/tests/package_boundary.rs @@ -24,6 +24,10 @@ fn storage_package_keeps_one_sqlite_authority_and_a_sealed_public_surface() { } assert!(manifest.contains("radroots_service_sqlite.workspace = true")); assert!(manifest.contains("sqlx.workspace = true")); + assert!(!manifest.contains("\nkeyring =")); + assert!(manifest.contains("secret-service = { version = \"=5.1.0\"")); + assert!(manifest.contains("security-framework = \"=3.7.0\"")); + assert!(manifest.contains("security-framework-sys = \"=2.17.0\"")); assert!(workspace_manifest.contains("sqlx = { version = \"=0.9.0\"")); for forbidden_package in ["rusqlite", "refinery"] { assert!( @@ -48,6 +52,10 @@ fn storage_package_keeps_one_sqlite_authority_and_a_sealed_public_surface() { } assert!(!root_source.contains("harvestcircle_initial_schema_sql")); assert!(!keyring_source.contains("PoisonError::into_inner")); + assert!(!keyring_source.contains("set_password")); + assert!(keyring_source.contains("add_generic_password")); + assert!(keyring_source.contains("CREDENTIAL_OPERATION_ATTRIBUTE")); + assert!(keyring_source.contains("false,\n \"application/octet-stream\"")); assert!(!database_source.contains("pub fn host")); assert!(!database_source.contains("pub const fn host")); @@ -62,7 +70,8 @@ fn storage_package_keeps_one_sqlite_authority_and_a_sealed_public_surface() { "impl harvestcircle_application::ports::DurableOperationRepository for harvestcircle_storage::Database", "harvestcircle_application::ports::BoxFuture", "pub fn harvestcircle_storage::OsKeyringSecretStore::contains(&self, harvestcircle_domain::key::PublicKey) -> harvestcircle_application::ports::BoxFuture", - "pub fn harvestcircle_storage::OsKeyringSecretStore::put(&self, harvestcircle_domain::key::PublicKey, harvestcircle_domain::key::SecretKeyInput) -> harvestcircle_application::ports::BoxFuture", + "pub fn harvestcircle_storage::OsKeyringSecretStore::put<'a>(&'a self, &'a harvestcircle_application::ports::DurableRequestId, harvestcircle_domain::key::PublicKey, harvestcircle_domain::key::SecretKeyInput) -> harvestcircle_application::ports::BoxFuture<'a", + "pub fn harvestcircle_storage::OsKeyringSecretStore::delete<'a>(&'a self, &'a harvestcircle_application::ports::DurableRequestId, harvestcircle_domain::key::PublicKey) -> harvestcircle_application::ports::BoxFuture<'a", "pub fn harvestcircle_storage::harvestcircle_migration_catalog()", "pub fn harvestcircle_storage::harvestcircle_schema_catalog()", ] { diff --git a/tools/xtask/src/lib.rs b/tools/xtask/src/lib.rs @@ -325,6 +325,15 @@ fn native_runtime_boundary(root: &Path, findings: &mut Vec<String>) { "response_receiver.await", "keyring worker", ), + ( + &keyring, + "const KEYRING_SHUTDOWN_DEADLINE: Duration = Duration::from_secs(30)", + "keyring worker", + ), + (&keyring, "OPERATION_QUEUED", "keyring worker"), + (&keyring, "OPERATION_STARTED", "keyring worker"), + (&keyring, "OPERATION_COMPLETED", "keyring worker"), + (&keyring, "OPERATION_CANCELLED", "keyring worker"), ] { if !source.contains(required) { findings.push(format!( @@ -337,6 +346,11 @@ fn native_runtime_boundary(root: &Path, findings: &mut Vec<String>) { "harvestcircle_ffi: keyring response blocks a Tokio runtime thread".to_owned(), ); } + if keyring.contains("std::sync::mpsc::Receiver") { + findings.push( + "harvestcircle_ffi: keyring response exposes a blocking receiver".to_owned(), + ); + } } fn namespace_audit(root: &Path, inventory: &Inventory, findings: &mut Vec<String>) {