From cfa875459d02bf61f64658bfd36baeeb60fadea2 Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 11 Aug 2026 10:03:01 +0200 Subject: [PATCH 1/5] Review fix: record withheld receipts in both tracking sets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P1: the manager-path withheld claim landed only in PrivateChatManager.sentReadReceipts, while the lifecycle read pass dedups against ChatViewModel's persisted set — enabling receipts before the next lifecycle pass could send a receipt for a message read while the setting was off. The withheld branch now records into the owner's persisted set too (markReceiptHandled, wired in the bootstrapper); test strengthened to require both sets. Co-Authored-By: Claude Fable 5 --- bitchat/Services/PrivateChatManager.swift | 12 ++++++++++-- bitchat/ViewModels/ChatViewModelBootstrapper.swift | 3 +++ bitchatTests/ChatViewModelTests.swift | 9 ++++++--- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/bitchat/Services/PrivateChatManager.swift b/bitchat/Services/PrivateChatManager.swift index 3141ec0b..a243d570 100644 --- a/bitchat/Services/PrivateChatManager.swift +++ b/bitchat/Services/PrivateChatManager.swift @@ -252,15 +252,23 @@ final class PrivateChatManager: ObservableObject { /// suites through the shared UserDefaults-backed setting. var sendsReadReceipts: () -> Bool = { ReadReceiptSettings.sendReadReceipts } + /// Records a withheld receipt in the owner's persisted set too: the + /// lifecycle read pass dedups against ChatViewModel.sentReadReceipts, + /// not this manager's set, so claiming only locally would let a receipt + /// for a message read while the setting was OFF fire after re-enabling. + var markReceiptHandled: ((String) -> Void)? + private func sendReadReceipt(for message: BitchatMessage) { guard !sentReadReceipts.contains(message.id), let senderPeerID = message.senderPeerID else { return } - // Withheld receipts are still claimed below as sent: re-enabling the - // setting must never fire a retroactive burst disclosing past reads. + // Withheld receipts are still claimed as sent — in BOTH tracking + // sets: re-enabling the setting must never fire a retroactive burst + // disclosing past reads, from this manager or the lifecycle pass. guard sendsReadReceipts() else { sentReadReceipts.insert(message.id) + markReceiptHandled?(message.id) return } diff --git a/bitchat/ViewModels/ChatViewModelBootstrapper.swift b/bitchat/ViewModels/ChatViewModelBootstrapper.swift index 3ccb7d12..ca769b05 100644 --- a/bitchat/ViewModels/ChatViewModelBootstrapper.swift +++ b/bitchat/ViewModels/ChatViewModelBootstrapper.swift @@ -90,6 +90,9 @@ private extension ChatViewModelBootstrapper { viewModel.privateChatManager.conversationStore = viewModel.conversations viewModel.privateChatManager.messageRouter = viewModel.messageRouter viewModel.privateChatManager.unifiedPeerService = viewModel.unifiedPeerService + viewModel.privateChatManager.markReceiptHandled = { [weak viewModel] messageID in + viewModel?.markReadReceiptSent(messageID) + } viewModel.unifiedPeerService.messageRouter = viewModel.messageRouter // Surface silent outbox drops (attempt cap, TTL expiry, overflow // eviction) as a visible failure. The store's no-downgrade rule does diff --git a/bitchatTests/ChatViewModelTests.swift b/bitchatTests/ChatViewModelTests.swift index 1bd66497..b4e9109e 100644 --- a/bitchatTests/ChatViewModelTests.swift +++ b/bitchatTests/ChatViewModelTests.swift @@ -462,10 +462,13 @@ struct ChatViewModelServiceLifecycleTests { #expect(!sentReadReceipt) // ...while the chat is still marked read locally and the receipt is - // recorded as handled, so re-enabling the setting never fires a - // retroactive burst disclosing past reading activity. + // recorded as handled in BOTH tracking sets (the lifecycle pass + // dedups against the owner's persisted set, the manager against its + // own), so re-enabling the setting never fires a retroactive burst + // disclosing past reading activity from either path. #expect(!viewModel.unreadPrivateMessages.contains(peerID)) - #expect(viewModel.sentReadReceipts.contains("read-2") || viewModel.privateChatManager.sentReadReceipts.contains("read-2")) + #expect(viewModel.sentReadReceipts.contains("read-2")) + #expect(viewModel.privateChatManager.sentReadReceipts.contains("read-2")) } @Test @MainActor From 6dd14961c6c26f8490c7fad5741c07e70b1924f9 Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 11 Aug 2026 10:06:45 +0200 Subject: [PATCH 2/5] Review fix: opt live-voice tests in via injectable provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P1: the fixtures assumed the old default-ON preference — on a clean CI process the coordinator guard drops their frames. Instead of mutating the shared UserDefaults per test (which races parallel suites), the live-voice reads are now injectable (ChatLiveVoiceCoordinator.liveVoiceEnabled), the fixtures opt in per instance, and the toggle-off test drives the provider directly. Co-Authored-By: Claude Fable 5 --- .../ViewModels/ChatLiveVoiceCoordinator.swift | 9 ++++++-- .../ChatViewModel+PrivateChat.swift | 2 +- .../ChatLiveVoiceCoordinatorTests.swift | 22 +++++++++++++++---- bitchatTests/ChatViewModelTests.swift | 3 +++ 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/bitchat/ViewModels/ChatLiveVoiceCoordinator.swift b/bitchat/ViewModels/ChatLiveVoiceCoordinator.swift index 78fa83e1..acc4a298 100644 --- a/bitchat/ViewModels/ChatLiveVoiceCoordinator.swift +++ b/bitchat/ViewModels/ChatLiveVoiceCoordinator.swift @@ -79,6 +79,11 @@ enum VoiceBurstScope: Hashable { /// bubble so nobody sees a duplicate. @MainActor final class ChatLiveVoiceCoordinator { + /// Injectable so tests exercise the live path without racing other + /// suites through the shared UserDefaults-backed preference (which now + /// defaults OFF). + var liveVoiceEnabled: () -> Bool = { PTTSettings.liveVoiceEnabled } + /// Burst IDs are sender-chosen, so they only identify a burst *within* /// an authenticated (peer, scope) pair: keying assemblies by the full /// triple stops an attacker who observed a public burst ID from racing @@ -187,7 +192,7 @@ final class ChatLiveVoiceCoordinator { // Live voice off means classic-notes-only in both directions: no live // bubble, no partial file, no early notification — the finalized // voice note still arrives through the normal pipeline. - guard PTTSettings.liveVoiceEnabled else { + guard liveVoiceEnabled() else { SecureLogger.debug("PTT: dropping inbound voice frame — live voice is toggled off", category: .session) return } @@ -403,7 +408,7 @@ final class ChatLiveVoiceCoordinator { case .directMessage: context.selectedPrivateChatPeer == peerID case .publicMesh: context.isViewingPublicMeshTimeline } - if PTTSettings.liveVoiceEnabled, PTTSettings.isAppActive, isViewing { + if liveVoiceEnabled(), PTTSettings.isAppActive, isViewing { assembly.player = PTTBurstPlayer() } diff --git a/bitchat/ViewModels/Extensions/ChatViewModel+PrivateChat.swift b/bitchat/ViewModels/Extensions/ChatViewModel+PrivateChat.swift index 9dad8030..968da1a9 100644 --- a/bitchat/ViewModels/Extensions/ChatViewModel+PrivateChat.swift +++ b/bitchat/ViewModels/Extensions/ChatViewModel+PrivateChat.swift @@ -69,7 +69,7 @@ extension ChatViewModel { @MainActor private func liveVoiceTarget() -> LiveVoiceTarget? { - guard PTTSettings.liveVoiceEnabled else { return nil } + guard liveVoiceCoordinator.liveVoiceEnabled() else { return nil } if let selectedPeer = selectedPrivateChatPeer { guard !selectedPeer.isGeoDM, !selectedPeer.isGeoChat, !selectedPeer.isGroup else { return nil } diff --git a/bitchatTests/ChatLiveVoiceCoordinatorTests.swift b/bitchatTests/ChatLiveVoiceCoordinatorTests.swift index 2e5ca317..67b509c7 100644 --- a/bitchatTests/ChatLiveVoiceCoordinatorTests.swift +++ b/bitchatTests/ChatLiveVoiceCoordinatorTests.swift @@ -127,6 +127,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func burstCreatesBubbleAndPersistsFramesInOrder() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0xA1) defer { fallbackFileURL(burstID: burstID, peerID: peer).map { try? FileManager.default.removeItem(at: $0) } } @@ -168,6 +169,7 @@ struct ChatLiveVoiceCoordinatorTests { let context = MockChatLiveVoiceContext() context.selectedPrivateChatPeer = peer let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0xB2) let hex = burstID.hexEncodedString() let fileName = "voice_\(hex).m4a" @@ -223,6 +225,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func absorbIgnoresUnrelatedVoiceNotes() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } // A classic voice note (date-stamped name) and a live-capture name // must both pass through untouched. @@ -247,6 +250,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func canceledBurstRemovesBubbleAndFile() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0xC3) send(try #require(VoiceBurstPacket(burstID: burstID, seq: 1, kind: .frames([Data(repeating: 9, count: 40)]))), to: coordinator, from: peer) @@ -262,6 +266,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func emptyBurstLeavesNoBubble() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0xD4) send(try #require(VoiceBurstPacket(burstID: burstID, seq: 0, kind: .start(codec: .aacLC16kMono))), to: coordinator, from: peer) @@ -275,6 +280,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func ignoresBlockedPeersAndUnknownControlPackets() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } context.blockedPeers = [peer] send(try #require(VoiceBurstPacket(burstID: makeBurstID(0xE5), seq: 0, kind: .start(codec: .aacLC16kMono))), to: coordinator, from: peer) @@ -290,6 +296,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func concurrentAssemblyCapDropsExtraBursts() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } var cleanup: [Data] = [] defer { @@ -309,12 +316,10 @@ struct ChatLiveVoiceCoordinatorTests { } @Test func liveVoiceToggleOffDropsInboundFrames() throws { - let previous = PTTSettings.liveVoiceEnabled - PTTSettings.liveVoiceEnabled = false - defer { PTTSettings.liveVoiceEnabled = previous } - let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } + coordinator.liveVoiceEnabled = { false } let burstID = makeBurstID(0xE8) // Off means classic-notes-only: no live bubble, no partial file. @@ -328,6 +333,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func publicBurstCreatesMeshBubbleAndTracksTalker() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0x71) defer { incomingFileURL(burstID: burstID, peerID: peer, scope: .publicMesh).map { try? FileManager.default.removeItem(at: $0) } @@ -369,6 +375,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func absorbEnforcesScopeBinding() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0x72) defer { fallbackFileURL(burstID: burstID, peerID: peer).map { try? FileManager.default.removeItem(at: $0) } } @@ -400,6 +407,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func collidingBurstIDFromAnotherPeerCannotHijackAssembly() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0x73) let attacker = PeerID(str: "ddddeeeeffff0002") defer { @@ -435,6 +443,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func sameBurstIDCoexistsAcrossScopes() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0x74) defer { fallbackFileURL(burstID: burstID, peerID: peer).map { try? FileManager.default.removeItem(at: $0) } @@ -492,6 +501,7 @@ struct ChatLiveVoiceCoordinatorTests { @Test func finalizedNoteBindsToItsAuthenticatedSender() throws { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, sweepsOnInit: false) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0x75) let hex = burstID.hexEncodedString() let attacker = PeerID(str: "ddddeeeeffff0002") @@ -550,6 +560,7 @@ struct ChatLiveVoiceCoordinatorTests { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, fileStore: store) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0x76) send(try #require(VoiceBurstPacket(burstID: burstID, seq: 0, kind: .start(codec: .aacLC16kMono))), to: coordinator, from: peer) @@ -571,6 +582,7 @@ struct ChatLiveVoiceCoordinatorTests { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, fileStore: store) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0x77) send(try #require(VoiceBurstPacket(burstID: burstID, seq: 0, kind: .start(codec: .aacLC16kMono))), to: coordinator, from: peer) send(try #require(VoiceBurstPacket(burstID: burstID, seq: 1, kind: .frames([Data(repeating: 8, count: 60)]))), to: coordinator, from: peer) @@ -618,6 +630,7 @@ struct ChatLiveVoiceCoordinatorTests { // sender out of range): the capture is the row's only audio. let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, fileStore: store) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0x79) let frame = Data(repeating: 0x0B, count: 60) send(try #require(VoiceBurstPacket(burstID: burstID, seq: 1, kind: .frames([frame]))), to: coordinator, from: peer) @@ -642,6 +655,7 @@ struct ChatLiveVoiceCoordinatorTests { let context = MockChatLiveVoiceContext() let coordinator = ChatLiveVoiceCoordinator(context: context, fileStore: store) + coordinator.liveVoiceEnabled = { true } let burstID = makeBurstID(0x7A) send(try #require(VoiceBurstPacket(burstID: burstID, seq: 1, kind: .frames([Data(repeating: 0x0C, count: 60)]))), to: coordinator, from: peer) send(try #require(VoiceBurstPacket(burstID: burstID, seq: 2, kind: .end(totalDataPackets: 1, durationMs: 64))), to: coordinator, from: peer) diff --git a/bitchatTests/ChatViewModelTests.swift b/bitchatTests/ChatViewModelTests.swift index b4e9109e..b7684aa3 100644 --- a/bitchatTests/ChatViewModelTests.swift +++ b/bitchatTests/ChatViewModelTests.swift @@ -1591,6 +1591,9 @@ struct ChatViewModelPrivateMediaDeletionTests { kind: .canceled )) let coordinator = viewModel.liveVoiceCoordinator + // The live-voice preference defaults OFF now; this fixture exercises + // the opted-in live path. + coordinator.liveVoiceEnabled = { true } defer { coordinator.handleVoiceFramePayload( from: peerID, From c58ad9af1090463fdb4bb5ee9b6d154b7d16fc7e Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 11 Aug 2026 10:09:57 +0200 Subject: [PATCH 3/5] Review fixes: unread flag + favorites-aware naming for the warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Codex P1: a background identity change appended the warning without marking the chat unread — a security event nobody was looking at stayed invisible until the conversation was manually opened. It now sets the unread flag unless the chat is open. - Codex P2: offline key rotation is the common case here, and resolveNickname falls back to an anon prefix precisely then (no mesh nickname, no social identity for the unverified new fingerprint). The persisted favorite relationship's nickname is preferred. Both pinned by tests. Co-Authored-By: Claude Fable 5 --- .../ChatPeerIdentityCoordinator.swift | 19 ++++++++++- ...tPeerIdentityCoordinatorContextTests.swift | 33 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift index 2f6f7270..b67fae5f 100644 --- a/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift +++ b/bitchat/ViewModels/ChatPeerIdentityCoordinator.swift @@ -24,6 +24,9 @@ protocol ChatPeerIdentityContext: AnyObject { var unreadPrivateMessages: Set { get } /// Clears the peer's unread flag (single-writer store intent). func markPrivateChatRead(_ peerID: PeerID) + /// Sets the peer's unread flag (shared requirement with the inbound DM + /// paths; witness on `ChatViewModel`). + func markPrivateChatUnread(_ peerID: PeerID) /// Moves all messages from `oldPeerID`'s chat into `newPeerID`'s chat /// (dedup by ID, order preserved, unread carried, old chat removed). func migratePrivateChat(from oldPeerID: PeerID, to newPeerID: PeerID) @@ -621,12 +624,20 @@ extension ChatPeerIdentityCoordinator { // inherit the thread's earned trust under the same nickname. Only // warn when there is a conversation to protect. if wasSelected || !context.privateMessages(for: newPeerID).isEmpty { + // Offline key rotation is the common case here (a favorite came + // back with new keys), and resolveNickname has no mesh nickname + // and no social identity for a fingerprint nobody verified yet — + // the persisted favorite relationship still knows who this is. + let favoriteNickname = newPeerID.noiseKey + .flatMap { context.favoriteRelationship(forNoiseKey: $0)?.peerNickname } + .flatMap { $0.isEmpty ? nil : $0 } + let displayName = favoriteNickname ?? resolveNickname(for: newPeerID) let notice = BitchatMessage( sender: "system", content: String( format: String(localized: "system.identity.key_changed", defaultValue: "%@'s identity key changed — this can mean a new device or a reset. earlier verification no longer applies; verify them again before trusting this chat.", comment: "Private-chat system warning after a peer's Noise identity key changed; placeholder is the peer's name"), locale: .current, - resolveNickname(for: newPeerID) + displayName ), timestamp: Date(), isRelay: false, @@ -636,6 +647,12 @@ extension ChatPeerIdentityCoordinator { senderPeerID: context.myPeerID ) context.appendPrivateMessage(notice, to: newPeerID) + // A security event nobody is looking at must not stay silent: + // surface it through the unread indicator unless the chat is + // open right now. + if !wasSelected { + context.markPrivateChatUnread(newPeerID) + } context.notifyUIChanged() } } diff --git a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift index d14f766a..0def7e2c 100644 --- a/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift +++ b/bitchatTests/ChatPeerIdentityCoordinatorContextTests.swift @@ -46,6 +46,10 @@ private final class MockChatPeerIdentityContext: ChatPeerIdentityContext { unreadPrivateMessages.remove(peerID) } + func markPrivateChatUnread(_ peerID: PeerID) { + unreadPrivateMessages.insert(peerID) + } + @discardableResult func appendPrivateMessage(_ message: BitchatMessage, to peerID: PeerID) -> Bool { var chat = privateChats[peerID] ?? [] @@ -390,6 +394,35 @@ struct ChatPeerIdentityCoordinatorContextTests { #expect(notice?.content.contains("identity key changed") == true) #expect(notice?.content.contains("alice") == true) #expect(context.privateChats[oldPeerID] == nil) + // A background security event must surface via the unread indicator. + #expect(context.unreadPrivateMessages.contains(newPeerID)) + } + + @Test @MainActor + func migrateNoiseKeyUpdate_namesOfflineFavoritesFromTheRelationship() async { + let context = MockChatPeerIdentityContext() + let coordinator = ChatPeerIdentityCoordinator(context: context) + let oldPeerID = PeerID(str: "5555555555555555") + let newKey = Data(repeating: 0xCD, count: 32) + let newPeerID = PeerID(hexData: newKey) + // Offline key rotation: no mesh nickname, no social identity for the + // unverified new fingerprint — only the favorite relationship knows + // who this is. + context.favoriteRelationshipsByNoiseKey[newKey] = FavoritesPersistenceService.FavoriteRelationship( + peerNoisePublicKey: newKey, + peerNostrPublicKey: nil, + peerNickname: "carol", + isFavorite: true, + theyFavoritedUs: true, + favoritedAt: Date(timeIntervalSince1970: 0), + lastUpdated: Date(timeIntervalSince1970: 0) + ) + context.privateChats[oldPeerID] = [makePrivateMessage(id: "m2", timestamp: Date(timeIntervalSince1970: 1))] + + coordinator.migrateNoiseKeyUpdate(oldPeerID: oldPeerID, newPeerID: newPeerID) + + let notice = (context.privateChats[newPeerID] ?? []).last + #expect(notice?.content.contains("carol") == true) } @Test @MainActor From d530921919fb30a08577ac7f908c896ecf9249c3 Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 11 Aug 2026 10:14:08 +0200 Subject: [PATCH 4/5] Review fixes: suffix-tolerant actor + name-token target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Codex P1: bounded-single-line was not "exact locally generated shape" — a self-attributed action could smuggle a preamble into the target slot ("… hugs SECURITY: reset your keys at evil…"). The target must now be a single name token ("you" or a whitespace-free ≤32-char nickname, optionally #abcd-suffixed); free text with spaces degrades to a plain message. - Codex P1: location-channel senders arrive suffixed (bob#ab12) while handleEmote embeds the unsuffixed nickname, so the actor match regressed every received geohash action to plain text. The actor is now compared by base name (splitSuffix), restoring legit rendering. Tests cover the suffixed-sender case, the suffixed target, and the preamble spoof. Co-Authored-By: Claude Fable 5 --- .../ChatPrivateConversationCoordinator.swift | 37 +++++++++++++++---- ...eConversationCoordinatorContextTests.swift | 11 +++++- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift b/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift index 877280d5..58a7335d 100644 --- a/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift +++ b/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift @@ -832,12 +832,16 @@ final class ChatPrivateConversationCoordinator { // a peer can only "hug"/"slap"/"screenshot" as themselves. guard message.content.hasPrefix("* "), message.content.hasSuffix(" *") else { return message } let inner = String(message.content.dropFirst(2).dropLast(2)) - let sender = message.sender + // Match the ACTOR by base name: location-channel senders arrive + // suffixed (`bob#ab12`) while handleEmote embeds the unsuffixed + // nickname, so a raw-string compare would regress every received + // geohash action to plain text. + let senderBase = message.sender.splitSuffix().0 let isActionMessage = - Self.matchesActionTemplate(inner, prefix: "🫂 \(sender) hugs ", suffix: "") - || Self.matchesActionTemplate(inner, prefix: "🐟 \(sender) slaps ", suffix: " around a bit with a large trout") - || inner == "\(sender) took a screenshot" + Self.matchesActionTemplate(inner, prefix: "🫂 \(senderBase) hugs ", suffix: "") + || Self.matchesActionTemplate(inner, prefix: "🐟 \(senderBase) slaps ", suffix: " around a bit with a large trout") + || inner == "\(senderBase) took a screenshot" guard isActionMessage else { return message } @@ -856,13 +860,30 @@ final class ChatPrivateConversationCoordinator { ) } - /// The target slot of an action template: bounded, single-line, and - /// non-empty — a nickname or the literal "you", never free text. + /// The target slot of an action template must be a single name token — + /// "you", or a nickname optionally carrying a `#abcd` suffix — never + /// free text. handleEmote only ever emits a resolved nickname or "you" + /// there; accepting arbitrary bounded text let a self-attributed action + /// smuggle a preamble ("… hugs SECURITY: reset your keys at evil…") into + /// the trusted system styling. static func matchesActionTemplate(_ inner: String, prefix: String, suffix: String) -> Bool { guard inner.hasPrefix(prefix), inner.hasSuffix(suffix), inner.count >= prefix.count + suffix.count + 1 else { return false } - let target = inner.dropFirst(prefix.count).dropLast(suffix.count) - return !target.isEmpty && target.count <= 64 && !target.contains("\n") + let target = String(inner.dropFirst(prefix.count).dropLast(suffix.count)) + return isNameToken(target) + } + + /// A display name as it appears in action content: "you", or a single + /// whitespace-free token of bounded length (a nickname, optionally with + /// a `#abcd` disambiguator). A space-containing nickname degrades to a + /// plain message rather than trusted styling — the safe direction. + static func isNameToken(_ token: String) -> Bool { + guard token == "you" else { + guard !token.isEmpty, token.count <= 32, + !token.contains(where: { $0.isWhitespace }) else { return false } + return true + } + return true } func migratePrivateChatsIfNeeded(for peerID: PeerID, senderNickname: String) { diff --git a/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift b/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift index b0353264..7b07e101 100644 --- a/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift +++ b/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift @@ -350,16 +350,25 @@ struct ChatPrivateConversationCoordinatorContextTests { #expect(processed("* 🐟 bob slaps alice around a bit with a large trout *").sender == "system") #expect(processed("* bob took a screenshot *").sender == "system") + // Location-channel senders arrive suffixed while content stays + // unsuffixed — must still render as a system action. + #expect(processed("* 🫂 bob hugs alice *", sender: "bob#ab12").sender == "system") + #expect(processed("* 🫂 bob hugs alice#1a2b *").sender == "system") + // The spoof this parser used to allow: arbitrary text between the // markers with a magic substring rendered as system-authored. #expect(processed("* SECURITY: your session key expired, re-verify at evil.example — bob took a screenshot *").sender == "bob") #expect(processed("* 🫂 admin hugs alice — send your keys to @admin *").sender == "bob") + // Self-attributed preamble smuggled into the target slot: the target + // must be a single name token, so free text with spaces is rejected. + #expect(processed("* 🫂 bob hugs SECURITY: reset your keys at evil.example *").sender == "bob") // Actor slot must be the actual sender, not someone else's name. #expect(processed("* alice took a screenshot *", sender: "bob").sender == "bob") #expect(processed("* 🫂 alice hugs you *", sender: "bob").sender == "bob") - // Free text smuggled into the target slot: bounded, single-line only. + // Target slot: single whitespace-free token, bounded length only. #expect(processed("* 🫂 bob hugs " + String(repeating: "x", count: 200) + " *").sender == "bob") #expect(processed("* 🫂 bob hugs a\nb *").sender == "bob") + #expect(processed("* 🫂 bob hugs a b *").sender == "bob") // Not an action shape at all. #expect(processed("hello there").sender == "bob") } From 9b72f849665f1ecf514d9d226f78c6119a6e14a1 Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 11 Aug 2026 10:16:53 +0200 Subject: [PATCH 5/5] Review fixes: consent on every add-favorite path; correct empty-star tooltip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Codex P1: the context-menu and VoiceOver "add favorite" actions called onToggleFavorite directly, bypassing the one-time consent dialog and disclosing the nostr key without asking. All add-favorite entry points now route through requestFavoriteToggle (consent gate); removals stay immediate. - Codex P2: the empty star showed the "favorited — offline messaging starts when they favorite you back" tooltip on peers that weren't favorited at all. The pending/mutual copy now applies only once isFavorite; an unfavorited star reads "add favorite". Co-Authored-By: Claude Fable 5 --- bitchat/Views/MeshPeerList.swift | 34 +++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/bitchat/Views/MeshPeerList.swift b/bitchat/Views/MeshPeerList.swift index 16b67d42..ca79801d 100644 --- a/bitchat/Views/MeshPeerList.swift +++ b/bitchat/Views/MeshPeerList.swift @@ -180,15 +180,7 @@ struct MeshPeerList: View { } if !isMe { - Button(action: { - // The first favorite ever asks once: the star - // notifies the peer and shares the nostr key. - if !peer.isFavorite, !FavoriteConsent.isAcknowledged { - pendingFavorite = peer - } else { - onToggleFavorite(peer.peerID) - } - }) { + Button(action: { requestFavoriteToggle(peer) }) { // Mutuality is the load-bearing state (one-sided // favorites don't enable offline delivery), so it // shows at the point of decision: half star until @@ -205,7 +197,7 @@ struct MeshPeerList: View { .contentShape(Rectangle()) } .buttonStyle(.plain) - .help(peer.isMutualFavorite ? Strings.favoriteMutualTooltip : Strings.favoritePendingTooltip) + .help(favoriteTooltip(for: peer)) } } .padding(.horizontal) @@ -221,7 +213,7 @@ struct MeshPeerList: View { onTapPeer(peer.peerID) } Button(peer.isFavorite ? Strings.removeFavorite : Strings.addFavorite) { - onToggleFavorite(peer.peerID) + requestFavoriteToggle(peer) } Button(Strings.showFingerprint) { onShowFingerprint(peer.peerID) @@ -246,7 +238,7 @@ struct MeshPeerList: View { .accessibilityActions { if !isMe { Button(peer.isFavorite ? Strings.removeFavorite : Strings.addFavorite) { - onToggleFavorite(peer.peerID) + requestFavoriteToggle(peer) } Button(Strings.showFingerprint) { onShowFingerprint(peer.peerID) @@ -291,6 +283,24 @@ struct MeshPeerList: View { } } + /// Routes every add-favorite entry point (star, context menu, VoiceOver) + /// through the one-time consent dialog; removals stay immediate. + private func requestFavoriteToggle(_ peer: MeshPeerRow) { + if !peer.isFavorite, !FavoriteConsent.isAcknowledged { + pendingFavorite = peer + } else { + onToggleFavorite(peer.peerID) + } + } + + /// Tooltip for the star: the mutual/pending copy applies only once the + /// peer is actually favorited — on an empty star it would claim a + /// favorite that doesn't exist. + private func favoriteTooltip(for peer: MeshPeerRow) -> String { + guard peer.isFavorite else { return Strings.addFavorite } + return peer.isMutualFavorite ? Strings.favoriteMutualTooltip : Strings.favoritePendingTooltip + } + /// One spoken sentence per row: name, how they're reachable, and any /// state badges — the visual row is icon soup for VoiceOver otherwise. private func accessibilityDescription(for peer: MeshPeerRow) -> String {