From 75b86fd60d949ba653954f3680337d06063339c9 Mon Sep 17 00:00:00 2001 From: Mike Teehan Date: Tue, 1 Sep 2026 02:35:04 +0000 Subject: [PATCH] fix: ACME ownership self-heal + daily timer, apply-all force, firewall baseline re-stamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit acme: - acme.sh chmods its tree to owner-only (700/600) every run, which broke the two-user model: a tree left owner-only by one user made every acme.sh call of the other exit 2 - normalize_acme_home() reopens group access (sudo chmod g+rwX, files only — setgid dirs trip RestrictSUIDSGID); _run_acme_preflight is the choke point before every daemon acme.sh call + startup - acme service now runs as the daemon user; --log persists the raw CA transcript; SYS_LOG=6 journals manual issue/renew runs - timer daily-only: two runs/day landed inside ZeroSSL's 24h validation backoff (Retry-After: 86400) — a permanent renewal lockout - _collect_acme no longer raises on cert-list failure; reports status.error (AcmeState.status) so the certs page can surface it firewall: re-stamp the applied baseline on live zone mutations (interfaces/services/rich-rules/masquerade/forward-ports) so cancel-all reverts to post-mutation state, not a stale install-era snapshot; set_masquerade syncs the declarative config for existing zones; add_forward_port records toaddr only with toport status: apply-all accepts {"force": true} (forwarded to the firewall apply only); ApplyConfirm force checkbox; applyResultToasts() — the errors map wins over the 200; ActionButton checks errors before the success toast; dashboard uses ApplyConfirm system_import: drift re-imports carry the existing apply-meta; first import stamps the adopted content as applied (it is the running state) — no phantom pending changes nginx: get_config only re-saves when migration actually changed the config (no more owner/mtime churn on every read) install: repair mis-owned top-level system dirs (tmpfiles unsafe-path-transition), warn with a full-repair command for deeper mis-ownership daemon/server: loop.get_exception_handler() (aiohttp API fix) tests: 888 pytest + 24 node passing; ruff clean --- daemon/handlers/acme.py | 67 ++++++++++++-- daemon/handlers/firewall.py | 28 +++++- daemon/handlers/status.py | 13 ++- daemon/server.py | 13 ++- docs/api.md | 21 ++++- lib/acme.py | 4 + lib/nginx.py | 8 +- lib/schema.py | 5 +- lib/state.py | 16 ++-- lib/system_import.py | 38 +++++++- scripts/install.sh | 26 ++++++ system/sudoers.d/vacuum-walld | 8 ++ system/systemd/vacuum-wall-acme.service | 11 ++- system/systemd/vacuum-wall-acme.timer | 6 +- system/systemd/vacuum-walld.service | 6 ++ tests/test-applyconfirm.js | 35 ++++++- tests/test_firewall.py | 91 +++++++++++++++++++ tests/test_handler_acme.py | 72 +++++++++++++++ tests/test_handler_network.py | 28 +++++- tests/test_nginx.py | 24 +++++ tests/test_schema_types.py | 1 + tests/test_state.py | 38 ++++++++ tests/test_status_pending.py | 39 ++++++++ tests/test_sync.py | 12 ++- tests/test_system_import.py | 85 ++++++++++++++++- webui/api/status.py | 6 +- .../static/hoover/components/applyconfirm.js | 55 ++++++++++- webui/static/hoover/components/data.js | 21 +++-- webui/static/pages/certs.js | 10 ++ webui/static/pages/dashboard.js | 4 +- 30 files changed, 738 insertions(+), 53 deletions(-) diff --git a/daemon/handlers/acme.py b/daemon/handlers/acme.py index 6db60a6..b813741 100644 --- a/daemon/handlers/acme.py +++ b/daemon/handlers/acme.py @@ -54,6 +54,52 @@ _ACME_ENVIRON = { _WEBROOT = PROJECT_DIR / "data" / "acme" / "www" + +def normalize_acme_home() -> None: + """Restore group access on the ACME home files around acme.sh runs. + + acme.sh hardens its tree on every run (``chmod 700`` on the config + home, ``chmod 600`` on keys and confs, owned by the running user). + The daemon reopens group read/write via the sudoers whitelist so + the shared two-user model keeps the tree readable. Run BEFORE an + acme.sh invocation too: acme.sh dot-sources ``account.conf`` on + startup, so a tree left owner-only by another user's run (e.g. a + manual debug run as the WebUI user) would make every daemon acme.sh + call exit 2 — normalizing first is the only self-heal path, since a + post-run normalize is unreachable while acme.sh cannot start. + + Files only: the directories in the tree are setgid (2775, group rwx + already), and chmodding a setgid directory issues fchmodat with the + S_ISGID bit set, which the unit's ``RestrictSUIDSGID=yes`` seccomp + filter rejects with EPERM even for root. + """ + files = [str(p) for p in _ACME_HOME.rglob("*") if p.is_file()] + if not files: + return + result = lib_common.run_proc( + ["chmod", "g+rwX", *files], + sudo=True, + check=False, + timeout=10, + ) + if result.returncode != 0: + logger.warning( + "Could not normalize ACME_HOME permissions: %s", + result.stderr.strip() or f"exit code {result.returncode}", + ) + + +def _run_acme_preflight(args: list[str]) -> str: + """Normalize ACME home permissions, then run acme.sh with *args*. + + Single choke point for every daemon acme.sh invocation: the + preflight normalize makes the run succeed even if a prior run by + another user left the tree owner-only. + """ + normalize_acme_home() + return _run_acme(args) + + # In-memory store for active issuance requests. _ISSUANCES: dict[str, "IssueRequest"] = {} @@ -839,14 +885,16 @@ async def _run_issue(req: IssueRequest) -> None: args.append("--force") # acme.sh is a blocking subprocess — run it off the event loop so # polling, WS broadcasts, and other requests keep responding. - output = await asyncio.to_thread(_run_acme, args) + output = await asyncio.to_thread(_run_acme_preflight, args) + normalize_acme_home() req.steps[0].status = "done" req.steps[0].message = output.strip()[:200] # Step 2: deploy req.steps[1].status = "running" await asyncio.to_thread( - _run_acme, ["--deploy", "-d", req.domain, "--deploy-hook", _DEPLOY_HOOK] + _run_acme_preflight, + ["--deploy", "-d", req.domain, "--deploy-hook", _DEPLOY_HOOK], ) req.steps[1].status = "done" req.steps[1].message = "Deploy hook registered" @@ -951,7 +999,9 @@ async def _run_renew(req: IssueRequest, force: bool) -> None: args: list[str] = ["--renew", "-d", req.domain] if force: args.append("--force") - output = await asyncio.to_thread(_run_acme, args) + output = await asyncio.to_thread(_run_acme_preflight, args) + # acme.sh hardens its tree even when it skips — normalize first. + normalize_acme_home() if "Skipping." in output: req.steps[0].status = "done" req.steps[0].message = "Renewal not yet due — skipped" @@ -968,7 +1018,8 @@ async def _run_renew(req: IssueRequest, force: bool) -> None: # Step 2: deploy req.steps[1].status = "running" await asyncio.to_thread( - _run_acme, ["--deploy", "-d", req.domain, "--deploy-hook", _DEPLOY_HOOK] + _run_acme_preflight, + ["--deploy", "-d", req.domain, "--deploy-hook", _DEPLOY_HOOK], ) req.steps[1].status = "done" req.steps[1].message = "Deploy hook registered" @@ -1003,7 +1054,7 @@ def remove_cert(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: domain = body.get("domain", "").strip() if not domain: raise ValueError("'domain' is required") - _run_acme(["--remove", "-d", domain]) + _run_acme_preflight(["--remove", "-d", domain]) logger.info("Certificate for %s removed", domain) refresh_state(["acme"]) return {"domain": domain} @@ -1021,7 +1072,7 @@ def set_email(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: email = body.get("email", "").strip() if not email: raise ValueError("'email' is required") - _run_acme(["--register-account", "-m", email]) + _run_acme_preflight(["--register-account", "-m", email]) # Persist to declarative ACME config acme_cfg = PROJECT_DIR / "config" / "acme" / "config.json" acme_cfg.parent.mkdir(parents=True, exist_ok=True) @@ -1159,7 +1210,7 @@ def register_account(_request: Any, body: dict[str, Any] | None) -> dict[str, An raise ValueError("Invalid email format") server = (body.get("server") or "letsencrypt").strip() - _run_acme(["--register-account", "-m", email, "--server", server]) + _run_acme_preflight(["--register-account", "-m", email, "--server", server]) acme_cfg = PROJECT_DIR / "config" / "acme" / "config.json" acme_cfg.parent.mkdir(parents=True, exist_ok=True) @@ -1179,7 +1230,7 @@ def register_account(_request: Any, body: dict[str, Any] | None) -> dict[str, An def deactivate_account(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: """DELETE /acme/account/deactivate — deactivate the ACME account.""" try: - _run_acme(["--deactivate-account"]) + _run_acme_preflight(["--deactivate-account"]) except RuntimeError as exc: logger.warning("acme.sh deactivate failed: %s", exc) acme_cfg = PROJECT_DIR / "config" / "acme" / "config.json" diff --git a/daemon/handlers/firewall.py b/daemon/handlers/firewall.py index 238f645..e553687 100644 --- a/daemon/handlers/firewall.py +++ b/daemon/handlers/firewall.py @@ -856,6 +856,9 @@ def set_zone_interfaces(_request: Any, body: dict[str, Any] | None) -> dict[str, old_zone_cfg["interfaces"] = new_ifaces elif "interfaces" in old_zone_cfg: del old_zone_cfg["interfaces"] + # This mutation already applied to live firewalld, so re-stamp the applied + # baseline: cancel-all must revert to this state, not an older snapshot. + stamp_applied(cfg) _save_config(cfg) logger.info("Zone '%s' interfaces set to %s", zone, interfaces) @@ -927,9 +930,12 @@ def set_zone_services(_request: Any, body: dict[str, Any] | None) -> dict[str, A _reload() # Keep the declarative config in sync so the next apply does not - # reconcile the live services back to the stale config value. + # reconcile the live services back to the stale config value. The + # mutation already applied to live firewalld, so re-stamp the applied + # baseline: cancel-all must revert to this state, not an older snapshot. cfg = _get_config() cfg.setdefault("zones", {}).setdefault(zone, {})["services"] = list(services) + stamp_applied(cfg) _save_config(cfg) logger.info("Zone '%s' services set to %s", zone, services) sync_result = bus.emit( @@ -979,6 +985,7 @@ def add_rich_rule(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: rule_id = uuid4().hex[:8] entry = {"id": rule_id, "rule": rule} cfg["zones"][zone]["rich_rules"].append(entry) + stamp_applied(cfg) _save_config(cfg) sync_result = bus.emit( SyncEvent( @@ -1035,6 +1042,7 @@ def remove_rich_rule(_request: Any, body: dict[str, Any] | None) -> dict[str, An zone_cfg["rich_rules"] = [ r for r in zone_cfg.get("rich_rules", []) if r.get("id") != rule_id ] + stamp_applied(cfg) _save_config(cfg) sync_result = bus.emit( SyncEvent( @@ -1084,6 +1092,10 @@ def list_rich_rules(_request: Any, body: dict[str, Any] | None) -> list[dict[str def set_masquerade(_request: Any, body: dict[str, Any] | None) -> dict[str, Any]: """Enable/disable masquerade on a zone. + Also syncs the declarative config (and re-stamps the applied baseline) + when the zone exists in the config, so the pending diff and cancel-all + stay consistent with the live zone. + Args: _request: The incoming HTTP request (unused). body: JSON body with ``zone`` and ``enable`` (boolean). @@ -1108,6 +1120,16 @@ def set_masquerade(_request: Any, body: dict[str, Any] | None) -> dict[str, Any] action = "--add-masquerade" if enable else "--remove-masquerade" run(["firewall-cmd", f"--zone={zone}", action, "--permanent"], sudo=True) _reload() + # Keep the declarative config in sync with the live zone so the pending + # diff and the cancel-all baseline stay consistent. Only touch zones that + # already exist in the config — creating a bare zone entry would + # manufacture spurious service/interface diffs on the next poll. + cfg = _get_config() + zone_cfg = cfg.get("zones", {}).get(zone) + if isinstance(zone_cfg, dict): + zone_cfg["masquerade"] = bool(enable) + stamp_applied(cfg) + _save_config(cfg) sync_result = bus.emit( SyncEvent( "firewall", "config_saved", {"action": "masquerade_set", "zone": zone} @@ -1161,13 +1183,14 @@ def add_forward_port(_request: Any, body: dict[str, Any] | None) -> dict[str, An _reload() fp_id = uuid4().hex[:8] entry: dict[str, Any] = {"id": fp_id, "port": int(port), "proto": proto} - if toaddr: + if toaddr and toport: entry["toaddr"] = toaddr if toport: entry["toport"] = int(toport) cfg = _get_config() cfg.setdefault("zones", {}).setdefault(zone, {}).setdefault("forward_ports", []) cfg["zones"][zone]["forward_ports"].append(entry) + stamp_applied(cfg) _save_config(cfg) sync_result = bus.emit( SyncEvent( @@ -1233,6 +1256,7 @@ def remove_forward_port(_request: Any, body: dict[str, Any] | None) -> dict[str, cfg["zones"][zone]["forward_ports"] = [ fp for fp in fps if not (fp.get("port") == port and fp.get("proto") == proto) ] + stamp_applied(cfg) _save_config(cfg) sync_result = bus.emit( SyncEvent( diff --git a/daemon/handlers/status.py b/daemon/handlers/status.py index 57222f1..5a16d3d 100644 --- a/daemon/handlers/status.py +++ b/daemon/handlers/status.py @@ -128,12 +128,20 @@ def status_apply_all(_request: Any, _body: Any) -> dict[str, Any]: Order: network -> firewall -> wireguard -> dnsmasq -> nginx. + Args: + _request: The incoming HTTP request (unused). + _body: Optional JSON body; ``{"force": true}`` is forwarded to the + firewall apply, overriding its management-lockout and + interface-coverage guards. Other subsystems ignore it. + Returns: Dict with applied subsystems and any errors encountered. """ applied = [] errors = {} + force = bool(_body and _body.get("force")) + pending_data = status_pending(None, None) fw_pending = pending_data["firewall"]["needs_apply"] hash_pending = { @@ -153,7 +161,10 @@ def status_apply_all(_request: Any, _body: Any) -> dict[str, Any]: handler = SYS_APPLY[name] try: - handler(None, None) + # Only the firewall apply honors `force` (its lockout and + # coverage guards); forward it there, not to other subsystems. + body = {"force": True} if (name == "firewall" and force) else None + handler(None, body) applied.append(name) except Exception as exc: label = SYS_LABELS.get(name, name) diff --git a/daemon/server.py b/daemon/server.py index fcbc1d9..5eb86b4 100644 --- a/daemon/server.py +++ b/daemon/server.py @@ -698,7 +698,7 @@ def main() -> None: _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 + prev_handler = loop.get_exception_handler() loop.set_exception_handler(_teardown_exception_handler) try: # Stop accepting new connections (also waits for open sockets, @@ -747,6 +747,17 @@ def main() -> None: if reconciled: logger.info("Reconciled subsystems: %s", ", ".join(reconciled)) + # Reopen group access on the ACME home before the first acme.sh + # collection: a tree left owner-only by a prior run (e.g. a manual + # run as the WebUI user) would otherwise fail every daemon acme.sh + # call until the next issue/renew. Never fatal at startup. + try: + from daemon.handlers.acme import normalize_acme_home + + normalize_acme_home() + except Exception: + logger.warning("ACME home normalization failed at startup", exc_info=True) + # Populate state from system (blocking — OK at startup) logger.info("Populating system state...") state_store.populate() diff --git a/docs/api.md b/docs/api.md index 9852b20..80f86bc 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1976,6 +1976,16 @@ POST /api/status/apply-all Apply pending changes for all subsystems in dependency order. +**Request Body (optional):** + +```json +{ "force": true } +``` + +`force` is forwarded to the firewall apply only — it overrides the +management-lockout and interface-coverage guards. Other subsystems +ignore it. + **Response (`data`):** | Field | Type | Description | @@ -1983,10 +1993,13 @@ Apply pending changes for all subsystems in dependency order. | `applied` | `[string, ...]` | List of subsystems that were applied | | `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. +The endpoint returns `200` even when some subsystems failed — per-subsystem +failures are reported in `errors`, so clients must check `errors` (not just +the HTTP status) before reporting success. Without `force`, the firewall +apply refuses if an interface would be left without zone coverage (the +coverage guard) or both https/ssh would be stripped from the default zone +(lockout guard); the `ConflictError` surfaces in `errors` under +`"Firewall"` while the other subsystems proceed. --- diff --git a/lib/acme.py b/lib/acme.py index 286f0d9..0372531 100644 --- a/lib/acme.py +++ b/lib/acme.py @@ -95,6 +95,10 @@ def _run_acme(args: list[str]) -> str: acme_home_env, "--config-home", acme_home_env, + # Append the full transcript to $ACME_HOME/acme.sh.log so manual + # runs (whose stdout is captured below) leave a persistent record + # of the raw CA exchange. + "--log", *args, ] diff --git a/lib/nginx.py b/lib/nginx.py index 91afd6f..ad9e923 100644 --- a/lib/nginx.py +++ b/lib/nginx.py @@ -181,12 +181,18 @@ def get_config() -> dict[str, Any]: raw = load_json(CONFIG_FILE) if not raw: raw = deepcopy(DEFAULT_CONFIG) + save_config(raw) + return raw if "ssl" not in raw: raw["ssl"] = deepcopy(DEFAULT_SSL) if "backends" not in raw: raw["backends"] = {} + pre = deepcopy(raw) raw = _migrate_config(raw) - save_config(raw) + # Read-only unless normalization/migration actually changed the config; + # re-saving on every read rewrites the file (owner/mtime churn). + if raw != pre: + save_config(raw) return raw diff --git a/lib/schema.py b/lib/schema.py index 233a711..f84796f 100644 --- a/lib/schema.py +++ b/lib/schema.py @@ -280,15 +280,18 @@ class AcmeState(TypedDict): """ACME state (collector: `_collect_acme`). Attributes: - certs: Certificate list. + certs: Certificate list (empty when collection failed). email: Registered ACME email. account: Account status (see AcmeAccount). + status: Collection status; ``error`` is ``None`` on success or + the failure message when cert collection was not possible. timestamp: ISO-8601 collection time. """ certs: list[AcmeCert] email: str account: AcmeAccount + status: dict[str, str | None] timestamp: str diff --git a/lib/state.py b/lib/state.py index 10cd526..860ffb0 100644 --- a/lib/state.py +++ b/lib/state.py @@ -914,16 +914,19 @@ def _collect_acme() -> schema.AcmeState: """ email = _get_acme_email() + # Non-fatal: a broken acme.sh (e.g. unreadable account.conf after an + # ownership flip) must not blank the whole dashboard via a cleared + # state store. Collect what we can and surface the failure in + # `status.error` so the poll diff still detects recovery. + cert_error: str | None = None try: from lib.acme import list_certs certs = list_certs() - except Exception: - logger.warning( - "ACME state collection failed, returning empty cert list", - exc_info=True, - ) - raise + except Exception as exc: + logger.warning("ACME state collection failed", exc_info=True) + certs = [] + cert_error = str(exc) account = _parse_account_conf() @@ -931,6 +934,7 @@ def _collect_acme() -> schema.AcmeState: "certs": certs, "email": email, "account": account, + "status": {"error": cert_error}, "timestamp": _now_iso(), } diff --git a/lib/system_import.py b/lib/system_import.py index 2e99d03..46754ef 100644 --- a/lib/system_import.py +++ b/lib/system_import.py @@ -8,10 +8,18 @@ caused by install.sh or manual edits to system files. import contextlib import logging import re +from copy import deepcopy from pathlib import Path from typing import Any -from lib.common import load_json, run, save_json +from lib.common import ( + _APPLY_HASH_KEY, + _LAST_APPLIED_CONFIG_KEY, + load_json, + run, + save_json, + stamp_applied, +) from lib.firewall import _live_target_to_config, _parse_all_zones_output logger = logging.getLogger(__name__) @@ -53,6 +61,24 @@ def import_all() -> list[str]: return updated +def _carry_apply_meta(cfg: dict[str, Any], existing: dict[str, Any]) -> None: + """Preserve apply bookkeeping when adopting live system state. + + Imported content replaces the declarative config but must not destroy + the applied-state baseline. When *existing* carries apply meta keys, + they are copied over so pending-change detection and cancel-all keep + working against the last-applied baseline. When no baseline exists + (first import), *cfg* is stamped as applied — the imported content is + exactly the state the system is currently running. + """ + if _APPLY_HASH_KEY in existing or _LAST_APPLIED_CONFIG_KEY in existing: + for key in (_APPLY_HASH_KEY, _LAST_APPLIED_CONFIG_KEY): + if key in existing: + cfg[key] = deepcopy(existing[key]) + else: + stamp_applied(cfg) + + # ------------------------------------------------------------------ # Dnsmasq # ------------------------------------------------------------------ @@ -86,12 +112,13 @@ def import_dnsmasq() -> bool: return False cfg_path = PROJECT_DIR / "config" / "dnsmasq" / "config.json" + existing: dict[str, Any] = {} if cfg_path.exists(): existing = load_json(cfg_path) if _cfgs_equal(existing, cfg): logger.debug("Skipping dnsmasq: config already matches") return False - + _carry_apply_meta(cfg, existing) save_json(cfg_path, cfg) summary = f"upstreams={len(cfg.get('dns', {}).get('upstreams', []))}, ranges={len(cfg.get('dhcp', {}).get('ranges', []))}" logger.info("Imported dnsmasq config from %s: %s", DNSMASQ_CONF, summary) @@ -247,12 +274,13 @@ def import_wireguard() -> bool: return False cfg_path = PROJECT_DIR / "config" / "wireguard" / "config.json" + existing: dict[str, Any] = {} if cfg_path.exists(): existing = load_json(cfg_path) if _cfgs_equal(existing, cfg): logger.debug("Skipping wireguard: config already matches") return False - + _carry_apply_meta(cfg, existing) save_json(cfg_path, cfg) peer_count = len(cfg.get("peers", {})) logger.info("Imported wireguard config from %s: peers=%d", WG_CONF, peer_count) @@ -941,7 +969,9 @@ def import_firewall() -> bool: logger.debug("Skipping firewall: no zones with interfaces") return False - save_json(cfg_path, {"zones": zone_configs}) + # Only reached when the config file is absent: the imported zones are + # exactly what firewalld is running, so stamp them as the applied state. + save_json(cfg_path, stamp_applied({"zones": zone_configs})) logger.info( "Imported firewall config: zones=%s", ", ".join(zone_configs.keys()), diff --git a/scripts/install.sh b/scripts/install.sh index da8c947..48cb069 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -144,6 +144,32 @@ fi # Shared group: use the WebUI user's primary group USER_GROUP=$(id -gn "$USER_NAME") +# Some appliance images ship with top-level system directories (and sometimes +# everything under them) owned by a regular user. This trips systemd-tmpfiles' +# "unsafe path transition" check and lets that user modify system paths. +# Repair the top level here; warn with a full-repair command if deeper +# mis-ownership is detected (depth-1 entries of /etc /usr /var /boot are +# always root-owned on Debian, so this check cannot false-positive). +_sys_dirs=(/ /bin /boot /etc /home /media /mnt /opt /root /sbin /srv /usr /var /var/lib /var/log) +_misowned=() +for _d in "${_sys_dirs[@]}"; do + [[ -e "$_d" ]] || continue + [[ "$(stat -c '%U' "$_d" 2>/dev/null)" == "root" ]] || _misowned+=("$_d") +done +if [[ ${#_misowned[@]} -gt 0 ]]; then + warn "System directories not owned by root: ${_misowned[*]}" + warn "Chowning to root:root (image shipped with mis-owned system paths)." + chown root:root "${_misowned[@]}" + _deep_count=$(find /etc /usr /var /boot -maxdepth 1 ! -user root 2>/dev/null | wc -l) + if [[ "$_deep_count" -gt 0 ]]; then + warn "Deeper mis-ownership detected ($_deep_count entries at depth 1)." + warn "Run a full repair, then re-run this installer:" + warn " sudo find / -xdev -path /proc -prune -o -path /sys -prune -o -path /dev -prune -o -path /run -prune -o -path /tmp -prune -o -path /home/$USER_NAME -prune -o -user $USER_NAME -print0 | xargs -0 -r chown root:root" + else + log "Repaired top-level system directory ownership." + fi +fi + echo "============================================" echo " Vacuum Wall Appliance Installer" echo " Install dir: $PROJECT_DIR" diff --git a/system/sudoers.d/vacuum-walld b/system/sudoers.d/vacuum-walld index aa9c898..fdeac31 100644 --- a/system/sudoers.d/vacuum-walld +++ b/system/sudoers.d/vacuum-walld @@ -45,6 +45,14 @@ Defaults:{{ USER_DAEMON_NAME }} secure_path="/usr/local/sbin:/usr/local/bin:/usr # Sysctl {{ USER_DAEMON_NAME }} ALL=(root) NOPASSWD: /usr/sbin/sysctl -w * +# ACME home permissions (acme.sh chmods its tree to owner-only modes: +# 700 on the config home, 600 on keys/confs — group access must be +# reopened so the shared two-user model can read the tree). Files only: +# the setgid directories (2775) already grant group rwx, and chmodding +# them would trip the daemon unit's RestrictSUIDSGID seccomp filter. +# The trailing * spans the file argument list. +{{ USER_DAEMON_NAME }} ALL=(root) NOPASSWD: /usr/bin/chmod g+rwX {{ ACME_HOME }}/* + # Misc {{ USER_DAEMON_NAME }} ALL=(root) NOPASSWD: /usr/bin/journalctl --unit=* -n * {{ USER_DAEMON_NAME }} ALL=(root) NOPASSWD: /usr/bin/cat /var/log/nginx/* diff --git a/system/systemd/vacuum-wall-acme.service b/system/systemd/vacuum-wall-acme.service index 1769a19..97e9dfc 100644 --- a/system/systemd/vacuum-wall-acme.service +++ b/system/systemd/vacuum-wall-acme.service @@ -3,8 +3,15 @@ Description=Vacuum Wall ACME Certificate Renewal [Service] Type=oneshot -User={{ USER_NAME }} +# Run as the daemon user, not the WebUI user: it owns the project tree +# (and the ACME home) in production, and acme.sh chmods its config home +# to 700 and its keys/confs to 600 on every run. Running as the WebUI +# user left the tree unreadable to the daemon (and vice versa) whenever +# the two users' runs interleaved. +User={{ USER_DAEMON_NAME }} WorkingDirectory={{ PROJECT_DIR }} Environment=ACME_HOME={{ PROJECT_DIR }}/data/acme Environment=HOME={{ PROJECT_DIR }} -ExecStart={{ ACME_HOME }}/acme.sh --cron --home {{ ACME_HOME }} --config-home {{ ACME_HOME }} +# --log: persistent on-disk transcript of the raw CA exchange (journald +# captures stdout regardless; the file survives journal retention). +ExecStart={{ ACME_HOME }}/acme.sh --cron --home {{ ACME_HOME }} --config-home {{ ACME_HOME }} --log diff --git a/system/systemd/vacuum-wall-acme.timer b/system/systemd/vacuum-wall-acme.timer index c38afd4..dc5a71b 100644 --- a/system/systemd/vacuum-wall-acme.timer +++ b/system/systemd/vacuum-wall-acme.timer @@ -2,8 +2,12 @@ Description=Vacuum Wall ACME Certificate Renewal Timer [Timer] +# Daily only: ZeroSSL backs off a failed validation for 24h per domain +# (Retry-After: 86400). With two runs a day every attempt landed inside +# the previous attempt's backoff window, re-arming it — a permanent +# renewal lockout. Attempts >24h apart are required for the backoff to +# ever expire (acme.sh discussion #6419). OnCalendar=*-*-* 00:00:00 -OnCalendar=*-*-* 12:00:00 Persistent=true RandomizedDelaySec=300 diff --git a/system/systemd/vacuum-walld.service b/system/systemd/vacuum-walld.service index 33f9259..4c4d241 100644 --- a/system/systemd/vacuum-walld.service +++ b/system/systemd/vacuum-walld.service @@ -16,6 +16,12 @@ TimeoutStopSec=15 Environment=PATH=/usr/local/bin:/usr/bin Environment=PYTHONUNBUFFERED=1 Environment=ACME_HOME={{ PROJECT_DIR }}/data/acme +# acme.sh routes its _info/_err lines through logger(1) -> journald when +# SYS_LOG is set (default: off). This journals manual issue/renew runs +# in real time under this unit, whose subprocess stdout is otherwise +# captured by the daemon and never seen by the journal. +# Levels: 3=error, 6=info, 7=debug. +Environment=SYS_LOG=6 Environment=HOME={{ PROJECT_DIR }} # Runtime directories created before namespace setup. ProtectSystem=strict diff --git a/tests/test-applyconfirm.js b/tests/test-applyconfirm.js index d3922d4..2fd12eb 100644 --- a/tests/test-applyconfirm.js +++ b/tests/test-applyconfirm.js @@ -5,7 +5,7 @@ * and integration behaviour. Run with `node tests/test-applyconfirm.js`. */ -import { buildRows, isPending, SUBSYSTEM_LIST } from '../webui/static/hoover/components/applyconfirm.js'; +import { buildRows, isPending, SUBSYSTEM_LIST, applyResultToasts } from '../webui/static/hoover/components/applyconfirm.js'; const SUBSYSTEM_KEYS = SUBSYSTEM_LIST.map(s => s.key); @@ -235,5 +235,38 @@ test('buildRows row VNodes have correct tag', () => { } }); +// === applyResultToasts === +// apply-all returns 200 with { applied, errors } even when subsystems +// failed — resp.ok alone is not a success signal; errors must win. +test('applyResultToasts: errors suppress the success toast', () => { + const t = applyResultToasts({ applied: ['Network'], errors: { Firewall: 'refused' } }, 'All changes applied'); + assertEq(t.success, null, 'no success toast when errors exist'); + assertIncludes(t.error, 'Firewall — refused'); +}); + +test('applyResultToasts: success toast when applied and no errors', () => { + const t = applyResultToasts({ applied: ['Firewall', 'Nginx'], errors: {} }, 'All changes applied'); + assertEq(t.error, null); + assertEq(t.success, 'All changes applied'); +}); + +test('applyResultToasts: no toast when nothing applied and no errors', () => { + const t = applyResultToasts({ applied: [], errors: {} }, 'All changes applied'); + assertEq(t.error, null); + assertEq(t.success, null); +}); + +test('applyResultToasts: multiple errors are joined', () => { + const t = applyResultToasts({ applied: [], errors: { Firewall: 'a', Nginx: 'b' } }, 'ok'); + assertIncludes(t.error, 'Firewall — a'); + assertIncludes(t.error, 'Nginx — b'); +}); + +test('applyResultToasts: null payload is safe', () => { + const t = applyResultToasts(null, 'ok'); + assertEq(t.error, null); + assertEq(t.success, null); +}); + console.log(`\n${passed} passed, ${failed} failed`); process.exit(failed > 0 ? 1 : 0); \ No newline at end of file diff --git a/tests/test_firewall.py b/tests/test_firewall.py index 55703f1..b4810f2 100644 --- a/tests/test_firewall.py +++ b/tests/test_firewall.py @@ -1099,6 +1099,97 @@ class TestDaemonConfigApplyStamp: assert saved[_LAST_APPLIED_CONFIG_KEY] == _STAMP_TEST_CFG +class TestDaemonMutatorBaselineStamp: + """Per-zone mutations apply to live firewalld immediately and must + re-stamp the applied baseline, so cancel-all reverts to the post-mutation + state instead of an older snapshot (regression: stale install-era + snapshot resurrected a phantom 'remove interface' pending change). + """ + + ZONES_OUT = "public\ninternal" + + @patch("daemon.handlers.firewall._reload") + @patch("daemon.handlers.firewall._get_config", return_value={"zones": {}}) + @patch("daemon.handlers.firewall.run", return_value=ZONES_OUT) + 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"), + ): + mock_bus.emit.return_value = MagicMock(affected_subsystems=[]) + daemonfirewall.set_zone_interfaces( + None, {"zone": "internal", "interfaces": ["eth1"]} + ) + saved = mock_save.call_args[0][0] + assert saved["zones"]["internal"]["interfaces"] == ["eth1"] + assert saved[_LAST_APPLIED_CONFIG_KEY] == strip_apply_meta(saved) + assert saved[_APPLY_HASH_KEY] == config_hash(saved) + assert saved[_LAST_APPLIED_CONFIG_KEY]["zones"]["internal"]["interfaces"] == [ + "eth1" + ] + + @patch("daemon.handlers.firewall._reload") + @patch("daemon.handlers.firewall._get_config", return_value={"zones": {}}) + @patch("daemon.handlers.firewall.run", return_value=ZONES_OUT) + def test_set_zone_services_stamps_baseline(self, mock_run, mock_cfg, mock_reload): + with ( + patch.object( + 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"), + ): + mock_bus.emit.return_value = MagicMock(affected_subsystems=[]) + daemonfirewall.set_zone_services( + None, {"zone": "internal", "services": ["ssh"]} + ) + saved = mock_save.call_args[0][0] + assert saved["zones"]["internal"]["services"] == ["ssh"] + assert saved[_LAST_APPLIED_CONFIG_KEY]["zones"]["internal"]["services"] == [ + "ssh" + ] + + @patch("daemon.handlers.firewall._reload") + @patch( + "daemon.handlers.firewall._get_config", + return_value={"zones": {"internal": {}}}, + ) + @patch("daemon.handlers.firewall.run") + def test_set_masquerade_syncs_config_and_stamps( + 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"), + ): + mock_bus.emit.return_value = MagicMock(affected_subsystems=[]) + daemonfirewall.set_masquerade(None, {"zone": "internal", "enable": True}) + saved = mock_save.call_args[0][0] + assert saved["zones"]["internal"]["masquerade"] is True + assert saved[_LAST_APPLIED_CONFIG_KEY] == strip_apply_meta(saved) + assert saved[_APPLY_HASH_KEY] == config_hash(saved) + + @patch("daemon.handlers.firewall._reload") + @patch("daemon.handlers.firewall._get_config", return_value={"zones": {}}) + @patch("daemon.handlers.firewall.run") + def test_set_masquerade_no_config_entry_skips_write( + self, mock_run, mock_cfg, mock_reload + ): + """A zone absent from the config must not gain a bare entry — that + 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"), + ): + mock_bus.emit.return_value = MagicMock(affected_subsystems=[]) + daemonfirewall.set_masquerade(None, {"zone": "public", "enable": False}) + mock_save.assert_not_called() + + class TestDaemonGetConfigEndpoint: def test_strips_apply_meta(self): with patch.object( diff --git a/tests/test_handler_acme.py b/tests/test_handler_acme.py index 3c3ed28..b85bc91 100644 --- a/tests/test_handler_acme.py +++ b/tests/test_handler_acme.py @@ -1,6 +1,7 @@ """Tests for daemon/handlers/acme.py — handler endpoint logic.""" import asyncio +import inspect import urllib.error from pathlib import Path from unittest.mock import MagicMock, patch @@ -1303,3 +1304,74 @@ class TestGetRenewStatus: assert status["domain"] == "example.com" assert status["status"] == "completed" assert [s["name"] for s in status["steps"]] == ["renew", "deploy", "refresh"] + + +class TestNormalizeAcmeHome: + def test_normalize_invokes_sudo_chmod_on_files(self, tmp_path): + f1 = tmp_path / "account.conf" + f1.write_text("x") + (tmp_path / "sub").mkdir() + f2 = tmp_path / "sub" / "dom.key" + f2.write_text("x") + with ( + patch("daemon.handlers.acme._ACME_HOME", tmp_path), + patch( + "lib.common.run_proc", + return_value=MagicMock(returncode=0, stderr=""), + ) as mock_proc, + ): + acme_mod.normalize_acme_home() + args = mock_proc.call_args.args[0] + assert args[:2] == ["chmod", "g+rwX"] + assert set(args[2:]) == {str(f1), str(f2)} + mock_proc.assert_called_once() + + def test_normalize_empty_tree_skips_sudo(self, tmp_path): + with ( + patch("daemon.handlers.acme._ACME_HOME", tmp_path), + patch("lib.common.run_proc") as mock_proc, + ): + acme_mod.normalize_acme_home() + mock_proc.assert_not_called() + + def test_normalize_failure_does_not_raise(self, tmp_path): + (tmp_path / "a.conf").write_text("x") + with ( + patch("daemon.handlers.acme._ACME_HOME", tmp_path), + patch( + "lib.common.run_proc", + return_value=MagicMock(returncode=1, stderr="denied"), + ), + patch.object(acme_mod, "logger"), + ): + acme_mod.normalize_acme_home() + + def test_preflight_normalizes_before_run(self): + calls = [] + with ( + patch.object( + acme_mod, + "normalize_acme_home", + side_effect=lambda: calls.append("normalize"), + ), + patch.object( + acme_mod, + "_run_acme", + side_effect=lambda args: calls.append("run:" + " ".join(args)) or "ok", + ), + ): + out = acme_mod._run_acme_preflight(["--list", "--listraw"]) + assert calls == ["normalize", "run:--list --listraw"] + assert out == "ok" + + +class TestPreflightWiring: + def test_issue_uses_preflight(self): + source = inspect.getsource(acme_mod._run_issue) + assert "_run_acme_preflight" in source + assert "normalize_acme_home" in source + + def test_renew_uses_preflight(self): + source = inspect.getsource(acme_mod._run_renew) + assert "_run_acme_preflight" in source + assert "normalize_acme_home" in source diff --git a/tests/test_handler_network.py b/tests/test_handler_network.py index 7e151bb..1bd3128 100644 --- a/tests/test_handler_network.py +++ b/tests/test_handler_network.py @@ -15,18 +15,36 @@ from daemon.handlers.network import ( save_interface, set_sysctl, ) +from lib import dnsmasq as _dm +from lib import firewall as _fw from lib import network as _net @pytest.fixture def tmp_network(tmp_path): - orig_config = _net.CONFIG_FILE - orig_data = _net.DATA_DIR + # Handler endpoints emit "networkd" sync events; the subscribers + # (lib.sync.NetworkToAllSync) read/write the firewall and dnsmasq + # configs, and apply_all re-stamps the dnsmasq config. Point all of + # those paths at tmp so tests never touch the real config files. + orig_net = (_net.CONFIG_FILE, _net.DATA_DIR) + orig_dm = (_dm.CONFIG_DIR, _dm.DATA_DIR, _dm.CONFIG_PATH, _dm.FRAGMENTS_DIR) + orig_fw = (_fw.CONFIG_DIR, _fw.CONFIG_FILE) _net.CONFIG_FILE = tmp_path / "config" / "network" / "config.json" _net.DATA_DIR = tmp_path / "data" / "networkd" + _dm.CONFIG_DIR = tmp_path / "config" / "dnsmasq" + _dm.DATA_DIR = tmp_path / "data" / "dnsmasq" + _dm.CONFIG_PATH = _dm.CONFIG_DIR / "config.json" + _dm.FRAGMENTS_DIR = _dm.DATA_DIR / "fragments" + _dm.CONFIG_DIR.mkdir(parents=True, exist_ok=True) + _dm.DATA_DIR.mkdir(parents=True, exist_ok=True) + _dm.FRAGMENTS_DIR.mkdir(parents=True, exist_ok=True) + _fw.CONFIG_DIR = tmp_path / "config" / "firewall" + _fw.CONFIG_FILE = _fw.CONFIG_DIR / "config.json" + _fw.CONFIG_DIR.mkdir(parents=True, exist_ok=True) yield tmp_path - _net.CONFIG_FILE = orig_config - _net.DATA_DIR = orig_data + _net.CONFIG_FILE, _net.DATA_DIR = orig_net + _dm.CONFIG_DIR, _dm.DATA_DIR, _dm.CONFIG_PATH, _dm.FRAGMENTS_DIR = orig_dm + _fw.CONFIG_DIR, _fw.CONFIG_FILE = orig_fw # ================================================================= @@ -363,7 +381,7 @@ class TestInferEndpoints: class TestSetSysctl: - def test_set_sysctl_success(self): + def test_set_sysctl_success(self, tmp_network): with ( patch("daemon.handlers.network.run") as mock_run, patch.object(Path, "read_text", return_value="1"), diff --git a/tests/test_nginx.py b/tests/test_nginx.py index bfae50f..dc09836 100644 --- a/tests/test_nginx.py +++ b/tests/test_nginx.py @@ -55,6 +55,30 @@ class TestGetConfig: assert "ssl" in cfg assert cfg["domains"] == {} + def test_read_does_not_rewrite_unchanged_file(self, temp_data_dir): + """get_config() must not re-save a file that needs no migration.""" + nginx.save_config( + { + "backends": {"webui": {"_migrated": True, "paths": {}}}, + "domains": {"app.example.com": {"backend": "webui"}}, + "ssl": {"protocols": "TLSv1.3"}, + } + ) + mtime_before = nginx.CONFIG_FILE.stat().st_mtime_ns + cfg = nginx.get_config() + assert cfg["domains"] == {"app.example.com": {"backend": "webui"}} + # No churn: reading a current-format config leaves the file alone. + assert nginx.CONFIG_FILE.stat().st_mtime_ns == mtime_before + + def test_read_saves_when_migration_applied(self, temp_data_dir): + """get_config() persists the file when migration actually changes it.""" + nginx.save_config({"domains": {"app.example.com": {"backend": "myapp"}}}) + mtime_before = nginx.CONFIG_FILE.stat().st_mtime_ns + cfg = nginx.get_config() + # Migration added the builtin webui backend. + assert cfg["backends"]["webui"]["_migrated"] is True + assert nginx.CONFIG_FILE.stat().st_mtime_ns != mtime_before + class TestSaveConfig: def test_saves_and_reloads(self, temp_data_dir): diff --git a/tests/test_schema_types.py b/tests/test_schema_types.py index f4a9bd1..81569a6 100644 --- a/tests/test_schema_types.py +++ b/tests/test_schema_types.py @@ -100,6 +100,7 @@ class TestCollectorShapesMatchSchema: result = lib.state._collect_acme() assert not _missing(schema.AcmeState.__required_keys__, result) + assert result["status"]["error"] is None def test_wireguard_state(self): with patch.object(lib.state, "run_proc") as mock_proc: diff --git a/tests/test_state.py b/tests/test_state.py index 4e791ae..2356039 100644 --- a/tests/test_state.py +++ b/tests/test_state.py @@ -3,6 +3,7 @@ import json from unittest.mock import patch +import lib from lib.state import State, state @@ -249,6 +250,43 @@ class TestCollectFailure: assert s.is_populated() is False +_ACCOUNT = {"registered": False, "email": "", "ca": ""} + + +class TestAcmeCollectNonFatal: + """A broken acme.sh must not clear the acme subsystem (dashboard guard).""" + + def test_list_failure_yields_empty_certs_and_error(self): + from lib.state import _collect_acme + + with ( + patch.object(lib.state, "_get_acme_email", return_value="a@b.c"), + patch( + "lib.acme.list_certs", + side_effect=RuntimeError("acme.sh failed with exit code 2"), + ), + patch.object(lib.state, "_parse_account_conf", return_value=_ACCOUNT), + ): + result = _collect_acme() + + assert result["certs"] == [] + assert result["email"] == "a@b.c" + assert result["status"]["error"] is not None + assert "exit code 2" in result["status"]["error"] + + def test_success_reports_no_error(self): + from lib.state import _collect_acme + + with ( + patch.object(lib.state, "_get_acme_email", return_value="a@b.c"), + patch("lib.acme.list_certs", return_value=[]), + patch.object(lib.state, "_parse_account_conf", return_value=_ACCOUNT), + ): + result = _collect_acme() + + assert result["status"] == {"error": None} + + class TestStateVersions: def test_version_starts_at_zero(self): s = State() diff --git a/tests/test_status_pending.py b/tests/test_status_pending.py index 5249649..f01605f 100644 --- a/tests/test_status_pending.py +++ b/tests/test_status_pending.py @@ -377,6 +377,45 @@ class TestStatusApplyAll: assert "Firewall" in result["errors"] mock_nginx.assert_called_once() + def test_force_body_forwarded_to_firewall_only(self): + mock_fw = MagicMock() + mock_nginx = MagicMock() + + pending_data = {**self._fake_pending_all} + pending_data["firewall"]["needs_apply"] = True + pending_data["firewall"]["change_count"] = 1 + pending_data["nginx"]["pending_changes"] = True + + with ( + patch("daemon.handlers.status.status_pending", return_value=pending_data), + patch("daemon.handlers.status.refresh_state"), + patch.dict( + "daemon.handlers.status.SYS_APPLY", + { + "firewall": mock_fw, + "nginx": mock_nginx, + }, + ), + ): + status.status_apply_all(None, {"force": True}) + mock_fw.assert_called_once_with(None, {"force": True}) + mock_nginx.assert_called_once_with(None, None) + + def test_no_body_passed_without_force(self): + mock_fw = MagicMock() + + pending_data = {**self._fake_pending_all} + pending_data["firewall"]["needs_apply"] = True + pending_data["firewall"]["change_count"] = 1 + + with ( + patch("daemon.handlers.status.status_pending", return_value=pending_data), + patch("daemon.handlers.status.refresh_state"), + patch.dict("daemon.handlers.status.SYS_APPLY", {"firewall": mock_fw}), + ): + status.status_apply_all(None, None) + mock_fw.assert_called_once_with(None, None) + class TestSysOrder: """Verify SYS_ORDER and SYS_LABELS constants.""" diff --git a/tests/test_sync.py b/tests/test_sync.py index 7521134..4a9e544 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -226,10 +226,14 @@ class TestGetAffected: class TestDnsToFirewallSync: + @patch("lib.sync.get_interface_ip", return_value="10.0.0.1") + @patch("lib.dnsmasq.save_config") @patch("lib.firewall.save_config") @patch("lib.firewall.get_config") @patch("lib.dnsmasq.get_config") - def test_adds_dhcp_dns(self, mock_dm_get, mock_fw_get, mock_fw_save): + def test_adds_dhcp_dns( + self, mock_dm_get, mock_fw_get, mock_fw_save, mock_dm_save, mock_ip + ): mock_dm_get.return_value = { "dhcp": { "ranges": [ @@ -288,10 +292,14 @@ class TestDnsToFirewallSync: assert "dhcp" not in saved_cfg["zones"]["internal"]["services"] assert "dns" not in saved_cfg["zones"]["internal"]["services"] + @patch("lib.sync.get_interface_ip", return_value="10.0.0.1") + @patch("lib.dnsmasq.save_config") @patch("lib.firewall.save_config") @patch("lib.firewall.get_config") @patch("lib.dnsmasq.get_config") - def test_idempotent(self, mock_dm_get, mock_fw_get, mock_fw_save): + def test_idempotent( + self, mock_dm_get, mock_fw_get, mock_fw_save, mock_dm_save, mock_ip + ): mock_dm_get.return_value = { "dhcp": { "ranges": [ diff --git a/tests/test_system_import.py b/tests/test_system_import.py index e1f53a9..40fd9f8 100644 --- a/tests/test_system_import.py +++ b/tests/test_system_import.py @@ -7,7 +7,7 @@ from unittest.mock import patch import pytest from lib import system_import -from lib.common import save_json +from lib.common import _APPLY_HASH_KEY, _LAST_APPLIED_CONFIG_KEY, config_hash, save_json @pytest.fixture @@ -137,6 +137,39 @@ class TestImportDnsmasq: ): assert not system_import.import_dnsmasq() + def test_preserves_apply_meta_on_drift(self, temp_project, tmp_path): + # Existing config differs from the live conf and carries apply + # bookkeeping — the rewrite must keep the baseline so pending + # detection and cancel-all survive daemon restarts. + conf = f"{system_import.DNSTART}\nserver=8.8.8.8\n{system_import.DNEND}" + self._write_conf(tmp_path, conf) + cfg_path = tmp_path / "config" / "dnsmasq" + cfg_path.mkdir(parents=True, exist_ok=True) + baseline = {"dns": {"upstreams": ["1.1.1.1"]}} + save_json( + cfg_path / "config.json", + { + **baseline, + _APPLY_HASH_KEY: "old-hash", + _LAST_APPLIED_CONFIG_KEY: baseline, + }, + ) + assert system_import.import_dnsmasq() + cfg = self._read_json(tmp_path) + assert cfg[_APPLY_HASH_KEY] == "old-hash" + assert cfg[_LAST_APPLIED_CONFIG_KEY] == baseline + assert cfg["dns"]["upstreams"] == ["8.8.8.8"] + + def test_stamps_applied_on_first_import(self, temp_project, tmp_path): + # No config file yet: the imported content is the running state, + # so it must be stamped as applied (no phantom pending changes). + conf = f"{system_import.DNSTART}\nserver=8.8.8.8\n{system_import.DNEND}" + self._write_conf(tmp_path, conf) + assert system_import.import_dnsmasq() + cfg = self._read_json(tmp_path) + assert cfg[_APPLY_HASH_KEY] == config_hash(cfg) + assert cfg[_LAST_APPLIED_CONFIG_KEY]["dns"]["upstreams"] == ["8.8.8.8"] + # ────────────────────────────────────────────────────────────────────── # WireGuard @@ -232,6 +265,44 @@ class TestImportWireguard: assert system_import.import_wireguard() assert not system_import.import_wireguard() + def test_preserves_apply_meta_on_drift(self, temp_project, tmp_path): + conf = ( + "[Interface]\n" + " PrivateKey = abc123\n" + " Address = 10.137.0.1/24\n" + " ListenPort = 51820\n" + ) + self._write_conf(tmp_path, conf) + cfg_path = tmp_path / "config" / "wireguard" + cfg_path.mkdir(parents=True, exist_ok=True) + baseline = {"interface": {"listen_port": 51821}, "peers": {}} + save_json( + cfg_path / "config.json", + { + **baseline, + _APPLY_HASH_KEY: "old-hash", + _LAST_APPLIED_CONFIG_KEY: baseline, + }, + ) + assert system_import.import_wireguard() + cfg = self._read_json(tmp_path) + assert cfg[_APPLY_HASH_KEY] == "old-hash" + assert cfg[_LAST_APPLIED_CONFIG_KEY] == baseline + assert cfg["interface"]["listen_port"] == 51820 + + def test_stamps_applied_on_first_import(self, temp_project, tmp_path): + conf = ( + "[Interface]\n" + " PrivateKey = abc123\n" + " Address = 10.137.0.1/24\n" + " ListenPort = 51820\n" + ) + self._write_conf(tmp_path, conf) + assert system_import.import_wireguard() + cfg = self._read_json(tmp_path) + assert cfg[_APPLY_HASH_KEY] == config_hash(cfg) + assert cfg[_LAST_APPLIED_CONFIG_KEY]["interface"]["private_key"] == "abc123" + # ────────────────────────────────────────────────────────────────────── # Networkd @@ -593,6 +664,18 @@ class TestImportFirewall: cfg = self._read_json(tmp_path) assert "dmz" not in cfg["zones"] + def test_import_stamps_applied(self, temp_project, tmp_path): + # Fresh import adopts the live firewalld state, which is by + # definition the applied state — the file must carry a baseline. + with patch("lib.system_import.run", return_value=FIREWALL_ZONES_OUTPUT): + assert system_import.import_firewall() + cfg = self._read_json(tmp_path) + assert cfg[_APPLY_HASH_KEY] == config_hash(cfg) + assert cfg[_LAST_APPLIED_CONFIG_KEY]["zones"]["public"]["interfaces"] == [ + "eth0", + "eth1", + ] + def test_parse_error_returns_false(self, temp_project, tmp_path): with patch("lib.system_import.run", return_value="garbage with no valid zones"): assert not system_import.import_firewall() diff --git a/webui/api/status.py b/webui/api/status.py index 141e97d..ed6bc4c 100644 --- a/webui/api/status.py +++ b/webui/api/status.py @@ -46,12 +46,16 @@ def apply_all(): Endpoint: POST /api/status/apply-all + Body: + {"force": true} (optional) — overrides the firewall safety guards + (management lockout, interface coverage) for this apply. Returns: JSON response with applied subsystems list and any errors encountered. """ + body = request.get_json(silent=True) try: - return _ok(post(POST_STATUS_APPLY_ALL)) + return _ok(post(POST_STATUS_APPLY_ALL, body)) except RuntimeError as exc: logger.error("Failed to apply all pending changes: %s", exc) return _error(str(exc), 500) diff --git a/webui/static/hoover/components/applyconfirm.js b/webui/static/hoover/components/applyconfirm.js index e9ecc57..25ad69f 100644 --- a/webui/static/hoover/components/applyconfirm.js +++ b/webui/static/hoover/components/applyconfirm.js @@ -54,17 +54,54 @@ ${hasPending ? html` 0); + let force = false; openModal((inner) => { const rows = buildRows(pendingData, expanded); @@ -106,7 +149,11 @@ async function openApplyModal(successMsg) { modalVNodes(inner, html`

Confirm: Apply All Changes

${rows}
-
+${fwPending ? html`` : ''} +
`); }); } diff --git a/webui/static/hoover/components/data.js b/webui/static/hoover/components/data.js index f5d1389..57b17fb 100644 --- a/webui/static/hoover/components/data.js +++ b/webui/static/hoover/components/data.js @@ -171,13 +171,22 @@ export function ActionButton(props = {}) { if (body !== undefined) opts.body = body; const resp = await apiFetch(props.url, opts); if (resp.ok) { - const synced = resp.data?.synced; - let msg = props.successMsg || ''; - if (synced && synced.length) { - if (msg) msg += ' '; - msg += '(auto-synced: ' + synced.join(', ') + ')'; + // Batch endpoints (e.g. /api/status/apply-all) return 200 + // with an `errors` map when some operations failed — + // `resp.ok` alone is not a success signal. + const errs = (resp.data && typeof resp.data.errors === 'object') ? resp.data.errors : null; + const errEntries = errs ? Object.entries(errs) : []; + if (errEntries.length) { + toast('Failed: ' + errEntries.map(([k, v]) => `${k} — ${v}`).join('; '), 'error', 8000); + } else { + const synced = resp.data?.synced; + let msg = props.successMsg || ''; + if (synced && synced.length) { + if (msg) msg += ' '; + msg += '(auto-synced: ' + synced.join(', ') + ')'; + } + if (msg) toast(msg, 'success'); } - if (msg) toast(msg, 'success'); if (props.onSuccess) props.onSuccess(); // No modelFetch — WS delta updates state store models. } else { diff --git a/webui/static/pages/certs.js b/webui/static/pages/certs.js index b89571f..d45e00b 100644 --- a/webui/static/pages/certs.js +++ b/webui/static/pages/certs.js @@ -336,6 +336,7 @@ export default definePage({ if (guard) return guard; const account = state.acme.data?.account || { registered: false, email: '', ca: '' }; + const certError = state.acme.data?.status?.error; const rows = (state.acme.data?.certs || []).map(c => { const badge = certStatusBadge({ expired: c.expired, daysRemaining: c.days_remaining }); @@ -363,6 +364,15 @@ export default definePage({ onClick=${() => issueCertModal(state)}>Issue Certificate`, }), _accountCard(account), + certError + ? html`
+
+
+ Certificate data unavailable: ${esc(certError)} +
+
+
` + : null, rows.length ? Table({ columns: ['Domain', 'Issuer', 'Expiry', 'Status', 'Actions'], diff --git a/webui/static/pages/dashboard.js b/webui/static/pages/dashboard.js index bf4697b..6445c12 100644 --- a/webui/static/pages/dashboard.js +++ b/webui/static/pages/dashboard.js @@ -1,4 +1,4 @@ -import { html, PageHeader, definePage, getModel, renderGuardMulti, ServiceStatus, StatCard, Table, Badge, ActionButton, CancelConfirm, fmtBytes } from '/static/hoover/index.js'; +import { html, PageHeader, definePage, getModel, renderGuardMulti, ServiceStatus, StatCard, Table, Badge, ApplyConfirm, CancelConfirm, fmtBytes } from '/static/hoover/index.js'; // Render a single firewall change as "current → new". // `live` is the currently applied value; `config` is the target value it @@ -158,7 +158,7 @@ export default definePage({ `)}
- <${ActionButton} url="/api/status/apply-all" label="Apply All Changes" + <${ApplyConfirm} pending=${true} label="Apply All Changes" successMsg="All changes applied" cls="btn btn-sm btn-primary" /> <${CancelConfirm} cls="btn btn-sm btn-danger" />