diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/datasource/EventFirmwareEditionLocalDataSource.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/datasource/EventFirmwareEditionLocalDataSource.kt index 891ed6efc1..095d2e54b9 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/datasource/EventFirmwareEditionLocalDataSource.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/datasource/EventFirmwareEditionLocalDataSource.kt @@ -16,6 +16,9 @@ */ package org.meshtastic.core.data.datasource +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.withContext import org.koin.core.annotation.Single import org.meshtastic.core.database.DatabaseProvider @@ -35,6 +38,10 @@ class EventFirmwareEditionLocalDataSource( suspend fun getByEdition(edition: String): EventFirmwareEditionEntity? = withContext(dispatchers.io) { dao.getByEdition(edition) } + @OptIn(ExperimentalCoroutinesApi::class) + fun observeByEdition(edition: String): Flow = + dbManager.currentDb.flatMapLatest { db -> db.eventFirmwareEditionDao().observeByEdition(edition) } + suspend fun upsertAll(editions: List) { withContext(dispatchers.io) { dbManager.withDb { it.eventFirmwareEditionDao().upsertAll(editions) } } } diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt index 7640e2285a..d876d93f18 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImpl.kt @@ -493,18 +493,10 @@ class MeshConfigFlowManagerImpl( null } + // Whether to silence new-node notifications is a read-decide-write over two preferences, so it lives behind one + // atomic prefs operation rather than being assembled from setter calls here. private fun applyEventFirmwareNotificationDefaults(edition: FirmwareEdition) { - if (edition != FirmwareEdition.VANILLA) { - if (!notificationPrefs.nodeEventsAutoDisabledForEvent.value) { - notificationPrefs.setNodeEventsEnabled(false) - notificationPrefs.setNodeEventsAutoDisabledForEvent(true) - } - } else { - if (notificationPrefs.nodeEventsAutoDisabledForEvent.value) { - notificationPrefs.setNodeEventsEnabled(true) - notificationPrefs.setNodeEventsAutoDisabledForEvent(false) - } - } + notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = edition != FirmwareEdition.VANILLA) } } diff --git a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/EventFirmwareRepositoryImpl.kt b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/EventFirmwareRepositoryImpl.kt index bf3835360c..1132193ded 100644 --- a/core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/EventFirmwareRepositoryImpl.kt +++ b/core/data/src/commonMain/kotlin/org/meshtastic/core/data/repository/EventFirmwareRepositoryImpl.kt @@ -17,6 +17,11 @@ package org.meshtastic.core.data.repository import co.touchlab.kermit.Logger +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emitAll +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.map import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -79,12 +84,26 @@ class EventFirmwareRepositoryImpl( override suspend fun getEdition(editionName: String): EventFirmwareEdition? = withContext(dispatchers.io) { ensureSeeded() + maybeRefresh(maxWaitMs = NETWORK_REFRESH_TIMEOUT_MS) + localDataSource.getByEdition(editionName)?.asExternalModel() + } + + override fun observeEdition(editionName: String): Flow = flow { + ensureSeeded() + // Don't wait — the emissions below carry whatever the refresh writes, so blocking would only delay the + // cached value the caller could already be showing. + maybeRefresh(maxWaitMs = 0) + emitAll(localDataSource.observeByEdition(editionName).map { it?.asExternalModel() }) + } + .flowOn(dispatchers.io) + + /** Refreshes if the cache is stale and the retry cooldown has elapsed. See [lastAttemptMillis] for the cooldown. */ + private suspend fun maybeRefresh(maxWaitMs: Long) { val stale = nowMillis - lastRefreshMillis > CACHE_EXPIRATION_TIME_MS val retryCooldownElapsed = nowMillis - lastAttemptMillis > REFRESH_RETRY_COOLDOWN_MS if (stale && retryCooldownElapsed) { - refresher.refresh(maxWaitMs = NETWORK_REFRESH_TIMEOUT_MS) + refresher.refresh(maxWaitMs = maxWaitMs) } - localDataSource.getByEdition(editionName)?.asExternalModel() } /** Seeds the table from the bundled snapshot if empty (fresh install, data clear). */ diff --git a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt index 4d227c9a40..fc69295d8b 100644 --- a/core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt +++ b/core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshConfigFlowManagerImplTest.kt @@ -821,50 +821,34 @@ class MeshConfigFlowManagerImplTest { } // ---------- Event firmware notification defaults ---------- + // + // The decision itself (which preference wins, and when) is a single atomic prefs operation and is covered in + // NotificationPrefsTest. These only assert that the config flow delegates with the right firmware classification. @Test - fun `handleMyInfo disables node notifications for event firmware`() = testScope.runTest { - every { notificationPrefs.nodeEventsAutoDisabledForEvent } returns MutableStateFlow(false) - - val eventMyInfo = protoMyNodeInfo.copy(firmware_edition = FirmwareEdition.DEFCON) - handleMyInfo(eventMyInfo) - advanceUntilIdle() - - verify { notificationPrefs.setNodeEventsEnabled(false) } - verify { notificationPrefs.setNodeEventsAutoDisabledForEvent(true) } - } - - @Test - fun `handleMyInfo does not re-disable if already auto-disabled`() = testScope.runTest { - every { notificationPrefs.nodeEventsAutoDisabledForEvent } returns MutableStateFlow(true) - - val eventMyInfo = protoMyNodeInfo.copy(firmware_edition = FirmwareEdition.DEFCON) - handleMyInfo(eventMyInfo) + fun `handleMyInfo applies the event node-event default for event firmware`() = testScope.runTest { + handleMyInfo(protoMyNodeInfo.copy(firmware_edition = FirmwareEdition.DEFCON)) advanceUntilIdle() - verify(mode = VerifyMode.not) { notificationPrefs.setNodeEventsEnabled(any()) } + verify { notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = true) } } @Test - fun `handleMyInfo re-enables node notifications when vanilla firmware reconnects`() = testScope.runTest { - every { notificationPrefs.nodeEventsAutoDisabledForEvent } returns MutableStateFlow(true) - + fun `handleMyInfo applies the vanilla node-event default for vanilla firmware`() = testScope.runTest { handleMyInfo(protoMyNodeInfo) advanceUntilIdle() - verify { notificationPrefs.setNodeEventsEnabled(true) } - verify { notificationPrefs.setNodeEventsAutoDisabledForEvent(false) } + verify { notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = false) } } @Test - fun `handleMyInfo does not touch prefs for vanilla when not previously auto-disabled`() = testScope.runTest { - every { notificationPrefs.nodeEventsAutoDisabledForEvent } returns MutableStateFlow(false) - - handleMyInfo(protoMyNodeInfo) + fun `handleMyInfo treats DIY_EDITION as event firmware`() = testScope.runTest { + // Any non-vanilla edition counts, including the DIY catch-all — it has no branding metadata but the same + // notification noise problem. + handleMyInfo(protoMyNodeInfo.copy(firmware_edition = FirmwareEdition.DIY_EDITION)) advanceUntilIdle() - verify(mode = VerifyMode.not) { notificationPrefs.setNodeEventsEnabled(any()) } - verify(mode = VerifyMode.not) { notificationPrefs.setNodeEventsAutoDisabledForEvent(any()) } + verify { notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = true) } } // ---------- onHandshakeProgress ---------- diff --git a/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/repository/EventFirmwareRepositoryImplTest.kt b/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/repository/EventFirmwareRepositoryImplTest.kt index bdde1a959f..462a6dfe11 100644 --- a/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/repository/EventFirmwareRepositoryImplTest.kt +++ b/core/data/src/jvmTest/kotlin/org/meshtastic/core/data/repository/EventFirmwareRepositoryImplTest.kt @@ -16,7 +16,9 @@ */ package org.meshtastic.core.data.repository +import app.cash.turbine.test import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import kotlinx.serialization.json.Json import okio.Buffer @@ -42,7 +44,9 @@ import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotNull import kotlin.test.assertNull +import kotlin.time.Duration.Companion.seconds class EventFirmwareRepositoryImplTest { @@ -129,6 +133,36 @@ class EventFirmwareRepositoryImplTest { assertNull(repository.getEdition("VANILLA")) } + @Test + fun observeEditionReEmitsWhenTheCacheIsRefreshed() = runBlocking { + // The point of the observable: an edition absent at connect time — because the bundled seed predates it — + // must reach the UI when the refresh lands, without the caller re-subscribing. The new value therefore has to + // arrive *through the refresh path*; writing it into the cache by hand would still pass if observeEdition + // stopped refreshing at all. + seed.editions = listOf(edition("HAMVENTION")) + api.response = EventFirmwareResponse(editions = listOf(edition("HAMVENTION"), edition("DEFCON"))) + + repository.observeEdition("DEFCON").test(timeout = EMISSION_TIMEOUT) { + // The first item is null only if the refresh is still in flight when collection starts; under + // Dispatchers.Unconfined it may already have completed inline. Which of the two happens is timing, not + // contract, so accept either and assert on the value that matters. + val observed = assertNotNull(awaitItem() ?: awaitItem(), "expected the refreshed DEFCON edition") + // DEFCON exists only in the network response, never in the seed, so observing it at all proves the value + // arrived via refresh → cache write → emission. + assertEquals("defcon", observed.displayName) + cancelAndIgnoreRemainingEvents() + } + // Collection is what drove that fetch: nothing else in this test touches the network or the cache. + assertEquals(1, api.eventFirmwareCalls) + } + + @Test + fun observeEditionEmitsNullForUnknownEdition() = runBlocking { + seed.editions = listOf(edition("HAMVENTION")) + + assertNull(repository.observeEdition("VANILLA").first()) + } + @Test fun absentAssetYieldsNullWithoutCrashing() = runBlocking { seed.present = false @@ -231,4 +265,9 @@ class EventFirmwareRepositoryImplTest { assertEquals("hamvention", restarted.getEdition("HAMVENTION")?.displayName) } + + private companion object { + /** Room's invalidation tracker plus the fake network round-trip; generous, since it only bounds a failure. */ + private val EMISSION_TIMEOUT = 10.seconds + } } diff --git a/core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/EventFirmwareEditionDao.kt b/core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/EventFirmwareEditionDao.kt index 901df227df..f60adf8cec 100644 --- a/core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/EventFirmwareEditionDao.kt +++ b/core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/EventFirmwareEditionDao.kt @@ -19,6 +19,7 @@ package org.meshtastic.core.database.dao import androidx.room3.Dao import androidx.room3.Query import androidx.room3.Upsert +import kotlinx.coroutines.flow.Flow import org.meshtastic.core.database.entity.EventFirmwareEditionEntity @Dao @@ -28,6 +29,14 @@ interface EventFirmwareEditionDao { @Query("SELECT * FROM event_firmware_edition WHERE edition = :edition") suspend fun getByEdition(edition: String): EventFirmwareEditionEntity? + /** + * Observes [edition], re-emitting whenever the table changes. This is what lets a background manifest refresh reach + * already-visible event branding: a one-shot read resolves once when the connection is established, so metadata + * that lands afterwards would sit in the table unseen until the user reconnected. + */ + @Query("SELECT * FROM event_firmware_edition WHERE edition = :edition LIMIT 1") + fun observeByEdition(edition: String): Flow + /** * Deletes rows whose edition is not in [keep]. WARNING: `NOT IN ()` is always true in SQLite, so an **empty** * [keep] deletes every row — call sites must guard against passing an empty list (see diff --git a/core/prefs/src/commonMain/kotlin/org/meshtastic/core/prefs/notification/NotificationPrefsImpl.kt b/core/prefs/src/commonMain/kotlin/org/meshtastic/core/prefs/notification/NotificationPrefsImpl.kt index b30167d7d5..89e032812d 100644 --- a/core/prefs/src/commonMain/kotlin/org/meshtastic/core/prefs/notification/NotificationPrefsImpl.kt +++ b/core/prefs/src/commonMain/kotlin/org/meshtastic/core/prefs/notification/NotificationPrefsImpl.kt @@ -61,6 +61,30 @@ class NotificationPrefsImpl( scope.launch { dataStore.edit { it[KEY_NODE_EVENTS_AUTO_DISABLED] = disabled } } } + override fun applyEventFirmwareNodeEventDefault(isEventFirmware: Boolean) { + // One edit block: DataStore serializes edits, so the values read here are the same snapshot written below. + // Reading the StateFlows and calling the setters instead would race — they lag these writes. + scope.launch { + dataStore.edit { prefs -> + val enabled = prefs[KEY_NODE_EVENTS_ENABLED] ?: true + val autoDisabled = prefs[KEY_NODE_EVENTS_AUTO_DISABLED] ?: false + when { + // Only claim the restore if node events were actually on — otherwise the vanilla branch would + // later enable a preference the user turned off themselves. + isEventFirmware && !autoDisabled && enabled -> { + prefs[KEY_NODE_EVENTS_ENABLED] = false + prefs[KEY_NODE_EVENTS_AUTO_DISABLED] = true + } + + !isEventFirmware && autoDisabled -> { + prefs[KEY_NODE_EVENTS_ENABLED] = true + prefs[KEY_NODE_EVENTS_AUTO_DISABLED] = false + } + } + } + } + } + override val lowBatteryEnabled: StateFlow = dataStore.data.map { it[KEY_LOW_BATTERY_ENABLED] ?: true }.stateIn(scope, SharingStarted.Eagerly, true) diff --git a/core/prefs/src/commonTest/kotlin/org/meshtastic/core/prefs/notification/NotificationPrefsTest.kt b/core/prefs/src/commonTest/kotlin/org/meshtastic/core/prefs/notification/NotificationPrefsTest.kt index 1ab64cbe3b..5f19e14157 100644 --- a/core/prefs/src/commonTest/kotlin/org/meshtastic/core/prefs/notification/NotificationPrefsTest.kt +++ b/core/prefs/src/commonTest/kotlin/org/meshtastic/core/prefs/notification/NotificationPrefsTest.kt @@ -85,6 +85,62 @@ class NotificationPrefsTest { assertFalse(notificationPrefs.nodeEventsEnabled.value) } + // ---------- applyEventFirmwareNodeEventDefault ---------- + + @Test + fun `event firmware disables node events and claims the restore`() = testScope.runTest { + notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = true) + + assertFalse(notificationPrefs.nodeEventsEnabled.value) + assertTrue(notificationPrefs.nodeEventsAutoDisabledForEvent.value) + } + + @Test + fun `event firmware leaves an already-off preference alone and claims nothing`() = testScope.runTest { + // The user turned node events off themselves. Claiming the restore here would make the next vanilla + // connection switch them back on, discarding a choice we never made. + notificationPrefs.setNodeEventsEnabled(false) + + notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = true) + + assertFalse(notificationPrefs.nodeEventsEnabled.value) + assertFalse(notificationPrefs.nodeEventsAutoDisabledForEvent.value) + } + + @Test + fun `vanilla firmware restores node events only when the restore was claimed`() = testScope.runTest { + notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = true) + assertFalse(notificationPrefs.nodeEventsEnabled.value) + + notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = false) + + assertTrue(notificationPrefs.nodeEventsEnabled.value) + assertFalse(notificationPrefs.nodeEventsAutoDisabledForEvent.value) + } + + @Test + fun `vanilla firmware does not enable node events the user had turned off`() = testScope.runTest { + // The full round trip of the bug this guards: off by the user, connect to event firmware, back to vanilla. + notificationPrefs.setNodeEventsEnabled(false) + + notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = true) + notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = false) + + assertFalse(notificationPrefs.nodeEventsEnabled.value) + assertFalse(notificationPrefs.nodeEventsAutoDisabledForEvent.value) + } + + @Test + fun `repeated event firmware connections do not re-disable a manual re-enable`() = testScope.runTest { + notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = true) + // User re-enables mid-event; the restore is still claimed, so reconnects must respect their choice. + notificationPrefs.setNodeEventsEnabled(true) + + notificationPrefs.applyEventFirmwareNodeEventDefault(isEventFirmware = true) + + assertTrue(notificationPrefs.nodeEventsEnabled.value) + } + @Test fun `setting lowBatteryEnabled updates preference`() = testScope.runTest { notificationPrefs.setLowBatteryEnabled(false) diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AppPreferences.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AppPreferences.kt index 60dc140469..c2f26b3d2c 100644 --- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AppPreferences.kt +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/AppPreferences.kt @@ -212,6 +212,19 @@ interface NotificationPrefs { val nodeEventsAutoDisabledForEvent: StateFlow + /** + * Applies the node-event notification default for the connected firmware, as a single atomic update. + * + * The decision reads both [nodeEventsEnabled] and [nodeEventsAutoDisabledForEvent] and conditionally writes both, + * so it cannot be expressed as separate reads and setter calls: the setters are asynchronous and the StateFlows lag + * them, so a caller doing it by hand can read stale values or clobber a concurrent user toggle. + * + * On event firmware, node events are disabled and the restore is claimed — but only if they were actually on, so a + * user who had already turned them off is not re-enabled later. On vanilla firmware, a previously claimed restore + * is honored and released. + */ + fun applyEventFirmwareNodeEventDefault(isEventFirmware: Boolean) + fun setNodeEventsAutoDisabledForEvent(disabled: Boolean) val lowBatteryEnabled: StateFlow diff --git a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/EventFirmwareRepository.kt b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/EventFirmwareRepository.kt index 94fc5d0550..8a2d1fcc2e 100644 --- a/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/EventFirmwareRepository.kt +++ b/core/repository/src/commonMain/kotlin/org/meshtastic/core/repository/EventFirmwareRepository.kt @@ -16,10 +16,20 @@ */ package org.meshtastic.core.repository +import kotlinx.coroutines.flow.Flow import org.meshtastic.core.model.EventFirmwareEdition /** Event-firmware display metadata, seeded from the bundled `event_firmware.json` snapshot. */ interface EventFirmwareRepository { /** Metadata for [editionName] (a `FirmwareEdition` enum name), or `null` if it is not a known event edition. */ suspend fun getEdition(editionName: String): EventFirmwareEdition? + + /** + * Metadata for [editionName], re-emitting when the cache is refreshed from the network. + * + * Prefer this over [getEdition] for anything long-lived on screen. A one-shot read resolves once — when the device + * connects — so an edition added to the hosted manifest after that point stays invisible until the user reconnects. + * The flow triggers a refresh on collection and then re-emits whatever lands. + */ + fun observeEdition(editionName: String): Flow } diff --git a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeNotificationPrefs.kt b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeNotificationPrefs.kt index 8c8825e61b..bb836d21a6 100644 --- a/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeNotificationPrefs.kt +++ b/core/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeNotificationPrefs.kt @@ -38,6 +38,20 @@ class FakeNotificationPrefs : NotificationPrefs { nodeEventsAutoDisabledForEvent.value = disabled } + override fun applyEventFirmwareNodeEventDefault(isEventFirmware: Boolean) { + when { + isEventFirmware && !nodeEventsAutoDisabledForEvent.value && nodeEventsEnabled.value -> { + nodeEventsEnabled.value = false + nodeEventsAutoDisabledForEvent.value = true + } + + !isEventFirmware && nodeEventsAutoDisabledForEvent.value -> { + nodeEventsEnabled.value = true + nodeEventsAutoDisabledForEvent.value = false + } + } + } + override val lowBatteryEnabled = MutableStateFlow(true) override fun setLowBatteryEnabled(enabled: Boolean) { diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/EventInfoSheet.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/EventInfoSheet.kt index ab64c3db03..3dfa00f756 100644 --- a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/EventInfoSheet.kt +++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/EventInfoSheet.kt @@ -66,6 +66,7 @@ import org.meshtastic.core.ui.util.EventBrandingIcon import org.meshtastic.core.ui.util.accentColorOrNull import org.meshtastic.core.ui.util.brandHighlightOrNull import org.meshtastic.core.ui.util.brandPalette +import org.meshtastic.core.ui.util.safeLinks /** * Bottom sheet shown when the user taps the event branding in [MainAppBar]. Surfaces the event metadata the bundled @@ -125,7 +126,8 @@ private fun EventDetails(edition: EventFirmwareEdition, palette: List, on edition.location?.takeIf { it.isNotBlank() }?.let { InfoRow(MeshtasticIcons.Place, it, iconTint) } dateRange(edition)?.let { InfoRow(MeshtasticIcons.CalendarMonth, it, iconTint) } - val links = edition.links.filter { it.url.isNotBlank() } + // safeLinks() drops anything that isn't an https URL — the manifest is remote, and these go to the URI handler. + val links = edition.safeLinks() if (links.isNotEmpty()) { HorizontalDivider() links.forEach { link -> diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/LocalEventBranding.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/LocalEventBranding.kt index b7f2c287c8..f02bbfb7dd 100644 --- a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/LocalEventBranding.kt +++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/LocalEventBranding.kt @@ -31,6 +31,7 @@ import org.jetbrains.compose.resources.DrawableResource import org.jetbrains.compose.resources.painterResource import org.jetbrains.compose.resources.vectorResource import org.meshtastic.core.model.EventFirmwareEdition +import org.meshtastic.core.model.EventFirmwareLink import org.meshtastic.core.resources.Res import org.meshtastic.core.resources.ic_meshtastic import org.meshtastic.core.resources.img_event_defcon @@ -57,9 +58,41 @@ fun eventIconFor(editionName: String): DrawableResource? = when (editionName) { } /** - * Event branding icon: loads the hosted [EventFirmwareEdition.iconUrl] when present, falling back to the bundled - * per-edition drawable ([eventIconFor]), and finally the Meshtastic logo. The fallback painter also backs Coil's - * loading/error states so there is never an empty slot. + * Whether [url] is safe to fetch or open from event metadata: an absolute `https` URL whose authority is a valid host + * with an optional numeric port. + * + * The manifest is first-party but arrives over the network, and its URLs reach an image loader and the platform URI + * handler. A URI handler will honour whatever scheme it is given, so without this check a bad or tampered manifest + * entry could invoke arbitrary handlers on the device. Scheme comparison is case-insensitive; everything else is + * rejected, including protocol-relative (`//host`) and scheme-relative input. + * + * The authority is matched whole rather than merely tested for emptiness, so a port with no host (`https://:443`), + * whitespace, a truncated IPv6 literal, and userinfo (`https://trusted.host@evil.example`, which wears a trusted + * prefix) are all rejected — none of those characters appear in the permitted set. + */ +fun safeBrandUrlOrNull(url: String?): String? { + val trimmed = url?.trim().orEmpty() + if (!trimmed.startsWith(HTTPS_SCHEME, ignoreCase = true)) return null + val authority = trimmed.drop(HTTPS_SCHEME.length).takeWhile { it != '/' && it != '?' && it != '#' } + // Return the trimmed form, not the caller's original: consumers must use the string that was actually validated, + // or a padded-but-otherwise-valid URL passes here and then fails as a malformed URI at the image loader. + return trimmed.takeIf { AUTHORITY_REGEX.matches(authority) } +} + +/** Whether [url] is safe to fetch or open — see [safeBrandUrlOrNull], which callers should prefer for the value. */ +fun isSafeBrandUrl(url: String?): Boolean = safeBrandUrlOrNull(url) != null + +/** + * Links that are safe to open, carrying the normalized URL — see [safeBrandUrlOrNull]. Unsafe or blank entries are + * dropped rather than rendered and refused on tap. + */ +fun EventFirmwareEdition.safeLinks(): List = + links.mapNotNull { link -> safeBrandUrlOrNull(link.url)?.let { link.copy(url = it) } } + +/** + * Event branding icon: loads the hosted [EventFirmwareEdition.iconUrl] when present *and* safe to fetch, falling back + * to the bundled per-edition drawable ([eventIconFor]), and finally the Meshtastic logo. The fallback painter also + * backs Coil's loading/error states so there is never an empty slot. */ @Composable fun EventBrandingIcon( @@ -70,7 +103,7 @@ fun EventBrandingIcon( val bundled = eventIconFor(edition.edition) val fallback = bundled?.let { painterResource(it) } ?: rememberVectorPainter(vectorResource(Res.drawable.ic_meshtastic)) - val url = edition.iconUrl + val url = safeBrandUrlOrNull(edition.iconUrl) if (url.isNullOrBlank()) { Image( painter = fallback, @@ -141,6 +174,10 @@ fun EventFirmwareEdition.brandPalette(): List { fun EventFirmwareEdition.brandHighlightOrNull(): Color? = parseBrandColor(theme?.colors?.accent) ?: parseBrandColor(theme?.colors?.secondary) +private const val HTTPS_SCHEME = "https://" + +/** A registered name or bracketed IPv6 literal, plus an optional numeric port. See [isSafeBrandUrl]. */ +private val AUTHORITY_REGEX = Regex("""(?:[A-Za-z0-9._~-]+|\[[0-9A-Fa-f:.]+])(?::\d+)?""") private const val RGB_HEX_LENGTH = 6 private const val HEX_RADIX = 16 private const val RED_SHIFT = 16 diff --git a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/UIViewModel.kt b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/UIViewModel.kt index 7027061656..95432feec0 100644 --- a/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/UIViewModel.kt +++ b/core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/viewmodel/UIViewModel.kt @@ -20,6 +20,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import androidx.navigation3.runtime.NavKey import co.touchlab.kermit.Logger +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow @@ -29,7 +30,10 @@ import kotlinx.coroutines.flow.asFlow import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapNotNull @@ -81,6 +85,7 @@ import org.meshtastic.proto.SharedContact * shared contacts, channel sets, unread counts, etc.). */ @KoinViewModel +@OptIn(ExperimentalCoroutinesApi::class) @Suppress("LongParameterList", "TooManyFunctions") class UIViewModel( private val nodeDB: NodeRepository, @@ -145,13 +150,14 @@ class UIViewModel( val eventEdition: StateFlow = combine(firmwareEdition, connectionState) { edition, state -> - // combine's transform is suspending, so the repository lookup runs here directly. - if (state is ConnectionState.Connected) { - edition?.let { eventFirmwareRepository.getEdition(it.name) } - } else { - null - } + edition?.name?.takeIf { state is ConnectionState.Connected } } + .distinctUntilChanged() + // Observe rather than read once, so a manifest refresh that lands after connecting reaches the branding + // already on screen instead of waiting for a reconnect. + .flatMapLatest { editionName -> + editionName?.let { eventFirmwareRepository.observeEdition(it) } ?: flowOf(null) + } .stateInWhileSubscribed(initialValue = null) val clientNotification: StateFlow = serviceRepository.clientNotification diff --git a/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/EventBrandingTest.kt b/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/EventBrandingTest.kt index 9e936b568f..ba5445ea63 100644 --- a/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/EventBrandingTest.kt +++ b/core/ui/src/commonTest/kotlin/org/meshtastic/core/ui/util/EventBrandingTest.kt @@ -18,6 +18,7 @@ package org.meshtastic.core.ui.util import androidx.compose.ui.graphics.Color import org.meshtastic.core.model.EventFirmwareEdition +import org.meshtastic.core.model.EventFirmwareLink import org.meshtastic.core.model.EventFirmwareTheme import org.meshtastic.core.model.EventFirmwareThemeColors import kotlin.test.Test @@ -124,6 +125,83 @@ class EventBrandingTest { assertTrue(EventFirmwareEdition(edition = "X").brandPalette().isEmpty()) } + @Test + fun safeBrandUrlAcceptsOnlyAbsoluteHttps() { + assertTrue(isSafeBrandUrl("https://api.meshtastic.org/resource/eventFirmware/defcon34.png")) + assertTrue(isSafeBrandUrl("HTTPS://defcon.org")) // scheme is case-insensitive + assertTrue(isSafeBrandUrl(" https://defcon.org ")) // surrounding whitespace tolerated + assertTrue(isSafeBrandUrl("https://defcon.org:8443/x.png")) // explicit port + assertTrue(isSafeBrandUrl("https://[2606:4700::1]/x.png")) // IPv6 literal + + assertFalse(isSafeBrandUrl("http://defcon.org")) // cleartext + assertFalse(isSafeBrandUrl("//defcon.org")) // protocol-relative + assertFalse(isSafeBrandUrl("defcon.org")) // no scheme + assertFalse(isSafeBrandUrl("https://")) // no host + assertFalse(isSafeBrandUrl("https:///path")) // empty host + assertFalse(isSafeBrandUrl(null)) + assertFalse(isSafeBrandUrl("")) + } + + @Test + fun safeBrandUrlRejectsMalformedAuthorities() { + // A non-empty authority is not the same as a valid host: each of these has an authority that is present but + // not a host, so the check has to match the authority whole rather than test it for emptiness. + assertFalse(isSafeBrandUrl("https://:443")) // port, no host + assertFalse(isSafeBrandUrl("https://[::1")) // unterminated IPv6 literal + assertFalse(isSafeBrandUrl("https://[]/x")) // empty IPv6 literal + assertFalse(isSafeBrandUrl("https://ho st/x")) // whitespace in host + assertFalse(isSafeBrandUrl("https://defcon.org:port")) // non-numeric port + assertFalse(isSafeBrandUrl("https://defcon.org:/x")) // empty port + } + + @Test + fun safeBrandUrlRejectsNonHttpSchemesAndUserinfo() { + // These are the cases that make an unchecked URL dangerous: the platform URI handler honours whatever scheme it + // is handed, and userinfo lets a hostile host masquerade as a trusted one in the visible prefix. + assertFalse(isSafeBrandUrl("javascript:alert(1)")) + assertFalse(isSafeBrandUrl("file:///etc/passwd")) + assertFalse(isSafeBrandUrl("intent://scan/#Intent;scheme=zxing;end")) + assertFalse(isSafeBrandUrl("meshtastic://settings")) + assertFalse(isSafeBrandUrl("https://api.meshtastic.org@evil.example/x.png")) + } + + @Test + fun safeBrandUrlReturnsTheValidatedFormNotTheOriginal() { + // Consumers hand this straight to the image loader / URI handler, so they must get the string that was + // validated — a padded URL that passed validation and then went out untrimmed would fail as a malformed URI. + assertEquals("https://defcon.org", safeBrandUrlOrNull(" https://defcon.org ")) + assertEquals("https://defcon.org", safeBrandUrlOrNull("https://defcon.org")) + assertNull(safeBrandUrlOrNull("http://defcon.org")) + } + + @Test + fun safeLinksNormalizesTheUrlItKeeps() { + val edition = + EventFirmwareEdition( + edition = "DEFCON", + links = listOf(EventFirmwareLink("Padded", "\n https://defcon.org/schedule \t")), + ) + assertEquals(listOf("https://defcon.org/schedule"), edition.safeLinks().map { it.url }) + assertEquals(listOf("Padded"), edition.safeLinks().map { it.label }) // label preserved + } + + @Test + fun safeLinksDropsUnsafeEntriesAndKeepsOrder() { + val edition = + EventFirmwareEdition( + edition = "DEFCON", + links = + listOf( + EventFirmwareLink("Event Website", "https://defcon.org"), + EventFirmwareLink("Bad scheme", "javascript:alert(1)"), + EventFirmwareLink("Cleartext", "http://defcon.org"), + EventFirmwareLink("Blank", ""), + EventFirmwareLink("Mastodon", "https://defcon.social"), + ), + ) + assertEquals(listOf("https://defcon.org", "https://defcon.social"), edition.safeLinks().map { it.url }) + } + @Test fun brandHighlightPrefersAccentThenSecondary() { val withAccent = diff --git a/desktopApp/build.gradle.kts b/desktopApp/build.gradle.kts index 29c2869c42..b48bb9aa02 100644 --- a/desktopApp/build.gradle.kts +++ b/desktopApp/build.gradle.kts @@ -346,6 +346,7 @@ dependencies { implementation(libs.jna) testRuntimeOnly(libs.junit.vintage.engine) + testImplementation(projects.core.testing) testImplementation(libs.koin.test) testImplementation(libs.kotlinx.coroutines.test) testImplementation(kotlin("test")) diff --git a/desktopApp/src/test/kotlin/org/meshtastic/desktop/notification/DesktopNotificationManagerTest.kt b/desktopApp/src/test/kotlin/org/meshtastic/desktop/notification/DesktopNotificationManagerTest.kt index 80e5b82aa4..ea2447c45b 100644 --- a/desktopApp/src/test/kotlin/org/meshtastic/desktop/notification/DesktopNotificationManagerTest.kt +++ b/desktopApp/src/test/kotlin/org/meshtastic/desktop/notification/DesktopNotificationManagerTest.kt @@ -17,12 +17,11 @@ package org.meshtastic.desktop.notification import kotlinx.coroutines.CoroutineStart -import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest import org.meshtastic.core.repository.Notification -import org.meshtastic.core.repository.NotificationPrefs +import org.meshtastic.core.testing.FakeNotificationPrefs import org.meshtastic.desktop.DesktopNotificationManager import kotlin.test.Test import kotlin.test.assertEquals @@ -43,41 +42,6 @@ class DesktopNotificationManagerTest { } } - /** Simple [NotificationPrefs] with all categories enabled by default. */ - private class FakeNotificationPrefs( - messages: Boolean = true, - nodeEvents: Boolean = true, - lowBattery: Boolean = true, - ) : NotificationPrefs { - override val messagesEnabled = MutableStateFlow(messages) - override val nodeEventsEnabled = MutableStateFlow(nodeEvents) - override val nodeEventsAutoDisabledForEvent = MutableStateFlow(false) - override val lowBatteryEnabled = MutableStateFlow(lowBattery) - - override fun setMessagesEnabled(enabled: Boolean) { - messagesEnabled.value = enabled - } - - override fun setNodeEventsEnabled(enabled: Boolean) { - nodeEventsEnabled.value = enabled - } - - override fun setNodeEventsAutoDisabledForEvent(disabled: Boolean) { - nodeEventsAutoDisabledForEvent.value = disabled - } - - override fun setLowBatteryEnabled(enabled: Boolean) { - lowBatteryEnabled.value = enabled - } - - override val geofenceAlertOptIns = MutableStateFlow>(emptySet()) - - override fun setGeofenceAlertOptIn(waypointId: Int, enabled: Boolean) { - geofenceAlertOptIns.value = - geofenceAlertOptIns.value.toMutableSet().apply { if (enabled) add(waypointId) else remove(waypointId) } - } - } - @Test fun `dispatch sends to native sender and reports success when enabled`() = runTest { val sender = FakeNativeSender() @@ -93,7 +57,7 @@ class DesktopNotificationManagerTest { @Test fun `dispatch reports false and skips native sender when preference disabled`() = runTest { val sender = FakeNativeSender() - val manager = DesktopNotificationManager(FakeNotificationPrefs(messages = false), sender) + val manager = DesktopNotificationManager(FakeNotificationPrefs().apply { setMessagesEnabled(false) }, sender) val dispatched = manager.dispatch(Notification(title = "Msg", message = "Hi", category = Notification.Category.Message)) @@ -105,7 +69,7 @@ class DesktopNotificationManagerTest { @Test fun `alerts are always dispatched even when messages disabled`() = runTest { val sender = FakeNativeSender() - val manager = DesktopNotificationManager(FakeNotificationPrefs(messages = false), sender) + val manager = DesktopNotificationManager(FakeNotificationPrefs().apply { setMessagesEnabled(false) }, sender) val dispatched = manager.dispatch(