mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-25 06:05:20 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -135,6 +135,11 @@ final class BLEService: NSObject {
|
|||||||
|
|
||||||
private var maintenanceTimer: DispatchSourceTimer? // Single timer for all maintenance tasks
|
private var maintenanceTimer: DispatchSourceTimer? // Single timer for all maintenance tasks
|
||||||
private var maintenanceCounter = 0 // Track maintenance cycles
|
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)
|
// MARK: - Connection budget & scheduling (central role)
|
||||||
private var connectionScheduler = BLEConnectionScheduler<CBPeripheral>()
|
private var connectionScheduler = BLEConnectionScheduler<CBPeripheral>()
|
||||||
@@ -233,7 +238,9 @@ final class BLEService: NSObject {
|
|||||||
#endif
|
#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()
|
startMaintenanceTimer()
|
||||||
|
|
||||||
// Publish initial empty state
|
// Publish initial empty state
|
||||||
@@ -432,7 +439,7 @@ final class BLEService: NSObject {
|
|||||||
/// `startServices()` — the latter matters after a panic reset, where
|
/// `startServices()` — the latter matters after a panic reset, where
|
||||||
/// `stopServices()` cancels and nils the timer.
|
/// `stopServices()` cancels and nils the timer.
|
||||||
private func startMaintenanceTimer() {
|
private func startMaintenanceTimer() {
|
||||||
guard maintenanceTimer == nil else { return }
|
guard maintenanceTimerEnabled, maintenanceTimer == nil else { return }
|
||||||
let timer = DispatchSource.makeTimerSource(queue: bleQueue)
|
let timer = DispatchSource.makeTimerSource(queue: bleQueue)
|
||||||
timer.schedule(deadline: .now() + TransportConfig.bleMaintenanceInterval,
|
timer.schedule(deadline: .now() + TransportConfig.bleMaintenanceInterval,
|
||||||
repeating: TransportConfig.bleMaintenanceInterval,
|
repeating: TransportConfig.bleMaintenanceInterval,
|
||||||
@@ -1616,7 +1623,7 @@ private extension BLEService {
|
|||||||
#if DEBUG
|
#if DEBUG
|
||||||
// Test-only helper to inject packets into the receive pipeline
|
// Test-only helper to inject packets into the receive pipeline
|
||||||
extension BLEService {
|
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 {
|
if preseedPeer {
|
||||||
// Ensure the synthetic peer is known and marked verified for public-message tests
|
// Ensure the synthetic peer is known and marked verified for public-message tests
|
||||||
let normalizedID = PeerID(hexData: packet.senderID)
|
let normalizedID = PeerID(hexData: packet.senderID)
|
||||||
@@ -1624,6 +1631,7 @@ extension BLEService {
|
|||||||
if var existing = peerRegistry.info(for: normalizedID) {
|
if var existing = peerRegistry.info(for: normalizedID) {
|
||||||
existing.isConnected = true
|
existing.isConnected = true
|
||||||
existing.isVerifiedNickname = true
|
existing.isVerifiedNickname = true
|
||||||
|
if let signingPublicKey { existing.signingPublicKey = signingPublicKey }
|
||||||
existing.lastSeen = Date()
|
existing.lastSeen = Date()
|
||||||
peerRegistry.upsert(existing)
|
peerRegistry.upsert(existing)
|
||||||
} else {
|
} else {
|
||||||
@@ -1632,7 +1640,7 @@ extension BLEService {
|
|||||||
nickname: "TestPeer_\(fromPeerID.id.prefix(4))",
|
nickname: "TestPeer_\(fromPeerID.id.prefix(4))",
|
||||||
isConnected: true,
|
isConnected: true,
|
||||||
noisePublicKey: packet.senderID,
|
noisePublicKey: packet.senderID,
|
||||||
signingPublicKey: nil,
|
signingPublicKey: signingPublicKey,
|
||||||
isVerifiedNickname: true,
|
isVerifiedNickname: true,
|
||||||
lastSeen: Date()
|
lastSeen: Date()
|
||||||
))
|
))
|
||||||
|
|||||||
@@ -14,23 +14,29 @@ import BitFoundation
|
|||||||
struct BLEServiceCoreTests {
|
struct BLEServiceCoreTests {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
func duplicatePacket_isDeduped() async {
|
func duplicatePacket_isDeduped() async throws {
|
||||||
let ble = makeService()
|
let ble = makeService()
|
||||||
let delegate = PublicCaptureDelegate()
|
let delegate = PublicCaptureDelegate()
|
||||||
ble.delegate = delegate
|
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 sender = PeerID(str: "1122334455667788")
|
||||||
let timestamp = UInt64(Date().timeIntervalSince1970 * 1000)
|
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(
|
let receivedFirst = await TestHelpers.waitUntil(
|
||||||
{ delegate.publicMessagesSnapshot().count == 1 },
|
{ delegate.publicMessagesSnapshot().count == 1 },
|
||||||
timeout: TestConstants.defaultTimeout
|
timeout: TestConstants.defaultTimeout
|
||||||
)
|
)
|
||||||
#expect(receivedFirst)
|
#expect(receivedFirst)
|
||||||
|
|
||||||
ble._test_handlePacket(packet, fromPeerID: sender)
|
ble._test_handlePacket(packet, fromPeerID: sender, signingPublicKey: signingKey)
|
||||||
let receivedDuplicate = await TestHelpers.waitUntil(
|
let receivedDuplicate = await TestHelpers.waitUntil(
|
||||||
{ delegate.publicMessagesSnapshot().count > 1 },
|
{ delegate.publicMessagesSnapshot().count > 1 },
|
||||||
timeout: TestConstants.shortTimeout
|
timeout: TestConstants.shortTimeout
|
||||||
|
|||||||
Reference in New Issue
Block a user