From f5977788c53ccde72273dfe3a9df3cdd45cacb83 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Fri, 7 Aug 2026 10:17:21 +0300
Subject: [PATCH 1/4] fix(keyring): stop concurrent writers wiping the shared
 secrets file

Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/openhuman/security/keyring/README.md      |   7 +-
 src/openhuman/security/keyring/backend.rs     | 110 +++++-----
 .../keyring/encrypted_file_backend.rs         |  66 ++----
 src/openhuman/security/keyring/file_store.rs  | 204 ++++++++++++++++++
 .../security/keyring/file_store_tests.rs      | 155 +++++++++++++
 src/openhuman/security/keyring/mod.rs         |  11 +
 src/openhuman/security/keyring/store.rs       |  52 ++++-
 src/openhuman/security/keyring/tests.rs       |  85 ++++++++
 8 files changed, 574 insertions(+), 116 deletions(-)
 create mode 100644 src/openhuman/security/keyring/file_store.rs
 create mode 100644 src/openhuman/security/keyring/file_store_tests.rs

diff --git a/src/openhuman/security/keyring/README.md b/src/openhuman/security/keyring/README.md
index 074f5c0fb2..d917c5e6b7 100644
--- a/src/openhuman/security/keyring/README.md
+++ b/src/openhuman/security/keyring/README.md
@@ -22,6 +22,7 @@ OS-keychain-backed secret storage with pluggable test/debug backends, plus a Cha
 | `src/openhuman/security/keyring/backend.rs` | `KeyringBackend` trait + `OsBackend` (native keychain via the `keyring` crate, service name `"openhuman"`), `FileBackend` (plaintext `dev-keychain.json`, test/debug only), and test-only `MockBackend`. |
 | `src/openhuman/security/keyring/encrypted_file_backend.rs` | `EncryptedFileBackend` — all secrets in one ChaCha20-Poly1305 `secrets.enc` file keyed by an app master key; `init_master_key`/`is_master_key_available`; legacy `dev-keychain.json` migration; corrupt-file quarantine. |
 | `src/openhuman/security/keyring/encrypted_store.rs` | `SecretStore` — config-field encryption (`enc2:` ChaCha20-Poly1305, legacy `enc:` XOR migration), keychain-backed master key with legacy `.secret_key` file migration, process-wide key cache, Windows ACL repair (`icacls`). |
+| `src/openhuman/security/keyring/file_store.rs` | Shared secrets-file primitives for both file backends: the cross-process advisory write lock (`lock_for_write`, on a sidecar `<path>.lock`), the `0600` unique-temp-then-rename `write_atomic`, and `quarantine_corrupt`. |
 | `src/openhuman/security/keyring/crypto.rs` | Shared ChaCha20-Poly1305 helpers (`chacha20_encrypt`/`chacha20_decrypt`), random-byte generation, hex encode/decode. Used by both `encrypted_store` and `encrypted_file_backend`. |
 | `src/openhuman/security/keyring/error.rs` | `KeyringError` (thiserror) with variants `Os`/`InvalidUtf8`/`MigrationReadFailed`/`VerifyFailed`/`MigrationDeleteFailed`/`RandomGeneration`/`Crypto`/`Backend`, plus a log-safe `diagnostic()` that preserves the `keyring::Error` variant + `OSStatus`. |
 | `src/openhuman/security/keyring/tests.rs` | Module tests (backend isolation via `force_backend_for_test`). |
@@ -62,7 +63,9 @@ Secret storage backend, selected once and frozen in a `OnceLock`:
 
 `SecretStore` additionally manages a master encryption key: keychain-backed (slot `secretstore.master_key`) in normal builds with one-time migration from the legacy `{data_dir}/openhuman/.secret_key` file; the file path is retained only for unit tests. Decoded keys are cached process-wide keyed by normalized path.
 
-Workspace dir resolves from `init_workspace`, else `OPENHUMAN_WORKSPACE`, else `~/.openhuman` (or `~/.openhuman-staging` under `OPENHUMAN_APP_ENV=staging`).
+Workspace dir resolves from `init_workspace`, else `OPENHUMAN_WORKSPACE`, else `~/.openhuman` (or `~/.openhuman-staging` under `OPENHUMAN_APP_ENV=staging`). **Under `cfg(test)` the environment is not consulted at all** — the fallback is a per-process scratch dir under the system temp dir, so test code cannot write into a developer's live store. A test that needs a specific location calls `init_workspace` before its first keyring call.
+
+Both file backends keep every secret in one file, so a `set` of one key rewrites all of them. That read-modify-write cycle is guarded by `file_store::lock_for_write` — an in-process mutex is not sufficient, because a desktop core, a `medulla` TUI embedding the same core, and a `cargo test` run that inherited `OPENHUMAN_WORKSPACE` all address the same path.
 
 ## Dependencies
 
@@ -86,6 +89,8 @@ Discovered consumers (`crate::openhuman::security::keyring::*`):
 - **`force_backend_for_test` panics if `BACKEND` is already initialized** — it must run before any keyring call in the same process (dedicated test binary or very top of a test).
 - **`migrate_from_file` never deletes the source unless the verified write succeeds**, so failures are retryable.
 - **`encrypted_file` corrupt/undecryptable files are quarantined** (renamed `secrets.enc.corrupt.<ts>`) and treated as empty rather than crashing.
+- **A corrupt file is never *overwritten*.** `file` reads degrade to an empty map (so a missing token means "sign in again"), but a `set`/`delete` over an unparseable file quarantines it and returns an error. Returning empty on the write path is what turned a corrupt file into a wipe: the write that followed persisted a map holding nothing but the key being set.
+- **Mutations hold a cross-process lock**, on `<secrets file>.lock` rather than the secrets file itself — `write_atomic` replaces the file by rename, so a lock on the old inode would guard nothing. Callers must hold it across the read *and* the write.
 - **`SecretStore` master-key file is write-once**; on Windows it survives transient AV-scanner sharing violations via retry/backoff and attempts `icacls` ACL self-repair on permission errors. Decoded keys are cached so repeated decrypts (e.g. snapshot polls) hit memory.
 - **Legacy formats:** `SecretStore` migrates `enc:` (XOR) → `enc2:` (ChaCha20-Poly1305) on decrypt; `EncryptedFileBackend` migrates plaintext `dev-keychain.json` → `secrets.enc` (renaming the legacy file `.json.migrated`).
 - **Errors never carry secret values** — only namespaced keys; `diagnostic()` is safe to log and preserves the underlying `keyring::Error` variant/`OSStatus`.
diff --git a/src/openhuman/security/keyring/backend.rs b/src/openhuman/security/keyring/backend.rs
index ef42c469be..60d4c40f29 100644
--- a/src/openhuman/security/keyring/backend.rs
+++ b/src/openhuman/security/keyring/backend.rs
@@ -15,9 +15,8 @@
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 
-use parking_lot::Mutex;
-
 use crate::openhuman::security::keyring::error::KeyringError;
+use crate::openhuman::security::keyring::file_store;
 
 // ── Trait ─────────────────────────────────────────────────────────────────────
 
@@ -105,15 +104,18 @@ impl KeyringBackend for OsBackend {
 /// keep unit tests and explicit recovery/debug overrides independent from the
 /// host OS keychain. Never use it in a production deployment.
 ///
-/// # Thread safety
+/// # Concurrency
 ///
-/// The `mutex` field serializes in-process read-modify-write operations on
-/// `set` and `delete`.  Cross-process safety relies on the atomic rename in
-/// `write_map`.
+/// Every secret lives in this one file, so a `set` or `delete` is a read →
+/// modify → write cycle over *all* of them. That cycle is guarded by the
+/// cross-process advisory lock in [`file_store::lock_for_write`] — an in-process
+/// mutex is not enough, because a desktop core, a `medulla` TUI embedding the
+/// same core, and a `cargo test` run that inherited `OPENHUMAN_WORKSPACE` all
+/// address the same path. Unguarded, the later writer's map (read before the
+/// earlier writer landed) silently discards the earlier one's secret; when that
+/// secret is the app session, the symptom is being signed out for no reason.
 pub struct FileBackend {
     path: PathBuf,
-    /// In-process lock covering the read→modify→write cycle in mutating ops.
-    mutex: Mutex<()>,
 }
 
 impl FileBackend {
@@ -121,7 +123,6 @@ impl FileBackend {
     pub fn new(workspace_dir: &Path) -> Self {
         Self {
             path: workspace_dir.join("dev-keychain.json"),
-            mutex: Mutex::new(()),
         }
     }
 
@@ -130,7 +131,18 @@ impl FileBackend {
         &self.path
     }
 
-    fn read_map(&self) -> Result<HashMap<String, String>, KeyringError> {
+    /// Read the whole map.
+    ///
+    /// `for_write` decides what an unparseable file means, and the two answers
+    /// are not interchangeable:
+    ///
+    /// - Reading (`false`): degrade to empty. The caller sees "no such secret",
+    ///   which for a session token means signing in again — recoverable, and
+    ///   better than failing every unrelated lookup.
+    /// - Writing (`true`): quarantine the bytes and fail. Returning empty here
+    ///   is what turned a corrupt file into a *wipe*, because the write that
+    ///   followed persisted a map holding nothing but the key being set.
+    fn read_map(&self, for_write: bool) -> Result<HashMap<String, String>, KeyringError> {
         if !self.path.exists() {
             return Ok(HashMap::new());
         }
@@ -141,82 +153,56 @@ impl FileBackend {
         if bytes.is_empty() {
             return Ok(HashMap::new());
         }
-        serde_json::from_slice::<HashMap<String, String>>(&bytes)
-            .map_err(|e| {
-                // Treat a corrupt file as empty so we degrade gracefully.
+        match serde_json::from_slice::<HashMap<String, String>>(&bytes) {
+            Ok(map) => Ok(map),
+            Err(e) if for_write => {
+                file_store::quarantine_corrupt(&self.path, "json");
+                Err(KeyringError::Backend(format!(
+                    "dev-keychain.json at {} could not be parsed ({e}); it was moved aside \
+                     rather than overwritten",
+                    self.path.display()
+                )))
+            }
+            Err(e) => {
                 log::warn!(
                     "[keyring] dev-keychain.json at {} is corrupt ({e}); treating as empty",
                     self.path.display()
                 );
-                // Return empty map by converting to a no-source variant.
-                drop(e);
-                KeyringError::VerifyFailed {
-                    key: "<parse>".to_string(),
-                }
-            })
-            .or_else(|_| Ok(HashMap::new()))
+                Ok(HashMap::new())
+            }
+        }
     }
 
     fn write_map(&self, map: &HashMap<String, String>) -> Result<(), KeyringError> {
-        if let Some(parent) = self.path.parent() {
-            std::fs::create_dir_all(parent).map_err(|e| KeyringError::MigrationReadFailed {
-                path: parent.display().to_string(),
-                source: e,
-            })?;
-        }
-
-        // Serialize the map to pretty JSON.  Propagate serialization failure so
-        // callers are not silently fed empty data on a write error.
+        // Propagate serialization failure so callers are not silently fed empty
+        // data on a write error.
         let json = serde_json::to_vec_pretty(map).map_err(|e| {
             KeyringError::Backend(format!("failed to serialize dev keychain map: {e}"))
         })?;
-
-        // Atomic write: temp file + rename.
-        let tmp_path = self.path.with_extension("tmp");
-        std::fs::write(&tmp_path, &json).map_err(|e| KeyringError::MigrationDeleteFailed {
-            path: tmp_path.display().to_string(),
-            source: e,
-        })?;
-
-        // Set mode 0600 on Unix before moving into place.
-        #[cfg(unix)]
-        {
-            use std::os::unix::fs::PermissionsExt;
-            let perms = std::fs::Permissions::from_mode(0o600);
-            if let Err(e) = std::fs::set_permissions(&tmp_path, perms) {
-                log::warn!("[keyring] could not set 0600 on dev-keychain.json tmp file: {e}");
-            }
-        }
-
-        std::fs::rename(&tmp_path, &self.path).map_err(|e| {
-            KeyringError::MigrationDeleteFailed {
-                path: self.path.display().to_string(),
-                source: e,
-            }
-        })?;
-
-        Ok(())
+        file_store::write_atomic(&self.path, &json)
     }
 }
 
 impl KeyringBackend for FileBackend {
     fn get(&self, namespaced_key: &str) -> Result<Option<String>, KeyringError> {
-        let map = self.read_map()?;
+        // No lock: `write_atomic` publishes by rename, so a reader sees either
+        // the whole previous file or the whole next one, never a mix.
+        let map = self.read_map(false)?;
         Ok(map.get(namespaced_key).cloned())
     }
 
     fn set(&self, namespaced_key: &str, value: &str) -> Result<(), KeyringError> {
-        // Hold the in-process lock for the full read→modify→write cycle.
-        let _guard = self.mutex.lock();
-        let mut map = self.read_map()?;
+        // Held across the read as well as the write: taking it around the write
+        // alone would still let a stale map overwrite a concurrent one.
+        let _guard = file_store::lock_for_write(&self.path)?;
+        let mut map = self.read_map(true)?;
         map.insert(namespaced_key.to_string(), value.to_string());
         self.write_map(&map)
     }
 
     fn delete(&self, namespaced_key: &str) -> Result<(), KeyringError> {
-        // Hold the in-process lock for the full read→modify→write cycle.
-        let _guard = self.mutex.lock();
-        let mut map = self.read_map()?;
+        let _guard = file_store::lock_for_write(&self.path)?;
+        let mut map = self.read_map(true)?;
         if map.remove(namespaced_key).is_some() {
             self.write_map(&map)?;
         }
diff --git a/src/openhuman/security/keyring/encrypted_file_backend.rs b/src/openhuman/security/keyring/encrypted_file_backend.rs
index 581579d111..e465661eff 100644
--- a/src/openhuman/security/keyring/encrypted_file_backend.rs
+++ b/src/openhuman/security/keyring/encrypted_file_backend.rs
@@ -13,11 +13,10 @@ use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 use std::sync::OnceLock;
 
-use parking_lot::Mutex;
-
 use crate::openhuman::security::keyring::backend::KeyringBackend;
 use crate::openhuman::security::keyring::crypto::{self, KEY_LEN};
 use crate::openhuman::security::keyring::error::KeyringError;
+use crate::openhuman::security::keyring::file_store;
 use crate::openhuman::security::keyring::store::BackendKind;
 
 const KEYCHAIN_SERVICE: &str = "openhuman";
@@ -174,10 +173,17 @@ fn master_key() -> Option<&'static [u8; KEY_LEN]> {
 
 // ── Backend ──────────────────────────────────────────────────────────────────
 
+/// Every secret in one ChaCha20-Poly1305 file.
+///
+/// Mutations are a read → decrypt → modify → encrypt → write cycle over the
+/// whole set, guarded by the cross-process advisory lock in
+/// [`file_store::lock_for_write`]. An in-process mutex would not do: more than
+/// one process routinely addresses the same workspace (a desktop core and a
+/// `medulla` TUI embedding the same core), and the later writer's snapshot —
+/// read before the earlier writer landed — silently drops the earlier secret.
 pub struct EncryptedFileBackend {
     path: PathBuf,
     workspace_dir: PathBuf,
-    mutex: Mutex<()>,
 }
 
 impl EncryptedFileBackend {
@@ -185,7 +191,6 @@ impl EncryptedFileBackend {
         Self {
             path: workspace_dir.join(SECRETS_FILENAME),
             workspace_dir: workspace_dir.to_path_buf(),
-            mutex: Mutex::new(()),
         }
     }
 
@@ -230,42 +235,13 @@ impl EncryptedFileBackend {
         key: &[u8; KEY_LEN],
         map: &HashMap<String, String>,
     ) -> Result<(), KeyringError> {
-        if let Some(parent) = self.path.parent() {
-            std::fs::create_dir_all(parent).map_err(|e| KeyringError::MigrationReadFailed {
-                path: parent.display().to_string(),
-                source: e,
-            })?;
-        }
-
         let json = serde_json::to_vec(map)
             .map_err(|e| KeyringError::Backend(format!("failed to serialize secrets: {e}")))?;
 
         let blob = crypto::chacha20_encrypt(key, &json)
             .map_err(|e| KeyringError::Backend(format!("encryption failed: {e}")))?;
 
-        let tmp_path = self.path.with_extension("enc.tmp");
-        std::fs::write(&tmp_path, &blob).map_err(|e| KeyringError::MigrationDeleteFailed {
-            path: tmp_path.display().to_string(),
-            source: e,
-        })?;
-
-        #[cfg(unix)]
-        {
-            use std::os::unix::fs::PermissionsExt;
-            let perms = std::fs::Permissions::from_mode(0o600);
-            if let Err(e) = std::fs::set_permissions(&tmp_path, perms) {
-                log::warn!("[keyring:encrypted_file] could not set 0600 on temp file: {e}");
-            }
-        }
-
-        std::fs::rename(&tmp_path, &self.path).map_err(|e| {
-            KeyringError::MigrationDeleteFailed {
-                path: self.path.display().to_string(),
-                source: e,
-            }
-        })?;
-
-        Ok(())
+        file_store::write_atomic(&self.path, &blob)
     }
 
     fn migrate_legacy_dev_keychain(
@@ -320,17 +296,10 @@ impl EncryptedFileBackend {
         Ok(map)
     }
 
+    /// Move an undecryptable / unparseable secrets file aside so the next call
+    /// starts fresh without destroying the bytes.
     fn handle_corruption(&self) {
-        let ts = chrono::Utc::now().format("%Y%m%d%H%M%S");
-        let corrupt_path = self.path.with_extension(format!("enc.corrupt.{ts}"));
-        if let Err(e) = std::fs::rename(&self.path, &corrupt_path) {
-            log::error!("[keyring:encrypted_file] could not rename corrupt file: {e}");
-        } else {
-            log::warn!(
-                "[keyring:encrypted_file] corrupt file renamed to {}",
-                corrupt_path.display()
-            );
-        }
+        file_store::quarantine_corrupt(&self.path, "enc");
     }
 }
 
@@ -339,7 +308,8 @@ impl KeyringBackend for EncryptedFileBackend {
         let Some(key) = master_key() else {
             return Ok(None);
         };
-        let _guard = self.mutex.lock();
+        // No lock: `write_atomic` publishes by rename, so a reader sees either
+        // the whole previous file or the whole next one, never a mix.
         let map = self.read_map(key)?;
         Ok(map.get(namespaced_key).cloned())
     }
@@ -350,7 +320,9 @@ impl KeyringBackend for EncryptedFileBackend {
                 "master key unavailable — cannot store secrets".to_string(),
             ));
         };
-        let _guard = self.mutex.lock();
+        // Held across the read as well as the write: taking it around the write
+        // alone would still let a stale map overwrite a concurrent one.
+        let _guard = file_store::lock_for_write(&self.path)?;
         let mut map = self.read_map(key)?;
         map.insert(namespaced_key.to_string(), value.to_string());
         self.write_map(key, &map)
@@ -360,7 +332,7 @@ impl KeyringBackend for EncryptedFileBackend {
         let Some(key) = master_key() else {
             return Ok(());
         };
-        let _guard = self.mutex.lock();
+        let _guard = file_store::lock_for_write(&self.path)?;
         let mut map = self.read_map(key)?;
         if map.remove(namespaced_key).is_some() {
             self.write_map(key, &map)?;
diff --git a/src/openhuman/security/keyring/file_store.rs b/src/openhuman/security/keyring/file_store.rs
new file mode 100644
index 0000000000..f6f8c27b67
--- /dev/null
+++ b/src/openhuman/security/keyring/file_store.rs
@@ -0,0 +1,204 @@
+//! Crash- and race-safe file primitives shared by the two file keyring backends.
+//!
+//! [`super::backend::FileBackend`] and [`super::encrypted_file_backend`] both
+//! keep every secret in **one** file and mutate it with a read → modify → write
+//! cycle. That shape is fine within a process — a mutex covers it — and unsafe
+//! across processes, which is the configuration this codebase actually runs in:
+//! a desktop core, a `medulla` TUI embedding the same core, and any `cargo test`
+//! run that inherits `OPENHUMAN_WORKSPACE` all address the same file.
+//!
+//! Two failures follow from doing that unguarded, and both destroy secrets
+//! rather than merely failing:
+//!
+//! - **Lost update.** A writes at t0 from a map it read at t-1, silently
+//!   discarding B's t-0.5 write. For the app-session entry that reads as "I
+//!   signed in, then got signed out again".
+//! - **Interleaved temp file.** Both backends staged their write through a
+//!   *fixed* sibling path (`dev-keychain.json.tmp` / `secrets.enc.tmp`), so two
+//!   writers wrote the same bytes-in-progress and one renamed the other's
+//!   half-written buffer into place. The result parses as garbage, and the
+//!   plaintext backend used to treat a parse failure as an empty map — so the
+//!   next `set` replaced thousands of secrets with one.
+//!
+//! [`lock_for_write`] closes the first (an advisory whole-file lock held for the
+//! full cycle, honoured across processes) and [`write_atomic`] closes the second
+//! (a temp name unique to this process and call). Neither is a substitute for
+//! the other.
+
+use std::fs::{File, OpenOptions};
+use std::io::Write;
+use std::path::{Path, PathBuf};
+use std::sync::atomic::{AtomicU64, Ordering};
+
+use fs2::FileExt;
+
+use crate::openhuman::security::keyring::error::KeyringError;
+
+/// Distinguishes concurrent temp files written by one process.
+///
+/// The pid alone is not enough: two threads in the same process stage their
+/// writes at the same instant, and the lock below is per *file*, not per
+/// backend instance, so a caller that legitimately holds no lock (a migration,
+/// a quarantine rewrite) can still overlap.
+static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0);
+
+/// An exclusive advisory lock on a secrets file, released on drop.
+///
+/// Held on a sidecar `<path>.lock` rather than the secrets file itself, because
+/// [`write_atomic`] replaces the secrets file by rename — a lock taken on the
+/// old inode would guard nothing once the rename lands.
+pub struct WriteLock {
+    /// Kept alive purely for its lock; `fs2` releases on close.
+    _file: File,
+}
+
+/// Take the exclusive write lock covering `path`'s read → modify → write cycle.
+///
+/// Blocks until the lock is free. Callers must hold the returned guard for the
+/// *whole* cycle — acquiring it around the write alone reintroduces the lost
+/// update it exists to prevent.
+///
+/// The lock is advisory and shared between threads as well as processes (`flock`
+/// and `LockFileEx` both serialize independent handles regardless of origin).
+///
+/// # Errors
+///
+/// Returns [`KeyringError::Backend`] when the lock file's directory cannot be
+/// created, the lock file cannot be opened, or the lock cannot be taken. Failing
+/// the operation is deliberate: proceeding unlocked is what loses secrets.
+pub fn lock_for_write(path: &Path) -> Result<WriteLock, KeyringError> {
+    let lock_path = lock_path_for(path);
+    if let Some(parent) = lock_path.parent() {
+        std::fs::create_dir_all(parent).map_err(|e| {
+            KeyringError::Backend(format!(
+                "could not create {} for the keyring lock: {e}",
+                parent.display()
+            ))
+        })?;
+    }
+    let file = OpenOptions::new()
+        .create(true)
+        .read(true)
+        .write(true)
+        .truncate(false)
+        .open(&lock_path)
+        .map_err(|e| {
+            KeyringError::Backend(format!(
+                "could not open the keyring lock at {}: {e}",
+                lock_path.display()
+            ))
+        })?;
+    file.lock_exclusive().map_err(|e| {
+        KeyringError::Backend(format!(
+            "could not lock the keyring at {}: {e}",
+            lock_path.display()
+        ))
+    })?;
+    Ok(WriteLock { _file: file })
+}
+
+/// The sidecar lock path for a secrets file.
+pub fn lock_path_for(path: &Path) -> PathBuf {
+    let mut name = path.file_name().unwrap_or_default().to_os_string();
+    name.push(".lock");
+    path.with_file_name(name)
+}
+
+/// Replace `path`'s contents with `bytes`, atomically and `0600`.
+///
+/// Staged through a temp file unique to this process and call, then renamed —
+/// so a concurrent writer can never observe, or rename into place, a partially
+/// written buffer. The temp file is removed if the rename fails, leaving no
+/// debris behind for the next run to trip over.
+///
+/// # Errors
+///
+/// Returns [`KeyringError::Backend`] when the parent directory cannot be
+/// created, or the temp file cannot be written or renamed.
+pub fn write_atomic(path: &Path, bytes: &[u8]) -> Result<(), KeyringError> {
+    if let Some(parent) = path.parent() {
+        std::fs::create_dir_all(parent).map_err(|e| {
+            KeyringError::Backend(format!(
+                "could not create {} for a keyring write: {e}",
+                parent.display()
+            ))
+        })?;
+    }
+
+    let tmp_path = temp_path_for(path);
+    let write = || -> std::io::Result<()> {
+        let mut file = OpenOptions::new()
+            .create_new(true)
+            .write(true)
+            .open(&tmp_path)?;
+        #[cfg(unix)]
+        {
+            use std::os::unix::fs::PermissionsExt;
+            // Before any bytes land: a window where the file is world-readable
+            // is a window where a secret is world-readable.
+            file.set_permissions(std::fs::Permissions::from_mode(0o600))?;
+        }
+        file.write_all(bytes)?;
+        // The rename below is atomic with respect to *ordering*, not durability.
+        // Without this a crash can leave the renamed file present but empty.
+        file.sync_all()
+    };
+    write().map_err(|e| {
+        let _ = std::fs::remove_file(&tmp_path);
+        KeyringError::Backend(format!(
+            "could not stage a keyring write at {}: {e}",
+            tmp_path.display()
+        ))
+    })?;
+
+    std::fs::rename(&tmp_path, path).map_err(|e| {
+        let _ = std::fs::remove_file(&tmp_path);
+        KeyringError::Backend(format!(
+            "could not replace the keyring file at {}: {e}",
+            path.display()
+        ))
+    })
+}
+
+/// A temp sibling of `path` that no other process or thread will pick.
+fn temp_path_for(path: &Path) -> PathBuf {
+    let seq = TEMP_COUNTER.fetch_add(1, Ordering::Relaxed);
+    let mut name = path.file_name().unwrap_or_default().to_os_string();
+    name.push(format!(".{}.{seq}.tmp", std::process::id()));
+    path.with_file_name(name)
+}
+
+/// Move a file that cannot be parsed aside, so its bytes survive for recovery.
+///
+/// Returns the quarantine path when the file was moved. A failure to move is
+/// logged and reported as `None` rather than propagated: the caller is already
+/// on an error path, and losing the quarantine is not worse than the corruption
+/// that triggered it.
+///
+/// `suffix` names the artefact (`"json"`, `"enc"`) so the quarantined file keeps
+/// a recognisable extension.
+pub fn quarantine_corrupt(path: &Path, suffix: &str) -> Option<PathBuf> {
+    let stamp = chrono::Utc::now().timestamp();
+    let target = path.with_extension(format!("{suffix}.corrupt.{stamp}"));
+    match std::fs::rename(path, &target) {
+        Ok(()) => {
+            log::error!(
+                "[keyring] {} could not be parsed; moved to {} and treated as empty",
+                path.display(),
+                target.display()
+            );
+            Some(target)
+        }
+        Err(e) => {
+            log::error!(
+                "[keyring] {} could not be parsed and could not be moved aside: {e}",
+                path.display()
+            );
+            None
+        }
+    }
+}
+
+#[cfg(test)]
+#[path = "file_store_tests.rs"]
+mod tests;
diff --git a/src/openhuman/security/keyring/file_store_tests.rs b/src/openhuman/security/keyring/file_store_tests.rs
new file mode 100644
index 0000000000..16f506fb6e
--- /dev/null
+++ b/src/openhuman/security/keyring/file_store_tests.rs
@@ -0,0 +1,155 @@
+//! Tests for the shared secrets-file primitives.
+
+use std::path::Path;
+
+use super::{lock_for_write, lock_path_for, quarantine_corrupt, temp_path_for, write_atomic};
+
+#[test]
+fn write_atomic_replaces_contents() {
+    let dir = tempfile::TempDir::new().unwrap();
+    let path = dir.path().join("secrets.json");
+
+    write_atomic(&path, b"first").unwrap();
+    assert_eq!(std::fs::read(&path).unwrap(), b"first");
+
+    write_atomic(&path, b"second").unwrap();
+    assert_eq!(std::fs::read(&path).unwrap(), b"second");
+}
+
+#[test]
+fn write_atomic_creates_missing_parents() {
+    let dir = tempfile::TempDir::new().unwrap();
+    let path = dir
+        .path()
+        .join("nested")
+        .join("deeper")
+        .join("secrets.json");
+
+    write_atomic(&path, b"value").unwrap();
+
+    assert_eq!(std::fs::read(&path).unwrap(), b"value");
+}
+
+/// The regression the unique temp name exists for: a second writer must not be
+/// able to collide with the first writer's staging file.
+#[test]
+fn temp_paths_are_unique_per_call() {
+    let path = Path::new("/tmp/openhuman-test/dev-keychain.json");
+    let first = temp_path_for(path);
+    let second = temp_path_for(path);
+
+    assert_ne!(first, second);
+    assert!(first.to_string_lossy().ends_with(".tmp"));
+    assert_eq!(first.parent(), path.parent());
+}
+
+#[test]
+fn write_atomic_leaves_no_temp_files_behind() {
+    let dir = tempfile::TempDir::new().unwrap();
+    let path = dir.path().join("secrets.json");
+
+    write_atomic(&path, b"value").unwrap();
+
+    let leftovers: Vec<_> = std::fs::read_dir(dir.path())
+        .unwrap()
+        .filter_map(Result::ok)
+        .map(|entry| entry.file_name().to_string_lossy().into_owned())
+        .filter(|name| name.ends_with(".tmp"))
+        .collect();
+    assert!(
+        leftovers.is_empty(),
+        "temp files left behind: {leftovers:?}"
+    );
+}
+
+#[cfg(unix)]
+#[test]
+fn write_atomic_writes_owner_only_permissions() {
+    use std::os::unix::fs::PermissionsExt;
+
+    let dir = tempfile::TempDir::new().unwrap();
+    let path = dir.path().join("secrets.json");
+
+    write_atomic(&path, b"value").unwrap();
+
+    let mode = std::fs::metadata(&path).unwrap().permissions().mode();
+    assert_eq!(mode & 0o777, 0o600, "unexpected mode {mode:o}");
+}
+
+#[test]
+fn lock_path_is_a_sibling_of_the_secrets_file() {
+    let path = Path::new("/tmp/openhuman-test/dev-keychain.json");
+    assert_eq!(
+        lock_path_for(path),
+        Path::new("/tmp/openhuman-test/dev-keychain.json.lock")
+    );
+}
+
+/// The lock must serialize independent holders, not merely independent
+/// processes: a second acquirer waits until the first guard is dropped.
+#[test]
+fn lock_serializes_concurrent_holders() {
+    use std::sync::mpsc;
+    use std::time::Duration;
+
+    let dir = tempfile::TempDir::new().unwrap();
+    let path = dir.path().join("dev-keychain.json");
+
+    let guard = lock_for_write(&path).unwrap();
+
+    let (tx, rx) = mpsc::channel();
+    let contender_path = path.clone();
+    let contender = std::thread::spawn(move || {
+        let _guard = lock_for_write(&contender_path).unwrap();
+        tx.send(()).unwrap();
+    });
+
+    // Still held here, so the contender must not have gotten through.
+    assert!(
+        rx.recv_timeout(Duration::from_millis(200)).is_err(),
+        "the lock let a second holder in while the first was live"
+    );
+
+    drop(guard);
+    rx.recv_timeout(Duration::from_secs(5))
+        .expect("the contender should acquire the lock once it is released");
+    contender.join().unwrap();
+}
+
+#[test]
+fn lock_survives_the_file_being_replaced() {
+    let dir = tempfile::TempDir::new().unwrap();
+    let path = dir.path().join("dev-keychain.json");
+    write_atomic(&path, b"before").unwrap();
+
+    let guard = lock_for_write(&path).unwrap();
+    // The rename inside `write_atomic` swaps the secrets file's inode; the lock
+    // is on the sidecar, so it is unaffected.
+    write_atomic(&path, b"after").unwrap();
+    drop(guard);
+
+    assert_eq!(std::fs::read(&path).unwrap(), b"after");
+    assert!(lock_path_for(&path).exists());
+}
+
+#[test]
+fn quarantine_moves_the_file_aside_and_reports_the_path() {
+    let dir = tempfile::TempDir::new().unwrap();
+    let path = dir.path().join("dev-keychain.json");
+    std::fs::write(&path, b"{ not json").unwrap();
+
+    let moved = quarantine_corrupt(&path, "json").expect("the file should be moved aside");
+
+    assert!(
+        !path.exists(),
+        "the corrupt file should no longer be in place"
+    );
+    assert_eq!(std::fs::read(&moved).unwrap(), b"{ not json");
+    assert!(moved.to_string_lossy().contains(".corrupt."));
+}
+
+#[test]
+fn quarantine_reports_none_when_there_is_nothing_to_move() {
+    let dir = tempfile::TempDir::new().unwrap();
+    assert!(quarantine_corrupt(&dir.path().join("absent.json"), "json").is_none());
+}
diff --git a/src/openhuman/security/keyring/mod.rs b/src/openhuman/security/keyring/mod.rs
index 2c26ad9cc2..c6bb726bee 100644
--- a/src/openhuman/security/keyring/mod.rs
+++ b/src/openhuman/security/keyring/mod.rs
@@ -27,12 +27,23 @@
 //! `false` when the `os` backend is selected.  Callers that opt out of keychain
 //! storage (file-encrypted JSON fallback) check this flag.  The `file` backend
 //! always reports as available.
+//!
+//! # File backends hold every secret in one file
+//!
+//! Both file backends keep the whole secret set in a single file, so a `set` of
+//! one key rewrites all of them. More than one process routinely addresses the
+//! same workspace, so that cycle is guarded by the cross-process lock in
+//! [`file_store`] — see that module for the two ways an unguarded cycle destroys
+//! secrets. Under `cfg(test)` the file backends resolve to a per-process scratch
+//! directory rather than any real workspace, so test code cannot reach a
+//! developer's live store.
 
 pub mod backend;
 pub mod crypto;
 pub mod encrypted_file_backend;
 pub mod encrypted_store;
 pub mod error;
+pub mod file_store;
 pub mod ops;
 pub mod store;
 
diff --git a/src/openhuman/security/keyring/store.rs b/src/openhuman/security/keyring/store.rs
index ff30e9c276..f761f278fc 100644
--- a/src/openhuman/security/keyring/store.rs
+++ b/src/openhuman/security/keyring/store.rs
@@ -155,11 +155,19 @@ fn is_staging_or_production_value(app_env: Option<&str>) -> bool {
 /// Uses the registered value from [`init_workspace`] if set; otherwise falls
 /// back to the same env-var / home-dir logic as the config subsystem.
 /// Always resolves to a stable absolute path — never CWD.
+///
+/// Under `cfg(test)` the environment is deliberately *not* consulted — see the
+/// test build of [`fallback_workspace_dir`].
 pub fn workspace_dir_for_file_backend() -> PathBuf {
-    if let Some(dir) = WORKSPACE_DIR.get() {
-        return dir.clone();
-    }
+    WORKSPACE_DIR
+        .get()
+        .cloned()
+        .unwrap_or_else(fallback_workspace_dir)
+}
 
+/// The directory to use when no caller registered one via [`init_workspace`].
+#[cfg(not(test))]
+fn fallback_workspace_dir() -> PathBuf {
     if let Ok(custom) = std::env::var("OPENHUMAN_WORKSPACE") {
         if !custom.trim().is_empty() {
             return PathBuf::from(custom);
@@ -169,11 +177,43 @@ pub fn workspace_dir_for_file_backend() -> PathBuf {
     let home = dirs::home_dir().unwrap_or_else(|| {
         PathBuf::from(std::env::var("HOME").unwrap_or_else(|_| "/tmp".to_string()))
     });
-    let openhuman_dir = match std::env::var("OPENHUMAN_APP_ENV").as_deref() {
+    match std::env::var("OPENHUMAN_APP_ENV").as_deref() {
         Ok("staging") => home.join(".openhuman-staging"),
         _ => home.join(".openhuman"),
-    };
-    openhuman_dir
+    }
+}
+
+/// A scratch directory owned by this test binary, used instead of any real
+/// workspace.
+///
+/// Test code must never be able to reach a developer's live secret store, and
+/// before this it could: the default honoured `OPENHUMAN_WORKSPACE`, which the
+/// Medulla TUI exports into every process it spawns. A `cargo test` run started
+/// from such a shell wrote its per-test secrets straight into the developer's
+/// `dev-keychain.json` — thousands of tempdir-scoped junk entries, sharing one
+/// read-modify-write file with the live app session sitting beside them.
+///
+/// A test that wants a specific location still gets one by calling
+/// [`init_workspace`] before its first keyring call; only the *fallback* is
+/// redirected. The directory is per-process and never cleaned up, because the
+/// backend outlives any single test (`BACKEND` is a `OnceLock`) — the OS clears
+/// it with the rest of the temp dir.
+#[cfg(test)]
+fn fallback_workspace_dir() -> PathBuf {
+    static TEST_DIR: OnceLock<PathBuf> = OnceLock::new();
+    TEST_DIR
+        .get_or_init(|| {
+            let dir =
+                std::env::temp_dir().join(format!("openhuman-test-keyring-{}", std::process::id()));
+            if let Err(e) = std::fs::create_dir_all(&dir) {
+                log::warn!(
+                    "[keyring] could not create the test keyring dir {}: {e}",
+                    dir.display()
+                );
+            }
+            dir
+        })
+        .clone()
 }
 
 #[cfg(test)]
diff --git a/src/openhuman/security/keyring/tests.rs b/src/openhuman/security/keyring/tests.rs
index 0342ba1609..71dee7072d 100644
--- a/src/openhuman/security/keyring/tests.rs
+++ b/src/openhuman/security/keyring/tests.rs
@@ -128,6 +128,91 @@ fn file_backend_multiple_keys_independent() {
     );
 }
 
+// ── FileBackend: the file holds every secret, so a bad write loses all of them ─
+
+/// The regression this whole guard exists for.
+///
+/// A corrupt file used to read as an *empty map*, so the `set` that followed
+/// persisted a map holding nothing but the key being written — every other
+/// secret in the file, including the app session, was gone. Signing in and then
+/// finding yourself signed out again is what that looks like from outside.
+#[test]
+fn file_backend_set_refuses_to_overwrite_an_unparseable_file() {
+    let dir = TempDir::new().expect("tempdir");
+    let fb = FileBackend::new(dir.path());
+    fb.set("session", "keep-me").unwrap();
+
+    let path = dir.path().join("dev-keychain.json");
+    let original = std::fs::read(&path).unwrap();
+    std::fs::write(&path, b"{ truncated by a racing writer").unwrap();
+
+    let err = fb
+        .set("other", "v")
+        .expect_err("a set over an unparseable file must fail, not replace it");
+    assert!(
+        err.to_string().contains("moved aside"),
+        "the error should say the file was preserved: {err}"
+    );
+
+    // The bytes survived under the quarantine name, so nothing is unrecoverable.
+    let quarantined: Vec<_> = std::fs::read_dir(dir.path())
+        .unwrap()
+        .filter_map(Result::ok)
+        .map(|e| e.file_name().to_string_lossy().into_owned())
+        .filter(|name| name.contains(".corrupt."))
+        .collect();
+    assert_eq!(quarantined.len(), 1, "expected one quarantined file");
+    assert_ne!(original, b"{ truncated by a racing writer".to_vec());
+}
+
+/// A *read* still degrades to "no such secret" rather than failing, because the
+/// caller's remedy (sign in again) is recoverable and failing every unrelated
+/// lookup is not.
+#[test]
+fn file_backend_get_treats_an_unparseable_file_as_empty() {
+    let dir = TempDir::new().expect("tempdir");
+    let fb = FileBackend::new(dir.path());
+    std::fs::write(dir.path().join("dev-keychain.json"), b"not json").unwrap();
+
+    assert!(fb.get("anything").unwrap().is_none());
+    assert!(
+        dir.path().join("dev-keychain.json").exists(),
+        "a read must not move the file aside"
+    );
+}
+
+/// Concurrent writers must not lose each other's entries. Before the
+/// cross-process lock, each writer persisted the map it had read *before* the
+/// other landed, so all but the last key vanished.
+#[test]
+fn file_backend_concurrent_sets_all_survive() {
+    let dir = TempDir::new().expect("tempdir");
+    let writers = 8;
+
+    std::thread::scope(|scope| {
+        for i in 0..writers {
+            let path = dir.path().to_path_buf();
+            scope.spawn(move || {
+                // A backend instance per thread: one shared instance would be
+                // covered by an in-process mutex, which is precisely the
+                // guarantee that turned out to be insufficient.
+                FileBackend::new(&path)
+                    .set(&format!("key{i}"), &format!("v{i}"))
+                    .unwrap();
+            });
+        }
+    });
+
+    let fb = FileBackend::new(dir.path());
+    for i in 0..writers {
+        assert_eq!(
+            fb.get(&format!("key{i}")).unwrap().as_deref(),
+            Some(format!("v{i}").as_str()),
+            "key{i} was lost to a concurrent write"
+        );
+    }
+}
+
 // ── FileBackend: migrate_from_file (via production function) ──────────────────
 //
 // These tests exercise the full `migrate_from_file` production function via a

From 66bd6b337cfc18bc48852528eedfb387567857d8 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Fri, 7 Aug 2026 11:38:34 +0300
Subject: [PATCH 2/4] fix(keyring): lock legacy migration reads

Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 .../security/keyring/encrypted_file_backend.rs      | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/openhuman/security/keyring/encrypted_file_backend.rs b/src/openhuman/security/keyring/encrypted_file_backend.rs
index e465661eff..4fcad02546 100644
--- a/src/openhuman/security/keyring/encrypted_file_backend.rs
+++ b/src/openhuman/security/keyring/encrypted_file_backend.rs
@@ -308,8 +308,17 @@ impl KeyringBackend for EncryptedFileBackend {
         let Some(key) = master_key() else {
             return Ok(None);
         };
-        // No lock: `write_atomic` publishes by rename, so a reader sees either
-        // the whole previous file or the whole next one, never a mix.
+        // Usually no lock is needed: `write_atomic` publishes by rename, so a
+        // reader sees either the whole previous file or the whole next one,
+        // never a mix. A missing encrypted file is different: `read_map`
+        // migrates the legacy store, which writes and renames files. Hold the
+        // same lock as writers across that read so its legacy snapshot cannot
+        // overwrite a concurrent `set`.
+        if !self.path.exists() {
+            let _guard = file_store::lock_for_write(&self.path)?;
+            let map = self.read_map(key)?;
+            return Ok(map.get(namespaced_key).cloned());
+        }
         let map = self.read_map(key)?;
         Ok(map.get(namespaced_key).cloned())
     }

From e566e86727217f098969488a2edb4efb40b3f2b5 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Fri, 7 Aug 2026 12:05:14 +0300
Subject: [PATCH 3/4] fix(keyring): serialize corrupt reads and retry temp
 files

Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 .../keyring/encrypted_file_backend.rs         | 16 +++-------
 src/openhuman/security/keyring/file_store.rs  | 32 ++++++++++++++++---
 .../security/keyring/file_store_tests.rs      | 21 +++++++++++-
 3 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/src/openhuman/security/keyring/encrypted_file_backend.rs b/src/openhuman/security/keyring/encrypted_file_backend.rs
index 4fcad02546..df98dc2b8c 100644
--- a/src/openhuman/security/keyring/encrypted_file_backend.rs
+++ b/src/openhuman/security/keyring/encrypted_file_backend.rs
@@ -308,17 +308,11 @@ impl KeyringBackend for EncryptedFileBackend {
         let Some(key) = master_key() else {
             return Ok(None);
         };
-        // Usually no lock is needed: `write_atomic` publishes by rename, so a
-        // reader sees either the whole previous file or the whole next one,
-        // never a mix. A missing encrypted file is different: `read_map`
-        // migrates the legacy store, which writes and renames files. Hold the
-        // same lock as writers across that read so its legacy snapshot cannot
-        // overwrite a concurrent `set`.
-        if !self.path.exists() {
-            let _guard = file_store::lock_for_write(&self.path)?;
-            let map = self.read_map(key)?;
-            return Ok(map.get(namespaced_key).cloned());
-        }
+        // `read_map` can mutate the filesystem: it migrates a missing file and
+        // quarantines corrupt ciphertext. Hold the same lock as writers for
+        // either case so a delayed quarantine cannot rename a replacement a
+        // concurrent `set` just published.
+        let _guard = file_store::lock_for_write(&self.path)?;
         let map = self.read_map(key)?;
         Ok(map.get(namespaced_key).cloned())
     }
diff --git a/src/openhuman/security/keyring/file_store.rs b/src/openhuman/security/keyring/file_store.rs
index f6f8c27b67..ad3f5a913b 100644
--- a/src/openhuman/security/keyring/file_store.rs
+++ b/src/openhuman/security/keyring/file_store.rs
@@ -125,12 +125,11 @@ pub fn write_atomic(path: &Path, bytes: &[u8]) -> Result<(), KeyringError> {
         })?;
     }
 
-    let tmp_path = temp_path_for(path);
+    // A stale temp file can survive a crash. PID reuse then makes the first
+    // sequence value collide, so keep allocating sequence values until a new
+    // staging path is reserved for this write.
+    let (tmp_path, mut file) = reserve_temp_file(|| temp_path_for(path))?;
     let write = || -> std::io::Result<()> {
-        let mut file = OpenOptions::new()
-            .create_new(true)
-            .write(true)
-            .open(&tmp_path)?;
         #[cfg(unix)]
         {
             use std::os::unix::fs::PermissionsExt;
@@ -160,6 +159,29 @@ pub fn write_atomic(path: &Path, bytes: &[u8]) -> Result<(), KeyringError> {
     })
 }
 
+/// Reserve a fresh temp file, advancing past leftovers from crashed writers.
+fn reserve_temp_file(
+    mut next_path: impl FnMut() -> PathBuf,
+) -> Result<(PathBuf, File), KeyringError> {
+    loop {
+        let tmp_path = next_path();
+        match OpenOptions::new()
+            .create_new(true)
+            .write(true)
+            .open(&tmp_path)
+        {
+            Ok(file) => break (tmp_path, file),
+            Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
+            Err(e) => {
+                return Err(KeyringError::Backend(format!(
+                    "could not stage a keyring write at {}: {e}",
+                    tmp_path.display()
+                )));
+            }
+        }
+    }
+}
+
 /// A temp sibling of `path` that no other process or thread will pick.
 fn temp_path_for(path: &Path) -> PathBuf {
     let seq = TEMP_COUNTER.fetch_add(1, Ordering::Relaxed);
diff --git a/src/openhuman/security/keyring/file_store_tests.rs b/src/openhuman/security/keyring/file_store_tests.rs
index 16f506fb6e..342189a90e 100644
--- a/src/openhuman/security/keyring/file_store_tests.rs
+++ b/src/openhuman/security/keyring/file_store_tests.rs
@@ -1,8 +1,12 @@
 //! Tests for the shared secrets-file primitives.
 
+use std::collections::VecDeque;
 use std::path::Path;
 
-use super::{lock_for_write, lock_path_for, quarantine_corrupt, temp_path_for, write_atomic};
+use super::{
+    lock_for_write, lock_path_for, quarantine_corrupt, reserve_temp_file, temp_path_for,
+    write_atomic,
+};
 
 #[test]
 fn write_atomic_replaces_contents() {
@@ -62,6 +66,21 @@ fn write_atomic_leaves_no_temp_files_behind() {
     );
 }
 
+#[test]
+fn reserve_temp_file_skips_a_stale_path() {
+    let dir = tempfile::TempDir::new().unwrap();
+    let stale = dir.path().join("stale.tmp");
+    let fresh = dir.path().join("fresh.tmp");
+
+    std::fs::write(&stale, b"stale").unwrap();
+    let mut paths = VecDeque::from([stale.clone(), fresh.clone()]);
+    let (claimed, file) = reserve_temp_file(|| paths.pop_front().unwrap()).unwrap();
+    drop(file);
+
+    assert_eq!(claimed, fresh);
+    assert_eq!(std::fs::read(&stale).unwrap(), b"stale");
+}
+
 #[cfg(unix)]
 #[test]
 fn write_atomic_writes_owner_only_permissions() {

From 51d691cbb99c8295efe9f37930962d2eb1588836 Mon Sep 17 00:00:00 2001
From: Steven Enamakel <enamakel@tinyhumans.ai>
Date: Fri, 7 Aug 2026 12:15:30 +0300
Subject: [PATCH 4/4] fix(keyring): compile race-safe file writes

Co-authored-by: Medulla <medulla@tinyhumans.ai>
---
 src/openhuman/security/keyring/file_store.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/openhuman/security/keyring/file_store.rs b/src/openhuman/security/keyring/file_store.rs
index ad3f5a913b..57cb65729d 100644
--- a/src/openhuman/security/keyring/file_store.rs
+++ b/src/openhuman/security/keyring/file_store.rs
@@ -129,7 +129,7 @@ pub fn write_atomic(path: &Path, bytes: &[u8]) -> Result<(), KeyringError> {
     // sequence value collide, so keep allocating sequence values until a new
     // staging path is reserved for this write.
     let (tmp_path, mut file) = reserve_temp_file(|| temp_path_for(path))?;
-    let write = || -> std::io::Result<()> {
+    let mut write = || -> std::io::Result<()> {
         #[cfg(unix)]
         {
             use std::os::unix::fs::PermissionsExt;
@@ -170,7 +170,7 @@ fn reserve_temp_file(
             .write(true)
             .open(&tmp_path)
         {
-            Ok(file) => break (tmp_path, file),
+            Ok(file) => break Ok((tmp_path, file)),
             Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
             Err(e) => {
                 return Err(KeyringError::Backend(format!(