Isolate tests from the developer's real login keychain (#1413)

* Isolate tests from the developer's real login keychain

Every test run prompted for the login keychain password (repeatedly,
since the xctest runner's code signature changes each build, so
"Always Allow" can never stick) and silently deleted the developer's
real Nostr identity via panic-mode tests.

Two holes let tests reach the real keychain:

- NostrIdentityBridge() defaulted to the real KeychainManager. Tests
  inject mocks, but app-side constructions with no injection point
  (LocationNotesManager's static bridge, GeohashPresenceService,
  BoardManager, AppRuntime) read the real chat.bitchat.nostr item
  when exercised under test.
- clearAllAssociations() used raw SecItem* calls that bypassed the
  injected keychain entirely, so panicClearAllData tests wiped the
  real Nostr identity items on every run.

Fix: centralize FavoritesPersistenceService's test-guarded in-memory
default as KeychainManager.makeDefault() and use it for all default
keychain parameters, and add deleteAll(service:) to
KeychainManagerProtocol so clearAllAssociations() goes through the
injected keychain like every other operation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Share one in-memory test keychain per process (Codex P2)

A fresh store per makeDefault() call diverges from production, where
separate default-constructed bridges share chat.bitchat.nostr —
BoardManager's publish and NIP-09 delete paths would derive different
geohash identities under test. PreviewKeychainManager gains a lock
since the shared instance is reached from arbitrary threads.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
jack
2026-07-08 17:20:00 +02:00
committed by GitHub
co-authored by jack Claude Fable 5
parent 4baeaab717
commit d499c3e415
9 changed files with 115 additions and 49 deletions
+5 -24
View File
@@ -15,7 +15,7 @@ final class NostrIdentityBridge {
private let keychain: KeychainManagerProtocol
init(keychain: KeychainManagerProtocol = KeychainManager()) {
init(keychain: KeychainManagerProtocol = KeychainManager.makeDefault()) {
self.keychain = keychain
}
@@ -49,29 +49,10 @@ final class NostrIdentityBridge {
/// Clear all Nostr identity associations and current identity
func clearAllAssociations() {
let query: [String: Any] = [
kSecClass as String: kSecClassGenericPassword,
kSecAttrService as String: keychainService,
kSecMatchLimit as String: kSecMatchLimitAll,
kSecReturnAttributes as String: true
]
var result: AnyObject?
let status = SecItemCopyMatching(query as CFDictionary, &result)
if status == errSecSuccess, let items = result as? [[String: Any]] {
for item in items {
var deleteQuery: [String: Any] = [
kSecClass as String: kSecClassGenericPassword,
kSecAttrService as String: keychainService
]
if let account = item[kSecAttrAccount as String] as? String {
deleteQuery[kSecAttrAccount as String] = account
}
SecItemDelete(deleteQuery as CFDictionary)
}
} else if status == errSecItemNotFound {
// nothing persisted; no action needed
}
// Must go through the injected keychain, not raw SecItem calls:
// under test that keychain is in-memory, and a direct delete here
// would wipe the developer's real Nostr identity on every test run.
keychain.deleteAll(service: keychainService)
deviceSeedCache = nil
// Also drop the in-memory derived per-geohash identities. These hold the