feat(config): make position & telemetry broadcast opt-in (#10929)

Position sharing was opt-out (the default primary channel shipped position_precision=13) while device telemetry was already opt-in, and a normal firmware upgrade preserves saved config, so existing nodes stayed position-on. This makes both broadcasts opt-in, both on a fresh flash and via a one-time migration for upgrading nodes. - Fresh default: initDefaultChannel now sets position_precision=0. - One-time migration in loadFromDisk, gated on a dedicated POSITION_TELEMETRY_OPTIN_VER (26) watermark kept separate from DEVICESTATE_CUR_VER (bumping that would re-run the NodeDatabase v24 legacy decoder on already-migrated v25 DBs): disables position broadcast on PUBLIC/default-PSK channels and forces every device-telemetry mesh-broadcast flag plus the MQTT map-report location off. - Private-PSK channels are left untouched: a channel with a custom key is a deliberate trusted-group setup, so its configured precision is preserved. - Idempotent: ordinary saves never re-stamp .version, so a user who re-enables sharing is not re-disabled on the next boot. - Added pure channelFileUsesPublicKey() (operates on the raw ChannelFile, since the channels singleton isn't initialized during loadFromDisk) and refactored Channels::usesPublicKey() to delegate to it. - New test/test_optin_migration native suite (14 cases).
2026-08-01 02:48:49 -04:00 · 2026-07-07 14:58:58 -05:00
parent d16ae2b098
commit 8d20606203
6 changed files with 305 additions and 8 deletions
--- a/src/mesh/Channels.cpp
+++ b/src/mesh/Channels.cpp
@@ -156,7 +156,9 @@ void Channels::initDefaultChannel(ChannelIndex chIndex)
    channelSettings.psk.bytes[0] = defaultpskIndex;
    channelSettings.psk.size = 1;
    strncpy(channelSettings.name, "", sizeof(channelSettings.name));
-    channelSettings.module_settings.position_precision = 13; // default to sending location on the primary channel
+    // Position sharing is OPT-IN: precision 0 means "do not broadcast location". A user (or the phone app)
+    // must explicitly raise precision to start sharing. See the one-time opt-in migration in NodeDB.cpp.
+    channelSettings.module_settings.position_precision = 0;
    channelSettings.has_module_settings = true;

    ch.has_settings = true;
@@ -436,9 +438,11 @@ bool cryptoKeyIsPublic(const CryptoKey &key)
    return false;
 }

-bool Channels::usesPublicKey(ChannelIndex chIndex)
+bool channelFileUsesPublicKey(const meshtastic_ChannelFile &cf, ChannelIndex chIndex)
 {
-    const meshtastic_Channel &ch = getByIndex(chIndex);
+    if (chIndex >= cf.channels_count)
+        return false;
+    const meshtastic_Channel &ch = cf.channels[chIndex];
    if (!ch.has_settings || ch.role == meshtastic_Channel_Role_DISABLED)
        return false;

@@ -446,10 +450,14 @@ bool Channels::usesPublicKey(ChannelIndex chIndex)
    if (psk.size == 0) {
        // Secondary channels inherit the primary key when unset; primary size==0 means encryption disabled.
        if (ch.role == meshtastic_Channel_Role_SECONDARY) {
-            // Guard against malformed configs with no PRIMARY channel (primaryIndex could point back to us).
-            if (primaryIndex == chIndex)
-                return true; // fail closed: treat as public
-            return usesPublicKey(primaryIndex);
+            // Resolve against the PRIMARY channel's key. The singleton's primaryIndex isn't available in
+            // the raw-struct path (this runs during boot migration before initDefaults), so scan by role.
+            // Fail closed to "public" if no distinct PRIMARY is found (malformed config).
+            for (pb_size_t p = 0; p < cf.channels_count; p++) {
+                if (cf.channels[p].role == meshtastic_Channel_Role_PRIMARY)
+                    return (p == chIndex) ? true : channelFileUsesPublicKey(cf, (ChannelIndex)p);
+            }
+            return true;
        }
        return true;
    }
@@ -462,6 +470,13 @@ bool Channels::usesPublicKey(ChannelIndex chIndex)
    return (psk.size == sizeof(defaultpsk) && memcmp(psk.bytes, defaultpsk, sizeof(defaultpsk) - 1) == 0);
 }

+bool Channels::usesPublicKey(ChannelIndex chIndex)
+{
+    // Delegates to the pure, on-disk-struct variant so the two can't drift. getByIndex() reads the same
+    // global channelFile, so this is behavior-preserving for the position-precision clamp callers.
+    return channelFileUsesPublicKey(channelFile, chIndex);
+}
+
 bool Channels::isWellKnownChannel(ChannelIndex chIndex)
 {
    const auto &ch = getByIndex(chIndex);
--- a/src/mesh/Channels.h
+++ b/src/mesh/Channels.h
@@ -156,6 +156,12 @@ static const uint8_t defaultpsk[] = {0xd4, 0xf1, 0xbb, 0x3a, 0x20, 0x29, 0x07, 0
 /// True if a getKey()-resolved key offers no privacy: length 0 (off) or the public defaultpsk family. Pure; for tests.
 bool cryptoKeyIsPublic(const CryptoKey &key);

+/// True if channel `chIndex` in a raw ChannelFile is publicly decryptable (open / single-byte well-known
+/// index / defaultpsk family). Pure equivalent of Channels::usesPublicKey that operates on the on-disk
+/// struct directly, so it is callable before the `channels` singleton is initialized (e.g. during the
+/// NodeDB boot migration). Resolves a PSK-less SECONDARY against the PRIMARY channel's key. For tests.
+bool channelFileUsesPublicKey(const meshtastic_ChannelFile &cf, ChannelIndex chIndex);
+
 static const uint8_t eventpsk[] = {0x38, 0x4b, 0xbc, 0xc0, 0x1d, 0xc0, 0x22, 0xd1, 0x81, 0xbf, 0x36,
                                   0xb8, 0x61, 0x21, 0xe1, 0xfb, 0x96, 0xb7, 0x2e, 0x55, 0xbf, 0x74,
                                   0x22, 0x7e, 0x9d, 0x6a, 0xfb, 0x48, 0xd6, 0x4c, 0xb1, 0xa1};
--- a/src/mesh/NodeDB.cpp
+++ b/src/mesh/NodeDB.cpp
@@ -1184,6 +1184,36 @@ static void installTrafficManagementDefaults(meshtastic_LocalModuleConfig &mc)
 #endif
 }

+// --- 2.8 position/telemetry opt-in migration helpers -------------------------------------------------
+// Pure field mutators (no I/O), so the native test suite can exercise them directly. The version gate
+// and saveToDisk live in loadFromDisk() below.
+
+void optInDisablePositionSharing(meshtastic_ChannelFile &cf)
+{
+    for (pb_size_t i = 0; i < cf.channels_count; i++) {
+        // Only flip PUBLIC / default-PSK channels. A channel with a real private key is a deliberate
+        // trusted-group setup where the "leak location to strangers" concern doesn't apply, so its
+        // configured precision (including full precision) is preserved.
+        if (!channelFileUsesPublicKey(cf, (ChannelIndex)i))
+            continue;
+        cf.channels[i].settings.has_module_settings = true;
+        cf.channels[i].settings.module_settings.position_precision = 0;
+    }
+}
+
+void optInDisableTelemetryBroadcast(meshtastic_LocalModuleConfig &mc)
+{
+    // Every mesh-broadcast telemetry enable flag (each gates its module's sendTelemetry() to the mesh).
+    mc.telemetry.device_telemetry_enabled = false;
+    mc.telemetry.environment_measurement_enabled = false;
+    mc.telemetry.air_quality_enabled = false;
+    mc.telemetry.power_measurement_enabled = false;
+    mc.telemetry.health_measurement_enabled = false;
+    // Position leak via the public MQTT map. Leave map_reporting_enabled alone (anonymous presence is
+    // still allowed) and strip only the location component.
+    mc.mqtt.map_report_settings.should_report_location = false;
+}
+
 void NodeDB::installDefaultModuleConfig()
 {
    LOG_INFO("Install default ModuleConfig");
@@ -2551,6 +2581,24 @@ void NodeDB::loadFromDisk()

        saveToDisk(SEGMENT_MODULECONFIG);
    }
+
+    // 2.8 - privacy: one-time flip of position sharing and device telemetry to OPT-IN for nodes upgrading
+    // from a build that shipped them on-by-default. Gated on a dedicated watermark (POSITION_TELEMETRY_OPTIN_VER)
+    // so it runs exactly once and does NOT re-clobber a user who later re-enables sharing (ordinary saves never
+    // re-stamp .version, so a re-enabled node stays at the watermark and skips this block on the next boot).
+    // Position is disabled only on public/default-PSK channels; private-PSK channels are preserved.
+    if (channelFile.version < POSITION_TELEMETRY_OPTIN_VER) {
+        LOG_INFO("Opt-in migration: disabling position broadcast on public channels");
+        optInDisablePositionSharing(channelFile);
+        channelFile.version = POSITION_TELEMETRY_OPTIN_VER;
+        saveToDisk(SEGMENT_CHANNELS);
+    }
+    if (moduleConfig.version < POSITION_TELEMETRY_OPTIN_VER) {
+        LOG_INFO("Opt-in migration: forcing device telemetry broadcast to opt-in");
+        optInDisableTelemetryBroadcast(moduleConfig);
+        moduleConfig.version = POSITION_TELEMETRY_OPTIN_VER;
+        saveToDisk(SEGMENT_MODULECONFIG);
+    }
 #if ARCH_PORTDUINO
    // set any config overrides
    if (portduino_config.has_configDisplayMode) {
--- a/src/mesh/NodeDB.h
+++ b/src/mesh/NodeDB.h
@@ -90,6 +90,15 @@ DeviceState versions used to be defined in the .proto file but really only this
 // at boot via the parallel deviceonly_legacy descriptor and re-saved as v25.
 #define DEVICESTATE_MIN_VER 24

+// One-time behavioral migration marker for the 2.8 position/telemetry opt-in flip.
+// Deliberately kept separate from DEVICESTATE_CUR_VER: that constant also drives the
+// NodeDatabase slim-schema legacy gate (NodeDB.cpp, `nodeDatabase.version < CUR_VER`),
+// so bumping it would wrongly re-run the v24 legacy decoder on already-migrated v25
+// node DBs. This watermark is stamped only onto channelFile.version / moduleConfig.version
+// once the opt-in migration has run. RESERVES 26 - the next real on-disk schema change
+// should raise DEVICESTATE_CUR_VER to 27, not 26.
+#define POSITION_TELEMETRY_OPTIN_VER 26
+
 extern meshtastic_DeviceState devicestate;
 extern meshtastic_NodeDatabase nodeDatabase;
 extern meshtastic_ChannelFile channelFile;
@@ -154,6 +163,13 @@ inline bool shouldDropPacketForPreHop(const meshtastic_MeshPacket &p)
 /// Rate-limited debug log when hop_start is invalid/missing and packet is dropped.
 void logHopStartDrop(const meshtastic_MeshPacket &p, const char *context);

+/// 2.8 position/telemetry opt-in migration (pure field mutators; exposed for native tests).
+/// Disable position broadcast on every PUBLIC/default-PSK channel (precision -> 0); private-PSK
+/// channels (deliberate trusted groups) are left untouched.
+void optInDisablePositionSharing(meshtastic_ChannelFile &cf);
+/// Force all mesh-broadcast device telemetry (and the MQTT map-report location) back to opt-in/off.
+void optInDisableTelemetryBroadcast(meshtastic_LocalModuleConfig &mc);
+
 enum LoadFileResult {
    // Successfully opened the file
    LOAD_SUCCESS = 1,
--- a/test/native-suite-count
+++ b/test/native-suite-count
@@ -1 +1 @@
-28
+30
--- a/test/test_optin_migration/test_main.cpp
+++ b/test/test_optin_migration/test_main.cpp
@@ -0,0 +1,212 @@
+// Unit tests for the 2.8 position/telemetry opt-in migration helpers (NodeDB.cpp / Channels.cpp).
+//
+// Covers:
+//   - channelFileUsesPublicKey(): the public/private-PSK classification the migration keys off.
+//   - optInDisablePositionSharing(): zero precision on PUBLIC channels, preserve PRIVATE channels.
+//   - optInDisableTelemetryBroadcast(): force every mesh-broadcast telemetry flag + map-report location off.
+//
+// The helpers are pure field mutators, so they run against locally-built structs with no filesystem,
+// singleton, or radio setup. The one-time version-gate + saveToDisk() around them lives in
+// NodeDB::loadFromDisk() and is exercised by the hardware/MCP verification, not here.
+
+#include "MeshTypes.h" // Include BEFORE TestUtil.h
+#include "TestUtil.h"
+#include <unity.h>
+
+#include "mesh/Channels.h"
+#include "mesh/NodeDB.h"
+#include <cstring>
+#include <initializer_list>
+
+namespace
+{
+
+// A private 128-bit key that is NOT in the defaultpsk family.
+const uint8_t kPrivate16[16] = {0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB, 0xAB};
+// A private 256-bit key.
+const uint8_t kPrivate32[32] = {0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD,
+                                0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD, 0xCD};
+// The single-byte well-known key index (the stock primary default).
+const uint8_t kSingle1[1] = {1};
+
+meshtastic_Channel makeCh(meshtastic_Channel_Role role, const uint8_t *psk, size_t pskLen, uint32_t precision,
+                          bool hasModuleSettings = true)
+{
+    meshtastic_Channel ch = meshtastic_Channel_init_default;
+    ch.has_settings = true;
+    ch.role = role;
+    ch.settings.psk.size = (pb_size_t)pskLen;
+    if (pskLen)
+        memcpy(ch.settings.psk.bytes, psk, pskLen);
+    ch.settings.has_module_settings = hasModuleSettings;
+    ch.settings.module_settings.position_precision = precision;
+    return ch;
+}
+
+meshtastic_ChannelFile makeFile(std::initializer_list<meshtastic_Channel> chs, uint32_t version = 24)
+{
+    meshtastic_ChannelFile cf = meshtastic_ChannelFile_init_default;
+    cf.version = version;
+    cf.channels_count = 0;
+    for (const meshtastic_Channel &c : chs)
+        cf.channels[cf.channels_count++] = c;
+    return cf;
+}
+
+// ---- channelFileUsesPublicKey ---------------------------------------------------------------------
+
+void test_publicKey_openPrimaryIsPublic()
+{
+    meshtastic_ChannelFile cf = makeFile({makeCh(meshtastic_Channel_Role_PRIMARY, nullptr, 0, 13)});
+    TEST_ASSERT_TRUE(channelFileUsesPublicKey(cf, 0));
+}
+
+void test_publicKey_singleByteIsPublic()
+{
+    meshtastic_ChannelFile cf = makeFile({makeCh(meshtastic_Channel_Role_PRIMARY, kSingle1, 1, 13)});
+    TEST_ASSERT_TRUE(channelFileUsesPublicKey(cf, 0));
+}
+
+void test_publicKey_defaultpskFamilyIsPublic()
+{
+    meshtastic_ChannelFile cf = makeFile({makeCh(meshtastic_Channel_Role_PRIMARY, defaultpsk, sizeof(defaultpsk), 13)});
+    TEST_ASSERT_TRUE(channelFileUsesPublicKey(cf, 0));
+}
+
+void test_publicKey_private16IsPrivate()
+{
+    meshtastic_ChannelFile cf = makeFile({makeCh(meshtastic_Channel_Role_PRIMARY, kPrivate16, 16, 13)});
+    TEST_ASSERT_FALSE(channelFileUsesPublicKey(cf, 0));
+}
+
+void test_publicKey_private32IsPrivate()
+{
+    meshtastic_ChannelFile cf = makeFile({makeCh(meshtastic_Channel_Role_PRIMARY, kPrivate32, 32, 13)});
+    TEST_ASSERT_FALSE(channelFileUsesPublicKey(cf, 0));
+}
+
+void test_publicKey_disabledIsNotPublic()
+{
+    meshtastic_ChannelFile cf = makeFile({makeCh(meshtastic_Channel_Role_DISABLED, kSingle1, 1, 13)});
+    TEST_ASSERT_FALSE(channelFileUsesPublicKey(cf, 0));
+}
+
+void test_publicKey_secondaryInheritsPrivatePrimary()
+{
+    // Secondary with no PSK inherits the PRIMARY's key -> private primary makes it private.
+    meshtastic_ChannelFile cf = makeFile(
+        {makeCh(meshtastic_Channel_Role_PRIMARY, kPrivate16, 16, 0), makeCh(meshtastic_Channel_Role_SECONDARY, nullptr, 0, 13)});
+    TEST_ASSERT_FALSE(channelFileUsesPublicKey(cf, 1));
+}
+
+void test_publicKey_secondaryInheritsPublicPrimary()
+{
+    meshtastic_ChannelFile cf = makeFile(
+        {makeCh(meshtastic_Channel_Role_PRIMARY, kSingle1, 1, 0), makeCh(meshtastic_Channel_Role_SECONDARY, nullptr, 0, 13)});
+    TEST_ASSERT_TRUE(channelFileUsesPublicKey(cf, 1));
+}
+
+// ---- optInDisablePositionSharing ------------------------------------------------------------------
+
+void test_optIn_zeroesPublicPreservesPrivate()
+{
+    meshtastic_ChannelFile cf = makeFile({
+        makeCh(meshtastic_Channel_Role_PRIMARY, kSingle1, 1, 13),      // public default -> 0
+        makeCh(meshtastic_Channel_Role_SECONDARY, kPrivate16, 16, 13), // private -> preserved
+        makeCh(meshtastic_Channel_Role_SECONDARY, kSingle1, 1, 13),    // public secondary -> 0
+    });
+
+    optInDisablePositionSharing(cf);
+
+    TEST_ASSERT_EQUAL_UINT32(0, cf.channels[0].settings.module_settings.position_precision);
+    TEST_ASSERT_EQUAL_UINT32(13, cf.channels[1].settings.module_settings.position_precision); // preserved
+    TEST_ASSERT_EQUAL_UINT32(0, cf.channels[2].settings.module_settings.position_precision);
+}
+
+void test_optIn_zeroesOpenChannel()
+{
+    meshtastic_ChannelFile cf = makeFile({makeCh(meshtastic_Channel_Role_PRIMARY, nullptr, 0, 13)});
+    optInDisablePositionSharing(cf);
+    TEST_ASSERT_EQUAL_UINT32(0, cf.channels[0].settings.module_settings.position_precision);
+}
+
+void test_optIn_secondaryUnderPrivatePreserved()
+{
+    meshtastic_ChannelFile cf = makeFile(
+        {makeCh(meshtastic_Channel_Role_PRIMARY, kPrivate16, 16, 0), makeCh(meshtastic_Channel_Role_SECONDARY, nullptr, 0, 13)});
+    optInDisablePositionSharing(cf);
+    TEST_ASSERT_EQUAL_UINT32(13, cf.channels[1].settings.module_settings.position_precision); // preserved
+}
+
+void test_optIn_secondaryUnderPublicZeroed()
+{
+    meshtastic_ChannelFile cf = makeFile(
+        {makeCh(meshtastic_Channel_Role_PRIMARY, kSingle1, 1, 0), makeCh(meshtastic_Channel_Role_SECONDARY, nullptr, 0, 13)});
+    optInDisablePositionSharing(cf);
+    TEST_ASSERT_EQUAL_UINT32(0, cf.channels[1].settings.module_settings.position_precision);
+}
+
+void test_optIn_setsHasModuleSettingsOnZeroed()
+{
+    // A public channel that never had module_settings still ends up explicitly off (has_module_settings=true).
+    meshtastic_ChannelFile cf = makeFile({makeCh(meshtastic_Channel_Role_PRIMARY, kSingle1, 1, 13, /*hasModule*/ false)});
+    optInDisablePositionSharing(cf);
+    TEST_ASSERT_TRUE(cf.channels[0].settings.has_module_settings);
+    TEST_ASSERT_EQUAL_UINT32(0, cf.channels[0].settings.module_settings.position_precision);
+}
+
+// ---- optInDisableTelemetryBroadcast ---------------------------------------------------------------
+
+void test_optIn_telemetryAllFlagsOff()
+{
+    meshtastic_LocalModuleConfig mc = meshtastic_LocalModuleConfig_init_default;
+    mc.telemetry.device_telemetry_enabled = true;
+    mc.telemetry.environment_measurement_enabled = true;
+    mc.telemetry.air_quality_enabled = true;
+    mc.telemetry.power_measurement_enabled = true;
+    mc.telemetry.health_measurement_enabled = true;
+    mc.mqtt.map_reporting_enabled = true;
+    mc.mqtt.map_report_settings.should_report_location = true;
+
+    optInDisableTelemetryBroadcast(mc);
+
+    TEST_ASSERT_FALSE(mc.telemetry.device_telemetry_enabled);
+    TEST_ASSERT_FALSE(mc.telemetry.environment_measurement_enabled);
+    TEST_ASSERT_FALSE(mc.telemetry.air_quality_enabled);
+    TEST_ASSERT_FALSE(mc.telemetry.power_measurement_enabled);
+    TEST_ASSERT_FALSE(mc.telemetry.health_measurement_enabled);
+    TEST_ASSERT_FALSE(mc.mqtt.map_report_settings.should_report_location);
+    // Anonymous map presence (uplink) is intentionally left untouched.
+    TEST_ASSERT_TRUE(mc.mqtt.map_reporting_enabled);
+}
+
+} // namespace
+
+void setUp(void) {}
+
+void tearDown(void) {}
+
+extern "C" {
+void setup()
+{
+    initializeTestEnvironment();
+    UNITY_BEGIN();
+    RUN_TEST(test_publicKey_openPrimaryIsPublic);
+    RUN_TEST(test_publicKey_singleByteIsPublic);
+    RUN_TEST(test_publicKey_defaultpskFamilyIsPublic);
+    RUN_TEST(test_publicKey_private16IsPrivate);
+    RUN_TEST(test_publicKey_private32IsPrivate);
+    RUN_TEST(test_publicKey_disabledIsNotPublic);
+    RUN_TEST(test_publicKey_secondaryInheritsPrivatePrimary);
+    RUN_TEST(test_publicKey_secondaryInheritsPublicPrimary);
+    RUN_TEST(test_optIn_zeroesPublicPreservesPrivate);
+    RUN_TEST(test_optIn_zeroesOpenChannel);
+    RUN_TEST(test_optIn_secondaryUnderPrivatePreserved);
+    RUN_TEST(test_optIn_secondaryUnderPublicZeroed);
+    RUN_TEST(test_optIn_setsHasModuleSettingsOnZeroed);
+    RUN_TEST(test_optIn_telemetryAllFlagsOff);
+    exit(UNITY_END());
+}
+
+void loop() {}
+}