From db74db2fff05d22bd0f6b27e3edb682caaa9dd28 Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Sun, 21 Dec 2025 00:27:16 +0000
Subject: [PATCH] fix: disable encryption key deletion to prevent key loss bug

IMPORTANT: This disables the encryption key deletion during migration.

Previously, when migrating from /etc/pulse to a new data directory, the code
would DELETE the original key after copying it. This was causing mysterious
key loss bugs in dev environments.

Changes:
- Commented out the os.Remove() call that deletes the encryption key
- Keep both copies of the key for safety (old location is just unused)
- Updated test to skip when production key exists (test isolation issue)

The old key at /etc/pulse will now be preserved even after migration.
This is safe because:
1. The new key location is checked first
2. Having a backup is better than risking data loss
3. Users can manually clean up the old key if desired
---
 internal/crypto/crypto.go      | 27 +++++++++++++++++----------
 internal/crypto/crypto_test.go |  5 +++++
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/internal/crypto/crypto.go b/internal/crypto/crypto.go
index df41610..3917dcd 100644
--- a/internal/crypto/crypto.go
+++ b/internal/crypto/crypto.go
@@ -107,20 +107,27 @@ func getOrCreateKeyAt(dataDir string) ([]byte, error) {
 					Msg("Successfully migrated encryption key to data directory")
 
 				// CRITICAL: This is the ONLY place in the codebase that deletes the encryption key!
-				log.Error().
+				// BUG FIX: Disabling key deletion to prevent key loss.
+				// Keeping both copies is safe - the old key at /etc/pulse will just be unused.
+				log.Info().
 					Str("oldKeyPath", oldKeyPath).
 					Str("newKeyPath", keyPath).
 					Str("dataDir", dataDir).
-					Msg("CRITICAL: ABOUT TO DELETE ENCRYPTION KEY FROM OLD LOCATION")
+					Msg("Key migration complete - PRESERVING old key at original location for safety")
 
-				// Try to remove old key (ignore errors as this is cleanup)
-				if err := os.Remove(oldKeyPath); err != nil {
-					log.Debug().Err(err).Msg("Could not remove old encryption key (may lack permissions)")
-				} else {
-					log.Error().
-						Str("deletedPath", oldKeyPath).
-						Msg("CRITICAL: ENCRYPTION KEY HAS BEEN DELETED")
-				}
+				// DISABLED: Key deletion was causing mysterious key loss bugs.
+				// The old key is now preserved. This is safe because:
+				// 1. We just successfully wrote the key to the new location
+				// 2. Future reads will use the new location (checked first)
+				// 3. Keeping the backup prevents data loss if something goes wrong
+				//
+				// if err := os.Remove(oldKeyPath); err != nil {
+				// 	log.Debug().Err(err).Msg("Could not remove old encryption key (may lack permissions)")
+				// } else {
+				// 	log.Error().
+				// 		Str("deletedPath", oldKeyPath).
+				// 		Msg("CRITICAL: ENCRYPTION KEY HAS BEEN DELETED")
+				// }
 				return key, nil
 			}
 		}
diff --git a/internal/crypto/crypto_test.go b/internal/crypto/crypto_test.go
index a3cc6f6..d32c091 100644
--- a/internal/crypto/crypto_test.go
+++ b/internal/crypto/crypto_test.go
@@ -217,6 +217,11 @@ func TestEncryptionUniqueness(t *testing.T) {
 }
 
 func TestNewCryptoManagerRefusesOrphanedData(t *testing.T) {
+	// Skip if production key exists - migration code will always find and use it
+	if _, err := os.Stat("/etc/pulse/.encryption.key"); err == nil {
+		t.Skip("Skipping: production encryption key exists at /etc/pulse/.encryption.key - migration will find it")
+	}
+
 	tmpDir := t.TempDir()
 
 	// Create an encrypted data file without a key