diff --git a/repeater/config_manager.py b/repeater/config_manager.py index 58c3494..d919dfb 100644 --- a/repeater/config_manager.py +++ b/repeater/config_manager.py @@ -316,6 +316,14 @@ class ConfigManager: self.daemon.repeater_handler.reload_runtime_config() logger.info("Reloaded RepeaterHandler runtime config") + # Re-apply login security when the repeater section changed; the + # repeater ACL captures repeater.security at registration, so a + # saved password change must be pushed to the live ACL explicitly. + if "repeater" in sections and self.daemon: + login_helper = getattr(self.daemon, "login_helper", None) + if login_helper is not None and hasattr(login_helper, "refresh_repeater_security"): + login_helper.refresh_repeater_security(daemon_config) + # Also reload advert_helper config if repeater section changed if self.daemon and hasattr(self.daemon, "advert_helper") and self.daemon.advert_helper: if "repeater" in sections: diff --git a/repeater/handler_helpers/login.py b/repeater/handler_helpers/login.py index 552f1d8..63e90e5 100644 --- a/repeater/handler_helpers/login.py +++ b/repeater/handler_helpers/login.py @@ -33,6 +33,9 @@ class LoginHelper: self.handlers = {} self.acls = {} # Per-identity ACLs keyed by hash_byte + # The repeater identity's ACL, kept so live config updates can re-apply + # repeater.security without re-registering the identity. + self._repeater_acl = None self._pending_tasks = set() # Shared across all identities so the node's total anon-reply rate is # bounded (mirrors firmware anon_limiter: ~4 requests / 2 min). @@ -116,6 +119,8 @@ class LoginHelper: ) self.acls[hash_byte] = identity_acl + if identity_type != "room_server": + self._repeater_acl = identity_acl logger.info(f"Created ACL for {identity_type} '{name}': hash=0x{hash_byte:02X}") # Create auth callback that uses this identity's ACL @@ -180,6 +185,34 @@ class LoginHelper: logger.info(f"Registered {identity_type} '{name}' login handler: hash=0x{hash_byte:02X}") + def refresh_repeater_security(self, config: dict = None) -> bool: + """Re-apply ``repeater.security`` from config to the live repeater ACL. + + The ACL captures its passwords at registration, so without this a saved + password change would only take effect after a restart. Room-server ACLs + keep their per-identity ``settings`` passwords and are not touched. + """ + acl = self._repeater_acl + if acl is None: + return False + + cfg = config if isinstance(config, dict) else self.config + security = (cfg or {}).get("repeater", {}).get("security", {}) or {} + + acl.admin_password = security.get("admin_password") or "" + acl.guest_password = security.get("guest_password") or "" + acl.allow_read_only = bool(security.get("allow_read_only", False)) + try: + acl.max_clients = int(security.get("max_clients", acl.max_clients)) + except (TypeError, ValueError): + logger.warning( + "Ignoring invalid repeater.security.max_clients=%r during security refresh", + security.get("max_clients"), + ) + + logger.info("Refreshed repeater ACL security settings from config") + return True + def _format_region_names(self) -> str: """Build the comma-separated region-names string for an anon regions reply. diff --git a/repeater/handler_helpers/mesh_cli.py b/repeater/handler_helpers/mesh_cli.py index cc0dff3..40b7ee2 100644 --- a/repeater/handler_helpers/mesh_cli.py +++ b/repeater/handler_helpers/mesh_cli.py @@ -39,6 +39,28 @@ class MeshCLI: self.repeater_config = config.get("repeater", {}) self.mesh_config = config.setdefault("mesh", {}) + def _save_config_and_apply(self, sections=None) -> bool: + """Persist the config, then live-apply the changed sections. + + ``ConfigManager.save_to_file`` returns a bare bool; the tuple form is + tolerated for older manager doubles. Live update only runs after a + successful save so a failed write never half-applies a change. Pass + no sections to stage a change: saved to disk, applied on restart + (radio parameters, matching firmware's reboot-to-apply). + """ + result = self.config_manager.save_to_file() + saved = result[0] if isinstance(result, tuple) else bool(result) + if not saved: + return False + if sections: + self.config_manager.live_update_daemon(sections) + return True + + def _get_security_config(self) -> Dict[str, Any]: + """Return the repeater login security section (the one LoginHelper reads).""" + security = self.repeater_config.get("security") + return security if isinstance(security, dict) else {} + def _get_node_name(self) -> str: """Return the configured node name, preferring the newer key when present.""" return self.repeater_config.get("node_name") or self.repeater_config.get("name", "Unknown") @@ -332,12 +354,12 @@ class MeshCLI: " set allow.read.only on|off Read-only access\n" " set advert.interval 60-240 minutes\n" " set flood.advert.interval
3-168 hours\n" - " set flood.max Max flood hops (max 64)\n" + " set flood.max Max flood hops (max 64; 0 = unlimited, unlike firmware)\n" " set path.hash.mode <0-2> Path hash mode (0=1B,1=2B,2=3B)\n" " set loop.detect Flood loop detection\n" - " set rxdelay RX delay base (>=0)\n" - " set txdelay TX delay factor (>=0)\n" - " set direct.txdelay Direct TX delay (>=0)\n" + " set rxdelay RX delay base (0-20)\n" + " set txdelay TX delay factor (0-2)\n" + " set direct.txdelay Direct TX delay (0-2)\n" " set multi.acks Multi-ack count\n" " set int.thresh Interference threshold\n" " set agc.reset.interval AGC reset (rounded to x4)" @@ -447,19 +469,14 @@ class MeshCLI: if not new_password: return "Error: Password cannot be empty" - # Update security config - if "security" not in self.config: - self.config["security"] = {} - - self.config["security"]["password"] = new_password + # LoginHelper authenticates from repeater.security.admin_password. + self.repeater_config.setdefault("security", {})["admin_password"] = new_password # Save config and live update try: - saved, err = self.config_manager.save_to_file() - if not saved: - logger.error(f"Failed to save password: {err}") - return f"Error: Failed to save config: {err}" - self.config_manager.live_update_daemon(["security"]) + if not self._save_config_and_apply(["repeater"]): + logger.error("Failed to save password: config save failed") + return "Error: Failed to save config" return f"password now: {new_password}" except Exception as e: logger.error(f"Failed to save password: {e}") @@ -538,7 +555,7 @@ class MeshCLI: return f"> {role}" elif param == "guest.password": - guest_pw = self.config.get("security", {}).get("guest_password", "") + guest_pw = self._get_security_config().get("guest_password") or "" return f"> {guest_pw}" elif param == "owner.info": @@ -546,7 +563,7 @@ class MeshCLI: return f"> {owner_info}" elif param == "allow.read.only": - allow = self.config.get("security", {}).get("allow_read_only", False) + allow = self._get_security_config().get("allow_read_only", False) return f"> {'on' if allow else 'off'}" elif param == "advert.interval": @@ -554,7 +571,8 @@ class MeshCLI: return f"> {interval}" elif param == "flood.advert.interval": - interval = self.repeater_config.get("flood_advert_interval_hours", 24) + # The engine's flood-advert timer consumes send_advert_interval_hours. + interval = self.repeater_config.get("send_advert_interval_hours", 10) return f"> {interval}" elif param == "flood.max": @@ -615,87 +633,95 @@ class MeshCLI: try: if key == "af": self.repeater_config["airtime_factor"] = float(value) - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "name": self._set_node_name(value) - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "repeat": self.repeater_config["mode"] = "forward" if value.lower() == "on" else "monitor" - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return f"OK - repeat is now {'ON' if self.repeater_config['mode'] == 'forward' else 'OFF'}" elif key == "lat": self.repeater_config["latitude"] = float(value) - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "lon": self.repeater_config["longitude"] = float(value) - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "radio": - # Format: freq bw sf cr + # Format: freq(MHz) bw(kHz) sf cr — the config stores Hz. radio_parts = value.split() if len(radio_parts) != 4: return "Error: Expected freq bw sf cr" - if "radio" not in self.config: - self.config["radio"] = {} + freq_mhz = float(radio_parts[0]) + bw_khz = float(radio_parts[1]) + sf = int(radio_parts[2]) + cr = int(radio_parts[3]) + if not ( + 150.0 <= freq_mhz <= 2500.0 + and 7.0 <= bw_khz <= 500.0 + and 5 <= sf <= 12 + and 5 <= cr <= 8 + ): + return "Error, invalid radio params" - self.config["radio"]["frequency"] = float(radio_parts[0]) - self.config["radio"]["bandwidth"] = float(radio_parts[1]) - self.config["radio"]["spreading_factor"] = int(radio_parts[2]) - self.config["radio"]["coding_rate"] = int(radio_parts[3]) - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["radio"]) + radio_config = self.config.setdefault("radio", {}) + radio_config["frequency"] = int(round(freq_mhz * 1_000_000)) + radio_config["bandwidth"] = int(round(bw_khz * 1_000)) + radio_config["spreading_factor"] = sf + radio_config["coding_rate"] = cr + if not self._save_config_and_apply(): + return "Error: Failed to save config" return "OK - restart repeater to apply" elif key == "freq": - if "radio" not in self.config: - self.config["radio"] = {} - self.config["radio"]["frequency"] = float(value) - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["radio"]) + # CLI input is MHz (firmware parity); the config stores Hz. + freq_mhz = float(value) + self.config.setdefault("radio", {})["frequency"] = int(round(freq_mhz * 1_000_000)) + if not self._save_config_and_apply(): + return "Error: Failed to save config" return "OK - restart repeater to apply" elif key == "tx": - if "radio" not in self.config: - self.config["radio"] = {} - self.config["radio"]["tx_power"] = int(value) - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["radio"]) - return "OK" + self.config.setdefault("radio", {})["tx_power"] = int(value) + if not self._save_config_and_apply(): + return "Error: Failed to save config" + return "OK - restart repeater to apply" elif key == "guest.password": - if "security" not in self.config: - self.config["security"] = {} - self.config["security"]["guest_password"] = value - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["security"]) + # LoginHelper authenticates from repeater.security.guest_password. + self.repeater_config.setdefault("security", {})["guest_password"] = value + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "owner.info": self.repeater_config["owner_info"] = value.replace("|", "\n") - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "allow.read.only": - if "security" not in self.config: - self.config["security"] = {} - self.config["security"]["allow_read_only"] = value.lower() == "on" - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["security"]) + # LoginHelper reads repeater.security.allow_read_only. + self.repeater_config.setdefault("security", {})["allow_read_only"] = ( + value.lower() == "on" + ) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "advert.interval": @@ -703,17 +729,18 @@ class MeshCLI: if mins > 0 and (mins < 60 or mins > 240): return "Error: interval range is 60-240 minutes" self.repeater_config["advert_interval_minutes"] = mins - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "flood.advert.interval": hours = int(value) if (hours > 0 and hours < 3) or hours > 168: return "Error: interval range is 3-168 hours" - self.repeater_config["flood_advert_interval_hours"] = hours - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + # The engine's flood-advert timer consumes send_advert_interval_hours. + self.repeater_config["send_advert_interval_hours"] = hours + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "flood.max": @@ -721,8 +748,8 @@ class MeshCLI: if max_val > 64: return "Error: max 64" self.repeater_config["max_flood_hops"] = max_val - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "path.hash.mode": @@ -730,8 +757,8 @@ class MeshCLI: if mode not in (0, 1, 2): return "Error: path.hash.mode must be 0, 1, or 2" self.mesh_config["path_hash_mode"] = mode - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["mesh"]) + if not self._save_config_and_apply(["mesh"]): + return "Error: Failed to save config" return "OK" elif key == "loop.detect": @@ -739,47 +766,47 @@ class MeshCLI: if mode not in ("off", "minimal", "moderate", "strict"): return "Error: loop.detect must be off, minimal, moderate, or strict" self.mesh_config["loop_detect"] = mode - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["mesh"]) + if not self._save_config_and_apply(["mesh"]): + return "Error: Failed to save config" return "OK" elif key == "rxdelay": delay = float(value) - if delay < 0: - return "Error: cannot be negative" + if delay < 0 or delay > 20.0: + return "Error, must be 0-20" self.config.setdefault("delays", {})["rx_delay_base"] = delay - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater", "delays"]) + if not self._save_config_and_apply(["repeater", "delays"]): + return "Error: Failed to save config" return "OK" elif key == "txdelay": delay = float(value) - if delay < 0: - return "Error: cannot be negative" + if delay < 0 or delay > 2.0: + return "Error, must be 0-2" self.config.setdefault("delays", {})["tx_delay_factor"] = delay - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater", "delays"]) + if not self._save_config_and_apply(["repeater", "delays"]): + return "Error: Failed to save config" return "OK" elif key == "direct.txdelay": delay = float(value) - if delay < 0: - return "Error: cannot be negative" + if delay < 0 or delay > 2.0: + return "Error, must be 0-2" self.config.setdefault("delays", {})["direct_tx_delay_factor"] = delay - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater", "delays"]) + if not self._save_config_and_apply(["repeater", "delays"]): + return "Error: Failed to save config" return "OK" elif key == "multi.acks": self.repeater_config["multi_acks"] = int(value) - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "int.thresh": self.repeater_config["interference_threshold"] = int(value) - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return "OK" elif key == "agc.reset.interval": @@ -787,8 +814,8 @@ class MeshCLI: # Round to nearest multiple of 4 rounded = (interval // 4) * 4 self.repeater_config["agc_reset_interval"] = rounded - saved, _ = self.config_manager.save_to_file() - self.config_manager.live_update_daemon(["repeater"]) + if not self._save_config_and_apply(["repeater"]): + return "Error: Failed to save config" return f"OK - interval rounded to {rounded}" else: diff --git a/repeater/handler_helpers/trace.py b/repeater/handler_helpers/trace.py index d90a4c4..c1acf40 100644 --- a/repeater/handler_helpers/trace.py +++ b/repeater/handler_helpers/trace.py @@ -135,11 +135,16 @@ class TraceHelper: logger.info(f"Path SNRs: [{', '.join(path_snrs)}], Hashes: [{', '.join(path_hashes)}]") should_forward = self._should_forward_trace(packet, trace_bytes, flags, hash_width) + mode_blocked = should_forward and not self._relay_allowed() + if mode_blocked: + should_forward = False + logger.info("Trace relay suppressed (repeat disabled)") if should_forward: await self._forward_trace_packet(packet, num_hops) else: - self._log_no_forward_reason(packet, trace_bytes, hash_width) + if not mode_blocked: + self._log_no_forward_reason(packet, trace_bytes, hash_width) if ( self.on_trace_complete and self._is_trace_complete(packet, trace_bytes, hash_width) @@ -271,6 +276,20 @@ class TraceHelper: return path_snrs, path_hashes + def _relay_allowed(self) -> bool: + """Firmware gates intermediate-hop TRACE relay on ``allowPacketForward`` + (Mesh.cpp), so a repeater that is not forwarding (monitor / no_tx mode) + must not relay traces. Locally originated pings are injected directly + and never pass through this gate. + """ + config = getattr(self.repeater_handler, "config", None) + mode = "forward" + if isinstance(config, dict): + mode = config.get("repeater", {}).get("mode", "forward") + if mode not in ("forward", "monitor", "no_tx"): + mode = "forward" + return mode == "forward" + def _should_forward_trace( self, packet, trace_bytes: bytes, flags: int, hash_width: int ) -> bool: diff --git a/repeater/packet_router.py b/repeater/packet_router.py index d08b9eb..122d379 100644 --- a/repeater/packet_router.py +++ b/repeater/packet_router.py @@ -273,14 +273,21 @@ class PacketRouter: return {} return companion_bridges - async def _fan_out_to_bridges(self, packet, bridges, *, context: str) -> bool: - """Offer packet to each bridge; True if any bridge authenticated it. + async def _fan_out_to_bridges(self, packet, bridges, *, context: str) -> tuple: + """Offer packet to each bridge; report ``(delivered, authenticated)``. Accepts a dict of bridges — pass a single-entry dict for targeted delivery to the bridge that owns ``dest_hash``. A bridge that raises is logged and skipped; ``result.authenticated`` is read directly (every bridge returns a HandlerResult) so a broken contract surfaces instead of being hidden. + + ``delivered`` is True when at least one bridge completed without raising — + the signal callers must use before ``_mark_delivered_to_companions`` so a + delivery where every bridge raised is retried on the next copy instead of + being suppressed for the dedupe TTL. ``authenticated`` is True when any + bridge authenticated the packet. """ + delivered = False authenticated = False for bridge in bridges.values(): try: @@ -288,9 +295,10 @@ class PacketRouter: except Exception as e: logger.debug("Companion bridge %s error: %s", context, e) continue + delivered = True if result.authenticated is True: authenticated = True - return authenticated + return delivered, authenticated async def _consume_via_local_candidates( self, packet, metadata: dict, dest_hash, helper, process_method_name: str @@ -316,9 +324,15 @@ class PacketRouter: consumed = False if has_companion: - bridge_result = await companion_bridges[dest_hash].process_received_packet(packet) - if bridge_result.authenticated: - consumed = True + # A raising bridge must not abort the candidate loop: the colliding + # room-server / repeater identity below still gets offered the packet. + try: + bridge_result = await companion_bridges[dest_hash].process_received_packet(packet) + except Exception as e: + logger.debug("Companion bridge candidate error: %s", e) + else: + if bridge_result.authenticated: + consumed = True # Offer to the room-server / repeater identity when it shares the hash # (collision) or when no local companion claims it at all (normal # server-owned + remote-forward handling). @@ -632,21 +646,21 @@ class PacketRouter: companion_bridges = self._companion_bridges_for_packet(packet, metadata) if dest_hash is not None and dest_hash in companion_bridges: if not self._was_delivered_to_companions(packet): - consumed = ( - await self._fan_out_to_bridges( - packet, {dest_hash: companion_bridges[dest_hash]}, context="PATH" - ) - or consumed + delivered, authenticated = await self._fan_out_to_bridges( + packet, {dest_hash: companion_bridges[dest_hash]}, context="PATH" ) - self._mark_delivered_to_companions(packet) + consumed = authenticated or consumed + if delivered: + self._mark_delivered_to_companions(packet) elif companion_bridges and not self._was_delivered_to_companions(packet): # Dest not in bridges: path-return with ephemeral dest (e.g. multi-hop login). # Deliver to all bridges; each will try to decrypt and ignore if not relevant. - consumed = ( - await self._fan_out_to_bridges(packet, companion_bridges, context="PATH") - or consumed + delivered, authenticated = await self._fan_out_to_bridges( + packet, companion_bridges, context="PATH" ) - self._mark_delivered_to_companions(packet) + consumed = authenticated or consumed + if delivered: + self._mark_delivered_to_companions(packet) logger.debug( "PATH dest=0x%02x (anon) delivered to %d bridge(s) for matching", dest_hash or 0, @@ -668,13 +682,13 @@ class PacketRouter: companion_bridges = self._companion_bridges_for_packet(packet, metadata) local_hash = getattr(self.daemon, "local_hash", None) if dest_hash is not None and dest_hash in companion_bridges: - consumed = await self._fan_out_to_bridges( + _, consumed = await self._fan_out_to_bridges( packet, {dest_hash: companion_bridges[dest_hash]}, context="RESPONSE" ) logger.info("RESPONSE dest=0x%02x delivered to companion bridge", dest_hash) elif dest_hash == local_hash and companion_bridges: # Response addressed to this repeater (e.g. path-based reply to first hop) - consumed = await self._fan_out_to_bridges( + _, consumed = await self._fan_out_to_bridges( packet, companion_bridges, context="RESPONSE" ) logger.info( @@ -686,7 +700,7 @@ class PacketRouter: # Dest not in bridges and not local: likely ANON_REQ response (dest = ephemeral # sender hash). Deliver to all bridges; each will try to decrypt and ignore if # not relevant (firmware-like behavior, works with multiple companion bridges). - consumed = await self._fan_out_to_bridges( + _, consumed = await self._fan_out_to_bridges( packet, companion_bridges, context="RESPONSE" ) logger.debug( @@ -711,8 +725,11 @@ class PacketRouter: companion_bridges = self._companion_bridges_for_packet(packet, metadata) final_hop = _is_direct_final_hop(packet) if companion_bridges and (final_hop or not self._was_delivered_to_companions(packet)): - await self._fan_out_to_bridges(packet, companion_bridges, context="RESPONSE") - self._mark_delivered_to_companions(packet) + delivered, _ = await self._fan_out_to_bridges( + packet, companion_bridges, context="RESPONSE" + ) + if delivered: + self._mark_delivered_to_companions(packet) if companion_bridges and final_hop: # DIRECT with empty path: we're the final hop, so consume after delivery. processed_by_injection = True diff --git a/tests/test_handler_helpers_mesh_cli.py b/tests/test_handler_helpers_mesh_cli.py index 966415a..6504176 100644 --- a/tests/test_handler_helpers_mesh_cli.py +++ b/tests/test_handler_helpers_mesh_cli.py @@ -15,6 +15,7 @@ def _base_config(): "longitude": 3.4, "airtime_factor": 1.1, "advert_interval_minutes": 120, + # Stale orphan key from the old CLI; nothing must read it. "flood_advert_interval_hours": 24, "max_flood_hops": 20, "rx_delay_base": 0.2, @@ -23,6 +24,12 @@ def _base_config(): "multi_acks": 2, "interference_threshold": -115, "agc_reset_interval": 8, + "security": { + "admin_password": "adminpw", + "guest_password": "guest", + "allow_read_only": True, + "max_clients": 5, + }, }, "radio": { "frequency": 915000000, @@ -32,13 +39,14 @@ def _base_config(): "tx_power": 22, }, "mesh": {"path_hash_mode": 0, "loop_detect": "minimal"}, - "security": {"guest_password": "guest", "allow_read_only": True}, + # Decoy: the legacy top-level section the CLI must no longer touch. + "security": {"guest_password": "stale-top-level", "allow_read_only": False}, } -def _cfg_mgr(save_ok=True, err=None): +def _cfg_mgr(save_ok=True): return SimpleNamespace( - save_to_file=MagicMock(return_value=(save_ok, err)), + save_to_file=MagicMock(return_value=save_ok), live_update_daemon=MagicMock(), ) @@ -95,11 +103,15 @@ def test_cmd_password_save_success_failure_and_exception(): assert cli_ok._cmd_password("password ") == "Error: Password cannot be empty" assert cli_ok._cmd_password("password newpw") == "password now: newpw" - ok_mgr.live_update_daemon.assert_called_once_with(["security"]) + # The password must land where LoginHelper reads it, not the legacy top-level section. + assert cfg["repeater"]["security"]["admin_password"] == "newpw" + assert "password" not in cfg["security"] + ok_mgr.live_update_daemon.assert_called_once_with(["repeater"]) - bad_mgr = _cfg_mgr(save_ok=False, err="disk") + bad_mgr = _cfg_mgr(save_ok=False) cli_bad = MeshCLI("/tmp/cfg.yaml", _base_config(), bad_mgr) assert "Failed to save config" in cli_bad._cmd_password("password x") + bad_mgr.live_update_daemon.assert_not_called() ex_mgr = SimpleNamespace( save_to_file=MagicMock(side_effect=RuntimeError("boom")), @@ -178,15 +190,26 @@ def test_cmd_set_updates_and_validation_errors(): assert cli._cmd_set("repeat off").endswith("OFF") assert cfg["repeater"]["mode"] == "monitor" - assert cli._cmd_set("radio 900000000 250000 9 6").startswith("OK") - assert cfg["radio"]["frequency"] == 900000000.0 + # CLI input is MHz/kHz (firmware parity); the config stores Hz. + assert cli._cmd_set("radio 900 250 9 6").startswith("OK") + assert cfg["radio"]["frequency"] == 900000000 + assert cfg["radio"]["bandwidth"] == 250000 + assert cfg["radio"]["spreading_factor"] == 9 + assert cfg["radio"]["coding_rate"] == 6 + assert cli._cmd_set("radio 100 250 9 6") == "Error, invalid radio params" + assert cli._cmd_set("radio 900 250 4 6") == "Error, invalid radio params" - assert cli._cmd_set("freq 868000000").startswith("OK") - assert cli._cmd_set("tx 17") == "OK" + assert cli._cmd_set("freq 868.5").startswith("OK") + assert cfg["radio"]["frequency"] == 868500000 + assert cli._cmd_set("tx 17") == "OK - restart repeater to apply" + assert cfg["radio"]["tx_power"] == 17 assert cli._cmd_set("guest.password g") == "OK" + assert cfg["repeater"]["security"]["guest_password"] == "g" + assert cfg["security"]["guest_password"] == "stale-top-level" assert cli._cmd_set("owner.info Alice|Ops") == "OK" assert cfg["repeater"]["owner_info"] == "Alice\nOps" assert cli._cmd_set("allow.read.only off") == "OK" + assert cfg["repeater"]["security"]["allow_read_only"] is False assert cli._cmd_set("path.hash.mode 2") == "OK" assert cfg["mesh"]["path_hash_mode"] == 2 assert cli._cmd_set("path.hash.mode 3") == "Error: path.hash.mode must be 0, 1, or 2" @@ -199,10 +222,18 @@ def test_cmd_set_updates_and_validation_errors(): assert cli._cmd_set("advert.interval 59").startswith("Error: interval range") assert cli._cmd_set("flood.advert.interval 2").startswith("Error: interval range") + assert cli._cmd_set("flood.advert.interval 12") == "OK" + # The engine's timer key is updated; the stale orphan key is left alone. + assert cfg["repeater"]["send_advert_interval_hours"] == 12 + assert cfg["repeater"]["flood_advert_interval_hours"] == 24 assert cli._cmd_set("flood.max 100") == "Error: max 64" - assert cli._cmd_set("rxdelay -1") == "Error: cannot be negative" - assert cli._cmd_set("txdelay -1") == "Error: cannot be negative" - assert cli._cmd_set("direct.txdelay -1") == "Error: cannot be negative" + assert cli._cmd_set("rxdelay -1") == "Error, must be 0-20" + assert cli._cmd_set("rxdelay 20.5") == "Error, must be 0-20" + assert cli._cmd_set("rxdelay 20") == "OK" + assert cli._cmd_set("txdelay -1") == "Error, must be 0-2" + assert cli._cmd_set("txdelay 2.5") == "Error, must be 0-2" + assert cli._cmd_set("direct.txdelay -1") == "Error, must be 0-2" + assert cli._cmd_set("direct.txdelay 2.5") == "Error, must be 0-2" assert cli._cmd_set("agc.reset.interval 10") == "OK - interval rounded to 8" assert cli._cmd_set("bad") == "Error: Missing value" @@ -442,3 +473,136 @@ def test_discovery_auto_add_skips_local_node_and_persists_remote(): ) assert remote_result["auto_added"] is True storage.record_advert.assert_called_once() + + +def test_cmd_set_save_failure_reports_error_and_skips_live_update(): + mgr = _cfg_mgr(save_ok=False) + cli = MeshCLI("/tmp/cfg.yaml", _base_config(), mgr) + + for command in ("af 2.0", "name x", "freq 915", "guest.password g", "flood.max 8"): + assert cli._cmd_set(command) == "Error: Failed to save config" + mgr.live_update_daemon.assert_not_called() + + +def test_cmd_get_reads_repeater_security_section(): + cli = MeshCLI("/tmp/cfg.yaml", _base_config(), _cfg_mgr()) + + # Values come from repeater.security, not the legacy top-level decoy. + assert cli._cmd_get("guest.password") == "> guest" + assert cli._cmd_get("allow.read.only") == "> on" + + +def test_cli_password_change_applies_to_live_repeater_acl(tmp_path): + """A CLI password change must reach the running LoginHelper ACL, so the + next login attempt authenticates against the new password.""" + from repeater.config_manager import ConfigManager + from repeater.handler_helpers.login import LoginHelper + + cfg = _base_config() + login_helper = LoginHelper(identity_manager=None, config=cfg) + identity = SimpleNamespace(get_public_key=lambda: b"\x42" + b"\x00" * 31) + login_helper.register_identity( + name="repeater", identity=identity, identity_type="repeater", config=cfg + ) + acl = login_helper.get_acl_for_identity(0x42) + assert acl.admin_password == "adminpw" + assert acl.guest_password == "guest" + + daemon = SimpleNamespace(config=cfg, login_helper=login_helper) + manager = ConfigManager( + config_path=str(tmp_path / "config.yaml"), config=cfg, daemon_instance=daemon + ) + cli = MeshCLI(str(tmp_path / "config.yaml"), cfg, manager) + + assert cli._cmd_password("password fresh-secret") == "password now: fresh-secret" + assert acl.admin_password == "fresh-secret" + + assert cli._cmd_set("guest.password fresh-guest") == "OK" + assert acl.guest_password == "fresh-guest" + + assert cli._cmd_set("allow.read.only off") == "OK" + assert acl.allow_read_only is False + + +def test_cmd_set_radio_commands_stage_without_live_apply(): + """Radio changes match firmware reboot-to-apply: saved to disk, never + live-applied (a live retune would cut off the admin mid-session).""" + cfg = _base_config() + mgr = _cfg_mgr() + cli = MeshCLI("/tmp/cfg.yaml", cfg, mgr) + + assert cli._cmd_set("radio 900 250 9 6") == "OK - restart repeater to apply" + assert cli._cmd_set("freq 868.5") == "OK - restart repeater to apply" + assert cli._cmd_set("tx 17") == "OK - restart repeater to apply" + + assert mgr.save_to_file.call_count == 3 + mgr.live_update_daemon.assert_not_called() + + +def test_cmd_get_flood_advert_interval_reads_engine_key(): + cfg = _base_config() + cli = MeshCLI("/tmp/cfg.yaml", cfg, _cfg_mgr()) + + # The orphan flood_advert_interval_hours (24) must be ignored; the engine + # key is absent so the engine default (10) is reported. + assert cli._cmd_get("flood.advert.interval") == "> 10" + + cfg["repeater"]["send_advert_interval_hours"] = 6 + assert cli._cmd_get("flood.advert.interval") == "> 6" + + +def test_cli_set_commands_persist_with_real_config_manager(tmp_path): + """Every CLI set command must work against the real ConfigManager bool save + contract: reply OK and leave the change on disk.""" + import yaml + + from repeater.config_manager import ConfigManager + + config_path = tmp_path / "config.yaml" + cfg = _base_config() + manager = ConfigManager(config_path=str(config_path), config=cfg, daemon_instance=None) + cli = MeshCLI(str(config_path), cfg, manager) + + commands = [ + "af 1.5", + "name node-b", + "repeat off", + "lat 10.5", + "lon -3.25", + "radio 869.618 250 9 6", + "freq 915.125", + "tx 17", + "guest.password gpw", + "owner.info Bob|Lab", + "allow.read.only on", + "advert.interval 90", + "flood.advert.interval 12", + "flood.max 8", + "path.hash.mode 1", + "loop.detect moderate", + "rxdelay 4.5", + "txdelay 1.5", + "direct.txdelay 0.25", + "multi.acks 1", + "int.thresh -110", + "agc.reset.interval 8", + ] + for command in commands: + reply = cli._cmd_set(command) + assert reply.startswith("OK"), f"set {command!r} replied {reply!r}" + + assert cli._cmd_password("password newadminpw") == "password now: newadminpw" + + saved = yaml.safe_load(config_path.read_text()) + assert saved["repeater"]["airtime_factor"] == 1.5 + assert saved["repeater"]["node_name"] == "node-b" + assert saved["repeater"]["mode"] == "monitor" + assert saved["radio"]["frequency"] == 915125000 + assert saved["radio"]["bandwidth"] == 250000 + assert saved["repeater"]["security"]["guest_password"] == "gpw" + assert saved["repeater"]["security"]["admin_password"] == "newadminpw" + assert saved["repeater"]["security"]["allow_read_only"] is True + assert saved["repeater"]["send_advert_interval_hours"] == 12 + assert saved["delays"]["rx_delay_base"] == 4.5 + assert saved["delays"]["tx_delay_factor"] == 1.5 + assert saved["delays"]["direct_tx_delay_factor"] == 0.25 diff --git a/tests/test_handler_helpers_trace_discovery_login.py b/tests/test_handler_helpers_trace_discovery_login.py index 9333f3b..ff9aa2d 100644 --- a/tests/test_handler_helpers_trace_discovery_login.py +++ b/tests/test_handler_helpers_trace_discovery_login.py @@ -201,6 +201,70 @@ async def test_trace_helper_forward_trace_packet_updates_recent_record_and_injec packet_injector.assert_awaited_once() +def _trace_relay_helper(mode, forward=True): + """TraceHelper wired so _should_forward_trace says yes; mode comes from config.""" + repeater_handler = MagicMock() + repeater_handler.is_duplicate.return_value = False + repeater_handler.calculate_packet_score.return_value = 0.9 + repeater_handler.config = {"repeater": {"mode": mode}} + helper = TraceHelper( + local_hash=0x42, + local_identity=FakeIdentity(0x42), + repeater_handler=repeater_handler, + ) + helper._forward_trace_packet = AsyncMock() + helper._extract_path_info = MagicMock(return_value=([], [])) + helper._should_forward_trace = MagicMock(return_value=forward) + helper.trace_handler._parse_trace_payload = MagicMock( + return_value={ + "valid": True, + "trace_path_bytes": b"\x42\x43", + "flags": 0, + "trace_hops": [b"\x42", b"\x43"], + "trace_path": [0x42, 0x43], + "tag": 1234, + } + ) + helper.trace_handler._format_trace_response = MagicMock(return_value="trace ok") + return helper + + +@pytest.mark.asyncio +async def test_trace_relay_suppressed_in_monitor_and_no_tx_mode(): + # Firmware gates TRACE relay on allowPacketForward, so a repeater with + # repeat off must not relay an intermediate-hop trace. + for mode in ("monitor", "no_tx"): + helper = _trace_relay_helper(mode) + packet = DummyPacket(path=b"\x01", payload=b"\xaa\xbb\xcc") + await helper.process_trace_packet(packet) + helper._forward_trace_packet.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_trace_relay_allowed_in_forward_and_unknown_mode(): + for mode in ("forward", "weird-mode"): + helper = _trace_relay_helper(mode) + packet = DummyPacket(path=b"\x01", payload=b"\xaa\xbb\xcc") + await helper.process_trace_packet(packet) + helper._forward_trace_packet.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_trace_ping_response_still_matches_in_monitor_mode(): + # Ping origination/response handling is not relay: a pending ping must + # resolve even when the repeater is not forwarding. + helper = _trace_relay_helper("monitor", forward=False) + tag = 555 + helper.trace_handler._parse_trace_payload.return_value["tag"] = tag + evt = helper.register_ping(tag, 0x42) + + packet = DummyPacket(path=b"\x01", payload=b"\xaa\xbb\xcc") + await helper.process_trace_packet(packet) + + assert evt.is_set() + helper._forward_trace_packet.assert_not_awaited() + + def test_trace_helper_cleanup_stale_pings(): helper = TraceHelper( local_hash=0x42, local_identity=FakeIdentity(0x42), repeater_handler=MagicMock() diff --git a/tests/test_packet_router.py b/tests/test_packet_router.py index 0f0d7b7..a1e46f5 100644 --- a/tests/test_packet_router.py +++ b/tests/test_packet_router.py @@ -1321,3 +1321,106 @@ class TestInjectedTxRawEcho(unittest.IsolatedAsyncioTestCase): ok = await router.inject_packet(_make_packet()) self.assertTrue(ok) + + +class TestCompanionDeliveryFailureHandling(unittest.IsolatedAsyncioTestCase): + """Dedupe marking and candidate-loop behaviour when companion bridges raise.""" + + async def test_path_all_bridges_raising_is_retried_on_next_copy(self): + """A PATH delivery where every bridge raised must not be marked + delivered; the next mesh copy gets another delivery attempt.""" + daemon = _make_daemon() + bridge = _make_bridge() + bridge.process_received_packet = AsyncMock(side_effect=RuntimeError("bridge down")) + daemon.companion_bridges = {0x01: bridge} + router = PacketRouter(daemon) + pkt = _make_packet(PathHandler.payload_type()) + pkt.payload = bytes([0x01, 0xAA]) + + await router._route_packet(pkt) + self.assertEqual(bridge.process_received_packet.await_count, 1) + + # Bridge recovers; the second copy must be delivered, not TTL-suppressed. + recovered = AsyncMock(return_value=HandlerResult.not_for_us()) + bridge.process_received_packet = recovered + await router._route_packet(pkt) + recovered.assert_awaited_once() + + async def test_path_partial_bridge_failure_still_marks_delivered(self): + """One healthy bridge is a delivery: the duplicate copy stays suppressed.""" + daemon = _make_daemon() + raising = _make_bridge() + raising.process_received_packet = AsyncMock(side_effect=RuntimeError("boom")) + healthy = _make_bridge() + healthy.process_received_packet = AsyncMock(return_value=HandlerResult.not_for_us()) + daemon.companion_bridges = {0x01: raising, 0x02: healthy} + router = PacketRouter(daemon) + pkt = _make_packet(PathHandler.payload_type()) + # Dest not in bridges: anon path-return, delivered to all bridges. + pkt.payload = bytes([0xEE, 0xAA]) + + await router._route_packet(pkt) + await router._route_packet(pkt) + + healthy.process_received_packet.assert_awaited_once() + raising.process_received_packet.assert_awaited_once() + + async def test_protocol_response_all_bridges_raising_is_retried_on_next_copy(self): + daemon = _make_daemon() + bridge = _make_bridge() + bridge.process_received_packet = AsyncMock(side_effect=RuntimeError("bridge down")) + daemon.companion_bridges = {0x01: bridge} + router = PacketRouter(daemon) + pkt = _make_packet(ProtocolResponseHandler.payload_type()) + pkt.header = ROUTE_TYPE_FLOOD # not a final hop: dedupe decides delivery + + with patch("repeater.packet_router.PathHandler.payload_type", return_value=0x55): + await router._route_packet(pkt) + self.assertEqual(bridge.process_received_packet.await_count, 1) + + recovered = AsyncMock(return_value=HandlerResult.not_for_us()) + bridge.process_received_packet = recovered + await router._route_packet(pkt) + recovered.assert_awaited_once() + + async def test_login_candidate_bridge_error_still_offers_local_identity(self): + """A raising companion bridge must not abort the candidate loop: the + hash-colliding room-server/repeater identity still gets the packet.""" + daemon = _make_daemon() + bridge = _make_bridge() + bridge.process_received_packet = AsyncMock(side_effect=RuntimeError("bridge down")) + daemon.companion_bridges = {0xF5: bridge} + daemon.login_helper = MagicMock() + daemon.login_helper.handlers = {0xF5: MagicMock()} + daemon.login_helper.process_login_packet = AsyncMock(return_value=True) + daemon.repeater_handler = AsyncMock() + daemon.repeater_handler.storage = MagicMock() + daemon.repeater_handler.record_packet_only = MagicMock() + router = PacketRouter(daemon) + pkt = _make_packet(LoginServerHandler.payload_type()) + pkt.payload = bytes([0xF5, 0x01]) + + await router._route_packet(pkt) + + bridge.process_received_packet.assert_awaited_once() + daemon.login_helper.process_login_packet.assert_awaited_once() + # The colliding identity consumed it, so the engine must not re-forward. + daemon.repeater_handler.assert_not_awaited() + + async def test_text_candidate_bridge_error_leaves_packet_for_engine(self): + """Bridge raises and no local identity claims the text: the packet must + still reach the forwarding engine instead of dying with the exception.""" + daemon = _make_daemon() + bridge = _make_bridge() + bridge.process_received_packet = AsyncMock(side_effect=RuntimeError("bridge down")) + daemon.companion_bridges = {0xF5: bridge} + daemon.text_helper = MagicMock() + daemon.text_helper.handlers = {} + daemon.text_helper.process_text_packet = AsyncMock(return_value=False) + router = PacketRouter(daemon) + pkt = _make_packet(TextMessageHandler.payload_type()) + pkt.payload = bytes([0xF5, 0x01]) + + await router._route_packet(pkt) + + daemon.repeater_handler.assert_awaited_once()