From 23be2d2189ffce46a67f11f76522b277b845630b Mon Sep 17 00:00:00 2001 From: Crank-Git Date: Sun, 8 Jun 2025 17:45:11 -0400 Subject: [PATCH 1/3] Add validation/prevent --set-owner, --set-owner-short, and --set-ham from accepting empty or whitespace-only names. This is in relation to meshtastic#6867 firmware feature request https://github.com/meshtastic/firmware/issues/6867 --- meshtastic/__main__.py | 41 +++++++++++++++++++ meshtastic/node.py | 6 +++ meshtastic/tests/test_main.py | 76 +++++++++++++++++++++++++++++++++++ meshtastic/tests/test_node.py | 74 +++++++++++++++++++++++++++++++++- 4 files changed, 195 insertions(+), 2 deletions(-) diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py index e5a92435..f316ecf3 100644 --- a/meshtastic/__main__.py +++ b/meshtastic/__main__.py @@ -342,6 +342,18 @@ def onConnected(interface): if args.set_owner or args.set_owner_short: closeNow = True waitForAckNak = True + + # Validate owner names before connecting to device + if args.set_owner is not None: + stripped_long_name = args.set_owner.strip() + if not stripped_long_name: + meshtastic.util.our_exit("ERROR: Long Name cannot be empty or contain only whitespace characters") + + if args.set_owner_short is not None: + stripped_short_name = args.set_owner_short.strip() + if not stripped_short_name: + meshtastic.util.our_exit("ERROR: Short Name cannot be empty or contain only whitespace characters") + if args.set_owner and args.set_owner_short: print(f"Setting device owner to {args.set_owner} and short name to {args.set_owner_short}") elif args.set_owner: @@ -644,11 +656,19 @@ def onConnected(interface): interface.getNode(args.dest, False, **getNode_kwargs).beginSettingsTransaction() if "owner" in configuration: + # Validate owner name before setting + owner_name = str(configuration["owner"]).strip() + if not owner_name: + meshtastic.util.our_exit("ERROR: Long Name cannot be empty or contain only whitespace characters") print(f"Setting device owner to {configuration['owner']}") waitForAckNak = True interface.getNode(args.dest, False, **getNode_kwargs).setOwner(configuration["owner"]) if "owner_short" in configuration: + # Validate owner short name before setting + owner_short_name = str(configuration["owner_short"]).strip() + if not owner_short_name: + meshtastic.util.our_exit("ERROR: Short Name cannot be empty or contain only whitespace characters") print( f"Setting device owner short to {configuration['owner_short']}" ) @@ -658,6 +678,10 @@ def onConnected(interface): ) if "ownerShort" in configuration: + # Validate owner short name before setting + owner_short_name = str(configuration["ownerShort"]).strip() + if not owner_short_name: + meshtastic.util.our_exit("ERROR: Short Name cannot be empty or contain only whitespace characters") print( f"Setting device owner short to {configuration['ownerShort']}" ) @@ -1088,6 +1112,7 @@ def export_config(interface) -> str: configObj["location"]["alt"] = alt config = MessageToDict(interface.localNode.localConfig) #checkme - Used as a dictionary here and a string below + #was used as a string here and a Dictionary above if config: # Convert inner keys to correct snake/camelCase prefs = {} @@ -1182,6 +1207,22 @@ def common(): meshtastic.util.support_info() meshtastic.util.our_exit("", 0) + # Early validation for owner names before attempting device connection + if hasattr(args, 'set_owner') and args.set_owner is not None: + stripped_long_name = args.set_owner.strip() + if not stripped_long_name: + meshtastic.util.our_exit("ERROR: Long Name cannot be empty or contain only whitespace characters") + + if hasattr(args, 'set_owner_short') and args.set_owner_short is not None: + stripped_short_name = args.set_owner_short.strip() + if not stripped_short_name: + meshtastic.util.our_exit("ERROR: Short Name cannot be empty or contain only whitespace characters") + + if hasattr(args, 'set_ham') and args.set_ham is not None: + stripped_ham_name = args.set_ham.strip() + if not stripped_ham_name: + meshtastic.util.our_exit("ERROR: Ham ID cannot be empty or contain only whitespace characters") + if have_powermon: create_power_meter() diff --git a/meshtastic/node.py b/meshtastic/node.py index e54963c1..d006075f 100644 --- a/meshtastic/node.py +++ b/meshtastic/node.py @@ -307,10 +307,16 @@ def setOwner(self, long_name: Optional[str]=None, short_name: Optional[str]=None nChars = 4 if long_name is not None: long_name = long_name.strip() + # Validate that long_name is not empty or whitespace-only + if not long_name: + our_exit("ERROR: Long Name cannot be empty or contain only whitespace characters") p.set_owner.long_name = long_name p.set_owner.is_licensed = is_licensed if short_name is not None: short_name = short_name.strip() + # Validate that short_name is not empty or whitespace-only + if not short_name: + our_exit("ERROR: Short Name cannot be empty or contain only whitespace characters") if len(short_name) > nChars: short_name = short_name[:nChars] print(f"Maximum is 4 characters, truncated to {short_name}") diff --git a/meshtastic/tests/test_main.py b/meshtastic/tests/test_main.py index 8b70a4e8..1d4040c8 100644 --- a/meshtastic/tests/test_main.py +++ b/meshtastic/tests/test_main.py @@ -2713,3 +2713,79 @@ def test_remove_ignored_node(): main() mocked_node.removeIgnored.assert_called_once_with("!12345678") + +@pytest.mark.unit +@pytest.mark.usefixtures("reset_mt_config") +def test_main_set_owner_short_to_bob(capsys): + """Test --set-owner-short bob""" + sys.argv = ["", "--set-owner-short", "bob"] + mt_config.args = sys.argv + + iface = MagicMock(autospec=SerialInterface) + with patch("meshtastic.serial_interface.SerialInterface", return_value=iface) as mo: + main() + out, err = capsys.readouterr() + assert re.search(r"Connected to radio", out, re.MULTILINE) + assert re.search(r"Setting device owner short to bob", out, re.MULTILINE) + assert err == "" + mo.assert_called() + + +@pytest.mark.unit +@pytest.mark.usefixtures("reset_mt_config") +def test_main_set_owner_whitespace_only(capsys): + """Test --set-owner with whitespace-only name""" + sys.argv = ["", "--set-owner", " "] + mt_config.args = sys.argv + + with pytest.raises(SystemExit) as excinfo: + main() + + out, err = capsys.readouterr() + assert "ERROR: Long Name cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 + + +@pytest.mark.unit +@pytest.mark.usefixtures("reset_mt_config") +def test_main_set_owner_empty_string(capsys): + """Test --set-owner with empty string""" + sys.argv = ["", "--set-owner", ""] + mt_config.args = sys.argv + + with pytest.raises(SystemExit) as excinfo: + main() + + out, err = capsys.readouterr() + assert "ERROR: Long Name cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 + + +@pytest.mark.unit +@pytest.mark.usefixtures("reset_mt_config") +def test_main_set_owner_short_whitespace_only(capsys): + """Test --set-owner-short with whitespace-only name""" + sys.argv = ["", "--set-owner-short", " "] + mt_config.args = sys.argv + + with pytest.raises(SystemExit) as excinfo: + main() + + out, err = capsys.readouterr() + assert "ERROR: Short Name cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 + + +@pytest.mark.unit +@pytest.mark.usefixtures("reset_mt_config") +def test_main_set_owner_short_empty_string(capsys): + """Test --set-owner-short with empty string""" + sys.argv = ["", "--set-owner-short", ""] + mt_config.args = sys.argv + + with pytest.raises(SystemExit) as excinfo: + main() + + out, err = capsys.readouterr() + assert "ERROR: Short Name cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 diff --git a/meshtastic/tests/test_node.py b/meshtastic/tests/test_node.py index ffdd8ea7..b1b235ce 100644 --- a/meshtastic/tests/test_node.py +++ b/meshtastic/tests/test_node.py @@ -1254,8 +1254,7 @@ def test_requestChannels_non_localNode_starting_index(caplog): # }, # 'id': 1692918436, # 'hopLimit': 3, -# 'priority': -# 'RELIABLE', +# 'priority': 'RELIABLE', # 'raw': 'fake', # 'fromId': '!9388f81c', # 'toId': '!9388f81c' @@ -1480,6 +1479,77 @@ def test_remove_ignored(ignored): iface.sendData.assert_called_once() +@pytest.mark.unit +def test_setOwner_whitespace_only_long_name(capsys): + """Test setOwner with whitespace-only long name""" + iface = MagicMock(autospec=MeshInterface) + anode = Node(iface, 123, noProto=True) + + with pytest.raises(SystemExit) as excinfo: + anode.setOwner(long_name=" ") + + out, err = capsys.readouterr() + assert "ERROR: Long Name cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 + + +@pytest.mark.unit +def test_setOwner_empty_long_name(capsys): + """Test setOwner with empty long name""" + iface = MagicMock(autospec=MeshInterface) + anode = Node(iface, 123, noProto=True) + + with pytest.raises(SystemExit) as excinfo: + anode.setOwner(long_name="") + + out, err = capsys.readouterr() + assert "ERROR: Long Name cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 + + +@pytest.mark.unit +def test_setOwner_whitespace_only_short_name(capsys): + """Test setOwner with whitespace-only short name""" + iface = MagicMock(autospec=MeshInterface) + anode = Node(iface, 123, noProto=True) + + with pytest.raises(SystemExit) as excinfo: + anode.setOwner(short_name=" ") + + out, err = capsys.readouterr() + assert "ERROR: Short Name cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 + + +@pytest.mark.unit +def test_setOwner_empty_short_name(capsys): + """Test setOwner with empty short name""" + iface = MagicMock(autospec=MeshInterface) + anode = Node(iface, 123, noProto=True) + + with pytest.raises(SystemExit) as excinfo: + anode.setOwner(short_name="") + + out, err = capsys.readouterr() + assert "ERROR: Short Name cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 + + +@pytest.mark.unit +def test_setOwner_valid_names(caplog): + """Test setOwner with valid names""" + iface = MagicMock(autospec=MeshInterface) + anode = Node(iface, 123, noProto=True) + + with caplog.at_level(logging.DEBUG): + anode.setOwner(long_name="ValidName", short_name="VN") + + # Should not raise any exceptions and should call _sendAdmin + iface._sendAdmin.assert_called_once() + assert re.search(r'p.set_owner.long_name:ValidName:', caplog.text, re.MULTILINE) + assert re.search(r'p.set_owner.short_name:VN:', caplog.text, re.MULTILINE) + + # TODO # @pytest.mark.unitslow # def test_waitForConfig(): From aa786c7ebdc8f300db60c06e6538f50075383c64 Mon Sep 17 00:00:00 2001 From: Crank-Git Date: Sun, 8 Jun 2025 19:51:21 -0400 Subject: [PATCH 2/3] Fix linting errors and duplicate function --- meshtastic/tests/test_main.py | 64 +++++++++++++++++++++-------------- meshtastic/tests/test_node.py | 20 +++++------ 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/meshtastic/tests/test_main.py b/meshtastic/tests/test_main.py index 1d4040c8..235829d4 100644 --- a/meshtastic/tests/test_main.py +++ b/meshtastic/tests/test_main.py @@ -2713,24 +2713,6 @@ def test_remove_ignored_node(): main() mocked_node.removeIgnored.assert_called_once_with("!12345678") - -@pytest.mark.unit -@pytest.mark.usefixtures("reset_mt_config") -def test_main_set_owner_short_to_bob(capsys): - """Test --set-owner-short bob""" - sys.argv = ["", "--set-owner-short", "bob"] - mt_config.args = sys.argv - - iface = MagicMock(autospec=SerialInterface) - with patch("meshtastic.serial_interface.SerialInterface", return_value=iface) as mo: - main() - out, err = capsys.readouterr() - assert re.search(r"Connected to radio", out, re.MULTILINE) - assert re.search(r"Setting device owner short to bob", out, re.MULTILINE) - assert err == "" - mo.assert_called() - - @pytest.mark.unit @pytest.mark.usefixtures("reset_mt_config") def test_main_set_owner_whitespace_only(capsys): @@ -2740,8 +2722,8 @@ def test_main_set_owner_whitespace_only(capsys): with pytest.raises(SystemExit) as excinfo: main() - - out, err = capsys.readouterr() + + out, _ = capsys.readouterr() assert "ERROR: Long Name cannot be empty or contain only whitespace characters" in out assert excinfo.value.code == 1 @@ -2755,8 +2737,8 @@ def test_main_set_owner_empty_string(capsys): with pytest.raises(SystemExit) as excinfo: main() - - out, err = capsys.readouterr() + + out, _ = capsys.readouterr() assert "ERROR: Long Name cannot be empty or contain only whitespace characters" in out assert excinfo.value.code == 1 @@ -2770,8 +2752,8 @@ def test_main_set_owner_short_whitespace_only(capsys): with pytest.raises(SystemExit) as excinfo: main() - - out, err = capsys.readouterr() + + out, _ = capsys.readouterr() assert "ERROR: Short Name cannot be empty or contain only whitespace characters" in out assert excinfo.value.code == 1 @@ -2785,7 +2767,37 @@ def test_main_set_owner_short_empty_string(capsys): with pytest.raises(SystemExit) as excinfo: main() - - out, err = capsys.readouterr() + + out, _ = capsys.readouterr() assert "ERROR: Short Name cannot be empty or contain only whitespace characters" in out assert excinfo.value.code == 1 + + +@pytest.mark.unit +@pytest.mark.usefixtures("reset_mt_config") +def test_main_set_ham_whitespace_only(capsys): + """Test --set-ham with whitespace-only name""" + sys.argv = ["", "--set-ham", " "] + mt_config.args = sys.argv + + with pytest.raises(SystemExit) as excinfo: + main() + + out, _ = capsys.readouterr() + assert "ERROR: Ham radio callsign cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 + + +@pytest.mark.unit +@pytest.mark.usefixtures("reset_mt_config") +def test_main_set_ham_empty_string(capsys): + """Test --set-ham with empty string""" + sys.argv = ["", "--set-ham", ""] + mt_config.args = sys.argv + + with pytest.raises(SystemExit) as excinfo: + main() + + out, _ = capsys.readouterr() + assert "ERROR: Ham radio callsign cannot be empty or contain only whitespace characters" in out + assert excinfo.value.code == 1 diff --git a/meshtastic/tests/test_node.py b/meshtastic/tests/test_node.py index b1b235ce..bb4e2d96 100644 --- a/meshtastic/tests/test_node.py +++ b/meshtastic/tests/test_node.py @@ -1487,8 +1487,8 @@ def test_setOwner_whitespace_only_long_name(capsys): with pytest.raises(SystemExit) as excinfo: anode.setOwner(long_name=" ") - - out, err = capsys.readouterr() + + out, _ = capsys.readouterr() assert "ERROR: Long Name cannot be empty or contain only whitespace characters" in out assert excinfo.value.code == 1 @@ -1501,8 +1501,8 @@ def test_setOwner_empty_long_name(capsys): with pytest.raises(SystemExit) as excinfo: anode.setOwner(long_name="") - - out, err = capsys.readouterr() + + out, _ = capsys.readouterr() assert "ERROR: Long Name cannot be empty or contain only whitespace characters" in out assert excinfo.value.code == 1 @@ -1515,8 +1515,8 @@ def test_setOwner_whitespace_only_short_name(capsys): with pytest.raises(SystemExit) as excinfo: anode.setOwner(short_name=" ") - - out, err = capsys.readouterr() + + out, _ = capsys.readouterr() assert "ERROR: Short Name cannot be empty or contain only whitespace characters" in out assert excinfo.value.code == 1 @@ -1529,8 +1529,8 @@ def test_setOwner_empty_short_name(capsys): with pytest.raises(SystemExit) as excinfo: anode.setOwner(short_name="") - - out, err = capsys.readouterr() + + out, _ = capsys.readouterr() assert "ERROR: Short Name cannot be empty or contain only whitespace characters" in out assert excinfo.value.code == 1 @@ -1540,10 +1540,10 @@ def test_setOwner_valid_names(caplog): """Test setOwner with valid names""" iface = MagicMock(autospec=MeshInterface) anode = Node(iface, 123, noProto=True) - + with caplog.at_level(logging.DEBUG): anode.setOwner(long_name="ValidName", short_name="VN") - + # Should not raise any exceptions and should call _sendAdmin iface._sendAdmin.assert_called_once() assert re.search(r'p.set_owner.long_name:ValidName:', caplog.text, re.MULTILINE) From 8a6ee5fb3540da8198661e0629af45ad58e215aa Mon Sep 17 00:00:00 2001 From: Crank-Git Date: Sun, 8 Jun 2025 20:39:26 -0400 Subject: [PATCH 3/3] fixed assertion errors by replace ham id with ham radio callsign, fixed _sendAdmin assertion --- meshtastic/__main__.py | 6 ++++-- meshtastic/tests/test_node.py | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/meshtastic/__main__.py b/meshtastic/__main__.py index f316ecf3..56dd29c0 100644 --- a/meshtastic/__main__.py +++ b/meshtastic/__main__.py @@ -349,7 +349,7 @@ def onConnected(interface): if not stripped_long_name: meshtastic.util.our_exit("ERROR: Long Name cannot be empty or contain only whitespace characters") - if args.set_owner_short is not None: + if hasattr(args, 'set_owner_short') and args.set_owner_short is not None: stripped_short_name = args.set_owner_short.strip() if not stripped_short_name: meshtastic.util.our_exit("ERROR: Short Name cannot be empty or contain only whitespace characters") @@ -414,6 +414,8 @@ def onConnected(interface): print(" ".join(fieldNames)) if args.set_ham: + if not args.set_ham.strip(): + meshtastic.util.our_exit("ERROR: Ham radio callsign cannot be empty or contain only whitespace characters") closeNow = True print(f"Setting Ham ID to {args.set_ham} and turning off encryption") interface.getNode(args.dest, **getNode_kwargs).setOwner(args.set_ham, is_licensed=True) @@ -1221,7 +1223,7 @@ def common(): if hasattr(args, 'set_ham') and args.set_ham is not None: stripped_ham_name = args.set_ham.strip() if not stripped_ham_name: - meshtastic.util.our_exit("ERROR: Ham ID cannot be empty or contain only whitespace characters") + meshtastic.util.our_exit("ERROR: Ham radio callsign cannot be empty or contain only whitespace characters") if have_powermon: create_power_meter() diff --git a/meshtastic/tests/test_node.py b/meshtastic/tests/test_node.py index bb4e2d96..c5cb6b3f 100644 --- a/meshtastic/tests/test_node.py +++ b/meshtastic/tests/test_node.py @@ -1544,8 +1544,8 @@ def test_setOwner_valid_names(caplog): with caplog.at_level(logging.DEBUG): anode.setOwner(long_name="ValidName", short_name="VN") - # Should not raise any exceptions and should call _sendAdmin - iface._sendAdmin.assert_called_once() + # Should not raise any exceptions + # Note: When noProto=True, _sendAdmin is not called as the method returns early assert re.search(r'p.set_owner.long_name:ValidName:', caplog.text, re.MULTILINE) assert re.search(r'p.set_owner.short_name:VN:', caplog.text, re.MULTILINE)