From c21639b7f186d7842594e11d7a4de5abe89ba362 Mon Sep 17 00:00:00 2001 From: Mike Teehan Date: Mon, 13 Jul 2026 17:26:45 +0000 Subject: [PATCH] docs: add comprehensive docstrings and inline comments Add docstrings to all handler functions in daemon/handlers/firewall.py, covering params, return values, and raised exceptions. Add inline comments to _config_apply() reconciliation steps and the request body merge order. Add docstrings across lib/ modules for emit helpers (_emit_str, _emit_int, etc.), volatile stripping logic, two-layer diff strategy, sync event dispatch, and all cross-subsystem sync subscribers (DnsToFirewall, WgToFirewall, FirewallToDhcp, NetworkToAllSync). Document WireGuard/networkd config parsers and key-value mappers in system_import.py. Add docstrings to _ep(), Registry.decorator, setup_logging, and _replace helper across daemon/ and lib/. --- daemon/client.py | 9 ++ daemon/handlers/firewall.py | 264 +++++++++++++++++++++++++++++++++++- daemon/iface.py | 9 ++ daemon/server.py | 4 +- lib/logging.py | 9 ++ lib/network.py | 55 ++++++++ lib/nginx.py | 16 ++- lib/state.py | 38 ++++-- lib/sync.py | 77 +++++++++++ lib/system_import.py | 46 ++++++- 10 files changed, 510 insertions(+), 17 deletions(-) diff --git a/daemon/client.py b/daemon/client.py index 679349b..2d8a515 100644 --- a/daemon/client.py +++ b/daemon/client.py @@ -94,6 +94,15 @@ def _format_path(path: str, params: dict[str, Any] | None) -> str: return path def _replace(m: re.Match[str]) -> str: + """Regex callback that replaces ```` segments with URL-encoded values. + + Args: + m: Match object containing the parameter name. + + Returns: + URL-encoded value from params dict, or original text if key + not found. + """ key = m.group(1) if key in params: return urllib.parse.quote(str(params[key]), safe="") diff --git a/daemon/handlers/firewall.py b/daemon/handlers/firewall.py index dbb3f49..1c93fdc 100644 --- a/daemon/handlers/firewall.py +++ b/daemon/handlers/firewall.py @@ -107,7 +107,12 @@ def _get_forward_ports(zone_name: str) -> list[str]: def _config_apply() -> dict[str, Any]: - """Apply the declarative config to live firewalld.""" + """Apply saved declarative config to live firewalld. + + For each zone in the config, reconciles interfaces, services, target, + masquerade, rich rules, and forward ports by removing old values first, + then adding desired values. Reloads firewalld at the end. + """ from lib.firewall import get_config as _get_lib_config cfg = _get_lib_config() @@ -126,6 +131,8 @@ def _config_apply() -> dict[str, Any]: available = run(["firewall-cmd", "--get-zones"], sudo=True).split() applied: list[str] = [] for zone_name, zone_cfg in cfg_zones.items(): + # Two-step reconciliation: remove current values, then add desired values. + # This ensures idempotency — running apply twice produces the same result. need_create = zone_name not in available if need_create: @@ -155,6 +162,9 @@ def _config_apply() -> dict[str, Any]: check=False, ) + # Step 2: Reconcile services — remove all current, add desired list. + # Firewall-cmd doesn't have a "set-services" bulk operation, so we + # remove each existing service and then add each desired service. current_svcs: list[str] = [] with suppress(Exception): current_svcs = _parse_zone_output( @@ -183,6 +193,7 @@ def _config_apply() -> dict[str, Any]: sudo=True, ) + # Step 3: Reconcile interfaces — same remove-then-add pattern. current_ifaces: list[str] = [] with suppress(Exception): current_ifaces = _parse_zone_output( @@ -211,6 +222,7 @@ def _config_apply() -> dict[str, Any]: sudo=True, ) + # Step 4: Toggle masquerade if explicitly set (None means "don't change"). mq = zone_cfg.get("masquerade", False) if mq is not None: action = "--add-masquerade" if mq else "--remove-masquerade" @@ -219,6 +231,9 @@ def _config_apply() -> dict[str, Any]: sudo=True, ) + # Step 5: Reconcile rich rules — remove all current, add desired. + # Note: firewall-cmd doesn't track rule IDs for rich rules, so we + # must remove by full rule string match. current_rules = _parse_zone_output( zone_name, run(["firewall-cmd", f"--zone={zone_name}", "--list-all"], sudo=True), @@ -253,6 +268,7 @@ def _config_apply() -> dict[str, Any]: check=False, ) + # Step 6: Reconcile forward ports — same pattern. current_fps = _get_forward_ports(zone_name) for fp_str in current_fps: run( @@ -319,6 +335,16 @@ def get_interfaces(_request: Any, _body: Any) -> list[dict[str, Any]]: @registry.register(GET_FIREWALL_ZONES) def get_zones(_request: Any, _body: Any) -> dict[str, Any]: + """Return active + available zones from state store. + + Args: + _request: The incoming HTTP request (unused). + _body: The request body (unused). + + Returns: + Dict with ``active`` (zone→interfaces mapping) and + ``available`` (list of all zone names). + """ fw = _get_fw_state() active = fw.get("active_zones", {}) zones = fw.get("zones", {}) @@ -327,6 +353,19 @@ def get_zones(_request: Any, _body: Any) -> dict[str, Any]: @registry.register(GET_FIREWALL_ZONES_INFO) def get_zone_info(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Return zone config by name; ``NotFoundError`` if absent. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone`` key (zone name). + + Returns: + Zone config dict. + + Raises: + ValueError: If ``zone`` key is missing from body. + NotFoundError: If the specified zone does not exist. + """ if not body or "zone" not in body: raise ValueError("'zone' is required") zone = body["zone"] @@ -339,6 +378,15 @@ def get_zone_info(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: @registry.register(GET_FIREWALL_ZONES_ALL) def get_all_zones_info(_request: Any, _body: Any) -> list[dict[str, Any]]: + """Return list of all active zone configs. + + Args: + _request: The incoming HTTP request (unused). + _body: The request body (unused). + + Returns: + List of zone config dicts for all active zones. + """ fw = _get_fw_state() active = fw.get("active_zones", {}) zones = fw.get("zones", {}) @@ -351,17 +399,53 @@ def get_all_zones_info(_request: Any, _body: Any) -> list[dict[str, Any]]: @registry.register(GET_FIREWALL_SERVICES) def get_services(_request: Any, _body: Any) -> list[str]: + """Return list of available firewall services. + + Args: + _request: The incoming HTTP request (unused). + _body: The request body (unused). + + Returns: + List of service names available in firewalld. + """ fw = _get_fw_state() return fw.get("available_services", []) @registry.register(GET_FIREWALL_CONFIG) def get_config(_request: Any, _body: Any) -> dict[str, Any]: + """Return declarative config from JSON store. + + Args: + _request: The incoming HTTP request (unused). + _body: The request body (unused). + + Returns: + Full firewall config dict. + """ return _get_config() @registry.register(POST_FIREWALL_CONFIG) def save_config_handler(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Save the full declarative firewall configuration. + + Validates that the request body contains a ``zones`` dict, persists it + to the JSON store, emits a ``config_saved`` sync event to the + cross-subsystem sync bus, and refreshes the state store. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with a ``zones`` dict mapping zone names to + zone configurations. + + Returns: + Dict with ``config_saved`` flag set to ``True``. + + Raises: + ValueError: If body is empty, missing ``zones`` key, + or ``zones`` is not a dict. + """ if not body or "zones" not in body: raise ValueError("'zones' key is required") if not isinstance(body["zones"], dict): @@ -377,6 +461,18 @@ def save_config_handler(_request: Any, body: dict[str, Any] | None) -> dict[str, @registry.register(PATCH_FIREWALL_CONFIG) def patch_config(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Deep-merge body into current config, save, emit sync event, refresh state. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with partial config to merge. + + Returns: + Dict with ``config_saved`` flag set to ``True``. + + Raises: + ValueError: If body is empty. + """ if not body: raise ValueError("Request body must be a JSON object") from lib.common import deep_merge @@ -394,6 +490,19 @@ def patch_config(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: @registry.register(GET_FIREWALL_CONFIG_PENDING) def config_pending_handler(_request: Any, _body: Any) -> dict[str, Any]: + """Return pending changes with human-readable summaries. + + Compares saved config against live firewalld state and returns a list + of differences. + + Args: + _request: The incoming HTTP request (unused). + _body: The request body (unused). + + Returns: + Dict with ``pending`` (list of change dicts) and ``pending_summary`` + (human-readable strings). + """ fw = _get_fw_state() pending = fw.get("pending", {}) @@ -408,6 +517,16 @@ def config_pending_handler(_request: Any, _body: Any) -> dict[str, Any]: @registry.register(POST_FIREWALL_CONFIG_APPLY) def config_apply(_request: Any, _body: Any) -> dict[str, Any]: + """Apply pending config to live firewalld, emit sync event, refresh state. + + Args: + _request: The incoming HTTP request (unused). + _body: The request body (unused). + + Returns: + Dict with ``applied_zones`` (list of zone names), ``backup`` (path), + and ``synced`` (affected subsystems). + """ result = _config_apply() logger.info("Firewall config applied: %s", result.get("applied_zones", [])) sync_result = bus.emit( @@ -420,6 +539,18 @@ def config_apply(_request: Any, _body: Any) -> dict[str, Any]: @registry.register(POST_FIREWALL_ZONES_CREATE) def create_zone(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Create new zone via firewall-cmd, emit sync event, refresh state. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``name`` (zone name) and optional ``target``. + + Returns: + Dict with ``zone`` key set to the zone name. + + Raises: + ValueError: If body is empty, missing name, or zone already exists. + """ if not body: raise ValueError("Request body required") zone_name = body.get("name", "").strip() @@ -451,6 +582,19 @@ def create_zone(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: @registry.register(DELETE_FIREWALL_ZONES_DELETE) def delete_zone(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Delete zone via firewall-cmd, emit sync event, refresh state. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone`` (zone name). + + Returns: + Dict with ``zone`` key set to the zone name. + + Raises: + ValueError: If body is empty or missing ``zone``. + NotFoundError: If the zone does not exist. + """ if not body or "zone" not in body: raise ValueError("'zone' is required") zone = body["zone"] @@ -469,6 +613,19 @@ def delete_zone(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: @registry.register(POST_FIREWALL_ZONES_INTERFACES) def set_zone_interfaces(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Replace zone interfaces, reassigning interfaces from old zones. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone`` and ``interfaces`` list. + + Returns: + Dict with ``zone`` and ``interfaces`` keys. + + Raises: + ValueError: If body is missing ``zone``. + NotFoundError: If the zone does not exist. + """ if not body: raise ValueError("Request body required") zone = body.get("zone", "").strip() @@ -544,6 +701,19 @@ def set_zone_interfaces(_request: Any, body: dict[str, Any] | None) -> dict[str, @registry.register(POST_FIREWALL_ZONES_SERVICES) def set_zone_services(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Replace zone services, emitting sync event and refreshing state. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone`` and ``services`` list. + + Returns: + Dict with ``zone`` and ``services`` keys. + + Raises: + ValueError: If body is missing ``zone``. + NotFoundError: If the zone does not exist. + """ if not body: raise ValueError("Request body required") zone = body.get("zone", "").strip() @@ -586,6 +756,20 @@ def set_zone_services(_request: Any, body: dict[str, Any] | None) -> dict[str, A @registry.register(POST_FIREWALL_RICH_RULES_ADD) def add_rich_rule(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Add rich rule with auto-generated ID from uuid4. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone`` (zone name) and ``rule`` + (rich rule string). + + Returns: + Dict with ``zone``, ``id`` (generated UUID), and ``rule`` keys. + + Raises: + ValueError: If body is missing ``zone`` or ``rule``. + NotFoundError: If the specified zone does not exist. + """ if not body: raise ValueError("Request body required") zone = body.get("zone", "").strip() @@ -622,6 +806,19 @@ def add_rich_rule(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: @registry.register(DELETE_FIREWALL_RICH_RULES_REMOVE) def remove_rich_rule(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Remove rich rule by ID, emit sync event, refresh state. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone`` and ``id`` (rule ID). + + Returns: + Dict with ``zone`` and ``id`` keys. + + Raises: + ValueError: If body is missing ``zone`` or ``id``. + NotFoundError: If the zone or rule does not exist. + """ if not body: raise ValueError("Request body required") zone = body.get("zone", "").strip() @@ -665,6 +862,21 @@ def remove_rich_rule(_request: Any, body: dict[str, Any] | None) -> dict[str, An @registry.register(GET_FIREWALL_RICH_RULES) def list_rich_rules(_request: Any, body: dict[str, Any] | None) -> list[dict[str, Any]]: + """Return rich rules list for a zone from state store, matched with config IDs. + + Rules from the live firewall that match a config entry get their ID + included; rules without a config entry are returned without an ID. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone`` key. + + Returns: + List of dicts with ``id`` (if available) and ``rule`` keys. + + Raises: + ValueError: If ``zone`` key is missing from body. + """ if not body or "zone" not in body: raise ValueError("'zone' is required") zone = body["zone"] @@ -685,6 +897,19 @@ def list_rich_rules(_request: Any, body: dict[str, Any] | None) -> list[dict[str @registry.register(POST_FIREWALL_MASQUERADE) def set_masquerade(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Enable/disable masquerade on a zone. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone`` and ``enable`` (boolean). + + Returns: + Dict with ``zone`` and ``masquerade`` keys. + + Raises: + ValueError: If body is missing ``zone`` or ``enable``, or if + attempting to enable masquerade on the ``public`` zone. + """ if not body: raise ValueError("Request body required") zone = body.get("zone", "").strip() @@ -709,6 +934,19 @@ def set_masquerade(_request: Any, body: dict[str, Any] | None) -> dict[str, Any] @registry.register(POST_FIREWALL_FORWARD_PORT_ADD) def add_forward_port(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Add port forwarding rule with auto-generated ID. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone``, ``port``, ``proto``, and optionally + ``toaddr`` and ``toport``. + + Returns: + Dict with ``zone``, ``id``, ``port``, and ``proto`` keys. + + Raises: + ValueError: If body is missing required fields. + """ if not body: raise ValueError("Request body required") zone = body.get("zone", "").strip() @@ -757,6 +995,19 @@ def add_forward_port(_request: Any, body: dict[str, Any] | None) -> dict[str, An @registry.register(DELETE_FIREWALL_FORWARD_PORT_REMOVE) def remove_forward_port(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: + """Remove port forwarding rule. + + Args: + _request: The incoming HTTP request (unused). + body: JSON body with ``zone``, ``port``, and ``proto``. + + Returns: + Dict with ``zone``, ``port``, and ``proto`` keys. + + Raises: + ValueError: If body is missing required fields. + NotFoundError: If the zone or forward port does not exist. + """ if not body: raise ValueError("Request body required") zone = body.get("zone", "").strip() @@ -809,6 +1060,17 @@ def remove_forward_port(_request: Any, body: dict[str, Any] | None) -> dict[str, @registry.register(GET_FIREWALL_STATE) def get_state(_request: Any, _body: Any) -> dict[str, Any]: + """Return complete firewall state snapshot. + + Args: + _request: The incoming HTTP request (unused). + _body: The request body (unused). + + Returns: + Dict with ``active_zones``, ``interfaces``, ``available_services``, + ``zones``, ``rich_rules``, and ``timestamp`` keys. Returns empty + dict if state is not populated yet. + """ fw = _get_state() if fw is None: return {} diff --git a/daemon/iface.py b/daemon/iface.py index 6586ba0..7ac7884 100644 --- a/daemon/iface.py +++ b/daemon/iface.py @@ -29,6 +29,15 @@ PathLike = str | Endpoint def _ep(method: str, path: str) -> Endpoint: + """Construct a frozen (method, path) endpoint tuple. + + Args: + method: HTTP method string (e.g. ``'GET'``). + path: URL path pattern. + + Returns: + Endpoint tuple suitable for ``registry.register()`` and client calls. + """ return (method, path) diff --git a/daemon/server.py b/daemon/server.py index d7837a8..2d788a5 100644 --- a/daemon/server.py +++ b/daemon/server.py @@ -102,6 +102,7 @@ class Registry: method = ep_method def decorator(fn: Callable) -> Callable: + """Inner decorator that stores *fn* in the registry and attaches Handler metadata.""" self._routes[(method.upper(), path)] = fn # type: ignore[arg-type] fn._handler = Handler(method, path) # type: ignore[attr-defined,reportArgumentType] return fn @@ -236,7 +237,8 @@ async def _handle_request(request: web.Request) -> web.Response: # Build body — merge order (highest wins): path params > JSON body > query params. # Path params come from the URL path (e.g. /interfaces/eth0) and should not - # be overridable by body or query parameters. + # be overridable by body or query parameters. This prevents callers from + # spoofing path-scoped parameters via request body. body: dict[str, Any] | None = pat_params if pat_params else None if request.content_type == "application/json": try: diff --git a/lib/logging.py b/lib/logging.py index b3822f6..e326ca7 100644 --- a/lib/logging.py +++ b/lib/logging.py @@ -76,6 +76,15 @@ def setup_logging(level: str | None = None) -> None: """ def _open(self): + """Override to open log file with group-write permissions (0664). + + Temporarily clears umask to ensure the file is writable by both the + WebUI process (vacuum-wall user) and daemon process (vacuum-walld user) + when they share a group. On existing stale files, chmod's to 0664. + + Returns: + Open file object in append mode. + """ # Ensure group-write on an existing stale file (e.g. left by the # other process with a stricter umask at creation time). with contextlib.suppress(OSError): diff --git a/lib/network.py b/lib/network.py index c0ff4b8..d1502c8 100644 --- a/lib/network.py +++ b/lib/network.py @@ -103,30 +103,73 @@ def save_config(cfg: dict[str, Any]) -> None: def _emit_str(lines: list[str], key: str, py_key: str, d: dict[str, Any]) -> None: + """Append a ``Key=Value`` line to *lines* if *py_key* has a non-None value in *d*. + + Args: + lines: Target list to append rendered line to. + key: INI key name for the output line. + py_key: Python dict key to look up in *d*. + d: Config entry dict to extract value from. + """ v = d.get(py_key) if v is not None: lines.append(f"{key}={v}") def _emit_int(lines: list[str], key: str, py_key: str, d: dict[str, Any]) -> None: + """Append a ``Key=Value`` line to *lines* for integer values. + + Args: + lines: Target list to append rendered line to. + key: INI key name for the output line. + py_key: Python dict key to look up in *d*. + d: Config entry dict to extract value from. + """ v = d.get(py_key) if v is not None: lines.append(f"{key}={v}") def _emit_bool(lines: list[str], key: str, py_key: str, d: dict[str, Any]) -> None: + """Append a ``Key=yes/no`` line to *lines* if *py_key* has a non-None value in *d*. + + Args: + lines: Target list to append rendered line to. + key: INI key name for the output line. + py_key: Python dict key to look up in *d*. + d: Config entry dict to extract value from. + """ v = d.get(py_key) if v is not None: lines.append(f"{key}={'yes' if v else 'no'}") def _emit_bool_opt(lines: list[str], key: str, py_key: str, d: dict[str, Any]) -> None: + """Identical to :func:`_emit_bool` — kept for API compatibility. + + Args: + lines: Target list to append rendered line to. + key: INI key name for the output line. + py_key: Python dict key to look up in *d*. + d: Config entry dict to extract value from. + """ v = d.get(py_key) if v is not None: lines.append(f"{key}={'yes' if v else 'no'}") def _emit_any(lines: list[str], key: str, py_key: str, d: dict[str, Any]) -> None: + """Append a ``Key=Value`` line handling both bool and non-bool types. + + Boolean values are rendered as ``yes``/``no``; all other types are + stringified directly. + + Args: + lines: Target list to append rendered line to. + key: INI key name for the output line. + py_key: Python dict key to look up in *d*. + d: Config entry dict to extract value from. + """ v = d.get(py_key) if v is not None: if isinstance(v, bool): @@ -570,6 +613,18 @@ _IS_LOCAL = [ def _is_local_dns(addr: str) -> bool: + """Return ``True`` if *addr* falls within a local/private IP range. + + Checks loopback, RFC 1918 (10/8, 172.16/12, 192.168/16), link-local + (169.254/16), and their IPv6 equivalents (fc00::/7, fe80::/10). + + Args: + addr: IP address string to test. + + Returns: + ``True`` if the address is local/private, ``False`` otherwise. + Invalid addresses are treated as non-local. + """ try: ip = ipaddress.ip_address(addr) for net in _IS_LOCAL: diff --git a/lib/nginx.py b/lib/nginx.py index 061e261..9e05037 100644 --- a/lib/nginx.py +++ b/lib/nginx.py @@ -130,14 +130,23 @@ def _ensure_webui_backend(raw: dict[str, Any]) -> None: def _migrate_mgmt_domains(raw: dict[str, Any]) -> None: - """Migrate legacy management domains to backend references.""" + """Migrate legacy management domains to backend references. + + Legacy format: management domains had inline paths pointing to + 127.0.0.1:9090 (Flask) and 127.0.0.1:9091 (WebSocket). + New format: domains reference the "webui" backend by name. + + Detection heuristic: if both "/" path points to 127.0.0.1:9090 + (is_management) and "/ws" path points to 127.0.0.1:9091 + (is_websocket), the domain is a management domain and gets migrated. + """ backends = raw.get("backends", {}) if not backends.get("webui", {}).get("_migrated"): return domains = raw.setdefault("domains", {}) for _name, dom in list(domains.items()): if dom.get("backend") == "webui": - continue + continue # Already migrated if dom.get("application") == "webui": del dom["application"] paths = dom.get("paths", {}) @@ -145,12 +154,15 @@ def _migrate_mgmt_domains(raw: dict[str, Any]) -> None: ws = paths.get("/ws", {}) root_backend = root.get("backend", {}) ws_backend = ws.get("backend", {}) + # Check if root path points to Flask management backend is_mgmt_root = root.get("is_management") or ( root_backend.get("host") == "127.0.0.1" and root_backend.get("port") == 9090 ) + # Check if WS path points to WebSocket management backend is_mgmt_ws = ws.get("is_websocket") or ( ws_backend.get("host") == "127.0.0.1" and ws_backend.get("port") == 9091 ) + # If both match, migrate: set backend reference, remove inline paths/auth if is_mgmt_root and is_mgmt_ws: dom["backend"] = "webui" dom.pop("paths", None) diff --git a/lib/state.py b/lib/state.py index cd961d7..c51c406 100644 --- a/lib/state.py +++ b/lib/state.py @@ -277,16 +277,21 @@ def _strip_volatile( for k in pop_keys: stripped.pop(k, None) for vpath in volatile: - # Determine if this is a list-of-dicts pattern + # Determine if this path uses list-of-dicts pattern (e.g. "peers[].transfer"). + # The [] marker signals that the parent key holds a list of dicts, and we + # must strip the volatile sub-key from each dict in the list. list_marker = vpath.index("[]") if "[]" in vpath else -1 if list_marker != -1: - # Split into prefix (before []), item keys (after []) + # Split into prefix (path before []), item keys (path after []). + # e.g. "status.peers[].transfer_received" → prefix=["status","peers"], + # item_keys=["transfer_received"] prefix = vpath[:list_marker].split(".") item_keys = ( vpath[list_marker + 3 :].split(".") if list_marker + 3 < len(vpath) else [] ) + # Navigate to the list container via the prefix path parent = stripped for seg in prefix: if isinstance(parent, dict) and seg in parent: @@ -305,6 +310,7 @@ def _strip_volatile( continue for item in items: + # parent should now be a list; iterate each dict and strip sub-keys if isinstance(item, dict): curr = item for i, ik in enumerate(item_keys): @@ -316,7 +322,7 @@ def _strip_volatile( else: break else: - # Scalar/dict path + # Scalar/dict path: navigate via segments and set final key to None segments = vpath.split(".") parent = stripped for i, seg in enumerate(segments): @@ -338,26 +344,36 @@ def _diff_layers( ) -> tuple[bool, bool]: """Compare *old* and *new* state using two-layer diff. - Strips ``timestamp`` from both before comparing. + The two-layer strategy distinguishes between: + 1. Structural changes (config, topology) → triggers full client re-fetch + 2. Volatile changes (byte counters, timestamps) → triggers lightweight tick + + If structural data changed, volatile is suppressed (False) because the + structural change already triggers a full re-fetch, making the volatile + signal redundant. + + Args: + old: Previous state data, or ``None`` if not yet populated. + new: New state data from collector. + volatile: Frozenset of volatile field paths. Returns: - ``(structural_changed, volatile_changed)`` — - ``True`` means that layer differs between old and new. - - If structural data changed, volatile is always ``False`` - (the structural change already triggers a full re-fetch, so - the volatile signal is suppressed). + ``(structural_changed, volatile_changed)``. """ if old is None: return (True, True) - # Structural diff: compare with volatile/timestamp fields zeroed + # Structural diff: compare with volatile fields zeroed out, plus timestamp + # removed. If these differ, the configuration or topology has changed. pop_keys = frozenset(("timestamp",)) old_struct = _strip_volatile(old, volatile, pop_keys) new_struct = _strip_volatile(new, volatile, pop_keys) structural = old_struct != new_struct # Volatile diff: compare without timestamp + # Volatile diff: only relevant if structural is unchanged. Compare full + # data (minus timestamp). If this differs, only volatile fields changed + # (e.g. WireGuard transfer counters), and a lightweight tick suffices. volatile_changed = False if not structural: old_no_ts = {k: v for k, v in old.items() if k != "timestamp"} diff --git a/lib/sync.py b/lib/sync.py index b5acebf..93a9238 100644 --- a/lib/sync.py +++ b/lib/sync.py @@ -147,10 +147,14 @@ class EventBus: """Core dispatch logic (called within try/finally of _dispatch).""" result = SyncResult() + # Iterate subscribers for this (subsystem, action) pair. + # Each subscriber is called in registration order. for handler in self._subscribers.get((event.subsystem, event.action), []): try: sub_result = handler(event) except Exception: + # Error containment: subscriber failures are logged, never abort + # the originating handler or other subscribers. logger.warning( "Sync subscriber %s failed for %s.%s", _safe_name(handler), @@ -163,10 +167,14 @@ class EventBus: if sub_result is None: continue + # Accumulate results from this subscriber into the aggregate. result.affected_subsystems.extend(sub_result.affected_subsystems) result.changes.extend(sub_result.changes) result.applied = result.applied or sub_result.applied + # Cascade: for each affected subsystem (that isn't the source), + # emit a new event so downstream subscribers react. The cascade + # event carries _cascade=source so subscribers can detect loops. for affected in sub_result.affected_subsystems: if affected == event.subsystem: continue @@ -180,6 +188,8 @@ class EventBus: result.changes.extend(cascaded.changes) result.applied = result.applied or cascaded.applied + # Dedupe affected list before returning (cascade events may cause + # the same subsystem to appear multiple times). result.affected_subsystems = _dedupe(result.affected_subsystems) return result @@ -247,6 +257,25 @@ class DnsToFirewallSync: @classmethod def on_dnsmasq_config_saved(cls, event: SyncEvent) -> SyncResult | None: + """Sync subscriber: dnsmasq config_saved → update firewall zone services. + + When DHCP ranges are added/removed on interfaces, this subscriber + ensures the corresponding firewall zones have ``dhcp`` and ``dns`` + services enabled/disabled to match. Back-propagates: ensures + DHCP ranges carry the gateway (interface IP) so clients get + their default route. + + Skips processing if event originated as a cascade from ``firewall`` + to prevent infinite loops. + + Args: + event: Sync event with ``config_saved`` action from dnsmasq. + + Returns: + SyncResult listing firewall and dnsmasq as affected subsystems, + with human-readable change descriptions. ``None`` if skipped + due to cascade guard. + """ if event.payload.get("_cascade") == "firewall": return None @@ -438,6 +467,23 @@ class WgToFirewallSync: @classmethod def on_wireguard_config_saved(cls, event: SyncEvent) -> SyncResult | None: + """Sync subscriber: wireguard config_saved → update firewall config. + + When WireGuard is active (has peers and interface), this subscriber + ensures the ``vpn`` zone exists with the WG interface assigned, + masquerade enabled, UDP 51820 accept rule, and inter-zone rich rules + for peer allowed_ips subnets. When WireGuard becomes inactive, + cleans up WireGuard-created entries from the vpn zone. + + Skips processing if event originated as a cascade from ``firewall``. + + Args: + event: Sync event with ``config_saved`` action from wireguard. + + Returns: + SyncResult listing firewall as affected subsystem with change + descriptions. ``None`` if skipped due to cascade guard. + """ if event.payload.get("_cascade") == "firewall": return None @@ -556,6 +602,23 @@ class FirewallToDhcpSync: @classmethod def on_firewall_config_saved(cls, event: SyncEvent) -> SyncResult | None: + """Sync subscriber: firewall config_saved → sync dnsmasq DHCP ranges. + + Removes DHCP ranges whose interface no longer belongs to any firewall + zone. When masquerade is enabled on a zone, ensures DHCP ranges on + that zone's interfaces carry the gateway (interface IP). Logs warnings + for zones with dhcp service but no range. + + Skips processing if event originated as a cascade from ``dnsmasq``. + + Args: + event: Sync event with ``config_saved`` action from firewall. + + Returns: + SyncResult listing dnsmasq as affected subsystem when ranges were + modified, with change descriptions. ``None`` if skipped due to + cascade guard. + """ if event.payload.get("_cascade") == "dnsmasq": return None @@ -665,6 +728,20 @@ class NetworkToAllSync: @classmethod def on_network_config_saved(cls, event: SyncEvent) -> SyncResult | None: + """Sync subscriber: network config_saved → update firewall zone interfaces. + + Suggests DHCP ranges for static-IP interfaces without ranges. + Removes interfaces from firewall zones that are no longer present + in the network config. Logs warnings for interfaces not assigned + to any zone. + + Args: + event: Sync event with ``config_saved`` action from network. + + Returns: + SyncResult listing firewall as affected subsystem when zone + interfaces were modified, with change descriptions. + """ try: from lib.dnsmasq import get_config as _get_dnsmasq_cfg from lib.firewall import get_config as _get_fw_cfg diff --git a/lib/system_import.py b/lib/system_import.py index 7c44a2b..90043f6 100644 --- a/lib/system_import.py +++ b/lib/system_import.py @@ -253,7 +253,12 @@ def import_wireguard() -> bool: def _parse_wireguard_conf(text: str) -> dict[str, Any]: - """Parse wg-quick INI format into JSON config dict.""" + """Parse wg-quick INI format into JSON config dict. + + Uses a simple state machine: [Interface] section populates the interface + dict; each [Peer] section accumulates into current_peer until the next + section header triggers _flush_peer() to commit it. + """ interface: dict[str, Any] = { "name": "wg0", "listen_port": 51820, @@ -270,6 +275,15 @@ def _parse_wireguard_conf(text: str) -> dict[str, Any]: current_peer: dict[str, Any] | None = None def _flush_peer() -> None: + """Flush the current peer dict into the peers map if it has a public key. + + Resets ``current_peer`` and ``current_peer_name`` to ``None``, + preparing for the next [Peer] section. + + Note: + Only peers with a ``public_key`` are stored; sections without + a key (malformed or incomplete) are silently skipped. + """ nonlocal current_peer, current_peer_name if ( current_peer is not None @@ -416,7 +430,13 @@ def import_networkd() -> bool: def _parse_network_file(path: Path) -> dict[str, Any] | None: - """Parse a .network INI file into interface config dict.""" + """Parse a .network INI file into interface config dict. + + State machine: [Match] section is skipped; [Link] keys go to iface["link"]; + [Network] keys go directly on iface. Numbered sections ([Address#N], [Route#N]) + accumulate into cur_addr / cur_route dicts until a section boundary triggers + _flush() to commit them into the corresponding list. + """ text = path.read_text() iface: dict[str, Any] = {} cur_section: str | None = None @@ -424,6 +444,7 @@ def _parse_network_file(path: Path) -> dict[str, Any] | None: cur_route: dict[str, Any] | None = None def _flush() -> None: + """Commit accumulated address/route dicts into the iface lists.""" nonlocal cur_addr, cur_route if cur_addr is not None: if "address" in cur_addr and len(cur_addr) == 1: @@ -541,6 +562,16 @@ def _parse_network_section( def _set_link_key(link: dict[str, Any], key: str, val: str) -> None: + """Parse a [Link] section key-value pair and set the corresponding config field. + + Maps systemd-networkd Link INI keys to snake_case config keys. + Boolean keys (ARP, Multicast, etc.) are auto-converted via ``_parse_bool``. + + Args: + link: Link config dict to populate. + key: INI key name from the .network file. + val: Value string from the .network file. + """ if key == "MTUBytes": parsed = _safe_int(val) if isinstance(parsed, int): @@ -563,6 +594,17 @@ def _set_link_key(link: dict[str, Any], key: str, val: str) -> None: def _set_network_key(iface: dict[str, Any], key: str, val: str) -> None: + """Parse a [Network] section key-value pair and set the corresponding config field. + + Maps systemd-networkd Network INI keys to snake_case config dict keys. + Comma-separated values (DNS, Domains, etc.) are split into lists. + Boolean and integer keys are auto-converted. + + Args: + iface: Interface config dict to populate. + key: INI key name from the .network file. + val: Value string from the .network file. + """ if key == "DHCP": iface["dhcp"] = val elif key == "Gateway":