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") +}