From ac52918df5d327a10aecbeff337d2ba06617c4dc Mon Sep 17 00:00:00 2001 From: Mike Teehan Date: Fri, 28 Aug 2026 23:38:21 +0000 Subject: [PATCH] firewall: interface-coverage apply guard, target drift, non-destructive DHCP sync Post-DHCP-incident hardening per HARDEN.md. - apply guard: refuse (ConflictError, `force` overrides) when a network-managed interface would end up in no zone; absent `interfaces` key = hands-off, explicit `[]` = unassign-all - surface `uncovered_interfaces` in firewall state (lo/wg* filtered) + advisory in /api/status/pending; zones.js banner + interfaces-picker last-zone confirm - target drift (Option A): absent or default-normalizing target is unmanaged: not diffed, never re-set by apply; create_zone runs --new-zone first and sets non-default targets only; importer omits the target key for default zones - FirewallToDhcpSync keeps stale DHCP ranges and flags them instead of deleting; `dnsmasq` affected only on a real gateway mutation - real pre-apply recovery snapshot in data/firewall/rules.json ({timestamp, default_zone, zones, config}); drop the empty post-apply skeleton - daemon shutdown: bounded grace for in-flight tasks + suppressed teardown exception noise on SIGTERM - also carries the firewall service-descriptions feature (get_service_descriptions + service_descriptions state field + UI) - tests + docs across firewall/status/state/sync/schema; ruff clean, 867 passing --- HARDEN.md | 289 +++++++++++++++ daemon/handlers/firewall.py | 193 +++++++--- daemon/handlers/status.py | 17 + daemon/server.py | 42 ++- docs/api.md | 9 +- docs/architecture.md | 4 +- docs/config.md | 8 +- docs/hoover.md | 13 + docs/overview.md | 2 +- docs/state-model.md | 10 +- lib/firewall.py | 150 ++++++-- lib/schema.py | 6 + lib/state.py | 20 + lib/sync.py | 48 +-- lib/system_import.py | 18 +- tests/test_api.py | 51 +++ tests/test_firewall.py | 468 ++++++++++++++++++++++++ tests/test_schema_types.py | 19 +- tests/test_state.py | 24 ++ tests/test_status_pending.py | 56 +++ tests/test_sync.py | 49 +-- tests/test_system_import.py | 28 +- webui/static/hoover/components/modal.js | 138 ++++++- webui/static/pages/zones.js | 60 ++- webui/static/style.css | 123 +++++++ 25 files changed, 1677 insertions(+), 168 deletions(-) create mode 100644 HARDEN.md diff --git a/HARDEN.md b/HARDEN.md new file mode 100644 index 0000000..cf947ae --- /dev/null +++ b/HARDEN.md @@ -0,0 +1,289 @@ +# Hardening Plan — Firewall Zone Handling (post DHCP-incident) + +Status: ready to implement (line refs re-verified against code 2026-08-28; review +corrections applied 2026-08-28; full cross-file re-verification with minor clarifications +applied 2026-08-28). +Background: LAN clients stopped receiving DHCP leases because the `internal` zone lost its +`eth1` assignment (in both live firewalld and `config/firewall/config.json` — incident-time +state; the appliance was recovered before this plan was written, so the repo's +`config.json` now shows `internal: eth1` again, per Sequencing item 4). With no zone +covering `eth1`, all inbound traffic hit the default `reject` policy — DHCP (and everything +else) from the LAN was dropped before reaching a healthy dnsmasq. Nothing flagged the +uncovered-interface condition; every apply silently reinforced it. Follow-ups below close +that blind spot and the related sharp edges. `/etc/sudoers.d/wall` stays as-is (dev privs, +user-acknowledged). + +Decisions (confirmed): +- Target drift: **Option A** — absent `target` = unmanaged (not diffed, not touched by apply). + The absent key is the *single* canonical form for "unmanaged". A zone whose `target` + normalizes to `default` (e.g. the legacy explicit `"DEFAULT"`) is treated as unmanaged in + the pending diff as well — it mirrors `_config_apply`, which only ever sets + `ACCEPT/DROP/REJECT`. No backward compatibility is needed. +- Coverage guard: **ConflictError + `force`** in `_config_apply`; UI confirm in the + interfaces picker when an interface's last zone is dropped (zone *deletion* is out of + scope — `delete_zone` stays unguarded). +- Guard scope: **explicit `lo`/`wg*` filter** — guarded ifaces = network-config keys minus + `lo` and `wg*` prefixes (regardless of config contents); the coverage union also counts + live interfaces of zones absent from config (apply never touches them). +- Config cleanup is applied via **`POST /api/firewall/config` (full replace) + apply**, not + a raw JSON edit (re-stamps `_last_applied_hash`/`_last_applied_config` so cancel-all + baselines stay consistent). `PATCH` cannot be used: `deep_merge` (lib/common.py:244-256) + has no key-removal path, so the current `patch_config` endpoint cannot delete the + `target` keys (a `null` value would be written instead). +- Execution: **parallel streams** — WI-1 (A/B/C) + WI-3 are file-disjoint and run concurrently; + WI-2 → WI-4 → WI-5 stay serial (shared files: `_config_apply`, `_compute_pending_changes`, + `tests/test_firewall.py`, `docs/config.md`). +- Pin `"target": "ACCEPT"` for `internal` in config (declared intent = trusted LAN). + +Extra latent bugs found during research (folded in): +- `create_zone` (daemon/handlers/firewall.py:655-663) never creates the zone at all: the + `--new-zone` step is missing and it runs `firewall-cmd --set-target=` on a + nonexistent zone. For the default target it would run `--set-target=default`, which the + codebase's defensive guards treat as un-settable (the new-zone branch of + `_config_apply`, lines 188-205, already guards this; `create_zone` does not). The + current firewalld man page (verified via live docs 2026-08-28) lists `default` as an + accepted `--set-target` value for zones; the planned defensive skip of default targets + is still correct under Option A. An appliance-side check of actual behavior is optional + during live verification, not required. +- `_compute_pending_changes` (lib/firewall.py:380-381) skips entire zones with no + `interfaces` key — no field drift is ever reported for such zones. +- The explicit `"target": "DEFAULT"` entries in `config/firewall/config.json` (public, + vpn-full, work) are a one-shot install-time import artifact: + `system_import.import_firewall` (lib/system_import.py:921-932) emits a `target` key for + every imported zone with interfaces, and `_live_target_to_config` maps live `default` to + the config token `"DEFAULT"` (lib/firewall.py:353-361). No other code path writes + `target` into config. Hand/UI-edited zones (e.g. `internal`) omit the key — two + notations for the same meaning. Fixed by WI-2. + +--- + +## WI-1 — Interface-coverage invariant + apply guard (core fix) + +Goal: an interface managed by the network subsystem that ends up in *no* zone becomes a loud, +always-visible condition, and bulk-apply cannot silently produce it. + +1. `daemon/handlers/firewall.py`, `_config_apply` (lines 138-403): + - Interfaces step (lines 252-279) — new semantics: + - **key absent → hands off**: skip the remove-then-add for that zone (matches the + existing "None = don't change" masquerade semantic at line 281). + - **explicit `[]` → intentional unassign-all** (the UI picker legitimately sends this). + - **Pre-mutation coverage guard**: compute post-apply coverage = union of each zone's + desired interfaces (explicit list if key present, else its *current live* set for + absent-key zones) **plus the live interfaces of zones absent from config** (apply + never touches those; without this the guard false-positives when a live-only zone + still holds an interface). Guarded interfaces = keys of + `lib.network.get_config()["interfaces"]` with `lo` and `wg*` prefixes **explicitly + filtered** (vpn zones are managed by `WgToFirewallSync`; `lo` is normally zoneless — + without the filter a networkd-managed `lo` would make every apply raise). If any + guarded interface would be uncovered → `ConflictError` + naming the interfaces and the consequence (clients lose connectivity/DHCP), overridable + with `body.force = true` (same pattern as the https/ssh lockout at lines 154-169; the + `config_apply` handler at 608-631 already receives `body`). Implementation note: the + guard needs current live interfaces for *all* zones before any mutation — take them + from one `firewall-cmd --get-active-zones` call before the zone loop (same pattern as + `set_zone_interfaces`, daemon/handlers/firewall.py:731-732); do not rely on the + per-zone `--list-all` reads that currently happen inside the mutation loop. + Place the guard alongside the https/ssh lockout check (lines 154-169), i.e. before + the pre-apply backup write — the existing lockout test + (`test_config_apply_blocks_lockout_before_backup`, tests/test_firewall.py:670-677) + pins `mock_backup.assert_not_called()`, and the new guard must respect the same + no-side-effect-on-conflict invariant. Update the `_config_apply` and `config_apply` + docstrings (both currently document only the lockout guard + `force`). +2. `set_zone_interfaces` (lines 706-791): when the new selection leaves an interface in *no* + zone, emit a prominent `logger.warning`. No block — deliberate UI action. +3. `lib/firewall.py`, `_compute_pending_changes` (lines 364-476): remove the blanket + `if not zone_cfg.get("interfaces"): continue` (line 380-381) — only gate the *interfaces* + diff on key presence; report services/target/masquerade/rules/fwd-ports drift for such + zones as today. Implementation detail: the interfaces diff itself must be gated — + `cfg_ifaces = set(zone_cfg.get("interfaces", []))` (line 383) would otherwise diff + `set()` against live for absent-key zones and emit a spurious entry; compute it only + when `"interfaces" in zone_cfg`. Behavior change: zones with `interfaces: []` + (live: `vpn`, `vpn-full`, `work`) will now report field drift on every poll, and + config zones absent from live entirely will diff against an empty zone — the + pending list may be non-empty immediately after merge. +4. `lib/state.py`, `_collect_firewall` (line 449+): compute `uncovered_interfaces` + (network-config ifaces not in any live zone) on every poll, so it is visible even with + zero pending changes. Apply the same `lo`/`wg*` filter as the WI-1.1 guard. A + network-config iface that is absent from live state entirely (down/renamed) counts as + uncovered too — not just zoneless-on-live. Add + `uncovered_interfaces: list[str]` to `schema.FirewallState` (lib/schema.py:87); the + TypedDict is shape-checked against the collector return by + `tests/test_schema_types.py::test_firewall_state` (lines 21-54), so the collector must + always include the key. Update the `FirewallState` block in `docs/state-model.md` + (lines 51-78 — the authoritative Markdown reference per lib/schema.py:3-5). Test + note: the network-config read is a file read, not `run()` — patch + `lib.network.get_config` in `test_firewall_state` (which mocks only `lib.state.run`) + so the value is deterministic. This is the detection that would have caught the + incident within 30s. +5. Surface it: + - `daemon/handlers/status.py` (lines 62-105): advisory coverage warnings in the firewall + section of `/api/status/pending` (not counted in `needs_apply`). + - `webui/static/pages/zones.js`: warning banner from + `state.firewall.data.uncovered_interfaces`. + - `docs/api.md` (line 1965): document the new advisory field in the + `/api/status/pending` firewall section (not counted in `needs_apply`/`total_changes`). + - `docs/api.md`: note that `POST /api/status/apply-all` runs the firewall apply with + `force=false` — a coverage `ConflictError` surfaces in the response `errors` dict + under "Firewall" while the other subsystems proceed (the desired no-silent-apply + behavior). +6. UI guard: the interfaces `MultiSelectModal` in zones.js gets a `confirm` hook (same + pattern as the services lockout at lines 89-99): if the selection would drop an + interface's last zone, warn that clients on that segment lose connectivity and DHCP. + +Tests: `tests/test_firewall.py` — absent-key zone keeps live interfaces on apply; explicit +`[]` unassigns; conflict raised when a network iface goes uncovered; `force` bypasses; +guard ignores `lo`/`wg*` even when present in network config; live-only-zone interfaces +count as covered; pending diff now reports services drift on interface-less zones +(absent-key zones emit no spurious interfaces entry). `tests/test_api.py` / +`tests/test_status_pending.py` for the new advisory field. `tests/test_schema_types.py` +for the new `FirewallState` key. Test setup note: the guard reads +`lib.network.get_config()` — a file read, not `run()` — so every non-force `_config_apply` +test must patch `lib.network.get_config` (e.g. return `{"interfaces": {"eth0": {}}}` +matching the mocked live state). Without it, the repo's real +`config/network/config.json` (carries `eth0`+`eth1`) combined with the mocked `run` +(one return string for all calls, so `--get-active-zones` does not cover `eth1`) +raises a spurious `ConflictError`; existing tests affected include +`test_applies_existing_zone` (~560) and `test_stamps_applied_baseline` (~750). +The patch value must be consistent with the mocked live state **per test**: +`{"interfaces": {"eth0": {}}}` only works where the config driving the guard (the +`lib.firewall.get_config` mock) carries an explicit `interfaces` list +(`test_stamps_applied_baseline`, `_STAMP_TEST_CFG` with `"interfaces": ["eth0"]`) — +the explicit list covers eth0 in the post-apply union. In `test_applies_existing_zone` +that mock also carries `"interfaces": ["eth0"]` (the `{"public": {}}` mock is +`_get_config`, used only for the end-of-apply stamp at firewall.py:396), so +`{"interfaces": {"eth0": {}}}` works there too — but the single-string `run` mock +makes `--get-active-zones` parse to garbage covering neither eth0 nor eth1, so the +simplest patch is `{"interfaces": {}}` (or upgrade the `run` mock to a `side_effect` +answering `--get-active-zones` with eth0 covered). Guard ordering: the +https/ssh lockout check (lines 154-169) must run **before** the coverage guard — +`test_config_apply_blocks_lockout_before_backup` (~670) asserts the "https and ssh" +message and mocks neither `run` nor the network config, so a coverage guard evaluated +first would hit the un-mocked `run`/file read and break that test. + +## WI-2 — Target-drift semantics (Option A: omit = unmanaged) + +Goal: stop the trap where config omits `target` (→ implicit "default"), live says `ACCEPT`, +the pending diff flags it, and apply can never clear it (firewalld cannot set "default" back) +→ permanent fake "pending" + dead apply button. + +- `lib/firewall.py`, `_compute_pending_changes`: skip the target diff when the zone config + has no explicit `target` key **or** the value normalizes to `default` (precedent: the + public-masquerade skip at lines 419-424). Defensive — it covers legacy configs still + carrying explicit `"DEFAULT"`. `_config_apply` already leaves default targets alone + (lines 207-219). Explicit `ACCEPT/DROP/REJECT` remains fully managed. +- Fix the source — `lib/system_import.py`, `import_firewall` (lines 921-932): emit the + `"target"` key only when the imported zone's live target normalizes to something other + than `default`. Today the importer emits a faithful snapshot with a `target` key for + every zone, and `_live_target_to_config` maps live `default` → `"DEFAULT"` (this is the + sole author of the explicit `"target": "DEFAULT"` entries; no other code path writes + `target` into config). Update `tests/test_system_import.py` (assertions at lines + 538-574). Keep `_live_target_to_config` itself (lib/firewall.py:353-361; still asserted + at tests/test_firewall.py:148-149). +- `create_zone` (daemon/handlers/firewall.py:632-670): rewrite the body to mirror the full + `_config_apply` new-zone branch (lines 188-205): run `--new-zone` first (currently + missing entirely — the endpoint would create no zone at all), then `--set-target` only + when the target normalizes to something other than `default`, then `_reload()`. +- Config cleanup (on the appliance): remove the legacy `"target": "DEFAULT"` entries from + the `public`, `vpn-full`, and `work` zones (making key-absence the one canonical + "unmanaged" notation), and add `"target": "ACCEPT"` to the `internal` zone — declares the + trusted-LAN intent and makes future apply enforce it and flag any drift. Apply via + `POST /api/firewall/config` (full replace) + apply, **not** a raw JSON edit and **not** + `PATCH` (which cannot delete keys — see the Decisions note above): GET the current + config, drop the three `target` entries, add `"target": "ACCEPT"` to `internal`, POST, + then apply. Apply re-stamps `_last_applied_hash`/`_last_applied_config` so cancel-all + baselines stay consistent. +- docs/config.md: document "target omitted (or normalizes to `default`) → live value is + preserved, not diffed, and never re-set by apply". + +Tests: pending-diff cases (absent target ⇒ no target entry; explicit `"DEFAULT"` ⇒ no +target entry; explicit `ACCEPT` vs live `default` ⇒ entry); `create_zone` paths +(`--new-zone` always called; `--set-target` only for non-default targets); +`import_firewall` omits `target` for default-target zones while keeping it for +`ACCEPT/DROP/REJECT`. + +## WI-3 — Make the sync bus non-destructive (stale DHCP ranges) + +Goal: a zone interface change must never delete user data. `FirewallToDhcpSync` currently +hard-deletes ranges the moment an interface loses zone coverage (lib/sync.py:806-822) — +exactly what ate the eth1 pool during the incident's mis-click. + +- `lib/sync.py`, `FirewallToDhcpSync.on_firewall_config_saved` (lines 804-822): + - Keep the range in the dnsmasq config. + - Log a warning and emit a `SyncResult.changes` entry: "DHCP range on '' has no + firewall zone coverage — inactive until a zone covers it". + - Report `dnsmasq` in `affected_subsystems` only when the gateway auto-fill step + (lines 824-855) actually mutated config — the return at sync.py:866 becomes + `["dnsmasq"] if changed else []`. + - Update the class docstring (lines 742-748) and method docstring accordingly. +- Rationale: a range is inert only while the firewall drops the traffic; keeping it makes + zone re-assignment self-heal with zero follow-up. + +Tests: `tests/test_sync.py` `TestFirewallToDhcpSync` (lines 763-920): `test_removes_stale_ranges` +becomes `test_flags_uncovered_range_without_deleting` (assert dnsmasq config untouched + +warning present). `test_keeps_global_ranges` also asserts the stale eth2 range is removed +(`len(saved_ranges) == 1`, `dnsmasq` affected, `mock_dm_save.call_args` read +unconditionally) and must be rewritten for the non-destructive semantics (both ranges kept, +no save, no affected subsystems, warning present). + +## WI-4 — Make the firewall "backup" real + +Goal: `data/firewall/rules.json` stores an empty skeleton before *and* after apply +(daemon/handlers/firewall.py:171-179, 385-393), so the disaster-recovery artifact promised by +docs/architecture.md:134 contains nothing. + +- `_config_apply`: save a **pre-apply snapshot only**, before any mutation: + `{timestamp, default_zone, zones: _parse_all_zones_output(firewall-cmd --list-all-zones --permanent), + config: }` → `data/firewall/rules.json` via `_save_backup`. The + permanent view is what is reproducible for manual recovery. Note: `_parse_all_zones_output` + must be added to the handler's `lib.firewall` import (daemon/handlers/firewall.py:38-43) — + it is not currently imported there. +- Remove the misleading post-apply skeleton write (lines 385-393); the apply response's + `backup` path field is unchanged. `load_backup` has no live consumers — no API changes. +- Docs: update architecture.md:134/298, config.md:485, overview.md:77 to describe the shape. + +Tests: the `_save_backup` patch sites in `tests/test_firewall.py` (~lines 566, 701, +751) carry `return_value="/tmp/rules.json"` and stay valid as-is — no existing test +asserts on its arguments or call count. Optionally add one assertion that the single +pre-apply call receives the snapshot payload (`default_zone`/`zones`/`config` keys). + +## WI-5 (optional, low) — Daemon shutdown noise + +"Task was destroyed but it is pending" + logging-error tracebacks on daemon SIGTERM (7 +occurrences since the Aug 22 restart, still happening on current code). + +- `daemon/server.py` shutdown path: stop accepting new connections, give in-flight request + tasks a bounded grace period (`await server.wait_closed()` with timeout) before + `runner.cleanup()`; suppress the asyncio default exception handler during the teardown + window. + +Cosmetic. Do last, or defer. + +--- + +## Sequencing & verification + +1. One branch; one commit per WI: **1 → 3 → 2 → 4 → 5**. WI-1 + WI-3 together fix the + incident class; WI-2/WI-4 are hygiene; WI-5 optional. + Execution: **Phase 1 in parallel** (file-disjoint streams — A: WI-1 backend, + `daemon/handlers/firewall.py` + `lib/firewall.py` pending-diff; B: WI-1 state surface, + `lib/state.py` + `lib/schema.py` + `daemon/handlers/status.py` + schema/status tests + + `docs/state-model.md` + `docs/api.md`; + C: WI-1 frontend, `webui/static/pages/zones.js`; D: WI-3, `lib/sync.py` + + `tests/test_sync.py`); **Phase 2 serial** — WI-2 → WI-4 → WI-5 (shared files: + `_config_apply`, `_compute_pending_changes`, `tests/test_firewall.py`, `docs/config.md`). +2. Per commit: + - `.venv/bin/ruff check lib/ webui/ daemon/ tests/` + - `.venv/bin/ruff format lib/ webui/ daemon/ tests/` + - `.venv/bin/python -m pytest tests/ -v` + - `node tests/test-*.js` for touched hoover components (zones.js itself has no node test + file — verify by loading the page in the running UI). +3. Live verification on the appliance after merge: + - Confirm `uncovered_interfaces` is empty in firewall state. + - Check the firewall pending list for the expected post-WI-1.3/WI-2 drift entries + (`interfaces: []` zones now report field drift; `internal` target pinned to + `ACCEPT`) and confirm nothing unexpected appears. + - Optional drill: create a throwaway zone and move `eth0` onto it via the API with + `force` omitted (expect ConflictError) and added (expect success + warning), then + restore. Skip if undesired — mocked tests cover the logic. +4. No live-system changes during implementation; DHCP/zone state stays as the operator left + it (internal=eth1, public=eth0, leases confirmed 01:29). diff --git a/daemon/handlers/firewall.py b/daemon/handlers/firewall.py index e507782..238f645 100644 --- a/daemon/handlers/firewall.py +++ b/daemon/handlers/firewall.py @@ -34,10 +34,13 @@ from daemon.iface import ( POST_FIREWALL_ZONES_SERVICES, ) from daemon.server import ConflictError, NotFoundError, refresh_state, registry +from lib import network from lib.common import load_json, run, save_json, stamp_applied, strip_apply_meta from lib.firewall import ( _normalize_target, + _now_iso, _parse_active_zones, + _parse_all_zones_output, _parse_zone_output, fw_change_summary, ) @@ -143,8 +146,15 @@ def _config_apply(force: bool = False) -> dict[str, Any]: then adding desired values. Reloads firewalld at the end. With *force* False (default), a ``ConflictError`` is raised before any - mutation if the config would strip both https and ssh from the default - zone; pass ``force=True`` to override. + mutation in two cases; pass ``force=True`` to override either: + + - the config would strip both https and ssh from the default zone + (management lockout); + - a network-subsystem-managed interface would end up with no firewall + zone coverage after apply (``lo`` and ``wg*`` interfaces are excluded). + Zones whose config omits the ``interfaces`` key are left hands-off, so + their current live interfaces count as coverage, as do the live + interfaces of zones that are live but absent from the config. """ from lib.firewall import get_config as _get_lib_config @@ -168,15 +178,52 @@ def _config_apply(force: bool = False) -> dict[str, Any]: f'to the zone\'s services, or pass {{"force": true}}.' ) - full_state: dict[str, Any] = { - "active_zones": {}, - "interfaces": [], - "available_services": [], - "zones": {}, - "rich_rules": {}, - "timestamp": "", - } - _save_backup(full_state) + # Coverage guard: after apply, every network-managed interface must + # belong to a zone or traffic (and DHCP) on that segment is dropped. + live_active = _parse_active_zones( + run(["firewall-cmd", "--get-active-zones"], sudo=True) + ) + covered: set[str] = set() + for zn, zc in cfg_zones.items(): + if "interfaces" in (zc if isinstance(zc, dict) else {}): + covered.update(zc["interfaces"]) + else: + covered.update(live_active.get(zn, [])) + covered.update( + iface + for zn, ifaces in live_active.items() + if zn not in cfg_zones + for iface in ifaces + ) + net_cfg = network.get_config() + guarded = [ + name + for name in net_cfg.get("interfaces", {}) + if name != "lo" and not name.startswith("wg") + ] + uncovered = [name for name in guarded if name not in covered] + if uncovered: + raise ConflictError( + "Refusing to apply: " + f"{', '.join(repr(n) for n in uncovered)} " + f"would have no firewall zone coverage after apply, so all " + f"traffic (including DHCP) from those segments would be " + f'dropped. Keep the interface in a zone, or pass {{"force": true}}.' + ) + + # Pre-apply snapshot for disaster recovery: the permanent zone view plus + # the declarative config, captured before any mutation. The permanent + # view is what is reproducible for manual recovery. + backup_path = _save_backup( + { + "timestamp": _now_iso(), + "default_zone": _default_zone(), + "zones": _parse_all_zones_output( + run(["firewall-cmd", "--list-all-zones", "--permanent"], sudo=True) + ), + "config": cfg, + } + ) available = run(["firewall-cmd", "--get-zones"], sudo=True).split() applied: list[str] = [] @@ -250,33 +297,38 @@ def _config_apply(force: bool = False) -> dict[str, Any]: ) # Step 3: Reconcile interfaces — same remove-then-add pattern. - current_ifaces: list[str] = [] - with suppress(Exception): - current_ifaces = _parse_zone_output( - zone_name, - run(["firewall-cmd", f"--zone={zone_name}", "--list-all"], sudo=True), - ).get("interfaces", []) - for iface in current_ifaces: - run( - [ - "firewall-cmd", - f"--zone={zone_name}", - "--remove-interface=" + iface, - "--permanent", - ], - sudo=True, - check=False, - ) - for iface in zone_cfg.get("interfaces", []): - run( - [ - "firewall-cmd", - f"--zone={zone_name}", - "--add-interface=" + iface, - "--permanent", - ], - sudo=True, - ) + # Absent "interfaces" key = hands off (keep the zone's live + # interfaces); an explicit empty list = intentional unassign-all. + if "interfaces" in zone_cfg: + current_ifaces: list[str] = [] + with suppress(Exception): + current_ifaces = _parse_zone_output( + zone_name, + run( + ["firewall-cmd", f"--zone={zone_name}", "--list-all"], sudo=True + ), + ).get("interfaces", []) + for iface in current_ifaces: + run( + [ + "firewall-cmd", + f"--zone={zone_name}", + "--remove-interface=" + iface, + "--permanent", + ], + sudo=True, + check=False, + ) + for iface in zone_cfg.get("interfaces", []): + run( + [ + "firewall-cmd", + f"--zone={zone_name}", + "--add-interface=" + iface, + "--permanent", + ], + sudo=True, + ) # Step 4: Toggle masquerade if explicitly set (None means "don't change"). # Skip 'public' — Step 7 handles masquerade propagation for nftables. @@ -382,15 +434,6 @@ def _config_apply(force: bool = False) -> dict[str, Any]: _save_config(cfg) _reload() - full_state = { - "active_zones": {}, - "interfaces": [], - "available_services": [], - "zones": {}, - "rich_rules": {}, - "timestamp": "", - } - backup_path = _save_backup(full_state) # Record the applied config snapshot + hash so pending-changes detection # and cancel/revert work like the hash-based subsystems. applied_cfg = _get_config() @@ -612,11 +655,16 @@ def config_apply(_request: Any, _body: Any) -> dict[str, Any]: Args: _request: The incoming HTTP request (unused). _body: Optional JSON body; ``{"force": true}`` overrides the - management-lockout guard for the default zone. + management-lockout guard and the interface-coverage guard. Returns: Dict with ``applied_zones`` (list of zone names), ``backup`` (path), and ``synced`` (affected subsystems). + + Raises: + ConflictError: If the config would strip both https and ssh from the + default zone, or would leave a network-managed interface without + zone coverage, and ``force`` is not set. """ force = bool(_body and _body.get("force")) result = _config_apply(force=force) @@ -631,7 +679,11 @@ 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. + """Create a new firewall zone, emit sync event, refresh state. + + Runs ``--new-zone`` first (required before ``--set-target``), then sets + the target only when it normalizes to something other than ``default`` + (the implicit firewalld target is never re-set), then reloads. Args: _request: The incoming HTTP request (unused). @@ -652,15 +704,21 @@ def create_zone(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: available = run(["firewall-cmd", "--get-zones"], sudo=True).split() if zone_name in available: raise ValueError(f"Zone '{zone_name}' already exists") - run( - [ - "firewall-cmd", - f"--zone={zone_name}", - f"--set-target={target}", - "--permanent", - ], - sudo=True, - ) + # Create the zone first; --set-target requires the zone to exist. + run(["firewall-cmd", f"--new-zone={zone_name}", "--permanent"], sudo=True) + # "default" is firewalld's implicit target and cannot be meaningfully + # re-set, so only explicit ACCEPT/DROP/REJECT targets are applied. + normalized_target = _normalize_target(target) + if normalized_target != "default": + run( + [ + "firewall-cmd", + f"--zone={zone_name}", + f"--set-target={normalized_target}", + "--permanent", + ], + sudo=True, + ) _reload() logger.info("Zone '%s' created (target=%s)", zone_name, target) sync_result = bus.emit( @@ -707,6 +765,11 @@ def delete_zone(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: def set_zone_interfaces(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: """Replace zone interfaces, reassigning interfaces from old zones. + When the new selection leaves an interface in no zone at all, a + prominent warning is logged (clients on that segment lose connectivity + and DHCP); the operation is not blocked since it is a deliberate UI + action. + Args: _request: The incoming HTTP request (unused). body: JSON body with ``zone`` and ``interfaces`` list. @@ -761,6 +824,20 @@ def set_zone_interfaces(_request: Any, body: dict[str, Any] | None) -> dict[str, sudo=True, ) + # Flag interfaces that ended up in no zone at all — clients on those + # segments lose connectivity (including DHCP). + for iface in set(active.get(zone, [])) - set(interfaces): + if not any( + iface in az_ifaces for az, az_ifaces in active.items() if az != zone + ): + logger.warning( + "Interface '%s' is now in NO firewall zone: clients on that " + "segment will lose connectivity and DHCP (zone '%s' no longer " + "covers it).", + iface, + zone, + ) + _reload() # Update config diff --git a/daemon/handlers/status.py b/daemon/handlers/status.py index a78a9a5..57222f1 100644 --- a/daemon/handlers/status.py +++ b/daemon/handlers/status.py @@ -65,6 +65,10 @@ def status_pending(_request: Any, _body: Any) -> dict[str, Any]: Returns: Dict with per-subsystem pending status and total change count. + The firewall section also carries advisory `uncovered_interfaces` + and `coverage_warnings` fields (network-config interfaces not in + any live zone); they are never counted in `needs_apply`, + `change_count`, or `total_changes`. """ fw = state_store.get("firewall") or {} pending_fw = fw.get("pending", {}) @@ -78,10 +82,23 @@ def status_pending(_request: Any, _body: Any) -> dict[str, Any]: summary = fw_change_summary(zone, ctype, c) fw_changes.append({"summary": summary, "detail": ""}) + uncovered = fw.get("uncovered_interfaces") or [] + coverage_warnings = ( + [ + "Interfaces not in any firewall zone: " + f"{', '.join(uncovered)} — clients on those segments lose " + "connectivity and DHCP" + ] + if uncovered + else [] + ) + fw_result = { "needs_apply": fw_needs_apply, "change_count": len(fw_changes), "changes": fw_changes, + "uncovered_interfaces": uncovered, + "coverage_warnings": coverage_warnings, } hash_subsystems = { diff --git a/daemon/server.py b/daemon/server.py index 9454902..fcbc1d9 100644 --- a/daemon/server.py +++ b/daemon/server.py @@ -681,14 +681,48 @@ def main() -> None: loop = asyncio.new_event_loop() + def _teardown_exception_handler(_loop, context) -> None: + # Swallow teardown noise ("Task was destroyed but it is pending", + # in-flight task exceptions on SIGTERM) instead of the default + # logging-error tracebacks. + logger.debug("Suppressed teardown exception: %s", context) + async def _shutdown() -> None: - """Graceful shutdown: cancel poller, close runner, teardown.""" + """Graceful shutdown: stop accepting, drain in-flight work, teardown. + + Bounded grace periods + a suppressed exception handler during the + teardown window avoid the "Task was destroyed but it is pending" and + logging-error tracebacks that otherwise appear on SIGTERM. + """ logger.info("Shutting down daemon...") _stop_polling() + # Suppress the default exception handler during teardown so that + # cancelling in-flight tasks does not spew tracebacks on SIGTERM. + prev_handler = loop.exception_handler + loop.set_exception_handler(_teardown_exception_handler) try: - await asyncio.wait_for(runner.cleanup(), timeout=5) - except TimeoutError: - logger.warning("Runner cleanup timed out, abandoning") + # Stop accepting new connections (also waits for open sockets, + # bounded so a stuck WebSocket can't hang shutdown). + try: + await asyncio.wait_for(runner.cleanup(), timeout=5) + except TimeoutError: + logger.warning("Runner cleanup timed out, abandoning") + # Give in-flight request/WS tasks a bounded grace period to + # finish; cancel anything still pending so they are not + # "destroyed but pending" when the loop closes. + pending = [ + t + for t in asyncio.all_tasks() + if t is not asyncio.current_task() and not t.done() + ] + if pending: + _, still_pending = await asyncio.wait(pending, timeout=3) + for t in still_pending: + t.cancel() + if still_pending: + await asyncio.wait(still_pending, timeout=1) + finally: + loop.set_exception_handler(prev_handler) if Path(socket_path).exists(): os.unlink(socket_path) logger.info("vacuum-walld stopped") diff --git a/docs/api.md b/docs/api.md index c4464b2..9852b20 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1962,7 +1962,7 @@ Aggregate pending changes across all subsystems. Useful for the dashboard to sho | Field | Type | Description | |-------|------|-------------| -| `firewall` | `object` | `{ needs_apply, change_count, changes: [{summary, detail}] }` | +| `firewall` | `object` | `{ needs_apply, change_count, changes: [{summary, detail}], uncovered_interfaces: [string], coverage_warnings: [string] }`. `uncovered_interfaces` lists network-config interfaces (excluding `lo`/`wg*`) that are in no live firewalld zone, and `coverage_warnings` carries the matching advisory text. Both are advisory only — they are **not** counted in `needs_apply`, `change_count`, or `total_changes` | | `dnsmasq` / `nginx` / `wireguard` / `networkd` | `object` | `{ pending_changes, summary, changes: [{summary, detail}] }` | | `total_changes` | `number` | Total count of pending changes across all subsystems | @@ -1981,7 +1981,12 @@ Apply pending changes for all subsystems in dependency order. | Field | Type | Description | |-------|------|-------------| | `applied` | `[string, ...]` | List of subsystems that were applied | -| `errors` | `[object, ...]` | Any errors encountered during apply | +| `errors` | `object` | Map of subsystem label → error message | + +The firewall apply runs with `force=false`, so if a firewall interface +would be left without zone coverage (the coverage guard), a +`ConflictError` surfaces in `errors` under `"Firewall"` while the other +subsystems proceed — the desired no-silent-apply behavior. --- diff --git a/docs/architecture.md b/docs/architecture.md index f0d6848..85f3df9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -131,7 +131,7 @@ Vacuum Wall uses a declarative configuration model. Persistent user-facing confi | Subsystem | Declarative Config | Runtime Data | Rendered Target | State Persistence | |---|---|---|---|---| -| firewalld | `config/firewall/config.json` | `data/firewall/rules.json` | N/A (commands issued directly to firewalld via D-Bus) | firewalld manages its own persistent state in `/etc/firewalld/`. `config.json` is the declarative source of truth. `rules.json` serves as an automated backup snapshot. | +| firewalld | `config/firewall/config.json` | `data/firewall/rules.json` | N/A (commands issued directly to firewalld via D-Bus) | firewalld manages its own persistent state in `/etc/firewalld/`. `config.json` is the declarative source of truth. `rules.json` is a pre-apply recovery snapshot (`{timestamp, default_zone, zones, config}`) written before every apply; `zones` is the permanent firewalld zone view. | | dnsmasq | `config/dnsmasq/config.json` | `data/dnsmasq/fragments/` | `/etc/dnsmasq.d/vacuum-wall.conf` | The JSON file is the source of truth. The rendered `.conf` file is overwritten on each apply. | | nginx | `config/nginx/config.json` | `data/nginx/.htpasswd`, `data/nginx/sites-enabled/` | `data/nginx/sites-enabled/.conf` + `/etc/nginx/conf.d/vacuum-wall.conf` | All proxy and management domain definitions are derived from the JSON config. Generated `.conf` files are overwritten on each apply. | | WireGuard | `config/wireguard/config.json` | `data/wireguard/` | `/etc/wireguard/wg0.conf` | The JSON file defines the interface and all peers. The rendered WireGuard config is overwritten on each apply. | @@ -295,7 +295,7 @@ data/ ├── dnsmasq/ │ └── fragments/ # User-defined dnsmasq config fragments (appended verbatim) ├── firewall/ -│ └── rules.json # Auto-generated firewall rule state backup +│ └── rules.json # Pre-apply firewall recovery snapshot ├── acme/ # ACME certificate files (acme.sh home) ├── logs/ │ └── vacuum-wall.log # Application log file diff --git a/docs/config.md b/docs/config.md index cdf7b03..babf6ef 100644 --- a/docs/config.md +++ b/docs/config.md @@ -482,7 +482,7 @@ If the interface is already up, `wg-quick up` will reconfigure it in place witho **File**: `config/firewall/config.json` -This file defines the declarative firewalld zone configuration. The application compares it against the live firewalld state via `_compute_pending_changes()` and applies incremental changes. Runtime state backups are stored in `data/firewall/rules.json`. +This file defines the declarative firewalld zone configuration. The application compares it against the live firewalld state via `_compute_pending_changes()` and applies incremental changes. Before every apply a **pre-apply recovery snapshot** is written to `data/firewall/rules.json`: `{timestamp, default_zone, zones, config}` where `zones` is the permanent firewalld zone view (`--list-all-zones --permanent`) and `config` is the declarative config at apply time. The permanent view is what is reproducible for manual recovery. ```json { @@ -519,7 +519,7 @@ The `zones` object maps zone names (keys) to zone configurations. Each zone corr |---|---|---|---| | `interfaces` | array | No | Network interfaces assigned to this zone. Computed against live state to detect pending changes. Default: `[]`. | | `services` | array | No | Firewalld services to allow in this zone (e.g., `ssh`, `https`, `dns`, `dhcp`). Default: `[]`. | -| `target` | string | No | Zone target policy. One of: `DEFAULT`, `ACCEPT`, `DROP`, `REJECT`. The code maps these to firewalld's canonical target values (`default`, `ACCEPT`, `DROP`, `REJECT`). Default: `DEFAULT`. | +| `target` | string | No | Zone target policy. `ACCEPT`, `DROP`, or `REJECT` is fully managed. When the key is **omitted** (the canonical "unmanaged" notation) or normalizes to `default` (e.g. a legacy explicit `"DEFAULT"`), the live value is **preserved** — it is not diffed and never re-set by apply (firewalld cannot set `default` back). | | `masquerade` | boolean | No | Enable IP masquerading (NAT) for this zone. Default: `false`. | | `forward_ports` | array | No | Port forwarding rules. Each entry has an auto-generated `id` field and the standard firewalld forward-port fields. Default: `[]`. | | `forward_ports[].id` | string | No | Auto-generated unique identifier for the port forwarding rule. Not user-settable. | @@ -532,12 +532,14 @@ The `zones` object maps zone names (keys) to zone configurations. Each zone corr ### Applying Firewall Configuration -The `config_pending()` function compares the declarative config in `config/firewall/config.json` against the live firewalld state returned by the daemon (via `daemon.handlers.firewall.get_state()`). It returns a diff indicating which zones have pending changes for interfaces, services, target, masquerade, forward ports, and rich rules. Zones that exist live but not in config are reported as `unmanaged_zones`. +The `config_pending()` function compares the declarative config in `config/firewall/config.json` against the live firewalld state returned by the daemon (via `daemon.handlers.firewall.get_state()`). It returns a diff indicating which zones have pending changes for interfaces, services, target, masquerade, forward ports, and rich rules. Zones that exist live but not in config are reported as `unmanaged_zones`. A `target` entry is only reported when the config carries an explicit target that normalizes to something other than `default`; an omitted key or a `default`-normalizing value is unmanaged, so live target drift is neither flagged nor applied. Likewise the `interfaces` entry is only reported for zones whose config explicitly carries the key. Both `/api/firewall/zones//services` and `/api/firewall/config/apply` reconcile **remove-then-add** against the live zone, so anything opened outside the declarative config (e.g. directly via `firewall-cmd`) is reverted on the next apply. Service changes made through the API are persisted to `config.json` to prevent this drift. **Management-lockout guard.** The firewalld *default zone* is the catch-all for interfaces with no explicit assignment (typically the WAN), and it carries the management plane (nginx https) plus remote recovery (ssh). Changing the default zone's service set so that **neither `https` nor `ssh`** remains raises `409 Conflict` — from `POST /firewall/zones//services` and `POST /firewall/config/apply` — before any mutation runs. Send `"force": true` in the request body to override (the UI shows a confirm dialog with this effect on the Zones page). If the default zone cannot be determined, the guard fails closed. +**Interface-coverage guard.** `POST /firewall/config/apply` also refuses (before any mutation) if applying would leave a network-subsystem-managed interface in **no** firewall zone — traffic (and DHCP) on that segment would be dropped. Guarded interfaces are the keys of the network config's `interfaces`, excluding `lo` and `wg*` (vpn zones are managed by the WireGuard sync and `lo` is normally zoneless). Zones whose config omits the `interfaces` key are left hands-off, so their current live interfaces count as coverage, as do the live interfaces of zones that are live but absent from the config. Send `"force": true` to override. The condition is always surfaced as the `uncovered_interfaces` field in firewall state (see `docs/state-model.md`) and as an advisory in `GET /api/status/pending`. + **Applied baseline.** Like the other config-backed subsystems, a successful apply records `_last_applied_hash` and `_last_applied_config` (the meta-stripped config snapshot) inside `config.json`. They are internal bookkeeping — ignored by all parsing, hashing, and UI surfaces — and let the aggregate cancel action (`POST /api/status/cancel-all`) revert this file to the last applied state. Configs that have never been applied have no baseline and are skipped by cancel. ## Networkd (IP Configuration) diff --git a/docs/hoover.md b/docs/hoover.md index 98e38d3..c1fdccd 100644 --- a/docs/hoover.md +++ b/docs/hoover.md @@ -1292,6 +1292,12 @@ h('button', { 'on:click': () => addZone(state) }, 'Add Zone') Factory that returns a function to open a multi-select modal. Use as an `on:click` handler in VNode props. +The picker is a scrollable, **filtered checkbox list** (not a native +``). Options are sorted; each row may carry optional + * description text. A live search box filters rows in place (typing does + * not re-render the modal, so input focus is preserved), a counter shows + * "N of M selected", and Select all / Clear act on the currently visible + * rows. + * + * When `props.common` is a non-empty array an advanced toggle appears: + * cleared (the default) the list shows only common options plus anything + * currently selected; checked it shows every option. Deselecting a + * non-common option while the advanced toggle is cleared hides its row + * again. + * + * Selection, the search query, and the advanced flag live in a closure per + * open call, so `refreshModals()` re-renders (e.g. the processing spinner) + * re-apply the current state instead of losing it. + * * @param {object} props * @param {string} props.title - Modal title * @param {string} props.url - API POST URL * @param {string[]} props.options - All selectable options * @param {string[]} props.selected - Currently selected values * @param {string} props.fieldKey - JSON key for the field + * @param {object} [props.descriptions] - Option value → description text + * @param {string[]} [props.common] - When set, enables the advanced toggle * @param {string} [props.successMsg] - Success toast message * @param {string|string[]} [props.refresh] - Legacy, ignored (accepted for backward compat) * @param {function} [props.confirm] - (body) => string|null; confirm gate, see apiSubmit @@ -223,24 +242,20 @@ export function formModal(inner, title, fields, actions) { */ export function MultiSelectModal(props = {}) { return () => { - const selectId = 'ms-' + props.fieldKey; + const allOpts = [...new Set(props.options || [])].sort(); + const sel = new Set(props.selected || []); + const hasAdv = Array.isArray(props.common) && props.common.length > 0; + const descs = props.descriptions || {}; + let query = ''; + let advanced = false; + openModal((inner) => { - formModal(inner, props.title, - [{ - label: props.fieldKey, - id: selectId, - tag: 'select', - multiple: true, - options: (props.options || []).map(o => [o, (props.selected || []).includes(o)]), - }], + formModal(inner, props.title, [], [ { label: 'Cancel', cls: 'btn-outline', action: 'c', handler: () => closeModal() }, ...apiSubmit({ url: props.url, - body: () => ({ - [props.fieldKey]: Array.from(document.getElementById(selectId).selectedOptions) - .map(o => o.value), - }), + body: () => ({ [props.fieldKey]: [...sel].sort() }), successMsg: props.successMsg || 'Updated', refresh: props.refresh, confirm: props.confirm, @@ -248,6 +263,103 @@ export function MultiSelectModal(props = {}) { }), ], ); + + const body = inner.querySelector('.modal-body'); + const showSearch = allOpts.length > 8; + const rowsHtml = allOpts.map((o) => { + const d = descs[o]; + return ''; + }).join(''); + body.innerHTML = '
' + + (showSearch + ? '
' + + '' + + '' + + '
' : '
') + + '
' + + '' + + '' + + (hasAdv + ? '' + : '') + + '
' + + '
' + rowsHtml + '
' + + '' + + '
'; + + const rows = [...body.querySelectorAll('.ms-row')]; + const countEl = body.querySelector('.ms-count'); + const searchEl = body.querySelector('.ms-search'); + const advEl = body.querySelector('.ms-adv-check'); + const emptyEl = body.querySelector('.ms-empty'); + + const isVisible = (o) => { + if (query && !o.toLowerCase().includes(query)) return false; + if (!hasAdv || advanced) return true; + return sel.has(o) || props.common.includes(o); + }; + + const apply = () => { + let visibleCount = 0; + for (const row of rows) { + const show = isVisible(row.dataset.msValue); + row.hidden = !show; + if (show) visibleCount++; + } + if (emptyEl) { + emptyEl.hidden = visibleCount > 0; + emptyEl.textContent = allOpts.length === 0 + ? 'Nothing to select.' + : 'No matches for "' + (query || '') + '".'; + } + countEl.textContent = sel.size + ' of ' + allOpts.length + ' selected'; + }; + + const syncRowChecks = () => { + for (const row of rows) { + row.querySelector('.ms-check').checked = sel.has(row.dataset.msValue); + } + }; + + if (searchEl) { + searchEl.addEventListener('input', () => { + query = searchEl.value.trim().toLowerCase(); + apply(); + }); + } + if (advEl) { + advEl.addEventListener('change', () => { + advanced = advEl.checked; + apply(); + }); + } + for (const row of rows) { + const cb = row.querySelector('.ms-check'); + cb.addEventListener('change', () => { + const o = row.dataset.msValue; + if (cb.checked) sel.add(o); + else sel.delete(o); + apply(); + }); + } + body.querySelector('.ms-selall').addEventListener('click', () => { + for (const row of rows) if (!row.hidden) sel.add(row.dataset.msValue); + syncRowChecks(); + apply(); + }); + body.querySelector('.ms-clear').addEventListener('click', () => { + for (const row of rows) if (!row.hidden) sel.delete(row.dataset.msValue); + syncRowChecks(); + apply(); + }); + + apply(); }); }; } diff --git a/webui/static/pages/zones.js b/webui/static/pages/zones.js index 4070ae8..76f7cc7 100644 --- a/webui/static/pages/zones.js +++ b/webui/static/pages/zones.js @@ -1,5 +1,14 @@ import { html, PageHeader, Badge, Empty, ConfirmDelete, renderGuard, enc, esc, $val, definePage, getModel, MultiSelectModal, QuickModal } from '/static/hoover/index.js'; +// Services shown by default in the service picker. Everything else is only +// visible with the "Show all options" toggle (or while it is already +// selected on the zone). +const COMMON_SERVICES = [ + 'amqp', 'cron', 'docker', 'ftp', 'ftps', 'http', 'https', 'irc', 'ldap', + 'mysql', 'nfs', 'ntp', 'postgresql', 'radius', 'rsync', 'sip', 'smtp', + 'smtps', 'snmp', 'ssh', 'telnet', 'vnc', 'xmpp', +]; + const addZone = QuickModal({ title: 'Add Zone', fields: [ @@ -24,12 +33,12 @@ export default definePage({ const guard = renderGuard(state.firewall, 'Zones', 'Firewall zones', state.firewall.data?.zones); if (guard) return guard; - const zones = Object.keys(state.firewall.data?.zones || {}); - const activeZones = state.firewall.data?.active_zones || {}; + // Live zone data (parsed `--list-all-zones`): carries interfaces, + // services, target, and masquerade for every defined zone. + const liveZones = state.firewall.data?.zones || {}; const zoneDetails = {}; - for (const name of zones) { - const activeIfaces = activeZones[name]; - zoneDetails[name] = { interfaces: Array.isArray(activeIfaces) ? activeIfaces : [] }; + for (const name of Object.keys(liveZones)) { + zoneDetails[name] = liveZones[name] || { interfaces: [] }; } const zoneCards = Object.entries(zoneDetails).map(([name, zdata]) => { @@ -66,12 +75,34 @@ export default definePage({ selected: ifacesArr, fieldKey: 'interfaces', successMsg: 'Interfaces updated', + confirm: (b) => { + const next = (b && b.interfaces) || []; + const coveredElsewhere = new Set(); + for (const [zn, zd] of Object.entries(liveZones)) { + if (zn === name) continue; + const other = zd && Array.isArray(zd.interfaces) + ? zd.interfaces : []; + for (const i of other) coveredElsewhere.add(i); + } + const dropped = ifacesArr.filter( + i => !next.includes(i) && !coveredElsewhere.has(i)); + if (dropped.length) { + return 'Removing ' + dropped.join(', ') + ' from this ' + + 'zone leaves it in no firewall zone. Clients on ' + + 'that segment will lose all connectivity, ' + + 'including DHCP, until the interface is added ' + + 'to another zone.\n\nRemove it anyway?'; + } + return null; + }, })()}>Interfaces `, }), + uncoveredBanner, zoneCards.length ? html`
${zoneCards}
` : Empty({ text: 'No zones configured. Add a zone to get started.' }), diff --git a/webui/static/style.css b/webui/static/style.css index a11fb73..b20aafe 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -473,7 +473,130 @@ body { font-size: 1.1rem; } +.btn-link { + background: none; + border: none; + padding: 0; + font-size: 0.8rem; + font-family: inherit; + color: var(--accent); + cursor: pointer; +} +.btn-link:hover { + color: var(--accent-hover); + text-decoration: underline; +} + +/* Multi-select picker (MultiSelectModal) */ +.ms-toolbar { + display: flex; + align-items: center; + gap: 0.6rem; + margin-bottom: 0.5rem; +} + +.ms-search { + flex: 1; + padding: 0.45rem 0.7rem; + font-size: 0.9rem; + font-family: inherit; + color: var(--text); + background: var(--bg-input); + border: 1px solid var(--border); + border-radius: 6px; + outline: none; + transition: border-color 0.2s; +} + +.ms-search:focus { + border-color: var(--accent); + box-shadow: 0 0 0 3px rgba(0, 180, 216, 0.15); +} + +.ms-count { + font-size: 0.75rem; + color: var(--text-muted); + white-space: nowrap; +} + +.ms-subbar { + display: flex; + align-items: center; + gap: 0.8rem; + margin-bottom: 0.5rem; +} + +.ms-advanced { + margin-left: auto; + display: flex; + align-items: center; + gap: 0.35rem; + font-size: 0.8rem; + color: var(--text-muted); + cursor: pointer; + user-select: none; +} + +.ms-list { + max-height: 280px; + overflow-y: auto; + border: 1px solid var(--border); + border-radius: 6px; + background: var(--bg-input); +} + +.ms-row { + display: flex; + align-items: baseline; + gap: 0.5rem; + padding: 0.4rem 0.7rem; + font-size: 0.9rem; + cursor: pointer; +} + +/* Author display rules beat the [hidden] UA rule unless re-declared. */ +.ms-row[hidden] { + display: none; +} + +.ms-row:not(:last-child) { + border-bottom: 1px solid var(--border); +} + +.ms-row:hover { + background: var(--bg-secondary); +} + +.ms-check { + width: 14px; + height: 14px; + margin: 0; + flex-shrink: 0; + accent-color: var(--accent); + align-self: center; +} + +.ms-name { + font-weight: 500; + white-space: nowrap; +} + +.ms-desc { + flex: 1; + color: var(--text-muted); + font-size: 0.78rem; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.ms-empty { + padding: 1rem; + text-align: center; + color: var(--text-muted); + font-size: 0.85rem; +} /* Toggle Switch */ .toggle-switch {