From d530921919fb30a08577ac7f908c896ecf9249c3 Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 11 Aug 2026 10:14:08 +0200 Subject: [PATCH] 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") }