From 3f3fd15959bb99ad76d7da4bd9856418ff22a0cd Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Mon, 9 Nov 2026 11:19:16 -0700 Subject: [PATCH] Release the clear's lock fully and clear conversion leftovers The exclusive traffic.lock a launch takes to clear is let go before it is taken shared, since Windows would otherwise hold both. Clearing the blob store also deletes what an interrupted encryption left beside it, so opening doesn't recover the old bodies. Co-Authored-By: Claude Opus 5.5 --- crates-proxy/yaak-proxy-lib/src/db.rs | 23 +++++++++++++++++++++-- crates-proxy/yaak-proxy-lib/src/lib.rs | 4 +++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/crates-proxy/yaak-proxy-lib/src/db.rs b/crates-proxy/yaak-proxy-lib/src/db.rs index 9c5ab4530..4a7c81ac4 100644 --- a/crates-proxy/yaak-proxy-lib/src/db.rs +++ b/crates-proxy/yaak-proxy-lib/src/db.rs @@ -271,9 +271,11 @@ pub fn remove_captured(data_dir: &Path) -> std::result::Result<(), String> { } /// Deletes `blobs.sqlite`, every body and message payload, for one made empty as it opens. -/// The data key stays, as `proxy.db` is still encrypted with it. Nothing may have it open. +/// What an interrupted conversion to encryption left beside it goes too, or opening would +/// recover it, bodies and all. The data key stays, as `proxy.db` is still encrypted with it. +/// Nothing may have it open. pub fn remove_blobs(data_dir: &Path) -> std::result::Result<(), String> { - for suffix in ["", "-wal", "-shm"] { + for suffix in ["", "-wal", "-shm", "-plain", "-encrypting"] { let path = data_dir.join(format!("{BLOBS_DB}{suffix}")); match std::fs::remove_file(&path) { Err(e) if e.kind() != std::io::ErrorKind::NotFound => { @@ -739,4 +741,21 @@ mod tests { let old = crate::flows::find(&db, "fl_old").unwrap(); assert!(!old.tunnelled); } + + /// Removing the blob store takes what an interrupted conversion left too, which opening + /// would otherwise recover, and leaves `proxy.db` and the key. + #[test] + fn removing_the_blobs_takes_a_conversion_s_leftovers() { + let dir = tempfile::tempdir().unwrap(); + let blobs = ["", "-wal", "-shm", "-plain", "-encrypting"].map(|s| format!("{BLOBS_DB}{s}")); + let kept = [PROXY_DB, DATA_KEY]; + for name in blobs.iter().map(String::as_str).chain(kept) { + std::fs::write(dir.path().join(name), b"x").unwrap(); + } + remove_blobs(dir.path()).unwrap(); + assert!(blobs.iter().all(|name| !dir.path().join(name).exists())); + assert!(kept.iter().all(|name| dir.path().join(name).exists())); + // Nothing there is fine too. + remove_blobs(dir.path()).unwrap(); + } } diff --git a/crates-proxy/yaak-proxy-lib/src/lib.rs b/crates-proxy/yaak-proxy-lib/src/lib.rs index 90eceaa73..e24b6f9a4 100644 --- a/crates-proxy/yaak-proxy-lib/src/lib.rs +++ b/crates-proxy/yaak-proxy-lib/src/lib.rs @@ -1484,8 +1484,10 @@ struct ShareOnDrop<'a>(&'a Mutex>); impl Drop for ShareOnDrop<'_> { fn drop(&mut self) { let held = self.0.lock().unwrap_or_else(PoisonError::into_inner); + // Let go first: Windows would otherwise add the shared lock beside the exclusive one. + // Unix's flock lets go between the two as well, so this is no wider a gap. if let Some(file) = held.as_ref() - && let Err(e) = file.lock_shared() + && let Err(e) = file.unlock().and_then(|()| file.lock_shared()) { warn!("Failed to share {TRAFFIC_LOCK}: {e}"); }