feat(hardware): ipmitool install route + node Hardware panel (#609 phase 1 step 3-4)

Step 3 — one-click ipmitool install:
* POST /api/clusters/<cid>/hardware/ipmitool/install (admin.settings) mirrors the
  proven StarWind installer but with a FULLY STATIC script (no user-controlled shell
  input), gated: cluster-access -> proxmox-only -> consent-required (the install
  mutates the node, so it must not run before the warning is acknowledged) -> per-node
  name validation -> run_per_node fan-out (cap 8, idempotent) -> audited.

Step 4 — frontend:
* Self-contained HardwareMonitoringPanel: consent fetch, mandatory compliance-warning
  modal (renders the server-versioned warning text, required ack checkbox, generic
  require_delay_seconds countdown ready for the Redfish phase), ipmitool install button
  when missing, sensor/power/FRU/SEL rollup with health badge.
* Wired as a 'Hardware' tab in BOTH node UIs (corporate CorporateNodeDetailView +
  default NodeModal) for parity. 21 i18n keys across all 7 languages.

Hardening from the adversarial review (3 confirmed findings, all fixed):
* MEDIUM — config-restore could enable hardware monitoring WITHOUT the versioned
  acknowledgement audit record (non-repudiation bypass): restore now drops the
  hardware_monitoring key and preserves the live consent state, so consent only ever
  moves through the audit-logged set_hw_monitoring_consent path (settings.py).
* LOW — a non-int stored/posted ack_version raised int() -> 500 on every gate; add
  _hw_int() so the gate fails CLOSED (disabled) instead of crashing.
* NIT — a crafted {"nodes":[123]}/[{...}] blew up _reject_bad_node's regex / set()
  -> 500; validate the raw list + reject non-strings centrally -> clean 400.

Tests: +9 integration cases (install gates admin/consent/proxmox/bad-node + the 3
hardening regressions). Frontend build clean (Babel). Full suite 307 passed.

Author: MrMasterbay

pegaprox/api/nodes.py

@@ -133,8 +133,10 @@ def get_node_network_api(cluster_id, node):
 def _reject_bad_node(node):
     """Return (None, None) if node looks valid, (response, 400) otherwise.
     Used by the SSH-fronted node endpoints below as a defense-in-depth gate
-    even though the SSH commands themselves don't interpolate `node`."""
-    if not node or not _NODE_NAME_RE.match(node):
+    even though the SSH commands themselves don't interpolate `node`.
+    Non-strings (e.g. a crafted JSON list `{"nodes":[123]}`) are rejected as
+    invalid rather than blowing up the regex with a TypeError -> clean 400."""
+    if not isinstance(node, str) or not _NODE_NAME_RE.match(node):
         return jsonify({'error': 'Invalid node name'}), 400
     return None, None
 
@@ -229,13 +231,23 @@ def get_node_temperature_history_api(cluster_id, node):
 # the audit log — so "I never enabled that" cannot hold ("dann heißt es nicht 'Ich
 # hab es nie bekommen'"). The read route refuses to serve until consent is on.
 
+def _hw_int(v):
+    """Coerce a stored/posted ack_version to int, degrading to 0 on any bad value
+    (e.g. a corrupt/crafted settings blob with ack_version='x' or [1]) so the gate
+    fails CLOSED — returns disabled — rather than raising a 500 on every read."""
+    try:
+        return int(v)
+    except (TypeError, ValueError):
+        return 0
+
+
 def _hw_consent_state():
     """(enabled, ack_record) for in-band hardware monitoring from server settings.
     enabled == acknowledged AND the stored ack is at/above the current warning ver."""
     from pegaprox.api.helpers import load_server_settings
     from pegaprox.core import bmc
     ack = (load_server_settings() or {}).get('hardware_monitoring') or {}
-    enabled = bool(ack.get('enabled')) and int(ack.get('ack_version') or 0) >= bmc.HW_CONSENT_VERSION
+    enabled = bool(ack.get('enabled')) and _hw_int(ack.get('ack_version')) >= bmc.HW_CONSENT_VERSION
     return enabled, ack
 
 
@@ -282,7 +294,7 @@ def set_hw_monitoring_consent():
                   'In-band hardware monitoring disabled')
         return jsonify({'enabled': False})
 
-    if data.get('acknowledge') is True and int(data.get('ack_version') or 0) == bmc.HW_CONSENT_VERSION:
+    if data.get('acknowledge') is True and _hw_int(data.get('ack_version')) == bmc.HW_CONSENT_VERSION:
         rec = {
             'enabled': True,
             'acknowledged_by': usr,
@@ -329,6 +341,119 @@ def get_node_hardware_api(cluster_id, node):
     return jsonify(result)
 
 
+# ipmitool one-click install (#609 step 3). Fully static script — no user-controlled
+# input reaches the shell (unlike the StarWind installer there is no repo/key URL), so
+# no SSRF/url-allowlist is needed. Read-only in-band IPMI (local KCS, no BMC creds).
+# Gated behind the SAME compliance acknowledgement as the read path: the install
+# MUTATES the node (package + IPMI kernel modules) so it must not run before the
+# warning is acknowledged. admin.settings + audited + idempotent + bounded-parallel.
+IPMITOOL_INSTALL_SCRIPT = """#!/usr/bin/env bash
+# PegaProx (#609) — install ipmitool for in-band hardware monitoring on this node.
+set -uo pipefail
+if command -v ipmitool >/dev/null 2>&1; then
+    ver="$(ipmitool -V 2>/dev/null | head -1 || echo ipmitool)"
+    echo "PP_OK already_installed $ver"; exit 0
+fi
+if ! command -v apt-get >/dev/null 2>&1; then echo 'PP_ERR no-apt-get'; exit 3; fi
+apt-get update -o Acquire::Retries=2 >/tmp/pp_ipmitool_apt.log 2>&1 || true
+if ! DEBIAN_FRONTEND=noninteractive apt-get install -y ipmitool >>/tmp/pp_ipmitool_apt.log 2>&1; then
+    echo 'PP_ERR apt-install-failed'; exit 5
+fi
+command -v ipmitool >/dev/null 2>&1 || { echo 'PP_ERR not-installed-after-apt'; exit 6; }
+ver="$(ipmitool -V 2>/dev/null | head -1 || echo ipmitool)"
+echo "PP_OK installed $ver"
+"""
+
+
+@bp.route('/api/clusters/<cluster_id>/hardware/ipmitool/install', methods=['POST'])
+@require_auth(perms=['admin.settings'])  # root apt-install on nodes -> admin-only, like starlvm
+def install_ipmitool_api(cluster_id):
+    """Install ipmitool on cluster node(s) over SSH for in-band hardware monitoring.
+
+    Body (optional): {nodes: [names]} — defaults to all cluster nodes. Idempotent
+    per node (skips if ipmitool present). Requires the hardware-monitoring feature
+    to be enabled (compliance acknowledged) first — the install changes the node.
+    """
+    ok, err = check_cluster_access(cluster_id)
+    if not ok: return err
+    if cluster_id not in cluster_managers:
+        return jsonify({'error': 'Cluster not found'}), 404
+    mgr = cluster_managers[cluster_id]
+    if getattr(mgr, 'cluster_type', 'proxmox') != 'proxmox':
+        return jsonify({'error': 'ipmitool install is Proxmox-only'}), 400
+
+    # consent gate — do not mutate a node before the warning is acknowledged
+    from pegaprox.core import bmc
+    enabled, _ack = _hw_consent_state()
+    if not enabled:
+        return jsonify({'error': 'Hardware monitoring is not enabled',
+                        'code': 'CONSENT_REQUIRED',
+                        'current_version': bmc.HW_CONSENT_VERSION}), 403
+
+    raw_nodes = (request.get_json(silent=True) or {}).get('nodes') or []
+    if not isinstance(raw_nodes, list):
+        return jsonify({'error': 'nodes must be a list of node names'}), 400
+    # validate each entry BEFORE set() — a dict/list entry would raise in set()
+    for n in raw_nodes:
+        bad, code = _reject_bad_node(n)
+        if bad is not None:
+            return bad, code
+    only = set(raw_nodes)
+    nodes = _cluster_node_names(mgr)
+    if only:
+        nodes = [n for n in nodes if n in only]
+    if not nodes:
+        return jsonify({'error': 'No nodes found in cluster'}), 404
+
+    def _install_one(node):
+        node_ip = mgr._get_node_ip(node)
+        if not node_ip:
+            return {'node': node, 'success': False, 'error': 'no SSH-reachable IP'}
+        ssh = None
+        try:
+            for attempt in range(3):
+                ssh = mgr._ssh_connect(node_ip)
+                if ssh:
+                    break
+                if attempt < 2:
+                    time.sleep(1.5)
+            if not ssh:
+                return {'node': node, 'success': False, 'error': 'SSH connect failed after 3 tries'}
+            _ssh_write_file(ssh, '/tmp/pegaprox-ipmitool-install.sh', IPMITOOL_INSTALL_SCRIPT, 0o755)
+            out, _e = _ssh_run_checked(ssh, 'bash /tmp/pegaprox-ipmitool-install.sh', timeout=180)
+            try:
+                ssh.exec_command('rm -f /tmp/pegaprox-ipmitool-install.sh')
+            except Exception:
+                pass
+            last = (out or '').strip().splitlines()[-1] if (out or '').strip() else ''
+            if 'PP_OK already_installed' in out:
+                return {'node': node, 'success': True, 'already_installed': True, 'detail': last}
+            if 'PP_OK installed' in out:
+                return {'node': node, 'success': True, 'already_installed': False, 'detail': last}
+            return {'node': node, 'success': False, 'error': last or 'unexpected installer output'}
+        except Exception as e:
+            return {'node': node, 'success': False, 'error': safe_error(e, 'install failed')}
+        finally:
+            if ssh:
+                try: ssh.close()
+                except Exception: pass
+
+    from pegaprox.utils.concurrent import run_per_node
+    res_map = run_per_node({n: _install_one for n in nodes}, max_concurrent=8, timeout=200)
+    results = [res_map.get(n) or {'node': n, 'success': False, 'error': 'timed out'} for n in nodes]
+    installed = sum(1 for r in results if r.get('success'))
+    usr = getattr(request, 'session', {}).get('user', 'system')
+    log_audit(usr, 'hardware_monitoring.ipmitool_install',
+              f"ipmitool install: {installed}/{len(nodes)} node(s)",
+              cluster=mgr.config.name)
+    return jsonify({
+        'success': installed == len(nodes),
+        'installed': installed,
+        'total': len(nodes),
+        'results': results,
+    })
+
+
 @bp.route('/api/clusters/<cluster_id>/nodes/<node>/network/<iface>', methods=['PUT'])
 @require_auth(perms=['node.network'])
 def update_node_network_api(cluster_id, node, iface):

pegaprox/api/settings.py

@@ -2427,14 +2427,23 @@ def restore_config():
             try:
                 if not dry_run:
                     current = load_server_settings()
+                    # #609: hardware-monitoring consent is a versioned, audit-logged
+                    # toggle that must ONLY move through set_hw_monitoring_consent (which
+                    # records the acknowledgement — "dann heißt es nicht 'Ich hab es nie
+                    # bekommen'"). A generic config-restore must neither enable nor disable
+                    # it, so we drop the backup's value and preserve the live consent state.
+                    incoming_ss = {k: v for k, v in (data['server_settings'] or {}).items()
+                                   if k != 'hardware_monitoring'}
                     if mode == 'merge':
                         # Only update non-empty values
-                        for key, value in data['server_settings'].items():
+                        for key, value in incoming_ss.items():
                             if value not in [None, '', []]:
                                 current[key] = value
                         save_server_settings(current)
                     else:
-                        save_server_settings(data['server_settings'])
+                        if (current or {}).get('hardware_monitoring') is not None:
+                            incoming_ss['hardware_monitoring'] = current['hardware_monitoring']
+                        save_server_settings(incoming_ss)
                 results['restored']['server_settings'] = True
             except Exception as e:
                 results['errors'].append(f"Server settings: {str(e)}")

tests/test_integration_hardware.py

@@ -205,3 +205,94 @@ def test_missing_cluster_manager_is_404_after_consent(api, seed, monkeypatch):
     resp = api.as_user(admin).get(HW_ROUTE)
     assert resp.status_code == 404, resp.get_data(as_text=True)
     assert 'not found' in resp.get_json()['error'].lower()
+
+
+# ===========================================================================
+# ipmitool INSTALL (step 3) — gates fire before the SSH fan-out. The actual
+# per-node apt install needs real hardware (like starlvm), so only the gates
+# are asserted here: admin-only, proxmox-only, and consent-required.
+# ===========================================================================
+
+INSTALL_ROUTE = f'/api/clusters/{CID}/hardware/ipmitool/install'
+
+
+def test_install_requires_admin_403(api, seed):
+    user = seed.user('joe', role='user', tenant_id='default')  # node.view, not admin.settings
+    api.set_manager(CID, _mgr(api))
+    resp = api.as_user(user).post(INSTALL_ROUTE, json={'nodes': [NODE]})
+    assert resp.status_code == 403, resp.get_data(as_text=True)
+
+
+def test_install_without_consent_is_403(api, seed):
+    # admin, proxmox cluster, but feature not enabled -> CONSENT_REQUIRED before fan-out.
+    admin = seed.user('root', role='admin', tenant_id='default')
+    api.set_manager(CID, _mgr(api))
+    resp = api.as_user(admin).post(INSTALL_ROUTE, json={'nodes': [NODE]})
+    assert resp.status_code == 403, resp.get_data(as_text=True)
+    assert resp.get_json()['code'] == 'CONSENT_REQUIRED'
+
+
+def test_install_non_proxmox_is_400(api, seed):
+    admin = seed.user('root', role='admin', tenant_id='default')
+    api.set_manager(CID, _mgr(api, cluster_type='vmware'))
+    resp = api.as_user(admin).post(INSTALL_ROUTE, json={'nodes': [NODE]})
+    assert resp.status_code == 400, resp.get_data(as_text=True)
+    assert 'proxmox' in resp.get_json()['error'].lower()
+
+
+def test_install_bad_node_name_is_400(api, seed, monkeypatch):
+    _consent_on(api, seed, monkeypatch)
+    admin = seed.user('root', role='admin', tenant_id='default')
+    api.set_manager(CID, _mgr(api))
+    resp = api.as_user(admin).post(INSTALL_ROUTE, json={'nodes': ['bad;rm -rf']})
+    assert resp.status_code == 400, resp.get_data(as_text=True)
+
+
+# ===========================================================================
+# Hardening regressions from the adversarial review (input hygiene / fail-closed)
+# ===========================================================================
+
+def test_install_non_string_node_is_400_not_500(api, seed, monkeypatch):
+    # crafted {"nodes":[123]} used to blow up _reject_bad_node's regex -> 500.
+    _consent_on(api, seed, monkeypatch)
+    admin = seed.user('root', role='admin', tenant_id='default')
+    api.set_manager(CID, _mgr(api))
+    resp = api.as_user(admin).post(INSTALL_ROUTE, json={'nodes': [123]})
+    assert resp.status_code == 400, resp.get_data(as_text=True)
+
+
+def test_install_unhashable_node_entry_is_400_not_500(api, seed, monkeypatch):
+    # a dict entry used to raise in set() before the loop even ran.
+    _consent_on(api, seed, monkeypatch)
+    admin = seed.user('root', role='admin', tenant_id='default')
+    api.set_manager(CID, _mgr(api))
+    resp = api.as_user(admin).post(INSTALL_ROUTE, json={'nodes': [{'x': 1}]})
+    assert resp.status_code == 400, resp.get_data(as_text=True)
+
+
+def test_install_nodes_not_a_list_is_400(api, seed, monkeypatch):
+    _consent_on(api, seed, monkeypatch)
+    admin = seed.user('root', role='admin', tenant_id='default')
+    api.set_manager(CID, _mgr(api))
+    resp = api.as_user(admin).post(INSTALL_ROUTE, json={'nodes': 'pve1'})
+    assert resp.status_code == 400, resp.get_data(as_text=True)
+
+
+def test_enable_non_int_ack_version_is_400_not_500(api, seed):
+    # POST {acknowledge:true, ack_version:'x'} used to raise int('x') -> 500.
+    admin = seed.user('root', role='admin', tenant_id='default')
+    resp = api.as_user(admin).post(CONSENT_ROUTE, json={'acknowledge': True, 'ack_version': 'x'})
+    assert resp.status_code == 400, resp.get_data(as_text=True)
+    assert resp.get_json()['code'] == 'ACK_REQUIRED'
+
+
+def test_corrupt_stored_ack_version_fails_closed(api, seed):
+    # a corrupt/crafted settings blob (e.g. via config-restore) with a non-int
+    # ack_version must NOT 500 the gate — it fails closed to CONSENT_REQUIRED.
+    from pegaprox.api.helpers import save_server_settings
+    save_server_settings({'hardware_monitoring': {'enabled': True, 'ack_version': 'abc'}})
+    admin = seed.user('root', role='admin', tenant_id='default')
+    api.set_manager(CID, _mgr(api, _get_node_ip='10.0.0.9', _ssh_run_command_output=SAMPLE))
+    resp = api.as_user(admin).get(HW_ROUTE)
+    assert resp.status_code == 403, resp.get_data(as_text=True)
+    assert resp.get_json()['code'] == 'CONSENT_REQUIRED'