Merge pull request #393 from Vaso73/fix/oci-glances-host-firewall

fix(oci): scope Glances host-monitor firewall access

lang/sk.json

@@ -7513,5 +7513,27 @@
   "⚠ Disk data will NOT be erased.": "⚠ Dáta na disku sa NEVYMAŽÚ.",
   "⚠ Disk will be unmounted and removed from /etc/fstab.": "⚠ Disk sa odpojí a odstráni z /etc/fstab.",
   "⚠ The /etc/fstab entry will be removed.": "⚠ Záznam v /etc/fstab bude odstránený.",
-  "⚠ The disk will be unmounted.": "⚠ Disk bude odpojený."
+  "⚠ The disk will be unmounted.": "⚠ Disk bude odpojený.",
+  "A host firewall rule is only valid for a host-monitor profile": "Pravidlo firewallu hostiteľa je platné iba pre profil monitorovania hostiteľa",
+  "Allowed source subnet:": "Povolená zdrojová podsieť:",
+  "Allowed web port:": "Povolený webový port:",
+  "Allow TCP port {port} from {subnet} through the host firewall? Existing firewall rules are not changed.": "Povoliť cez firewall hostiteľa TCP port {port} zo siete {subnet}? Existujúce pravidlá firewallu sa nemenia.",
+  "Could not add the confirmed host firewall rule": "Potvrdené pravidlo firewallu hostiteľa sa nepodarilo pridať",
+  "Could not read the host firewall rules": "Pravidlá firewallu hostiteľa sa nepodarilo načítať",
+  "Host firewall": "Firewall hostiteľa",
+  "Host firewall already allows:": "Firewall hostiteľa už povoľuje:",
+  "Host firewall rule added:": "Pravidlo firewallu hostiteľa bolo pridané:",
+  "Invalid host-monitor firewall plan": "Neplatný plán firewallu pre monitor hostiteľa",
+  "Selected bridge:": "Vybraný bridge:",
+  "The host-monitor firewall bridge does not match the selected bridge": "Bridge firewallu pre monitor hostiteľa nezodpovedá vybranému bridgeu",
+  "The host-monitor firewall declaration is invalid": "Deklarácia firewallu pre monitor hostiteľa je neplatná",
+  "The host-monitor firewall port is invalid": "Port firewallu pre monitor hostiteľa je neplatný",
+  "The host-monitor firewall port is not declared as the web port": "Port firewallu pre monitor hostiteľa nie je deklarovaný ako webový port",
+  "The host-monitor firewall port is not declared by this profile": "Port firewallu pre monitor hostiteľa nie je deklarovaný týmto profilom",
+  "The host-monitor firewall subnet changed; no firewall rule was added": "Podsieť firewallu pre monitor hostiteľa sa zmenila; žiadne pravidlo sa nepridalo",
+  "The host-monitor web interface uses the host network.": "Webové rozhranie monitora hostiteľa používa sieť hostiteľa.",
+  "The selected bridge has no IPv4 subnet for the host-monitor firewall": "Vybraný bridge nemá IPv4 podsieť pre firewall monitora hostiteľa",
+  "allow TCP {port} from {subnet} via {bridge}": "povoliť TCP {port} zo siete {subnet} cez {bridge}",
+  "from": "zo siete",
+  "not changed": "bez zmeny"
 }

oci/catalog/overlays/glances.json

@@ -21,7 +21,16 @@
         "prompt_user": false
       }
     ],
-    "volumes": []
+    "volumes": [],
+    "ports": [
+      {
+        "container_port": 61208,
+        "published_example": 61208,
+        "protocol": "tcp",
+        "required": true,
+        "proxmox_behavior": "listener-on-dedicated-lxc-address-no-nat"
+      }
+    ]
   },
   "proxmox": {
     "defaults": {
@@ -43,6 +52,10 @@
     "installer_profile": {
       "host_monitor": "glances",
       "host_monitor_optional": true,
+      "host_monitor_firewall": {
+        "protocol": "tcp",
+        "web_port": 61208
+      },
       "host_monitor_mounts": [
         {
           "source": "/etc/os-release",

oci/remote/install_oci.sh

@@ -204,6 +204,78 @@ validate_host_monitor() {
   [[ -z $(ss -H -ltn "sport = :${port}") ]] || die "$(translate "The host port is already in use:") ${port}"
 }
 
+validate_host_monitor_firewall() {
+  HOST_FIREWALL_ENABLED=0
+  HOST_FIREWALL_BRIDGE=""
+  HOST_FIREWALL_SOURCE=""
+  HOST_FIREWALL_PORT=""
+  [[ $(jq -r '.host_firewall == null or .host_firewall == {}' "$DEPLOYMENT_FILE") == true ]] && return 0
+  [[ -n ${HOST_MONITOR:-} ]] || die "$(translate "A host firewall rule is only valid for a host-monitor profile")"
+  jq -e '.host_firewall | type == "object"
+    and (.confirmed == true)
+    and (.bridge | type == "string" and test("^[A-Za-z0-9_.-]+$"))
+    and (.source | type == "string" and test("^[0-9./]+$"))
+    and (.protocol == "tcp")
+    and (.port | type == "number" and floor == . and . >= 1 and . <= 65535)' \
+    "$DEPLOYMENT_FILE" >/dev/null \
+    || die "$(translate "Invalid host-monitor firewall plan")"
+  HOST_FIREWALL_BRIDGE=$(jq -r '.host_firewall.bridge' "$DEPLOYMENT_FILE")
+  HOST_FIREWALL_SOURCE=$(jq -r '.host_firewall.source' "$DEPLOYMENT_FILE")
+  HOST_FIREWALL_PORT=$(jq -r '.host_firewall.port' "$DEPLOYMENT_FILE")
+  [[ $HOST_FIREWALL_BRIDGE == "$BRIDGE" ]] \
+    || die "$(translate "The host-monitor firewall bridge does not match the selected bridge")"
+  local cidr subnet declared_port contract_count
+  cidr=$(ip -4 -o addr show dev "$BRIDGE" scope global | awk 'NR==1 {print $4}')
+  [[ -n $cidr ]] || die "$(translate "The selected bridge has no IPv4 subnet for the host-monitor firewall")"
+  subnet=$(python3 - "$cidr" <<'PY'
+import ipaddress
+import sys
+try:
+    print(ipaddress.ip_interface(sys.argv[1]).network)
+except ValueError:
+    raise SystemExit(1)
+PY
+) || die "$(translate "The selected bridge has no IPv4 subnet for the host-monitor firewall")"
+  [[ $HOST_FIREWALL_SOURCE == "$subnet" ]] \
+    || die "$(translate "The host-monitor firewall subnet changed; no firewall rule was added")"
+  declared_port=$(jq -r '.proxmox.installer_profile.host_monitor_firewall.web_port // empty' "$TEMPLATE_FILE")
+  [[ $declared_port == "$HOST_FIREWALL_PORT" ]] \
+    || die "$(translate "The host-monitor firewall port is not declared by this profile")"
+  contract_count=$(jq --argjson port "$HOST_FIREWALL_PORT" '[.container_contract.ports[]? | select(
+    .protocol == "tcp" and .container_port == $port)] | length' "$TEMPLATE_FILE")
+  [[ $contract_count == 1 ]] \
+    || die "$(translate "The host-monitor firewall port is not declared as the web port")"
+  HOST_FIREWALL_ENABLED=1
+}
+
+apply_host_monitor_firewall() {
+  [[ ${HOST_FIREWALL_ENABLED:-0} == 1 ]] || return 0
+  # Updates/recreates use a saved plan non-interactively.  They never add a
+  # missing host rule; only the original, separately confirmed installation
+  # may modify the host firewall.
+  if [[ -n ${PROXMENUX_OCI_TRANSACTION:-} ]]; then
+    oci_log "Host firewall plan retained without changing firewall during an OCI transaction"
+    return 0
+  fi
+  local node comment rules existing
+  node=$(hostname)
+  comment="ProxMenux OCI host monitor CT ${VMID}"
+  rules=$(pvesh get "/nodes/${node}/firewall/rules" --output-format json) \
+    || die "$(translate "Could not read the host firewall rules")"
+  existing=$(jq -r --arg source "$HOST_FIREWALL_SOURCE" --arg port "$HOST_FIREWALL_PORT" '
+    [.[]? | select((.type | ascii_downcase) == "in" and (.action | ascii_upcase) == "ACCEPT"
+      and (.proto | ascii_downcase) == "tcp" and (.source // "") == $source
+      and ((.dport | tostring) == $port))] | length' <<<"$rules")
+  if (( existing > 0 )); then
+    msg_ok "$(translate "Host firewall already allows:") TCP ${HOST_FIREWALL_PORT} $(translate "from") ${HOST_FIREWALL_SOURCE}"
+    return 0
+  fi
+  pvesh create "/nodes/${node}/firewall/rules" --type in --action ACCEPT --proto tcp \
+    --dport "$HOST_FIREWALL_PORT" --source "$HOST_FIREWALL_SOURCE" --comment "$comment" \
+    || die "$(translate "Could not add the confirmed host firewall rule")"
+  msg_ok "$(translate "Host firewall rule added:") TCP ${HOST_FIREWALL_PORT} $(translate "from") ${HOST_FIREWALL_SOURCE}"
+}
+
 apply_host_monitor() {
   [[ -n ${HOST_MONITOR:-} ]] || return 0
   # PVE permits lxc.include but not namespace keys directly in the CT config.
@@ -1101,6 +1173,7 @@ MAC_ADDRESS=$(jq -r '.network.mac_address // empty' "$DEPLOYMENT_FILE")
 GATEWAY=$(jq -r '.network.gateway // empty' "$DEPLOYMENT_FILE")
 FIREWALL=$(json_value '.network.firewall | if . then 1 else 0 end' "$DEPLOYMENT_FILE")
 validate_host_monitor
+validate_host_monitor_firewall
 ONBOOT=$(json_value '.onboot | if . then 1 else 0 end' "$DEPLOYMENT_FILE")
 START_AFTER=$(json_value '.start_after_create | if . then 1 else 0 end' "$DEPLOYMENT_FILE")
 SHUTDOWN_TIMEOUT=$(json_value '.shutdown_timeout_seconds' "$DEPLOYMENT_FILE")
@@ -1923,6 +1996,7 @@ elif [[ $HAOS_HEALTHCHECK != 0 ]]; then
   URLS='[]'
   msg_info2 "$(translate "Home Assistant OS was not started: its addresses will be known once Core is running.")"
 fi
+apply_host_monitor_firewall
 INSTALL_COMPLETE=1
 if [[ $(jq -r '.stack_managed // false' "$DEPLOYMENT_FILE") == true ]]; then
   msg_ok "$(translate "Container prepared for the stack:") CT $VMID ($HOSTNAME)"

oci/src/proxmenux_oci/cli.py

@@ -291,6 +291,14 @@ def _deployment_summary_text(template: dict[str, Any], deployment: dict[str, Any
     network = plan.get("network", {})
     if plan.get("host_monitor"):
         row(translate("Network"), translate("IP address and firewall of the host"))
+        firewall = plan.get("host_firewall")
+        if firewall:
+            row(translate("Host firewall"),
+                translate("allow TCP {port} from {subnet} via {bridge}").format(
+                    port=firewall["port"], subnet=firewall["source"], bridge=firewall["bridge"]
+                ))
+        else:
+            row(translate("Host firewall"), translate("not changed"))
     elif "frontend_bridge" in network:
         addresses = [network[key] for key in ("frontend_ipv4", "machine_learning_frontend_ipv4") if network.get(key)]
         addresses += [service["frontend_ipv4"] for service in plan.get("services", []) if service.get("frontend_ipv4")]

oci/src/proxmenux_oci/host.py

@@ -61,6 +61,23 @@ def default_bridge(preferred: str) -> str:
     return names[0]
 
 
+def ipv4_subnet(bridge: str) -> str | None:
+    """The IPv4 subnet configured on ``bridge``, if it has one.
+
+    This is deliberately taken from the node's bridge configuration instead
+    of guessing from a container address.  A host-monitor shares the host
+    network namespace, so its firewall source scope must be the selected
+    host bridge's network.
+    """
+    row = next((item for item in bridges(include_private=True)
+                if item.get("iface") == bridge), None)
+    try:
+        interface = ipaddress.ip_interface(str((row or {}).get("cidr") or ""))
+    except ValueError:
+        return None
+    return str(interface.network) if interface.version == 4 else None
+
+
 def timezone() -> str:
     try:
         value = Path("/etc/timezone").read_text(encoding="utf-8").strip()

oci/src/proxmenux_oci/installer.py

@@ -111,6 +111,45 @@ def ask_bridge(ui, text: str, default: str, mode: str = ADVANCED_MODE) -> str:
     return selected
 
 
+def host_monitor_firewall_plan(template: dict[str, Any], bridge: str) -> dict[str, Any] | None:
+    """Build the narrow firewall change a host-monitor profile explicitly declares.
+
+    Profiles without this opt-in declaration never propose a host firewall
+    change.  The source network comes from the selected Proxmox bridge, not
+    from a catalog constant or the address of a particular test lab.
+    """
+    profile = template.get("proxmox", {}).get("installer_profile", {})
+    declared = profile.get("host_monitor_firewall")
+    if declared is None:
+        return None
+    if not isinstance(declared, dict) or declared.get("protocol") != "tcp":
+        raise InstallError(translate("The host-monitor firewall declaration is invalid"))
+    port = declared.get("web_port")
+    if not isinstance(port, int) or not 1 <= port <= 65535:
+        raise InstallError(translate("The host-monitor firewall port is invalid"))
+    subnet = host.ipv4_subnet(bridge)
+    if not subnet:
+        raise InstallError(translate("The selected bridge has no IPv4 subnet for the host-monitor firewall"))
+    return {"bridge": bridge, "source": subnet, "protocol": "tcp", "port": port,
+            "confirmed": True}
+
+
+def confirm_host_monitor_firewall(ui, template: dict[str, Any], bridge: str) -> dict[str, Any] | None:
+    """Ask separately before allowing a narrowly-scoped host firewall rule."""
+    plan = host_monitor_firewall_plan(template, bridge)
+    if plan is None:
+        return None
+    summary = translate("The host-monitor web interface uses the host network.")
+    question = translate("Allow TCP port {port} from {subnet} through the host firewall? Existing firewall rules are not changed.").format(
+        port=plan["port"], subnet=plan["source"]
+    )
+    if ui.confirm(f"{summary}\n\n{translate('Selected bridge:')} {plan['bridge']}\n"
+                  f"{translate('Allowed source subnet:')} {plan['source']}\n"
+                  f"{translate('Allowed web port:')} TCP {plan['port']}\n\n{question}", False):
+        return plan
+    return None
+
+
 def _confirm_warning(ui, warning: str | None, fallback: str, question: str) -> bool:
     return ui.confirm(f"{translate(warning) if warning else translate(fallback)}\n\n{translate(question)}", False)
 
@@ -258,6 +297,8 @@ def build_deployment(
         onboot = bool(defaults["onboot"])
         start_after = True
 
+    host_firewall = confirm_host_monitor_firewall(ui, template, bridge) if host_monitor else None
+
     environment: list[dict[str, str]] = []
     for item in template["container_contract"]["environment"]:
         name = item["name"]
@@ -423,6 +464,7 @@ def build_deployment(
     return {
         "host_monitor": host_monitor,
         "monitor_scope": monitor_scope,
+        "host_firewall": host_firewall,
         "vmid": int(vmid_text) if vmid_text else None,
         "ostype": defaults.get("ostype", "auto-from-image"),
         "hostname": hostname,

oci/tests/test_glances_ports.py

@@ -0,0 +1,23 @@
+"""Glances' Web UI is the only listener used by the native ``-w`` profile."""
+
+from pathlib import Path
+import unittest
+
+from proxmenux_oci.catalog import Catalog
+
+
+class GlancesPortContractTests(unittest.TestCase):
+    def test_web_profile_exposes_only_the_web_ui_port(self):
+        root = Path(__file__).resolve().parents[1]
+        template = Catalog(root).compose("glances")
+
+        ports = template["container_contract"]["ports"]
+        self.assertEqual([item["container_port"] for item in ports], [61208])
+        self.assertEqual(template["first_run"]["endpoints"], [{
+            "label": "Web UI",
+            "scheme": "http",
+            "port": 61208,
+            "path": "/",
+            "source": "compose-metadata",
+        }])
+        self.assertEqual(template["proxmox"]["installer_profile"]["startup_healthcheck"]["port"], 61208)

oci/tests/test_host_monitor_firewall.py

@@ -0,0 +1,73 @@
+"""The host-monitor firewall plan is opt-in and bound to its selected bridge."""
+
+from pathlib import Path
+import sys
+import unittest
+from unittest.mock import patch
+
+
+ROOT = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(ROOT / "src"))
+
+from proxmenux_oci.installer import (InstallError, confirm_host_monitor_firewall,
+                                     host_monitor_firewall_plan)
+
+
+class ConfirmUI:
+    def __init__(self, answer):
+        self.answer = answer
+        self.prompts = []
+
+    def confirm(self, text, default=False):
+        self.prompts.append((text, default))
+        return self.answer
+
+
+def template(declared={"protocol": "tcp", "web_port": 61208}):
+    profile = {} if declared is None else {"host_monitor_firewall": declared}
+    return {"proxmox": {"installer_profile": profile}}
+
+
+class HostMonitorFirewallPlanTests(unittest.TestCase):
+    @patch("proxmenux_oci.installer.host.ipv4_subnet", return_value="192.0.2.0/24")
+    def test_uses_selected_bridge_subnet_and_declared_port(self, subnet):
+        plan = host_monitor_firewall_plan(template(), "vmbr7")
+        self.assertEqual(plan, {"bridge": "vmbr7", "source": "192.0.2.0/24",
+                                "protocol": "tcp", "port": 61208, "confirmed": True})
+        subnet.assert_called_once_with("vmbr7")
+
+    @patch("proxmenux_oci.installer.host.ipv4_subnet", return_value="198.51.100.0/25")
+    def test_confirmation_displays_scope_and_declines_without_plan(self, _subnet):
+        ui = ConfirmUI(False)
+        self.assertIsNone(confirm_host_monitor_firewall(ui, template(), "vmbr3"))
+        self.assertFalse(ui.prompts[0][1])
+        self.assertIn("vmbr3", ui.prompts[0][0])
+        self.assertIn("198.51.100.0/25", ui.prompts[0][0])
+        self.assertIn("61208", ui.prompts[0][0])
+
+    @patch("proxmenux_oci.installer.host.ipv4_subnet", return_value=None)
+    def test_refuses_to_guess_a_subnet(self, _subnet):
+        with self.assertRaises(InstallError):
+            host_monitor_firewall_plan(template(), "vmbr0")
+
+    def test_undeclared_profiles_never_offer_a_firewall_change(self):
+        self.assertIsNone(host_monitor_firewall_plan(template(None), "vmbr0"))
+
+
+class HostMonitorFirewallInstallerContractTests(unittest.TestCase):
+    def test_remote_installer_revalidates_and_uses_proxmox_rule_api(self):
+        source = (ROOT / "remote" / "install_oci.sh").read_text(encoding="utf-8")
+        self.assertIn("validate_host_monitor_firewall", source)
+        self.assertIn("The host-monitor firewall subnet changed; no firewall rule was added", source)
+        self.assertIn('pvesh create "/nodes/${node}/firewall/rules"', source)
+        self.assertIn("--dport \"$HOST_FIREWALL_PORT\" --source \"$HOST_FIREWALL_SOURCE\"", source)
+        self.assertIn("only the original, separately confirmed installation", source)
+
+    def test_oci_menu_wrapper_does_not_hide_engine_errors_with_the_main_menu(self):
+        wrapper = (ROOT.parent / "scripts" / "oci" / "oci_manager_apps.sh").read_text(encoding="utf-8")
+        self.assertIn('OCI_STATUS=$?', wrapper)
+        self.assertIn('exit "$OCI_STATUS"', wrapper)
+
+
+if __name__ == "__main__":
+    unittest.main()

scripts/oci/oci_manager_apps.sh

@@ -67,4 +67,13 @@ if [[ $# -gt 0 ]]; then
     PYTHONPATH="$OCI_ENGINE_DIR/src" exec python3 -m proxmenux_oci "$@"
 fi
 PYTHONPATH="$OCI_ENGINE_DIR/src" python3 -m proxmenux_oci
+OCI_STATUS=$?
+
+# Do not replace an OCI engine traceback with the main menu.  Returning to the
+# menu is correct after a normal cancellation, but an unexpected non-zero exit
+# must remain visible so the user can report and diagnose it.
+if [[ $OCI_STATUS -ne 0 ]]; then
+    exit "$OCI_STATUS"
+fi
+
 exec bash "$LOCAL_SCRIPTS/menus/main_menu.sh"