From a008850c9ea5abe2f58a9db8ab17911594596312 Mon Sep 17 00:00:00 2001 From: Shubham Bhandari Date: Thu, 10 Sep 2026 13:05:30 +0800 Subject: [PATCH] Check a message row against its own name, and gate the vouched panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four things from review. Two were bugs, one was a miss, one was an overstated claim of mine. **A row could get its seal back by renaming away and back.** Moving off the rendered name fixed three false positives and introduced this: a message row renders `message.sender`, frozen at receipt, while the gate asked about the peer's CURRENT name. So verify as "ravi", announce "medic", post to the public timeline, announce "ravi" again — the live name is bound once more and the archived row renders `<@medic ✓>`, a seal beside a name the key was never trusted under. Exactly what this PR exists to stop. The fix is not to check both names but to notice that a live row and a historical row ask different questions: live row (peer list, DM header, encryption status) is the name this peer is announcing NOW the one it was verified under? message row (public timeline, DM rows) is the name ON THIS ROW the one it was verified under? So `sealAppliesToRow(fingerprint:renderedSender:)` compares the row's own sender against the pinned baseline and never consults the live name. That kills the rename-back, and it also makes each row self-consistent in the other direction: a historical "ravi ✓" stays sealed while that key is currently renamed, because that row really was ravi. Requiring both names to match would have retroactively unsealed accurate history on every ordinary rename. Comparing a rendered name again needs the `#abcd` collision suffix stripped, which is what pushed me off it before — `splitSuffix()` also removes every "@", and nothing in `validateNickname` forbids one, so "ravi@hq" compared unequal to itself. Hence `withoutCollisionSuffix`, which strips only a trailing "#" plus four hex digits and touches nothing else. **The vouched panel in the fingerprint sheet was ungated** — `FingerprintView.swift:192`, the teal hollow seal with "vouched by N" and the voucher names, read `voucherCount` and never consulted the binding. That is the seal this PR is named for, in the sheet I had singled out as the place the distinction should be explained. `voucherCount` and `voucherNames` are now suppressed together: a voucher list beside a name nobody vouched for is the same claim as the seal. **Correcting my own claim.** I wrote that nothing branches on `.noiseVerified` beyond the glyph and that every behavioural site treats it and `.noiseSecured` alike. That was 3-for-4. `FingerprintView.swift:87` tints the status icon `== .noiseVerified ? .green : textColor`, and `accessibilityDescription` differs per case, so VoiceOver stops saying "verified" on a demoted DM header. Both are the right outcome, but "nothing moves" was wrong and the sheet now shows a plain lock above a bold green "verified" until the copy lands. **Collision counting now folds.** `PeerDisplayNameResolver` counted collisions on the displayed name, so the suffix that exists to make namesakes distinguishable stayed silent for a case variant or a fullwidth Ｍ. The count key is now `precomposedStringWithCompatibilityMapping.lowercased()` — one line, no new data — which closes case, fullwidth, mathematical, script and roman numeral forms, and subsumes the canonical folding Swift's String keys were giving for free. Cross-script homoglyphs still need a real table and are pinned as a known gap with a test to flip. The displayed name is never folded. Also: both seal sites took a `queue.sync` via `isVerified` and then a second for the binding, and in the formatter the check feeds the format cache key so the cache does not shield it. Both are one call now. The two derived queries are protocol requirements with default implementations, so the concrete manager answers in a single lock while conformances only supply the primitives. Tests: 6 new for `PeerDisplayNameResolver`, which had none, including the known-gap rows; 5 more in the binding suite for the per-row rule — the rename-back, a historical row keeping its seal, suffix and "@" handling, and the fail-open cases. With the row check reverted to the live name, 3 of them fail. Suites here: 187 pass / 1908 tests, no result differing from main; 34 still need an app bundle this machine cannot provide. --- bitchat/App/ConversationUIModel.swift | 10 ++- bitchat/App/VerificationModel.swift | 24 +++++- .../Identity/SecureIdentityStateManager.swift | 85 ++++++++++++++++--- bitchat/Utils/PeerDisplayNameResolver.swift | 25 +++++- bitchat/Utils/String+Nickname.swift | 15 ++++ bitchat/ViewModels/ChatMessageFormatter.swift | 8 +- bitchat/ViewModels/ChatViewModel.swift | 14 +++ bitchatTests/Mocks/MockIdentityManager.swift | 2 + ...tityStateManagerNicknameBindingTests.swift | 85 +++++++++++++++++++ .../Services/UnifiedPeerServiceTests.swift | 1 + .../Utils/PeerDisplayNameResolverTests.swift | 82 ++++++++++++++++++ 11 files changed, 327 insertions(+), 24 deletions(-) create mode 100644 bitchatTests/Utils/PeerDisplayNameResolverTests.swift diff --git a/bitchat/App/ConversationUIModel.swift b/bitchat/App/ConversationUIModel.swift index 508ca493..a201eba6 100644 --- a/bitchat/App/ConversationUIModel.swift +++ b/bitchat/App/ConversationUIModel.swift @@ -164,10 +164,12 @@ final class ConversationUIModel: ObservableObject { !isSentByCurrentUser(message), let peerID = message.senderPeerID else { return false } guard let fingerprint = chatViewModel.getFingerprint(for: peerID) else { return false } - guard chatViewModel.peerIdentityStore.isVerified(fingerprint) else { return false } - // A verified key that renamed itself onto a name the user trusts must - // not carry the seal beside the new one. - return !chatViewModel.trustedNicknameMismatch(fingerprint) + // The row renders `message.sender`, frozen at receipt, so the question + // is whether THAT name is the one this key was verified under — not + // whether its current name is. Checking the current name would let a + // rename away, a post, and a rename back put a seal beside a name the + // key was never trusted under. One call, one lock. + return chatViewModel.sealAppliesToRow(fingerprint, renderedSender: message.sender) } func senderDisplayName(for peerID: PeerID, fallbackMessages: [BitchatMessage]) -> String? { diff --git a/bitchat/App/VerificationModel.swift b/bitchat/App/VerificationModel.swift index 4e1de457..992713c2 100644 --- a/bitchat/App/VerificationModel.swift +++ b/bitchat/App/VerificationModel.swift @@ -16,6 +16,15 @@ struct FingerprintPresentationState: Equatable { let voucherCount: Int /// Display names of the (verified) vouchers, where known. let voucherNames: [String] + /// The nickname this key was announcing when it was verified or first + /// vouched, if anything was bound. Non-nil and different from the name on + /// screen is what `nameChangedSinceVerification` reports. + let verifiedAsNickname: String? + /// The key is trusted, but it now announces a different name than when + /// that trust was established. The sheet is the one place this should be + /// explained rather than collapsed into a glyph, so it is surfaced here + /// instead of silently suppressing the badge. + let nameChangedSinceVerification: Bool /// Vouched for by ≥1 peer the user verified (and not explicitly verified). var isVouched: Bool { voucherCount > 0 } @@ -136,6 +145,13 @@ final class VerificationModel: ObservableObject { let isVerified = theirFingerprint.map { peerIdentityStore.isVerified($0) } ?? false let localPetname = theirFingerprint .flatMap { chatViewModel.identityManager.getSocialIdentity(for: $0)?.localPetname } + // A vouched seal is bound to the name it was earned under, exactly like + // the verified one. Missing baseline means nothing was bound, so this + // fails open like every other site. + let nameBound = theirFingerprint + .map { !chatViewModel.identityManager.trustedNicknameMismatch(fingerprint: $0) } ?? true + let verifiedAsNickname = theirFingerprint + .flatMap { chatViewModel.identityManager.trustedNickname(fingerprint: $0) } // Vouch state is recomputed on read: only vouchers still in the // verified set count, so removing a verification silently retires the @@ -161,8 +177,12 @@ final class VerificationModel: ObservableObject { myFingerprint: chatViewModel.getMyFingerprint(), isVerified: isVerified, localPetname: localPetname, - voucherCount: vouchers.count, - voucherNames: voucherNames + // Suppressed together: a voucher list beside a name nobody + // vouched for is the same claim as the seal itself. + voucherCount: nameBound ? vouchers.count : 0, + voucherNames: nameBound ? voucherNames : [], + verifiedAsNickname: verifiedAsNickname, + nameChangedSinceVerification: !nameBound ) } diff --git a/bitchat/Identity/SecureIdentityStateManager.swift b/bitchat/Identity/SecureIdentityStateManager.swift index 91b82624..622d9c58 100644 --- a/bitchat/Identity/SecureIdentityStateManager.swift +++ b/bitchat/Identity/SecureIdentityStateManager.swift @@ -130,9 +130,19 @@ protocol SecureIdentityStateManagerProtocol { func setVerified(fingerprint: String, verified: Bool) func isVerified(fingerprint: String) -> Bool func getVerifiedFingerprints() -> Set + /// The nickname `fingerprint` was announcing when trust in it was + /// established, or nil if nothing was bound. Shown to the user, so a sheet + /// can say *which* name was verified rather than only that one changed. + func trustedNickname(fingerprint: String) -> String? /// Whether this peer now announces a different nickname than the one its /// trust was earned under. func trustedNicknameMismatch(fingerprint: String) -> Bool + /// Verified AND still presenting the name it was verified under — what a + /// seal beside a LIVE name is actually asserting. One lock, not two. + func isVerifiedAndNameBound(fingerprint: String) -> Bool + /// Verified AND `renderedSender` is the name it was verified under — what + /// a seal beside a name frozen on a message row is asserting. + func sealAppliesToRow(fingerprint: String, renderedSender: String) -> Bool // MARK: Vouching (transitive verification) @discardableResult @@ -153,6 +163,26 @@ protocol SecureIdentityStateManagerProtocol { func hasObservedPrivateMediaCapability(fingerprint: String) -> Bool } +extension SecureIdentityStateManagerProtocol { + // Default implementations so a conformance only has to provide the two + // primitives. `SecureIdentityStateManager` overrides both to answer in a + // single lock acquisition — these seal checks run per row, per render, and + // the format cache does not shield the one in ChatMessageFormatter because + // the answer is part of its cache key. + func isVerifiedAndNameBound(fingerprint: String) -> Bool { + isVerified(fingerprint: fingerprint) && !trustedNicknameMismatch(fingerprint: fingerprint) + } + + func sealAppliesToRow(fingerprint: String, renderedSender: String) -> Bool { + guard isVerified(fingerprint: fingerprint) else { return false } + guard let pinned = trustedNickname(fingerprint: fingerprint), !pinned.isEmpty + else { return true } + let shown = renderedSender.withoutCollisionSuffix + guard !shown.isEmpty else { return true } + return pinned.normalizedNickname == shown.normalizedNickname + } +} + /// Singleton manager for secure identity state persistence and retrieval. /// Provides thread-safe access to identity mappings with encryption at rest. /// All identity data is stored encrypted in the device Keychain for security. @@ -765,19 +795,54 @@ final class SecureIdentityStateManager: SecureIdentityStateManagerProtocol { /// before this existed have none, and dropping their seals on upgrade would /// train users to ignore the signal. func trustedNicknameMismatch(fingerprint: String) -> Bool { + queue.sync { liveNameMismatchLocked(fingerprint) } + } + + func trustedNickname(fingerprint: String) -> String? { + queue.sync { cache.trustedNicknames?[fingerprint] } + } + + func isVerifiedAndNameBound(fingerprint: String) -> Bool { queue.sync { - guard let pinned = cache.trustedNicknames?[fingerprint], !pinned.isEmpty - else { return false } - let social = cache.socialIdentities[fingerprint] - // A local petname outranks the claimed nickname everywhere it is - // displayed, so there is nothing to spoof and the seal stands. - if let petname = social?.localPetname, !petname.isEmpty { return false } - guard let claimed = social?.claimedNickname, !claimed.isEmpty else { return false } - // NFC, matching `normalizedNickname` everywhere else nicknames are - // compared: a decomposed and a precomposed "café" are one name. - return pinned.normalizedNickname != claimed.normalizedNickname + guard cache.verifiedFingerprints.contains(fingerprint) else { return false } + return !liveNameMismatchLocked(fingerprint) } } + + /// A message row shows the sender name frozen at receipt, so asking about + /// the peer's CURRENT name is the wrong question: rename away, post, rename + /// back, and the live name matches the baseline again while the archived row + /// still reads the name it was posted under. Each row is therefore checked + /// against its own name, which also makes every row self-consistent — a + /// historical "ravi ✓" stays sealed even while that key is currently + /// renamed, because that row really was ravi. + func sealAppliesToRow(fingerprint: String, renderedSender: String) -> Bool { + queue.sync { + guard cache.verifiedFingerprints.contains(fingerprint) else { return false } + guard let pinned = cache.trustedNicknames?[fingerprint], !pinned.isEmpty + else { return true } // nothing bound: fail open + // No petname escape here, unlike the live check: the formatter + // renders `message.sender`, never a petname, so a petname does not + // stand between the reader and a spoofed name on this row. + let shown = renderedSender.withoutCollisionSuffix + guard !shown.isEmpty else { return true } + return pinned.normalizedNickname == shown.normalizedNickname + } + } + + /// Requires `queue`. + private func liveNameMismatchLocked(_ fingerprint: String) -> Bool { + guard let pinned = cache.trustedNicknames?[fingerprint], !pinned.isEmpty + else { return false } + let social = cache.socialIdentities[fingerprint] + // A local petname outranks the claimed nickname everywhere a LIVE name + // is displayed, so there is nothing to spoof and the seal stands. + if let petname = social?.localPetname, !petname.isEmpty { return false } + guard let claimed = social?.claimedNickname, !claimed.isEmpty else { return false } + // NFC, matching `normalizedNickname` everywhere else nicknames are + // compared: a decomposed and a precomposed "café" are one name. + return pinned.normalizedNickname != claimed.normalizedNickname + } func getVerifiedFingerprints() -> Set { queue.sync { diff --git a/bitchat/Utils/PeerDisplayNameResolver.swift b/bitchat/Utils/PeerDisplayNameResolver.swift index a92953d3..cf6ec094 100644 --- a/bitchat/Utils/PeerDisplayNameResolver.swift +++ b/bitchat/Utils/PeerDisplayNameResolver.swift @@ -3,23 +3,40 @@ import BitFoundation /// Resolves a stable display name for peers, adding a short suffix when collisions exist. struct PeerDisplayNameResolver { + /// The form two nicknames are judged to collide in. Never displayed. + static func collisionKey(_ nickname: String) -> String { + nickname.precomposedStringWithCompatibilityMapping.lowercased() + } + /// Computes display names with a `#xxxx` suffix for connected peers when nickname collisions occur. /// - Parameters: /// - peers: Array of tuples (peerID, nickname, isConnected). /// - selfNickname: The local user's current nickname, included in collision counts to suffix remotes matching it. /// - Returns: Map of peerID -> displayName. static func resolve(_ peers: [(peerID: PeerID, nickname: String, isConnected: Bool)], selfNickname: String) -> [PeerID: String] { - // Count collisions among connected peers and include our own nickname + // Count collisions on a FOLDED key, not the displayed name. The point + // of the suffix is to make namesakes distinguishable, so anything that + // renders alike has to count as a collision. + // + // Compatibility mapping plus case folding covers more than it looks: + // fullwidth Ｍedic, mathematical 𝐌edic, script ℳedic and roman-numeral + // Ⅿedic all fold to "medic", and NFKC subsumes the canonical folding + // that Swift's String keys were already giving us for free. + // + // What it does NOT cover is cross-script homoglyphs — Cyrillic М and + // Greek Ο have their own compatibility forms — which needs a UTS #39 + // confusables table. Suffixing the displayed name is unchanged; only + // the collision key folds. var counts: [String: Int] = [:] for p in peers where p.isConnected { - counts[p.nickname, default: 0] += 1 + counts[Self.collisionKey(p.nickname), default: 0] += 1 } - counts[selfNickname, default: 0] += 1 + counts[Self.collisionKey(selfNickname), default: 0] += 1 var result: [PeerID: String] = [:] for p in peers { var name = p.nickname - if p.isConnected, (counts[p.nickname] ?? 0) > 1 { + if p.isConnected, (counts[Self.collisionKey(p.nickname)] ?? 0) > 1 { name += "#" + String(p.peerID.id.prefix(4)) } result[p.peerID] = name diff --git a/bitchat/Utils/String+Nickname.swift b/bitchat/Utils/String+Nickname.swift index 9a652667..2902613f 100644 --- a/bitchat/Utils/String+Nickname.swift +++ b/bitchat/Utils/String+Nickname.swift @@ -17,6 +17,21 @@ extension String { precomposedStringWithCanonicalMapping } + /// Strips ONLY a trailing `#abcd` collision suffix, leaving everything + /// else alone. + /// + /// Deliberately not `splitSuffix()`: that also removes every `@` in the + /// string, which is right for parsing a mention but wrong for comparing a + /// nickname — nothing in `validateNickname` forbids `@`, so `ravi@hq` + /// would compare unequal to itself. + var withoutCollisionSuffix: String { + guard count >= 5 else { return self } + let tail = suffix(5) + guard tail.first == "#", + tail.dropFirst().allSatisfy({ $0.isHexDigit }) else { return self } + return String(dropLast(5)) + } + /// Split a nickname into base and a '#abcd' suffix if present func splitSuffix() -> (String, String) { let name = self.replacingOccurrences(of: "@", with: "") diff --git a/bitchat/ViewModels/ChatMessageFormatter.swift b/bitchat/ViewModels/ChatMessageFormatter.swift index bb661a35..6b5f9899 100644 --- a/bitchat/ViewModels/ChatMessageFormatter.swift +++ b/bitchat/ViewModels/ChatMessageFormatter.swift @@ -448,10 +448,10 @@ private extension ChatMessageFormatter { let fingerprint = viewModel.getFingerprint(for: peerID) else { return false } - guard viewModel.peerIdentityStore.isVerified(fingerprint) else { return false } - // Bound to the name the key announces, for the same reason as the peer - // list and the DM seal: the badge attests to a key but is read as a name. - return !viewModel.trustedNicknameMismatch(fingerprint) + // Bound to the name ON THIS ROW, which is frozen at receipt — see + // `sealAppliesToRow`. This check feeds the format cache key, so the + // cache does not shield it; a single call keeps it to one lock. + return viewModel.sealAppliesToRow(fingerprint, renderedSender: message.sender) } func appendVerifiedSeal( diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index b2d9f48c..9e098ff9 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1982,6 +1982,20 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage identityManager.trustedNicknameMismatch(fingerprint: fingerprint) } + /// The name this key was verified under, for a sheet that needs to say + /// which one it was. + @MainActor + func trustedNickname(_ fingerprint: String) -> String? { + identityManager.trustedNickname(fingerprint: fingerprint) + } + + /// A seal beside a name frozen on a message row: is THAT name the one this + /// key was verified under? See `sealAppliesToRow`. + @MainActor + func sealAppliesToRow(_ fingerprint: String, renderedSender: String) -> Bool { + identityManager.sealAppliesToRow(fingerprint: fingerprint, renderedSender: renderedSender) + } + // MARK: - BitchatDelegate Methods // MARK: - Command Handling diff --git a/bitchatTests/Mocks/MockIdentityManager.swift b/bitchatTests/Mocks/MockIdentityManager.swift index f0e1b656..2e1fc12d 100644 --- a/bitchatTests/Mocks/MockIdentityManager.swift +++ b/bitchatTests/Mocks/MockIdentityManager.swift @@ -102,6 +102,8 @@ final class MockIdentityManager: SecureIdentityStateManagerProtocol { var claimedNicknames: [String: String] = [:] + func trustedNickname(fingerprint: String) -> String? { trustedNicknames[fingerprint] } + func trustedNicknameMismatch(fingerprint: String) -> Bool { guard let pinned = trustedNicknames[fingerprint], !pinned.isEmpty, let claimed = claimedNicknames[fingerprint], !claimed.isEmpty else { return false } diff --git a/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift b/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift index 33945919..060a2693 100644 --- a/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift +++ b/bitchatTests/Services/SecureIdentityStateManagerNicknameBindingTests.swift @@ -195,6 +195,91 @@ struct SecureIdentityStateManagerNicknameBindingTests { #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) } + // MARK: - A seal beside a name frozen on a message row + + @Test + func renamingBackDoesNotRestoreTheSealOnARowPostedUnderTheOtherName() { + // The hole in checking only the CURRENT name: a message row renders + // `message.sender` frozen at receipt, so rename away, post, rename + // back, and the live name matches the baseline again while the archived + // row still reads the name it was posted under. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + + announce(manager, vouchee, as: "medic") // rename + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee)) + announce(manager, vouchee, as: "ravi") // ...and back + #expect(!manager.trustedNicknameMismatch(fingerprint: vouchee), + "the live name is bound again, which is why the live check alone is not enough") + + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic"), + "the row posted as medic must not be sealed") + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi"), + "a row posted as ravi still is — that row really was ravi") + } + + @Test + func aHistoricalRowKeepsItsSealWhileTheKeyIsCurrentlyRenamed() { + // The converse, so the rule is not simply "suppress harder": a row + // posted under the verified name stays truthful even once that key has + // moved on, otherwise ordinary renames retroactively unseal history. + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + manager.setVerified(fingerprint: vouchee, verified: true) + announce(manager, vouchee, as: "medic") + + #expect(manager.trustedNicknameMismatch(fingerprint: vouchee), "live name is unbound") + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi"), + "but the archived ravi row is still accurate") + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic")) + } + + @Test + func aRowSenderCarryingACollisionSuffixIsStillMatched() { + // Message senders render as "ravi#a1b2" when nicknames collide. Only a + // trailing #abcd is stripped — `splitSuffix()` would also remove every + // "@", and nothing forbids one in a nickname. + let manager = makeManager() + announce(manager, vouchee, as: "ravi@hq") + manager.setVerified(fingerprint: vouchee, verified: true) + + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi@hq")) + #expect(manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi@hq#a1b2")) + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi")) + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "medic#a1b2")) + } + + @Test + func aRowIsNotSealedForAnUnverifiedKeyAndFailsOpenWithNoBaseline() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + #expect(!manager.sealAppliesToRow(fingerprint: vouchee, renderedSender: "ravi"), + "no verification, no seal") + + // Verified before any announce: nothing bound, so rows stay sealed as + // they did before this existed. + let unbound = makeManager() + unbound.setVerified(fingerprint: vouchee, verified: true) + #expect(unbound.sealAppliesToRow(fingerprint: vouchee, renderedSender: "anything")) + } + + @Test + func theCombinedLiveQueryAgreesWithItsParts() { + let manager = makeManager() + announce(manager, vouchee, as: "ravi") + #expect(!manager.isVerifiedAndNameBound(fingerprint: vouchee), "not verified yet") + + manager.setVerified(fingerprint: vouchee, verified: true) + #expect(manager.isVerifiedAndNameBound(fingerprint: vouchee)) + #expect(manager.trustedNickname(fingerprint: vouchee) == "ravi") + + announce(manager, vouchee, as: "medic") + #expect(!manager.isVerifiedAndNameBound(fingerprint: vouchee)) + #expect(manager.trustedNickname(fingerprint: vouchee) == "ravi", + "the baseline is what the sheet needs to name") + } + // MARK: - Rebinding and clearing @Test diff --git a/bitchatTests/Services/UnifiedPeerServiceTests.swift b/bitchatTests/Services/UnifiedPeerServiceTests.swift index 0176634a..ec604c23 100644 --- a/bitchatTests/Services/UnifiedPeerServiceTests.swift +++ b/bitchatTests/Services/UnifiedPeerServiceTests.swift @@ -291,6 +291,7 @@ private final class TestIdentityManager: SecureIdentityStateManagerProtocol { // No name binding is exercised here: these tests drive peer-list assembly, // not trust display. + func trustedNickname(fingerprint: String) -> String? { nil } func trustedNicknameMismatch(fingerprint: String) -> Bool { false } // MARK: Vouching (unused by these tests) diff --git a/bitchatTests/Utils/PeerDisplayNameResolverTests.swift b/bitchatTests/Utils/PeerDisplayNameResolverTests.swift new file mode 100644 index 00000000..10a2ee48 --- /dev/null +++ b/bitchatTests/Utils/PeerDisplayNameResolverTests.swift @@ -0,0 +1,82 @@ +import Foundation +import BitFoundation +import Testing + +@testable import bitchat + +/// The `#abcd` suffix exists so two peers claiming one nickname are +/// distinguishable. It is therefore only as good as the notion of "one +/// nickname" it counts with — anything that renders alike has to collide, or +/// the guard stays silent exactly when two rows look the same. +@MainActor +struct PeerDisplayNameResolverTests { + private let real = PeerID(str: "aaaa1111") + private let other = PeerID(str: "bbbb2222") + + /// True when the resolver decided these two need telling apart. + private func suffixed(_ a: String, _ b: String, + connected: Bool = true, selfNickname: String = "me") -> Bool { + let out = PeerDisplayNameResolver.resolve( + [(peerID: real, nickname: a, isConnected: true), + (peerID: other, nickname: b, isConnected: connected)], + selfNickname: selfNickname) + return (out[real] ?? "").contains("#") || (out[other] ?? "").contains("#") + } + + @Test + func namesThatRenderAlikeCollide() { + #expect(suffixed("Medic", "Medic"), "exact duplicate") + #expect(suffixed("Medic", "medic"), "case alone must not make two peers look distinct") + #expect(suffixed("Medic", "\u{FF2D}edic"), "fullwidth Ｍ") + #expect(suffixed("Medic", "\u{1D40C}edic"), "mathematical bold 𝐌") + #expect(suffixed("Medic", "\u{2133}edic"), "script ℳ") + #expect(suffixed("Medic", "\u{216F}edic"), "roman numeral Ⅿ") + #expect(suffixed("Jos\u{00E9}", "Jose\u{0301}"), "canonically equivalent spellings") + } + + @Test + func unrelatedNamesDoNotCollide() { + #expect(!suffixed("Medic", "Zebra")) + #expect(!suffixed("Medic", "Medic2")) + #expect(!suffixed("Medic", "")) + } + + @Test + func crossScriptHomoglyphsAreStillMissed() { + // Cyrillic М and Greek Ο have their own compatibility forms, so + // folding cannot reach them — that needs a UTS #39 confusables table. + // Pinned as a KNOWN gap so it is explicit rather than a surprise, and + // so whoever adds the table has a test to flip. + #expect(!suffixed("Medic", "\u{041C}\u{0435}dic"), "Cyrillic М + е: known gap") + #expect(!suffixed("Organizer", "\u{039F}rganizer"), "Greek Ο: known gap") + } + + @Test + func onlyConnectedPeersAreSuffixed() { + // Unchanged by the folding: an offline namesake still gets no suffix, + // which is its own gap and not one this touches. + #expect(!suffixed("Medic", "medic", connected: false)) + } + + @Test + func ourOwnNicknameCountsTowardCollisions() { + // A remote peer claiming a folded variant of MY nickname must be + // suffixed, or it renders as me. + let out = PeerDisplayNameResolver.resolve( + [(peerID: other, nickname: "\u{FF2D}edic", isConnected: true)], + selfNickname: "Medic") + #expect((out[other] ?? "").contains("#")) + } + + @Test + func theDisplayedNameIsNeverFolded() { + // Only the collision KEY folds. What is shown stays as its owner typed + // it, suffix aside. + let out = PeerDisplayNameResolver.resolve( + [(peerID: real, nickname: "Medic", isConnected: true), + (peerID: other, nickname: "\u{FF2D}edic", isConnected: true)], + selfNickname: "me") + #expect(out[real]?.hasPrefix("Medic") == true) + #expect(out[other]?.hasPrefix("\u{FF2D}edic") == true, "the fullwidth Ｍ is preserved on screen") + } +}