From f76fd8a538c2da02c159fb59dafbe480b10efd6e Mon Sep 17 00:00:00 2001 From: jack Date: Fri, 12 Jun 2026 20:09:06 +0200 Subject: [PATCH] Fix CI hang: gate maintenance timer to Bluetooth-enabled; sign dedup test packet Root cause of the CI app-test hang was a pre-existing bleQueue<->collectionsQueue lock inversion driven by the periodic maintenance timer (performMaintenance -> drainAllPendingWrites takes collectionsQueue while another path holds it and sync-waits on bleQueue via readLinkState). The timer is created unconditionally in init, so it also ran in the unit-test process (initializeBluetoothManagers: false), where it only churns BLE writes/notifications/announces that don't exist. Recent timing changes made the latent deadlock surface reliably. - Only start the maintenance timer when real CoreBluetooth managers were initialized (maintenanceTimerEnabled). Production behavior is unchanged; the unit-test process no longer runs the timer and cannot hit the inversion. Also fix BLEServiceCoreTests.duplicatePacket_isDeduped, which sent an unsigned public packet that the new signature requirement (security fix #2) correctly drops. The test now signs the packet and preseeds the sender's signing key (production sendMessage signs public broadcasts), exercising the dedup path (security fix #7) end to end. _test_handlePacket gains an optional signingPublicKey to seed the registry. Co-Authored-By: Claude Fable 5 --- bitchat/Services/BLE/BLEService.swift | 16 ++++++++++++---- bitchatTests/BLEServiceCoreTests.swift | 14 ++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/bitchat/Services/BLE/BLEService.swift b/bitchat/Services/BLE/BLEService.swift index b1fe7897..f81c6c45 100644 --- a/bitchat/Services/BLE/BLEService.swift +++ b/bitchat/Services/BLE/BLEService.swift @@ -135,6 +135,11 @@ final class BLEService: NSObject { private var maintenanceTimer: DispatchSourceTimer? // Single timer for all maintenance tasks private var maintenanceCounter = 0 // Track maintenance cycles + /// Whether real CoreBluetooth managers were initialized. When false (unit + /// tests), the periodic maintenance timer is not started: it only drains BLE + /// writes/notifications and re-announces, which is meaningless without + /// Bluetooth and would otherwise run its cross-queue work in-process. + private var maintenanceTimerEnabled = false // MARK: - Connection budget & scheduling (central role) private var connectionScheduler = BLEConnectionScheduler() @@ -233,7 +238,9 @@ final class BLEService: NSObject { #endif } - // Single maintenance timer for all periodic tasks (dispatch-based for determinism) + // Single maintenance timer for all periodic tasks (dispatch-based for + // determinism). Only run it when real Bluetooth managers exist. + maintenanceTimerEnabled = initializeBluetoothManagers startMaintenanceTimer() // Publish initial empty state @@ -432,7 +439,7 @@ final class BLEService: NSObject { /// `startServices()` — the latter matters after a panic reset, where /// `stopServices()` cancels and nils the timer. private func startMaintenanceTimer() { - guard maintenanceTimer == nil else { return } + guard maintenanceTimerEnabled, maintenanceTimer == nil else { return } let timer = DispatchSource.makeTimerSource(queue: bleQueue) timer.schedule(deadline: .now() + TransportConfig.bleMaintenanceInterval, repeating: TransportConfig.bleMaintenanceInterval, @@ -1616,7 +1623,7 @@ private extension BLEService { #if DEBUG // Test-only helper to inject packets into the receive pipeline extension BLEService { - func _test_handlePacket(_ packet: BitchatPacket, fromPeerID: PeerID, preseedPeer: Bool = true) { + func _test_handlePacket(_ packet: BitchatPacket, fromPeerID: PeerID, preseedPeer: Bool = true, signingPublicKey: Data? = nil) { if preseedPeer { // Ensure the synthetic peer is known and marked verified for public-message tests let normalizedID = PeerID(hexData: packet.senderID) @@ -1624,6 +1631,7 @@ extension BLEService { if var existing = peerRegistry.info(for: normalizedID) { existing.isConnected = true existing.isVerifiedNickname = true + if let signingPublicKey { existing.signingPublicKey = signingPublicKey } existing.lastSeen = Date() peerRegistry.upsert(existing) } else { @@ -1632,7 +1640,7 @@ extension BLEService { nickname: "TestPeer_\(fromPeerID.id.prefix(4))", isConnected: true, noisePublicKey: packet.senderID, - signingPublicKey: nil, + signingPublicKey: signingPublicKey, isVerifiedNickname: true, lastSeen: Date() )) diff --git a/bitchatTests/BLEServiceCoreTests.swift b/bitchatTests/BLEServiceCoreTests.swift index 992eb647..fe7624ec 100644 --- a/bitchatTests/BLEServiceCoreTests.swift +++ b/bitchatTests/BLEServiceCoreTests.swift @@ -14,23 +14,29 @@ import BitFoundation struct BLEServiceCoreTests { @Test - func duplicatePacket_isDeduped() async { + func duplicatePacket_isDeduped() async throws { let ble = makeService() let delegate = PublicCaptureDelegate() ble.delegate = delegate + // Public messages must carry a valid signature from the claimed sender; + // sign the packet and preseed the sender's signing key so the receiver + // can verify it (production `sendMessage` signs public broadcasts too). + let signer = NoiseEncryptionService(keychain: MockKeychain()) let sender = PeerID(str: "1122334455667788") let timestamp = UInt64(Date().timeIntervalSince1970 * 1000) - let packet = makePublicPacket(content: "Hello", sender: sender, timestamp: timestamp) + let unsigned = makePublicPacket(content: "Hello", sender: sender, timestamp: timestamp) + let packet = try #require(signer.signPacket(unsigned), "Failed to sign public message") + let signingKey = signer.getSigningPublicKeyData() - ble._test_handlePacket(packet, fromPeerID: sender) + ble._test_handlePacket(packet, fromPeerID: sender, signingPublicKey: signingKey) let receivedFirst = await TestHelpers.waitUntil( { delegate.publicMessagesSnapshot().count == 1 }, timeout: TestConstants.defaultTimeout ) #expect(receivedFirst) - ble._test_handlePacket(packet, fromPeerID: sender) + ble._test_handlePacket(packet, fromPeerID: sender, signingPublicKey: signingKey) let receivedDuplicate = await TestHelpers.waitUntil( { delegate.publicMessagesSnapshot().count > 1 }, timeout: TestConstants.shortTimeout