From 041956ea446c851dc0b0d2aacf35ced89f238616 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Wed, 4 Nov 2026 13:49:54 -0800 Subject: [PATCH] fix(ios): saturate the push dedupe cutoff so extreme timestamps can't crash (#9010) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context Follow-up to [#8973](https://github.com/block/buzz/pull/8973), from Codex's [review](https://github.com/block/buzz/pull/8973#discussion_r4458876285) of its final head. The push dedupe cutoff subtracts the 15 minute reorder window from the newest shown time. A signed event can carry any `created_at`. One within 900 seconds of `Int.min` made that subtraction overflow and trap, crashing the Notification Service Extension. The claim was never saved, so every later push crashed the same way while the relay kept returning that event. The relay's own drift check normally rejects such timestamps, but the extension shouldn't depend on that. ## Change The cutoff now uses saturating subtraction (`BuzzNotificationCache.presentationCutoff(after:)`), in both the claim check and retention pruning. ## Validation - New test `testExtremeTimestampsDoNotCrashTheClaim` claims messages dated `Int.min`, `Int.min + 1`, now, and `Int.max`. It checks that each behaves like any other message and nothing crashes. With the plain subtraction restored, it crashes with signal 5 (overflow trap). - Full BuzzPushKit suite: 108 XCTest and 66 Swift Testing tests passed. - Automated app test: not run. This is a two-line arithmetic change inside the extension's cache. The live APNs run on [#8973](https://github.com/block/buzz/pull/8973) covers the flow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Tom Brow Co-authored-by: Tom Brow Co-authored-by: studio Co-authored-by: Claude Opus 5.5 --- .../BuzzPushKit/BuzzNotificationCache.swift | 12 ++++++++-- .../BuzzPushNotificationResolverTests.swift | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzNotificationCache.swift b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzNotificationCache.swift index 33ca6a9484..d4bf33c70d 100644 --- a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzNotificationCache.swift +++ b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzNotificationCache.swift @@ -102,6 +102,14 @@ public final class BuzzNotificationCache: @unchecked Sendable { /// message can carry an earlier time than one already shown. Anything older is /// history, not the message a push is for. public static let presentationReorderWindow = 15 * 60 + + /// The oldest time still within ``presentationReorderWindow`` of `newest`. Saturates, + /// because a signed event can carry any `created_at` and an overflow would crash the + /// extension on every push. + static func presentationCutoff(after newest: Int) -> Int { + let (cutoff, overflow) = newest.subtractingReportingOverflow(presentationReorderWindow) + return overflow ? .min : cutoff + } /// Retention keeps up to this many entries by `watermark` and as many by `createdAt`, /// so neither future-dated entries nor a burst of recent ones can evict the other. static let maximumPresentedMessages = 256 @@ -309,7 +317,7 @@ public final class BuzzNotificationCache: @unchecked Sendable { return index case .success(let entries): guard !entries.contains(where: { $0.id == candidate.event.id }) else { continue } - if let newestShown, watermark < newestShown - Self.presentationReorderWindow { + if let newestShown, watermark < Self.presentationCutoff(after: newestShown) { continue } } @@ -409,7 +417,7 @@ public final class BuzzNotificationCache: @unchecked Sendable { if let presented = state.presented, let newest = presented.map(\.watermark).max() { // The age check rejects anything older than the window, so those entries can // go. A future-dated entry stays while the relay may still rank it first. - let cutoff = newest - Self.presentationReorderWindow + let cutoff = Self.presentationCutoff(after: newest) // Entries are newest claim first, so ties keep the most recent claims. let current = Array(presented.filter { $0.createdAt >= cutoff }.enumerated()) func newestIDs(by key: (PresentedMessage) -> Int) -> [String] { diff --git a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift index 2e0555d843..d04cd15380 100644 --- a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift +++ b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzPushNotificationResolverTests.swift @@ -513,6 +513,29 @@ final class BuzzPushNotificationResolverTests: XCTestCase { XCTAssertNil(push(), "a future-dated message is not shown again") } + func testExtremeTimestampsDoNotCrashTheClaim() throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + addTeardownBlock { try? FileManager.default.removeItem(at: root) } + let scope = try XCTUnwrap(BuzzNotificationCacheScope(community: community())) + let cache = BuzzNotificationCache( + directory: root, now: { Date(timeIntervalSince1970: TimeInterval(Self.now)) }, + isCurrent: { $0 == scope }) + func claim(_ event: VerifiedNostrEvent) throws -> Bool { + try cache.claimFirstUnpresented([(event: event, scope: scope)], scopes: [scope]) == 0 + } + func message(_ content: String, createdAt: Int) throws -> VerifiedNostrEvent { + try Self.signedEvent(privateKey: Self.profilePrivateKey, createdAt: createdAt, + kind: 9, tags: [["h", Self.channelID]], content: content) + } + let ancient = try message("Ancient", createdAt: Int.min) + XCTAssertTrue(try claim(ancient)) + XCTAssertFalse(try claim(ancient)) + XCTAssertTrue(try claim(try message("Current", createdAt: Self.now))) + XCTAssertFalse(try claim(try message("Also ancient", createdAt: Int.min + 1)), + "a message far older than one already shown is history") + XCTAssertTrue(try claim(try message("Far future", createdAt: Int.max))) + } + func testFutureDatedEntriesDoNotEvictAClaimedMessage() throws { let root = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) addTeardownBlock { try? FileManager.default.removeItem(at: root) }