From 25a3219a536108c9fc7499390f3337b34fb5f0f2 Mon Sep 17 00:00:00 2001 From: Taksh Kothari Date: Thu, 24 Sep 2026 13:04:04 +0530 Subject: [PATCH] fix: refuse an ambiguous nickname instead of picking the first peer (#1689) getPeerID(for:) and geohash geoNicknames.first(where:) returned the first match, so /msg and /block could hit the wrong person when two peers share a name. Accept an unsuffixed nick only when it is unique; otherwise require the people-list #xxxx suffix. --- bitchat/Services/UnifiedPeerService.swift | 20 +++++++----- bitchat/Utils/NicknameLookup.swift | 29 +++++++++++++++++ .../ChatPeerIdentityCoordinator.swift | 21 ++++++++---- ...tPeerIdentityCoordinatorContextTests.swift | 21 ++++++++++++ .../Services/UnifiedPeerServiceTests.swift | 32 +++++++++++++++++++ 5 files changed, 108 insertions(+), 15 deletions(-) create mode 100644 bitchat/Utils/NicknameLookup.swift diff --git a/bitchat/Services/UnifiedPeerService.swift b/bitchat/Services/UnifiedPeerService.swift index f1e4f8fa..b52de90a 100644 --- a/bitchat/Services/UnifiedPeerService.swift +++ b/bitchat/Services/UnifiedPeerService.swift @@ -251,17 +251,21 @@ final class UnifiedPeerService: ObservableObject, TransportPeerEventsDelegate { return peerIndex[peerID] } - /// Get peer ID for nickname + /// Get peer ID for nickname. + /// + /// An unsuffixed name is accepted only when it identifies exactly one peer. + /// Colliding nicknames must be disambiguated with the people-list `#xxxx` + /// suffix (first four hex characters of the peer ID). func getPeerID(for nickname: String) -> PeerID? { - // Normalize both sides: the query may come from typed content and - // stored names may predate NFC-at-ingest (e.g. persisted favorites). - let target = nickname.normalizedNickname - for peer in peers { - if peer.displayName.normalizedNickname == target || peer.nickname.normalizedNickname == target { - return peer.peerID + guard let id = NicknameLookup.uniquePeerIDString( + for: nickname, + peers: peers.map { peer in + (id: peer.peerID.id, nickname: peer.nickname, displayName: peer.displayName) } + ) else { + return nil } - return nil + return peers.first(where: { $0.peerID.id == id })?.peerID } /// Check if peer is blocked diff --git a/bitchat/Utils/NicknameLookup.swift b/bitchat/Utils/NicknameLookup.swift new file mode 100644 index 00000000..0e3b8ff4 --- /dev/null +++ b/bitchat/Utils/NicknameLookup.swift @@ -0,0 +1,29 @@ +import Foundation + +/// Resolve a typed nickname to a peer ID string. +/// +/// An unsuffixed name is accepted only when it identifies exactly one peer. +/// Colliding nicknames must be disambiguated with the people-list `#xxxx` +/// suffix (first four hex characters of the peer ID). +enum NicknameLookup { + static func uniquePeerIDString( + for query: String, + peers: [(id: String, nickname: String, displayName: String)] + ) -> String? { + let target = query.normalizedNickname + var matches: [String] = [] + for peer in peers { + let nick = peer.nickname.normalizedNickname + let display = peer.displayName.normalizedNickname + let prefix = String(peer.id.prefix(4)) + let suffixedNick = (nick + "#" + prefix).normalizedNickname + let suffixedDisplay = (display + "#" + prefix).normalizedNickname + if display == target || nick == target || suffixedNick == target || suffixedDisplay == target { + if !matches.contains(peer.id) { + matches.append(peer.id) + } + } + } + return matches.count == 1 ? matches.first : nil + } +} diff --git a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift index e2d0fae8..b52f7598 100644 --- a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift +++ b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift @@ -518,12 +518,15 @@ final class ChatPeerIdentityCoordinator { let nickname = nickname.normalizedNickname switch context.activeChannel { case .location: - if nickname.contains("#"), - let person = context.visibleGeohashPeople() - .first(where: { $0.displayName == nickname }) { - let conversationKey = PeerID(nostr_: person.id) - context.registerNostrKeyMapping(person.id, for: conversationKey) - return conversationKey + if nickname.contains("#") { + let people = context.visibleGeohashPeople().filter { + $0.displayName.normalizedNickname == nickname + } + if people.count == 1, let person = people.first { + let conversationKey = PeerID(nostr_: person.id) + context.registerNostrKeyMapping(person.id, for: conversationKey) + return conversationKey + } } let base = nickname @@ -531,11 +534,15 @@ final class ChatPeerIdentityCoordinator { .first .map(String.init)? .lowercased() ?? nickname.lowercased() - if let pubkey = context.geoNicknames.first(where: { $0.value.lowercased() == base })?.key { + let geoMatches = context.geoNicknames.filter { $0.value.lowercased() == base } + if geoMatches.count == 1, let pubkey = geoMatches.keys.first { let conversationKey = PeerID(nostr_: pubkey) context.registerNostrKeyMapping(pubkey, for: conversationKey) return conversationKey } + if geoMatches.count > 1 { + return nil + } case .mesh: break diff --git a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift index 0bfcd306..404fd8b0 100644 --- a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift +++ b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift @@ -446,6 +446,27 @@ struct ChatPeerIdentityCoordinatorContextTests { context.peerIDsByNickname["carol"] = meshPeer #expect(coordinator.getPeerIDForNickname("carol") == meshPeer) } + + @Test @MainActor + func getPeerIDForNickname_inGeohashChannel_refusesAmbiguousBaseName() async { + let context = MockChatPeerIdentityContext() + let coordinator = ChatPeerIdentityCoordinator(context: context) + let aliceA = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + let aliceB = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + context.activeChannel = .location(GeohashChannel(level: .city, geohash: "u4pruy")) + context.geohashPeople = [ + GeoPerson(id: aliceA, displayName: "alice#aaaa", lastSeen: Date()), + GeoPerson(id: aliceB, displayName: "alice#bbbb", lastSeen: Date()) + ] + context.geoNicknames[aliceA] = "alice" + context.geoNicknames[aliceB] = "alice" + context.peerIDsByNickname["alice"] = PeerID(str: "1122334455667788") + + #expect(coordinator.getPeerIDForNickname("alice") == nil) + #expect(coordinator.getPeerIDForNickname("alice#aaaa") == PeerID(nostr_: aliceA)) + #expect(coordinator.getPeerIDForNickname("alice#bbbb") == PeerID(nostr_: aliceB)) + } + @Test @MainActor func toggleFavorite_forNoiseKeyPeer_usesInjectedFavoritesStore() async { let context = MockChatPeerIdentityContext() diff --git a/bitchatTests/Services/UnifiedPeerServiceTests.swift b/bitchatTests/Services/UnifiedPeerServiceTests.swift index 4bc6f775..35194698 100644 --- a/bitchatTests/Services/UnifiedPeerServiceTests.swift +++ b/bitchatTests/Services/UnifiedPeerServiceTests.swift @@ -27,6 +27,38 @@ struct UnifiedPeerServiceTests { #expect(fingerprint == "fp-1") } + @Test @MainActor + func getPeerID_acceptsAUniqueNickname() async { + let transport = MockTransport() + let identity = TestIdentityManager() + let idBridge = NostrIdentityBridge(keychain: MockKeychainHelper()) + let service = UnifiedPeerService(meshService: transport, idBridge: idBridge, identityManager: identity) + + let alice = PeerID(str: "1111111111111111") + transport.simulateConnect(alice, nickname: "alice") + service.didUpdatePeerSnapshots(transport.currentPeerSnapshots()) + + #expect(service.getPeerID(for: "alice") == alice) + } + + @Test @MainActor + func getPeerID_refusesAmbiguousNicknameUnlessSuffixed() async { + let transport = MockTransport() + let identity = TestIdentityManager() + let idBridge = NostrIdentityBridge(keychain: MockKeychainHelper()) + let service = UnifiedPeerService(meshService: transport, idBridge: idBridge, identityManager: identity) + + let aliceA = PeerID(str: "1111111111111111") + let aliceB = PeerID(str: "2222222222222222") + transport.simulateConnect(aliceA, nickname: "alice") + transport.simulateConnect(aliceB, nickname: "alice") + service.didUpdatePeerSnapshots(transport.currentPeerSnapshots()) + + #expect(service.getPeerID(for: "alice") == nil) + #expect(service.getPeerID(for: "alice#1111") == aliceA) + #expect(service.getPeerID(for: "alice#2222") == aliceB) + } + @Test @MainActor func isBlocked_usesSocialIdentity() async { let transport = MockTransport()