fix(connections): gate manual + recent TCP connects on ACCESS_LOCAL_NETWORK (#6766)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
James RichandClaude Opus 5 authored and GitHub committed 2026-08-18 23:29:46 +00:00
1 parent 94e77a5c92
commit 656f481c68
7 files changed
+281 -2

No files matched your search

+1
View File
@@ -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
@@ -962,6 +962,7 @@
<string name="loading">Loading</string>
<!-- LOCAL -->
<string name="local_mbtiles_file">Local MBTiles File</string>
<string name="local_network_permission_denied_hint">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.</string>
<string name="local_stats_bad">Bad %1$d</string>
<string name="local_stats_battery">Battery: %1$d%</string>
<string name="local_stats_diagnostics_prefix">Diagnostics: %1$s</string>
@@ -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.
@@ -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<Pair<PermissionStatus, () -> 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) },
)
@@ -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 <https://www.gnu.org/licenses/>.
*/
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
}
@@ -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
@@ -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 <https://www.gnu.org/licenses/>.
*/
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,
),
)
}
}