refactor: daemon collectors, thin webui proxies, pure config reads

- move state collectors from lib/state.py to daemon/collectors/ (7 modules, registration side-effect; daemon/server.py imports the package before the first populate()) - webui/api: new daemon_route() decorator factory in common.py collapses the try/except daemon-proxy boilerplate in all 8 blueprints (rules/params/body/transform keep responses identical) - firewall: interface-coverage invariant — config is the source of truth for zone interfaces (absent key = empty, no hands-off zones); pure validate_coverage() enforced at save (400) and apply (409, force: true overrides), top-level `unmanaged` exemption - lib: get_config() reads are now pure (no dir creation or writes); new lib/bootstrap.py creates runtime dirs and persists the one-shot nginx legacy migration at daemon start, after system_import (lib.nginx.migrate_config_file) - lib/common: compute_pending() apply-bookkeeping helper - daemon: emit_and_refresh() handler helper; refresh_state(bump=) so /status/refresh no longer bumps versions (poll/mutation only) - acme: move --log last so acme.sh never treats a real arg as the log-file argument - docs: AGENTS.md, config.md, state-model.md, api.md updated; HARDEN.md dropped (plan implemented); apply-confirm force wording Tests: 917 passed; ruff check + format clean.
2026-09-03 00:40:56 +00:00
parent 89b64960f3
commit faa076370d
49 changed files with 2834 additions and 3821 deletions
@@ -5,6 +5,7 @@ from unittest.mock import MagicMock, call, patch

 import pytest

+from daemon.handlers import common as daemoncommon
 from daemon.handlers import firewall as daemonfirewall
 from daemon.server import ConflictError, NotFoundError
 from lib import firewall
@@ -377,25 +378,26 @@ _PENDING_LIVE_PUBLIC = {


 class TestComputePendingChangesAbsentInterfaces:
-    """Zones whose config lacks the 'interfaces' key are hands-off on apply,
-    so their interfaces diff must not be reported; other field drift is."""
+    """The config is the source of truth for zone interfaces: an absent
+    'interfaces' key counts as an empty list, so every config zone is
+    diffed on interfaces (no hands-off zones)."""

-    def test_services_drift_reported_without_interfaces_key(self):
+    def test_services_and_interfaces_drift_reported_without_interfaces_key(self):
        cfg = {"zones": {"public": {"services": ["http", "ssh"]}}}
        result = firewall._compute_pending_changes(
            cfg, {"public": _PENDING_LIVE_PUBLIC}
        )
        types = {c["type"] for c in result["pending"]}
        assert "services" in types
-        assert "interfaces" not in types
+        # Absent key counts as an empty list: live eth0 is a pending removal.
+        assert "interfaces" in types

-    def test_no_spurious_interfaces_entry_for_absent_key_zone(self):
-        # Config in sync on everything except a missing interfaces key: the
-        # zone's live interfaces are intentionally left alone by apply.
+    def test_absent_key_zone_in_sync_live_reports_nothing(self):
+        # Config lacks the interfaces key and the live zone has no
+        # interfaces either — absent key equals the empty live set.
+        live = {**_PENDING_LIVE_PUBLIC, "interfaces": []}
        cfg = {"zones": {"public": {"services": ["http"]}}}
-        result = firewall._compute_pending_changes(
-            cfg, {"public": _PENDING_LIVE_PUBLIC}
-        )
+        result = firewall._compute_pending_changes(cfg, {"public": live})
        assert result["pending"] == []
        assert result["needs_apply"] is False

@@ -457,6 +459,51 @@ class TestTargetDriftSemantics:
 # ---------------------------------------------------------------------------


+class TestValidateCoverage:
+    def test_all_covered(self):
+        fw = {"zones": {"public": {"interfaces": ["eth0"]}}}
+        net = {"interfaces": {"eth0": {}}}
+        assert firewall.validate_coverage(fw, net) == []
+
+    def test_uncovered_reported_sorted(self):
+        fw = {"zones": {"public": {"interfaces": ["eth1"]}}}
+        net = {"interfaces": {"eth5": {}, "eth0": {}}}
+        assert firewall.validate_coverage(fw, net) == ["eth0", "eth5"]
+
+    def test_unmanaged_exempts(self):
+        fw = {
+            "zones": {"public": {"interfaces": ["eth1"]}},
+            "unmanaged": ["eth0"],
+        }
+        net = {"interfaces": {"eth0": {}, "eth1": {}}}
+        assert firewall.validate_coverage(fw, net) == []
+
+    def test_lo_and_wg_exempt(self):
+        fw = {"zones": {}}
+        net = {"interfaces": {"lo": {}, "wg0": {}, "wg-full": {}}}
+        assert firewall.validate_coverage(fw, net) == []
+
+    def test_absent_key_counts_as_empty(self):
+        # A zone without an 'interfaces' key covers nothing.
+        fw = {"zones": {"public": {"services": ["http"]}}}
+        net = {"interfaces": {"eth0": {}}}
+        assert firewall.validate_coverage(fw, net) == ["eth0"]
+
+    def test_empty_network_config(self):
+        assert firewall.validate_coverage({"zones": {}}, {"interfaces": {}}) == []
+        assert firewall.validate_coverage({"zones": {}}, {}) == []
+
+    def test_non_dict_zone_and_non_list_unmanaged_ignored(self):
+        fw = {"zones": {"public": "oops"}, "unmanaged": "eth0"}
+        net = {"interfaces": {"eth0": {}}}
+        assert firewall.validate_coverage(fw, net) == ["eth0"]
+
+    def test_non_string_entries_ignored(self):
+        fw = {"zones": {"public": {"interfaces": [None, 7]}}, "unmanaged": [None]}
+        net = {"interfaces": {"eth0": {}}}
+        assert firewall.validate_coverage(fw, net) == ["eth0"]
+
+
 class TestGetZoneInfo:
    def test_parses_zone_info(self):
        result = firewall._parse_zone_output(
@@ -656,7 +703,7 @@ class TestDaemonConfigApply:
                "daemon.handlers.firewall._get_state",
                return_value={"zones": {"public": {}}},
            ),
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch("daemon.handlers.common.refresh_state"),
            patch(
                "daemon.handlers.firewall._get_config",
                return_value={"zones": {"public": {}}},
@@ -702,8 +749,8 @@ class TestDaemonMgmtLockoutGuard:
            patch.object(daemonfirewall, "_reload"),
            patch.object(daemonfirewall, "_get_config", return_value={"zones": {}}),
            patch.object(daemonfirewall, "_save_config") as mock_save,
-            patch.object(daemonfirewall, "bus") as mock_bus,
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch.object(daemoncommon, "bus") as mock_bus,
+            patch("daemon.handlers.common.refresh_state"),
        ):
            mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
            result = daemonfirewall.set_zone_services(
@@ -723,8 +770,8 @@ class TestDaemonMgmtLockoutGuard:
            patch.object(daemonfirewall, "_reload"),
            patch.object(daemonfirewall, "_get_config", return_value={"zones": {}}),
            patch.object(daemonfirewall, "_save_config"),
-            patch.object(daemonfirewall, "bus") as mock_bus,
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch.object(daemoncommon, "bus") as mock_bus,
+            patch("daemon.handlers.common.refresh_state"),
        ):
            mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
            result = daemonfirewall.set_zone_services(
@@ -791,7 +838,7 @@ class TestDaemonMgmtLockoutGuard:
                "daemon.handlers.firewall._get_state",
                return_value={"zones": {"public": {}}},
            ),
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch("daemon.handlers.common.refresh_state"),
            patch(
                "daemon.handlers.firewall._get_config",
                return_value={"zones": {"public": {}}},
@@ -803,8 +850,10 @@ class TestDaemonMgmtLockoutGuard:


 # ---------------------------------------------------------------------------
-# Interface-coverage guard: apply must not leave a network-managed interface
-# in no zone (clients lose connectivity/DHCP) unless forced.
+# Coverage invariant: the config must cover every network-managed
+# interface (or declare it unmanaged). Pure config check — the config is
+# the source of truth for zone interfaces (absent key = empty), so there
+# is no live-state comparison and no hands-off zones.
 # ---------------------------------------------------------------------------


@@ -849,7 +898,7 @@ def _apply_with(
        ) as mock_run,
        patch("daemon.handlers.firewall._default_zone", return_value="internal"),
        patch("daemon.handlers.firewall._save_backup", return_value=backup),
-        patch("daemon.handlers.firewall.refresh_state"),
+        patch("daemon.handlers.common.refresh_state"),
        patch(
            "daemon.handlers.firewall._get_config",
            return_value=deepcopy(cfg),
@@ -860,21 +909,88 @@ def _apply_with(
    return result, mock_run


-class TestDaemonInterfaceCoverageGuard:
-    def test_absent_key_zone_keeps_live_interfaces_on_apply(self):
-        cfg = {"zones": {"public": {"services": ["http"], "masquerade": False}}}
+class TestDaemonCoverageInvariant:
+    def test_conflict_when_network_iface_uncovered(self):
+        cfg = {"zones": {"public": {"interfaces": ["eth1"], "services": []}}}
+        with (
+            patch("lib.network.get_config", return_value={"interfaces": {"eth0": {}}}),
+            patch("lib.firewall.get_config", return_value=cfg, create=True),
+            patch("daemon.handlers.firewall.run") as mock_run,
+            patch("daemon.handlers.firewall._default_zone", return_value="internal"),
+            patch("daemon.handlers.firewall._save_backup") as mock_backup,
+            pytest.raises(ConflictError) as exc,
+        ):
+            daemonfirewall._config_apply()
+        msg = str(exc.value)
+        assert "eth0" in msg
+        assert "unmanaged" in msg
+        assert "force" in msg
+        mock_backup.assert_not_called()
+        # Pure config check: the guard performs no live-state reads at all.
+        mock_run.assert_not_called()
+
+    def test_unmanaged_exempts_iface(self):
+        cfg = {
+            "zones": {"public": {"interfaces": ["eth1"], "services": []}},
+            "unmanaged": ["eth0"],
+        }
        result, mock_run = _apply_with(cfg, {"eth0": {}}, "public\n  eth0\n")
        assert result["applied_zones"] == ["public"]
-        # The guard reads live zones once, up front.
-        assert mock_run.call_args_list[0].args[0] == [
+        cmds = [c.args[0] for c in mock_run.call_args_list]
+        # Live eth0 is removed, config eth1 added exactly once (permanent).
+        assert [
            "firewall-cmd",
-            "--get-active-zones",
-        ]
-        # Hands off: no interface mutation commands for the absent-key zone.
-        for c in mock_run.call_args_list:
-            for arg in c.args[0]:
-                assert not arg.startswith("--remove-interface=")
-                assert not arg.startswith("--add-interface=")
+            "--zone=public",
+            "--remove-interface=eth0",
+            "--permanent",
+        ] in cmds
+        assert (
+            cmds.count(
+                ["firewall-cmd", "--zone=public", "--add-interface=eth1", "--permanent"]
+            )
+            == 1
+        )
+
+    def test_absent_key_zone_counts_as_empty(self):
+        # No hands-off zones: a zone without an 'interfaces' key covers
+        # nothing, so a managed interface left out of every zone blocks.
+        cfg = {"zones": {"public": {"services": ["http"], "masquerade": False}}}
+        with (
+            patch("lib.network.get_config", return_value={"interfaces": {"eth0": {}}}),
+            patch("lib.firewall.get_config", return_value=cfg, create=True),
+            patch("daemon.handlers.firewall.run"),
+            patch("daemon.handlers.firewall._default_zone", return_value="internal"),
+            pytest.raises(ConflictError) as exc,
+        ):
+            daemonfirewall._config_apply()
+        assert "eth0" in str(exc.value)
+
+    def test_live_only_zone_does_not_count_as_covered(self):
+        # eth1 is held by 'guest' live but the config (the source of
+        # truth) does not cover it — apply is blocked regardless of live.
+        cfg = {"zones": {"public": {"interfaces": [], "services": []}}}
+        with (
+            patch("lib.network.get_config", return_value={"interfaces": {"eth1": {}}}),
+            patch("lib.firewall.get_config", return_value=cfg, create=True),
+            patch(
+                "daemon.handlers.firewall.run",
+                side_effect=_make_run("public\n  eth0\nguest\n  eth1\n"),
+            ),
+            patch("daemon.handlers.firewall._default_zone", return_value="internal"),
+            pytest.raises(ConflictError),
+        ):
+            daemonfirewall._config_apply()
+
+    def test_force_bypasses_coverage_guard(self):
+        cfg = {"zones": {"public": {"interfaces": ["eth1"], "services": []}}}
+        result, _ = _apply_with(cfg, {"eth0": {}}, "public\n  eth0\n", force=True)
+        assert result["applied_zones"] == ["public"]
+
+    def test_guard_ignores_lo_and_wg(self):
+        # lo/wg* are never guarded even though the network config carries them.
+        cfg = {"zones": {"public": {"interfaces": [], "services": []}}}
+        result, _ = _apply_with(cfg, {"lo": {}, "wg0": {}}, "public\n  eth0\n")
+        assert result["applied_zones"] == ["public"]

    def test_explicit_empty_list_unassigns(self):
        cfg = {"zones": {"public": {"interfaces": [], "services": []}}}
@@ -891,60 +1007,80 @@ class TestDaemonInterfaceCoverageGuard:
            any(a.startswith("--add-interface=") for a in cmd) for cmd in cmds
        )

-    def test_conflict_when_network_iface_goes_uncovered(self):
-        cfg = {"zones": {"public": {"interfaces": ["eth1"], "services": []}}}
+
+class TestDaemonSaveCoverageValidation:
+    """The coverage invariant is enforced at save time too (POST/PATCH
+    /firewall/config), so bad configs are rejected before they are written."""
+
+    def test_save_blocks_uncovered(self):
+        body = {"zones": {"public": {"interfaces": ["eth1"], "services": []}}}
        with (
            patch("lib.network.get_config", return_value={"interfaces": {"eth0": {}}}),
-            patch("lib.firewall.get_config", return_value=cfg, create=True),
-            patch(
-                "daemon.handlers.firewall.run",
-                side_effect=_make_run("public\n  eth0\n"),
-            ),
-            patch("daemon.handlers.firewall._default_zone", return_value="internal"),
-            pytest.raises(ConflictError) as exc,
+            patch("daemon.handlers.firewall._save_config") as mock_save,
+            pytest.raises(ValueError) as exc,
        ):
-            daemonfirewall._config_apply()
+            daemonfirewall.save_config_handler(None, body)
        assert "eth0" in str(exc.value)
-        assert "force" in str(exc.value)
+        assert "unmanaged" in str(exc.value)
+        mock_save.assert_not_called()

-    def test_force_bypasses_coverage_guard(self):
-        cfg = {"zones": {"public": {"interfaces": ["eth1"], "services": []}}}
-        result, _ = _apply_with(cfg, {"eth0": {}}, "public\n  eth0\n", force=True)
-        assert result["applied_zones"] == ["public"]
-
-    def test_guard_ignores_lo_and_wg(self):
-        # lo/wg* are never guarded even though the network config carries them.
-        cfg = {"zones": {"public": {"interfaces": [], "services": []}}}
-        result, _ = _apply_with(cfg, {"lo": {}, "wg0": {}}, "public\n  eth0\n")
-        assert result["applied_zones"] == ["public"]
-
-    def test_live_only_zone_interfaces_count_as_covered(self):
-        # eth1 is held by 'guest', which is live but absent from the config —
-        # apply never touches it, so eth1 counts as covered.
-        cfg = {"zones": {"public": {"interfaces": [], "services": []}}}
-        result, _ = _apply_with(cfg, {"eth1": {}}, "public\n  eth0\nguest\n  eth1\n")
-        assert result["applied_zones"] == ["public"]
-
-    def test_coverage_guard_conflict_writes_no_backup(self):
-        # Guard conflict must be side-effect free, like the lockout conflict.
-        cfg = {"zones": {"public": {"interfaces": ["eth1"], "services": []}}}
+    def test_save_allows_unmanaged(self):
+        body = {
+            "zones": {"public": {"interfaces": ["eth1"], "services": []}},
+            "unmanaged": ["eth0"],
+        }
        with (
            patch("lib.network.get_config", return_value={"interfaces": {"eth0": {}}}),
-            patch("lib.firewall.get_config", return_value=cfg, create=True),
-            patch(
-                "daemon.handlers.firewall.run",
-                side_effect=_make_run("public\n  eth0\n"),
-            ) as mock_run,
-            patch("daemon.handlers.firewall._default_zone", return_value="internal"),
-            patch("daemon.handlers.firewall._save_backup") as mock_backup,
-            pytest.raises(ConflictError),
+            patch("daemon.handlers.firewall._save_config") as mock_save,
+            patch.object(daemoncommon, "bus") as mock_bus,
+            patch("daemon.handlers.common.refresh_state"),
        ):
-            daemonfirewall._config_apply()
-        mock_backup.assert_not_called()
-        # Only the guard's live-zone read ran — no mutation commands at all.
-        assert [c.args[0] for c in mock_run.call_args_list] == [
-            ["firewall-cmd", "--get-active-zones"]
-        ]
+            mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
+            result = daemonfirewall.save_config_handler(None, body)
+        assert result == {"config_saved": True}
+        mock_save.assert_called_once()
+
+    def test_save_rejects_non_list_unmanaged(self):
+        body = {"zones": {"public": {"interfaces": ["eth0"]}}, "unmanaged": "eth0"}
+        with (
+            patch("lib.network.get_config", return_value={"interfaces": {"eth0": {}}}),
+            pytest.raises(ValueError) as exc,
+        ):
+            daemonfirewall.save_config_handler(None, body)
+        assert "unmanaged" in str(exc.value)
+
+    def test_patch_blocks_merge_that_uncovers(self):
+        current = {"zones": {"public": {"interfaces": ["eth0"], "services": []}}}
+        body = {"zones": {"public": {"interfaces": ["eth1"]}}}
+        with (
+            patch("lib.network.get_config", return_value={"interfaces": {"eth0": {}}}),
+            patch(
+                "daemon.handlers.firewall._get_config", return_value=deepcopy(current)
+            ),
+            patch("daemon.handlers.firewall._save_config") as mock_save,
+            pytest.raises(ValueError) as exc,
+        ):
+            daemonfirewall.patch_config(None, body)
+        assert "eth0" in str(exc.value)
+        mock_save.assert_not_called()
+
+    def test_patch_allows_merge_that_covers(self):
+        current = {"zones": {"public": {"interfaces": ["eth0"], "services": []}}}
+        body = {"zones": {"internal": {"interfaces": ["eth1"], "services": []}}}
+        with (
+            patch("lib.network.get_config", return_value={"interfaces": {"eth0": {}}}),
+            patch(
+                "daemon.handlers.firewall._get_config", return_value=deepcopy(current)
+            ),
+            patch("daemon.handlers.firewall._save_config") as mock_save,
+            patch.object(daemoncommon, "bus") as mock_bus,
+            patch("daemon.handlers.common.refresh_state"),
+        ):
+            mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
+            result = daemonfirewall.patch_config(None, body)
+        assert result == {"config_saved": True}
+        saved = mock_save.call_args[0][0]
+        assert set(saved["zones"]) == {"public", "internal"}


 # ---------------------------------------------------------------------------
@@ -957,8 +1093,8 @@ class TestDaemonCreateZone:
        with (
            patch("daemon.handlers.firewall.run", return_value=run_return) as mock_run,
            patch.object(daemonfirewall, "_reload"),
-            patch.object(daemonfirewall, "bus") as mock_bus,
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch.object(daemoncommon, "bus") as mock_bus,
+            patch("daemon.handlers.common.refresh_state"),
        ):
            mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
            result = daemonfirewall.create_zone(None, body)
@@ -1023,7 +1159,7 @@ class TestDaemonConfigApplyBackup:
                "daemon.handlers.firewall._save_backup",
                return_value="/tmp/rules.json",
            ) as mock_backup,
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch("daemon.handlers.common.refresh_state"),
            patch("daemon.handlers.firewall._get_config", return_value=deepcopy(cfg)),
            patch("daemon.handlers.firewall._save_config"),
        ):
@@ -1082,7 +1218,7 @@ class TestDaemonConfigApplyStamp:
                "daemon.handlers.firewall._get_state",
                return_value={"zones": {"public": {}}},
            ),
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch("daemon.handlers.common.refresh_state"),
            patch(
                "daemon.handlers.firewall._get_config",
                return_value=deepcopy(_STAMP_TEST_CFG),
@@ -1114,8 +1250,8 @@ class TestDaemonMutatorBaselineStamp:
    def test_set_zone_interfaces_stamps_baseline(self, mock_run, mock_cfg, mock_reload):
        with (
            patch.object(daemonfirewall, "_save_config") as mock_save,
-            patch.object(daemonfirewall, "bus") as mock_bus,
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch.object(daemoncommon, "bus") as mock_bus,
+            patch("daemon.handlers.common.refresh_state"),
        ):
            mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
            daemonfirewall.set_zone_interfaces(
@@ -1138,8 +1274,8 @@ class TestDaemonMutatorBaselineStamp:
                daemonfirewall, "_parse_zone_output", return_value={"services": []}
            ),
            patch.object(daemonfirewall, "_save_config") as mock_save,
-            patch.object(daemonfirewall, "bus") as mock_bus,
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch.object(daemoncommon, "bus") as mock_bus,
+            patch("daemon.handlers.common.refresh_state"),
        ):
            mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
            daemonfirewall.set_zone_services(
@@ -1162,8 +1298,8 @@ class TestDaemonMutatorBaselineStamp:
    ):
        with (
            patch.object(daemonfirewall, "_save_config") as mock_save,
-            patch.object(daemonfirewall, "bus") as mock_bus,
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch.object(daemoncommon, "bus") as mock_bus,
+            patch("daemon.handlers.common.refresh_state"),
        ):
            mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
            daemonfirewall.set_masquerade(None, {"zone": "internal", "enable": True})
@@ -1182,8 +1318,8 @@ class TestDaemonMutatorBaselineStamp:
        would manufacture spurious service diffs on the next poll."""
        with (
            patch.object(daemonfirewall, "_save_config") as mock_save,
-            patch.object(daemonfirewall, "bus") as mock_bus,
-            patch("daemon.handlers.firewall.refresh_state"),
+            patch.object(daemoncommon, "bus") as mock_bus,
+            patch("daemon.handlers.common.refresh_state"),
        ):
            mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
            daemonfirewall.set_masquerade(None, {"zone": "public", "enable": False})
@@ -1208,10 +1344,10 @@ class TestDaemonGetConfigEndpoint:
        "daemon.handlers.firewall._config_apply",
        return_value={"applied_zones": ["public"], "backup": "/tmp/rules.json"},
    )
-    @patch("daemon.handlers.firewall.bus")
+    @patch("daemon.handlers.common.bus")
    def test_config_apply_handler_force_propagation(self, mock_bus, mock_apply):
        mock_bus.emit.return_value = MagicMock(affected_subsystems=[])
-        with patch("daemon.handlers.firewall.refresh_state"):
+        with patch("daemon.handlers.common.refresh_state"):
            daemonfirewall.config_apply(None, None)
            mock_apply.assert_called_once_with(force=False)
            mock_apply.reset_mock()