Merge pull request #782 from Crank-Git/validate-short-and-long-names

Add validation/prevent --set-owner, --set-owner-short, and --set-ham from accepting empty or whitespace-only names

Author: ianmcorvidae

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 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 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:
@@ -402,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)
@@ -644,12 +658,20 @@ 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"])
                     time.sleep(0.5)
 
                 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']}"
                     )
@@ -660,6 +682,10 @@ def onConnected(interface):
                     time.sleep(0.5)
 
                 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']}"
                     )
@@ -1096,6 +1122,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 = {}
@@ -1190,6 +1217,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 radio callsign cannot be empty or contain only whitespace characters")
+
         if have_powermon:
             create_power_meter()
 

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}")

meshtastic/tests/test_main.py

@@ -2713,3 +2713,91 @@ 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_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, _ = 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, _ = 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, _ = 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, _ = 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

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, _ = 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, _ = 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, _ = 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, _ = 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
+    # 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)
+
+
 # TODO
 # @pytest.mark.unitslow
 # def test_waitForConfig():