Skip to content

Commit be7c008

Browse files
jamesarichCopilot
andcommitted
fix(permissions): gate LOCAL_NETWORK to TAK Server, use core/ui helper instead of Accompanist
- Remove global ACCESS_LOCAL_NETWORK declaration from manifest (only needed for TAK localhost binding) - Add rememberRequestLocalNetworkPermission to core/ui PlatformUtils API - Implement for Android (gated to API 31+), iOS/JVM (no-ops) - Replace Accompanist in TAK permission handler with core/ui helper - Reduces per-reinstall permission prompts; users only see dialog when enabling TAK - Consistent with project's existing permission pattern (location/BLE/notifications) - Net -18 LOC Fixes unnecessary LOCAL_NETWORK permission dialog on app reinstall when TAK Server is disabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ef44e41 commit be7c008

7 files changed

Lines changed: 46 additions & 30 deletions

File tree

AGENTS.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ Do NOT duplicate content into agent-specific files. When you modify architecture
7272
- **Dependency Discipline:** Never add a library without first checking `libs.versions.toml` and justifying its inclusion against the project's size and complexity goals. Prefer removing dependencies over adding them.
7373
- **Zero Lint Tolerance:** A task is incomplete if `detekt` fails or `spotlessCheck` does not pass for touched modules.
7474
- **Read Before Refactoring:** When a pattern contradicts best practices, analyze whether it is legacy debt or a deliberate architectural choice before proposing a change.
75+
- **Verify Before Push:** Treat any "push", "commit and push", or "push and pr" request as **verify-then-push**. Before `git push`, run `./gradlew spotlessApply detekt` (and the relevant `:module:test` / `:module:lint<Flavor>Debug` for touched modules). CI has repeatedly failed on `UnusedParameter`, `CyclomaticComplexMethod`, and `MagicNumber` from skipping this step. Only push on green; if a check fails, fix it before pushing.
76+
- **Never Touch Protos or Secrets:** `core/proto/src/main/proto` is an upstream submodule — **do not modify** any `.proto` file. If a feature request requires a proto change, stop and report it as upstream (label issue `upstream`, point at `meshtastic/protobufs`). Likewise, never `git add` `app/google-services.json`, `local.properties`, `secrets.properties`, or any `*.keystore` / `*.jks` file — these are gitignored and contain secrets.
77+
- **Multi-Flavor Install Hygiene:** When using the `android` CLI MCP to install/run on a connected device, the `fdroid` (`com.geeksville.mesh`) and `google` (`com.geeksville.mesh.google`) flavors have different signatures and **cannot coexist**. Before any install: pick a flavor explicitly, force-stop and uninstall the other flavor on every connected device, then install. Stale installs of the other flavor are a recurring source of "the fix didn't work" red herrings.
78+
- **Verify UI With Annotated Screenshots:** For any UI/UX task, do **not** claim a fix works based on logs or assumed state. Capture an annotated screenshot via the `android` CLI MCP (or its annotated-screenshot tool) on a real connected device, and inspect the result before reporting back.
79+
- **Branch Scope Discipline:** If a working branch grows beyond ~5 logical commits, crosses unrelated concerns, or accumulates a large blast radius, proactively propose a fresh branch off `upstream/main` and cherry-pick only the high-signal, low-risk changes (see `.skills/new-branch/SKILL.md`). Don't keep piling onto a sprawling branch.
7580
</rules>
7681

7782
<copilot_cli_workflow>

app/src/main/AndroidManifest.xml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@
4949

5050
<!-- This permission is required for analytics - and soon the MQTT gateway -->
5151
<uses-permission android:name="android.permission.INTERNET" />
52-
<!-- Required for Android 17+ (API 37) Local Networking for TAK Server localhost loopback -->
53-
<uses-permission android:name="android.permission.ACCESS_LOCAL_NETWORK" />
5452

5553
<uses-permission android:name="android.permission.WAKE_LOCK" />
5654

core/ui/src/androidMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,21 @@ actual fun rememberRequestNotificationPermission(onGranted: () -> Unit, onDenied
256256
return remember(launcher) { { launcher.launch(android.Manifest.permission.POST_NOTIFICATIONS) } }
257257
}
258258

259+
@Composable
260+
actual fun rememberRequestLocalNetworkPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit {
261+
if (android.os.Build.VERSION.SDK_INT < android.os.Build.VERSION_CODES.S) {
262+
// Pre-Android 12, no local network permission required
263+
return remember { { onGranted() } }
264+
}
265+
val currentOnGranted = rememberUpdatedState(onGranted)
266+
val currentOnDenied = rememberUpdatedState(onDenied)
267+
val launcher =
268+
rememberLauncherForActivityResult(ActivityResultContracts.RequestPermission()) { granted ->
269+
if (granted) currentOnGranted.value() else currentOnDenied.value()
270+
}
271+
return remember(launcher) { { launcher.launch(android.Manifest.permission.ACCESS_LOCAL_NETWORK) } }
272+
}
273+
259274
@Composable
260275
actual fun isLocationPermissionGranted(): Boolean {
261276
val context = LocalContext.current

core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ expect fun rememberSaveFileLauncher(
6767
/** Returns a launcher to request Bluetooth scan + connect permissions. No-op on platforms without runtime BLE perms. */
6868
@Composable expect fun rememberRequestBluetoothPermission(onGranted: () -> Unit, onDenied: () -> Unit = {}): () -> Unit
6969

70+
/** Returns a launcher to request the ACCESS_LOCAL_NETWORK permission. No-op on platforms that don't require it. */
71+
@Composable
72+
expect fun rememberRequestLocalNetworkPermission(onGranted: () -> Unit, onDenied: () -> Unit = {}): () -> Unit
73+
7074
/** Returns a launcher to request the POST_NOTIFICATIONS permission. No-op on platforms that don't require it. */
7175
@Composable
7276
expect fun rememberRequestNotificationPermission(onGranted: () -> Unit, onDenied: () -> Unit = {}): () -> Unit

core/ui/src/iosMain/kotlin/org/meshtastic/core/ui/util/NoopStubs.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ actual fun rememberOpenFileLauncher(onUriReceived: (CommonUri?) -> Unit): (mimeT
5858

5959
@Composable actual fun rememberRequestBluetoothPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {}
6060

61+
@Composable
62+
actual fun rememberRequestLocalNetworkPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {}
63+
6164
@Composable
6265
actual fun rememberRequestNotificationPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {}
6366

core/ui/src/jvmMain/kotlin/org/meshtastic/core/ui/util/PlatformUtils.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,12 @@ actual fun rememberOpenLocationSettings(): () -> Unit = { Logger.w { "Location s
134134
@Composable
135135
actual fun rememberRequestBluetoothPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = { onGranted() }
136136

137+
/** JVM no-op — Desktop does not require runtime local network permissions. */
138+
@Composable
139+
actual fun rememberRequestLocalNetworkPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {
140+
onGranted()
141+
}
142+
137143
/** JVM no-op — Desktop does not require runtime notification permissions. */
138144
@Composable
139145
actual fun rememberRequestNotificationPermission(onGranted: () -> Unit, onDenied: () -> Unit): () -> Unit = {

feature/settings/src/androidMain/kotlin/org/meshtastic/feature/settings/tak/TakPermissionUtil.kt

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,38 +16,23 @@
1616
*/
1717
package org.meshtastic.feature.settings.tak
1818

19-
import android.os.Build
2019
import androidx.compose.runtime.Composable
21-
import androidx.compose.runtime.LaunchedEffect
22-
import com.google.accompanist.permissions.ExperimentalPermissionsApi
23-
import com.google.accompanist.permissions.isGranted
24-
import com.google.accompanist.permissions.rememberPermissionState
20+
import org.meshtastic.core.ui.util.rememberRequestLocalNetworkPermission
2521

26-
private val SDK_INT_ANDROID_16 = Build.VERSION_CODES.BAKLAVA
27-
28-
@OptIn(ExperimentalPermissionsApi::class)
2922
@Composable
3023
actual fun TakPermissionHandler(isTakServerEnabled: Boolean, onPermissionResult: (Boolean) -> Unit) {
31-
if (Build.VERSION.SDK_INT >= SDK_INT_ANDROID_16) {
32-
val permissionState =
33-
rememberPermissionState("android.permission.ACCESS_LOCAL_NETWORK") { granted ->
34-
// Callback fires after the system dialog is dismissed — report the result
35-
// directly so onPermissionResult is the single authority for grant/deny.
36-
if (isTakServerEnabled) onPermissionResult(granted)
37-
}
24+
// ACCESS_LOCAL_NETWORK permission (Android 12+) is only required for TAK Server's localhost
25+
// socket binding (127.0.0.1:8087). It is NOT required for NSD mDNS service discovery
26+
// (which uses CHANGE_WIFI_MULTICAST_STATE). By gating the permission request to only when
27+
// TAK Server is explicitly enabled, casual users avoid the system dialog and the app works
28+
// without the permission if TAK is disabled.
29+
val requestPermission =
30+
rememberRequestLocalNetworkPermission(
31+
onGranted = { onPermissionResult(true) },
32+
onDenied = { onPermissionResult(false) },
33+
)
3834

39-
LaunchedEffect(isTakServerEnabled) {
40-
if (isTakServerEnabled) {
41-
if (permissionState.status.isGranted) {
42-
// Already granted — confirm immediately so the orchestrator may proceed.
43-
onPermissionResult(true)
44-
} else {
45-
// Show system dialog; result is delivered via the callback above.
46-
permissionState.launchPermissionRequest()
47-
}
48-
}
49-
}
50-
} else {
51-
LaunchedEffect(isTakServerEnabled) { onPermissionResult(true) }
35+
if (isTakServerEnabled) {
36+
requestPermission()
5237
}
5338
}

0 commit comments

Comments
 (0)