From 8912fdc5acf68237026c3318119d5b7aa2d8f1a2 Mon Sep 17 00:00:00 2001 From: npub15uvq03jgm52chkxa5x7f2ud28xh9v48287f5fj5fsuu0wqpm63ns8hn2er Date: Fri, 24 Jul 2026 11:30:41 -0700 Subject: [PATCH] fix(mobile/ios): harden push lifecycle failures Co-authored-by: npub15uvq03jgm52chkxa5x7f2ud28xh9v48287f5fj5fsuu0wqpm63ns8hn2er Signed-off-by: npub15uvq03jgm52chkxa5x7f2ud28xh9v48287f5fj5fsuu0wqpm63ns8hn2er --- .../Sources/BuzzPushKit/PushWatermark.swift | 51 +++++++++++++ .../BuzzPushKitTests/PushWatermarkTests.swift | 73 +++++++++++++++++++ .../NotificationService.swift | 35 +++++++-- mobile/lib/shared/auth/auth_provider.dart | 8 +- .../shared/community/community_provider.dart | 50 +++++++++++-- mobile/lib/shared/push/push_bridge.dart | 11 +++ .../test/shared/auth/auth_provider_test.dart | 64 ++++++++++++++++ .../community/community_provider_test.dart | 16 ++++ 8 files changed, 291 insertions(+), 17 deletions(-) create mode 100644 mobile/ios/BuzzPushKit/Sources/BuzzPushKit/PushWatermark.swift create mode 100644 mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/PushWatermarkTests.swift diff --git a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/PushWatermark.swift b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/PushWatermark.swift new file mode 100644 index 0000000000..34367c8a95 --- /dev/null +++ b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/PushWatermark.swift @@ -0,0 +1,51 @@ +import Foundation + +public enum PushWatermark { + /// Nostr permits modest clock drift, but an authenticated author must not + /// be able to pin notification queries arbitrarily far into the future. + public static let allowedFutureSkewSeconds = 300 + public static let keyPrefix = "buzz.push.watermark." + + public static func key(communityID: String) -> String { + keyPrefix + communityID + } + + public static func persistedTimestamp( + eventTimestamp: Int, + now: Int = Int(Date().timeIntervalSince1970), + allowedFutureSkew: Int = allowedFutureSkewSeconds + ) -> Int { + min(eventTimestamp, now + allowedFutureSkew) + } + + public static func isAcceptable( + eventTimestamp: Int, + now: Int = Int(Date().timeIntervalSince1970), + allowedFutureSkew: Int = allowedFutureSkewSeconds + ) -> Bool { + eventTimestamp <= now + allowedFutureSkew + } + + public static func queryTimestamp( + storedWatermark: Int, + now: Int = Int(Date().timeIntervalSince1970) + ) -> Int { + min(storedWatermark, now) + } + + /// Nostr `since` is inclusive. Keeping the watermark itself allows a later + /// event created in the same second to remain queryable. + public static func querySince(watermark: Int) -> Int? { + watermark > 0 ? watermark : nil + } + + public static func staleKeys( + in storedKeys: [String], + activeCommunityIDs: Set + ) -> [String] { + storedKeys.filter { key in + guard key.hasPrefix(keyPrefix) else { return false } + return !activeCommunityIDs.contains(String(key.dropFirst(keyPrefix.count))) + } + } +} diff --git a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/PushWatermarkTests.swift b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/PushWatermarkTests.swift new file mode 100644 index 0000000000..af2558e567 --- /dev/null +++ b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/PushWatermarkTests.swift @@ -0,0 +1,73 @@ +import XCTest +@testable import BuzzPushKit + +final class PushWatermarkTests: XCTestCase { + func testClampsFutureTimestampToAllowedClockSkew() { + XCTAssertEqual( + PushWatermark.persistedTimestamp( + eventTimestamp: 2_000, + now: 1_000, + allowedFutureSkew: 300 + ), + 1_300 + ) + } + + func testPreservesTimestampWithinAllowedClockSkew() { + XCTAssertEqual( + PushWatermark.persistedTimestamp( + eventTimestamp: 1_200, + now: 1_000, + allowedFutureSkew: 300 + ), + 1_200 + ) + } + + func testRejectsEventsBeyondAllowedClockSkew() { + XCTAssertFalse( + PushWatermark.isAcceptable( + eventTimestamp: 1_301, + now: 1_000, + allowedFutureSkew: 300 + ) + ) + XCTAssertTrue( + PushWatermark.isAcceptable( + eventTimestamp: 1_300, + now: 1_000, + allowedFutureSkew: 300 + ) + ) + } + + func testRepairsPoisonedStoredWatermarkToCurrentTime() { + XCTAssertEqual( + PushWatermark.queryTimestamp(storedWatermark: 2_000, now: 1_000), + 1_000 + ) + XCTAssertEqual( + PushWatermark.queryTimestamp(storedWatermark: 900, now: 1_000), + 900 + ) + } + + func testQuerySinceIsInclusiveForSameSecondEvents() { + XCTAssertEqual(PushWatermark.querySince(watermark: 1_000), 1_000) + XCTAssertNil(PushWatermark.querySince(watermark: 0)) + } + + func testFindsOnlyRemovedCommunityWatermarks() { + XCTAssertEqual( + PushWatermark.staleKeys( + in: [ + PushWatermark.key(communityID: "kept"), + PushWatermark.key(communityID: "removed"), + "unrelated", + ], + activeCommunityIDs: ["kept"] + ), + [PushWatermark.key(communityID: "removed")] + ) + } +} diff --git a/mobile/ios/NotificationService/NotificationService.swift b/mobile/ios/NotificationService/NotificationService.swift index 69110e9fbc..b85017ffb4 100644 --- a/mobile/ios/NotificationService/NotificationService.swift +++ b/mobile/ios/NotificationService/NotificationService.swift @@ -77,7 +77,9 @@ final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { } func resolve(completion: @escaping (BuzzPushResolution?) -> Void) { - let communities = loadCommunities().filter { + let loadedCommunities = loadCommunities() + removeStaleWatermarks(activeCommunityIDs: Set(loadedCommunities.map(\.id))) + let communities = loadedCommunities.filter { $0.pubkey?.isEmpty == false && loadPrivateKey(communityID: $0.id) != nil } guard !communities.isEmpty else { completion(nil); return } @@ -96,10 +98,13 @@ final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { group.notify(queue: .global(qos: .userInitiated)) { [weak self] in guard let self else { return } let newest = candidates.max { - $0.1.createdAt == $1.1.createdAt ? $0.1.id < $1.1.id : $0.1.createdAt < $1.1.createdAt + $0.1.createdAt == $1.1.createdAt ? $0.1.id > $1.1.id : $0.1.createdAt < $1.1.createdAt } for candidate in candidates { - self.defaults?.set(candidate.1.createdAt, forKey: self.watermarkKey(candidate.2.id)) + self.defaults?.set( + PushWatermark.persistedTimestamp(eventTimestamp: candidate.1.createdAt), + forKey: PushWatermark.key(communityID: candidate.2.id) + ) } completion(newest?.0) } @@ -113,8 +118,11 @@ final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { completion(nil); return } var filter: [String: Any] = ["kinds": [9, 40002, 45001, 45003], "#p": [pubkey], "limit": 10] - let watermark = defaults?.integer(forKey: watermarkKey(community.id)) ?? 0 - if watermark > 0 { filter["since"] = watermark + 1 } + let watermarkKey = PushWatermark.key(communityID: community.id) + let storedWatermark = defaults?.integer(forKey: watermarkKey) ?? 0 + let watermark = PushWatermark.queryTimestamp(storedWatermark: storedWatermark) + if watermark != storedWatermark { defaults?.set(watermark, forKey: watermarkKey) } + if let since = PushWatermark.querySince(watermark: watermark) { filter["since"] = since } guard let body = try? JSONSerialization.data(withJSONObject: [filter]) else { completion(nil); return } let url = URL(string: "/query", relativeTo: community.relayURL)! var request = URLRequest(url: url) @@ -128,7 +136,12 @@ final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { guard let response = response as? HTTPURLResponse, (200..<300).contains(response.statusCode), let data, let events = try? JSONDecoder().decode([VerifiedNostrEvent].self, from: data) else { completion(nil); return } - completion(Self.decodeResolution(events: events.filter { $0.hasValidIDAndSignature() }, community: community)) + completion(Self.decodeResolution( + events: events.filter { + $0.hasValidIDAndSignature() && PushWatermark.isAcceptable(eventTimestamp: $0.createdAt) + }, + community: community + )) }.resume() } @@ -164,7 +177,15 @@ final class BuzzPushNotificationResolver: BuzzPushNotificationResolving { pubkey.count > 8 ? String(pubkey.prefix(8)) + "…" : pubkey } - private func watermarkKey(_ id: String) -> String { "buzz.push.watermark.\(id)" } + private func removeStaleWatermarks(activeCommunityIDs: Set) { + guard let defaults else { return } + for key in PushWatermark.staleKeys( + in: Array(defaults.dictionaryRepresentation().keys), + activeCommunityIDs: activeCommunityIDs + ) { + defaults.removeObject(forKey: key) + } + } private func loadPrivateKey(communityID: String) -> String? { var query: [String: Any] = [ diff --git a/mobile/lib/shared/auth/auth_provider.dart b/mobile/lib/shared/auth/auth_provider.dart index b612eea1f2..606a0680b6 100644 --- a/mobile/lib/shared/auth/auth_provider.dart +++ b/mobile/lib/shared/auth/auth_provider.dart @@ -24,7 +24,7 @@ class AuthNotifier extends AsyncNotifier { final storage = ref.read(communityStorageProvider); final communities = await storage.loadAll(); if (communities.isEmpty) { - await ref.read(communitySnapshotWriterProvider)(communities); + await syncCommunitySnapshot(ref, communities); return const AuthState(status: AuthStatus.unauthenticated); } @@ -37,7 +37,7 @@ class AuthNotifier extends AsyncNotifier { await storage.saveActiveId(active.id); if (_hasValidNsec(active.nsec)) { - await ref.read(communitySnapshotWriterProvider)(communities); + await syncCommunitySnapshot(ref, communities); return AuthState(status: AuthStatus.authenticated, community: active); } @@ -49,7 +49,7 @@ class AuthNotifier extends AsyncNotifier { } await storage.clearActiveId(); - await ref.read(communitySnapshotWriterProvider)(communities); + await syncCommunitySnapshot(ref, communities); return const AuthState(status: AuthStatus.unauthenticated); } @@ -83,7 +83,7 @@ class AuthNotifier extends AsyncNotifier { // forcing the user back to the pairing screen. Refreshing the complete // snapshot also removes revoked keys from the extension Keychain. final remaining = await storage.loadAll(); - await ref.read(communitySnapshotWriterProvider)(remaining); + await syncCommunitySnapshot(ref, remaining); // Invalidate community providers so other consumers pick up the change. ref.invalidate(communityListProvider); diff --git a/mobile/lib/shared/community/community_provider.dart b/mobile/lib/shared/community/community_provider.dart index 98dd435132..842877c9cd 100644 --- a/mobile/lib/shared/community/community_provider.dart +++ b/mobile/lib/shared/community/community_provider.dart @@ -21,9 +21,47 @@ final communitySnapshotWriterProvider = Provider(( return registerBuzzPushCommunitySnapshot; }); +final _communitySnapshotSyncProvider = Provider<_CommunitySnapshotSync>((ref) { + return _CommunitySnapshotSync(ref.read(communitySnapshotWriterProvider)); +}); + +class _CommunitySnapshotSync { + _CommunitySnapshotSync(this._writer); + + final CommunitySnapshotWriter _writer; + String? _lastSuccessfulSnapshot; + + Future write(List communities) async { + final fingerprint = communities + .map( + (community) => [ + community.id, + community.name, + community.relayUrl, + community.pubkey, + community.nsec, + ].join('\u0000'), + ) + .join('\u0001'); + if (fingerprint == _lastSuccessfulSnapshot) return; + + await _writer(communities); + _lastSuccessfulSnapshot = fingerprint; + } +} + +Future syncCommunitySnapshot(Ref ref, List communities) async { + try { + await ref.read(_communitySnapshotSyncProvider).write(communities); + pushCommunitySnapshotError.value = null; + } catch (error, stackTrace) { + reportPushCommunitySnapshotError(error, stackTrace); + } +} + Future syncStoredCommunitySnapshot(Ref ref) async { final communities = await ref.read(communityStorageProvider).loadAll(); - await ref.read(communitySnapshotWriterProvider)(communities); + await syncCommunitySnapshot(ref, communities); } class CommunityListNotifier extends AsyncNotifier> { @@ -31,7 +69,7 @@ class CommunityListNotifier extends AsyncNotifier> { Future> build() async { final storage = ref.read(communityStorageProvider); final communities = await storage.loadAll(); - await ref.read(communitySnapshotWriterProvider)(communities); + await syncCommunitySnapshot(ref, communities); return communities; } @@ -56,14 +94,14 @@ class CommunityListNotifier extends AsyncNotifier> { final updatedList = [...current]; updatedList[existingIndex] = updated; state = AsyncData(updatedList); - await registerBuzzPushCommunitySnapshot(updatedList); + await syncCommunitySnapshot(ref, updatedList); return existing.id; } await storage.save(community); final updatedList = [...current, community]; state = AsyncData(updatedList); - await registerBuzzPushCommunitySnapshot(updatedList); + await syncCommunitySnapshot(ref, updatedList); return community.id; } @@ -74,7 +112,7 @@ class CommunityListNotifier extends AsyncNotifier> { final current = state.value ?? []; final updatedList = current.where((w) => w.id != id).toList(); state = AsyncData(updatedList); - await registerBuzzPushCommunitySnapshot(updatedList); + await syncCommunitySnapshot(ref, updatedList); // If we removed the active community, switch to another or sign out. final activeId = await storage.loadActiveId(); @@ -115,7 +153,7 @@ class CommunityListNotifier extends AsyncNotifier> { final updatedList = [...current]; updatedList[index] = updated; state = AsyncData(updatedList); - await registerBuzzPushCommunitySnapshot(updatedList); + await syncCommunitySnapshot(ref, updatedList); } } diff --git a/mobile/lib/shared/push/push_bridge.dart b/mobile/lib/shared/push/push_bridge.dart index 7df5566a84..24021af194 100644 --- a/mobile/lib/shared/push/push_bridge.dart +++ b/mobile/lib/shared/push/push_bridge.dart @@ -14,6 +14,17 @@ const _channel = MethodChannel('buzz/push'); final apnsDeviceToken = ValueNotifier(null); final apnsRegistrationError = ValueNotifier(null); +/// Latest failure to export the community snapshot used by the iOS +/// notification service extension. Snapshot export is push enrichment and must +/// never gate authentication or community persistence. +final pushCommunitySnapshotError = ValueNotifier(null); + +void reportPushCommunitySnapshotError(Object error, StackTrace stackTrace) { + pushCommunitySnapshotError.value = error.toString(); + debugPrint('Push community snapshot export failed: $error'); + debugPrintStack(stackTrace: stackTrace); +} + Future registerBuzzPushCommunitySnapshot( List communities, ) async { diff --git a/mobile/test/shared/auth/auth_provider_test.dart b/mobile/test/shared/auth/auth_provider_test.dart index 9acd88a870..bc442ce0e8 100644 --- a/mobile/test/shared/auth/auth_provider_test.dart +++ b/mobile/test/shared/auth/auth_provider_test.dart @@ -1,3 +1,4 @@ +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:nostr/nostr.dart' as nostr; @@ -5,6 +6,7 @@ import 'package:buzz/shared/auth/auth_provider.dart'; import 'package:buzz/shared/community/community.dart'; import 'package:buzz/shared/community/community_provider.dart'; import 'package:buzz/shared/community/community_storage.dart'; +import 'package:buzz/shared/push/push_bridge.dart'; import '../community/community_storage_test.dart'; @@ -122,6 +124,68 @@ void main() { }, ); + test( + 'snapshot export failure does not gate startup authentication', + () async { + final storage = CommunityStorage(secure: FakeSecureStorage()); + final community = Community.create( + name: 'Existing', + relayUrl: 'https://existing.example', + nsec: nostr.Keys.generate().nsec, + ); + await storage.save(community); + await storage.saveActiveId(community.id); + final container = ProviderContainer( + overrides: [ + communityStorageProvider.overrideWithValue(storage), + communitySnapshotWriterProvider.overrideWithValue((_) async { + throw PlatformException( + code: 'save_failed', + message: 'Keychain unavailable', + ); + }), + ], + ); + addTearDown(container.dispose); + + final auth = await container.read(authProvider.future); + + expect(auth.status, AuthStatus.authenticated); + expect(auth.community?.id, community.id); + expect(pushCommunitySnapshotError.value, contains('save_failed')); + }, + ); + + test('snapshot export failure does not gate direct authentication', () async { + final storage = CommunityStorage(secure: FakeSecureStorage()); + final community = Community.create( + name: 'Added', + relayUrl: 'https://added.example', + nsec: nostr.Keys.generate().nsec, + ); + final container = ProviderContainer( + overrides: [ + communityStorageProvider.overrideWithValue(storage), + communitySnapshotWriterProvider.overrideWithValue((_) async { + throw PlatformException( + code: 'save_failed', + message: 'Keychain unavailable', + ); + }), + ], + ); + addTearDown(container.dispose); + + await container + .read(authProvider.notifier) + .authenticateWithCommunity(community); + + final auth = await container.read(authProvider.future); + expect(auth.status, AuthStatus.authenticated); + expect(auth.community?.id, community.id); + expect((await storage.loadAll()).single.id, community.id); + }); + test('falls through to the next valid saved community', () async { final storage = CommunityStorage(secure: FakeSecureStorage()); final invalid = Community.create( diff --git a/mobile/test/shared/community/community_provider_test.dart b/mobile/test/shared/community/community_provider_test.dart index b599093618..bd828c083a 100644 --- a/mobile/test/shared/community/community_provider_test.dart +++ b/mobile/test/shared/community/community_provider_test.dart @@ -59,6 +59,22 @@ void main() { expect(fakeSecure['buzz_workspaces'], isNull); }); + test('skips an unchanged snapshot after provider invalidation', () async { + final community = Community.create( + name: 'Stored', + relayUrl: 'https://stored.example.com', + nsec: nostr.Keys.generate().nsec, + ); + await communityStorage.save(community); + container = createContainer(); + + await container.read(communityListProvider.future); + container.invalidate(communityListProvider); + await container.read(communityListProvider.future); + + expect(snapshots, hasLength(1)); + }); + test('addCommunity adds to list', () async { container = createContainer(); await container.read(communityListProvider.future);