diff --git a/app/models.py b/app/models.py index e8c3153..61f0675 100644 --- a/app/models.py +++ b/app/models.py @@ -628,10 +628,22 @@ class RepeaterAdvertIntervalsResponse(BaseModel): class RepeaterOwnerInfoResponse(BaseModel): - """Owner info and guest password from a repeater.""" + """Owner info, firmware, and guest password from a repeater. + + ``owner_info``, ``firmware_version``, and ``name`` come from the + guest-accessible binary owner-info request (REQ_TYPE_GET_OWNER_INFO / 0x07). + ``guest_password`` is admin-only and still comes from the CLI, so guests see + ``None`` for it. + """ owner_info: str | None = Field(default=None, description="Owner info string") - guest_password: str | None = Field(default=None, description="Guest password") + firmware_version: str | None = Field( + default=None, description="Firmware version string (from binary owner-info request)" + ) + name: str | None = Field( + default=None, description="Repeater name (from binary owner-info request)" + ) + guest_password: str | None = Field(default=None, description="Guest password (admin only)") class RepeaterRegionEntry(BaseModel): diff --git a/app/routers/repeaters.py b/app/routers/repeaters.py index a094aad..9d14f61 100644 --- a/app/routers/repeaters.py +++ b/app/routers/repeaters.py @@ -30,6 +30,7 @@ from app.repository import ContactRepository, RepeaterTelemetryRepository from app.routers.contacts import _ensure_on_radio, _resolve_contact_or_404 from app.routers.server_control import ( batch_cli_fetch, + fetch_repeater_owner_info_binary, prepare_authenticated_contact_connection, require_server_capable_contact, send_contact_cli_command, @@ -386,20 +387,32 @@ async def repeater_advert_intervals(public_key: str) -> RepeaterAdvertIntervalsR @router.post("/{public_key}/repeater/owner-info", response_model=RepeaterOwnerInfoResponse) async def repeater_owner_info(public_key: str) -> RepeaterOwnerInfoResponse: - """Fetch owner info and guest password from a repeater via CLI commands.""" + """Fetch owner info, firmware, and guest password from a repeater. + + Owner info + firmware + name come from the guest-accessible binary request + (REQ_TYPE_GET_OWNER_INFO / 0x07), which the firmware serves to any logged-in + client. The guest password is admin-only and still comes from the CLI, so a + guest sees it blank. See issue #306. + """ radio_manager.require_connected() contact = await _resolve_contact_or_404(public_key) _require_repeater(contact) - results = await _batch_cli_fetch( + owner = await fetch_repeater_owner_info_binary(contact) or {} + + # Guest password is admin-only; still fetched via CLI (guests get None). + cli = await _batch_cli_fetch( contact, "repeater_owner_info", - [ - ("get owner.info", "owner_info"), - ("get guest.password", "guest_password"), - ], + [("get guest.password", "guest_password")], + ) + + return RepeaterOwnerInfoResponse( + owner_info=owner.get("owner_info"), + firmware_version=owner.get("firmware_version"), + name=owner.get("name"), + guest_password=cli.get("guest_password"), ) - return RepeaterOwnerInfoResponse(**results) # The firmware's `region` dump is written into a fixed ~160-char buffer diff --git a/app/routers/server_control.py b/app/routers/server_control.py index fdd715d..831da18 100644 --- a/app/routers/server_control.py +++ b/app/routers/server_control.py @@ -1,6 +1,7 @@ import asyncio import logging import time +from enum import Enum from typing import TYPE_CHECKING from fastapi import HTTPException @@ -347,6 +348,105 @@ async def batch_cli_fetch( return results +class _RepeaterBinaryReqType(Enum): + """Binary request types not (yet) wrapped by the installed meshcore library. + + ``REQ_TYPE_GET_OWNER_INFO`` (0x07) was added at repeater ``FIRMWARE_VER_LEVEL >= 2``. + The firmware serves it from ``handleRequest`` with no admin gate, so any + logged-in client — including a guest — can fetch it, unlike the CLI + ``get owner.info`` / ``ver`` path which the firmware only routes for admins. + """ + + OWNER_INFO = 0x07 + + +def _parse_owner_info_payload(data_hex: str) -> dict[str, str | None] | None: + """Parse a REQ_TYPE_GET_OWNER_INFO (0x07) binary response payload. + + The repeater replies with ``"{firmware}\\n{name}\\n{owner_info}"`` (the 4-byte + request tag is already stripped by the library's frame parser, so + ``payload["data"]`` starts at the firmware string). ``owner_info`` may itself + contain newlines, so only the first two separators are split. + """ + if not data_hex: + return None + try: + raw = bytes.fromhex(data_hex) + except ValueError: + return None + text = raw.decode("utf-8", "ignore").strip("\x00") + if not text.strip(): + return None + parts = text.split("\n", 2) + firmware = parts[0].strip() if len(parts) > 0 else "" + name = parts[1].strip() if len(parts) > 1 else "" + owner = parts[2].strip() if len(parts) > 2 else "" + return { + "firmware_version": firmware or None, + "name": name or None, + "owner_info": owner or None, + } + + +async def fetch_repeater_owner_info_binary( + contact: Contact, + *, + operation_name: str = "repeater_owner_info_binary", + timeout: float = 10.0, + min_timeout: float = 5.0, +) -> dict[str, str | None] | None: + """Fetch firmware/name/owner via the guest-accessible binary request (0x07). + + This is the path Liam and other apps use to show owner info + firmware for a + guest: a binary ``REQ_TYPE_GET_OWNER_INFO`` request rather than an admin-only + CLI command. Returns ``None`` when the repeater does not answer — older + firmware (level < 2), not logged in, or out of range — so callers can fall + back or leave the fields blank. See issue #306. + """ + async with radio_manager.radio_operation( + operation_name, pause_polling=True, suspend_auto_fetch=True + ) as mc: + # Ensure contact is on radio for reply routing. + await _ensure_on_radio(mc, contact) + await asyncio.sleep(1.0) # settle after add_contact + + send_result = await mc.commands.send_binary_req( + contact.public_key, + _RepeaterBinaryReqType.OWNER_INFO, + timeout=timeout, + min_timeout=min_timeout, + ) + if send_result.type == EventType.ERROR: + logger.debug("owner-info binary req send error: %s", send_result.payload) + return None + + expected_ack = send_result.payload.get("expected_ack") + if expected_ack is None: + logger.debug("owner-info binary req missing expected_ack: %s", send_result.payload) + return None + exp_tag = expected_ack.hex() + + wait_timeout = ( + timeout if timeout > 0 else send_result.payload.get("suggested_timeout", 4000) / 800 + ) + wait_timeout = max(wait_timeout, min_timeout) + + response = await mc.wait_for_event( + EventType.BINARY_RESPONSE, + attribute_filters={"tag": exp_tag}, + timeout=wait_timeout, + ) + if response is None: + logger.info( + "No owner-info binary response from %s within %.1fs", + contact.public_key[:12], + wait_timeout, + ) + return None + + return _parse_owner_info_payload(response.payload.get("data", "")) + + async def send_contact_cli_command( contact: Contact, command: str, diff --git a/frontend/src/components/repeater/RepeaterOwnerInfoPane.tsx b/frontend/src/components/repeater/RepeaterOwnerInfoPane.tsx index 402c349..5244347 100644 --- a/frontend/src/components/repeater/RepeaterOwnerInfoPane.tsx +++ b/frontend/src/components/repeater/RepeaterOwnerInfoPane.tsx @@ -28,6 +28,8 @@ export function OwnerInfoPane({ ) : (
+ + {data.name && }
)} diff --git a/frontend/src/test/useRepeaterDashboard.test.ts b/frontend/src/test/useRepeaterDashboard.test.ts index b596902..e415d20 100644 --- a/frontend/src/test/useRepeaterDashboard.test.ts +++ b/frontend/src/test/useRepeaterDashboard.test.ts @@ -349,6 +349,8 @@ describe('useRepeaterDashboard', () => { }); mockApi.repeaterOwnerInfo.mockResolvedValueOnce({ owner_info: null, + firmware_version: null, + name: null, guest_password: null, }); mockApi.repeaterLppTelemetry.mockResolvedValueOnce({ sensors: [] }); diff --git a/frontend/src/types.ts b/frontend/src/types.ts index f127dd1..e486a97 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -503,6 +503,8 @@ export interface RepeaterAdvertIntervalsResponse { export interface RepeaterOwnerInfoResponse { owner_info: string | null; + firmware_version: string | null; + name: string | null; guest_password: string | null; } diff --git a/tests/test_repeater_routes.py b/tests/test_repeater_routes.py index 9cff6a8..2bb698a 100644 --- a/tests/test_repeater_routes.py +++ b/tests/test_repeater_routes.py @@ -107,6 +107,9 @@ def _mock_mc(): mc.commands.req_telemetry_sync = AsyncMock() mc.commands.req_regions_sync = AsyncMock(return_value=None) mc.commands.send_cmd = AsyncMock(return_value=_radio_result(EventType.OK)) + mc.commands.send_binary_req = AsyncMock( + return_value=_radio_result(EventType.MSG_SENT, {"expected_ack": b"\xaa\xbb\xcc\xdd"}) + ) mc.commands.get_msg = AsyncMock() mc.commands.add_contact = AsyncMock(return_value=_radio_result(EventType.OK)) mc.commands.send_trace = AsyncMock(return_value=_radio_result(EventType.OK)) @@ -1342,39 +1345,45 @@ class TestRepeaterAdvertIntervals: class TestRepeaterOwnerInfo: @pytest.mark.asyncio async def test_success(self, test_db): + # Owner info + firmware + name come from the guest-accessible binary + # request (0x07); the guest password still comes from the admin CLI. mc = _mock_mc() await _insert_contact(KEY_A, name="Repeater", contact_type=2) - responses = [ - _radio_result( - EventType.CONTACT_MSG_RECV, - { - "pubkey_prefix": KEY_A[:12], - "text": "John Doe - Contact: john@example.com", - "txt_type": 1, - }, - ), - _radio_result( - EventType.CONTACT_MSG_RECV, - {"pubkey_prefix": KEY_A[:12], "text": "guestpw123", "txt_type": 1}, - ), - ] - mc.commands.get_msg = AsyncMock(side_effect=responses) + owner_payload = "v1.15.0\nRepeater One\nJohn Doe - Contact: john@example.com" + mc.wait_for_event = AsyncMock( + return_value=_radio_result( + EventType.BINARY_RESPONSE, {"tag": "aabbccdd", "data": owner_payload.encode().hex()} + ) + ) + mc.commands.get_msg = AsyncMock( + side_effect=[ + _radio_result( + EventType.CONTACT_MSG_RECV, + {"pubkey_prefix": KEY_A[:12], "text": "guestpw123", "txt_type": 1}, + ), + ] + ) with ( patch("app.routers.repeaters.radio_manager.require_connected", return_value=mc), patch.object(radio_manager, "_meshcore", mc), patch(_MONOTONIC, side_effect=_advancing_clock()), + patch("app.routers.server_control.asyncio.sleep", new_callable=AsyncMock), ): response = await repeater_owner_info(KEY_A) assert response.owner_info == "John Doe - Contact: john@example.com" + assert response.firmware_version == "v1.15.0" + assert response.name == "Repeater One" assert response.guest_password == "guestpw123" @pytest.mark.asyncio async def test_timeout_returns_none_fields(self, test_db): + # Older firmware / out of range: no binary response and no CLI response. mc = _mock_mc() await _insert_contact(KEY_A, name="Repeater", contact_type=2) + mc.wait_for_event = AsyncMock(return_value=None) mc.commands.get_msg = AsyncMock(return_value=_radio_result(EventType.NO_MORE_MSGS)) clock_ticks = [] @@ -1391,8 +1400,75 @@ class TestRepeaterOwnerInfo: response = await repeater_owner_info(KEY_A) assert response.owner_info is None + assert response.firmware_version is None + assert response.name is None assert response.guest_password is None + @pytest.mark.asyncio + async def test_binary_req_sends_owner_info_type_and_no_cli_owner_command(self, test_db): + # Regression guard for #306: owner info must NOT go through the admin-only + # CLI 'get owner.info'; it must use the binary 0x07 request instead. + mc = _mock_mc() + await _insert_contact(KEY_A, name="Repeater", contact_type=2) + mc.wait_for_event = AsyncMock( + return_value=_radio_result( + EventType.BINARY_RESPONSE, + {"tag": "aabbccdd", "data": b"v1.15.0\nRpt\nowner".hex()}, + ) + ) + mc.commands.get_msg = AsyncMock(return_value=_radio_result(EventType.NO_MORE_MSGS)) + + with ( + patch("app.routers.repeaters.radio_manager.require_connected", return_value=mc), + patch.object(radio_manager, "_meshcore", mc), + patch(_MONOTONIC, side_effect=_advancing_clock()), + patch("app.routers.server_control.asyncio.sleep", new_callable=AsyncMock), + ): + await repeater_owner_info(KEY_A) + + # Binary request issued with the OWNER_INFO (0x07) request type. + assert mc.commands.send_binary_req.await_count == 1 + req_type_arg = mc.commands.send_binary_req.await_args.args[1] + assert req_type_arg.value == 0x07 + # Only the admin-only guest.password goes over CLI — never 'get owner.info'. + cli_cmds = [call.args[1] for call in mc.commands.send_cmd.await_args_list] + assert "get guest.password" in cli_cmds + assert "get owner.info" not in cli_cmds + + +class TestParseOwnerInfoPayload: + def test_parses_firmware_name_owner(self): + from app.routers.server_control import _parse_owner_info_payload + + result = _parse_owner_info_payload(b"v1.15.0\nMy Repeater\nJane Doe".hex()) + assert result == { + "firmware_version": "v1.15.0", + "name": "My Repeater", + "owner_info": "Jane Doe", + } + + def test_owner_info_keeps_internal_newlines(self): + from app.routers.server_control import _parse_owner_info_payload + + result = _parse_owner_info_payload(b"v1.15.0\nRpt\nline1\nline2".hex()) + assert result is not None + assert result["owner_info"] == "line1\nline2" + + def test_empty_owner_info_is_none(self): + from app.routers.server_control import _parse_owner_info_payload + + result = _parse_owner_info_payload(b"v1.15.0\nRpt\n".hex()) + assert result is not None + assert result["firmware_version"] == "v1.15.0" + assert result["owner_info"] is None + + def test_empty_and_bad_input_returns_none(self): + from app.routers.server_control import _parse_owner_info_payload + + assert _parse_owner_info_payload("") is None + assert _parse_owner_info_payload("nothex!!") is None + assert _parse_owner_info_payload(b"\x00\x00".hex()) is None + class TestParseRegionDump: def test_parses_indented_hierarchy_with_flags(self):