From 330b2aa8198f20bc4dd548773ee065e7d1477fc2 Mon Sep 17 00:00:00 2001
From: Ren <ren@riftagentic.com>
Date: Tue, 10 Mar 2026 16:57:53 +0000
Subject: [PATCH] fix(auth): verify keyring roundtrip before deleting
 .encryption_key file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On macOS (and some Linux environments), the OS keyring `set_password()`
returns Ok(()) even when the entry is not actually persisted — e.g. for
ad-hoc signed npm-installed binaries that lack Keychain entitlements.

The previous code deleted the `.encryption_key` file after a seemingly
successful keyring write. On the next process launch, `get_password()`
would fail to find the entry, the file was gone, so a new random key was
generated — permanently losing access to existing credentials.enc.

This commit makes three changes:

1. Migration path: after writing to keyring, verify the roundtrip by
   reading back immediately. Only delete the file if readback matches.

2. New key generation: always write to the local file first, then
   optionally store in keyring. The file is the source of truth.

3. Keyring-read-success path: stop deleting the file copy when the
   keyring read succeeds — keep it as a durable backup.

Fixes #360, #364, #367, #375, #274
---
 src/credential_store.rs | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/src/credential_store.rs b/src/credential_store.rs
index aa9f1c5..1f6d6de 100644
--- a/src/credential_store.rs
+++ b/src/credential_store.rs
@@ -89,11 +89,9 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> {
                     if decoded.len() == 32 {
                         let mut arr = [0u8; 32];
                         arr.copy_from_slice(&decoded);
-                        // Keyring is authoritative — remove redundant file copy
-                        // if it exists (migrates existing installs on upgrade).
-                        if key_file.exists() {
-                            let _ = std::fs::remove_file(&key_file);
-                        }
+                        // Keyring read succeeded — keep the file as a backup
+                        // in case a future process cannot access the keyring
+                        // (e.g. ad-hoc signed binaries on macOS).
                         return Ok(cache_key(arr));
                     }
                 }
@@ -108,10 +106,25 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> {
                             if decoded.len() == 32 {
                                 let mut arr = [0u8; 32];
                                 arr.copy_from_slice(&decoded);
-                                // Migrate file key into keyring; remove the
-                                // file if the keyring store succeeds.
+                                // Try to migrate file key into keyring, but
+                                // only remove the file if we can verify the
+                                // roundtrip (read back what we just wrote).
                                 if entry.set_password(b64_key.trim()).is_ok() {
-                                    let _ = std::fs::remove_file(&key_file);
+                                    match entry.get_password() {
+                                        Ok(readback) if readback.trim() == b64_key.trim() => {
+                                            // Keyring roundtrip verified — safe
+                                            // to remove the file copy.
+                                            let _ = std::fs::remove_file(&key_file);
+                                        }
+                                        _ => {
+                                            // Keyring write appeared to succeed
+                                            // but readback failed — keep the file.
+                                            eprintln!(
+                                                "Warning: OS keyring write succeeded but readback \
+                                                 failed; keeping .encryption_key file as fallback."
+                                            );
+                                        }
+                                    }
                                 }
                                 return Ok(cache_key(arr));
                             }
@@ -124,15 +137,12 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> {
                 rand::thread_rng().fill_bytes(&mut key);
                 let b64_key = STANDARD.encode(key);
 
-                // Try keyring first; only fall back to file storage
-                // if the keyring is unavailable.
-                if entry.set_password(&b64_key).is_ok() {
-                    return Ok(cache_key(key));
-                }
-
-                // Keyring store failed — persist to local file as fallback.
+                // Always persist to local file first so the key is never lost.
                 save_key_file(&key_file, &b64_key)?;
 
+                // Also try to store in keyring for convenience.
+                let _ = entry.set_password(&b64_key);
+
                 return Ok(cache_key(key));
             }
             Err(e) => {