Skip to content

Add validation/prevent --set-owner, --set-owner-short, and --set-ham … #782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions meshtastic/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,18 @@
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")

Check warning on line 350 in meshtastic/__main__.py

View check run for this annotation

Codecov / codecov/patch

meshtastic/__main__.py#L350

Added line #L350 was not covered by tests

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

Check warning on line 355 in meshtastic/__main__.py

View check run for this annotation

Codecov / codecov/patch

meshtastic/__main__.py#L355

Added line #L355 was not covered by tests

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:
Expand Down Expand Up @@ -402,6 +414,8 @@
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")

Check warning on line 418 in meshtastic/__main__.py

View check run for this annotation

Codecov / codecov/patch

meshtastic/__main__.py#L418

Added line #L418 was not covered by tests
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)
Expand Down Expand Up @@ -644,11 +658,19 @@
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")

Check warning on line 664 in meshtastic/__main__.py

View check run for this annotation

Codecov / codecov/patch

meshtastic/__main__.py#L662-L664

Added lines #L662 - L664 were not covered by tests
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")

Check warning on line 673 in meshtastic/__main__.py

View check run for this annotation

Codecov / codecov/patch

meshtastic/__main__.py#L671-L673

Added lines #L671 - L673 were not covered by tests
print(
f"Setting device owner short to {configuration['owner_short']}"
)
Expand All @@ -658,6 +680,10 @@
)

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

Check warning on line 686 in meshtastic/__main__.py

View check run for this annotation

Codecov / codecov/patch

meshtastic/__main__.py#L684-L686

Added lines #L684 - L686 were not covered by tests
print(
f"Setting device owner short to {configuration['ownerShort']}"
)
Expand Down Expand Up @@ -1088,6 +1114,7 @@
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 = {}
Expand Down Expand Up @@ -1182,6 +1209,22 @@
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()

Expand Down
6 changes: 6 additions & 0 deletions meshtastic/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
88 changes: 88 additions & 0 deletions meshtastic/tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
74 changes: 72 additions & 2 deletions meshtastic/tests/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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():
Expand Down
Loading