fix(device): Keep a log line for rejected malformed addresses

Structurally broken addresses now fail the octet parse instead of throwing, so the catch-all error log no longer fires for them. Log the rejection at WARN with a redacted address so support logs still show it. Fixes review finding F1.
fix(monitor): Re-arm the teardown when a start request is short-circuited
2026-09-14 18:26:11 -04:00 · 2026-08-31 15:54:51 +02:00 · 2026-08-31 15:54:51 +02:00 · 2026-08-31 15:54:51 +02:00 · 2026-08-31 15:54:51 +02:00 · 2026-08-31 15:54:51 +02:00
14 changed files with 790 additions and 58 deletions
@@ -86,6 +86,7 @@
            android:label="Service trigger">
            <intent-filter>
                <action android:name="android.bluetooth.device.action.ACL_CONNECTED" />
+                <action android:name="android.bluetooth.device.action.ACL_DISCONNECTED" />
            </intent-filter>
        </receiver>

@@ -237,6 +237,26 @@ class BluetoothManager2 @Inject constructor(
    private val seenDevicesLock = Mutex()
    private val seenDevicesCache = mutableMapOf<String, Instant>()

+    /**
+     * Stamps the connect time from the ACL broadcast, which reaches a manifest-registered receiver
+     * whether or not [connectedDevices] is being collected. An existing stamp wins: the earlier one
+     * is the real connect time.
+     */
+    fun markDeviceConnected(address: BluetoothAddress) {
+        appScope.launch {
+            seenDevicesLock.withLock {
+                if (seenDevicesCache.containsKey(address)) return@withLock
+                seenDevicesCache[address] = timeSource.now()
+            }
+        }
+    }
+
+    fun markDeviceDisconnected(address: BluetoothAddress) {
+        appScope.launch {
+            seenDevicesLock.withLock { seenDevicesCache.remove(address) }
+        }
+    }
+
    val connectedDevices: Flow<List<BluetoothDevice2>> = isBluetoothEnabled
        .flatMapLatest { enabled ->
            if (enabled) monitorProfile(BluetoothProfile.HEADSET)
@@ -311,12 +331,10 @@ class BluetoothManager2 @Inject constructor(
                address = device.address,
                name = device.name,
                internal = device,
+                // Read-only: a bonded device is not necessarily connected, and writing here would
+                // re-stamp entries that [connectedDevices] just pruned.
                seenFirstAt = seenDevicesLock.withLock {
-                    seenDevicesCache[device.address] ?: run {
-                        val now = timeSource.now()
-                        seenDevicesCache[device.address] = now
-                        now
-                    }
+                    seenDevicesCache[device.address] ?: timeSource.now()
                }
            )

@@ -1,5 +1,6 @@
 package eu.darken.capod.monitor.core.aap

+import eu.darken.capod.common.debug.logging.Logging.Priority.VERBOSE
 import eu.darken.capod.common.debug.logging.log
 import eu.darken.capod.common.debug.logging.logTag
 import eu.darken.capod.common.flow.setupCommonEventHandlers
@@ -33,15 +34,22 @@ class AapKeyPersister @Inject constructor(
                return@onEach
            }

-            val updated = profile.copy(
-                identityKey = keys.irk ?: profile.identityKey,
-                encryptionKey = keys.encKey ?: profile.encryptionKey,
-            )
+            // The profile's key fields are ByteArrays, so the generated equals() compares them by
+            // reference — content comparison is what decides whether anything actually changed.
+            val irkChanged = keys.irk != null && !keys.irk.contentEquals(profile.identityKey)
+            val encChanged = keys.encKey != null && !keys.encKey.contentEquals(profile.encryptionKey)

-            if (updated != profile) {
-                profilesRepo.updateProfile(updated)
-                log(TAG) { "Persisted keys for $address (IRK=${keys.irk != null}, ENC=${keys.encKey != null})" }
+            if (!irkChanged && !encChanged) {
+                log(TAG, VERBOSE) { "Keys for $address are unchanged, skipping key persistence" }
+                return@onEach
            }
+
+            val updated = profile.copy(
+                identityKey = if (irkChanged) keys.irk else profile.identityKey,
+                encryptionKey = if (encChanged) keys.encKey else profile.encryptionKey,
+            )
+            profilesRepo.updateProfile(updated)
+            log(TAG) { "Persisted keys for $address (IRK changed=$irkChanged, ENC changed=$encChanged)" }
        }
        .map { }
        .setupCommonEventHandlers(TAG) { "keyPersister" }
@@ -7,6 +7,7 @@ import android.content.BroadcastReceiver
 import android.content.Context
 import android.content.Intent
 import dagger.hilt.android.AndroidEntryPoint
+import eu.darken.capod.common.bluetooth.BluetoothManager2
 import eu.darken.capod.common.bluetooth.hasFeature
 import eu.darken.capod.common.debug.logging.Logging.Priority.WARN
 import eu.darken.capod.common.debug.logging.log
@@ -19,6 +20,7 @@ import javax.inject.Inject
 class BluetoothEventReceiver : BroadcastReceiver() {

    @Inject lateinit var monitorControl: MonitorControl
+    @Inject lateinit var bluetoothManager: BluetoothManager2

    override fun onReceive(context: Context, intent: Intent) {
        log(TAG) { "onReceive($context, $intent)" }
@@ -48,6 +50,15 @@ class BluetoothEventReceiver : BroadcastReceiver() {
            log { "Device has the following we features we support $supportedFeatures" }
        }

+        when (intent.action) {
+            BluetoothDevice.ACTION_ACL_CONNECTED -> bluetoothManager.markDeviceConnected(bluetoothDevice.address)
+            BluetoothDevice.ACTION_ACL_DISCONNECTED -> {
+                bluetoothManager.markDeviceDisconnected(bluetoothDevice.address)
+                // A disconnect is not a reason to start monitoring.
+                return
+            }
+        }
+
        log(TAG) { "Starting monitor" }
        monitorControl.startMonitor(forceStart = false)
    }
@@ -55,6 +55,9 @@ import kotlinx.coroutines.Job
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.cancelChildren
 import kotlinx.coroutines.delay
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.catch
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
@@ -95,6 +98,7 @@ class MonitorService : Service() {

    private val monitorScope = MonitorCoroutineScope()
    private var monitoringJob: Job? = null
+    private val startSignal = MutableStateFlow(0L)
    @Volatile private var monitorGeneration = 0
    private var foregroundStartFailed = false
    private var injectionComplete = false
@@ -212,6 +216,8 @@ class MonitorService : Service() {

        if (monitoringJob?.isActive == true && !forceStart) {
            log(TAG) { "Already monitoring and forceStart=false, keeping current session." }
+            // Fresh evidence of activity: re-arm any pending teardown countdown.
+            startSignal.value++
            return START_STICKY
        }

@@ -314,7 +320,7 @@ class MonitorService : Service() {
            }
            .launchIn(monitorScope)

-        permissionTool.missingScanPermissions
+        val modeStates = permissionTool.missingScanPermissions
            .flatMapLatest { missingPermsFlow ->
                if (missingPermsFlow.isNotEmpty()) {
                    log(TAG, WARN) { "Aborting, scan permissions are missing: $missingPermsFlow" }
@@ -333,38 +339,13 @@ class MonitorService : Service() {
            }
            .distinctUntilChanged()
            .setupCommonEventHandlers(TAG) { "MonitorMode" }
-            .flatMapLatest { state ->
-                log(TAG) { "Monitor mode: ${state.mode}" }
-                log(TAG) { "connectedAddresses: ${state.connectedAddresses}" }
-                log(TAG) { "knownAddresses: ${state.knownAddresses}" }

-                when (state.mode) {
-                    MonitorMode.MANUAL -> flow<Unit> {
-                        monitorScope.coroutineContext.cancelChildren()
-                    }
-
-                    MonitorMode.ALWAYS -> emptyFlow()
-                    MonitorMode.AUTOMATIC -> flow {
-                        when {
-                            !state.hasProfiles && state.connectedAddresses.isNotEmpty() -> {
-                                log(TAG, WARN) { "Main device address not set, staying alive while any is connected" }
-                            }
-
-                            state.knownAddresses.any { it in state.connectedAddresses } || state.hasAapSession -> {
-                                log(TAG) { "A device is connected, aborting any timeout." }
-                            }
-
-                            else -> {
-                                log(TAG) { "No known Pods are connected, stopping service soon." }
-                                delay(15 * 1000)
-                                log(TAG) { "Stopping service now, still no Pods connected." }
-
-                                monitorScope.coroutineContext.cancelChildren()
-                            }
-                        }
-                    }
-                }
-            }
+        monitorModeFlow(
+            tag = TAG,
+            modeStates = modeStates,
+            startSignal = startSignal,
+            onTeardown = { monitorScope.coroutineContext.cancelChildren() },
+        )
            .catch {
                log(TAG, WARN) { "MonitorMode Flow failed:\n${it.asLog()}" }
            }
@@ -492,6 +473,60 @@ internal data class MonitorModeState(
    val hasAapSession: Boolean,
 )

+/**
+ * Decides whether a monitor session may keep running. In [MonitorMode.AUTOMATIC] with nothing
+ * connected the teardown runs [timeoutMillis] after the state was seen.
+ *
+ * [startSignal] is bumped by a start request that found a live session and short-circuited. The
+ * bump restarts this flow, arming a fresh window, and the pre-teardown re-check covers the case
+ * where the bump lands while the expired countdown is already unwinding.
+ */
+internal fun monitorModeFlow(
+    tag: String,
+    modeStates: Flow<MonitorModeState>,
+    startSignal: StateFlow<Long>,
+    timeoutMillis: Long = 15 * 1000,
+    onTeardown: () -> Unit,
+): Flow<Unit> = combine(modeStates, startSignal) { state, signal ->
+    state to signal
+}.flatMapLatest { (state, armedSignal) ->
+    log(tag) { "Monitor mode: ${state.mode}" }
+    log(tag) { "connectedAddresses: ${state.connectedAddresses}" }
+    log(tag) { "knownAddresses: ${state.knownAddresses}" }
+
+    when (state.mode) {
+        MonitorMode.MANUAL -> flow<Unit> {
+            onTeardown()
+        }
+
+        MonitorMode.ALWAYS -> emptyFlow()
+        MonitorMode.AUTOMATIC -> flow {
+            when {
+                !state.hasProfiles && state.connectedAddresses.isNotEmpty() -> {
+                    log(tag, WARN) { "Main device address not set, staying alive while any is connected" }
+                }
+
+                state.knownAddresses.any { it in state.connectedAddresses } || state.hasAapSession -> {
+                    log(tag) { "A device is connected, aborting any timeout." }
+                }
+
+                else -> {
+                    log(tag) { "No known Pods are connected, stopping service soon." }
+                    delay(timeoutMillis)
+
+                    if (startSignal.value != armedSignal) {
+                        log(tag) { "A start request arrived during the timeout, staying alive." }
+                        return@flow
+                    }
+                    log(tag) { "Stopping service now, still no Pods connected." }
+
+                    onTeardown()
+                }
+            }
+        }
+    }
+}
+
 internal fun buildMonitorModeState(
    mode: MonitorMode,
    profiles: List<DeviceProfile>,
@@ -70,9 +70,12 @@ class AppleFactory @Inject constructor(
        val factory = podFactories.firstOrNull { it.isResponsible(proximityMessage) } ?: unknownAppleFactory

        val profiles = profilesRepo.currentProfiles().filterIsInstance<AppleDeviceProfile>()
-        var profile = profiles.firstOrNull {
-            it.identityKey != null && rpaChecker.verify(scanResult.address, it.identityKey)
+        val irkMatch = profiles.firstNotNullOfOrNull { candidate ->
+            val identityKey = candidate.identityKey ?: return@firstNotNullOfOrNull null
+            rpaChecker.resolve(scanResult.address, identityKey)?.let { candidate to it }
        }
+        var profile = irkMatch?.first
+        val irkOrder = irkMatch?.second

        val isIrkMatch = profile != null
        if (isIrkMatch) {
@@ -115,7 +118,10 @@ class AppleFactory @Inject constructor(
                    .filter { it.identityKey != null && (it.model == PodModel.UNKNOWN || it.model == tempDevice.model) }
                    .firstOrNull { it.minimumSignalQuality <= tempDevice.signalQuality }
                if (legacyCandidate != null) {
-                    log(TAG, WARN) { "Keyed profile ${legacyCandidate.id} would match via old fallback (IRK failed) — stale key?" }
+                    log(TAG, WARN) {
+                        "Keyed profile ${legacyCandidate.id} would match via old fallback, " +
+                            "its key resolved neither address order"
+                    }
                }
            }
        }
@@ -136,7 +142,8 @@ class AppleFactory @Inject constructor(
            val publicHex = payload.public.data.joinToString(" ") { "%02X".format(it.toInt()) }
            val privateHex = payload.private?.data?.joinToString(" ") { "%02X".format(it.toInt()) } ?: "-"
            "Apple decoded: model=${device.model}, addr=${scanResult.address.redactedForLogs()}, " +
-                "irkMatch=$isIrkMatch, raw=[$rawHex], public=[$publicHex], private=[$privateHex]"
+                "irkMatch=$isIrkMatch, irkOrder=${irkOrder?.name ?: "-"}, " +
+                "raw=[$rawHex], public=[$publicHex], private=[$privateHex]"
        }

        device
@@ -136,6 +136,7 @@ class PodHistoryRepo @Inject constructor(

            if (profile != null) {
                recognizedDevice = knownDevices.values
+                    .filter { it.boundProfileId == null || it.boundProfileId == profile.id }
                    .firstOrNull { rpaChecker.verify(it.lastAddress, profile.identityKey!!) }
                    .also { log(TAG, VERBOSE) { "search1: Recovered via IRK: ${it?.logSummary()}" } }
            }
@@ -2,30 +2,73 @@ package eu.darken.capod.pods.core.apple.ble.protocol

 import android.annotation.SuppressLint
 import eu.darken.capod.common.bluetooth.BluetoothAddress
+import eu.darken.capod.common.bluetooth.redactedForLogs
 import eu.darken.capod.common.debug.logging.Logging
 import eu.darken.capod.common.debug.logging.asLog
 import eu.darken.capod.common.debug.logging.log
 import eu.darken.capod.common.debug.logging.logTag
-import okio.ByteString.Companion.toByteString
 import javax.crypto.Cipher
 import javax.crypto.spec.SecretKeySpec
 import javax.inject.Inject

 class RPAChecker @Inject constructor() {

+    enum class AddressOrder {
+        STANDARD,
+        REVERSED,
+    }
+
    // Resolvable-Private-Address
-    fun verify(address: BluetoothAddress, irk: IdentityResolvingKey): Boolean = try {
-        val rpa = address.split(":").map { it.toInt(16).toByte() }.reversed().toByteArray()
-        val prand = rpa.copyOfRange(3, 6)
-        val hash = rpa.copyOfRange(0, 3)
-        val computedHash = ah(irk, prand)
-        hash.contentEquals(computedHash)
+    fun verify(address: BluetoothAddress, irk: IdentityResolvingKey): Boolean = resolve(address, irk) != null
+
+    /**
+     * Returns the octet order the [address] resolved in, or null if it resolves in neither.
+     *
+     * The reversed attempt is gated on the address being RPA-shaped in that order, the standard
+     * attempt is not: a device that resolves today must keep resolving byte-for-byte the same way.
+     */
+    fun resolve(address: BluetoothAddress, irk: IdentityResolvingKey): AddressOrder? = try {
+        val octets = address.parseOctets()
+        when {
+            octets == null -> {
+                log(TAG, Logging.Priority.WARN) {
+                    "Failed to resolve RPA, malformed address: ${address.redactedForLogs()}"
+                }
+                null
+            }
+            matchesHash(octets, irk) -> AddressOrder.STANDARD
+            else -> octets.reversedArray()
+                .takeIf { it.isRpaShaped() && matchesHash(it, irk) }
+                ?.let { AddressOrder.REVERSED }
+        }
    } catch (e: Exception) {
        log(
            TAG,
            Logging.Priority.ERROR
-        ) { "Failed to verify RPA\naddress=${address}\nIRK=${irk.toByteString()}\n${e.asLog()}" }
-        false
+        ) { "Failed to resolve RPA\naddress=${address.redactedForLogs()}\nIRK=${irk.size}B\n${e.asLog()}" }
+        null
+    }
+
+    /** Six octets, most-significant first, as printed. Null if [this] isn't a well-formed address. */
+    private fun BluetoothAddress.parseOctets(): ByteArray? {
+        val parts = split(":")
+        if (parts.size != ADDRESS_OCTETS) return null
+        val octets = ByteArray(ADDRESS_OCTETS)
+        parts.forEachIndexed { index, part ->
+            val value = part.toIntOrNull(16) ?: return null
+            if (value !in 0..255) return null
+            octets[index] = value.toByte()
+        }
+        return octets
+    }
+
+    private fun ByteArray.isRpaShaped(): Boolean = (this[0].toInt() and 0xC0) == 0x40
+
+    private fun matchesHash(octets: ByteArray, irk: IdentityResolvingKey): Boolean {
+        val rpa = octets.reversedArray()
+        val prand = rpa.copyOfRange(3, 6)
+        val hash = rpa.copyOfRange(0, 3)
+        return hash.contentEquals(ah(irk, prand))
    }

    // E function (Encryption function):
@@ -47,6 +90,7 @@ class RPAChecker @Inject constructor() {
    }

    companion object {
+        private const val ADDRESS_OCTETS = 6
        private val TAG = logTag("Monitor", "BlePodMonitor", "RPAChecker")
    }
 }
@@ -0,0 +1,120 @@
+package eu.darken.capod.common.bluetooth
+
+import android.bluetooth.BluetoothAdapter
+import android.bluetooth.BluetoothDevice
+import android.bluetooth.BluetoothManager
+import android.content.Context
+import io.kotest.matchers.shouldBe
+import io.mockk.every
+import io.mockk.mockk
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.cancel
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.test.UnconfinedTestDispatcher
+import kotlinx.coroutines.test.runTest
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.Test
+import testhelpers.BaseTest
+import testhelpers.TestTimeSource
+import testhelpers.coroutine.TestDispatcherProvider
+import java.time.Duration
+
+class BluetoothManager2Test : BaseTest() {
+
+    private val deviceA = mockk<BluetoothDevice>().apply {
+        every { address } returns ADDRESS_A
+        every { name } returns "Pods A"
+    }
+    private val deviceB = mockk<BluetoothDevice>().apply {
+        every { address } returns ADDRESS_B
+        every { name } returns "Pods B"
+    }
+    private val btAdapter = mockk<BluetoothAdapter>().apply {
+        every { bondedDevices } returns setOf(deviceA, deviceB)
+    }
+    private val btManager = mockk<BluetoothManager>().apply {
+        every { adapter } returns btAdapter
+    }
+    private val timeSource = TestTimeSource()
+
+    // Unconfined, so the appScope.launch bodies of the mark* methods complete in place.
+    private val appScope = CoroutineScope(UnconfinedTestDispatcher())
+
+    @AfterEach
+    fun teardown() {
+        appScope.cancel()
+    }
+
+    private fun create() = BluetoothManager2(
+        appScope = appScope,
+        dispatcherProvider = TestDispatcherProvider(),
+        context = mockk<Context>(),
+        manager = btManager,
+        timeSource = timeSource,
+    )
+
+    private suspend fun BluetoothManager2.bonded(address: BluetoothAddress) =
+        bondedDevices().first().single { it.address == address }
+
+    /**
+     * A bonded device is not necessarily a connected one, so querying bonded devices must not put a
+     * timestamp into the cache that [BluetoothManager2.connectedDevices] later reads as a connect
+     * time.
+     */
+    @Test
+    fun `querying bonded devices does not cache a timestamp`() = runTest {
+        val manager = create()
+
+        manager.bonded(ADDRESS_A).seenFirstAt shouldBe timeSource.now()
+
+        timeSource.advanceBy(Duration.ofSeconds(60))
+
+        manager.bonded(ADDRESS_A).seenFirstAt shouldBe timeSource.now()
+    }
+
+    /**
+     * The ACL broadcast arrives whether or not anything is collecting the connected-devices flow,
+     * so the stamp it leaves has to survive until the device disconnects.
+     */
+    @Test
+    fun `an ACL connect stamp survives the passage of time`() = runTest {
+        val manager = create()
+        val connectedAt = timeSource.now()
+
+        manager.markDeviceConnected(ADDRESS_A)
+
+        timeSource.advanceBy(Duration.ofSeconds(60))
+
+        manager.bonded(ADDRESS_A).seenFirstAt shouldBe connectedAt
+    }
+
+    @Test
+    fun `a repeated ACL connect keeps the first stamp`() = runTest {
+        val manager = create()
+        val connectedAt = timeSource.now()
+
+        manager.markDeviceConnected(ADDRESS_A)
+
+        timeSource.advanceBy(Duration.ofSeconds(60))
+        manager.markDeviceConnected(ADDRESS_A)
+
+        manager.bonded(ADDRESS_A).seenFirstAt shouldBe connectedAt
+    }
+
+    @Test
+    fun `an ACL disconnect drops the stamp`() = runTest {
+        val manager = create()
+
+        manager.markDeviceConnected(ADDRESS_A)
+
+        timeSource.advanceBy(Duration.ofSeconds(60))
+        manager.markDeviceDisconnected(ADDRESS_A)
+
+        manager.bonded(ADDRESS_A).seenFirstAt shouldBe timeSource.now()
+    }
+
+    companion object {
+        private const val ADDRESS_A = "AA:BB:CC:DD:EE:F1"
+        private const val ADDRESS_B = "AA:BB:CC:DD:EE:F2"
+    }
+}
@@ -1,13 +1,20 @@
 package eu.darken.capod.monitor.core

+import eu.darken.capod.common.debug.logging.Logging
 import eu.darken.capod.common.fromHex
+import eu.darken.capod.common.toHex
 import eu.darken.capod.pods.core.apple.ble.protocol.RPAChecker
+import io.kotest.matchers.nulls.shouldBeNull
 import io.kotest.matchers.shouldBe
 import org.junit.jupiter.api.Test
 import testhelpers.BaseTest
+import testhelpers.logging.JUnitLogger

 class RPACheckerTest : BaseTest() {

+    private val irkHex = "79-04-65-1E-E2-CC-D9-26-F2-6E-20-EE-3E-CC-DE-79"
+    private val resolvingAddress = "5A:16:2B:91:D1:CD"
+
    @Test
    fun `test check`() {
        val checker = RPAChecker()
@@ -37,4 +44,150 @@ class RPACheckerTest : BaseTest() {
            irk = "".fromHex(),
        ) shouldBe false
    }
+
+    @Test
+    fun `resolve reports the standard octet order`() {
+        val checker = RPAChecker()
+        checker.resolve(
+            address = resolvingAddress,
+            irk = irkHex.fromHex(),
+        ) shouldBe RPAChecker.AddressOrder.STANDARD
+        checker.verify(
+            address = resolvingAddress,
+            irk = irkHex.fromHex(),
+        ) shouldBe true
+    }
+
+    @Test
+    fun `resolve reports the reversed octet order`() {
+        val checker = RPAChecker()
+        // The resolving address with its octets reversed, as a vendor stack delivering them backwards.
+        checker.resolve(
+            address = "CD:D1:91:2B:16:5A",
+            irk = irkHex.fromHex(),
+        ) shouldBe RPAChecker.AddressOrder.REVERSED
+        checker.verify(
+            address = "CD:D1:91:2B:16:5A",
+            irk = irkHex.fromHex(),
+        ) shouldBe true
+    }
+
+    @Test
+    fun `a reversed candidate still has to pass the hash comparison`() {
+        val checker = RPAChecker()
+        // Reversed form 5B:16:2B:91:D1:CD is RPA-shaped, so the marker gate passes and only the
+        // hash comparison can reject it.
+        checker.resolve(
+            address = "CD:D1:91:2B:16:5B",
+            irk = irkHex.fromHex(),
+        ).shouldBeNull()
+        checker.verify(
+            address = "CD:D1:91:2B:16:5B",
+            irk = irkHex.fromHex(),
+        ) shouldBe false
+    }
+
+    @Test
+    fun `a reversed candidate is gated on the RPA type marker`() {
+        val checker = RPAChecker()
+        // Reversed form 1A:16:2B:85:33:DC resolves cryptographically, but its type marker is 00,
+        // so only the gate rejects it.
+        checker.resolve(
+            address = "DC:33:85:2B:16:1A",
+            irk = irkHex.fromHex(),
+        ).shouldBeNull()
+        checker.verify(
+            address = "DC:33:85:2B:16:1A",
+            irk = irkHex.fromHex(),
+        ) shouldBe false
+    }
+
+    @Test
+    fun `malformed addresses are rejected instead of mis-sliced`() {
+        val checker = RPAChecker()
+        // Seven components whose tail is the resolving address — sliced rather than rejected, this
+        // resolves.
+        checker.resolve(
+            address = "00:$resolvingAddress",
+            irk = irkHex.fromHex(),
+        ).shouldBeNull()
+        checker.verify(
+            address = "00:$resolvingAddress",
+            irk = irkHex.fromHex(),
+        ) shouldBe false
+        // 0x15A truncates to the resolving octet 0x5A.
+        checker.resolve(
+            address = "15A:16:2B:91:D1:CD",
+            irk = irkHex.fromHex(),
+        ).shouldBeNull()
+        checker.verify(
+            address = "15A:16:2B:91:D1:CD",
+            irk = irkHex.fromHex(),
+        ) shouldBe false
+        checker.resolve(
+            address = "5A:16:2B:91:D1",
+            irk = irkHex.fromHex(),
+        ).shouldBeNull()
+    }
+
+    @Test
+    fun `the failure log carries neither the identity key nor the full address`() {
+        val malformedIrk = "79-04-65-1E-E2-CC-D9-26-F2-6E-20-EE-3E-CC-DE".fromHex()
+        val captured = mutableListOf<Pair<Logging.Priority, String>>()
+        val capturingLogger = object : Logging.Logger {
+            override fun log(
+                priority: Logging.Priority,
+                tag: String,
+                message: String,
+                metaData: Map<String, Any>?,
+            ) {
+                captured.add(priority to message)
+            }
+        }
+
+        Logging.clearAll()
+        Logging.install(capturingLogger)
+        try {
+            RPAChecker().verify(address = resolvingAddress, irk = malformedIrk) shouldBe false
+        } finally {
+            Logging.clearAll()
+            Logging.install(JUnitLogger())
+        }
+
+        captured.any { (priority, message) ->
+            priority == Logging.Priority.ERROR && message.contains("Failed to resolve RPA")
+        } shouldBe true
+
+        val keyHex = malformedIrk.toHex(separator = "")
+        captured.any { (_, message) -> message.contains(keyHex, ignoreCase = true) } shouldBe false
+        captured.any { (_, message) -> message.contains(resolvingAddress) } shouldBe false
+    }
+
+    @Test
+    fun `a malformed address is logged as a warning`() {
+        val captured = mutableListOf<Pair<Logging.Priority, String>>()
+        val capturingLogger = object : Logging.Logger {
+            override fun log(
+                priority: Logging.Priority,
+                tag: String,
+                message: String,
+                metaData: Map<String, Any>?,
+            ) {
+                captured.add(priority to message)
+            }
+        }
+
+        Logging.clearAll()
+        Logging.install(capturingLogger)
+        try {
+            RPAChecker().verify(address = "123", irk = irkHex.fromHex()) shouldBe false
+        } finally {
+            Logging.clearAll()
+            Logging.install(JUnitLogger())
+        }
+
+        captured.any { (priority, message) ->
+            priority == Logging.Priority.WARN && message.contains("malformed")
+        } shouldBe true
+    }
 }
@@ -0,0 +1,108 @@
+package eu.darken.capod.monitor.core.aap
+
+import eu.darken.capod.common.bluetooth.BluetoothAddress
+import eu.darken.capod.common.fromHex
+import eu.darken.capod.pods.core.apple.PodModel
+import eu.darken.capod.pods.core.apple.aap.AapConnectionManager
+import eu.darken.capod.pods.core.apple.aap.protocol.KeyExchangeResult
+import eu.darken.capod.profiles.core.AppleDeviceProfile
+import eu.darken.capod.profiles.core.DeviceProfile
+import eu.darken.capod.profiles.core.DeviceProfilesRepo
+import io.kotest.matchers.shouldBe
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.slot
+import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.test.runTest
+import org.junit.jupiter.api.Test
+import testhelpers.BaseTest
+
+/**
+ * The key fields on [AppleDeviceProfile] are ByteArrays, so a wrong null-handling or
+ * reference-comparison here either writes the profile on every connect or stops persisting keys
+ * entirely.
+ */
+class AapKeyPersisterTest : BaseTest() {
+
+    private val address: BluetoothAddress = "AA:BB:CC:DD:EE:FF"
+    private val irkHex = "79-04-65-1E-E2-CC-D9-26-F2-6E-20-EE-3E-CC-DE-79"
+    private val encHex = "11-22-33-44-55-66-77-88-99-AA-BB-CC-DD-EE-FF-00"
+
+    private val aapManager = mockk<AapConnectionManager>()
+    private val profilesRepo = mockk<DeviceProfilesRepo>()
+
+    private suspend fun runPersister(profile: AppleDeviceProfile, keys: KeyExchangeResult) {
+        val keysReceived = MutableSharedFlow<Pair<BluetoothAddress, KeyExchangeResult>>(replay = 1)
+        keysReceived.tryEmit(address to keys)
+
+        every { aapManager.keysReceived } returns keysReceived
+        every { profilesRepo.profiles } returns flowOf(listOf(profile))
+        coEvery { profilesRepo.updateProfile(any()) } returns Unit
+
+        AapKeyPersister(aapManager, profilesRepo).monitor().first()
+    }
+
+    private fun profile(
+        identityKey: ByteArray? = null,
+        encryptionKey: ByteArray? = null,
+    ) = AppleDeviceProfile(
+        label = "Mine",
+        model = PodModel.AIRPODS_PRO2_USBC,
+        identityKey = identityKey,
+        encryptionKey = encryptionKey,
+        address = address,
+    )
+
+    private fun capturedProfile(): AppleDeviceProfile {
+        val captured = slot<DeviceProfile>()
+        coVerify(exactly = 1) { profilesRepo.updateProfile(capture(captured)) }
+        return captured.captured as AppleDeviceProfile
+    }
+
+    @Test
+    fun `a first key is persisted`() = runTest {
+        runPersister(
+            profile = profile(),
+            keys = KeyExchangeResult(irk = irkHex.fromHex(), encKey = null),
+        )
+
+        capturedProfile().identityKey.contentEquals(irkHex.fromHex()) shouldBe true
+    }
+
+    @Test
+    fun `a key equal by content is not persisted again`() = runTest {
+        runPersister(
+            profile = profile(identityKey = irkHex.fromHex(), encryptionKey = encHex.fromHex()),
+            keys = KeyExchangeResult(irk = irkHex.fromHex(), encKey = encHex.fromHex()),
+        )
+
+        coVerify(exactly = 0) { profilesRepo.updateProfile(any()) }
+    }
+
+    @Test
+    fun `a missing incoming key leaves the stored one alone`() = runTest {
+        runPersister(
+            profile = profile(identityKey = irkHex.fromHex(), encryptionKey = encHex.fromHex()),
+            keys = KeyExchangeResult(irk = null, encKey = encHex.fromHex()),
+        )
+
+        coVerify(exactly = 0) { profilesRepo.updateProfile(any()) }
+    }
+
+    @Test
+    fun `a changed encryption key is persisted without touching the identity key`() = runTest {
+        val newEnc = "00-11-22-33-44-55-66-77-88-99-AA-BB-CC-DD-EE-FF".fromHex()
+        runPersister(
+            profile = profile(identityKey = irkHex.fromHex(), encryptionKey = encHex.fromHex()),
+            keys = KeyExchangeResult(irk = irkHex.fromHex(), encKey = newEnc),
+        )
+
+        val written = capturedProfile()
+        written.encryptionKey.contentEquals(newEnc) shouldBe true
+        written.identityKey.contentEquals(irkHex.fromHex()) shouldBe true
+    }
+}
@@ -0,0 +1,125 @@
+package eu.darken.capod.monitor.core.worker
+
+import eu.darken.capod.common.bluetooth.BluetoothAddress
+import eu.darken.capod.main.core.MonitorMode
+import io.kotest.matchers.shouldBe
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.test.advanceTimeBy
+import kotlinx.coroutines.test.runCurrent
+import kotlinx.coroutines.test.runTest
+import org.junit.jupiter.api.Test
+import testhelpers.BaseTest
+
+/**
+ * The AUTOMATIC teardown is only cancelled by a new state emission, but a start request that finds
+ * a live session short-circuits without producing one — the connection state that would abort the
+ * countdown lags the Bluetooth event by roughly a second. A request landing in the tail of the
+ * window used to be acknowledged and the session torn down anyway.
+ */
+class MonitorModeFlowTest : BaseTest() {
+
+    private fun state(
+        mode: MonitorMode = MonitorMode.AUTOMATIC,
+        hasProfiles: Boolean = true,
+        knownAddresses: Set<BluetoothAddress> = setOf(KNOWN_ADDRESS),
+        connectedAddresses: Set<BluetoothAddress> = emptySet(),
+        hasAapSession: Boolean = false,
+    ) = MonitorModeState(
+        mode = mode,
+        hasProfiles = hasProfiles,
+        knownAddresses = knownAddresses,
+        connectedAddresses = connectedAddresses,
+        hasAapSession = hasAapSession,
+    )
+
+    @Test
+    fun `nothing connected tears the session down after the timeout`() = runTest {
+        var teardowns = 0
+        val job = monitorModeFlow(
+            tag = TAG,
+            modeStates = flowOf(state()),
+            startSignal = MutableStateFlow(0L),
+            onTeardown = { teardowns++ },
+        ).launchIn(this)
+
+        advanceTimeBy(TIMEOUT)
+        teardowns shouldBe 0
+
+        runCurrent()
+        teardowns shouldBe 1
+
+        job.cancel()
+    }
+
+    @Test
+    fun `a start request during the window re-arms the countdown`() = runTest {
+        var teardowns = 0
+        val startSignal = MutableStateFlow(0L)
+        val job = monitorModeFlow(
+            tag = TAG,
+            modeStates = flowOf(state()),
+            startSignal = startSignal,
+            onTeardown = { teardowns++ },
+        ).launchIn(this)
+
+        val bumpedAt = 14_750L
+        advanceTimeBy(bumpedAt)
+        runCurrent()
+        startSignal.value++
+
+        // The original window would have expired here.
+        advanceTimeBy(TIMEOUT - bumpedAt)
+        runCurrent()
+        teardowns shouldBe 0
+
+        // The re-armed window runs from the start request, not from the state emission.
+        advanceTimeBy(bumpedAt)
+        teardowns shouldBe 0
+
+        runCurrent()
+        teardowns shouldBe 1
+
+        job.cancel()
+    }
+
+    @Test
+    fun `a connected device aborts the countdown`() = runTest {
+        var teardowns = 0
+        val job = monitorModeFlow(
+            tag = TAG,
+            modeStates = flowOf(state(connectedAddresses = setOf(KNOWN_ADDRESS))),
+            startSignal = MutableStateFlow(0L),
+            onTeardown = { teardowns++ },
+        ).launchIn(this)
+
+        advanceTimeBy(10 * TIMEOUT)
+        runCurrent()
+        teardowns shouldBe 0
+
+        job.cancel()
+    }
+
+    @Test
+    fun `manual mode tears the session down immediately`() = runTest {
+        var teardowns = 0
+        val job = monitorModeFlow(
+            tag = TAG,
+            modeStates = flowOf(state(mode = MonitorMode.MANUAL)),
+            startSignal = MutableStateFlow(0L),
+            onTeardown = { teardowns++ },
+        ).launchIn(this)
+
+        runCurrent()
+        teardowns shouldBe 1
+
+        job.cancel()
+    }
+
+    companion object {
+        private const val TAG = "MonitorModeFlowTest"
+        private const val TIMEOUT = 15 * 1000L
+        private const val KNOWN_ADDRESS = "AA:BB:CC:DD:EE:FF"
+    }
+}
@@ -13,6 +13,7 @@ import io.kotest.matchers.types.shouldNotBeSameInstanceAs
 import io.mockk.mockk
 import io.mockk.verify
 import kotlinx.coroutines.Job
+import kotlinx.coroutines.flow.StateFlow
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.Robolectric
@@ -42,6 +43,8 @@ class MonitorServiceTest {
    private fun MonitorService.getField(name: String): Any? =
        MonitorService::class.java.getDeclaredField(name).apply { isAccessible = true }.get(this)

+    private fun MonitorService.startSignal(): Long = (getField("startSignal") as StateFlow<*>).value as Long
+
    private fun notification(title: String): Notification =
        NotificationCompat.Builder(context, MonitorNotifications.NOTIFICATION_CHANNEL_ID)
            .setContentTitle(title)
@@ -204,6 +207,22 @@ class MonitorServiceTest {
        service.getField("lastNotification").shouldBeNull()
    }

+    /**
+     * A start request that finds a live session is acknowledged without touching it — including a
+     * teardown countdown that is already running. Bumping the signal is what re-arms that window.
+     */
+    @Test
+    fun `a short-circuited start bumps the start signal`() {
+        val service = createService()
+        service.readyForMonitoring()
+
+        val before = service.startSignal()
+
+        service.onStartCommand(MonitorService.intent(context), 0, 1) shouldBe Service.START_STICKY
+
+        service.startSignal() shouldBe before + 1
+    }
+
    @Test
    fun `onDestroy skips notification cleanup when injection never completed`() {
        val service = createService()
@@ -27,8 +27,11 @@ class PodHistoryFuzzyCollisionTest : BaseBlePodsTest() {
    // IRK + a resolvable RPA pair lifted from RPACheckerTest.
    private val irkHex = "79-04-65-1E-E2-CC-D9-26-F2-6E-20-EE-3E-CC-DE-79"
    private val myRpa = "5A:16:2B:91:D1:CD"        // resolves against irkHex
+    private val myRotatedRpa = "45:23:51:E3:40:6E" // also resolves against irkHex (computed)
    private val foreignAddress = "77:49:4C:D8:25:0C" // does NOT resolve against irkHex

+    private fun String.reversedOctets(): String = split(":").reversed().joinToString(":")
+
    // A valid AirPods Pro 2 (USB-C) proximity advertisement (model 0x2420), reused for both frames.
    private val payload = "07 19 01 24 20 0B 99 8F 11 00 04 BD A7 3B FF 2D 8A 3C AF 9B 1A 7C 74 B7 A9 D1 C3"

@@ -75,7 +78,6 @@ class PodHistoryFuzzyCollisionTest : BaseBlePodsTest() {
                address = "AA:BB:CC:DD:EE:FF",
            )
        )
-        val myRotatedRpa = "45:23:51:E3:40:6E" // also resolves against irkHex (computed)

        lateinit var first: AirPodsPro2Usbc
        create<AirPodsPro2Usbc>(payload, address = myRpa) { first = this }
@@ -87,4 +89,84 @@ class PodHistoryFuzzyCollisionTest : BaseBlePodsTest() {
        // Same physical device across rotation -> same stable identity (recovered via IRK).
        second.identifier shouldBe first.identifier
    }
+
+    @Test
+    fun `a device whose address arrives octet-reversed keeps one identity across a rotation`() = runTest {
+        profileList.add(
+            AppleDeviceProfile(
+                label = "Mine",
+                model = PodModel.AIRPODS_PRO2_USBC,
+                identityKey = irkHex.fromHex(),
+                address = "AA:BB:CC:DD:EE:FF",
+            )
+        )
+
+        lateinit var first: AirPodsPro2Usbc
+        create<AirPodsPro2Usbc>(payload, address = myRpa.reversedOctets()) { first = this }
+        first.meta.isIRKMatch shouldBe true
+        first.meta.profile shouldNotBe null
+
+        lateinit var second: AirPodsPro2Usbc
+        create<AirPodsPro2Usbc>(payload, address = myRotatedRpa.reversedOctets()) { second = this }
+        second.meta.isIRKMatch shouldBe true
+        second.identifier shouldBe first.identifier
+    }
+
+    @Test
+    fun `a reversed foreign address must not inherit a keyed device's identity`() = runTest {
+        profileList.add(
+            AppleDeviceProfile(
+                label = "Mine",
+                model = PodModel.AIRPODS_PRO2_USBC,
+                identityKey = irkHex.fromHex(),
+                address = "AA:BB:CC:DD:EE:FF",
+            )
+        )
+
+        lateinit var mine: AirPodsPro2Usbc
+        create<AirPodsPro2Usbc>(payload, address = myRpa) { mine = this }
+        mine.meta.isIRKMatch shouldBe true
+
+        // The reversed foreign address is RPA-shaped in its reversed form, so the alternate-order
+        // path runs its hash comparison on it — and must still refuse to attribute it.
+        lateinit var foreign: AirPodsPro2Usbc
+        create<AirPodsPro2Usbc>(payload, address = foreignAddress.reversedOctets()) { foreign = this }
+        foreign.meta.isIRKMatch shouldBe false
+        foreign.meta.profile shouldBe null
+        foreign.identifier shouldNotBe mine.identifier
+    }
+
+    @Test
+    fun `a history bound to another profile is not claimed by identity recovery`() = runTest {
+        // Both profiles carry the SAME key: with two independent keys the candidate would already
+        // fail the key check and the binding check would never be reached.
+        val profileOne = AppleDeviceProfile(
+            label = "First",
+            model = PodModel.AIRPODS_PRO2_USBC,
+            identityKey = irkHex.fromHex(),
+            address = "AA:BB:CC:DD:EE:F1",
+        )
+        val profileTwo = AppleDeviceProfile(
+            label = "Second",
+            model = PodModel.AIRPODS_PRO2_USBC,
+            identityKey = irkHex.fromHex(),
+            address = "AA:BB:CC:DD:EE:F2",
+        )
+        profileList.add(profileOne)
+        profileList.add(profileTwo)
+
+        // The first resolving profile wins, so this frame binds the history to profileOne.
+        lateinit var first: AirPodsPro2Usbc
+        create<AirPodsPro2Usbc>(payload, address = myRpa) { first = this }
+        first.meta.profile?.id shouldBe profileOne.id
+
+        // profileOne is gone, so the next frame resolves to profileTwo — which must not be handed
+        // the history that is already bound to profileOne.
+        profileList.remove(profileOne)
+
+        lateinit var second: AirPodsPro2Usbc
+        create<AirPodsPro2Usbc>(payload, address = myRotatedRpa) { second = this }
+        second.meta.profile?.id shouldBe profileTwo.id
+        second.identifier shouldNotBe first.identifier
+    }
 }
Author	SHA1	Message	Date
darken	92dac661c4	fix(device): Keep a log line for rejected malformed addresses Structurally broken addresses now fail the octet parse instead of throwing, so the catch-all error log no longer fires for them. Log the rejection at WARN with a redacted address so support logs still show it. Fixes review finding F1.	2026-08-31 15:54:51 +02:00
darken	7712b18388	fix(monitor): Re-arm the teardown when a start request is short-circuited In AUTOMATIC mode with nothing connected the session is torn down 15 seconds after the state that said so, and only a new state emission cancels that. A start request arriving while a session is live is acknowledged and returns without touching the pending teardown, and the connection state that would abort it lags the Bluetooth event that caused the start by roughly 0.75s. A request landing in the tail of the window was therefore answered with "keeping current session" and the session went down anyway, taking every reaction with it — in one recording the popup reaction was down for 34 seconds spanning an entire lid cycle. The short-circuit now bumps a start signal that the mode pipeline combines in, so the bump cancels the pending inner flow through the existing cancellation topology and arms a fresh window. The countdown also re-reads the signal after its delay, which closes the case where the bump lands while an expired countdown is already unwinding. A re-armed window runs 15 seconds from the start request, so the total dwell can exceed 15 seconds: the request is fresh evidence of activity. The decision segment moves to a top-level internal function so it can be driven directly in tests, following MonitorModeState and buildMonitorModeState which are top-level for the same reason.	2026-08-31 15:54:51 +02:00
darken	d7988189a0	fix(reaction): Stamp connect times from the ACL broadcast The connection timestamp cache was only maintained while the connected-devices flow had a subscriber. If the process died while a device stayed connected and that device then disconnected and reconnected with nothing running, the restarted collection found the old entry still keyed by a currently-connected address, kept it through the prune, and reported the original connect time — so the popup age check rejected an arbitrarily old connection. The ACL broadcasts arrive at a manifest-registered receiver that wakes the process regardless of any flow subscription, so the stamp is taken there instead: connect stamps (keeping an existing one, which is the earlier and therefore truer time), disconnect drops the entry. The flow keeps its own stamp-on-first-sight and prune as a backstop for the force-stopped state and missed broadcasts. ACL_DISCONNECTED was already in the receiver's expected actions but was never registered in the manifest. It does not start the monitor: a disconnect is not a reason to start monitoring, and the start triggers here are deliberately conservative.	2026-08-31 15:54:51 +02:00
darken	ff5d0cedfa	fix(reaction): Stop bonded-device queries from poisoning connect times seenFirstAt on a connected device is meant to be the connect time, and the connected-devices flow maintains that by pruning its cache to the currently connected addresses on every emission. bondedDevices() wrote into the same cache for every bonded device, connected or not, and never pruned. AapAutoConnect queries bonded devices on every connected-devices emission, the disconnect one included, so an entry pruned at disconnect was re-stamped milliseconds later at disconnect time. The next reconnect then inherited the previous disconnect as its connect time, and the popup reaction rejects a connection older than 30 seconds — so any reconnect more than half a minute after the previous disconnect silently lost its popup. bondedDevices() is now a cache reader: a connected bonded device still reports its true connect time, a non-connected one gets the current time, which no caller reads. The connected-devices path becomes the cache's only writer, which is the invariant its prune-and-stamp logic already assumed.	2026-08-31 15:54:51 +02:00
darken	f82adeba42	fix(monitor): Persist AAP keys only when their content changed AppleDeviceProfile is a data class whose key fields are ByteArrays, so the generated equals() compares them by reference and the "did anything change" guard was true on every key exchange. That meant a redundant profile write on every connect and a "Persisted keys" line that said nothing about whether a key had actually changed. Compare content instead, per key, and name the key that changed in the log.	2026-08-31 15:54:51 +02:00
darken	cc99ad59ab	fix(device): Resolve identity keys against reversed address octets On at least one vendor stack the address handed up by the BLE scan callback and the identity key stored for a profile disagree on octet order: the key resolves the address only when its octets are reversed. Identity resolution then fails on every advertisement, so no frame is attributed to the profile and the case popup, connection popup, encrypted 1% battery granularity and session reconnection all go with it. RPAChecker gains resolve(), which reports the order that resolved. The standard-order attempt is unchanged and ungated, so no currently-resolving device can start failing. The reversed attempt only runs when the reversed form carries the resolvable-private-address type marker (top bits 01), which skips roughly three quarters of the extra comparisons for generic random addresses. verify() is now a thin wrapper over resolve(). Address parsing is validated explicitly: exactly six components, each in 0..255. A seven-component string used to be silently mis-sliced, and "15A:..." truncated to a valid octet — both cases could resolve. The failure log no longer prints the identity key. A malformed address or key wrote an identity-tracking secret into exactly the debug logs users mail to support; the event and its level stay, the key is reduced to its length and the address goes through redactedForLogs(). The history lookup that recovers a device by key now skips candidates bound to a different profile. A 24-bit forward collision could already select the wrong history; attempting two orders roughly doubles that exposure. Test vectors are synthetic and derived from the key already committed in RPACheckerTest. Two of them are complementary: one has an RPA-shaped reversed form that fails the hash (proving the comparison runs), the other has a reversed form that resolves cryptographically but carries marker 00 (proving the gate runs).	2026-08-31 15:54:51 +02:00