diff --git a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImplTest.kt b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImplTest.kt index 8f9e6948bb..900e70dd24 100644 --- a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImplTest.kt +++ b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConnectionManagerImplTest.kt @@ -16,10 +16,7 @@ */ package org.meshtastic.core.data.manager -import co.touchlab.kermit.LogWriter -import co.touchlab.kermit.Logger import co.touchlab.kermit.Severity -import co.touchlab.kermit.platformLogWriter import dev.mokkery.MockMode import dev.mokkery.answering.calls import dev.mokkery.answering.returns @@ -67,6 +64,7 @@ import org.meshtastic.core.repository.RadioInterfaceService import org.meshtastic.core.repository.ServiceRepository import org.meshtastic.core.repository.SessionManager import org.meshtastic.core.repository.UiPrefs +import org.meshtastic.core.testing.CapturingLogWriter import org.meshtastic.core.testing.FakeLockdownCoordinator import org.meshtastic.core.testing.FakeNodeRepository import org.meshtastic.proto.Config @@ -195,22 +193,9 @@ class MeshConnectionManagerImplTest { @AfterTest fun tearDown() { - Logger.setLogWriters(platformLogWriter()) + CapturingLogWriter.uninstall() } - private class CapturingLogWriter : LogWriter() { - val entries = mutableListOf>() - - override fun log(severity: Severity, message: String, tag: String, throwable: Throwable?) { - entries += severity to message - } - } - - private fun captureLogs(): CapturingLogWriter = CapturingLogWriter().also { Logger.setLogWriters(it) } - - private fun CapturingLogWriter.messages(severity: Severity): List = - entries.filter { it.first == severity }.map { it.second } - @Test fun `Connected state triggers broadcast and config start`() = runTest(testDispatcher) { manager = createManager(backgroundScope) @@ -991,7 +976,7 @@ class MeshConnectionManagerImplTest { @Test fun `TCP Stage 1 stall report names TCP`() = runTest(testDispatcher) { every { radioInterfaceService.getDeviceAddress() } returns "t192.168.1.42" - val logs = captureLogs() + val logs = CapturingLogWriter.install() manager = createManager(backgroundScope) radioConnectionState.value = ConnectionState.Connected advanceTimeBy(200) @@ -1012,7 +997,7 @@ class MeshConnectionManagerImplTest { @Test fun `USB Stage 1 stall report names USB`() = runTest(testDispatcher) { every { radioInterfaceService.getDeviceAddress() } returns "s/dev/bus/usb/001/002" - val logs = captureLogs() + val logs = CapturingLogWriter.install() manager = createManager(backgroundScope) radioConnectionState.value = ConnectionState.Connected advanceTimeBy(200) @@ -1034,7 +1019,7 @@ class MeshConnectionManagerImplTest { @Test fun `Demo Mode keeps the fast stall budget and names MOCK`() = runTest(testDispatcher) { every { radioInterfaceService.getDeviceAddress() } returns "m" - val logs = captureLogs() + val logs = CapturingLogWriter.install() manager = createManager(backgroundScope) radioConnectionState.value = ConnectionState.Connected advanceTimeBy(200) @@ -1056,7 +1041,7 @@ class MeshConnectionManagerImplTest { @Test fun `BLE Stage 1 stall report names BLE`() = runTest(testDispatcher) { every { radioInterfaceService.getDeviceAddress() } returns "xAA:BB:CC:DD:EE:FF" - val logs = captureLogs() + val logs = CapturingLogWriter.install() manager = createManager(backgroundScope) radioConnectionState.value = ConnectionState.Connected advanceTimeBy(200) @@ -1075,7 +1060,7 @@ class MeshConnectionManagerImplTest { @Test fun `TCP fast watchdog report names TCP`() = runTest(testDispatcher) { every { radioInterfaceService.getDeviceAddress() } returns "t192.168.1.42" - val logs = captureLogs() + val logs = CapturingLogWriter.install() manager = createManager(backgroundScope) radioConnectionState.value = ConnectionState.Connected advanceTimeBy(200) diff --git a/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/BootloaderWarningDataSource.kt b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/BootloaderWarningDataSource.kt index ce6f9d0497..78617ad44e 100644 --- a/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/BootloaderWarningDataSource.kt +++ b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/BootloaderWarningDataSource.kt @@ -21,9 +21,8 @@ import androidx.datastore.preferences.core.stringPreferencesKey import co.touchlab.kermit.Logger import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map -import kotlinx.serialization.SerializationException -import kotlinx.serialization.json.Json import org.koin.core.annotation.Single +import org.meshtastic.core.common.util.safeCatching import org.meshtastic.core.datastore.di.CorePreferencesDataStore @Single @@ -37,13 +36,10 @@ open class BootloaderWarningDataSource(private val dataStore: CorePreferencesDat dataStore.data.map { preferences -> val jsonString = preferences[PreferencesKeys.DISMISSED_BOOTLOADER_ADDRESSES] ?: return@map emptySet() - runCatching { Json.decodeFromString>(jsonString).toSet() } + // The stored value is a list of device addresses, so the log names only the exception type. + safeCatching { DatastoreJson.decodeFromString>(jsonString).toSet() } .onFailure { e -> - if (e is IllegalArgumentException || e is SerializationException) { - Logger.w(e) { "Failed to parse dismissed bootloader warning addresses, resetting preference" } - } else { - Logger.w(e) { "Unexpected error while parsing dismissed bootloader warning addresses" } - } + Logger.w { "Ignoring unreadable dismissed bootloader warning addresses (${e::class.simpleName})" } } .getOrDefault(emptySet()) } @@ -58,7 +54,7 @@ open class BootloaderWarningDataSource(private val dataStore: CorePreferencesDat val updated = (current + address).toList() dataStore.edit { preferences -> - preferences[PreferencesKeys.DISMISSED_BOOTLOADER_ADDRESSES] = Json.encodeToString(updated) + preferences[PreferencesKeys.DISMISSED_BOOTLOADER_ADDRESSES] = DatastoreJson.encodeToString(updated) } } } diff --git a/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/DatastoreJson.kt b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/DatastoreJson.kt new file mode 100644 index 0000000000..e03f44a184 --- /dev/null +++ b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/DatastoreJson.kt @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.datastore + +import kotlinx.serialization.ExperimentalSerializationApi +import kotlinx.serialization.json.Json + +/** + * Json for the JSON strings this module stores, which hold user data such as device addresses. Exceptions omit the JSON + * input, though a message can still quote one token, so callers log only the exception type. Not injected from Koin: + * Json is sealed, so Mokkery could no longer mock a data source that takes one. + */ +@OptIn(ExperimentalSerializationApi::class) +internal val DatastoreJson = Json { exceptionsWithDebugInfo = false } diff --git a/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/FirmwareRecoveryDataSource.kt b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/FirmwareRecoveryDataSource.kt index b1496685d8..c7471b98cf 100644 --- a/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/FirmwareRecoveryDataSource.kt +++ b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/FirmwareRecoveryDataSource.kt @@ -21,9 +21,8 @@ import androidx.datastore.preferences.core.stringPreferencesKey import co.touchlab.kermit.Logger import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map -import kotlinx.serialization.SerializationException -import kotlinx.serialization.json.Json import org.koin.core.annotation.Single +import org.meshtastic.core.common.util.safeCatching import org.meshtastic.core.datastore.di.CorePreferencesDataStore import org.meshtastic.core.datastore.model.PendingFirmwareRecovery @@ -42,20 +41,19 @@ open class FirmwareRecoveryDataSource(private val dataStore: CorePreferencesData open val pending: Flow = dataStore.data.map { preferences -> val jsonString = preferences[PreferencesKeys.PENDING_RECOVERY] ?: return@map null - runCatching { Json.decodeFromString(jsonString) } + // The stored record holds the device address and name, so the log names only the exception type. + safeCatching { DatastoreJson.decodeFromString(jsonString) } .onFailure { e -> - if (e is IllegalArgumentException || e is SerializationException) { - Logger.w(e) { "Failed to parse pending firmware recovery, clearing preference" } - } else { - Logger.w(e) { "Unexpected error parsing pending firmware recovery" } - } + Logger.w { "Ignoring unreadable pending firmware recovery (${e::class.simpleName})" } } .getOrNull() } /** Records [recovery] as the outstanding interrupted update, replacing any previous record. */ open suspend fun set(recovery: PendingFirmwareRecovery) { - dataStore.edit { preferences -> preferences[PreferencesKeys.PENDING_RECOVERY] = Json.encodeToString(recovery) } + dataStore.edit { preferences -> + preferences[PreferencesKeys.PENDING_RECOVERY] = DatastoreJson.encodeToString(recovery) + } } /** Clears the outstanding recovery record (update finished, or the device returned on its own). */ diff --git a/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/RecentAddressesDataSource.kt b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/RecentAddressesDataSource.kt index 3fb79f3c08..f92f0eaf62 100644 --- a/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/RecentAddressesDataSource.kt +++ b/core/datastore/src/commonMain/kotlin/org/meshtastic/core/datastore/RecentAddressesDataSource.kt @@ -22,18 +22,19 @@ import co.touchlab.kermit.Logger import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map -import kotlinx.serialization.SerializationException -import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.contentOrNull import kotlinx.serialization.json.jsonArray -import kotlinx.serialization.json.jsonPrimitive import org.koin.core.annotation.Single import org.meshtastic.core.datastore.di.CorePreferencesDataStore import org.meshtastic.core.datastore.model.RecentAddress +/** + * The stored addresses and names are user data, so no log line here carries the stored value or an exception message, + * which kotlinx.serialization can fill with the input it failed on. + */ @Single open class RecentAddressesDataSource(private val dataStore: CorePreferencesDataStore) { private object PreferencesKeys { @@ -45,14 +46,10 @@ open class RecentAddressesDataSource(private val dataStore: CorePreferencesDataS val jsonString = preferences[PreferencesKeys.RECENT_IP_ADDRESSES] if (jsonString != null) { try { - Json.decodeFromString>(jsonString) + DatastoreJson.decodeFromString>(jsonString) } catch (e: IllegalArgumentException) { - Logger.w { "Could not parse recent addresses, falling back to legacy parsing: ${e.message}" } - // Fallback to legacy parsing - parseLegacyRecentAddresses(jsonString) - } catch (e: SerializationException) { - Logger.w { "Could not parse recent addresses, falling back to legacy parsing: ${e.message}" } - // Fallback to legacy parsing + // SerializationException is an IllegalArgumentException. + Logger.w { "Could not parse recent addresses (${e::class.simpleName}), trying legacy format" } parseLegacyRecentAddresses(jsonString) } } else { @@ -60,19 +57,21 @@ open class RecentAddressesDataSource(private val dataStore: CorePreferencesDataS } } - private fun parseLegacyRecentAddresses(jsonAddresses: String): List { - val jsonArray = Json.parseToJsonElement(jsonAddresses).jsonArray - return jsonArray.mapNotNull(::parseLegacyRecentAddress) + private fun parseLegacyRecentAddresses(jsonAddresses: String): List = try { + DatastoreJson.parseToJsonElement(jsonAddresses).jsonArray.mapNotNull(::parseLegacyRecentAddress) + } catch (e: IllegalArgumentException) { + Logger.w { "Discarding unreadable recent addresses (${e::class.simpleName})" } + emptyList() } private fun parseLegacyRecentAddress(item: kotlinx.serialization.json.JsonElement): RecentAddress? = when (item) { is JsonObject -> { - val address = item["address"]?.jsonPrimitive?.contentOrNull - val name = item["name"]?.jsonPrimitive?.contentOrNull + val address = (item["address"] as? JsonPrimitive)?.contentOrNull + val name = (item["name"] as? JsonPrimitive)?.contentOrNull if (address != null && name != null) { RecentAddress(address = address, name = name) } else { - Logger.w { "Skipping malformed recent address object: $item" } + Logger.w { "Skipping malformed recent address object" } null } } @@ -82,20 +81,20 @@ open class RecentAddressesDataSource(private val dataStore: CorePreferencesDataS if (address != null) { RecentAddress(address = address, name = "Meshtastic") } else { - Logger.w { "Skipping malformed recent address primitive: $item" } + Logger.w { "Skipping malformed recent address primitive" } null } } is JsonArray -> { - Logger.w { "Skipping nested array in recent IP addresses: $item" } + Logger.w { "Skipping nested array in recent IP addresses" } null } } open suspend fun setRecentAddresses(addresses: List) { dataStore.edit { preferences -> - preferences[PreferencesKeys.RECENT_IP_ADDRESSES] = Json.encodeToString(addresses) + preferences[PreferencesKeys.RECENT_IP_ADDRESSES] = DatastoreJson.encodeToString(addresses) } } diff --git a/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/BootloaderWarningDataSourceTest.kt b/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/BootloaderWarningDataSourceTest.kt new file mode 100644 index 0000000000..7158b045e1 --- /dev/null +++ b/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/BootloaderWarningDataSourceTest.kt @@ -0,0 +1,88 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.datastore + +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringPreferencesKey +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import okio.FileSystem +import okio.Path +import org.meshtastic.core.datastore.di.asCorePreferencesDataStore +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import kotlin.uuid.Uuid + +class BootloaderWarningDataSourceTest { + private lateinit var tmpDir: Path + private lateinit var dataStore: DataStore + private lateinit var dataSource: BootloaderWarningDataSource + private lateinit var logs: CapturingLogWriter + + private val testScope = TestScope(UnconfinedTestDispatcher()) + + @BeforeTest + fun setup() { + tmpDir = FileSystem.SYSTEM_TEMPORARY_DIRECTORY / "bootloaderWarningTest-${Uuid.random()}" + FileSystem.SYSTEM.createDirectories(tmpDir) + dataStore = + PreferenceDataStoreFactory.createWithPath( + scope = testScope, + produceFile = { tmpDir / "test.preferences_pb" }, + ) + dataSource = BootloaderWarningDataSource(dataStore.asCorePreferencesDataStore()) + logs = CapturingLogWriter.install() + } + + @AfterTest + fun tearDown() { + CapturingLogWriter.uninstall() + FileSystem.SYSTEM.deleteRecursively(tmpDir) + } + + @Test + fun `a dismissed address reads back as dismissed`() = testScope.runTest { + dataSource.dismiss("AA:BB:CC:00:00:01") + + assertTrue(dataSource.isDismissed("AA:BB:CC:00:00:01")) + assertFalse(dataSource.isDismissed("AA:BB:CC:00:00:02")) + } + + @Test + fun `unreadable stored value counts as not dismissed and is never logged`() = testScope.runTest { + dataStore.edit { it[stringPreferencesKey("dismissed-bootloader-addresses")] = """["AA:BB:CC:11:22:33",""" } + + assertFalse(dataSource.isDismissed("AA:BB:CC:11:22:33")) + logs.assertNotLogged("AA:BB:CC:11:22:33") + } + + @Test + fun `dismiss replaces an unreadable stored value with a readable one`() = testScope.runTest { + dataStore.edit { it[stringPreferencesKey("dismissed-bootloader-addresses")] = """["AA:BB:CC:11:22:33",""" } + + dataSource.dismiss("AA:BB:CC:00:00:01") + + assertTrue(dataSource.isDismissed("AA:BB:CC:00:00:01")) + } +} diff --git a/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/CapturingLogWriter.kt b/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/CapturingLogWriter.kt new file mode 100644 index 0000000000..8a09d10f06 --- /dev/null +++ b/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/CapturingLogWriter.kt @@ -0,0 +1,55 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.datastore + +import co.touchlab.kermit.LogWriter +import co.touchlab.kermit.Logger +import co.touchlab.kermit.Severity +import co.touchlab.kermit.platformLogWriter +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Records every Kermit line, including an attached throwable's full text, so a test can assert what never reached a + * log. Install it in setup and call [uninstall] in teardown: it replaces the global writers. + */ +internal class CapturingLogWriter : LogWriter() { + private val entries = mutableListOf() + + override fun log(severity: Severity, message: String, tag: String, throwable: Throwable?) { + entries += message + throwable?.let { entries += it.stackTraceToString() } + } + + fun assertNotLogged(vararg values: String) { + assertTrue(entries.isNotEmpty(), "Expected the failed parse to log a warning") + for (value in values) { + assertFalse(entries.any { value in it }, "'$value' leaked into logs: $entries") + } + } + + companion object { + fun install(): CapturingLogWriter = CapturingLogWriter().also { + Logger.setLogWriters(it) + Logger.setMinSeverity(Severity.Verbose) + } + + fun uninstall() { + Logger.setLogWriters(platformLogWriter()) + } + } +} diff --git a/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/FirmwareRecoveryDataSourceTest.kt b/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/FirmwareRecoveryDataSourceTest.kt new file mode 100644 index 0000000000..caa866d2be --- /dev/null +++ b/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/FirmwareRecoveryDataSourceTest.kt @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.datastore + +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringPreferencesKey +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import okio.FileSystem +import okio.Path +import org.meshtastic.core.datastore.di.asCorePreferencesDataStore +import org.meshtastic.core.datastore.model.PendingFirmwareRecovery +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.uuid.Uuid + +class FirmwareRecoveryDataSourceTest { + private lateinit var tmpDir: Path + private lateinit var dataStore: DataStore + private lateinit var dataSource: FirmwareRecoveryDataSource + private lateinit var logs: CapturingLogWriter + + private val testScope = TestScope(UnconfinedTestDispatcher()) + + @BeforeTest + fun setup() { + tmpDir = FileSystem.SYSTEM_TEMPORARY_DIRECTORY / "firmwareRecoveryTest-${Uuid.random()}" + FileSystem.SYSTEM.createDirectories(tmpDir) + dataStore = + PreferenceDataStoreFactory.createWithPath( + scope = testScope, + produceFile = { tmpDir / "test.preferences_pb" }, + ) + dataSource = FirmwareRecoveryDataSource(dataStore.asCorePreferencesDataStore()) + logs = CapturingLogWriter.install() + } + + @AfterTest + fun tearDown() { + CapturingLogWriter.uninstall() + FileSystem.SYSTEM.deleteRecursively(tmpDir) + } + + @Test + fun `a recorded recovery reads back until cleared`() = testScope.runTest { + val recovery = + PendingFirmwareRecovery( + fullAddress = "xAA:BB:CC:00:00:01", + hwModel = 9, + pioEnv = "rak4631", + releaseType = "STABLE", + deviceName = "Base", + ) + + dataSource.set(recovery) + assertEquals(recovery, dataSource.pending.first()) + + dataSource.clear() + assertNull(dataSource.pending.first()) + } + + @Test + fun `unreadable stored record reads as null without logging it`() = testScope.runTest { + val truncated = """{"fullAddress":"xAA:BB:CC:11:22:33","deviceName":"CabinBase","hwModel":""" + dataStore.edit { it[stringPreferencesKey("pending-firmware-recovery")] = truncated } + + assertNull(dataSource.pending.first()) + logs.assertNotLogged("AA:BB:CC:11:22:33", "CabinBase") + } +} diff --git a/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/RecentAddressesDataSourceTest.kt b/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/RecentAddressesDataSourceTest.kt index 7584f2a93f..0bfc5d8f21 100644 --- a/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/RecentAddressesDataSourceTest.kt +++ b/core/datastore/src/commonTest/kotlin/org/meshtastic/core/datastore/RecentAddressesDataSourceTest.kt @@ -16,20 +16,15 @@ */ package org.meshtastic.core.datastore +import androidx.datastore.core.DataStore import androidx.datastore.preferences.core.PreferenceDataStoreFactory -import kotlinx.coroutines.flow.Flow +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringPreferencesKey import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.flow import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest -import kotlinx.serialization.json.Json -import kotlinx.serialization.json.JsonArray -import kotlinx.serialization.json.JsonObject -import kotlinx.serialization.json.JsonPrimitive -import kotlinx.serialization.json.contentOrNull -import kotlinx.serialization.json.jsonArray -import kotlinx.serialization.json.jsonPrimitive import okio.FileSystem import okio.Path import org.meshtastic.core.datastore.di.asCorePreferencesDataStore @@ -44,7 +39,9 @@ import kotlin.uuid.Uuid class RecentAddressesDataSourceTest { private lateinit var tmpDir: Path + private lateinit var dataStore: DataStore private lateinit var dataSource: RecentAddressesDataSource + private lateinit var logs: CapturingLogWriter private val testDispatcher = UnconfinedTestDispatcher() private val testScope = TestScope(testDispatcher) @@ -53,19 +50,25 @@ class RecentAddressesDataSourceTest { fun setup() { tmpDir = FileSystem.SYSTEM_TEMPORARY_DIRECTORY / "recentAddressesTest-${Uuid.random()}" FileSystem.SYSTEM.createDirectories(tmpDir) - val dataStore = + dataStore = PreferenceDataStoreFactory.createWithPath( scope = testScope, produceFile = { tmpDir / "test.preferences_pb" }, ) dataSource = RecentAddressesDataSource(dataStore.asCorePreferencesDataStore()) + logs = CapturingLogWriter.install() } @AfterTest fun tearDown() { + CapturingLogWriter.uninstall() FileSystem.SYSTEM.deleteRecursively(tmpDir) } + private suspend fun storeRaw(value: String) { + dataStore.edit { it[stringPreferencesKey("recent-ip-addresses")] = value } + } + // ---- recentAddresses flow ---- @Test @@ -97,6 +100,16 @@ class RecentAddressesDataSourceTest { assertEquals("5.6.7.8", result[0].address) } + @Test + fun `corrupt stored value yields an empty list without logging the stored addresses`() = testScope.runTest { + storeRaw("""[{"address":"10.20.30.40","name":"CabinRadio",""") + + val result = dataSource.recentAddresses.first() + + assertTrue(result.isEmpty()) + logs.assertNotLogged("10.20.30.40", "CabinRadio") + } + // ---- add() LRU behaviour ---- @Test @@ -186,13 +199,12 @@ class RecentAddressesDataSourceTest { assertTrue(dataSource.recentAddresses.first().isEmpty()) } - // ---- legacy JSON parsing (via LegacyParsingHarness) ---- + // ---- legacy stored formats ---- @Test fun `legacy JsonObject array is parsed correctly`() = testScope.runTest { - val legacyJson = - """[{"address":"192.168.1.100","name":"NodeA"},{"address":"192.168.1.101","name":"NodeB"}]""" - val result = LegacyParsingHarness(legacyJson).recentAddresses.first() + storeRaw("""[{"address":"192.168.1.100","name":"NodeA"},{"address":"192.168.1.101","name":"NodeB"}]""") + val result = dataSource.recentAddresses.first() assertEquals(2, result.size) assertEquals("192.168.1.100", result[0].address) @@ -204,8 +216,8 @@ class RecentAddressesDataSourceTest { @Test fun `legacy bare string JsonPrimitive array is parsed correctly`() = testScope.runTest { // Old clients stored plain IP strings with no name field - val legacyJson = """["192.168.1.50","10.0.0.2"]""" - val result = LegacyParsingHarness(legacyJson).recentAddresses.first() + storeRaw("""["192.168.1.50","10.0.0.2"]""") + val result = dataSource.recentAddresses.first() assertEquals(2, result.size) assertEquals("192.168.1.50", result[0].address) @@ -214,10 +226,19 @@ class RecentAddressesDataSourceTest { assertEquals("Meshtastic", result[1].name) } + @Test + fun `legacy value is parsed without logging the stored addresses`() = testScope.runTest { + storeRaw("""["192.168.1.50","10.0.0.2"]""") + + dataSource.recentAddresses.first() + + logs.assertNotLogged("192.168.1.50", "10.0.0.2") + } + @Test fun `legacy JsonObject missing address field is skipped`() = testScope.runTest { - val legacyJson = """[{"name":"NoAddress"},{"address":"1.2.3.4","name":"Good"}]""" - val result = LegacyParsingHarness(legacyJson).recentAddresses.first() + storeRaw("""[{"name":"NoAddress"},{"address":"1.2.3.4","name":"Good"}]""") + val result = dataSource.recentAddresses.first() assertEquals(1, result.size) assertEquals("1.2.3.4", result[0].address) @@ -225,63 +246,44 @@ class RecentAddressesDataSourceTest { @Test fun `legacy JsonObject missing name field is skipped`() = testScope.runTest { - val legacyJson = """[{"address":"1.2.3.4"},{"address":"5.6.7.8","name":"Good"}]""" - val result = LegacyParsingHarness(legacyJson).recentAddresses.first() + storeRaw("""[{"address":"1.2.3.4"},{"address":"5.6.7.8","name":"Good"}]""") + val result = dataSource.recentAddresses.first() assertEquals(1, result.size) assertEquals("5.6.7.8", result[0].address) } + @Test + fun `legacy JsonObject with non-primitive fields is skipped and keeps the other entries`() = testScope.runTest { + storeRaw( + """[{"address":{},"name":"BadA"},{"address":"9.9.9.9","name":["BadB"]},""" + + """{"address":"1.2.3.4","name":"Good"}]""", + ) + val result = dataSource.recentAddresses.first() + + assertEquals(listOf(RecentAddress("1.2.3.4", "Good")), result) + logs.assertNotLogged("BadA", "9.9.9.9", "BadB", "1.2.3.4", "Good") + } + @Test fun `legacy nested JsonArray entries are skipped`() = testScope.runTest { - val legacyJson = """[["nested","array"],{"address":"1.2.3.4","name":"Good"}]""" - val result = LegacyParsingHarness(legacyJson).recentAddresses.first() + storeRaw("""[["nested","array"],{"address":"1.2.3.4","name":"Good"}]""") + val result = dataSource.recentAddresses.first() assertEquals(1, result.size) assertEquals("1.2.3.4", result[0].address) } @Test - fun `legacy mixed array handles all element types`() = testScope.runTest { + fun `legacy mixed array handles all element types without logging entries`() = testScope.runTest { // JsonPrimitive + valid JsonObject + malformed JsonObject + nested JsonArray - val legacyJson = """["10.0.0.1",{"address":"10.0.0.2","name":"Node"},{"name":"bad"},[1,2]]""" - val result = LegacyParsingHarness(legacyJson).recentAddresses.first() + storeRaw("""["10.0.0.1",{"address":"10.0.0.2","name":"Node"},{"name":"BadEntryName"},["10.9.9.9"]]""") + val result = dataSource.recentAddresses.first() assertEquals(2, result.size) assertEquals("10.0.0.1", result[0].address) assertEquals("Meshtastic", result[0].name) assertEquals("10.0.0.2", result[1].address) - } -} - -/** - * Test harness that mirrors the private legacy parsing logic of [RecentAddressesDataSource] without needing to bypass - * encapsulation. Exposes a [Flow] that emits the result of parsing a raw legacy JSON string using the same rules as the - * production fallback path. - */ -private class LegacyParsingHarness(private val rawJson: String) { - val recentAddresses: Flow> = flow { - val jsonArray = Json.parseToJsonElement(rawJson).jsonArray - emit( - jsonArray.mapNotNull { item -> - when (item) { - is JsonObject -> { - val address = item["address"]?.jsonPrimitive?.contentOrNull - val name = item["name"]?.jsonPrimitive?.contentOrNull - if (address != null && name != null) { - RecentAddress(address = address, name = name) - } else { - null - } - } - - is JsonPrimitive -> { - item.contentOrNull?.let { RecentAddress(address = it, name = "Meshtastic") } - } - - is JsonArray -> null - } - }, - ) + logs.assertNotLogged("10.0.0.1", "10.0.0.2", "BadEntryName", "10.9.9.9") } } diff --git a/core/konsist/src/jvmTest/kotlin/org/meshtastic/core/konsist/BleAddressLoggingTest.kt b/core/konsist/src/jvmTest/kotlin/org/meshtastic/core/konsist/BleAddressLoggingTest.kt index ac95f15ff4..93e1bc29e5 100644 --- a/core/konsist/src/jvmTest/kotlin/org/meshtastic/core/konsist/BleAddressLoggingTest.kt +++ b/core/konsist/src/jvmTest/kotlin/org/meshtastic/core/konsist/BleAddressLoggingTest.kt @@ -29,12 +29,23 @@ import kotlin.test.assertTrue * attempt anonymised the hand-written log statements in `core/ble` and missed the Kable `identifier`, which stamps the * address onto *every* line the BLE library emits, plus further sites in the DFU transports and WiFi provisioning. * - * Scoped to the BLE-adjacent modules so matching on the `address` suffix stays low-noise. + * Scoped to the BLE-adjacent modules so matching on the `address` suffix stays low-noise. That scope includes the + * transport modules, so TCP hosts go through `anonymizePublicHost()`, which keeps a host on the user's own network + * readable. */ class BleAddressLoggingTest { private val scannedPathFragments = - listOf("/core/ble/", "/feature/firmware/", "/feature/wifi-provision/", "/feature/connections/") + listOf( + "/core/ble/", + "/core/network/", + "/core/service/", + "/feature/firmware/", + "/feature/wifi-provision/", + "/feature/connections/", + "/androidApp/", + "/desktopApp/", + ) /** * Files where an address is used as an identity rather than as diagnostic text — building the connection string or @@ -42,6 +53,12 @@ class BleAddressLoggingTest { */ private val identityUseAllowlist = listOf("DeviceListEntry.kt") + /** + * Files whose `address` names hardware, not a person. The Android serial transport's address is the USB + * vendor-product pair (`usbSerialStableKey()`), which identifies the chip model. + */ + private val notPersonalAddressFiles = listOf("SerialRadioTransport.kt") + /** Interpolation of anything ending in `address`, e.g. `${device.address}` or `$address`. */ private val interpolatedAddress = Regex("""\$\{?[A-Za-z0-9_.]*[aA]ddress}?""") @@ -56,16 +73,19 @@ class BleAddressLoggingTest { .filterNot { it.isNestedAgentWorktree() } .filter { file -> scannedPathFragments.any { it in file.scanPath } } .filterNot { file -> identityUseAllowlist.any { file.scanPath.endsWith(it) } } + .filterNot { file -> notPersonalAddressFiles.any { file.scanPath.endsWith(it) } } @Test fun `the scan actually reaches the BLE sources`() { val paths = scannedFiles().map { it.scanPath } assertTrue(paths.isNotEmpty(), emptyScanMessage("BLE-scoped scan")) - assertTrue( - paths.any { it.endsWith("KableBleConnection.kt") }, - "expected core/ble sources in scope; got ${paths.size} files, e.g. ${paths.take(3)}", - ) + for (file in listOf("KableBleConnection.kt", "BleRadioTransport.kt", "SharedRadioInterfaceService.kt")) { + assertTrue( + paths.any { it.endsWith(file) }, + "expected $file in scope; got ${paths.size} files, e.g. ${paths.take(3)}", + ) + } } @Test diff --git a/core/model/src/commonMain/kotlin/org/meshtastic/core/model/util/HostAnonymize.kt b/core/model/src/commonMain/kotlin/org/meshtastic/core/model/util/HostAnonymize.kt new file mode 100644 index 0000000000..4066130747 --- /dev/null +++ b/core/model/src/commonMain/kotlin/org/meshtastic/core/model/util/HostAnonymize.kt @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +@file:Suppress("MagicNumber") + +package org.meshtastic.core.model.util + +/** + * [anonymize] for a TCP `host`, `host:port` or `[ipv6]:port`, except that a host which can only name a machine on the + * user's own network is returned whole: loopback, RFC 1918, IPv4 and IPv6 link-local, IPv6 unique local, or an mDNS + * `.local` name. A public address or DNS name, DDNS and MagicDNS included, can identify the user, so it is anonymized. + */ +fun String.anonymizePublicHost(): String = if (isLocalNetworkHost(hostOf(this))) this else anonymize() + +private fun hostOf(address: String): String { + val host = + when { + address.startsWith("[") -> address.substringAfter('[').substringBefore(']') + address.count { it == ':' } == 1 -> address.substringBefore(':') + else -> address + } + return host.substringBefore('%').trimEnd('.').lowercase() +} + +private fun isLocalNetworkHost(host: String): Boolean = + host == "localhost" || host.endsWith(".local") || isLocalIpv4(host) || isLocalIpv6(host) + +private fun isLocalIpv4(host: String): Boolean { + val parts = host.split('.') + val octets = parts.mapNotNull { part -> part.toIntOrNull()?.takeIf { it in 0..255 } } + if (parts.size != 4 || octets.size != 4) return false + val (first, second) = octets + return when (first) { + 127, + 10, + -> true + + 172 -> second in 16..31 + + 192 -> second == 168 + + 169 -> second == 254 + + else -> false + } +} + +private fun isLocalIpv6(host: String): Boolean { + val isLiteral = host.count { it == ':' } >= 2 && host.all { it == ':' || it.digitToIntOrNull(16) != null } + val firstHextet = host.substringBefore(':').takeIf { it.length in 1..4 }?.toIntOrNull(16) + // fe80::/10 link-local, fc00::/7 unique local. + val isLocalRange = firstHextet != null && (firstHextet in 0xfe80..0xfebf || firstHextet in 0xfc00..0xfdff) + return isLiteral && (host == "::1" || isLocalRange) +} diff --git a/core/model/src/commonTest/kotlin/org/meshtastic/core/model/util/HostAnonymizeTest.kt b/core/model/src/commonTest/kotlin/org/meshtastic/core/model/util/HostAnonymizeTest.kt new file mode 100644 index 0000000000..fbce2ecaf5 --- /dev/null +++ b/core/model/src/commonTest/kotlin/org/meshtastic/core/model/util/HostAnonymizeTest.kt @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.model.util + +import kotlin.test.Test +import kotlin.test.assertEquals + +class HostAnonymizeTest { + + private fun assertKept(vararg hosts: String) { + for (host in hosts) assertEquals(host, host.anonymizePublicHost(), "expected $host to stay readable") + } + + private fun assertAnonymized(vararg hosts: String) { + for (host in hosts) assertEquals(host.anonymize(), host.anonymizePublicHost(), "expected $host anonymized") + } + + @Test + fun `loopback hosts stay readable`() { + assertKept("127.0.0.1", "127.0.0.1:4403", "localhost", "::1", "[::1]:4403") + } + + @Test + fun `RFC 1918 private IPv4 hosts stay readable`() { + assertKept("10.0.0.5", "172.16.0.1", "172.31.255.254:4403", "192.168.1.50", "192.168.1.50:4403") + } + + @Test + fun `addresses just outside the RFC 1918 blocks are anonymized`() { + assertAnonymized("172.15.0.1", "172.32.0.1", "11.0.0.1", "192.169.1.1") + } + + @Test + fun `IPv4 and IPv6 link-local hosts stay readable`() { + assertKept("169.254.10.20", "fe80::1", "febf::1", "[fe80::1%en0]:4403") + } + + @Test + fun `IPv6 unique local hosts stay readable`() { + assertKept("fc00::1", "fd12:3456:789a::1", "[fdab::2]:4403") + } + + @Test + fun `IPv6 addresses outside the local blocks are anonymized`() { + assertAnonymized("fec0::1", "2001:db8::1", "[2606:4700::1111]:4403") + } + + @Test + fun `mDNS local names stay readable`() { + assertKept("meshtastic.local", "Meshtastic.local:4403", "node.local.") + } + + @Test + fun `public IPv4 addresses are anonymized`() { + assertAnonymized("8.8.8.8", "8.8.8.8:4403", "203.0.113.7") + } + + @Test + fun `DDNS and MagicDNS names are anonymized`() { + assertAnonymized("mynode.duckdns.org", "node.tail1234.ts.net:4403", "meshnode") + } + + @Test + fun `malformed IPv4 literals are anonymized`() { + assertAnonymized("256.1.1.1", "192.168.1", "10.0.0.1.5") + } +} diff --git a/core/model/src/commonTest/kotlin/org/meshtastic/core/model/util/WireExtensionsTest.kt b/core/model/src/commonTest/kotlin/org/meshtastic/core/model/util/WireExtensionsTest.kt index 905956cf74..54e471a3c8 100644 --- a/core/model/src/commonTest/kotlin/org/meshtastic/core/model/util/WireExtensionsTest.kt +++ b/core/model/src/commonTest/kotlin/org/meshtastic/core/model/util/WireExtensionsTest.kt @@ -16,10 +16,10 @@ */ package org.meshtastic.core.model.util -import co.touchlab.kermit.LogWriter import co.touchlab.kermit.Logger import co.touchlab.kermit.Severity import co.touchlab.kermit.loggerConfigInit +import org.meshtastic.core.testing.CapturingLogWriter import org.meshtastic.proto.Position import kotlin.test.Test import kotlin.test.assertEquals @@ -27,15 +27,7 @@ import kotlin.test.assertNull class WireExtensionsTest { - private class CapturingWriter : LogWriter() { - val severities = mutableListOf() - - override fun log(severity: Severity, message: String, tag: String, throwable: Throwable?) { - severities += severity - } - } - - private val writer = CapturingWriter() + private val writer = CapturingLogWriter() private val logger = Logger(loggerConfigInit(writer), tag = "Test") // Garbage bytes that are not a valid encoding of any message: decoding must fail, not merely @@ -47,7 +39,7 @@ class WireExtensionsTest { val result = Position.ADAPTER.decodeOrNull(garbage, logger) assertNull(result) - assertEquals(listOf(Severity.Warn), writer.severities) + assertEquals(listOf(Severity.Warn), writer.entries.map { it.severity }) } @Test @@ -55,7 +47,7 @@ class WireExtensionsTest { val result = Position.ADAPTER.decodeOrNull(bytes = garbage, logger = logger) assertNull(result) - assertEquals(listOf(Severity.Warn), writer.severities) + assertEquals(listOf(Severity.Warn), writer.entries.map { it.severity }) } @Test @@ -63,7 +55,7 @@ class WireExtensionsTest { val result = Position.ADAPTER.decodeOrNull(garbage) assertNull(result) - assertEquals(emptyList(), writer.severities) + assertEquals(emptyList(), writer.entries.map { it.severity }) } @Test @@ -71,6 +63,6 @@ class WireExtensionsTest { val result = Position.ADAPTER.decodeOrNull(bytes = null as ByteArray?, logger = logger) assertNull(result) - assertEquals(emptyList(), writer.severities) + assertEquals(emptyList(), writer.entries.map { it.severity }) } } diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.kt index 688b0b823b..9d5148a398 100644 --- a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.kt +++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.kt @@ -161,7 +161,7 @@ class BleRadioTransport( private val cleanupScope: CoroutineScope = CoroutineScope(SupervisorJob() + scope.coroutineContext.minusKey(Job)) private val exceptionHandler = CoroutineExceptionHandler { _, throwable -> - Logger.w(throwable) { "[$address] Uncaught exception in connectionScope" } + Logger.w(throwable) { "[${address.anonymize()}] Uncaught exception in connectionScope" } if (throwable !is CancellationException) { val session = activeSession.value if (session != null) { @@ -173,7 +173,9 @@ class BleRadioTransport( if (activeSession.value == null) { disconnectGatt("exception handler") } else { - Logger.d { "[$address] Skipping exception-handler GATT release; a new session is active" } + Logger.d { + "[${address.anonymize()}] Skipping exception-handler GATT release; a new session is active" + } } } } @@ -271,7 +273,7 @@ class BleRadioTransport( throw e } catch (e: Exception) { val failureTime = (nowMillis - connectionStartTime).milliseconds - Logger.w(e) { "[$address] Failed to connect after $failureTime" } + Logger.w(e) { "[${address.anonymize()}] Failed to connect after $failureTime" } BleReconnectPolicy.Outcome.Failed(e) } }, @@ -283,7 +285,9 @@ class BleRadioTransport( // observability surface for this transient event. if (!sessionFailed.value) { error?.let { - Logger.w(it) { "[$address] BLE reconnect attempt failed; continuing automatic retry" } + Logger.w(it) { + "[${address.anonymize()}] BLE reconnect attempt failed; continuing automatic retry" + } } callback.onDisconnect(isPermanent = false) } @@ -307,7 +311,7 @@ class BleRadioTransport( @Suppress("CyclomaticComplexMethod", "LongMethod", "ReturnCount") private suspend fun attemptConnection(): BleReconnectPolicy.Outcome { connectionStartTime = nowMillis - Logger.i { "[$address] BLE connection attempt started" } + Logger.i { "[${address.anonymize()}] BLE connection attempt started" } awaitPendingSessionCleanup() sessionFailed.value = false @@ -354,7 +358,7 @@ class BleRadioTransport( val state = bleConnection.connectAndAwait(device, CONNECTION_TIMEOUT) if (state !is BleConnectionState.Connected) { - throw RadioNotConnectedException("Failed to connect to device at address $address") + throw RadioNotConnectedException("Failed to connect to device at address ${address.anonymize()}") } // GATT cache invalidation has two triggers, both repaired the same way — refresh the platform's cached @@ -389,7 +393,9 @@ class BleRadioTransport( // If a fatal session failure (fromRadio/logRadio error) forced disconnect during setup, // skip the Connected gate — return a retryable failure so BleReconnectPolicy handles it. session.failureCause.value?.let { failure -> - Logger.w(failure) { "[$address] Session failed during profile setup — returning failed outcome" } + Logger.w(failure) { + "[${address.anonymize()}] Session failed during profile setup; returning failed outcome" + } return BleReconnectPolicy.Outcome.Failed(failure) } @@ -412,7 +418,9 @@ class BleRadioTransport( } if (connectedReached == null) { val failure = session.failureCause.value ?: RuntimeException("Timed out waiting for Connected state gate") - Logger.w(failure) { "[$address] Session failed before Connected gate — returning failed outcome" } + Logger.w(failure) { + "[${address.anonymize()}] Session failed before Connected gate; returning failed outcome" + } // Force cleanup only for this exact profile generation. If another path already retired it, await that // generation's cleanup instead of issuing a second disconnect that could race later lifecycle work. isFullyConnected = false @@ -443,14 +451,16 @@ class BleRadioTransport( onDisconnected(session) } - Logger.i { "[$address] BLE connection dropped (reason: $disconnectReason), preparing to reconnect" } + Logger.i { + "[${address.anonymize()}] BLE connection dropped (reason: $disconnectReason), preparing to reconnect" + } // Internal session failures (write/read exceptions that triggered handleFailure → // disconnect) must NOT be treated as intentional/user disconnects — the reconnect policy // needs to escalate backoff for these. val internalFailure = session.failureCause.value if (internalFailure != null) { - Logger.w(internalFailure) { "[$address] Session forced disconnect due to internal failure" } + Logger.w(internalFailure) { "[${address.anonymize()}] Session forced disconnect due to internal failure" } } val wasIntentional = if (internalFailure != null) { @@ -469,7 +479,7 @@ class BleRadioTransport( if (!wasStable && !wasIntentional) { Logger.w { - "[$address] Connection lasted only $connectionUptime " + + "[${address.anonymize()}] Connection lasted only $connectionUptime " + "(< ${reconnectPolicy.minStableConnection}) — treating as unstable" } } @@ -520,10 +530,10 @@ class BleRadioTransport( // Bond before connecting: firmware may require an encrypted link, and without a bond Android fails with // status 5 or 133. Non-Android targets use repository-specific no-op behavior. - Logger.i { "[$address] Device not bonded, initiating bonding" } + Logger.i { "[${address.anonymize()}] Device not bonded, initiating bonding" } try { bluetoothRepository.bond(device) - Logger.i { "[$address] Bonding successful" } + Logger.i { "[${address.anonymize()}] Bonding successful" } } catch (e: CancellationException) { throw e } catch (e: Exception) { @@ -531,9 +541,12 @@ class BleRadioTransport( // setup. If the device is still not bonded, continuing would fail later with a cryptic status (5/133), so // stop now and let BleReconnectPolicy own the retry/backoff. if (bluetoothRepository.isBonded(address)) { - Logger.w(e) { "[$address] Bonding reported failure but device is bonded; continuing" } + Logger.w(e) { "[${address.anonymize()}] Bonding reported failure but device is bonded; continuing" } } else { - Logger.w(e) { "[$address] Bonding failed and device is still not bonded; stopping connection attempt" } + Logger.w(e) { + "[${address.anonymize()}] Bonding failed and device is still not bonded; " + + "stopping connection attempt" + } throw RadioNotConnectedException("Bonding failed and device is still not bonded", e) } } @@ -551,7 +564,7 @@ class BleRadioTransport( } catch (e: CancellationException) { throw e } catch (e: Exception) { - Logger.w(e) { "[$address] Failed to read initial connection RSSI" } + Logger.w(e) { "[${address.anonymize()}] Failed to read initial connection RSSI" } } } @@ -560,7 +573,7 @@ class BleRadioTransport( scheduleSessionCleanup(retired, disconnectGatt = false, phase = "remote disconnect") // Atomic first-writer-wins: if another failure already claimed this session's callback, skip the duplicate. val firstWriter = sessionFailed.compareAndSet(expect = false, update = true) - Logger.i { "[$address] BLE disconnected - ${formatSessionStats()}" } + Logger.i { "[${address.anonymize()}] BLE disconnected - ${formatSessionStats()}" } if (firstWriter) callback.onDisconnect(isPermanent = false) } @@ -578,27 +591,27 @@ class BleRadioTransport( radioService.fromRadio .onEach { packet -> - Logger.v { "[$address] Received packet fromRadio (${packet.size} bytes)" } + Logger.v { "[${address.anonymize()}] Received packet fromRadio (${packet.size} bytes)" } dispatchPacket(packet, session) } .catch { e -> - Logger.w(e) { "[$address] Error in fromRadio flow" } + Logger.w(e) { "[${address.anonymize()}] Error in fromRadio flow" } handleFailure(e, session) } .launchIn(this) radioService.logRadio .onEach { packet -> - Logger.v { "[$address] Received packet logRadio (${packet.size} bytes)" } + Logger.v { "[${address.anonymize()}] Received packet logRadio (${packet.size} bytes)" } dispatchPacket(packet, session) } .catch { e -> - Logger.w(e) { "[$address] Error in logRadio flow" } + Logger.w(e) { "[${address.anonymize()}] Error in logRadio flow" } handleFailure(e, session) } .launchIn(this) - Logger.i { "[$address] Profile service active and characteristics subscribed" } + Logger.i { "[${address.anonymize()}] Profile service active and characteristics subscribed" } // Wait for FROMNUM CCCD write before triggering the Meshtastic handshake. // Bounded: if fromRadio fails before subscriptionReady completes, handleFailure @@ -617,14 +630,17 @@ class BleRadioTransport( ?: RuntimeException("Timed out waiting for FROMNUM subscription readiness") Logger.w(cause) { val reason = if (!subscriptionReady) "timed out" else "failed" - "[$address] Subscription wait $reason — aborting setup" + "[${address.anonymize()}] Subscription wait $reason; aborting setup" } throw cause } // Log negotiated MTU for diagnostics val maxLen = bleConnection.maximumWriteValueLength(BleWriteType.WITHOUT_RESPONSE) - Logger.i { "[$address] BLE Radio Session Ready. Max write length (WITHOUT_RESPONSE): $maxLen bytes" } + Logger.i { + "[${address.anonymize()}] BLE Radio Session Ready. " + + "Max write length (WITHOUT_RESPONSE): $maxLen bytes" + } requestHighPriorityAndScheduleDowngrade() @@ -656,7 +672,9 @@ class BleRadioTransport( false } if (!published) { - Logger.w { "[$address] Session failed or transport closed during setup — skipping onConnect" } + Logger.w { + "[${address.anonymize()}] Session failed or transport closed during setup; skipping onConnect" + } } } return checkNotNull(setupSession) { "BLE profile setup completed without publishing a session" } @@ -666,7 +684,7 @@ class BleRadioTransport( withContext(NonCancellable) { cleanupProfileSetupFailure("cancellation cleanup", setupSession) } throw e } catch (e: Exception) { - Logger.w(e) { "[$address] Profile service discovery or operation failed" } + Logger.w(e) { "[${address.anonymize()}] Profile service discovery or operation failed" } // Retire any partially-published profile so the next attempt starts clean. Without this, a failure after // profile publication but before callback.onConnect() could leave a stale generation behind. withContext(NonCancellable) { cleanupProfileSetupFailure("profile error cleanup", setupSession) } @@ -677,7 +695,7 @@ class BleRadioTransport( private suspend fun cleanupProfileSetupFailure(phase: String, expectedSession: BleSession?) { val currentSession = activeSession.value if (expectedSession != null && currentSession != null && currentSession !== expectedSession) { - Logger.w { "[$address] Ignoring $phase from an unpublished BLE profile generation" } + Logger.w { "[${address.anonymize()}] Ignoring $phase from an unpublished BLE profile generation" } return } @@ -706,14 +724,14 @@ class BleRadioTransport( */ private suspend fun CoroutineScope.requestHighPriorityAndScheduleDowngrade() { if (bleConnection.requestHighConnectionPriority()) { - Logger.d { "[$address] Requested high BLE connection priority" } + Logger.d { "[${address.anonymize()}] Requested high BLE connection priority" } // Wait for the connection parameter update before starting heavy traffic. delay(1.seconds) } launch { delay(PRIORITY_DOWNGRADE_DELAY) if (bleConnection.requestBalancedConnectionPriority()) { - Logger.d { "[$address] Downgraded to balanced BLE connection priority" } + Logger.d { "[${address.anonymize()}] Downgraded to balanced BLE connection priority" } } } } @@ -793,18 +811,21 @@ class BleRadioTransport( } val sent = packetsSent.incrementAndGet() val txBytes = bytesSent.addAndGet(packet.size.toLong()) - Logger.v { "[$address] Wrote packet #$sent to toRadio (${packet.size} bytes, total TX: $txBytes bytes)" } + Logger.v { + "[${address.anonymize()}] Wrote packet #$sent to toRadio " + + "(${packet.size} bytes, total TX: $txBytes bytes)" + } } catch (e: CancellationException) { throw e } catch (e: Exception) { if (activeSession.value === session) { Logger.w(e) { - "[$address] Failed to write packet to toRadioCharacteristic after " + + "[${address.anonymize()}] Failed to write packet to toRadioCharacteristic after " + "${packetsSent.value} successful writes" } handleFailure(e, session) } else { - Logger.d(e) { "[$address] Stale write failure ignored because the session was replaced" } + Logger.d(e) { "[${address.anonymize()}] Stale write failure ignored because the session was replaced" } } } } @@ -825,16 +846,20 @@ class BleRadioTransport( // Closing the outer gate rejects new sends while allowing writes admitted before close to finish. // Once those leases drain, cancel reconnect/heartbeat work before retiring the profile and GATT. connectionScope.cancel() - Logger.i { "[$address] Disconnecting. ${formatSessionStats()}" } + Logger.i { "[${address.anonymize()}] Disconnecting. ${formatSessionStats()}" } val session = retireActiveSession() val sessionClosed = session?.lifecycle?.close() ?: true awaitPendingSessionCleanup() disconnectGatt("close") if (!sessionClosed) { - Logger.w { "[$address] BLE profile teardown did not complete within its lifecycle bounds" } + Logger.w { + "[${address.anonymize()}] BLE profile teardown did not complete within its lifecycle bounds" + } } } - if (!completed) Logger.w { "[$address] BLE teardown did not complete within its lifecycle bounds" } + if (!completed) { + Logger.w { "[${address.anonymize()}] BLE teardown did not complete within its lifecycle bounds" } + } } finally { if (!completed) { // The cleanup scope is detached, so a timed-out outer gate needs one final bounded GATT release attempt @@ -852,7 +877,8 @@ class BleRadioTransport( val received = packetsReceived.incrementAndGet() val rxBytes = bytesReceived.addAndGet(packet.size.toLong()) Logger.v { - "[$address] Dispatching packet #$received " + "(${packet.size} bytes, total RX: $rxBytes bytes)" + "[${address.anonymize()}] Dispatching packet #$received " + + "(${packet.size} bytes, total RX: $rxBytes bytes)" } callback.handleFromRadio(packet) true @@ -870,7 +896,7 @@ class BleRadioTransport( recordSessionFailureCause(throwable, expectedSession) val retired = retireActiveSession(expectedSession) if (retired == null) { - Logger.d(throwable) { "[$address] Ignoring failure from a retired BLE profile generation" } + Logger.d(throwable) { "[${address.anonymize()}] Ignoring failure from a retired BLE profile generation" } return } val firstFailure = sessionFailed.compareAndSet(expect = false, update = true) @@ -879,7 +905,7 @@ class BleRadioTransport( val (isPermanent, msg) = throwable.toDisconnectReason() callback.onDisconnect(isPermanent, errorMessage = if (isPermanent) msg else null) } - Logger.w(throwable) { "[$address] Session failure — forcing cleanup for reconnect" } + Logger.w(throwable) { "[${address.anonymize()}] Session failure; forcing cleanup for reconnect" } scheduleSessionCleanup(retired, disconnectGatt = true, phase = "session failure") } @@ -901,7 +927,9 @@ class BleRadioTransport( } else { session.lifecycle.close() } - if (!completed) Logger.w { "[$address] BLE profile cleanup timed out during $phase" } + if (!completed) { + Logger.w { "[${address.anonymize()}] BLE profile cleanup timed out during $phase" } + } } .also { pendingSessionCleanup = it } } @@ -927,7 +955,7 @@ class BleRadioTransport( } catch (e: CancellationException) { throw e } catch (e: Exception) { - Logger.w(e) { "[$address] Failed to disconnect during $phase" } + Logger.w(e) { "[${address.anonymize()}] Failed to disconnect during $phase" } } } diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/TcpRadioTransport.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/TcpRadioTransport.kt index 3fcc197900..dd0431bf5b 100644 --- a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/TcpRadioTransport.kt +++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/TcpRadioTransport.kt @@ -26,6 +26,7 @@ import kotlinx.coroutines.joinAll import kotlinx.coroutines.withTimeoutOrNull import org.meshtastic.core.common.util.handledLaunch import org.meshtastic.core.di.CoroutineDispatchers +import org.meshtastic.core.model.util.anonymizePublicHost import org.meshtastic.core.network.transport.StreamFrameCodec import org.meshtastic.core.network.transport.TcpTransport import org.meshtastic.core.repository.RadioTransport @@ -88,7 +89,7 @@ internal constructor( dispatchers = dispatchers, scope = scope, listener = listener, - logTag = "TcpRadioTransport[$address]", + logTag = "TcpRadioTransport[${address.anonymizePublicHost()}]", ), ) }, @@ -125,7 +126,10 @@ internal constructor( override fun start() { lifecycle.runIfOpen { if (transportStopped.value) { - Logger.w { "[$address] Ignoring start on a stopped TCP transport; a fresh transport is required" } + Logger.w { + "[${address.anonymizePublicHost()}] Ignoring start on a stopped TCP transport; " + + "a fresh transport is required" + } } else { transport.start(address) } @@ -133,7 +137,7 @@ internal constructor( } override suspend fun close() { - Logger.d { "[$address] Closing TCP transport" } + Logger.d { "[${address.anonymizePublicHost()}] Closing TCP transport" } val completed = lifecycle.close( teardown = { @@ -141,14 +145,16 @@ internal constructor( cancelOutstandingOperations() }, ) - if (!completed) Logger.w { "[$address] TCP teardown did not complete within its lifecycle bounds" } + if (!completed) { + Logger.w { "[${address.anonymizePublicHost()}] TCP teardown did not complete within its lifecycle bounds" } + } // Do NOT emit onDisconnect(isPermanent = true) here. The explicit-disconnect signal is the service layer's // responsibility (SharedRadioInterfaceService.stopTransportLocked); emitting it here causes a double-disconnect // and prevents the auto-reconnect loop from owning its transient lifecycle. } override fun keepAlive() { - Logger.d { "[$address] TCP keepAlive" } + Logger.d { "[${address.anonymizePublicHost()}] TCP keepAlive" } launchConnectionOperation("heartbeat") { transport.sendHeartbeat() } } @@ -173,7 +179,9 @@ internal constructor( scope.handledLaunch { val completed = withTimeoutOrNull(OPERATION_TIMEOUT) { block() } != null if (!completed) { - Logger.w { "[$address] TCP $operation timed out after $OPERATION_TIMEOUT" } + Logger.w { + "[${address.anonymizePublicHost()}] TCP $operation timed out after $OPERATION_TIMEOUT" + } // Cancellation may leave a framed write partially emitted. Stopping this one-shot // transport forces the service reconnect path to create a fresh transport before another // send. @@ -197,7 +205,9 @@ internal constructor( val jobs = synchronized(operationJobsLock) { operationJobs.toList() } jobs.forEach { it.cancel() } val joined = withTimeoutOrNull(OPERATION_TIMEOUT) { jobs.joinAll() } != null - if (!joined) Logger.w { "[$address] TCP operation jobs did not stop after transport teardown" } + if (!joined) { + Logger.w { "[${address.anonymizePublicHost()}] TCP operation jobs did not stop after transport teardown" } + } } private fun stopTransport() { diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/transport/TcpTransport.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/transport/TcpTransport.kt index b3c1cfb434..f2355e349f 100644 --- a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/transport/TcpTransport.kt +++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/transport/TcpTransport.kt @@ -40,6 +40,7 @@ import kotlinx.io.IOException import org.meshtastic.core.common.util.handledLaunch import org.meshtastic.core.common.util.nowMillis import org.meshtastic.core.di.CoroutineDispatchers +import org.meshtastic.core.model.util.anonymizePublicHost import org.meshtastic.proto.ToRadio import kotlin.concurrent.Volatile @@ -209,11 +210,11 @@ class TcpTransport( try { connectAndRead(address) } catch (ex: TimeoutCancellationException) { - Logger.w(ex) { "$logTag: [$address] TCP connect timed out" } + Logger.w(ex) { "$logTag: [${address.anonymizePublicHost()}] TCP connect timed out" } disconnectSocket() false } catch (ex: IOException) { - Logger.w(ex) { "$logTag: [$address] TCP connection error" } + Logger.w(ex) { "$logTag: [${address.anonymizePublicHost()}] TCP connection error" } disconnectSocket() false } catch (ce: CancellationException) { @@ -225,9 +226,9 @@ class TcpTransport( // (UnresolvedAddressException, which is an IllegalArgumentException and so misses the IOException // branch above). Log it, retry it, but keep it out of error tracking. if (ex.isExpectedConnectionFailure()) { - Logger.w(ex) { "$logTag: [$address] Radio unreachable" } + Logger.w(ex) { "$logTag: [${address.anonymizePublicHost()}] Radio unreachable" } } else { - Logger.e(ex) { "$logTag: [$address] TCP exception" } + Logger.e(ex) { "$logTag: [${address.anonymizePublicHost()}] TCP exception" } } disconnectSocket() false @@ -238,18 +239,22 @@ class TcpTransport( // growing so the radio has time to recover between reconnect attempts. val sessionUptime = if (connectionStartTime > 0) nowMillis - connectionStartTime else 0 if (shouldResetBackoff(hadData, sessionUptime, SHORT_SESSION_THRESHOLD_MS)) { - Logger.d { "$logTag: [$address] Resetting backoff after successful data exchange (${sessionUptime}ms)" } + Logger.d { + "$logTag: [${address.anonymizePublicHost()}] Resetting backoff after successful data exchange " + + "(${sessionUptime}ms)" + } retryCount = 1 backoff = MIN_BACKOFF_MILLIS } else if (hadData) { val backoffSec = backoff / MILLIS_PER_SECOND Logger.d { - "$logTag: [$address] Short session (${sessionUptime}ms) — keeping backoff at ${backoffSec}s" + "$logTag: [${address.anonymizePublicHost()}] Short session (${sessionUptime}ms); " + + "keeping backoff at ${backoffSec}s" } } val delaySec = backoff / MILLIS_PER_SECOND - Logger.i { "$logTag: [$address] Reconnect #$retryCount in ${delaySec}s" } + Logger.i { "$logTag: [${address.anonymizePublicHost()}] Reconnect #$retryCount in ${delaySec}s" } delay(backoff) retryCount++ backoff = minOf(backoff * 2, MAX_BACKOFF_MILLIS) @@ -264,7 +269,7 @@ class TcpTransport( private suspend fun connectAndRead(address: String): Boolean = withContext(dispatchers.io) { val (host, port) = parseHostAndPort(address) - Logger.i { "$logTag: [$address] Connecting to $host:$port" } + Logger.i { "$logTag: [${address.anonymizePublicHost()}] Connecting to ${host.anonymizePublicHost()}:$port" } val attemptStart = nowMillis val selector = SelectorManager(dispatchers.io) @@ -284,7 +289,7 @@ class TcpTransport( resetMetrics() codec.reset() - Logger.i { "$logTag: [$address] Socket connected in ${connectTime}ms" } + Logger.i { "$logTag: [${address.anonymizePublicHost()}] Socket connected in ${connectTime}ms" } val output = sock.openWriteChannel(autoFlush = false) writeChannel = output @@ -342,12 +347,12 @@ class TcpTransport( timeoutCount++ timeoutEvents++ if (timeoutCount % TIMEOUT_LOG_INTERVAL == 0) { - Logger.d { "$logTag: [$address] Timeout $timeoutCount/$SOCKET_RETRIES" } + Logger.d { "$logTag: [${address.anonymizePublicHost()}] Timeout $timeoutCount/$SOCKET_RETRIES" } } } read == -1 -> { - Logger.i { "$logTag: [$address] EOF after $packetsReceived packets" } + Logger.i { "$logTag: [${address.anonymizePublicHost()}] EOF after $packetsReceived packets" } return } @@ -360,7 +365,7 @@ class TcpTransport( } } } - Logger.w { "$logTag: [$address] Closing after $SOCKET_RETRIES consecutive timeouts" } + Logger.w { "$logTag: [${address.anonymizePublicHost()}] Closing after $SOCKET_RETRIES consecutive timeouts" } } // Guards against recursive disconnects triggered by listener callbacks. @@ -375,14 +380,14 @@ class TcpTransport( if (s != null) { val uptime = if (connectionStartTime > 0) nowMillis - connectionStartTime else 0 Logger.i { - "$logTag: [$currentAddress] Disconnecting - Uptime: ${uptime}ms, " + + "$logTag: [${currentAddress?.anonymizePublicHost()}] Disconnecting - Uptime: ${uptime}ms, " + "RX: $packetsReceived ($bytesReceived bytes), " + "TX: $packetsSent ($bytesSent bytes)" } try { s.close() } catch (ex: IOException) { - Logger.w(ex) { "$logTag: [$currentAddress] Error closing socket" } + Logger.w(ex) { "$logTag: [${currentAddress?.anonymizePublicHost()}] Error closing socket" } } } selectorManager?.close() @@ -408,13 +413,15 @@ class TcpTransport( val stream = writeChannel ?: run { - Logger.w { "$logTag: [$currentAddress] Cannot send ${p.size} bytes: not connected" } + Logger.w { + "$logTag: [${currentAddress?.anonymizePublicHost()}] Cannot send ${p.size} bytes: not connected" + } return } try { stream.writeFully(p) } catch (ex: IOException) { - Logger.w(ex) { "$logTag: [$currentAddress] TCP write error" } + Logger.w(ex) { "$logTag: [${currentAddress?.anonymizePublicHost()}] TCP write error" } disconnectSocket() } } @@ -424,7 +431,7 @@ class TcpTransport( try { stream.flush() } catch (ex: IOException) { - Logger.w(ex) { "$logTag: [$currentAddress] TCP flush error" } + Logger.w(ex) { "$logTag: [${currentAddress?.anonymizePublicHost()}] TCP flush error" } disconnectSocket() } } diff --git a/core/network/src/commonTest/kotlin/org/meshtastic/core/network/repository/KermitMqttLoggerTest.kt b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/repository/KermitMqttLoggerTest.kt index 500c2e6b16..fcd4688beb 100644 --- a/core/network/src/commonTest/kotlin/org/meshtastic/core/network/repository/KermitMqttLoggerTest.kt +++ b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/repository/KermitMqttLoggerTest.kt @@ -16,26 +16,18 @@ */ package org.meshtastic.core.network.repository -import co.touchlab.kermit.LogWriter import co.touchlab.kermit.Logger import co.touchlab.kermit.Severity import co.touchlab.kermit.loggerConfigInit import kotlinx.io.IOException +import org.meshtastic.core.testing.CapturingLogWriter import org.meshtastic.mqtt.MqttLogLevel import kotlin.test.Test import kotlin.test.assertEquals class KermitMqttLoggerTest { - private class CapturingWriter : LogWriter() { - val entries = mutableListOf>() - - override fun log(severity: Severity, message: String, tag: String, throwable: Throwable?) { - entries += Triple(severity, tag, message) - } - } - - private val writer = CapturingWriter() + private val writer = CapturingLogWriter() private val mqttLogger = KermitMqttLogger(Logger(loggerConfigInit(writer), tag = "Test")) @Test @@ -50,8 +42,8 @@ class KermitMqttLoggerTest { ) assertEquals(1, writer.entries.size) - assertEquals(Severity.Warn, writer.entries[0].first) - assertEquals("MqttConnection", writer.entries[0].second) + assertEquals(Severity.Warn, writer.entries[0].severity) + assertEquals("MqttConnection", writer.entries[0].tag) } @Test @@ -63,7 +55,7 @@ class KermitMqttLoggerTest { throwable = IllegalStateException("bad state"), ) - assertEquals(Severity.Error, writer.entries.single().first) + assertEquals(Severity.Error, writer.entries.single().severity) } @Test @@ -71,7 +63,7 @@ class KermitMqttLoggerTest { // A bare message carries no stack or type to triage, so it must not count as an application error. mqttLogger.log(level = MqttLogLevel.ERROR, tag = "MqttClient", message = "boom", throwable = null) - assertEquals(Severity.Warn, writer.entries.single().first) + assertEquals(Severity.Warn, writer.entries.single().severity) } @Test @@ -83,7 +75,7 @@ class KermitMqttLoggerTest { throwable = null, ) - assertEquals(Severity.Warn, writer.entries.single().first) + assertEquals(Severity.Warn, writer.entries.single().severity) } @Test @@ -96,7 +88,7 @@ class KermitMqttLoggerTest { throwable = null, ) - assertEquals(Severity.Warn, writer.entries.single().first) + assertEquals(Severity.Warn, writer.entries.single().severity) } @Test @@ -105,8 +97,8 @@ class KermitMqttLoggerTest { mqttLogger.log(MqttLogLevel.INFO, "MqttClient", "info", null) mqttLogger.log(MqttLogLevel.DEBUG, "MqttClient", "debug", null) - assertEquals(listOf(Severity.Warn, Severity.Info, Severity.Debug), writer.entries.map { it.first }) - assertEquals(listOf("MqttClient", "MqttClient", "MqttClient"), writer.entries.map { it.second }) + assertEquals(listOf(Severity.Warn, Severity.Info, Severity.Debug), writer.entries.map { it.severity }) + assertEquals(listOf("MqttClient", "MqttClient", "MqttClient"), writer.entries.map { it.tag }) } @Test diff --git a/core/network/src/jvmMain/kotlin/org/meshtastic/core/network/repository/JvmServiceDiscovery.kt b/core/network/src/jvmMain/kotlin/org/meshtastic/core/network/repository/JvmServiceDiscovery.kt index 8eb0e01837..dd7bbfba8c 100644 --- a/core/network/src/jvmMain/kotlin/org/meshtastic/core/network/repository/JvmServiceDiscovery.kt +++ b/core/network/src/jvmMain/kotlin/org/meshtastic/core/network/repository/JvmServiceDiscovery.kt @@ -22,6 +22,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.flowOn import org.koin.core.annotation.Single +import org.meshtastic.core.common.util.safeCatching import org.meshtastic.core.di.CoroutineDispatchers import java.io.IOException import java.net.InetAddress @@ -38,7 +39,8 @@ class JvmServiceDiscovery(private val dispatchers: CoroutineDispatchers) : Servi trySend(emptyList()) // Emit initial empty list so downstream combine() is not blocked val bindAddress = findLanAddress() ?: InetAddress.getLocalHost() - Logger.i { "JmDNS binding to ${bindAddress.hostAddress}" } + val interfaceName = safeCatching { NetworkInterface.getByInetAddress(bindAddress)?.name }.getOrNull() + Logger.i { "JmDNS binding to interface ${interfaceName ?: "unknown"}" } val jmdns = try { diff --git a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/CapturingLogWriter.kt b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/CapturingLogWriter.kt new file mode 100644 index 0000000000..d4b1db7417 --- /dev/null +++ b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/CapturingLogWriter.kt @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.testing + +import co.touchlab.kermit.LogWriter +import co.touchlab.kermit.Logger +import co.touchlab.kermit.Severity +import co.touchlab.kermit.platformLogWriter +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * A Kermit writer that records every line, for tests that assert what was or was not logged. Hand it to + * `Logger(loggerConfigInit(writer))`, or [install] it on the global `Logger` and call [uninstall] in teardown. + */ +class CapturingLogWriter : LogWriter() { + data class Entry(val severity: Severity, val tag: String, val message: String, val throwable: Throwable?) + + private val recorded = mutableListOf() + + val entries: List + get() = recorded.toList() + + override fun log(severity: Severity, message: String, tag: String, throwable: Throwable?) { + recorded += Entry(severity, tag, message, throwable) + } + + fun messages(severity: Severity? = null): List = + entries.filter { severity == null || it.severity == severity }.map { it.message } + + /** Asserts that something was logged and that no message or attached throwable text contains any of [values]. */ + fun assertNotLogged(vararg values: String) { + val text = entries.flatMap { listOfNotNull(it.message, it.throwable?.stackTraceToString()) } + assertTrue(text.isNotEmpty(), "Expected something to be logged") + for (value in values) { + assertFalse(text.any { value in it }, "'$value' leaked into logs: $text") + } + } + + companion object { + /** Replaces the global `Logger` writers with a new capturing writer that keeps every severity. */ + fun install(): CapturingLogWriter = CapturingLogWriter().also { + Logger.setLogWriters(it) + Logger.setMinSeverity(Severity.Verbose) + } + + /** Restores the platform writer on the global `Logger`. */ + fun uninstall() { + Logger.setLogWriters(platformLogWriter()) + } + } +} diff --git a/core/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt b/core/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt index 6e18664f82..1ba2b763af 100644 --- a/core/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt +++ b/core/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt @@ -55,8 +55,8 @@ actual fun rememberShowToastResource(): suspend (StringResource) -> Unit = { _ - /** JVM stub — map opening is not available on Desktop. */ @Composable -actual fun rememberOpenMap(): (latitude: Double, longitude: Double, label: String) -> Unit = { lat, lon, label -> - Logger.i { "Open map: $lat, $lon ($label)" } +actual fun rememberOpenMap(): (latitude: Double, longitude: Double, label: String) -> Unit = { _, _, _ -> + Logger.i { "Open map requested; not available on Desktop" } } /** JVM stub — URL opening via Desktop browse API. */ diff --git a/desktopApp/src/main/kotlin/org/meshtastic/desktop/DesktopLogging.kt b/desktopApp/src/main/kotlin/org/meshtastic/desktop/DesktopLogging.kt new file mode 100644 index 0000000000..3573bbf6f4 --- /dev/null +++ b/desktopApp/src/main/kotlin/org/meshtastic/desktop/DesktopLogging.kt @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.desktop + +import co.touchlab.kermit.Logger +import co.touchlab.kermit.Severity +import co.touchlab.kermit.platformLogWriter +import org.meshtastic.core.common.log.InMemoryLogBuffer + +/** + * Sends Kermit output to the console and to [InMemoryLogBuffer], which the Debug screen views and exports. Release + * builds keep Info and above, as Android release does; debug builds keep every level. + */ +internal fun installDesktopLogging(isDebug: Boolean) { + Logger.setMinSeverity(if (isDebug) Severity.Verbose else Severity.Info) + Logger.setLogWriters(listOf(platformLogWriter(), InMemoryLogBuffer)) +} diff --git a/desktopApp/src/main/kotlin/org/meshtastic/desktop/Main.kt b/desktopApp/src/main/kotlin/org/meshtastic/desktop/Main.kt index ec16dbed16..4530b31116 100644 --- a/desktopApp/src/main/kotlin/org/meshtastic/desktop/Main.kt +++ b/desktopApp/src/main/kotlin/org/meshtastic/desktop/Main.kt @@ -52,7 +52,6 @@ import androidx.compose.ui.window.isTraySupported import androidx.compose.ui.window.rememberTrayState import androidx.compose.ui.window.rememberWindowState import co.touchlab.kermit.Logger -import co.touchlab.kermit.platformLogWriter import coil3.ImageLoader import coil3.annotation.ExperimentalCoilApi import coil3.compose.setSingletonImageLoaderFactory @@ -78,7 +77,6 @@ import org.koin.plugin.module.dsl.startKoin import org.maplibre.compose.desktop.ProvideMapPresentationHost import org.maplibre.compose.desktop.rememberAwtComposeMapPresentationHost import org.meshtastic.core.common.BuildConfigProvider -import org.meshtastic.core.common.log.InMemoryLogBuffer import org.meshtastic.core.common.state.LaunchOptions import org.meshtastic.core.common.util.CommonUri import org.meshtastic.core.common.util.ioDispatcher @@ -163,8 +161,7 @@ fun main(args: Array) { // No MapLibre.configure() call: the first map applies a default cache configuration process-wide. application(exitProcessOnExit = false) { val koinApp = remember { - // Keep console output and also capture into the in-memory buffer the Debug screen views/exports. - Logger.setLogWriters(listOf(platformLogWriter(), InMemoryLogBuffer)) + installDesktopLogging(isDebug = DesktopBuildConfig.IS_DEBUG) Logger.i { "Meshtastic Desktop — Starting" } startKoin {} .also { app -> diff --git a/desktopApp/src/main/kotlin/org/meshtastic/desktop/radio/DesktopRadioTransportFactory.kt b/desktopApp/src/main/kotlin/org/meshtastic/desktop/radio/DesktopRadioTransportFactory.kt index 12edbcb098..b6b303338e 100644 --- a/desktopApp/src/main/kotlin/org/meshtastic/desktop/radio/DesktopRadioTransportFactory.kt +++ b/desktopApp/src/main/kotlin/org/meshtastic/desktop/radio/DesktopRadioTransportFactory.kt @@ -24,6 +24,7 @@ import org.meshtastic.core.ble.BluetoothRepository import org.meshtastic.core.di.CoroutineDispatchers import org.meshtastic.core.model.DeviceType import org.meshtastic.core.model.InterfaceId +import org.meshtastic.core.model.util.anonymize import org.meshtastic.core.network.SerialTransport import org.meshtastic.core.network.radio.BaseRadioTransportFactory import org.meshtastic.core.network.radio.MockRadioTransport @@ -83,6 +84,6 @@ class DesktopRadioTransportFactory( ) } - else -> error("Unsupported transport for address: $address") + else -> error("Unsupported transport for address: ${address.anonymize()}") } } diff --git a/desktopApp/src/test/kotlin/org/meshtastic/desktop/DesktopLoggingTest.kt b/desktopApp/src/test/kotlin/org/meshtastic/desktop/DesktopLoggingTest.kt new file mode 100644 index 0000000000..9827ae2000 --- /dev/null +++ b/desktopApp/src/test/kotlin/org/meshtastic/desktop/DesktopLoggingTest.kt @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.desktop + +import co.touchlab.kermit.Logger +import co.touchlab.kermit.Severity +import co.touchlab.kermit.platformLogWriter +import org.meshtastic.core.common.log.InMemoryLogBuffer +import java.util.UUID +import kotlin.test.AfterTest +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class DesktopLoggingTest { + + @AfterTest + fun tearDown() { + Logger.setLogWriters(platformLogWriter()) + Logger.setMinSeverity(Severity.Verbose) + } + + private fun marker(level: String) = "desktop-logging-test-$level-${UUID.randomUUID()}" + + @Test + fun `release build keeps Verbose and Debug lines out of the exportable buffer`() { + installDesktopLogging(isDebug = false) + val verbose = marker("verbose") + val debug = marker("debug") + val info = marker("info") + + Logger.v { verbose } + Logger.d { debug } + Logger.i { info } + + val buffer = InMemoryLogBuffer.snapshot() + assertFalse(verbose in buffer, "Verbose line reached the release buffer") + assertFalse(debug in buffer, "Debug line reached the release buffer") + assertTrue(info in buffer, "Info line is missing from the release buffer") + } + + @Test + fun `debug build keeps every level in the exportable buffer`() { + installDesktopLogging(isDebug = true) + val verbose = marker("verbose") + val debug = marker("debug") + + Logger.v { verbose } + Logger.d { debug } + + val buffer = InMemoryLogBuffer.snapshot() + assertTrue(verbose in buffer, "Verbose line is missing from the debug buffer") + assertTrue(debug in buffer, "Debug line is missing from the debug buffer") + } +} diff --git a/docs/en/user/debug-logs.md b/docs/en/user/debug-logs.md index c72700d1ed..0bbd23777e 100644 --- a/docs/en/user/debug-logs.md +++ b/docs/en/user/debug-logs.md @@ -2,7 +2,7 @@ title: Debug Logs parent: User Guide nav_order: 22 -last_updated: 2026-08-30 +last_updated: 2026-09-28 description: View and export the app's own debug logs from inside the app, and attach a capture to a GitHub issue to help diagnose bugs — no adb required. aliases: - debug-logs @@ -48,7 +48,7 @@ Attach that file to your GitHub issue. ## Desktop -The desktop app has no system logcat, so the **App logs** tab shows the app's own captured log output instead. Search, filtering, and export work the same way. +The desktop app has no system logcat, so the **App logs** tab shows the app's own captured log output instead. Search, filtering, and export work the same way. Release builds capture Info, Warn, and Error lines; Verbose and Debug lines appear only in development builds. ## Related Topics diff --git a/feature/wifi-provision/src/commonMain/kotlin/org/meshtastic/feature/wifiprovision/domain/NymeaWifiService.kt b/feature/wifi-provision/src/commonMain/kotlin/org/meshtastic/feature/wifiprovision/domain/NymeaWifiService.kt index eb1f50c7e4..c7f38988f6 100644 --- a/feature/wifi-provision/src/commonMain/kotlin/org/meshtastic/feature/wifiprovision/domain/NymeaWifiService.kt +++ b/feature/wifi-provision/src/commonMain/kotlin/org/meshtastic/feature/wifiprovision/domain/NymeaWifiService.kt @@ -117,7 +117,7 @@ class NymeaWifiService( .onEach { bytes -> val message = reassembler.feed(bytes) if (message != null) { - Logger.d { "$TAG: ← $message" } + Logger.d { "$TAG: ← response (${message.length} chars)" } responseChannel.trySend(message) } if (!subscribed.isCompleted) subscribed.complete(Unit) @@ -145,14 +145,14 @@ class NymeaWifiService( */ suspend fun scanNetworks(): Result> = safeCatching { // Trigger scan - sendCommand(NymeaJson.encodeToString(NymeaSimpleCommand(CMD_SCAN))) + sendCommand(CMD_SCAN, NymeaJson.encodeToString(NymeaSimpleCommand(CMD_SCAN))) val scanAck = NymeaJson.decodeFromString(waitForResponse()) if (scanAck.responseCode != RESPONSE_SUCCESS) { error("Scan command failed: ${nymeaErrorMessage(scanAck.responseCode)}") } // Fetch results - sendCommand(NymeaJson.encodeToString(NymeaSimpleCommand(CMD_GET_NETWORKS))) + sendCommand(CMD_GET_NETWORKS, NymeaJson.encodeToString(NymeaSimpleCommand(CMD_GET_NETWORKS))) val networksResponse = NymeaJson.decodeFromString(waitForResponse()) if (networksResponse.responseCode != RESPONSE_SUCCESS) { error("GetNetworks failed: ${nymeaErrorMessage(networksResponse.responseCode)}") @@ -186,7 +186,7 @@ class NymeaWifiService( ) return safeCatching { - sendCommand(json) + sendCommand(cmd, json) val response = NymeaJson.decodeFromString(waitForResponse()) if (response.responseCode == RESPONSE_SUCCESS) { val ipAddress = @@ -224,9 +224,13 @@ class NymeaWifiService( // region Internal helpers - /** Encode [json] into ≤20-byte packets and write each one WITH_RESPONSE to the commander characteristic. */ - private suspend fun sendCommand(json: String) { - Logger.d { "$TAG: → $json" } + /** + * Encode [json] into ≤20-byte packets and write each one WITH_RESPONSE to the commander characteristic. Only the + * [command] code is logged: a Connect payload carries the WiFi password, and even its length gives away the SSID + * and password lengths. + */ + private suspend fun sendCommand(command: Int, json: String) { + Logger.d { "$TAG: → command=$command" } val packets = NymeaPacketCodec.encode(json) bleConnection.profile(WIRELESS_SERVICE_UUID) { service -> for (packet in packets) { @@ -245,7 +249,7 @@ class NymeaWifiService( * Uses a short timeout because this is an optional enrichment for UX, not a provisioning success criterion. */ private suspend fun fetchConnectionIpAddress(): String? = safeCatching { - sendCommand(NymeaJson.encodeToString(NymeaSimpleCommand(CMD_GET_CONNECTION))) + sendCommand(CMD_GET_CONNECTION, NymeaJson.encodeToString(NymeaSimpleCommand(CMD_GET_CONNECTION))) val response = NymeaJson.decodeFromString(waitForResponse(timeout = CONNECTION_INFO_TIMEOUT)) if (response.responseCode == RESPONSE_SUCCESS) { diff --git a/feature/wifi-provision/src/commonTest/kotlin/org/meshtastic/feature/wifiprovision/domain/NymeaWifiServiceTest.kt b/feature/wifi-provision/src/commonTest/kotlin/org/meshtastic/feature/wifiprovision/domain/NymeaWifiServiceTest.kt index e356daa267..b29ac3b64f 100644 --- a/feature/wifi-provision/src/commonTest/kotlin/org/meshtastic/feature/wifiprovision/domain/NymeaWifiServiceTest.kt +++ b/feature/wifi-provision/src/commonTest/kotlin/org/meshtastic/feature/wifiprovision/domain/NymeaWifiServiceTest.kt @@ -22,6 +22,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.meshtastic.core.ble.BleWriteType +import org.meshtastic.core.testing.CapturingLogWriter import org.meshtastic.core.testing.FakeBleConnection import org.meshtastic.core.testing.FakeBleConnectionFactory import org.meshtastic.core.testing.FakeBleDevice @@ -29,6 +30,7 @@ import org.meshtastic.core.testing.FakeBleScanner import org.meshtastic.feature.wifiprovision.NymeaBleConstants.COMMANDER_RESPONSE_UUID import org.meshtastic.feature.wifiprovision.NymeaBleConstants.WIRELESS_COMMANDER_UUID import org.meshtastic.feature.wifiprovision.model.ProvisionResult +import kotlin.test.AfterTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertIs @@ -43,6 +45,11 @@ class NymeaWifiServiceTest { private val address = "AA:BB:CC:DD:EE:FF" + @AfterTest + fun tearDown() { + CapturingLogWriter.uninstall() + } + private fun createService( scanner: FakeBleScanner = FakeBleScanner(), connection: FakeBleConnection = FakeBleConnection(), @@ -293,6 +300,24 @@ class NymeaWifiServiceTest { assertTrue(writes.contains("\"c\":2"), "Should send CMD_CONNECT_HIDDEN (2)") } + @Test + fun `provision never logs the WiFi credentials or the device response payload`() = runTest { + val connection = FakeBleConnection() + val (service, scanner) = createService(connection = connection) + connectService(service, scanner) + val logs = CapturingLogWriter.install() + + emitResponse(connection, """{"c":1,"r":0,"p":{"i":"10.77.88.99"}}""") + val result = service.provision("SecretHomeNet", "hunter2-wifi-pass") + + assertIs(result) + assertTrue( + logs.messages().any { it.endsWith("command=1") }, + "Command send should be logged with nothing after the code: ${logs.messages()}", + ) + logs.assertNotLogged("hunter2-wifi-pass", "SecretHomeNet", "10.77.88.99") + } + @Test fun `provision returns Failure on exception`() = runTest { // Create a service with a connection that will fail writes after connecting