mirror of
https://github.com/sudosylabs/vnidrop.git
synced 2026-08-12 05:29:57 +02:00
fix(desktop): unblock protected-core startup snackbars
Run Secret Service IO on spawn_blocking so Linux zbus cannot nest Tokio runtimes during init, and wait for core initialize before experimental saved-device coordinators refresh. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -87,11 +87,14 @@ open domain stores via `persistence::open_all`.
|
||||
4. **Cancel:** signal active-transfer oneshot **synchronously** before async DB
|
||||
work. Use existing `take_active_transfer` / facade cancel path. Do not reintroduce
|
||||
nested exclusive `Runtime::block_on` deadlocks.
|
||||
5. **No lock across await:** Clippy `await_holding_lock` fails CI.
|
||||
6. **ReceiveOutputSink:** after successful `start_file`, exactly one of
|
||||
5. **SecureSecretStore:** never call the sync store from an async task body.
|
||||
Linux Secret Service / zbus blocking nests Tokio `block_on`; `SecretCustody`
|
||||
must keep those calls on `spawn_blocking`.
|
||||
6. **No lock across await:** Clippy `await_holding_lock` fails CI.
|
||||
7. **ReceiveOutputSink:** after successful `start_file`, exactly one of
|
||||
`finish_file` or `abort_file` (see `OutputSinkFile` Drop).
|
||||
7. **No-overwrite publish** for path receives (temp + hard link / exclusive rename).
|
||||
8. Integration tests must use the **public** API + `tests/support/` only.
|
||||
8. **No-overwrite publish** for path receives (temp + hard link / exclusive rename).
|
||||
9. Integration tests must use the **public** API + `tests/support/` only.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -395,12 +395,10 @@ impl SecretCustody {
|
||||
) -> Result<SecretHandle, VnidropError> {
|
||||
validate_material(kind, &material, expected_identity)?;
|
||||
let handle = SecretHandle::generate(kind);
|
||||
self.store
|
||||
.put(&handle, material.clone())
|
||||
.map_err(map_store_error)?;
|
||||
self.store_put(handle.clone(), material.clone()).await?;
|
||||
#[cfg(test)]
|
||||
self.maybe_crash(CustodyCrashPoint::StoreWrite)?;
|
||||
let stored = self.store.get(&handle).map_err(map_store_error)?;
|
||||
let stored = self.store_get(handle.clone()).await?;
|
||||
if stored != material {
|
||||
return Err(VnidropError::SecureStorageCorrupted {
|
||||
reason: "credential store did not preserve protected material".to_string(),
|
||||
@@ -408,7 +406,7 @@ impl SecretCustody {
|
||||
}
|
||||
validate_material(kind, &stored, expected_identity)?;
|
||||
if let Err(error) = self.metadata.stage(&handle, kind, expected_identity).await {
|
||||
self.delete_if_present(&handle)?;
|
||||
self.delete_if_present(&handle).await?;
|
||||
return Err(error);
|
||||
}
|
||||
#[cfg(test)]
|
||||
@@ -430,7 +428,7 @@ impl SecretCustody {
|
||||
reason: "protected secret is not active".to_string(),
|
||||
});
|
||||
}
|
||||
let material = self.store.get(handle).map_err(map_store_error)?;
|
||||
let material = self.store_get(handle.clone()).await?;
|
||||
validate_material(
|
||||
metadata.kind,
|
||||
&material,
|
||||
@@ -444,7 +442,7 @@ impl SecretCustody {
|
||||
if self.metadata.find(handle).await?.is_some() {
|
||||
self.metadata.disable(handle).await?;
|
||||
}
|
||||
self.delete_if_present(handle)
|
||||
self.delete_if_present(handle).await
|
||||
}
|
||||
|
||||
pub(crate) async fn list_active_handles(
|
||||
@@ -522,9 +520,14 @@ impl SecretCustody {
|
||||
.contains_kind(SecretKind::EndpointIdentity)
|
||||
.await?
|
||||
{
|
||||
return Err(VnidropError::SecureStorageUnavailable {
|
||||
reason: "protected endpoint identity is disabled".to_string(),
|
||||
});
|
||||
// Concurrent first-start may have staged (not yet active) metadata.
|
||||
// Wait for activation before treating leftover rows as disabled.
|
||||
return match self.wait_for_active_endpoint_identity().await {
|
||||
Ok(handle) => self.load(&handle).await,
|
||||
Err(_) => Err(VnidropError::SecureStorageUnavailable {
|
||||
reason: "protected endpoint identity is disabled".to_string(),
|
||||
}),
|
||||
};
|
||||
}
|
||||
match tokio::fs::try_exists(legacy_path).await {
|
||||
Ok(true) => {
|
||||
@@ -544,34 +547,38 @@ impl SecretCustody {
|
||||
.await
|
||||
{
|
||||
Ok(handle) => self.load(&handle).await,
|
||||
Err(error) => {
|
||||
let winner = tokio::time::timeout(Duration::from_secs(1), async {
|
||||
loop {
|
||||
if let Some(active) = self
|
||||
.metadata
|
||||
.find_active_kind(SecretKind::EndpointIdentity)
|
||||
.await?
|
||||
{
|
||||
return self.load(&active.handle).await;
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(10)).await;
|
||||
}
|
||||
})
|
||||
.await;
|
||||
match winner {
|
||||
Ok(result) => result,
|
||||
Err(_) => Err(error),
|
||||
}
|
||||
}
|
||||
Err(error) => match self.wait_for_active_endpoint_identity().await {
|
||||
Ok(handle) => self.load(&handle).await,
|
||||
Err(_) => Err(error),
|
||||
},
|
||||
}
|
||||
}
|
||||
Err(error) => Err(VnidropError::filesystem(error)),
|
||||
}
|
||||
}
|
||||
|
||||
async fn wait_for_active_endpoint_identity(&self) -> Result<SecretHandle, VnidropError> {
|
||||
tokio::time::timeout(Duration::from_secs(1), async {
|
||||
loop {
|
||||
if let Some(active) = self
|
||||
.metadata
|
||||
.find_active_kind(SecretKind::EndpointIdentity)
|
||||
.await?
|
||||
{
|
||||
return Ok(active.handle);
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(10)).await;
|
||||
}
|
||||
})
|
||||
.await
|
||||
.map_err(|_| VnidropError::SecureStorageUnavailable {
|
||||
reason: "timed out waiting for protected endpoint identity".to_string(),
|
||||
})?
|
||||
}
|
||||
|
||||
pub(crate) async fn reconcile(&self) -> Result<ReconciliationSummary, VnidropError> {
|
||||
let metadata = self.metadata.list().await?;
|
||||
let stored_handles = self.store.list_handles().map_err(map_store_error)?;
|
||||
let stored_handles = self.store_list_handles().await?;
|
||||
let known_handles = metadata
|
||||
.iter()
|
||||
.map(|entry| entry.handle.clone())
|
||||
@@ -580,16 +587,16 @@ impl SecretCustody {
|
||||
|
||||
for entry in metadata {
|
||||
if entry.state == SecretMetadataState::Disabled {
|
||||
self.delete_if_present(&entry.handle)?;
|
||||
self.delete_if_present(&entry.handle).await?;
|
||||
continue;
|
||||
}
|
||||
match self.store.get(&entry.handle) {
|
||||
match self.store_get_raw(entry.handle.clone()).await? {
|
||||
Ok(material) => {
|
||||
if validate_material(entry.kind, &material, entry.expected_identity.as_deref())
|
||||
.is_err()
|
||||
{
|
||||
self.metadata.disable(&entry.handle).await?;
|
||||
self.delete_if_present(&entry.handle)?;
|
||||
self.delete_if_present(&entry.handle).await?;
|
||||
summary.disabled += 1;
|
||||
} else if entry.state == SecretMetadataState::Staged {
|
||||
self.metadata.activate(&entry.handle).await?;
|
||||
@@ -598,7 +605,7 @@ impl SecretCustody {
|
||||
}
|
||||
Err(SecureSecretStoreError::Missing | SecureSecretStoreError::Corrupted) => {
|
||||
self.metadata.disable(&entry.handle).await?;
|
||||
self.delete_if_present(&entry.handle)?;
|
||||
self.delete_if_present(&entry.handle).await?;
|
||||
summary.disabled += 1;
|
||||
}
|
||||
Err(error) => return Err(map_store_error(error)),
|
||||
@@ -607,20 +614,74 @@ impl SecretCustody {
|
||||
|
||||
for handle in stored_handles {
|
||||
if !known_handles.contains(&handle) {
|
||||
self.store.delete(&handle).map_err(map_store_error)?;
|
||||
self.store_delete(handle).await?;
|
||||
summary.orphans_deleted += 1;
|
||||
}
|
||||
}
|
||||
Ok(summary)
|
||||
}
|
||||
|
||||
fn delete_if_present(&self, handle: &SecretHandle) -> Result<(), VnidropError> {
|
||||
match self.store.delete(handle) {
|
||||
async fn delete_if_present(&self, handle: &SecretHandle) -> Result<(), VnidropError> {
|
||||
match self.store_delete_raw(handle.clone()).await? {
|
||||
Ok(()) | Err(SecureSecretStoreError::Missing) => Ok(()),
|
||||
Err(error) => Err(map_store_error(error)),
|
||||
}
|
||||
}
|
||||
|
||||
// Platform credential stores (especially Linux Secret Service via zbus
|
||||
// blocking) nest their own Tokio `block_on`. Calling them on a worker
|
||||
// already inside Vnidrop's runtime panics with "Cannot start a runtime
|
||||
// from within a runtime" and breaks protected-core desktop startup.
|
||||
async fn store_put(
|
||||
&self,
|
||||
handle: SecretHandle,
|
||||
material: SecretMaterial,
|
||||
) -> Result<(), VnidropError> {
|
||||
let store = Arc::clone(&self.store);
|
||||
tokio::task::spawn_blocking(move || store.put(&handle, material))
|
||||
.await
|
||||
.map_err(VnidropError::internal)?
|
||||
.map_err(map_store_error)
|
||||
}
|
||||
|
||||
async fn store_get(&self, handle: SecretHandle) -> Result<SecretMaterial, VnidropError> {
|
||||
self.store_get_raw(handle).await?.map_err(map_store_error)
|
||||
}
|
||||
|
||||
async fn store_get_raw(
|
||||
&self,
|
||||
handle: SecretHandle,
|
||||
) -> Result<Result<SecretMaterial, SecureSecretStoreError>, VnidropError> {
|
||||
let store = Arc::clone(&self.store);
|
||||
tokio::task::spawn_blocking(move || store.get(&handle))
|
||||
.await
|
||||
.map_err(VnidropError::internal)
|
||||
}
|
||||
|
||||
async fn store_delete(&self, handle: SecretHandle) -> Result<(), VnidropError> {
|
||||
self.store_delete_raw(handle)
|
||||
.await?
|
||||
.map_err(map_store_error)
|
||||
}
|
||||
|
||||
async fn store_delete_raw(
|
||||
&self,
|
||||
handle: SecretHandle,
|
||||
) -> Result<Result<(), SecureSecretStoreError>, VnidropError> {
|
||||
let store = Arc::clone(&self.store);
|
||||
tokio::task::spawn_blocking(move || store.delete(&handle))
|
||||
.await
|
||||
.map_err(VnidropError::internal)
|
||||
}
|
||||
|
||||
async fn store_list_handles(&self) -> Result<Vec<SecretHandle>, VnidropError> {
|
||||
let store = Arc::clone(&self.store);
|
||||
tokio::task::spawn_blocking(move || store.list_handles())
|
||||
.await
|
||||
.map_err(VnidropError::internal)?
|
||||
.map_err(map_store_error)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn crash_once_at(&self, point: CustodyCrashPoint) {
|
||||
*self.crash_point.lock().unwrap() = Some(point);
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
use std::{collections::HashMap, sync::Arc};
|
||||
|
||||
// Blocking Secret Service / zbus owns a nested Tokio runtime. Never call this
|
||||
// adapter from a thread already inside Vnidrop's runtime — SecretCustody routes
|
||||
// store IO through `spawn_blocking` for that reason.
|
||||
use secret_service::{blocking::SecretService, EncryptionType, Error};
|
||||
|
||||
use super::{
|
||||
|
||||
@@ -47,12 +47,12 @@ fn lock_exclusive_nonblocking(file: &File) -> Result<(), VnidropError> {
|
||||
return Ok(());
|
||||
}
|
||||
let err = io::Error::last_os_error();
|
||||
return Err(match err.kind() {
|
||||
Err(match err.kind() {
|
||||
io::ErrorKind::WouldBlock => VnidropError::SecureStorageUnavailable {
|
||||
reason: "another protected core is already using this profile".to_string(),
|
||||
},
|
||||
_ => VnidropError::filesystem(err),
|
||||
});
|
||||
})
|
||||
}
|
||||
#[cfg(windows)]
|
||||
{
|
||||
|
||||
@@ -509,6 +509,9 @@ fn secret_service_identity_survives_core_restart() {
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn experimental_secret_service_identity_survives_core_restart_on_linux() {
|
||||
// Regression: protected init used to call blocking Secret Service on the
|
||||
// Tokio worker that drives `CoreInner::start`, which nested `block_on` and
|
||||
// aborted desktop startup with "Cannot start a runtime from within a runtime".
|
||||
let data_dir = tempfile::tempdir().unwrap();
|
||||
let sink = Arc::new(RecordingSink {
|
||||
events: Mutex::new(Vec::new()),
|
||||
|
||||
Reference in New Issue
Block a user