From 7e3c260b0bdf9ebbc425c4b17a32fb747a75e736 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 07:10:23 -0300 Subject: [PATCH 01/14] chore: create instalation marker utility --- Bitkit/Utilities/InstallationMarker.swift | 47 +++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 Bitkit/Utilities/InstallationMarker.swift diff --git a/Bitkit/Utilities/InstallationMarker.swift b/Bitkit/Utilities/InstallationMarker.swift new file mode 100644 index 00000000..92ee51cc --- /dev/null +++ b/Bitkit/Utilities/InstallationMarker.swift @@ -0,0 +1,47 @@ +import Foundation + +/// Utility to manage an installation marker file that helps detect orphaned keychain entries. +/// +/// The marker is placed in the app sandbox Documents directory (NOT the app group container) +/// because this directory is deleted when the app is uninstalled, while the keychain persists. +/// +/// By checking if the marker exists at app startup, we can detect if: +/// - The marker exists: App was installed before, keychain is valid +/// - The marker doesn't exist but keychain has data: Orphaned keychain from previous install +/// +/// This helps prevent security issues where a reinstalled app might find old keychain data +/// without corresponding wallet data (LDK, CoreDB, UserDefaults). +enum InstallationMarker { + private static let markerFileName = ".bitkit_installed" + + /// App sandbox Documents directory (NOT app group) - gets deleted on uninstall + private static var sandboxDocumentsUrl: URL { + FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] + } + + static var markerPath: URL { + sandboxDocumentsUrl.appendingPathComponent(markerFileName) + } + + /// Check if the installation marker exists + static func exists() -> Bool { + FileManager.default.fileExists(atPath: markerPath.path) + } + + /// Create the installation marker file + /// Should be called after handling any orphaned keychain detection + static func create() throws { + let data = UUID().uuidString.data(using: .utf8)! + try data.write(to: markerPath) + Logger.info("Installation marker created", context: "InstallationMarker") + } + + /// Delete the installation marker file + /// Should be called during app reset/wipe to ensure clean state on next install + static func delete() throws { + if exists() { + try FileManager.default.removeItem(at: markerPath) + Logger.info("Installation marker deleted", context: "InstallationMarker") + } + } +} From e747b79f08cbc77d43e89a2995cb577321973d86 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 07:11:33 -0300 Subject: [PATCH 02/14] fix: update keychain attributes to prevent from updating to the cloud --- Bitkit/Services/MigrationsService.swift | 20 ++++++++++++++++++++ Bitkit/Utilities/Keychain.swift | 3 ++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/Bitkit/Services/MigrationsService.swift b/Bitkit/Services/MigrationsService.swift index 51d02d8e..dcb7d332 100644 --- a/Bitkit/Services/MigrationsService.swift +++ b/Bitkit/Services/MigrationsService.swift @@ -451,6 +451,26 @@ extension MigrationsService { return exists } + /// Returns true if RN keychain has mnemonic but RN app files don't exist. + /// This indicates an orphaned keychain from a deleted RN app install. + func hasOrphanedRNKeychain() -> Bool { + // Check if RN keychain has mnemonic + guard hasRNWalletData() else { + return false + } + + // If keychain has mnemonic, check if RN app files exist + // RN app would have created MMKV or LDK files if it was actually used + let hasRNFiles = hasRNMmkvData() || hasRNLdkData() + + // Orphaned = has keychain data but no app files + let isOrphaned = !hasRNFiles + if isOrphaned { + Logger.warn("Detected orphaned RN keychain (mnemonic exists but no MMKV/LDK files)", context: "Migration") + } + return isOrphaned + } + func migrateFromReactNative(walletIndex: Int = 0) async throws { Logger.info("Starting RN migration", context: "Migration") diff --git a/Bitkit/Utilities/Keychain.swift b/Bitkit/Utilities/Keychain.swift index 6fd90cb6..65d820dd 100644 --- a/Bitkit/Utilities/Keychain.swift +++ b/Bitkit/Utilities/Keychain.swift @@ -28,7 +28,8 @@ class Keychain { let query = [ kSecClass as String: kSecClassGenericPassword as String, - kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock as String, + kSecAttrAccessible as String: kSecAttrAccessibleWhenUnlockedThisDeviceOnly as String, + kSecAttrSynchronizable as String: kCFBooleanFalse!, kSecAttrAccount as String: key.storageKey, kSecValueData as String: data, kSecAttrAccessGroup as String: Env.keychainGroup, From 4812373d71e93de271e1590ce26bf942c9fb8143 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 07:12:29 -0300 Subject: [PATCH 03/14] chore: add RN cleanup methods --- Bitkit/Services/MigrationsService.swift | 55 +++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/Bitkit/Services/MigrationsService.swift b/Bitkit/Services/MigrationsService.swift index dcb7d332..662ffe26 100644 --- a/Bitkit/Services/MigrationsService.swift +++ b/Bitkit/Services/MigrationsService.swift @@ -576,6 +576,61 @@ extension MigrationsService { func markMigrationChecked() { UserDefaults.standard.set(true, forKey: Self.rnMigrationCheckedKey) } + + // MARK: - RN Data Cleanup + + /// Delete RN keychain entries (mnemonic, passphrase, PIN) + func cleanupRNKeychain() { + let keysToDelete: [RNKeychainKey] = [ + .mnemonic(walletName: rnWalletName), + .passphrase(walletName: rnWalletName), + .pin, + ] + + for key in keysToDelete { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: key.service, + ] + let status = SecItemDelete(query as CFDictionary) + if status == noErr || status == errSecItemNotFound { + Logger.info("Deleted RN keychain key: \(key.service)", context: "Migration") + } else { + Logger.warn("Failed to delete RN keychain key \(key.service): \(status)", context: "Migration") + } + } + } + + /// Delete RN MMKV and LDK files from Documents directory + func cleanupRNFiles() { + // Delete MMKV directory + let mmkvDir = rnMmkvPath.deletingLastPathComponent() // ~/Documents/mmkv/ + if fileManager.fileExists(atPath: mmkvDir.path) { + do { + try fileManager.removeItem(at: mmkvDir) + Logger.info("Deleted RN MMKV directory", context: "Migration") + } catch { + Logger.warn("Failed to delete RN MMKV directory: \(error)", context: "Migration") + } + } + + // Delete LDK directory + if fileManager.fileExists(atPath: rnLdkBasePath.path) { + do { + try fileManager.removeItem(at: rnLdkBasePath) + Logger.info("Deleted RN LDK directory", context: "Migration") + } catch { + Logger.warn("Failed to delete RN LDK directory: \(error)", context: "Migration") + } + } + } + + /// Full cleanup after successful migration - removes all RN data + func cleanupAfterMigration() { + cleanupRNKeychain() + cleanupRNFiles() + Logger.info("RN cleanup completed", context: "Migration") + } } // MARK: - MMKV Data Migration From e46e5d0194040cac6b601f791d2a86558b241ade Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 07:13:28 -0300 Subject: [PATCH 04/14] fix: cleanup RN files after success migration --- Bitkit/Services/MigrationsService.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Bitkit/Services/MigrationsService.swift b/Bitkit/Services/MigrationsService.swift index 662ffe26..3f14b110 100644 --- a/Bitkit/Services/MigrationsService.swift +++ b/Bitkit/Services/MigrationsService.swift @@ -491,6 +491,10 @@ extension MigrationsService { UserDefaults.standard.set(true, forKey: Self.rnMigrationCompletedKey) UserDefaults.standard.set(true, forKey: Self.rnMigrationCheckedKey) + + // Clean up RN data after successful migration + cleanupAfterMigration() + Logger.info("RN migration completed", context: "Migration") } From 333ff18b6877b8df132e2d540a66d9a7e63a0a49 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 07:16:28 -0300 Subject: [PATCH 05/14] fix: implement orphaned keychain check --- Bitkit/AppScene.swift | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift index 02344467..d904d574 100644 --- a/Bitkit/AppScene.swift +++ b/Bitkit/AppScene.swift @@ -307,9 +307,45 @@ struct AppScene: View { } } + /// Handle orphaned keychain entries from previous app installs. + /// If the installation marker doesn't exist but keychain has data, the app was reinstalled + /// and the keychain data is orphaned (corresponding wallet data was deleted with the app). + private func handleOrphanedKeychain() { + // If marker exists, app was installed before - keychain is valid + if InstallationMarker.exists() { + Logger.debug("Installation marker exists, skipping orphaned keychain check", context: "AppScene") + return + } + + // Check if native keychain has data (orphaned from previous install) + let hasNativeKeychain = (try? Keychain.exists(key: .bip39Mnemonic(index: 0))) == true + + // Check if RN keychain has data without corresponding RN files (orphaned) + let hasOrphanedRNKeychain = MigrationsService.shared.hasOrphanedRNKeychain() + + if hasNativeKeychain || hasOrphanedRNKeychain { + Logger.warn("Orphaned keychain detected, wiping", context: "AppScene") + try? Keychain.wipeEntireKeychain() + + if hasOrphanedRNKeychain { + MigrationsService.shared.cleanupRNKeychain() + } + } + + // Create marker for this installation + do { + try InstallationMarker.create() + } catch { + Logger.error("Failed to create installation marker: \(error)", context: "AppScene") + } + } + @Sendable private func setupTask() async { do { + // Handle orphaned keychain before anything else + handleOrphanedKeychain() + await checkAndPerformRNMigration() try wallet.setWalletExistsState() @@ -340,8 +376,9 @@ struct AppScene: View { return } - guard migrations.hasRNWalletData() else { - Logger.info("No RN wallet data found, skipping migration", context: "AppScene") + // Check if RN wallet data exists AND is not orphaned (has corresponding files) + guard migrations.hasRNWalletData(), !migrations.hasOrphanedRNKeychain() else { + Logger.info("No valid RN wallet data found, skipping migration", context: "AppScene") migrations.markMigrationChecked() return } From d3f8f8866cd2369c287c9af6b1ab92b88d64996b Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 07:17:48 -0300 Subject: [PATCH 06/14] fix: delete marker on app reset --- Bitkit/Utilities/AppReset.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Bitkit/Utilities/AppReset.swift b/Bitkit/Utilities/AppReset.swift index d76fe6ed..8ee38e7c 100644 --- a/Bitkit/Utilities/AppReset.swift +++ b/Bitkit/Utilities/AppReset.swift @@ -29,6 +29,9 @@ enum AppReset { // Wipe keychain try Keychain.wipeEntireKeychain() + // Delete installation marker so next install can detect orphaned keychain + try? InstallationMarker.delete() + // Wipe user defaults if let bundleID = Bundle.main.bundleIdentifier { UserDefaults.standard.removePersistentDomain(forName: bundleID) From 850d03caf2244ebe604ff3cf58235d9176320eef Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 09:48:36 -0300 Subject: [PATCH 07/14] test: creation marker tests --- BitkitTests/InstallationMarkerTests.swift | 88 +++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 BitkitTests/InstallationMarkerTests.swift diff --git a/BitkitTests/InstallationMarkerTests.swift b/BitkitTests/InstallationMarkerTests.swift new file mode 100644 index 00000000..c1882b07 --- /dev/null +++ b/BitkitTests/InstallationMarkerTests.swift @@ -0,0 +1,88 @@ +import XCTest + +final class InstallationMarkerTests: XCTestCase { + override func setUp() { + super.setUp() + // Clean up before each test + try? InstallationMarker.delete() + } + + override func tearDown() { + // Clean up after each test + try? InstallationMarker.delete() + super.tearDown() + } + + func testMarkerDoesNotExistInitially() { + // After cleanup in setUp, marker should not exist + XCTAssertFalse(InstallationMarker.exists()) + } + + func testCreateMarker() throws { + // Initially should not exist + XCTAssertFalse(InstallationMarker.exists()) + + // Create the marker + try InstallationMarker.create() + + // Now should exist + XCTAssertTrue(InstallationMarker.exists()) + + // Verify file actually exists at the expected path + XCTAssertTrue(FileManager.default.fileExists(atPath: InstallationMarker.markerPath.path)) + } + + func testDeleteMarker() throws { + // Create the marker first + try InstallationMarker.create() + XCTAssertTrue(InstallationMarker.exists()) + + // Delete it + try InstallationMarker.delete() + + // Should no longer exist + XCTAssertFalse(InstallationMarker.exists()) + XCTAssertFalse(FileManager.default.fileExists(atPath: InstallationMarker.markerPath.path)) + } + + func testDeleteNonExistentMarkerDoesNotThrow() { + // Ensure marker doesn't exist + XCTAssertFalse(InstallationMarker.exists()) + + // Deleting non-existent marker should not throw + XCTAssertNoThrow(try InstallationMarker.delete()) + } + + func testMarkerPathUsesSandboxDocuments() { + // Get the expected sandbox Documents directory + let sandboxDocuments = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] + + // Verify marker path is within sandbox Documents, not app group + XCTAssertTrue(InstallationMarker.markerPath.path.hasPrefix(sandboxDocuments.path)) + + // Verify it's NOT using the app group container + // App group would contain "group.bitkit" in the path + XCTAssertFalse(InstallationMarker.markerPath.path.contains("group.bitkit")) + } + + func testCreateMarkerIsIdempotent() throws { + // Create the marker + try InstallationMarker.create() + XCTAssertTrue(InstallationMarker.exists()) + + // Creating again should overwrite without error + // (the file content is a new UUID each time, but that's fine) + try InstallationMarker.create() + XCTAssertTrue(InstallationMarker.exists()) + } + + func testMarkerPersistsAcrossChecks() throws { + // Create the marker + try InstallationMarker.create() + + // Multiple checks should all return true + XCTAssertTrue(InstallationMarker.exists()) + XCTAssertTrue(InstallationMarker.exists()) + XCTAssertTrue(InstallationMarker.exists()) + } +} From 82f22a1068be4f17a95c4f521b5674538e42acc8 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 09:52:18 -0300 Subject: [PATCH 08/14] test: orphaned keychain tests --- BitkitTests/OrphanedKeychainTests.swift | 138 ++++++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 BitkitTests/OrphanedKeychainTests.swift diff --git a/BitkitTests/OrphanedKeychainTests.swift b/BitkitTests/OrphanedKeychainTests.swift new file mode 100644 index 00000000..913d3ad0 --- /dev/null +++ b/BitkitTests/OrphanedKeychainTests.swift @@ -0,0 +1,138 @@ +import XCTest + +/// Tests for orphaned keychain detection and handling. +/// These tests verify that the app correctly identifies and handles keychain data +/// that persists after app uninstall/reinstall. +final class OrphanedKeychainTests: XCTestCase { + override func setUp() { + super.setUp() + // Clean state before each test + try? Keychain.wipeEntireKeychain() + try? InstallationMarker.delete() + } + + override func tearDown() { + // Clean state after each test + try? Keychain.wipeEntireKeychain() + try? InstallationMarker.delete() + super.tearDown() + } + + // MARK: - Installation Marker Tests + + func testFreshInstallScenario() { + // Scenario: Fresh install - no marker, no keychain data + + // Verify initial state + XCTAssertFalse(InstallationMarker.exists()) + XCTAssertFalse((try? Keychain.exists(key: .bip39Mnemonic(index: 0))) ?? false) + + // This is what handleOrphanedKeychain() should do: + // 1. No orphaned data to clean + // 2. Create the marker + + // After handling, marker should be created + XCTAssertNoThrow(try InstallationMarker.create()) + XCTAssertTrue(InstallationMarker.exists()) + } + + func testNormalAppLaunchWithMarker() throws { + // Scenario: Normal app launch - marker exists, keychain has valid data + + // Setup: Create marker and keychain data (simulating previous successful install) + try InstallationMarker.create() + try Keychain.saveString(key: .bip39Mnemonic(index: 0), str: "test mnemonic words") + + // Verify state + XCTAssertTrue(InstallationMarker.exists()) + XCTAssertTrue(try Keychain.exists(key: .bip39Mnemonic(index: 0))) + + // When marker exists, keychain should NOT be wiped + // (handleOrphanedKeychain returns early when marker exists) + + // Keychain data should still be there + XCTAssertEqual(try Keychain.loadString(key: .bip39Mnemonic(index: 0)), "test mnemonic words") + } + + func testOrphanedNativeKeychainDetection() throws { + // Scenario: Reinstall - no marker, but keychain has mnemonic (orphaned) + + // Setup: Keychain has data but no marker (simulates reinstall after uninstall) + try Keychain.saveString(key: .bip39Mnemonic(index: 0), str: "orphaned mnemonic") + XCTAssertFalse(InstallationMarker.exists()) + + // Detect orphaned state + let hasNativeKeychain = (try? Keychain.exists(key: .bip39Mnemonic(index: 0))) == true + XCTAssertTrue(hasNativeKeychain) + + // This is orphaned: keychain exists but marker doesn't + // handleOrphanedKeychain() should wipe the keychain + + try Keychain.wipeEntireKeychain() + + // After wipe, keychain should be empty + XCTAssertFalse((try? Keychain.exists(key: .bip39Mnemonic(index: 0))) ?? false) + + // Then marker should be created + try InstallationMarker.create() + XCTAssertTrue(InstallationMarker.exists()) + } + + func testMultipleOrphanedKeysAreAllWiped() throws { + // Scenario: Multiple keychain entries exist without marker + + // Setup: Multiple keychain entries + try Keychain.saveString(key: .bip39Mnemonic(index: 0), str: "mnemonic 0") + try Keychain.saveString(key: .bip39Passphrase(index: 0), str: "passphrase 0") + try Keychain.saveString(key: .bip39Mnemonic(index: 1), str: "mnemonic 1") + try Keychain.saveString(key: .securityPin, str: "123456") + + // Verify all keys exist + XCTAssertTrue(try Keychain.exists(key: .bip39Mnemonic(index: 0))) + XCTAssertTrue(try Keychain.exists(key: .bip39Passphrase(index: 0))) + XCTAssertTrue(try Keychain.exists(key: .bip39Mnemonic(index: 1))) + XCTAssertTrue(try Keychain.exists(key: .securityPin)) + + // Wipe entire keychain (what handleOrphanedKeychain does) + try Keychain.wipeEntireKeychain() + + // All keys should be gone + XCTAssertFalse((try? Keychain.exists(key: .bip39Mnemonic(index: 0))) ?? false) + XCTAssertFalse((try? Keychain.exists(key: .bip39Passphrase(index: 0))) ?? false) + XCTAssertFalse((try? Keychain.exists(key: .bip39Mnemonic(index: 1))) ?? false) + XCTAssertFalse((try? Keychain.exists(key: .securityPin)) ?? false) + } + + func testOrphanedKeychainWithPassphraseOnly() throws { + // Edge case: Only passphrase exists (no mnemonic) - still orphaned + + try Keychain.saveString(key: .bip39Passphrase(index: 0), str: "orphaned passphrase") + + // Detection should still work via wipeEntireKeychain checking all keys + let allKeys = Keychain.getAllKeyChainStorageKeys() + XCTAssertFalse(allKeys.isEmpty) + + // Wipe should clean it + try Keychain.wipeEntireKeychain() + XCTAssertTrue(Keychain.getAllKeyChainStorageKeys().isEmpty) + } + + func testAppResetDeletesMarker() throws { + // Scenario: App reset should delete marker so next launch detects as fresh + + // Setup: Normal state with marker and keychain + try InstallationMarker.create() + try Keychain.saveString(key: .bip39Mnemonic(index: 0), str: "test mnemonic") + + XCTAssertTrue(InstallationMarker.exists()) + XCTAssertTrue(try Keychain.exists(key: .bip39Mnemonic(index: 0))) + + // Simulate app reset: wipe keychain and delete marker + try Keychain.wipeEntireKeychain() + try InstallationMarker.delete() + + // After reset, both should be gone + XCTAssertFalse(InstallationMarker.exists()) + XCTAssertFalse((try? Keychain.exists(key: .bip39Mnemonic(index: 0))) ?? false) + } +} From 9f1fb0ab651d61f6e7aafc593d3c24c412cf0270 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 09:53:47 -0300 Subject: [PATCH 09/14] test: update keychain tests --- BitkitTests/KeychainTests.swift | 85 ++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/BitkitTests/KeychainTests.swift b/BitkitTests/KeychainTests.swift index 86bdaede..7f70241c 100644 --- a/BitkitTests/KeychainTests.swift +++ b/BitkitTests/KeychainTests.swift @@ -1,3 +1,4 @@ +import Security import XCTest final class KeychainTests: XCTestCase { @@ -6,9 +7,91 @@ final class KeychainTests: XCTestCase { } override func tearDownWithError() throws { - // Put teardown code here. This method is called after the invocation of each test method in the class. + try Keychain.wipeEntireKeychain() + } + + // MARK: - Security Attribute Tests + + func testKeychainSecurityAttributes() throws { + // Save a test item + let testValue = "test_mnemonic_for_security_check" + try Keychain.saveString(key: .bip39Mnemonic(index: 0), str: testValue) + + // Query the item with attributes returned + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: KeychainEntryType.bip39Mnemonic(index: 0).storageKey, + kSecAttrAccessGroup as String: Env.keychainGroup, + kSecReturnAttributes as String: true, + kSecReturnData as String: false, + ] + + var result: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &result) + + XCTAssertEqual(status, errSecSuccess, "Failed to query keychain item") + + guard let attributes = result as? [String: Any] else { + XCTFail("Failed to get keychain attributes") + return + } + + // Verify accessibility is set to WhenUnlockedThisDeviceOnly + if let accessible = attributes[kSecAttrAccessible as String] as? String { + XCTAssertEqual( + accessible, + kSecAttrAccessibleWhenUnlockedThisDeviceOnly as String, + "Keychain item should use kSecAttrAccessibleWhenUnlockedThisDeviceOnly" + ) + } else { + XCTFail("Could not read kSecAttrAccessible attribute") + } + + // Verify synchronizable is false (item should not sync to iCloud) + if let synchronizable = attributes[kSecAttrSynchronizable as String] as? Bool { + XCTAssertFalse( + synchronizable, + "Keychain item should not be synchronizable (kSecAttrSynchronizable should be false)" + ) + } + // Note: If synchronizable is nil/missing, that's also acceptable as the default is false + } + + func testKeychainItemsAreDeviceOnly() throws { + // This test verifies that keychain items cannot sync to other devices + // by checking the accessibility attribute + + try Keychain.saveString(key: .securityPin, str: "123456") + + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: KeychainEntryType.securityPin.storageKey, + kSecAttrAccessGroup as String: Env.keychainGroup, + kSecReturnAttributes as String: true, + ] + + var result: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &result) + + XCTAssertEqual(status, errSecSuccess) + + guard let attributes = result as? [String: Any] else { + XCTFail("Failed to get keychain attributes") + return + } + + // The "ThisDeviceOnly" suffix means the item won't migrate to a new device + if let accessible = attributes[kSecAttrAccessible as String] as? String { + XCTAssertTrue( + accessible.contains("ThisDeviceOnly") || + accessible == (kSecAttrAccessibleWhenUnlockedThisDeviceOnly as String), + "Keychain items should be device-only (not transferable to other devices)" + ) + } } + // MARK: - Basic Keychain Tests + func testKeychain() throws { let testMnemonic = "test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999)) test\(Int.random(in: 0 ... 99999))" From 4027c12775b77bf745a1aff9359e2a439ab05046 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 09:57:57 -0300 Subject: [PATCH 10/14] test: RN migration cleanup tests --- BitkitTests/RNMigrationCleanupTests.swift | 234 ++++++++++++++++++++++ 1 file changed, 234 insertions(+) create mode 100644 BitkitTests/RNMigrationCleanupTests.swift diff --git a/BitkitTests/RNMigrationCleanupTests.swift b/BitkitTests/RNMigrationCleanupTests.swift new file mode 100644 index 00000000..c7db2c13 --- /dev/null +++ b/BitkitTests/RNMigrationCleanupTests.swift @@ -0,0 +1,234 @@ +import Security +import XCTest + +/// Tests for RN (React Native) migration cleanup functionality. +/// These tests verify the detection of orphaned RN keychain data and proper cleanup +/// of RN data after migration. +final class RNMigrationCleanupTests: XCTestCase { + private let migrations = MigrationsService.shared + private let fileManager = FileManager.default + + // RN wallet name used by the migration service + private let rnWalletName = "wallet0" + + // Sandbox Documents directory (where RN stored its data) + private var sandboxDocuments: URL { + FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] + } + + private var rnMmkvPath: URL { + sandboxDocuments.appendingPathComponent("mmkv") + } + + private var rnLdkPath: URL { + sandboxDocuments.appendingPathComponent("ldk") + } + + override func setUp() { + super.setUp() + // Clean up any existing test data + cleanupRNTestData() + try? Keychain.wipeEntireKeychain() + } + + override func tearDown() { + // Clean up after tests + cleanupRNTestData() + try? Keychain.wipeEntireKeychain() + super.tearDown() + } + + // MARK: - Helper Methods + + private func cleanupRNTestData() { + // Clean up RN keychain entries + migrations.cleanupRNKeychain() + + // Clean up RN files + try? fileManager.removeItem(at: rnMmkvPath) + try? fileManager.removeItem(at: rnLdkPath) + } + + private func createRNKeychainEntry(walletName: String, mnemonic: String) { + // Simulate RN keychain storage format + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: walletName, + kSecAttrAccount as String: walletName, + kSecValueData as String: mnemonic.data(using: .utf8)!, + ] + SecItemAdd(query as CFDictionary, nil) + } + + private func createRNMmkvDirectory() throws { + // Create MMKV directory structure as RN would + let mmkvDefaultPath = rnMmkvPath.appendingPathComponent("mmkv.default") + try fileManager.createDirectory(at: rnMmkvPath, withIntermediateDirectories: true) + // Create a dummy mmkv.default file + try "dummy_mmkv_data".data(using: .utf8)!.write(to: mmkvDefaultPath) + } + + private func createRNLdkDirectory() throws { + // Create LDK directory structure as RN would + let accountPath = rnLdkPath.appendingPathComponent("\(rnWalletName)bitcoinRegtestldkaccountv3") + try fileManager.createDirectory(at: accountPath, withIntermediateDirectories: true) + // Create a dummy channel_manager.bin file + let channelManagerPath = accountPath.appendingPathComponent("channel_manager.bin") + try "dummy_channel_manager".data(using: .utf8)!.write(to: channelManagerPath) + } + + // MARK: - Orphaned RN Keychain Detection Tests + + func testHasOrphanedRNKeychainReturnsFalseWhenNoData() { + // No RN keychain data, no RN files + XCTAssertFalse(migrations.hasOrphanedRNKeychain()) + } + + func testHasOrphanedRNKeychainReturnsTrueWhenOnlyKeychain() { + // Setup: RN keychain exists but no MMKV or LDK files + createRNKeychainEntry(walletName: rnWalletName, mnemonic: "test mnemonic words here") + + // Should detect as orphaned (keychain without files) + XCTAssertTrue(migrations.hasOrphanedRNKeychain()) + } + + func testHasOrphanedRNKeychainReturnsFalseWhenMmkvExists() throws { + // Setup: RN keychain AND MMKV files exist (valid RN data) + createRNKeychainEntry(walletName: rnWalletName, mnemonic: "test mnemonic words here") + try createRNMmkvDirectory() + + // Should NOT be orphaned (has corresponding files) + XCTAssertFalse(migrations.hasOrphanedRNKeychain()) + } + + func testHasOrphanedRNKeychainReturnsFalseWhenLdkExists() throws { + // Setup: RN keychain AND LDK files exist (valid RN data) + createRNKeychainEntry(walletName: rnWalletName, mnemonic: "test mnemonic words here") + try createRNLdkDirectory() + + // Should NOT be orphaned (has corresponding files) + XCTAssertFalse(migrations.hasOrphanedRNKeychain()) + } + + func testHasOrphanedRNKeychainReturnsFalseWhenBothFilesExist() throws { + // Setup: RN keychain AND both MMKV and LDK files exist + createRNKeychainEntry(walletName: rnWalletName, mnemonic: "test mnemonic words here") + try createRNMmkvDirectory() + try createRNLdkDirectory() + + // Should NOT be orphaned (has corresponding files) + XCTAssertFalse(migrations.hasOrphanedRNKeychain()) + } + + // MARK: - RN Keychain Cleanup Tests + + func testCleanupRNKeychainDeletesEntries() { + // Setup: Create RN keychain entries + createRNKeychainEntry(walletName: rnWalletName, mnemonic: "test mnemonic") + + // Also create passphrase entry + let passphraseQuery: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: "\(rnWalletName)passphrase", + kSecAttrAccount as String: "\(rnWalletName)passphrase", + kSecValueData as String: "test passphrase".data(using: .utf8)!, + ] + SecItemAdd(passphraseQuery as CFDictionary, nil) + + // Verify entries exist + XCTAssertTrue(migrations.hasRNWalletData()) + + // Clean up + migrations.cleanupRNKeychain() + + // Verify entries are deleted + XCTAssertFalse(migrations.hasRNWalletData()) + } + + func testCleanupRNKeychainHandlesMissingEntries() { + // No RN keychain entries exist + XCTAssertFalse(migrations.hasRNWalletData()) + + // Cleanup should not throw even with no entries + migrations.cleanupRNKeychain() + + // Still no data (no crash) + XCTAssertFalse(migrations.hasRNWalletData()) + } + + // MARK: - RN Files Cleanup Tests + + func testCleanupRNFilesDeletesMmkvDirectory() throws { + // Setup: Create MMKV directory + try createRNMmkvDirectory() + XCTAssertTrue(fileManager.fileExists(atPath: rnMmkvPath.path)) + + // Clean up + migrations.cleanupRNFiles() + + // MMKV directory should be deleted + XCTAssertFalse(fileManager.fileExists(atPath: rnMmkvPath.path)) + } + + func testCleanupRNFilesDeletesLdkDirectory() throws { + // Setup: Create LDK directory + try createRNLdkDirectory() + XCTAssertTrue(fileManager.fileExists(atPath: rnLdkPath.path)) + + // Clean up + migrations.cleanupRNFiles() + + // LDK directory should be deleted + XCTAssertFalse(fileManager.fileExists(atPath: rnLdkPath.path)) + } + + func testCleanupRNFilesDeletesBothDirectories() throws { + // Setup: Create both directories + try createRNMmkvDirectory() + try createRNLdkDirectory() + XCTAssertTrue(fileManager.fileExists(atPath: rnMmkvPath.path)) + XCTAssertTrue(fileManager.fileExists(atPath: rnLdkPath.path)) + + // Clean up + migrations.cleanupRNFiles() + + // Both directories should be deleted + XCTAssertFalse(fileManager.fileExists(atPath: rnMmkvPath.path)) + XCTAssertFalse(fileManager.fileExists(atPath: rnLdkPath.path)) + } + + func testCleanupRNFilesHandlesMissingDirectories() { + // No directories exist + XCTAssertFalse(fileManager.fileExists(atPath: rnMmkvPath.path)) + XCTAssertFalse(fileManager.fileExists(atPath: rnLdkPath.path)) + + // Cleanup should not throw + migrations.cleanupRNFiles() + + // Still no directories (no crash) + XCTAssertFalse(fileManager.fileExists(atPath: rnMmkvPath.path)) + XCTAssertFalse(fileManager.fileExists(atPath: rnLdkPath.path)) + } + + // MARK: - Full Cleanup Tests + + func testCleanupAfterMigrationDeletesEverything() throws { + // Setup: Create all RN data + createRNKeychainEntry(walletName: rnWalletName, mnemonic: "test mnemonic") + try createRNMmkvDirectory() + try createRNLdkDirectory() + + // Verify data exists + XCTAssertTrue(migrations.hasRNWalletData()) + XCTAssertTrue(fileManager.fileExists(atPath: rnMmkvPath.path)) + XCTAssertTrue(fileManager.fileExists(atPath: rnLdkPath.path)) + + // Full cleanup + migrations.cleanupAfterMigration() + + // Everything should be deleted + XCTAssertFalse(migrations.hasRNWalletData()) + XCTAssertFalse(fileManager.fileExists(atPath: rnMmkvPath.path)) + XCTAssertFalse(fileManager.fileExists(atPath: rnLdkPath.path)) + } +} From d503eb393c7769f0631a18f8d3f0871a73f72ae8 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 10:03:49 -0300 Subject: [PATCH 11/14] test: test isolation --- BitkitTests/InstallationMarkerTests.swift | 1 + BitkitTests/KeychainTests.swift | 18 +++++++++++++----- BitkitTests/OrphanedKeychainTests.swift | 8 ++++---- BitkitTests/RNMigrationCleanupTests.swift | 1 + 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/BitkitTests/InstallationMarkerTests.swift b/BitkitTests/InstallationMarkerTests.swift index c1882b07..248b8a20 100644 --- a/BitkitTests/InstallationMarkerTests.swift +++ b/BitkitTests/InstallationMarkerTests.swift @@ -1,3 +1,4 @@ +@testable import Bitkit import XCTest final class InstallationMarkerTests: XCTestCase { diff --git a/BitkitTests/KeychainTests.swift b/BitkitTests/KeychainTests.swift index 7f70241c..554f5441 100644 --- a/BitkitTests/KeychainTests.swift +++ b/BitkitTests/KeychainTests.swift @@ -1,3 +1,4 @@ +@testable import Bitkit import Security import XCTest @@ -119,9 +120,14 @@ final class KeychainTests: XCTestCase { try Keychain.saveString(key: .bip39Passphrase(index: i), str: "\(testPassphrase) index\(i)") } - // Check all keys are saved correctly + // Check all keys are saved correctly (verify each key exists) + for i in 0 ... 5 { + XCTAssertTrue(try Keychain.exists(key: .bip39Mnemonic(index: i))) + XCTAssertTrue(try Keychain.exists(key: .bip39Passphrase(index: i))) + } + + // Also verify they appear in listed keys let listedKeys = Keychain.getAllKeyChainStorageKeys() - XCTAssertEqual(listedKeys.count, 12) for i in 0 ... 5 { XCTAssertTrue(listedKeys.contains("bip39_mnemonic_\(i)")) XCTAssertTrue(listedKeys.contains("bip39_passphrase_\(i)")) @@ -136,8 +142,10 @@ final class KeychainTests: XCTestCase { // Wipe try Keychain.wipeEntireKeychain() - // Check all keys are gone - let listedKeysAfterWipe = Keychain.getAllKeyChainStorageKeys() - XCTAssertEqual(listedKeysAfterWipe.count, 0) + // Check all our keys are gone (using exists() which respects access group) + for i in 0 ... 5 { + XCTAssertFalse((try? Keychain.exists(key: .bip39Mnemonic(index: i))) ?? false) + XCTAssertFalse((try? Keychain.exists(key: .bip39Passphrase(index: i))) ?? false) + } } } diff --git a/BitkitTests/OrphanedKeychainTests.swift b/BitkitTests/OrphanedKeychainTests.swift index 913d3ad0..887aa29a 100644 --- a/BitkitTests/OrphanedKeychainTests.swift +++ b/BitkitTests/OrphanedKeychainTests.swift @@ -1,3 +1,4 @@ +@testable import Bitkit import XCTest /// Tests for orphaned keychain detection and handling. @@ -108,13 +109,12 @@ final class OrphanedKeychainTests: XCTestCase { try Keychain.saveString(key: .bip39Passphrase(index: 0), str: "orphaned passphrase") - // Detection should still work via wipeEntireKeychain checking all keys - let allKeys = Keychain.getAllKeyChainStorageKeys() - XCTAssertFalse(allKeys.isEmpty) + // Verify passphrase was saved + XCTAssertTrue(try Keychain.exists(key: .bip39Passphrase(index: 0))) // Wipe should clean it try Keychain.wipeEntireKeychain() - XCTAssertTrue(Keychain.getAllKeyChainStorageKeys().isEmpty) + XCTAssertFalse((try? Keychain.exists(key: .bip39Passphrase(index: 0))) ?? false) } func testAppResetDeletesMarker() throws { diff --git a/BitkitTests/RNMigrationCleanupTests.swift b/BitkitTests/RNMigrationCleanupTests.swift index c7db2c13..cf67a1aa 100644 --- a/BitkitTests/RNMigrationCleanupTests.swift +++ b/BitkitTests/RNMigrationCleanupTests.swift @@ -1,3 +1,4 @@ +@testable import Bitkit import Security import XCTest From e0408538e497eceb586573a70150c68a761749ce Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 10:28:53 -0300 Subject: [PATCH 12/14] chore: remove unwrap focing --- Bitkit/Utilities/Keychain.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Bitkit/Utilities/Keychain.swift b/Bitkit/Utilities/Keychain.swift index 65d820dd..b1f7287f 100644 --- a/Bitkit/Utilities/Keychain.swift +++ b/Bitkit/Utilities/Keychain.swift @@ -29,7 +29,7 @@ class Keychain { [ kSecClass as String: kSecClassGenericPassword as String, kSecAttrAccessible as String: kSecAttrAccessibleWhenUnlockedThisDeviceOnly as String, - kSecAttrSynchronizable as String: kCFBooleanFalse!, + kSecAttrSynchronizable as String: false, kSecAttrAccount as String: key.storageKey, kSecValueData as String: data, kSecAttrAccessGroup as String: Env.keychainGroup, From f621dc4c2ebfbdcf41d206143a3efd7f1758689e Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 10:34:47 -0300 Subject: [PATCH 13/14] test: remove redundtant check --- BitkitTests/KeychainTests.swift | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/BitkitTests/KeychainTests.swift b/BitkitTests/KeychainTests.swift index 554f5441..a66dbdfb 100644 --- a/BitkitTests/KeychainTests.swift +++ b/BitkitTests/KeychainTests.swift @@ -120,20 +120,7 @@ final class KeychainTests: XCTestCase { try Keychain.saveString(key: .bip39Passphrase(index: i), str: "\(testPassphrase) index\(i)") } - // Check all keys are saved correctly (verify each key exists) - for i in 0 ... 5 { - XCTAssertTrue(try Keychain.exists(key: .bip39Mnemonic(index: i))) - XCTAssertTrue(try Keychain.exists(key: .bip39Passphrase(index: i))) - } - - // Also verify they appear in listed keys - let listedKeys = Keychain.getAllKeyChainStorageKeys() - for i in 0 ... 5 { - XCTAssertTrue(listedKeys.contains("bip39_mnemonic_\(i)")) - XCTAssertTrue(listedKeys.contains("bip39_passphrase_\(i)")) - } - - // Check each value + // Check all keys are saved correctly by verifying their values for i in 0 ... 5 { XCTAssertEqual(try Keychain.loadString(key: .bip39Mnemonic(index: i)), "\(testMnemonic) index\(i)") XCTAssertEqual(try Keychain.loadString(key: .bip39Passphrase(index: i)), "\(testPassphrase) index\(i)") From aba6816c23c6d4571995eff1af66f3227b83fac3 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 13 Jan 2026 15:09:42 -0300 Subject: [PATCH 14/14] fix: change accessibility level to notification extension --- Bitkit/Utilities/Keychain.swift | 17 ++++++++++++- BitkitTests/KeychainTests.swift | 43 ++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/Bitkit/Utilities/Keychain.swift b/Bitkit/Utilities/Keychain.swift index b1f7287f..2f69d3a6 100644 --- a/Bitkit/Utilities/Keychain.swift +++ b/Bitkit/Utilities/Keychain.swift @@ -19,6 +19,21 @@ enum KeychainEntryType { return "security_pin" } } + + /// Returns the appropriate keychain accessibility level for this key type. + /// - pushNotificationPrivateKey: Uses AfterFirstUnlock because the notification service extension + /// needs to decrypt push payloads even when the device is locked. + /// - All other keys (mnemonic, passphrase, PIN): Use WhenUnlockedThisDeviceOnly for maximum security. + var accessibility: CFString { + switch self { + case .pushNotificationPrivateKey: + // Must be accessible when device is locked for notification extension to decrypt payloads + return kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly + case .bip39Mnemonic, .bip39Passphrase, .securityPin: + // Maximum security: only accessible when device is unlocked + return kSecAttrAccessibleWhenUnlockedThisDeviceOnly + } + } } class Keychain { @@ -28,7 +43,7 @@ class Keychain { let query = [ kSecClass as String: kSecClassGenericPassword as String, - kSecAttrAccessible as String: kSecAttrAccessibleWhenUnlockedThisDeviceOnly as String, + kSecAttrAccessible as String: key.accessibility as String, kSecAttrSynchronizable as String: false, kSecAttrAccount as String: key.storageKey, kSecValueData as String: data, diff --git a/BitkitTests/KeychainTests.swift b/BitkitTests/KeychainTests.swift index a66dbdfb..e3e6dab5 100644 --- a/BitkitTests/KeychainTests.swift +++ b/BitkitTests/KeychainTests.swift @@ -14,7 +14,7 @@ final class KeychainTests: XCTestCase { // MARK: - Security Attribute Tests func testKeychainSecurityAttributes() throws { - // Save a test item + // Save a test item (mnemonic should use WhenUnlockedThisDeviceOnly) let testValue = "test_mnemonic_for_security_check" try Keychain.saveString(key: .bip39Mnemonic(index: 0), str: testValue) @@ -37,12 +37,12 @@ final class KeychainTests: XCTestCase { return } - // Verify accessibility is set to WhenUnlockedThisDeviceOnly + // Verify accessibility is set to WhenUnlockedThisDeviceOnly for mnemonic if let accessible = attributes[kSecAttrAccessible as String] as? String { XCTAssertEqual( accessible, kSecAttrAccessibleWhenUnlockedThisDeviceOnly as String, - "Keychain item should use kSecAttrAccessibleWhenUnlockedThisDeviceOnly" + "Mnemonic should use kSecAttrAccessibleWhenUnlockedThisDeviceOnly for maximum security" ) } else { XCTFail("Could not read kSecAttrAccessible attribute") @@ -58,6 +58,43 @@ final class KeychainTests: XCTestCase { // Note: If synchronizable is nil/missing, that's also acceptable as the default is false } + func testPushNotificationKeyAccessibility() throws { + // Save a push notification private key (should use AfterFirstUnlockThisDeviceOnly) + let testKey = Data([0x01, 0x02, 0x03, 0x04]) + try Keychain.save(key: .pushNotificationPrivateKey, data: testKey) + + // Query the item with attributes returned + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: KeychainEntryType.pushNotificationPrivateKey.storageKey, + kSecAttrAccessGroup as String: Env.keychainGroup, + kSecReturnAttributes as String: true, + kSecReturnData as String: false, + ] + + var result: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &result) + + XCTAssertEqual(status, errSecSuccess, "Failed to query keychain item") + + guard let attributes = result as? [String: Any] else { + XCTFail("Failed to get keychain attributes") + return + } + + // Verify accessibility is set to AfterFirstUnlockThisDeviceOnly for push notification key + // This allows the notification service extension to decrypt payloads when device is locked + if let accessible = attributes[kSecAttrAccessible as String] as? String { + XCTAssertEqual( + accessible, + kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly as String, + "Push notification key should use kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly for background access" + ) + } else { + XCTFail("Could not read kSecAttrAccessible attribute") + } + } + func testKeychainItemsAreDeviceOnly() throws { // This test verifies that keychain items cannot sync to other devices // by checking the accessibility attribute