diff --git a/bitchat/Nostr/NostrProtocol.swift b/bitchat/Nostr/NostrProtocol.swift index adf0f997..1ff4947d 100644 --- a/bitchat/Nostr/NostrProtocol.swift +++ b/bitchat/Nostr/NostrProtocol.swift @@ -51,6 +51,10 @@ struct NostrProtocol { static let maximumPrivateEnvelopeCiphertextBytes = 64 * 1024 /// Bound the inner authenticated message JSON before allocation/parsing. + /// This is intentionally an envelope limit, not the generic composer + /// limit. Raising it alone would not make larger private-message packets + /// compatible with released readers; callers must surface a rejected + /// packet or envelope as a failed send. static let maximumPrivateEnvelopePlaintextBytes = 32 * 1024 /// The outer authenticated seal JSON contains a Base64-encoded encrypted diff --git a/bitchat/Services/NostrTransport.swift b/bitchat/Services/NostrTransport.swift index e6eced74..71f6befb 100644 --- a/bitchat/Services/NostrTransport.swift +++ b/bitchat/Services/NostrTransport.swift @@ -323,6 +323,11 @@ final class NostrTransport: Transport, @unchecked Sendable { SecureLogger.debug("NostrTransport: preparing PM to \(recipientNpub.prefix(16))… id=\(messageID.prefix(8))…", category: .session) guard let embedded = NostrEmbeddedBitChat.encodePMForNostr(content: content, messageID: messageID, recipientPeerID: peerID, senderPeerID: senderPeerID) else { SecureLogger.error("NostrTransport: failed to embed PM packet", category: .session) + handlePrivateEnvelopeFailure( + events: [], + registerPending: false, + policy: .userMessage(messageID: messageID) + ) return } sendPrivateEnvelope( @@ -387,22 +392,48 @@ extension NostrTransport { } // MARK: Geohash DMs (per-geohash identity) - func sendPrivateMessageGeohash(content: String, toRecipientHex recipientHex: String, from identity: NostrIdentity, messageID: String) { - Task { @MainActor in - guard !recipientHex.isEmpty else { return } - SecureLogger.debug("GeoDM: send PM mid=\(messageID.prefix(8))…", category: .session) - guard let embedded = NostrEmbeddedBitChat.encodePMForNostrNoRecipient(content: content, messageID: messageID, senderPeerID: senderPeerID) else { - SecureLogger.error("NostrTransport: failed to embed geohash PM packet", category: .session) - return - } - sendPrivateEnvelope( - content: embedded, - recipientHex: recipientHex, - senderIdentity: identity, - registerPending: true, - failurePolicy: .userMessage(messageID: messageID) + /// Returns true only when the complete migration pair entered the relay + /// delivery queue. GeoDM callers use this synchronous admission result + /// before showing "sent"; deterministic packet/envelope failures must not + /// be hidden behind an unobserved MainActor task. + @MainActor + @discardableResult + func sendPrivateMessageGeohash( + content: String, + toRecipientHex recipientHex: String, + from identity: NostrIdentity, + messageID: String + ) -> Bool { + let failurePolicy = PrivateEnvelopeFailurePolicy.userMessage(messageID: messageID) + guard !recipientHex.isEmpty else { + handlePrivateEnvelopeFailure( + events: [], + registerPending: false, + policy: failurePolicy ) + return false } + SecureLogger.debug("GeoDM: send PM mid=\(messageID.prefix(8))…", category: .session) + guard let embedded = NostrEmbeddedBitChat.encodePMForNostrNoRecipient( + content: content, + messageID: messageID, + senderPeerID: senderPeerID + ) else { + SecureLogger.error("NostrTransport: failed to embed geohash PM packet", category: .session) + handlePrivateEnvelopeFailure( + events: [], + registerPending: false, + policy: failurePolicy + ) + return false + } + return sendPrivateEnvelope( + content: embedded, + recipientHex: recipientHex, + senderIdentity: identity, + registerPending: true, + failurePolicy: failurePolicy + ) } } @@ -424,20 +455,35 @@ extension NostrTransport { /// Creates and sends a BitChat private-envelope event over Nostr. @MainActor + @discardableResult private func sendPrivateEnvelope( content: String, recipientHex: String, senderIdentity: NostrIdentity, registerPending: Bool = false, failurePolicy: PrivateEnvelopeFailurePolicy - ) { - guard let events = try? NostrProtocol.createPrivateEnvelopePublicationBatch( - content: content, - recipientPubkey: recipientHex, - senderIdentity: senderIdentity - ) else { - SecureLogger.error("NostrTransport: failed to build Nostr private-envelope batch", category: .session) - return + ) -> Bool { + let events: [NostrEvent] + do { + events = try NostrProtocol.createPrivateEnvelopePublicationBatch( + content: content, + recipientPubkey: recipientHex, + senderIdentity: senderIdentity + ) + } catch { + SecureLogger.error( + "NostrTransport: failed to build Nostr private-envelope batch: \(error)", + category: .session + ) + // Construction failures are deterministic. User-authored messages + // must become visibly failed; control payloads have no valid event + // pair to retain and retry. + handlePrivateEnvelopeFailure( + events: [], + registerPending: false, + policy: failurePolicy + ) + return false } let accepted = dependencies.sendPrivateEnvelopeBatch(events) { [self] in handlePrivateEnvelopeFailure( @@ -456,9 +502,10 @@ extension NostrTransport { registerPending: registerPending, policy: failurePolicy ) - return + return false } registerPendingPrivateEnvelopesIfNeeded(events, registerPending: registerPending) + return true } @MainActor @@ -477,6 +524,10 @@ extension NostrTransport { )) )) case .retry(let retryKey): + // A deterministic packet/envelope construction failure has no + // events to retry. Only relay admission/delivery failures reach + // this branch with the complete atomic pair. + guard !events.isEmpty else { return } envelopeRetryQueue.enqueue( key: retryKey, events: events, diff --git a/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift b/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift index b888f988..3e0cb952 100644 --- a/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift +++ b/bitchat/ViewModels/ChatPrivateConversationCoordinator.swift @@ -86,7 +86,13 @@ protocol ChatPrivateConversationContext: AnyObject { @discardableResult func routeReadReceipt(_ receipt: ReadReceipt, to peerID: PeerID) -> Bool func sendMeshReadReceipt(_ receipt: ReadReceipt, to peerID: PeerID) - func sendGeohashPrivateMessage(_ content: String, toRecipientHex recipientHex: String, from identity: NostrIdentity, messageID: String) + @discardableResult + func sendGeohashPrivateMessage( + _ content: String, + toRecipientHex recipientHex: String, + from identity: NostrIdentity, + messageID: String + ) -> Bool func sendGeohashDeliveryAck(for messageID: String, toRecipientHex recipientHex: String, from identity: NostrIdentity) func sendGeohashReadReceipt(_ messageID: String, toRecipientHex recipientHex: String, from identity: NostrIdentity) @@ -174,7 +180,13 @@ extension ChatViewModel: ChatPrivateConversationContext { meshService.sendReadReceipt(receipt, to: peerID) } - func sendGeohashPrivateMessage(_ content: String, toRecipientHex recipientHex: String, from identity: NostrIdentity, messageID: String) { + @discardableResult + func sendGeohashPrivateMessage( + _ content: String, + toRecipientHex recipientHex: String, + from identity: NostrIdentity, + messageID: String + ) -> Bool { makeGeohashNostrTransport().sendPrivateMessageGeohash( content: content, toRecipientHex: recipientHex, @@ -399,13 +411,21 @@ final class ChatPrivateConversationCoordinator { "GeoDM: local send mid=\(messageID.prefix(8))… to=\(recipientHex.prefix(8))… conv=\(peerID)", category: .session ) - context.sendGeohashPrivateMessage( + let accepted = context.sendGeohashPrivateMessage( content, toRecipientHex: recipientHex, from: identity, messageID: messageID ) - context.setPrivateDeliveryStatus(.sent, forMessageID: messageID, peerID: peerID) + let status: DeliveryStatus = accepted + ? .sent + : .failed( + reason: String( + localized: "content.delivery.reason.not_delivered", + comment: "Failure reason shown when a private message could not enter the relay delivery queue" + ) + ) + context.setPrivateDeliveryStatus(status, forMessageID: messageID, peerID: peerID) } catch { context.setPrivateDeliveryStatus( .failed( diff --git a/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift b/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift index 5fad9fd0..411b3b72 100644 --- a/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift +++ b/bitchatTests/ChatPrivateConversationCoordinatorContextTests.swift @@ -176,6 +176,7 @@ private final class MockChatPrivateConversationContext: ChatPrivateConversationC private(set) var geoPrivateMessages: [(content: String, recipientHex: String, messageID: String)] = [] private(set) var geoDeliveryAcks: [(messageID: String, recipientHex: String)] = [] private(set) var geoReadReceipts: [(messageID: String, recipientHex: String)] = [] + var geohashPrivateMessageAccepted = true func routePrivateMessage(_ content: String, to peerID: PeerID, recipientNickname: String, messageID: String) { routedPrivateMessages.append((content, peerID, messageID)) @@ -191,8 +192,14 @@ private final class MockChatPrivateConversationContext: ChatPrivateConversationC meshReadReceipts.append((receipt.originalMessageID, peerID)) } - func sendGeohashPrivateMessage(_ content: String, toRecipientHex recipientHex: String, from identity: NostrIdentity, messageID: String) { + func sendGeohashPrivateMessage( + _ content: String, + toRecipientHex recipientHex: String, + from identity: NostrIdentity, + messageID: String + ) -> Bool { geoPrivateMessages.append((content, recipientHex, messageID)) + return geohashPrivateMessageAccepted } func sendGeohashDeliveryAck(for messageID: String, toRecipientHex recipientHex: String, from identity: NostrIdentity) { @@ -279,6 +286,11 @@ private func isRead(_ status: DeliveryStatus?, by expected: String) -> Bool { return false } +private func isFailed(_ status: DeliveryStatus?) -> Bool { + if case .failed = status { return true } + return false +} + private func makeFavoriteRelationship( noiseKey: Data, nostrPublicKey: String? = nil, @@ -412,6 +424,25 @@ struct ChatPrivateConversationCoordinatorContextTests { #expect(context.privateChats[convKey]?.count == 1) } + @Test @MainActor + func sendGeohashDM_rejectedAdmissionNeverBecomesSent() async { + let context = MockChatPrivateConversationContext() + let coordinator = ChatPrivateConversationCoordinator(context: context) + let recipientHex = String(repeating: "33", count: 32) + let peerID = PeerID(nostr_: recipientHex) + context.activeChannel = .location( + GeohashChannel(level: .city, geohash: "u4pruy") + ) + context.nostrKeyMapping[peerID] = recipientHex + context.geohashPrivateMessageAccepted = false + + coordinator.sendGeohashDM("rejected", to: peerID) + + #expect(context.geoPrivateMessages.map(\.content) == ["rejected"]) + #expect(context.privateChats[peerID]?.count == 1) + #expect(isFailed(context.privateChats[peerID]?.first?.deliveryStatus)) + } + @Test @MainActor func handleViewingThisChat_clearsUnreadAndSendsRoutedReadReceiptOnce() async { let context = MockChatPrivateConversationContext() diff --git a/bitchatTests/NostrProtocolTests.swift b/bitchatTests/NostrProtocolTests.swift index 7601872d..a64f7a07 100644 --- a/bitchatTests/NostrProtocolTests.swift +++ b/bitchatTests/NostrProtocolTests.swift @@ -399,6 +399,27 @@ struct NostrProtocolTests { } } + @Test func privateEnvelopePublicationBatchRejectsMaximumComposerPayload() throws { + let sender = try NostrIdentity.generate() + let recipient = try NostrIdentity.generate() + let composerMaximum = String( + repeating: "x", + count: InputValidator.Limits.maxMessageLength + ) + + #expect( + composerMaximum.utf8.count + > NostrProtocol.maximumPrivateEnvelopePlaintextBytes + ) + expectInvalidCiphertext { + _ = try NostrProtocol.createPrivateEnvelopePublicationBatch( + content: composerMaximum, + recipientPubkey: recipient.publicKeyHex, + senderIdentity: sender + ) + } + } + @Test func privateEnvelopeRejectsOversizedNestedJSONBeforeParsing() { let oversizedJSON = String( repeating: "{", diff --git a/bitchatTests/Services/NostrTransportTests.swift b/bitchatTests/Services/NostrTransportTests.swift index d3378c10..e823e3cf 100644 --- a/bitchatTests/Services/NostrTransportTests.swift +++ b/bitchatTests/Services/NostrTransportTests.swift @@ -302,6 +302,128 @@ struct NostrTransportTests { #expect(reported) } + @Test("Envelope construction failure rejects visibly without publishing") + @MainActor + func envelopeConstructionFailureRejectsVisibly() async throws { + let keychain = MockKeychain() + let idBridge = NostrIdentityBridge(keychain: keychain) + let sender = try NostrIdentity.generate() + let eventProbe = NostrTransportEventProbe() + let publicationProbe = NostrTransportProbe() + let transport = NostrTransport( + keychain: keychain, + idBridge: idBridge, + dependencies: makeDependencies( + currentIdentity: { sender }, + sendPrivateEnvelopeBatch: { events, _ in + publicationProbe.record(batch: events) + } + ) + ) + transport.senderPeerID = PeerID(str: "0123456789abcdef") + transport.eventDelegate = eventProbe + + // Non-empty but invalid hex reaches envelope construction after the + // embedded BitChat packet succeeds, reproducing the throwing batch + // builder path without allocating an oversized test fixture. + let accepted = transport.sendPrivateMessageGeohash( + content: "must fail before sent", + toRecipientHex: "not-a-hex-public-key", + from: sender, + messageID: "build-reject" + ) + + #expect(!accepted) + #expect(publicationProbe.sentEvents.isEmpty) + #expect(eventProbe.failedMessageIDs == ["build-reject"]) + } + + @Test("Unencodable user message rejects visibly without publishing") + @MainActor + func unencodableUserMessageRejectsVisibly() async throws { + let keychain = MockKeychain() + let idBridge = NostrIdentityBridge(keychain: keychain) + let sender = try NostrIdentity.generate() + let recipient = try NostrIdentity.generate() + let eventProbe = NostrTransportEventProbe() + let publicationProbe = NostrTransportProbe() + let transport = NostrTransport( + keychain: keychain, + idBridge: idBridge, + dependencies: makeDependencies( + currentIdentity: { sender }, + sendPrivateEnvelopeBatch: { events, _ in + publicationProbe.record(batch: events) + } + ) + ) + transport.senderPeerID = PeerID(str: "0123456789abcdef") + transport.eventDelegate = eventProbe + + // PrivateMessagePacket uses the deployed UInt8 content length. Do not + // create a larger wire shape that released clients cannot decode. + let accepted = transport.sendPrivateMessageGeohash( + content: String(repeating: "x", count: 256), + toRecipientHex: recipient.publicKeyHex, + from: sender, + messageID: "packet-reject" + ) + + #expect(!accepted) + #expect(publicationProbe.sentEvents.isEmpty) + #expect(eventProbe.failedMessageIDs == ["packet-reject"]) + } + + @Test("Unencodable direct message emits a visible failure") + @MainActor + func unencodableDirectMessageEmitsVisibleFailure() async throws { + let keychain = MockKeychain() + let idBridge = NostrIdentityBridge(keychain: keychain) + let sender = try NostrIdentity.generate() + let recipient = try NostrIdentity.generate() + let noiseKey = Data((224..<256).map(UInt8.init)) + let peerID = PeerID(hexData: noiseKey) + let relationship = makeRelationship( + peerNoisePublicKey: noiseKey, + peerNostrPublicKey: recipient.npub, + peerNickname: "Oversized peer" + ) + let eventProbe = NostrTransportEventProbe() + let publicationProbe = NostrTransportProbe() + let transport = NostrTransport( + keychain: keychain, + idBridge: idBridge, + dependencies: makeDependencies( + favoriteStatusForNoiseKey: { + $0 == noiseKey ? relationship : nil + }, + currentIdentity: { sender }, + sendPrivateEnvelopeBatch: { events, _ in + publicationProbe.record(batch: events) + } + ) + ) + transport.senderPeerID = PeerID(str: "0123456789abcdef") + transport.eventDelegate = eventProbe + + transport.sendPrivateMessage( + String(repeating: "x", count: 256), + to: peerID, + recipientNickname: "Oversized peer", + messageID: "direct-packet-reject" + ) + + let failed = await TestHelpers.waitUntil( + { + eventProbe.failedMessageIDs + == ["direct-packet-reject"] + }, + timeout: 5.0 + ) + #expect(failed) + #expect(publicationProbe.sentEvents.isEmpty) + } + @Test("Rejected favorite notification retains and retries the exact pair") @MainActor func rejectedFavoriteNotificationRetriesExactPair() async throws {