refactor(node): Improve public key conflict handling (#4486)

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>

core/database/src/androidTest/kotlin/org/meshtastic/core/database/dao/NodeInfoDaoTest.kt

@@ -22,6 +22,7 @@ import androidx.test.platform.app.InstrumentationRegistry
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.runBlocking
+import okio.ByteString
 import okio.ByteString.Companion.toByteString
 import org.junit.After
 import org.junit.Assert.assertEquals
@@ -338,10 +339,77 @@ class NodeInfoDaoTest {
 
     @Test
     fun testPkcMismatch() = runBlocking {
-        val newNode = testNodes[1].copy(user = testNodes[1].user.copy(public_key = ByteArray(32) { 99 }.toByteString()))
-        nodeInfoDao.putAll(listOf(newNode))
-        val nodes = getNodes()
-        val containsMismatchNode = nodes.any { it.mismatchKey }
-        assertTrue(containsMismatchNode)
+        // First, ensure the node is in the DB with Key A
+        val nodeA = testNodes[10].copy(publicKey = ByteArray(32) { 1 }.toByteString())
+        nodeInfoDao.upsert(nodeA)
+
+        // Now upsert with Key B (mismatch)
+        val nodeB =
+            nodeA.copy(
+                publicKey = ByteArray(32) { 2 }.toByteString(),
+                user = nodeA.user.copy(public_key = ByteArray(32) { 2 }.toByteString()),
+            )
+        nodeInfoDao.upsert(nodeB)
+
+        val stored = nodeInfoDao.getNodeByNum(nodeA.num)!!.node
+        assertEquals(NodeEntity.ERROR_BYTE_STRING, stored.publicKey)
+        assertTrue(stored.toModel().mismatchKey)
+    }
+
+    @Test
+    fun testRoutineUpdatePreservesKey() = runBlocking {
+        // First, ensure the node is in the DB with Key A
+        val keyA = ByteArray(32) { 1 }.toByteString()
+        val nodeA = testNodes[10].copy(publicKey = keyA, user = testNodes[10].user.copy(public_key = keyA))
+        nodeInfoDao.upsert(nodeA)
+
+        // Now upsert with an empty key (common in position/telemetry updates)
+        val nodeEmpty = nodeA.copy(publicKey = null, user = nodeA.user.copy(public_key = ByteString.EMPTY))
+        nodeInfoDao.upsert(nodeEmpty)
+
+        val stored = nodeInfoDao.getNodeByNum(nodeA.num)!!.node
+        assertEquals(keyA, stored.publicKey)
+        assertFalse(stored.toModel().mismatchKey)
+    }
+
+    @Test
+    fun testRecoveryFromErrorState() = runBlocking {
+        // Start in Error state
+        val nodeError =
+            testNodes[10].copy(
+                publicKey = NodeEntity.ERROR_BYTE_STRING,
+                user = testNodes[10].user.copy(public_key = NodeEntity.ERROR_BYTE_STRING),
+            )
+        nodeInfoDao.doUpsert(nodeError)
+        assertTrue(nodeInfoDao.getNodeByNum(nodeError.num)!!.toModel().mismatchKey)
+
+        // Now upsert with a valid Key C
+        val keyC = ByteArray(32) { 3 }.toByteString()
+        val nodeC = nodeError.copy(publicKey = keyC, user = nodeError.user.copy(public_key = keyC))
+        nodeInfoDao.upsert(nodeC)
+
+        val stored = nodeInfoDao.getNodeByNum(nodeError.num)!!.node
+        assertEquals(keyC, stored.publicKey)
+        assertFalse(stored.toModel().mismatchKey)
+    }
+
+    @Test
+    fun testLicensedUserClearsKey() = runBlocking {
+        // Start with a key
+        val keyA = ByteArray(32) { 1 }.toByteString()
+        val nodeA = testNodes[10].copy(publicKey = keyA, user = testNodes[10].user.copy(public_key = keyA))
+        nodeInfoDao.upsert(nodeA)
+
+        // Upsert as licensed user
+        val nodeLicensed =
+            nodeA.copy(
+                user = nodeA.user.copy(is_licensed = true, public_key = ByteString.EMPTY),
+                publicKey = ByteString.EMPTY,
+            )
+        nodeInfoDao.upsert(nodeLicensed)
+
+        val stored = nodeInfoDao.getNodeByNum(nodeA.num)!!.node
+        assertTrue(stored.publicKey == null || (stored.publicKey?.size ?: 0) == 0)
+        assertFalse(stored.toModel().mismatchKey)
     }
 }

core/database/src/main/kotlin/org/meshtastic/core/database/dao/NodeInfoDao.kt

@@ -84,6 +84,7 @@ interface NodeInfoDao {
         return newNode
     }
 
+    @Suppress("CyclomaticComplexMethod", "MagicNumber")
     private fun handleExistingNodeUpsertValidation(existingNode: NodeEntity, incomingNode: NodeEntity): NodeEntity {
         val isPlaceholder = incomingNode.user.hw_model == HardwareModel.UNSET
         val hasExistingUser = existingNode.user.hw_model != HardwareModel.UNSET
@@ -113,27 +114,40 @@ interface NodeInfoDao {
             )
         }
 
-        // A public key is considered matching if the incoming key equals the existing key,
-        // OR if the existing key is empty (allowing a new key to be set or an update to proceed).
-        val existingResolvedKey = existingNode.publicKey ?: existingNode.user.public_key
-        val isPublicKeyMatchingOrExistingIsEmpty = existingResolvedKey == incomingNode.publicKey || !existingNode.hasPKC
+        val existingKey = existingNode.publicKey ?: existingNode.user.public_key
+        val incomingKey = incomingNode.publicKey
+
+        val incomingHasKey = (incomingKey?.size ?: 0) == 32
+        val existingHasKey = (existingKey?.size ?: 0) == 32 && existingKey != NodeEntity.ERROR_BYTE_STRING
+
+        val resolvedKey =
+            when {
+                incomingHasKey -> {
+                    if (existingHasKey && incomingKey != existingKey) {
+                        // Actual mismatch between two non-empty keys
+                        NodeEntity.ERROR_BYTE_STRING
+                    } else {
+                        // New key, same key, or recovery from Error state
+                        incomingKey
+                    }
+                }
+                incomingNode.user.is_licensed -> {
+                    // Explicitly clear key for licensed (HAM) users
+                    ByteString.EMPTY
+                }
+                else -> {
+                    // Routine update without key: preserve what we have (even if it's currently Error)
+                    existingKey
+                }
+            }
 
         val resolvedNotes = if (incomingNode.notes.isBlank()) existingNode.notes else incomingNode.notes
 
-        return if (isPublicKeyMatchingOrExistingIsEmpty) {
-            // Keys match or existing key was empty: trust the incoming node data completely.
-            // This allows for legitimate updates to user info and other fields.
-            incomingNode.copy(notes = resolvedNotes)
-        } else {
-            // Public key mismatch: This could be a factory reset or a hardware ID collision.
-            // We allow the name and user info to update, but we clear the public key
-            // to indicate that this node is no longer "verified" against the previous key.
-            incomingNode.copy(
-                user = incomingNode.user.copy(public_key = NodeEntity.ERROR_BYTE_STRING),
-                publicKey = NodeEntity.ERROR_BYTE_STRING,
-                notes = resolvedNotes,
-            )
-        }
+        return incomingNode.copy(
+            user = incomingNode.user.copy(public_key = resolvedKey ?: ByteString.EMPTY),
+            publicKey = resolvedKey,
+            notes = resolvedNotes,
+        )
     }
 
     @Query("SELECT * FROM my_node")

core/ui/src/main/kotlin/org/meshtastic/core/ui/component/EditBase64Preference.kt

@@ -51,7 +51,7 @@ import org.meshtastic.core.strings.Res
 import org.meshtastic.core.strings.error
 import org.meshtastic.core.strings.reset
 
-@Suppress("LongMethod")
+@Suppress("LongMethod", "CyclomaticComplexMethod", "MagicNumber")
 @Composable
 fun EditBase64Preference(
     modifier: Modifier = Modifier,
@@ -65,14 +65,16 @@ fun EditBase64Preference(
     onGenerateKey: (() -> Unit)? = null,
     trailingIcon: (@Composable () -> Unit)? = null,
 ) {
-    var valueState by remember { mutableStateOf(value.encodeToString()) }
-    val isError = value.encodeToString() != valueState
+    val isMismatch = value.size == 32 && value.toByteArray().all { it == 0.toByte() }
+    val errorString = stringResource(Res.string.error)
+    var valueState by remember { mutableStateOf(if (isMismatch) errorString else value.encodeToString()) }
+    val isError = value.encodeToString() != valueState || isMismatch
 
     // don't update values while the user is editing
     var isFocused by remember { mutableStateOf(false) }
     LaunchedEffect(value) {
         if (!isFocused) {
-            valueState = value.encodeToString()
+            valueState = if (isMismatch) errorString else value.encodeToString()
         }
     }
 

core/ui/src/main/kotlin/org/meshtastic/core/ui/component/NodeKeyStatusIcon.kt

@@ -63,6 +63,7 @@ import org.meshtastic.core.strings.encryption_pkc
 import org.meshtastic.core.strings.encryption_pkc_text
 import org.meshtastic.core.strings.encryption_psk
 import org.meshtastic.core.strings.encryption_psk_text
+import org.meshtastic.core.strings.error
 import org.meshtastic.core.strings.security_icon_help_dismiss
 import org.meshtastic.core.strings.security_icon_help_show_all
 import org.meshtastic.core.strings.security_icon_help_show_less
@@ -170,6 +171,7 @@ enum class NodeKeySecurityState(
     ),
 }
 
+@Suppress("LongMethod", "MagicNumber")
 @Composable
 private fun KeyStatusDialog(title: StringResource, text: StringResource, key: ByteString?, onDismiss: () -> Unit = {}) {
     var showAll by rememberSaveable { mutableStateOf(false) }
@@ -190,16 +192,30 @@ private fun KeyStatusDialog(title: StringResource, text: StringResource, key: By
                 Column(horizontalAlignment = Alignment.CenterHorizontally) {
                     Text(text = stringResource(text), textAlign = TextAlign.Center)
                     Spacer(Modifier.height(16.dp))
-                    if (key != null && title == Res.string.encryption_pkc) {
-                        val keyString = Base64.encodeToString(key.toByteArray(), Base64.NO_WRAP)
+                    if (key != null && (title == Res.string.encryption_pkc || title == Res.string.encryption_error)) {
+                        val isMismatch = key.size == 32 && key.toByteArray().all { it == 0.toByte() }
+                        val keyString =
+                            if (isMismatch) {
+                                stringResource(Res.string.error)
+                            } else {
+                                Base64.encodeToString(key.toByteArray(), Base64.NO_WRAP)
+                            }
                         Text(
                             text = stringResource(Res.string.config_security_public_key) + ":",
                             textAlign = TextAlign.Center,
                         )
                         Spacer(Modifier.height(8.dp))
-                        SelectionContainer { Text(text = keyString, textAlign = TextAlign.Center) }
-                        Spacer(Modifier.height(8.dp))
-                        CopyIconButton(valueToCopy = keyString, modifier = Modifier.padding(start = 8.dp))
+                        SelectionContainer {
+                            Text(
+                                text = keyString,
+                                textAlign = TextAlign.Center,
+                                color = if (isMismatch) MaterialTheme.colorScheme.error else Color.Unspecified,
+                            )
+                        }
+                        if (!isMismatch) {
+                            Spacer(Modifier.height(8.dp))
+                            CopyIconButton(valueToCopy = keyString, modifier = Modifier.padding(start = 8.dp))
+                        }
                         Spacer(Modifier.height(16.dp))
                     }
                 }

feature/node/src/main/kotlin/org/meshtastic/feature/node/component/NodeDetailsSection.kt

@@ -59,6 +59,7 @@ import org.meshtastic.core.strings.copy
 import org.meshtastic.core.strings.details
 import org.meshtastic.core.strings.encryption_error
 import org.meshtastic.core.strings.encryption_error_text
+import org.meshtastic.core.strings.error
 import org.meshtastic.core.strings.hops_away
 import org.meshtastic.core.strings.node_id
 import org.meshtastic.core.strings.node_number
@@ -289,11 +290,18 @@ private fun MqttAndVerificationRow(node: Node) {
 }
 
 @OptIn(ExperimentalFoundationApi::class)
+@Suppress("LongMethod", "MagicNumber")
 @Composable
 private fun PublicKeyItem(publicKeyBytes: ByteArray) {
     val clipboard: Clipboard = LocalClipboard.current
     val coroutineScope = rememberCoroutineScope()
-    val publicKeyBase64 = Base64.encodeToString(publicKeyBytes, Base64.DEFAULT).trim()
+    val isMismatch = publicKeyBytes.all { it == 0.toByte() } && publicKeyBytes.size == 32
+    val publicKeyBase64 =
+        if (isMismatch) {
+            stringResource(Res.string.error)
+        } else {
+            Base64.encodeToString(publicKeyBytes, Base64.DEFAULT).trim()
+        }
     val label = stringResource(Res.string.public_key)
     val copyLabel = stringResource(Res.string.copy)
 
@@ -303,8 +311,10 @@ private fun PublicKeyItem(publicKeyBytes: ByteArray) {
             .defaultMinSize(minHeight = 48.dp)
             .combinedClickable(
                 onLongClick = {
-                    coroutineScope.launch {
-                        clipboard.setClipEntry(ClipEntry(ClipData.newPlainText(label, publicKeyBase64)))
+                    if (!isMismatch) {
+                        coroutineScope.launch {
+                            clipboard.setClipEntry(ClipEntry(ClipData.newPlainText(label, publicKeyBase64)))
+                        }
                     }
                 },
                 onLongClickLabel = copyLabel,
@@ -319,13 +329,18 @@ private fun PublicKeyItem(publicKeyBytes: ByteArray) {
                 imageVector = MeshtasticIcons.Lock,
                 contentDescription = null,
                 modifier = Modifier.size(14.dp),
-                tint = MaterialTheme.colorScheme.primary.copy(alpha = 0.8f),
+                tint =
+                if (isMismatch) {
+                    MaterialTheme.colorScheme.error
+                } else {
+                    MaterialTheme.colorScheme.primary.copy(alpha = 0.8f)
+                },
             )
             Spacer(Modifier.width(6.dp))
             Text(
                 text = label,
                 style = MaterialTheme.typography.labelSmall,
-                color = MaterialTheme.colorScheme.onSurfaceVariant,
+                color = if (isMismatch) MaterialTheme.colorScheme.error else MaterialTheme.colorScheme.onSurfaceVariant,
                 fontWeight = FontWeight.Bold,
             )
         }
@@ -333,7 +348,7 @@ private fun PublicKeyItem(publicKeyBytes: ByteArray) {
         Text(
             text = publicKeyBase64,
             style = MaterialTheme.typography.bodySmall.copy(fontFamily = FontFamily.Monospace),
-            color = MaterialTheme.colorScheme.onSurface,
+            color = if (isMismatch) MaterialTheme.colorScheme.error else MaterialTheme.colorScheme.onSurface,
         )
     }
 }