From 656f481c68b6f08e690e2fefd0f9c16a43597e8a Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:29:46 +0000 Subject: [PATCH] fix(connections): gate manual + recent TCP connects on ACCESS_LOCAL_NETWORK (#6766) Co-authored-by: Claude Opus 5 --- .skills/compose-ui/strings-index.txt | 1 + .../composeResources/values/strings.xml | 1 + .../feature/connections/ScannerViewModel.kt | 22 ++++ .../connections/ui/ConnectionsScreen.kt | 57 +++++++++- .../connections/ui/LocalNetworkGateAction.kt | 85 +++++++++++++++ .../connections/ScannerViewModelTest.kt | 15 +++ .../connections/ui/LocalNetworkGateTest.kt | 102 ++++++++++++++++++ 7 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/LocalNetworkGateAction.kt create mode 100644 feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ui/LocalNetworkGateTest.kt diff --git a/.skills/compose-ui/strings-index.txt b/.skills/compose-ui/strings-index.txt index f0b1f7c8e0..47da89c5e8 100644 --- a/.skills/compose-ui/strings-index.txt +++ b/.skills/compose-ui/strings-index.txt @@ -932,6 +932,7 @@ load_indexed loading ### LOCAL ### local_mbtiles_file +local_network_permission_denied_hint local_stats_bad local_stats_battery local_stats_diagnostics_prefix diff --git a/core/resources/src/commonMain/composeResources/values/strings.xml b/core/resources/src/commonMain/composeResources/values/strings.xml index d836a47eb3..475c8f8fad 100644 --- a/core/resources/src/commonMain/composeResources/values/strings.xml +++ b/core/resources/src/commonMain/composeResources/values/strings.xml @@ -962,6 +962,7 @@ Loading Local MBTiles File + Local network access is turned off for Meshtastic. If this radio is on your local network, the connection will fail until you allow local network access in system settings. Bad %1$d Battery: %1$d% Diagnostics: %1$s diff --git a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt index 097a030232..3a24f57a50 100644 --- a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt +++ b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt @@ -66,6 +66,7 @@ import org.meshtastic.core.resources.bluetooth_scan_start_failed import org.meshtastic.core.resources.bluetooth_scan_too_frequent import org.meshtastic.core.resources.getPluralStringSuspend import org.meshtastic.core.resources.getStringSuspend +import org.meshtastic.core.resources.local_network_permission_denied_hint import org.meshtastic.core.ui.viewmodel.safeLaunch import org.meshtastic.core.ui.viewmodel.stateInWhileSubscribed import org.meshtastic.feature.connections.model.DeviceListEntry @@ -78,6 +79,12 @@ internal val BLE_SCAN_START_FAILURE_RETRY_COOLDOWN = 15.seconds private const val BLE_SCAN_START_FAILURE_MESSAGE_FALLBACK = "Bluetooth scan couldn't start. Try again, or toggle Bluetooth if the problem continues." +// English fallback for local_network_permission_denied_hint when resource lookup is unavailable. Internal so tests +// can assert the exact surfaced text. +internal const val LOCAL_NETWORK_PERMISSION_DENIED_HINT_FALLBACK = + "Local network access is turned off for Meshtastic. If this radio is on your local network, " + + "the connection will fail until you allow local network access in system settings." + /** * How long to block scan restarts after a scan-start failure. * @@ -605,6 +612,21 @@ open class ScannerViewModel( changeDeviceAddress(fullAddress) } + /** + * Surfaces the local-network warning for a TCP connect that proceeds without `ACCESS_LOCAL_NETWORK` — either the + * permission is permanently denied (no prompt possible) or an in-context request just resolved as a denial. The + * connect is attempted regardless (the target may be a public host or VPN peer, which the permission does not + * govern); this message names the fix for the case where it IS local and the connect is about to time out. + */ + fun warnLocalNetworkPermissionDenied() { + safeLaunch(tag = "warnLocalNetworkPermissionDenied") { + val message = + safeCatchingAll { getStringSuspend(Res.string.local_network_permission_denied_hint) } + .getOrDefault(LOCAL_NETWORK_PERMISSION_DENIED_HINT_FALLBACK) + serviceRepository.setErrorMessage(text = message, severity = Severity.Warn) + } + } + /** * Called by the UI when a device has been tapped. BLE and USB entries may still need bonding/permission — the * concrete return value tells the caller whether the connection was initiated immediately. diff --git a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt index 3dcf990163..625d0a228a 100644 --- a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt +++ b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/ConnectionsScreen.kt @@ -44,6 +44,9 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalUriHandler @@ -161,6 +164,45 @@ fun ConnectionsScreen( val localNetworkPermission = rememberLocalNetworkPermissionState() val bluetoothPermission = rememberBluetoothPermissionState() + // ACCESS_LOCAL_NETWORK gates the socket, not just discovery — a blocked local TCP connect times out rather than + // failing fast — but a TCP address says nothing about locality: public-IP/port-forward/VPN radios need no + // permission at all. Policy (see LocalNetworkGateAction): prompt when possible, warn when not, never block. + // A connect issued while the prompt is up is stashed with the status it saw; the LaunchedEffect below resolves it + // on the status transition the request produces — grant runs it, a denial warns and runs it anyway. + var pendingTcpConnect by remember { mutableStateOf Unit>?>(null) } + val gateTcpConnect: (connect: () -> Unit) -> Unit = { connect -> + when (localNetworkGateAction(localNetworkPermission.status)) { + LocalNetworkGateAction.PROCEED -> connect() + + LocalNetworkGateAction.REQUEST_PERMISSION -> { + pendingTcpConnect = localNetworkPermission.status to connect + localNetworkPermission.request() + } + + LocalNetworkGateAction.PROCEED_WITH_WARNING -> { + scanModel.warnLocalNetworkPermissionDenied() + connect() + } + } + } + LaunchedEffect(localNetworkPermission.status) { + val pending = pendingTcpConnect ?: return@LaunchedEffect + when (resolvePendingTcpConnect(stashedStatus = pending.first, currentStatus = localNetworkPermission.status)) { + PendingTcpConnectResolution.CONNECT -> { + pendingTcpConnect = null + pending.second() + } + + PendingTcpConnectResolution.CONNECT_WITH_WARNING -> { + pendingTcpConnect = null + scanModel.warnLocalNetworkPermissionDenied() + pending.second() + } + + PendingTcpConnectResolution.KEEP_WAITING -> Unit + } + } + // Adapter-state, distinct from permission state: a permission can be granted while Bluetooth is off or the device // is off Wi-Fi. Detected separately so the UI can route to the adapter's settings rather than re-prompting. val bluetoothDisabled = isBluetoothDisabled() @@ -450,7 +492,16 @@ fun ConnectionsScreen( isBleScanning = isBleScanning, isNetworkScanning = isNetworkScanning, activeTransport = activeTransport, - onSelectDevice = { scanModel.onSelected(it) }, + onSelectDevice = { entry -> + // Recent TCP addresses are persisted, so this list renders without a scan — and + // therefore without the scan toggle's permission request ever having run. BLE and + // USB are unaffected by ACCESS_LOCAL_NETWORK, so only gate Tcp. + if (entry is DeviceListEntry.Tcp) { + gateTcpConnect { scanModel.onSelected(entry) } + } else { + scanModel.onSelected(entry) + } + }, onToggleBleScan = { when { // Always allow stopping an in-progress scan. @@ -488,7 +539,9 @@ fun ConnectionsScreen( } }, onAddManualAddress = { _, fullAddress -> - scanModel.connectToManualAddress(fullAddress) + // Typing an address is always allowed — the target may be a public host or VPN + // peer. The gate runs at connect, where the permission can actually matter. + gateTcpConnect { scanModel.connectToManualAddress(fullAddress) } }, onRemoveRecentAddress = { scanModel.removeRecentAddress(it.fullAddress) }, ) diff --git a/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/LocalNetworkGateAction.kt b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/LocalNetworkGateAction.kt new file mode 100644 index 0000000000..1e43945bfc --- /dev/null +++ b/feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ui/LocalNetworkGateAction.kt @@ -0,0 +1,85 @@ +/* + * 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.feature.connections.ui + +import org.meshtastic.core.ui.util.PermissionStatus + +/** + * What the Connections UI should do when the user takes a TCP-connect action that may need `ACCESS_LOCAL_NETWORK`. + * + * The policy is prompt-when-possible, warn-when-not, and NEVER block: on Android 17 (API 37) the permission gates the + * socket itself — an ungranted local connect times out rather than failing fast — but a TCP address says nothing about + * locality. A radio reached over a public IP, a port-forward, or a VPN needs no local-network permission at all, so a + * hard block on the permission would break connections that were never subject to it. + */ +enum class LocalNetworkGateAction { + /** The permission is held; run the connect. */ + PROCEED, + + /** The system will still show a prompt; request in-context and run the connect once the request resolves. */ + REQUEST_PERMISSION, + + /** + * The system will no longer prompt. Surface the warning that names the fix (system settings), then run the connect + * anyway — the target may not be local, and the OS enforces the permission at the socket regardless. + */ + PROCEED_WITH_WARNING, +} + +/** + * Pure classifier for gating a TCP-connect action on `ACCESS_LOCAL_NETWORK`. Kept side-effect-free and + * platform-agnostic so it can be unit-tested in `commonTest` without an Android `Activity`. + * + * Inert everywhere else: `rememberLocalNetworkPermissionState()` reports [PermissionStatus.GRANTED] below API 37 and on + * desktop/iOS, so this always returns [LocalNetworkGateAction.PROCEED] there. + */ +fun localNetworkGateAction(status: PermissionStatus): LocalNetworkGateAction = when (status) { + PermissionStatus.GRANTED -> LocalNetworkGateAction.PROCEED + PermissionStatus.PERMANENTLY_DENIED -> LocalNetworkGateAction.PROCEED_WITH_WARNING + PermissionStatus.NOT_REQUESTED -> LocalNetworkGateAction.REQUEST_PERMISSION + PermissionStatus.DENIED_CAN_RETRY -> LocalNetworkGateAction.REQUEST_PERMISSION +} + +/** How a connect stashed behind an in-flight permission request should be resolved once the status moves. */ +enum class PendingTcpConnectResolution { + /** The grant landed; run the stashed connect. */ + CONNECT, + + /** The request resolved as a denial; warn, then run the stashed connect anyway (see the policy above). */ + CONNECT_WITH_WARNING, + + /** The status has not moved since the stash; the request is still unresolved. */ + KEEP_WAITING, +} + +/** + * Resolves a TCP connect stashed while a permission request was in flight, given the status at stash time and now. + * + * A status *transition* is the only observable signal that the request resolved: the result callback recomputes the + * status, but a same-value recomputation is invisible to composition. Denials do transition on modern Android + * (NOT_REQUESTED → DENIED_CAN_RETRY, and a second denial → PERMANENTLY_DENIED), so the practical dangling case is + * limited to OEM flows that re-answer with an identical status — where the stash simply waits to be overwritten by the + * next tap. + */ +fun resolvePendingTcpConnect( + stashedStatus: PermissionStatus, + currentStatus: PermissionStatus, +): PendingTcpConnectResolution = when { + currentStatus == PermissionStatus.GRANTED -> PendingTcpConnectResolution.CONNECT + currentStatus != stashedStatus -> PendingTcpConnectResolution.CONNECT_WITH_WARNING + else -> PendingTcpConnectResolution.KEEP_WAITING +} diff --git a/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt b/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt index 7cb3efb7db..a8e637cd77 100644 --- a/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt +++ b/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt @@ -43,6 +43,7 @@ import org.meshtastic.core.resources.bluetooth_scan_start_failed import org.meshtastic.core.resources.bluetooth_scan_too_frequent import org.meshtastic.core.resources.getPluralStringSuspend import org.meshtastic.core.resources.getStringSuspend +import org.meshtastic.core.resources.local_network_permission_denied_hint import org.meshtastic.core.testing.FakeBleDevice import org.meshtastic.feature.connections.model.DeviceListEntry import org.meshtastic.feature.connections.model.DiscoveredDevices @@ -199,6 +200,20 @@ class ScannerViewModelTest { ) } + @Test + fun `warnLocalNetworkPermissionDenied surfaces the settings hint as a warning`() = runTest { + // Computing the expectation first also pre-warms the resource, keeping the production lookup observable + // synchronously (see warmScanFailureStrings). The safeCatchingAll-with-fallback shape matches production, + // so the expected text is identical whether resources resolve or the fallback fires. + val expected = + safeCatchingAll { getStringSuspend(Res.string.local_network_permission_denied_hint) } + .getOrDefault(LOCAL_NETWORK_PERMISSION_DENIED_HINT_FALLBACK) + + viewModel.warnLocalNetworkPermissionDenied() + + assertEquals(expected, serviceRepository.errorMessage.value) + } + @Test fun `scan startup failure cooldown prevents immediate retry and allows later manual retry`() = runTest { var scanAttempts = 0 diff --git a/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ui/LocalNetworkGateTest.kt b/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ui/LocalNetworkGateTest.kt new file mode 100644 index 0000000000..248d4538a9 --- /dev/null +++ b/feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ui/LocalNetworkGateTest.kt @@ -0,0 +1,102 @@ +/* + * 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.feature.connections.ui + +import org.meshtastic.core.ui.util.PermissionStatus +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * Covers the `ACCESS_LOCAL_NETWORK` policy applied to the Connections TCP-connect paths: prompt when possible, warn + * when not, never block. A TCP address says nothing about locality — public-IP/port-forward/VPN radios need no + * permission — so no branch may refuse the connect outright; on Android 17 the OS enforces the permission at the socket + * regardless. + */ +class LocalNetworkGateTest { + + @Test + fun `granted proceeds silently`() { + assertEquals(LocalNetworkGateAction.PROCEED, localNetworkGateAction(PermissionStatus.GRANTED)) + } + + @Test + fun `never requested prompts in-context`() { + assertEquals(LocalNetworkGateAction.REQUEST_PERMISSION, localNetworkGateAction(PermissionStatus.NOT_REQUESTED)) + } + + @Test + fun `retryable denial prompts again`() { + assertEquals( + LocalNetworkGateAction.REQUEST_PERMISSION, + localNetworkGateAction(PermissionStatus.DENIED_CAN_RETRY), + ) + } + + @Test + fun `permanent denial warns but still proceeds because the system will not prompt again`() { + assertEquals( + LocalNetworkGateAction.PROCEED_WITH_WARNING, + localNetworkGateAction(PermissionStatus.PERMANENTLY_DENIED), + ) + } + + // ── Resolution of a connect stashed behind an in-flight permission request ── + + @Test + fun `a grant runs the stashed connect`() { + assertEquals( + PendingTcpConnectResolution.CONNECT, + resolvePendingTcpConnect( + stashedStatus = PermissionStatus.NOT_REQUESTED, + currentStatus = PermissionStatus.GRANTED, + ), + ) + } + + @Test + fun `a first denial warns and runs the stashed connect anyway`() { + assertEquals( + PendingTcpConnectResolution.CONNECT_WITH_WARNING, + resolvePendingTcpConnect( + stashedStatus = PermissionStatus.NOT_REQUESTED, + currentStatus = PermissionStatus.DENIED_CAN_RETRY, + ), + ) + } + + @Test + fun `a denial that becomes permanent warns and runs the stashed connect anyway`() { + assertEquals( + PendingTcpConnectResolution.CONNECT_WITH_WARNING, + resolvePendingTcpConnect( + stashedStatus = PermissionStatus.DENIED_CAN_RETRY, + currentStatus = PermissionStatus.PERMANENTLY_DENIED, + ), + ) + } + + @Test + fun `an unchanged status keeps waiting for the request to resolve`() { + assertEquals( + PendingTcpConnectResolution.KEEP_WAITING, + resolvePendingTcpConnect( + stashedStatus = PermissionStatus.NOT_REQUESTED, + currentStatus = PermissionStatus.NOT_REQUESTED, + ), + ) + } +}