From 7264026d66b61d55a4c44589422ec09eac666054 Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Tue, 23 Dec 2025 14:14:14 +0700
Subject: [PATCH] Fix unlocker rm to succeed when keychain item is missing

When removing a keychain unlocker, if the keychain item doesn't exist
(e.g., already manually deleted or vault synced from another machine),
the removal should still succeed since the goal is to remove the
unlocker and the keychain item being gone already satisfies that goal.
---
 internal/secret/keychainunlocker.go      | 13 +++++++++++++
 internal/secret/keychainunlocker_test.go | 17 +++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/internal/secret/keychainunlocker.go b/internal/secret/keychainunlocker.go
index 8de30de..c19705a 100644
--- a/internal/secret/keychainunlocker.go
+++ b/internal/secret/keychainunlocker.go
@@ -530,6 +530,8 @@ func retrieveFromKeychain(itemName string) ([]byte, error) {
 }
 
 // deleteFromKeychain removes an item from the macOS keychain using keybase/go-keychain
+// If the item doesn't exist, this function returns nil (not an error) since the goal
+// is to ensure the item is gone, and it already being gone satisfies that goal.
 func deleteFromKeychain(itemName string) error {
 	if err := validateKeychainItemName(itemName); err != nil {
 		return fmt.Errorf("invalid keychain item name: %w", err)
@@ -541,6 +543,17 @@ func deleteFromKeychain(itemName string) error {
 	item.SetAccount(itemName)
 
 	if err := keychain.DeleteItem(item); err != nil {
+		// If the item doesn't exist, that's not an error - the goal is to ensure
+		// the item is gone, and it already being gone satisfies that goal.
+		// This is important for cleaning up unlocker directories when the keychain
+		// item has already been removed (e.g., manually by user, or synced vault
+		// from a different machine).
+		if err == keychain.ErrorItemNotFound {
+			Debug("Keychain item not found during deletion, ignoring", "item_name", itemName)
+
+			return nil
+		}
+
 		return fmt.Errorf("failed to delete item from keychain: %w", err)
 	}
 
diff --git a/internal/secret/keychainunlocker_test.go b/internal/secret/keychainunlocker_test.go
index 698de11..981ee6e 100644
--- a/internal/secret/keychainunlocker_test.go
+++ b/internal/secret/keychainunlocker_test.go
@@ -165,3 +165,20 @@ func TestKeychainLargeData(t *testing.T) {
 	// Clean up
 	_ = deleteFromKeychain(testItemName)
 }
+
+func TestDeleteNonExistentKeychainItem(t *testing.T) {
+	// Skip test if not on macOS
+	if runtime.GOOS != "darwin" {
+		t.Skip("Keychain tests only run on macOS")
+	}
+
+	// Ensure item doesn't exist
+	testItemName := "test-nonexistent-keychain-item-12345"
+	_ = deleteFromKeychain(testItemName)
+
+	// Deleting a non-existent item should NOT return an error
+	// This is important for cleaning up unlocker directories when the keychain item
+	// has already been removed (e.g., manually by user, or on a different machine)
+	err := deleteFromKeychain(testItemName)
+	assert.NoError(t, err, "Deleting non-existent keychain item should not return an error")
+}