Skip to content

Conversation

@thebentern
Copy link
Contributor

Closes #8170

Avoid storing user.id as anything but a null terminated empty string and send to client as a generated value

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses security/privacy concerns by preventing user.id values from being stored or transmitted except when sending to the phone. The changes ensure that user.id is always derived from the node number rather than being directly stored or transmitted.

  • Clears user.id fields to empty strings before storage/transmission
  • Re-encodes packets with derived user.id values when sending to phone
  • Adds validation to ensure all loaded nodes have properly derived user.id values

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/modules/NodeInfoModule.cpp Clears user.id before storage and re-encodes with derived ID for phone transmission
src/modules/AdminModule.cpp Prevents user.id from being settable in admin operations
src/mesh/NodeDB.cpp Ensures user.id is always derived from node number in database operations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@thebentern thebentern added the bugfix Pull request that fixes bugs label Oct 4, 2025
@thebentern thebentern merged commit 7c5e2bc into develop Oct 4, 2025
18 of 24 checks passed
@thebentern thebentern deleted the node-user-id branch October 4, 2025 11:54
thebentern added a commit that referenced this pull request Oct 4, 2025
* Create channel-mute toggle function

* Added mute state to channel settings

* Create node-mute toggle functions

* Added mute state to nodedb entries

* Rebase protos

* Decouple protobuf changes

* Disable bell-invoked ext notifs for muted nodes

* Clearly dilineate module mute from sender or channel mute

* Disable bell-invoked ext notifs for muted channels

* Trunk fmt

* Disable message-invoked ext notifs for muted channels and nodes

* Disable on-screen 'new message' popup for muted nodes and channels

* Don't mute alerts

* Make use of pre-existing channel_settings.module_settings.is_client_muted setting

* Revert previous commit - this needs it's own proto

* Regen protos

* T-Lora Pager: Interrupt based rotary encoder

* T-Lora Pager: Fix amplifier fuzzing/popping

* Fix build for other variants

* Fix - reference actual channel when changing settings

* Update protos

* Refactor ref syntax

* Fix defines

* Update comments and remove unused function

* Regen protos

* Regen protobuffs again

* Fix build failure in ci, add missing argument

* Format

* InputPollable: System for polling after interrupts

* T-Lora Pager: Use InputPollable for RotaryEncoderImpl

* Rename RotaryEncoderImpl to TLoraPagerRotaryEncoder

* Revert "Rename RotaryEncoderImpl to TLoraPagerRotaryEncoder"

This reverts commit a76cc88.

* Revert unnecessary ifdefs

* Regen protos

* Use latest protos

* Regen protos for latest changes

* Decouple node-mute from channel-mute

* Regen protos

* Fix desktop build

* More flexible InputPollable paradigm

* Custom xPortInIsrContext() for nRF52/RP2xx0

* Use channel as specified in the received packet

* Use channel as specified in the received packet for OLED screen notifications

* add heltec tracker v2 board.

* Use common power amp definition for Heltec v4 and Heltec Tracker v2

* Set appropriate mqtt root upon lora region change

* Use user preferences root topic if present

* delete SX126X_MAX_POWER=11

* Assume previous root on topic change

* update mqtt root when region is changed via OLED menu handler

* Regen protos

* Removed magic numbers

* Add DIRECT_MSG_ONLY buzzer mode (#8158)

* Handle existing special case for M5STACK_UNITC6L for DIRECT_MSG_ONLY buzz mode

There already exists a special case for M5STACK_UNITC6L.
Modified it to adhere to new DIRECT_MSG_ONLY buzzer mode

* Add new buzzer mode DIRECT_MSG_ONLY to BuzzerModeMenu

* Disable notifications when buzzer mode is DIRECT_MSG_ONLY

* Change alert_message_buzzer in notification module in DIRECT_MSG_ONLY buzz mode

Better comments

* Fixed spelling in debug log

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: nexpspace <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Ben Meadors <[email protected]>

* run trunk fmt

* Update variants/esp32s3/heltec_wireless_tracker_v2/variant.h

Co-authored-by: Copilot <[email protected]>

* Regen protos

* Pull latest protobufs

* Don't use IS_ONE_OF when loading Modules

* GAT562: Use PRIVATE_HW (fix build) (#8198)

* ESP32s2 doesn't implement HWCDC (#8199)

* Fix build script failure under certain conditions for devices that use UF2 binaries  (#8150)

* Validate CR and SF lora config (#8146)

* Validate CR and SF lora config

* No zero-bw

* Update src/modules/AdminModule.cpp

Co-authored-by: Copilot <[email protected]>

* Fix braces

---------

Co-authored-by: Copilot <[email protected]>

* Quote firmware paths given to uf2conv

---------

Co-authored-by: Ben Meadors <[email protected]>
Co-authored-by: Copilot <[email protected]>

* Calculate airtime of transmitted and received packets separately (#8205)

* Correcting GPS PINs (#8087)

#8084

Co-authored-by: Ben Meadors <[email protected]>

* Clear out user.id except for sending to phone (#8202)

* Null out user.id except for sending to phone

* Fix

* Update src/modules/NodeInfoModule.cpp

Co-authored-by: Copilot <[email protected]>

* Copilot garbage

* This is unnecessary, because we don't stored user.id on userlite

* Don't need this

* Fix warning

* Just alter the protobuf

* Alter protobuf doesn't do anything with the altered data, so let's re-encode it

* Check inputbroker before access

---------

Co-authored-by: Copilot <[email protected]>

* Add dropped packet count to LocalStats (#8207)

* Add dropped packet count to LocalStats
In case the transmit queue was full

* Trunked

---------

Co-authored-by: Ben Meadors <[email protected]>

---------

Co-authored-by: ford-jones <[email protected]>
Co-authored-by: WillyJL <[email protected]>
Co-authored-by: Ford Jones <[email protected]>
Co-authored-by: Tom Fifield <[email protected]>
Co-authored-by: Quency-D <[email protected]>
Co-authored-by: Quency-D <[email protected]>
Co-authored-by: nexpspace <[email protected]>
Co-authored-by: nexpspace <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Austin Lane <[email protected]>
Co-authored-by: Ken Piper <[email protected]>
Co-authored-by: GUVWAF <[email protected]>
Co-authored-by: Szetya <[email protected]>
@h3lix1
Copy link
Contributor

h3lix1 commented Oct 6, 2025

#8228 found with this..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Firmware allows the user-id to be set to an arbitrary string

4 participants