Merge branch 'main' into feat/adguard-export-plugin

test/plugins/test_fritzbox.py

@@ -0,0 +1,414 @@
+"""
+Tests for Fritz!Box plugin (fritzbox.py).
+
+fritzbox.py is imported directly. Its module-level side effects
+(get_setting_value, Logger, Plugin_Objects) are patched out before the
+first import so no live config reads, log files, or result files are
+created during tests.
+"""
+
+import sys
+import os
+from unittest.mock import patch, MagicMock
+
+from utils.crypto_utils import string_to_fake_mac
+
+import pytest
+
+# ---------------------------------------------------------------------------
+# Path setup
+# ---------------------------------------------------------------------------
+
+_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
+_SERVER = os.path.join(_ROOT, "server")
+_PLUGIN_DIR = os.path.join(_ROOT, "front", "plugins", "fritzbox")
+
+for _p in [_ROOT, _SERVER, _PLUGIN_DIR]:
+    if _p not in sys.path:
+        sys.path.insert(0, _p)
+
+# ---------------------------------------------------------------------------
+# Import fritzbox with module-level side effects patched
+# ---------------------------------------------------------------------------
+# fritzbox.py calls get_setting_value(), Logger(), and Plugin_Objects() at
+# module level. Patching these before the first import prevents live config
+# reads, log-file creation, and result-file creation during tests.
+
+with patch("helper.get_setting_value", return_value="UTC"), \
+     patch("logger.Logger"), \
+     patch("plugin_helper.Plugin_Objects"):
+    import fritzbox  # noqa: E402
+
+from plugin_helper import normalize_mac  # noqa: E402
+
+# ---------------------------------------------------------------------------
+# Shared helpers
+# ---------------------------------------------------------------------------
+
+
+def _make_host_entry(mac="AA:BB:CC:DD:EE:FF", ip="192.168.1.10",
+                     hostname="testdevice", active=1, interface="Ethernet"):
+    return {
+        "NewMACAddress": mac,
+        "NewIPAddress": ip,
+        "NewHostName": hostname,
+        "NewActive": active,
+        "NewInterfaceType": interface,
+    }
+
+
+@pytest.fixture
+def mock_fritz_hosts():
+    """
+    Patches fritzbox.FritzHosts so that get_connected_devices() uses a
+    controllable mock.  Yields the FritzHosts *instance* (what FritzHosts(fc)
+    returns).
+    """
+    hosts_instance = MagicMock()
+    with patch("fritzbox.FritzHosts", return_value=hosts_instance):
+        yield hosts_instance
+
+
+# ===========================================================================
+# get_connected_devices
+# ===========================================================================
+
+class TestGetConnectedDevices:
+
+    def test_returns_active_device(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 1
+        mock_fritz_hosts.get_generic_host_entry.return_value = _make_host_entry(active=1)
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=True)
+        assert len(devices) == 1
+        assert devices[0]["active_status"] == "Active"
+
+    def test_active_only_filters_inactive_device(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 2
+        mock_fritz_hosts.get_generic_host_entry.side_effect = [
+            _make_host_entry(mac="AA:BB:CC:DD:EE:01", active=1),
+            _make_host_entry(mac="AA:BB:CC:DD:EE:02", active=0),
+        ]
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=True)
+        assert len(devices) == 1
+        assert devices[0]["mac_address"] == "aa:bb:cc:dd:ee:01"
+
+    def test_active_only_false_includes_inactive_device(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 2
+        mock_fritz_hosts.get_generic_host_entry.side_effect = [
+            _make_host_entry(mac="AA:BB:CC:DD:EE:01", active=1),
+            _make_host_entry(mac="AA:BB:CC:DD:EE:02", active=0),
+        ]
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=False)
+        assert len(devices) == 2
+        assert devices[1]["active_status"] == "Inactive"
+
+    def test_device_without_mac_is_skipped(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 2
+        mock_fritz_hosts.get_generic_host_entry.side_effect = [
+            _make_host_entry(mac=""),
+            _make_host_entry(mac="AA:BB:CC:DD:EE:01"),
+        ]
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=False)
+        assert len(devices) == 1
+        assert devices[0]["mac_address"] == "aa:bb:cc:dd:ee:01"
+
+    def test_ethernet_interface_maps_to_lan(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 1
+        mock_fritz_hosts.get_generic_host_entry.return_value = _make_host_entry(interface="Ethernet")
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=False)
+        assert devices[0]["interface_type"] == "LAN"
+
+    def test_wifi_interface_maps_to_wifi(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 1
+        mock_fritz_hosts.get_generic_host_entry.return_value = _make_host_entry(interface="802.11")
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=False)
+        assert devices[0]["interface_type"] == "WiFi"
+
+    def test_unknown_interface_is_preserved(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 1
+        mock_fritz_hosts.get_generic_host_entry.return_value = _make_host_entry(interface="SomeOtherType")
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=False)
+        assert devices[0]["interface_type"] == "SomeOtherType"
+
+    def test_mac_address_is_normalized_to_lowercase(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 1
+        mock_fritz_hosts.get_generic_host_entry.return_value = _make_host_entry(mac="AA:BB:CC:DD:EE:FF")
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=False)
+        assert devices[0]["mac_address"] == "aa:bb:cc:dd:ee:ff"
+
+    def test_missing_hostname_defaults_to_unknown(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 1
+        mock_fritz_hosts.get_generic_host_entry.return_value = _make_host_entry(hostname="")
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=False)
+        assert devices[0]["hostname"] == "Unknown"
+
+    def test_failed_host_entry_does_not_abort_remaining(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 3
+        mock_fritz_hosts.get_generic_host_entry.side_effect = [
+            _make_host_entry(mac="AA:BB:CC:DD:EE:01"),
+            Exception("TR-064 timeout"),
+            _make_host_entry(mac="AA:BB:CC:DD:EE:03"),
+        ]
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=False)
+        assert len(devices) == 2
+
+    def test_empty_host_list_returns_empty(self, mock_fritz_hosts):
+        mock_fritz_hosts.host_numbers = 0
+        devices = fritzbox.get_connected_devices(MagicMock(), active_only=False)
+        assert devices == []
+
+
+# ===========================================================================
+# check_guest_wifi_status
+# ===========================================================================
+
+class TestCheckGuestWifiStatus:
+
+    def test_disabled_service_returns_inactive(self):
+        fc = MagicMock()
+        fc.call_action.return_value = {"NewEnable": False, "NewSSID": ""}
+        result = fritzbox.check_guest_wifi_status(fc, guest_service_num=3)
+        assert result["active"] is False
+
+    def test_enabled_service_returns_active(self):
+        fc = MagicMock()
+        fc.call_action.return_value = {"NewEnable": True, "NewSSID": "MyGuestWiFi"}
+        result = fritzbox.check_guest_wifi_status(fc, guest_service_num=3)
+        assert result["active"] is True
+        assert result["ssid"] == "MyGuestWiFi"
+
+    def test_queries_correct_service_number(self):
+        fc = MagicMock()
+        fc.call_action.return_value = {"NewEnable": True, "NewSSID": "Guest"}
+        fritzbox.check_guest_wifi_status(fc, guest_service_num=2)
+        fc.call_action.assert_called_once_with("WLANConfiguration2", "GetInfo")
+
+    def test_service_exception_returns_inactive(self):
+        fc = MagicMock()
+        fc.call_action.side_effect = Exception("Service unavailable")
+        result = fritzbox.check_guest_wifi_status(fc, guest_service_num=3)
+        assert result["active"] is False
+
+    def test_empty_ssid_uses_default_label(self):
+        fc = MagicMock()
+        fc.call_action.return_value = {"NewEnable": True, "NewSSID": ""}
+        result = fritzbox.check_guest_wifi_status(fc, guest_service_num=3)
+        assert result["active"] is True
+        assert result["ssid"] == "Guest WiFi"
+
+    def test_service1_can_be_guest(self):
+        fc = MagicMock()
+        fc.call_action.return_value = {"NewEnable": True, "NewSSID": "Gast"}
+        result = fritzbox.check_guest_wifi_status(fc, guest_service_num=1)
+        assert result["active"] is True
+        fc.call_action.assert_called_once_with("WLANConfiguration1", "GetInfo")
+
+
+# ===========================================================================
+# create_guest_wifi_device
+# ===========================================================================
+
+class TestCreateGuestWifiDevice:
+
+    def _fc_with_mac(self, mac):
+        fc = MagicMock()
+        fc.call_action.return_value = {"NewMACAddress": mac}
+        return fc
+
+    def test_returns_device_dict(self):
+        device = fritzbox.create_guest_wifi_device(self._fc_with_mac("AA:BB:CC:DD:EE:FF"))
+        assert device is not None
+        assert "mac_address" in device
+        assert device["hostname"] == "Guest WiFi Network"
+        assert device["active_status"] == "Active"
+        assert device["interface_type"] == "Access Point"
+        assert device["ip_address"] == ""
+        # MAC must match string_to_fake_mac output (fa:ce: prefix)
+        assert device["mac_address"].startswith("fa:ce:")
+
+    def test_guest_mac_has_locally_administered_bit(self):
+        """The locally-administered bit (0x02) must be set in the first byte.
+        string_to_fake_mac uses the 'fa:ce:' prefix; 0xFA & 0x02 == 0x02."""
+        device = fritzbox.create_guest_wifi_device(self._fc_with_mac("AA:BB:CC:DD:EE:FF"))
+        first_byte = int(device["mac_address"].split(":")[0], 16)
+        assert first_byte & 0x02 != 0
+
+    def test_guest_mac_format_is_valid(self):
+        """MAC must be 6 colon-separated lowercase hex pairs."""
+        device = fritzbox.create_guest_wifi_device(self._fc_with_mac("AA:BB:CC:DD:EE:FF"))
+        parts = device["mac_address"].split(":")
+        assert len(parts) == 6
+        for part in parts:
+            assert len(part) == 2
+            int(part, 16)  # raises ValueError if not valid hex
+
+    def test_guest_mac_is_deterministic(self):
+        """Same Fritz!Box MAC must always produce the same guest MAC."""
+        fc = self._fc_with_mac("AA:BB:CC:DD:EE:FF")
+        mac1 = fritzbox.create_guest_wifi_device(fc)["mac_address"]
+        mac2 = fritzbox.create_guest_wifi_device(fc)["mac_address"]
+        assert mac1 == mac2
+
+    def test_different_fritzbox_macs_produce_different_guest_macs(self):
+        mac_a = fritzbox.create_guest_wifi_device(self._fc_with_mac("AA:BB:CC:DD:EE:01"))["mac_address"]
+        mac_b = fritzbox.create_guest_wifi_device(self._fc_with_mac("AA:BB:CC:DD:EE:02"))["mac_address"]
+        assert mac_a != mac_b
+
+    def test_no_fritzbox_mac_uses_fallback(self):
+        """When DeviceInfo returns no MAC, fall back to a sentinel-derived MAC."""
+        fc = MagicMock()
+        fc.call_action.return_value = {"NewMACAddress": ""}
+        device = fritzbox.create_guest_wifi_device(fc)
+        assert device["mac_address"] == string_to_fake_mac("FRITZBOX_GUEST")
+
+    def test_device_info_exception_returns_none(self):
+        """If DeviceInfo call raises, create_guest_wifi_device must return None."""
+        fc = MagicMock()
+        fc.call_action.side_effect = Exception("Connection refused")
+        device = fritzbox.create_guest_wifi_device(fc)
+        assert device is None
+
+    def test_known_mac_produces_known_guest_mac(self):
+        """
+        Regression anchor: for a fixed Fritz!Box MAC, the expected guest MAC
+        is derived via string_to_fake_mac(normalize_mac(...)).  If the hashing
+        logic in fritzbox.py or string_to_fake_mac changes, this test fails.
+        """
+        fritzbox_mac = normalize_mac("AA:BB:CC:DD:EE:FF")
+        expected = string_to_fake_mac(fritzbox_mac)
+
+        device = fritzbox.create_guest_wifi_device(self._fc_with_mac("AA:BB:CC:DD:EE:FF"))
+        assert device["mac_address"] == expected
+
+
+# ===========================================================================
+# get_fritzbox_connection
+# ===========================================================================
+
+class TestGetFritzboxConnection:
+
+    def test_successful_connection(self):
+        fc_instance = MagicMock()
+        fc_instance.modelname = "FRITZ!Box 7590"
+        fc_instance.system_version = "7.57"
+        fc_class = MagicMock(return_value=fc_instance)
+
+        with patch("fritzbox.FritzConnection", fc_class):
+            result = fritzbox.get_fritzbox_connection("fritz.box", 49443, "admin", "pass", True)
+
+        assert result is fc_instance
+        fc_class.assert_called_once_with(
+            address="fritz.box", port=49443, user="admin", password="pass", use_tls=True, timeout=10,
+        )
+
+    def test_import_error_returns_none(self):
+        with patch("fritzbox.FritzConnection", side_effect=ImportError("fritzconnection not found")):
+            result = fritzbox.get_fritzbox_connection("fritz.box", 49443, "admin", "pass", True)
+
+        assert result is None
+
+    def test_connection_exception_returns_none(self):
+        with patch("fritzbox.FritzConnection", side_effect=Exception("Connection refused")):
+            result = fritzbox.get_fritzbox_connection("fritz.box", 49443, "admin", "pass", True)
+
+        assert result is None
+
+
+# ===========================================================================
+# main
+# ===========================================================================
+
+class TestMain:
+
+    _SETTINGS = {
+        "FRITZBOX_HOST": "fritz.box",
+        "FRITZBOX_PORT": 49443,
+        "FRITZBOX_USER": "admin",
+        "FRITZBOX_PASS": "secret",
+        "FRITZBOX_USE_TLS": True,
+        "FRITZBOX_REPORT_GUEST": False,
+        "FRITZBOX_GUEST_SERVICE": 3,
+        "FRITZBOX_ACTIVE_ONLY": True,
+    }
+
+    def _patch_settings(self):
+        return patch.object(
+            fritzbox, "get_setting_value",
+            side_effect=lambda key: self._SETTINGS[key],
+        )
+
+    def test_connection_failure_returns_1(self):
+        mock_po = MagicMock()
+        with self._patch_settings(), \
+             patch.object(fritzbox, "get_fritzbox_connection", return_value=None), \
+             patch.object(fritzbox, "plugin_objects", mock_po):
+            result = fritzbox.main()
+
+        assert result == 1
+        mock_po.write_result_file.assert_called_once()
+        mock_po.add_object.assert_not_called()
+
+    def test_scan_processes_devices(self):
+        devices = [
+            {"mac_address": "aa:bb:cc:dd:ee:01", "ip_address": "192.168.1.10",
+             "hostname": "device1", "active_status": "Active", "interface_type": "LAN"},
+            {"mac_address": "aa:bb:cc:dd:ee:02", "ip_address": "192.168.1.11",
+             "hostname": "device2", "active_status": "Active", "interface_type": "WiFi"},
+        ]
+        mock_po = MagicMock()
+
+        with self._patch_settings(), \
+             patch.object(fritzbox, "get_fritzbox_connection", return_value=MagicMock()), \
+             patch.object(fritzbox, "get_connected_devices", return_value=devices), \
+             patch.object(fritzbox, "plugin_objects", mock_po):
+            result = fritzbox.main()
+
+        assert result == 0
+        assert mock_po.add_object.call_count == 2
+        mock_po.write_result_file.assert_called_once()
+
+    def test_guest_wifi_device_appended_when_active(self):
+        devices = [
+            {"mac_address": "aa:bb:cc:dd:ee:01", "ip_address": "192.168.1.10",
+             "hostname": "device1", "active_status": "Active", "interface_type": "LAN"},
+        ]
+        guest_device = {
+            "mac_address": "02:a1:b2:c3:d4:e5", "ip_address": "",
+            "hostname": "Guest WiFi Network", "active_status": "Active",
+            "interface_type": "Access Point",
+        }
+        settings = {**self._SETTINGS, "FRITZBOX_REPORT_GUEST": True}
+        mock_po = MagicMock()
+
+        with patch.object(fritzbox, "get_setting_value", side_effect=lambda k: settings[k]), \
+             patch.object(fritzbox, "get_fritzbox_connection", return_value=MagicMock()), \
+             patch.object(fritzbox, "get_connected_devices", return_value=devices), \
+             patch.object(fritzbox, "check_guest_wifi_status", return_value={"active": True, "ssid": "Guest"}), \
+             patch.object(fritzbox, "create_guest_wifi_device", return_value=guest_device), \
+             patch.object(fritzbox, "plugin_objects", mock_po):
+            result = fritzbox.main()
+
+        assert result == 0
+        assert mock_po.add_object.call_count == 2  # 1 device + 1 guest
+        # Verify the guest device was passed correctly
+        guest_call = mock_po.add_object.call_args_list[1]
+        assert guest_call.kwargs["primaryId"] == "02:a1:b2:c3:d4:e5"
+        assert guest_call.kwargs["watched3"] == "Access Point"
+
+    def test_guest_wifi_not_appended_when_inactive(self):
+        devices = [
+            {"mac_address": "aa:bb:cc:dd:ee:01", "ip_address": "192.168.1.10",
+             "hostname": "device1", "active_status": "Active", "interface_type": "LAN"},
+        ]
+        settings = {**self._SETTINGS, "FRITZBOX_REPORT_GUEST": True}
+        mock_po = MagicMock()
+
+        with patch.object(fritzbox, "get_setting_value", side_effect=lambda k: settings[k]), \
+             patch.object(fritzbox, "get_fritzbox_connection", return_value=MagicMock()), \
+             patch.object(fritzbox, "get_connected_devices", return_value=devices), \
+             patch.object(fritzbox, "check_guest_wifi_status", return_value={"active": False, "ssid": ""}), \
+             patch.object(fritzbox, "plugin_objects", mock_po):
+            result = fritzbox.main()
+
+        assert result == 0
+        assert mock_po.add_object.call_count == 1  # only the real device

test/plugins/test_sync_protocol.py

@@ -46,7 +46,7 @@ def _send_data(api_token, file_content, encryption_key, file_path, node_name, pr
     }
     headers = {"Authorization": f"Bearer {api_token}"}
     try:
-        response = requests.post(hub_url + API_ENDPOINT, data=data, headers=headers, timeout=5)
+        response = requests.post(hub_url + API_ENDPOINT, json=data, headers=headers, timeout=5)
         return response.status_code == 200
     except requests.RequestException:
         return False
@@ -68,9 +68,33 @@ def _get_data(api_token, node_url):
 
 
 def _node_name_from_filename(file_name: str) -> str:
-    """Mirror of the node-name extraction in sync.main()."""
-    parts = file_name.split(".")
-    return parts[2] if ("decoded" in file_name or "encoded" in file_name) else parts[1]
+    """Mirror of the node-name extraction in sync.main() (Mode 3).
+
+    PUSH shape: last_result.PLUGIN.(decoded|encoded).NodeName.N.log
+      — marker present AND the second-to-last segment (before .log) is a digit
+    PULL shape: last_result.NodeName.log
+      — no marker, or marker present but no digit counter
+        (e.g. node name is 'office.encoded.lab')
+
+    Both forms handle dots anywhere in PLUGIN or NodeName.
+    """
+    marker_present = '.decoded.' in file_name or '.encoded.' in file_name
+    is_push = marker_present and file_name.rsplit('.', 2)[1].isdigit()
+    if is_push:
+        marker = '.decoded.' if '.decoded.' in file_name else '.encoded.'
+        _, after = file_name.split(marker, 1)
+        return after.rsplit('.', 2)[0]
+    return file_name[len('last_result.'):-len('.log')]
+
+
+def _should_delete_after_process(filename: str) -> bool:
+    """Mirror of the delete-after-process condition in execute_plugin() (server/plugin.py).
+
+    Only node-sync intermediary files (.encoded. / .decoded.) are removed after
+    processing.  Local plugin result files (last_result.ARPSCAN.log etc.) must
+    survive so SYNC Mode 1 can read and forward them to the hub.
+    """
+    return ".encoded." in filename or ".decoded." in filename
 
 
 def _determine_mode(hub_url: str, send_devices: bool, plugins_to_sync: list, pull_nodes: list):
@@ -205,7 +229,7 @@ class TestSendData:
         with patch("requests.post", return_value=resp) as mock_post:
             _send_data(API_TOKEN, '{"data":[]}', ENCRYPTION_KEY,
                        "/tmp/file.log", "node1", "SYNC", HUB_URL)
-        payload = mock_post.call_args[1]["data"]
+        payload = mock_post.call_args[1]["json"]
         assert "data" in payload          # encrypted blob
         assert payload["file_path"] == "/tmp/file.log"
         assert payload["plugin"] == "SYNC"
@@ -219,7 +243,7 @@ class TestSendData:
         with patch("requests.post", return_value=resp) as mock_post:
             _send_data(API_TOKEN, plaintext, ENCRYPTION_KEY,
                        "/tmp/file.log", "node1", "SYNC", HUB_URL)
-        transmitted = mock_post.call_args[1]["data"]["data"]
+        transmitted = mock_post.call_args[1]["json"]["data"]
         assert transmitted != plaintext
         # Verify it round-trips correctly
         assert decrypt_data(transmitted, ENCRYPTION_KEY) == plaintext
@@ -296,23 +320,59 @@ class TestGetData:
 
 class TestNodeNameExtraction:
 
-    def test_simple_filename(self):
-        # last_result.MyNode.log  →  "MyNode"
+    def test_pull_mode_filename(self):
+        # PULL mode: last_result.MyNode.log  →  "MyNode"
         assert _node_name_from_filename("last_result.MyNode.log") == "MyNode"
 
-    def test_decoded_filename(self):
-        # last_result.decoded.MyNode.1.log  →  "MyNode"
-        assert _node_name_from_filename("last_result.decoded.MyNode.1.log") == "MyNode"
+    def test_push_decoded_filename(self):
+        # PUSH mode (post-decode): last_result.ARPSCAN.decoded.MyNode.1.log  →  "MyNode"
+        assert _node_name_from_filename("last_result.ARPSCAN.decoded.MyNode.1.log") == "MyNode"
 
-    def test_encoded_filename(self):
-        # last_result.encoded.MyNode.1.log  →  "MyNode"
-        assert _node_name_from_filename("last_result.encoded.MyNode.1.log") == "MyNode"
+    def test_push_encoded_filename(self):
+        # PUSH mode (pre-decode): last_result.ARPSCAN.encoded.MyNode.1.log  →  "MyNode"
+        assert _node_name_from_filename("last_result.ARPSCAN.encoded.MyNode.1.log") == "MyNode"
 
-    def test_node_name_with_underscores(self):
+    def test_pull_node_name_with_underscores(self):
         assert _node_name_from_filename("last_result.Wladek_Site.log") == "Wladek_Site"
 
-    def test_decoded_node_name_with_underscores(self):
-        assert _node_name_from_filename("last_result.decoded.Wladek_Site.1.log") == "Wladek_Site"
+    def test_push_decoded_node_name_with_underscores(self):
+        assert _node_name_from_filename("last_result.ARPSCAN.decoded.Wladek_Site.1.log") == "Wladek_Site"
+
+    def test_push_decoded_node_name_with_counter_gt_1(self):
+        # Counter increments when multiple pushes arrive before SYNC runs
+        assert _node_name_from_filename("last_result.ARPSCAN.decoded.Node_Vlan01.3.log") == "Node_Vlan01"
+
+    def test_push_decoded_different_plugins(self):
+        for plugin in ("NMAP", "PIHOLE", "DHCPLEASES"):
+            fname = f"last_result.{plugin}.decoded.HubNode.1.log"
+            assert _node_name_from_filename(fname) == "HubNode", \
+                f"Expected 'HubNode' from {fname}"
+
+    # --- dot-in-identifier regression (fragile parts[3] fix) ---
+
+    def test_pull_node_name_with_dots(self):
+        # PULL mode: node name set to e.g. "node.home" or an IP like "192.168.1.82"
+        assert _node_name_from_filename("last_result.node.home.log") == "node.home"
+        assert _node_name_from_filename("last_result.192.168.1.82.log") == "192.168.1.82"
+
+    def test_push_decoded_node_name_with_dots(self):
+        # Node name "Node.Vlan01" must survive the filename round-trip intact
+        assert _node_name_from_filename("last_result.ARPSCAN.decoded.Node.Vlan01.1.log") == "Node.Vlan01"
+
+    def test_push_decoded_plugin_name_with_dots(self):
+        # Hypothetical plugin with a dot in its name must not shift the node index
+        assert _node_name_from_filename("last_result.MY.PLUGIN.decoded.NodeA.1.log") == "NodeA"
+
+    def test_push_both_identifiers_with_dots(self):
+        assert _node_name_from_filename(
+            "last_result.A.B.decoded.x.y.z.1.log"
+        ) == "x.y.z"
+
+    def test_pull_with_encoded_in_node_name(self):
+        # Regression: PULL file whose node name contains '.encoded.' must NOT
+        # be mis-classified as a PUSH artifact (no digit counter → PULL branch).
+        assert _node_name_from_filename("last_result.office.encoded.lab.log") == "office.encoded.lab"
+        assert _node_name_from_filename("last_result.site.decoded.backup.log") == "site.decoded.backup"
 
 
 # ===========================================================================
@@ -409,5 +469,345 @@ class TestReceiveInsert:
         inserted = sync_insert_devices(conn, [device], existing_macs=set())
         assert inserted == 1
 
+
+# ===========================================================================
+# Plugin result file retention (regression for execute_plugin delete bug)
+# ===========================================================================
+
+class TestPluginFileRetention:
+    """Regression for the execute_plugin() delete-condition bug (server/plugin.py).
+
+    Before the fix the condition was ``filename != "last_result.log"``.  No
+    plugin ever writes to that literal name — all write ``last_result.ARPSCAN.log``
+    etc. — so every local result file was deleted immediately after processing,
+    before SYNC Mode 1 had a chance to read and forward it to the hub.
+
+    The corrected condition deletes ONLY ``.encoded.`` / ``.decoded.``
+    node-sync intermediary files.  Local plugin result files must survive.
+    """
+
+    def test_local_result_file_not_flagged_for_deletion(self):
+        assert _should_delete_after_process("last_result.ARPSCAN.log") is False
+
+    def test_local_result_files_for_common_plugins_not_flagged(self):
+        for plugin in ("NMAP", "PIHOLE", "SYNC", "DHCPLEASES", "ARPSCAN"):
+            fname = f"last_result.{plugin}.log"
+            assert _should_delete_after_process(fname) is False, \
+                f"{fname} must NOT be deleted — SYNC Mode 1 still needs it"
+
+    def test_encoded_node_sync_file_flagged_for_deletion(self):
+        assert _should_delete_after_process("last_result.ARPSCAN.encoded.Node1.1.log") is True
+
+    def test_decoded_node_sync_file_flagged_for_deletion(self):
+        assert _should_delete_after_process("last_result.ARPSCAN.decoded.Node1.1.log") is True
+
+    def test_encoded_files_with_various_node_names_flagged(self):
+        for node in ("Node1", "Home_Hub", "Site_B", "OfficeNode"):
+            fname = f"last_result.ARPSCAN.encoded.{node}.1.log"
+            assert _should_delete_after_process(fname) is True, \
+                f"{fname} should be deleted after processing"
+
+    def test_decoded_files_with_various_node_names_flagged(self):
+        for node in ("Node1", "Home_Hub", "Site_B"):
+            fname = f"last_result.ARPSCAN.decoded.{node}.2.log"
+            assert _should_delete_after_process(fname) is True, \
+                f"{fname} should be deleted after processing"
+
     def test_empty_device_list_returns_zero(self, conn):
         assert sync_insert_devices(conn, [], existing_macs=set()) == 0
+
+
+# ===========================================================================
+# Mode 3 JSON-skip behaviour
+# Regression: local plugin result files (pipe-delimited) must not crash Mode 3.
+# ===========================================================================
+
+def _parse_sync_payload(file_path: str) -> list:
+    """Mirror of the json.load + data['data'] block in sync.main() Mode 3.
+
+    Returns the list of device dicts on success, or raises nothing on invalid
+    input — callers should catch JSONDecodeError / KeyError and skip the file.
+    """
+    with open(file_path, "r") as f:
+        data = json.load(f)
+    return data["data"]
+
+
+class TestMode3JsonSkip:
+    """Regression for the crash when Mode 3 encountered pipe-delimited plugin files.
+
+    Before the fix, sync.py called json.load() on every last_result.*.log file
+    returned by decode_and_rename_files(), including local plugin result files
+    (e.g. last_result.DIGSCAN.log) which are pipe-delimited and not JSON.  The
+    fix wraps the load in try/except(JSONDecodeError, KeyError) and continues.
+    """
+
+    def test_valid_sync_payload_is_parsed(self, tmp_path):
+        payload = {"data": [{"devMac": "aa:bb:cc:dd:ee:01", "devName": "TestDevice"}]}
+        f = tmp_path / "last_result.ARPSCAN.decoded.Node1.1.log"
+        f.write_text(json.dumps(payload))
+        result = _parse_sync_payload(str(f))
+        assert len(result) == 1
+        assert result[0]["devMac"] == "aa:bb:cc:dd:ee:01"
+
+    def test_pipe_delimited_file_raises_json_error(self, tmp_path):
+        """Pipe-delimited plugin file must raise JSONDecodeError so callers can skip it."""
+        f = tmp_path / "last_result.DIGSCAN.log"
+        f.write_text("aa:bb:cc:dd:ee:01|192.168.1.1|2026-01-01 00:00:00|hostname||subnet||DIGSCAN|||||\n")
+        with pytest.raises(json.JSONDecodeError):
+            _parse_sync_payload(str(f))
+
+    def test_json_without_data_key_raises_key_error(self, tmp_path):
+        """JSON that lacks the 'data' key must raise KeyError so callers can skip it."""
+        f = tmp_path / "last_result.UNKNOWN.log"
+        f.write_text(json.dumps({"result": []}))
+        with pytest.raises(KeyError):
+            _parse_sync_payload(str(f))
+
+    def test_empty_file_raises_json_error(self, tmp_path):
+        f = tmp_path / "last_result.EMPTY.log"
+        f.write_text("")
+        with pytest.raises(json.JSONDecodeError):
+            _parse_sync_payload(str(f))
+
+
+# ===========================================================================
+# SYNC_BEHAVIOR - three hub device-write modes (Mode 3 - RECEIVE)
+# ===========================================================================
+
+class TestSyncBehavior:
+    """Covers the three SYNC_BEHAVIOR modes for hub-side device writes.
+
+    copy-new     (default) — INSERT new MACs only, skip existing.
+    carbon-copy            — UPSERT all MACs; node values overwrite hub values.
+    hub-defaults           — skip direct write; let hub pipeline handle it.
+    """
+
+    # ------------------------------------------------------------------
+    # copy-new (default – backward compatible)
+    # ------------------------------------------------------------------
+
+    def test_copy_new_inserts_new_device(self, conn):
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01")
+        written = sync_insert_devices(conn, [device], existing_macs=set(), behavior="copy-new")
+        assert written == 1
+        cur = conn.cursor()
+        cur.execute("SELECT devMac FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:01",))
+        assert cur.fetchone() is not None
+
+    def test_copy_new_skips_existing_device(self, conn):
+        cur = conn.cursor()
+        cur.execute("INSERT INTO Devices (devMac, devName) VALUES (?, ?)", ("aa:bb:cc:dd:ee:01", "Original"))
+        conn.commit()
+
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01", devName="Updated")
+        written = sync_insert_devices(conn, [device], existing_macs={"aa:bb:cc:dd:ee:01"}, behavior="copy-new")
+        assert written == 0
+        cur.execute("SELECT devName FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:01",))
+        assert cur.fetchone()["devName"] == "Original"
+
+    def test_copy_new_only_new_in_mixed_batch(self, conn):
+        cur = conn.cursor()
+        cur.execute("INSERT INTO Devices (devMac, devName) VALUES (?, ?)", ("aa:bb:cc:dd:ee:existing", "Existing"))
+        conn.commit()
+
+        devices = [
+            make_device_dict(mac="aa:bb:cc:dd:ee:existing"),
+            make_device_dict(mac="aa:bb:cc:dd:ee:new1"),
+            make_device_dict(mac="aa:bb:cc:dd:ee:new2"),
+        ]
+        written = sync_insert_devices(conn, devices, existing_macs={"aa:bb:cc:dd:ee:existing"}, behavior="copy-new")
+        assert written == 2
+
+    # ------------------------------------------------------------------
+    # carbon-copy — UPSERT, node is authoritative
+    # ------------------------------------------------------------------
+
+    def test_carbon_copy_inserts_new_device(self, conn):
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01")
+        written = sync_insert_devices(conn, [device], behavior="carbon-copy")
+        assert written == 1
+        cur = conn.cursor()
+        cur.execute("SELECT devMac FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:01",))
+        assert cur.fetchone() is not None
+
+    def test_carbon_copy_overwrites_existing_device(self, conn):
+        cur = conn.cursor()
+        cur.execute("INSERT INTO Devices (devMac, devName) VALUES (?, ?)", ("aa:bb:cc:dd:ee:01", "OldName"))
+        conn.commit()
+
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01", devName="NewName")
+        written = sync_insert_devices(conn, [device], behavior="carbon-copy")
+        assert written == 1
+        cur.execute("SELECT devName FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:01",))
+        assert cur.fetchone()["devName"] == "NewName"
+
+    def test_carbon_copy_processes_all_devices_in_batch(self, conn):
+        cur = conn.cursor()
+        cur.execute("INSERT INTO Devices (devMac, devName) VALUES (?, ?)", ("aa:bb:cc:dd:ee:01", "OldName"))
+        conn.commit()
+
+        devices = [
+            make_device_dict(mac="aa:bb:cc:dd:ee:01", devName="UpdatedName"),
+            make_device_dict(mac="aa:bb:cc:dd:ee:02"),
+        ]
+        written = sync_insert_devices(conn, devices, behavior="carbon-copy")
+        assert written == 2
+
+        cur.execute("SELECT devName FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:01",))
+        assert cur.fetchone()["devName"] == "UpdatedName"
+
+    def test_carbon_copy_does_not_duplicate_existing_device(self, conn):
+        cur = conn.cursor()
+        cur.execute("INSERT INTO Devices (devMac, devName) VALUES (?, ?)", ("aa:bb:cc:dd:ee:01", "Original"))
+        conn.commit()
+
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01", devName="Updated")
+        sync_insert_devices(conn, [device], behavior="carbon-copy")
+
+        cur.execute("SELECT COUNT(*) AS cnt FROM Devices WHERE devMac = ?", ("aa:bb:cc:dd:ee:01",))
+        assert cur.fetchone()["cnt"] == 1
+
+    def test_carbon_copy_does_not_overwrite_devPresentLastScan(self, conn):
+        """Regression: carbon-copy must NOT clobber devPresentLastScan.
+
+        Scenario: device is online on the hub (devPresentLastScan=1) but the
+        node reports it as offline (devPresentLastScan=0).  Without the fix the
+        UPSERT would flip presence to 0, triggering a Device Down event on the
+        next scan cycle and a Connected event on the scan after that, causing
+        the device to accumulate enough churn events to be flagged as Flapping.
+        """
+        cur = conn.cursor()
+        # Hub already knows this device and currently sees it as online.
+        cur.execute(
+            "INSERT INTO Devices (devMac, devName, devPresentLastScan) VALUES (?, ?, ?)",
+            ("aa:bb:cc:dd:ee:01", "HubDevice", 1),
+        )
+        conn.commit()
+
+        # Node reports same MAC as offline.
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01", devPresentLastScan=0)
+        sync_insert_devices(conn, [device], behavior="carbon-copy")
+
+        cur.execute(
+            "SELECT devPresentLastScan FROM Devices WHERE devMac = ?",
+            ("aa:bb:cc:dd:ee:01",),
+        )
+        row = cur.fetchone()
+        assert row["devPresentLastScan"] == 1, (
+            "carbon-copy must not overwrite devPresentLastScan with a node's offline value"
+        )
+
+    # ------------------------------------------------------------------
+    # hub-defaults — no direct write, hub pipeline handles it
+    # ------------------------------------------------------------------
+
+    def test_hub_defaults_writes_nothing(self, conn):
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01")
+        written = sync_insert_devices(conn, [device], behavior="hub-defaults")
+        assert written == 0
+
+    def test_hub_defaults_leaves_db_empty(self, conn):
+        devices = [make_device_dict(mac=f"aa:bb:cc:dd:ee:0{i}") for i in range(3)]
+        sync_insert_devices(conn, devices, behavior="hub-defaults")
+        cur = conn.cursor()
+        cur.execute("SELECT COUNT(*) AS cnt FROM Devices")
+        assert cur.fetchone()["cnt"] == 0
+
+    def test_hub_defaults_returns_zero_for_empty_input(self, conn):
+        assert sync_insert_devices(conn, [], behavior="hub-defaults") == 0
+
+    # ------------------------------------------------------------------
+    # "New Device" events — copy-new and carbon-copy must fire; hub-defaults must not
+    # ------------------------------------------------------------------
+
+    def test_copy_new_fires_new_device_event(self, conn):
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01")
+        sync_insert_devices(conn, [device], existing_macs=set(), behavior="copy-new")
+        cur = conn.cursor()
+        cur.execute("SELECT COUNT(*) AS cnt FROM Events WHERE eveEventType='New Device' AND eveMac=?", ("aa:bb:cc:dd:ee:01",))
+        assert cur.fetchone()["cnt"] == 1
+
+    def test_copy_new_does_not_fire_event_for_existing_device(self, conn):
+        cur = conn.cursor()
+        cur.execute("INSERT INTO Devices (devMac) VALUES (?)", ("aa:bb:cc:dd:ee:01",))
+        conn.commit()
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01")
+        sync_insert_devices(conn, [device], existing_macs={"aa:bb:cc:dd:ee:01"}, behavior="copy-new")
+        cur.execute("SELECT COUNT(*) AS cnt FROM Events WHERE eveEventType='New Device'")
+        assert cur.fetchone()["cnt"] == 0
+
+    def test_carbon_copy_fires_new_device_event_for_new_mac(self, conn):
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01")
+        sync_insert_devices(conn, [device], existing_macs=set(), behavior="carbon-copy")
+        cur = conn.cursor()
+        cur.execute("SELECT COUNT(*) AS cnt FROM Events WHERE eveEventType='New Device' AND eveMac=?", ("aa:bb:cc:dd:ee:01",))
+        assert cur.fetchone()["cnt"] == 1
+
+    def test_carbon_copy_does_not_fire_event_for_existing_mac(self, conn):
+        cur = conn.cursor()
+        cur.execute("INSERT INTO Devices (devMac) VALUES (?)", ("aa:bb:cc:dd:ee:01",))
+        conn.commit()
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01", devName="Updated")
+        sync_insert_devices(conn, [device], existing_macs={"aa:bb:cc:dd:ee:01"}, behavior="carbon-copy")
+        cur.execute("SELECT COUNT(*) AS cnt FROM Events WHERE eveEventType='New Device'")
+        assert cur.fetchone()["cnt"] == 0
+
+    def test_hub_defaults_fires_no_events_directly(self, conn):
+        devices = [make_device_dict(mac=f"aa:bb:cc:dd:ee:0{i}") for i in range(3)]
+        sync_insert_devices(conn, devices, existing_macs=set(), behavior="hub-defaults")
+        cur = conn.cursor()
+        cur.execute("SELECT COUNT(*) AS cnt FROM Events WHERE eveEventType='New Device'")
+        assert cur.fetchone()["cnt"] == 0
+
+    def test_copy_new_fires_events_only_for_new_macs_in_mixed_batch(self, conn):
+        cur = conn.cursor()
+        cur.execute("INSERT INTO Devices (devMac) VALUES (?)", ("aa:bb:cc:dd:ee:existing",))
+        conn.commit()
+
+        devices = [
+            make_device_dict(mac="aa:bb:cc:dd:ee:existing"),
+            make_device_dict(mac="aa:bb:cc:dd:ee:new1"),
+            make_device_dict(mac="aa:bb:cc:dd:ee:new2"),
+        ]
+        sync_insert_devices(conn, devices, existing_macs={"aa:bb:cc:dd:ee:existing"}, behavior="copy-new")
+
+        cur.execute("SELECT eveMac FROM Events WHERE eveEventType='New Device'")
+        event_macs = {r["eveMac"] for r in cur.fetchall()}
+        assert event_macs == {"aa:bb:cc:dd:ee:new1", "aa:bb:cc:dd:ee:new2"}
+
+    def test_carbon_copy_fires_events_only_for_new_macs_in_mixed_batch(self, conn):
+        cur = conn.cursor()
+        cur.execute("INSERT INTO Devices (devMac, devName) VALUES (?, ?)", ("aa:bb:cc:dd:ee:existing", "Old"))
+        conn.commit()
+
+        devices = [
+            make_device_dict(mac="aa:bb:cc:dd:ee:existing", devName="Updated"),
+            make_device_dict(mac="aa:bb:cc:dd:ee:new1"),
+        ]
+        sync_insert_devices(conn, devices, existing_macs={"aa:bb:cc:dd:ee:existing"}, behavior="carbon-copy")
+
+        cur.execute("SELECT eveMac FROM Events WHERE eveEventType='New Device'")
+        event_macs = {r["eveMac"] for r in cur.fetchall()}
+        assert event_macs == {"aa:bb:cc:dd:ee:new1"}
+
+    def test_new_device_event_fields_are_correct(self, conn):
+        device = make_device_dict(mac="aa:bb:cc:dd:ee:01", devLastIP="10.0.0.1", devVendor="Acme")
+        sync_insert_devices(conn, [device], existing_macs=set(), behavior="copy-new")
+        cur = conn.cursor()
+        cur.execute(
+            "SELECT * FROM Events WHERE eveEventType='New Device' AND eveMac=?",
+            ("aa:bb:cc:dd:ee:01",),
+        )
+        row = cur.fetchone()
+        assert row is not None
+        assert row["eveMac"] == "aa:bb:cc:dd:ee:01"
+        assert row["eveIp"] == "10.0.0.1"
+        assert row["eveAdditionalInfo"] == "Acme"
+        assert row["evePendingAlertEmail"] == 1
+        # Confirm exactly one event was inserted (no duplicates).
+        cur.execute(
+            "SELECT COUNT(*) AS cnt FROM Events WHERE eveEventType='New Device' AND eveMac=?",
+            ("aa:bb:cc:dd:ee:01",),
+        )
+        assert cur.fetchone()["cnt"] == 1