diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index 715a7703..eda16d4b 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -171,6 +171,11 @@ final class AppRuntime: ObservableObject { Self.isDirectChatRestorable( $0, favorites: .shared, + hasStoredCryptographicIdentity: { + !chatViewModel.identityManager + .getCryptoIdentitiesByPeerIDPrefix($0.toShort()) + .isEmpty + }, isPeerBlocked: { chatViewModel.isPeerBlocked($0) } ) } @@ -178,13 +183,18 @@ final class AppRuntime: ObservableObject { var didOpenDirectChat = false if case .restoredDirectChat(let peerID) = presentation { // `startPrivateChat`'s gate (ChatPeerIdentityCoordinator) rejects a - // now-blocked or non-mutual-favorite peer by emitting a system - // message and returning WITHOUT opening the chat. At launch that - // message would land in the current (public mesh) timeline, so pass + // now-blocked peer by emitting a system message and returning + // WITHOUT opening the chat. At launch that message would land in + // the current (public mesh) timeline, so pass // `suppressSystemMessages: true` — the reject stays silent and we // detect it via `selectedPrivateChatPeer`, which is only set on the - // success path. `isDirectChatRestorable` already screens for the - // same conditions; this is the belt-and-suspenders second line. + // success path. + // + // Not a second line of defence any more. Post-#1415 that gate + // screens only self, group and blocked, so it catches nothing + // `isDirectChatRestorable` has not already caught. It is kept for + // the narrow race where the peer is blocked between the predicate + // and this call, and for the silent-failure detection above. chatViewModel.startPrivateChat(with: peerID, suppressSystemMessages: true) didOpenDirectChat = chatViewModel.selectedPrivateChatPeer == peerID } @@ -201,24 +211,54 @@ final class AppRuntime: ObservableObject { /// presence (mesh discovery is async, so no peer is connected yet). A /// syntactically valid `PeerID` is NOT sufficient: an unknown peer would /// otherwise fall straight through `startPrivateChat` into an empty phantom - /// DM. Mirrors the open-path gate - /// (`ChatPeerIdentityCoordinator.startPrivateChat`): restorable iff the peer - /// is a MUTUAL favorite (we favorite them AND they favorite us) and NOT - /// blocked. The gate's third relaxation term, `isConnected`, is always false - /// at launch, so it drops out of the launch-effective predicate. A geohash/ - /// Nostr DM is *not* special-cased: its full Nostr key is rebuilt only from - /// inbound ephemeral events, so at launch a restored `nostr_` id cannot - /// resolve and would open an unsendable phantom unless it is also a mutual - /// favorite. Favorites are keychain-backed and keyed by stable Noise public - /// key, so this is presence-independent and pure, hence unit-testable. + /// DM. + /// + /// Restorable iff the peer is NOT blocked and we hold durable evidence the + /// conversation is real: either side has favorited the other, or we have a + /// stored cryptographic identity for them. + /// + /// This tracks `ChatPeerIdentityCoordinator.startPrivateChat`, which #1415 + /// relaxed — it no longer requires a mutual favorite, on the grounds that + /// store-and-forward (couriers, bridge drops, retained outbox) needs only + /// the recipient's noise key, so "the router decides what delivery looks + /// like, not chat entry". Keeping the old mutual-favorite rule here would + /// have made launch-restore stricter than chat entry: a one-way-favorite or + /// merely-known peer whose DM is perfectly sendable would fail to restore + /// and drop the user on the conversation list instead. + /// + /// It cannot simply defer to that gate, though. Post-#1415 the open path + /// screens only self, group and blocked, so this predicate is the sole + /// defence against restoring into a phantom DM, and it has to hold the line + /// on its own. + /// + /// The terms are chosen for durability at launch. Favorites are + /// keychain-backed; stored cryptographic identities are on disk. The outbox + /// and live Noise session state are deliberately NOT consulted — the outbox + /// defers loading until protected data is available and session state is + /// in-memory, so both read empty at launch regardless of the truth. + /// + /// Geohash/Nostr DMs stay excluded: a `nostr_` id's full Nostr key is + /// rebuilt only from inbound ephemeral events, so at launch it cannot + /// resolve, and `startPrivateChat` skips the handshake for geoDMs — a + /// phantom would open with no error at all. Their one durable anchor is a + /// favorite record, so for them the favorite terms decide and the identity + /// term is refused outright. static func isDirectChatRestorable( _ peerID: PeerID, isPeerFavorited: (PeerID) -> Bool, theyFavoritedUs: (PeerID) -> Bool, + hasStoredCryptographicIdentity: (PeerID) -> Bool, isPeerBlocked: (PeerID) -> Bool ) -> Bool { + // Blocked stays a veto, never one term among several. guard !isPeerBlocked(peerID) else { return false } - return isPeerFavorited(peerID) && theyFavoritedUs(peerID) + if isPeerFavorited(peerID) || theyFavoritedUs(peerID) { return true } + // Explicit rather than incidental: a `nostr_` id does satisfy + // `isShort` (the prefix is not part of the length check), and today it + // misses only because the identity lookup re-attaches that prefix and + // no hex fingerprint starts with it. That is luck, not a rule. + guard !peerID.isGeoDM, !peerID.isGeoChat else { return false } + return hasStoredCryptographicIdentity(peerID) } /// Production wiring of `isDirectChatRestorable`, extracted so the real @@ -231,9 +271,19 @@ final class AppRuntime: ObservableObject { /// silently fail to restore. The block lookup mirrors the open-path gate's /// `unifiedIsBlocked` (fingerprint-resolved, so it works for offline /// favorites). + /// + /// The identity lookup is passed in rather than reached for: it lives on + /// the injected `SecureIdentityStateManagerProtocol`, so tests stub it the + /// same way they stub the favorites service instead of sharing + /// process-wide state. It needs the same `toShort()` normalization — + /// `getCryptoIdentitiesByPeerIDPrefix` guards on `isShort` and returns an + /// empty result otherwise, so a 64-hex id would read as "no identity" + /// rather than as an error. Synchronous and disk-backed, so it is safe on + /// the launch path. static func isDirectChatRestorable( _ peerID: PeerID, favorites: FavoritesPersistenceService, + hasStoredCryptographicIdentity: (PeerID) -> Bool, isPeerBlocked: (PeerID) -> Bool ) -> Bool { isDirectChatRestorable( @@ -244,6 +294,7 @@ final class AppRuntime: ObservableObject { theyFavoritedUs: { favorites.getFavoriteStatus(forPeerID: $0.toShort())?.theyFavoritedUs ?? false }, + hasStoredCryptographicIdentity: hasStoredCryptographicIdentity, isPeerBlocked: isPeerBlocked ) } diff --git a/bitchat/Localizable.xcstrings b/bitchat/Localizable.xcstrings index f7141e82..2d2759a3 100644 --- a/bitchat/Localizable.xcstrings +++ b/bitchat/Localizable.xcstrings @@ -37946,6 +37946,12 @@ "value" : "público · cerca" } }, + "fa" : { + "stringUnit" : { + "state" : "translated", + "value" : "عمومی · اطراف شما" + } + }, "fil" : { "stringUnit" : { "state" : "translated", @@ -38126,6 +38132,12 @@ "value" : "público, cerca" } }, + "fa" : { + "stringUnit" : { + "state" : "translated", + "value" : "عمومی، اطراف شما" + } + }, "fil" : { "stringUnit" : { "state" : "translated", diff --git a/bitchat/ViewModels/ChatViewModel.swift b/bitchat/ViewModels/ChatViewModel.swift index 9ab6b66b..2c89dd22 100644 --- a/bitchat/ViewModels/ChatViewModel.swift +++ b/bitchat/ViewModels/ChatViewModel.swift @@ -1502,10 +1502,11 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage } /// #1064 launch-restore variant of `startPrivateChat`. When - /// `suppressSystemMessages` is `true`, a gate rejection (blocked / - /// non-mutual favorite) no-ops silently instead of emitting a system message - /// into the current (public mesh) timeline, so a rejected DM restore falls - /// back to the conversation list cleanly. Kept as a distinct non-defaulted + /// `suppressSystemMessages` is `true`, a gate rejection no-ops silently + /// instead of emitting a system message into the current (public mesh) + /// timeline, so a rejected DM restore falls back to the conversation list + /// cleanly. Since #1415 removed the mutual-favorite gate that is exactly + /// one message — the blocked one. Kept as a distinct non-defaulted /// overload — a defaulted extra parameter would not satisfy the context /// protocols above and a default would make the plain call ambiguous. @MainActor diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index 9f4523bd..ab27e6bd 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -210,6 +210,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { peerID, isPeerFavorited: { _ in false }, theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) @@ -225,21 +226,104 @@ final class ConversationStoreLastActiveTests: XCTestCase { peerID, isPeerFavorited: { _ in true }, theyFavoritedUs: { _ in true }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } - func test_oneWayFavoritePeerIsNotRestorable() { - // We favorite them but they do NOT favorite us: the open-path gate - // rejects this at launch (isConnected is false), so auto-restoring it - // would inject a "requires favorite" system message into the public - // timeline. The predicate must refuse it up front. - XCTAssertFalse( + func test_oneWayFavoritePeerIsRestorable() { + // INVERTED by #1415, deliberately kept as the record of that change. + // This used to be false: the open path required a mutual favorite, so + // restoring a one-way favorite would have injected a "requires + // favorite" system message into the public timeline. #1415 removed that + // gate — store-and-forward needs only the recipient's noise key — so a + // one-way favorite is now perfectly sendable, and refusing to restore + // it would make launch stricter than chat entry. + XCTAssertTrue( AppRuntime.isDirectChatRestorable( peerID, isPeerFavorited: { _ in true }, theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_peerWhoFavoritedUsIsRestorable() { + // The other direction of the same relaxation. + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in true }, + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_establishedNonFavoritePeerIsRestorable() { + // No favorite either way, but we hold a stored cryptographic identity — + // durable, on disk, and enough for the router to deliver via courier or + // the retained outbox. Restoring it is not a phantom. + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in true }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_blockedEstablishedPeerIsNotRestorable() { + // Blocked is a veto, not one term among several: a stored identity must + // not buy its way past the block the way it passes the favorite terms. + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + peerID, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in true }, + isPeerBlocked: { _ in true } + ) + ) + } + + func test_geoDMWithStoredIdentityIsNotRestorable() { + // The identity term must NOT extend to geohash DMs. A `nostr_` id's + // full Nostr key is rebuilt only from inbound ephemeral events, and + // startPrivateChat skips the handshake for geoDMs, so a phantom would + // open with no error at all. Today the lookup would miss anyway (the + // id's prefix is re-attached and no hex fingerprint starts with it), + // which is luck — this pins the refusal so a change to that lookup + // cannot quietly turn geoDM phantoms back on. + let geoDMPeer = PeerID(str: "nostr_0123456789abcdef") + XCTAssertTrue(geoDMPeer.isGeoDM) + XCTAssertFalse( + AppRuntime.isDirectChatRestorable( + geoDMPeer, + isPeerFavorited: { _ in false }, + theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in true }, + isPeerBlocked: { _ in false } + ) + ) + } + + func test_geoDMWithFavoriteIsRestorable() { + // A favorite record IS a geoDM's durable anchor (it carries + // peerNostrPublicKey), so the favorite terms still admit one. + let geoDMPeer = PeerID(str: "nostr_0123456789abcdef") + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + geoDMPeer, + isPeerFavorited: { _ in true }, + theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) @@ -253,6 +337,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { peerID, isPeerFavorited: { _ in true }, theyFavoritedUs: { _ in true }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in true } ) ) @@ -270,6 +355,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { geoDMPeer, isPeerFavorited: { _ in false }, theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) @@ -289,6 +375,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { $0, isPeerFavorited: { _ in false }, theyFavoritedUs: { _ in false }, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) } @@ -316,15 +403,17 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } - func test_production_oneWayFavoriteIsNotRestorable() { - // We favorite them but they never favorited us: not mutual, so the - // open-path gate would reject it at launch. The production resolver must - // refuse it rather than auto-open a gated DM. + func test_production_oneWayFavoriteIsRestorable() { + // INVERTED by #1415, through the real favorites wiring. This used to + // refuse a non-mutual favorite because the open-path gate would have + // rejected it at launch; that gate is gone, so refusing here would make + // launch stricter than chat entry for a DM the router can deliver. let favorites = FavoritesPersistenceService(keychain: MockKeychain()) let noiseKey = Data((0..<32).map(UInt8.init)) favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") @@ -332,10 +421,11 @@ final class ConversationStoreLastActiveTests: XCTestCase { let fullHexPeer = PeerID(str: noiseKey.hexEncodedString()) XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs) - XCTAssertFalse( + XCTAssertTrue( AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) @@ -355,6 +445,7 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in true } ) ) @@ -370,17 +461,31 @@ final class ConversationStoreLastActiveTests: XCTestCase { AppRuntime.isDirectChatRestorable( peerID, favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, isPeerBlocked: { _ in false } ) ) } - func test_production_unfavoritedPeerWhoStillFavoritesUsIsNotRestorable() { + func test_production_unfavoritedPeerWhoStillFavoritesUsIsRestorable() { + // INVERTED by #1415, and the inversion worth arguing about. + // // removeFavorite RETAINS a record (isFavorite: false, theyFavoritedUs: - // true) when the peer still favorites us. The resolver must key on - // isFavorite, not mere record existence — otherwise a DM to a peer we - // deliberately unfavorited reopens on restart, contradicting the - // "is a persisted favorite" contract. Regression for Codex P2 review. + // true) when the peer still favorites us. This previously refused to + // restore, so that a DM to a peer we deliberately unfavorited would not + // reopen on restart — the "is a persisted favorite" contract, added for + // an earlier Codex review. + // + // #1415 dissolved that contract: chat entry no longer consults + // favorites at all, so this conversation is openable by hand and + // sendable through the router. Refusing to restore it would single out + // launch for a stricter rule than every other way in. + // + // Note the protection is largely moot in practice regardless: any peer + // we have actually corresponded with has a stored cryptographic + // identity, which admits them through the identity term whatever the + // favorite record says. Unfavoriting is not blocking — blocking still + // refuses, and that is the control for "do not reopen this". let favorites = FavoritesPersistenceService(keychain: MockKeychain()) let noiseKey = Data((0..<32).map(UInt8.init)) favorites.addFavorite(peerNoisePublicKey: noiseKey, peerNickname: "Alice") @@ -391,11 +496,24 @@ final class ConversationStoreLastActiveTests: XCTestCase { // The record survives (they still favorite us) but isFavorite is false. XCTAssertNotNil(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())) XCTAssertFalse(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.isFavorite) + XCTAssertTrue(favorites.getFavoriteStatus(forPeerID: fullHexPeer.toShort())!.theyFavoritedUs) + XCTAssertTrue( + AppRuntime.isDirectChatRestorable( + fullHexPeer, + favorites: favorites, + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in false } + ) + ) + + // Blocking is the control that still refuses, with everything else + // about this peer unchanged. XCTAssertFalse( AppRuntime.isDirectChatRestorable( fullHexPeer, favorites: favorites, - isPeerBlocked: { _ in false } + hasStoredCryptographicIdentity: { _ in false }, + isPeerBlocked: { _ in true } ) ) }