fix: validate atomic configuration updates and expire stale monitoring snapshots

app/pigway_pi_control.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 import time,socket,subprocess,configparser,signal,re,argparse,json,os,platform
 from pathlib import Path
-APP_VERSION="3.11.2"
+APP_VERSION="3.11.3"
 import sys
 sys.path.append("/usr/local/share/pigway-pi-control")
 from plugin_api import Publisher,registered,discover
@@ -185,7 +185,7 @@ def internet():
     except: return False
 
 def load_cfg():
-    c=configparser.ConfigParser(); c.read(CFG)
+    c=configparser.ConfigParser(interpolation=None); c.read(CFG)
     watches=[]
     if c.has_section("services"):
         for label,target in c["services"].items():
@@ -331,10 +331,10 @@ def update_active_alerts(cpu,temp,mem,disk,sample_elapsed=0.0):
     else: alert_manager.clear("MEMORY_HIGH")
     if disk is not None:
         if disk>=DISK_CRIT:
-            alert_manager.request("DISK_CRITICAL",2,"DISK CRITICAL",f"USED {disk:.1f}%","FREE < 5%","CHECK STORAGE")
+            alert_manager.request("DISK_CRITICAL",2,"DISK CRITICAL",f"USED {disk:.1f}%",f"USED >= {DISK_CRIT:g}%","CHECK STORAGE")
             alert_manager.clear("DISK_HIGH")
         elif disk>=DISK_HIGH:
-            alert_manager.request("DISK_HIGH",1,"DISK SPACE LOW",f"USED {disk:.1f}%","FREE < 10%","CHECK STORAGE")
+            alert_manager.request("DISK_HIGH",1,"DISK SPACE LOW",f"USED {disk:.1f}%",f"USED >= {DISK_HIGH:g}%","CHECK STORAGE")
             alert_manager.clear("DISK_CRITICAL")
         else:
             alert_manager.clear("DISK_HIGH"); alert_manager.clear("DISK_CRITICAL")
@@ -408,13 +408,14 @@ class AlertManager:
                "first_seen":old["first_seen"] if old else now,"metadata":metadata or {}}
         value["last_changed"]=old["last_changed"] if old and all(old[k]==v for k,v in value.items()) else now
         self.requests[request_id]=value
-        if old is None:log("WARN","ALERT_ACTIVE",alert=request_id,severity=severity,detail=l2)
+        if old is None or old["severity"]!=severity:
+            log({1:"WARN",2:"CRITICAL",3:"EMERGENCY"}[severity],"ALERT_ACTIVE" if old is None else "ALERT_CHANGED",alert=request_id,severity=severity,detail=l2)
 
     def clear(self,request_id):
         if self.requests.pop(request_id,None) is not None:log("INFO","ALERT_RECOVERED",alert=request_id)
 
     def status_snapshot(self):
-        values=sorted(self.requests.values(),key=lambda a:(-a["priority"],a["first_seen"],a["id"]))
+        values=sorted(self.requests.values(),key=lambda a:(-a["severity"],-a["priority"],a["first_seen"],a["id"]))
         return {"owner":values[0]["id"] if values else "NORMAL_HOME","alerts":values}
 
 

app/pigway_pi_control_api.py

@@ -2,26 +2,30 @@
 import sys
 sys.path.append("/usr/local/share/pigway-pi-control")
 from plugin_api import discover,registered,request as plugin_request,socket_path
+import math,functools,tempfile
 import configparser,ipaddress,json,re,secrets,shutil,subprocess,threading,time,socket
-from concurrent.futures import ThreadPoolExecutor,as_completed
-from http import HTTPStatus
 from http.server import BaseHTTPRequestHandler,ThreadingHTTPServer
 from pathlib import Path
-STORAGE_SCAN_LOCK=threading.Lock()
-from urllib.parse import parse_qs,urlencode,urlparse
-from urllib.request import Request,urlopen
+from urllib.parse import parse_qs,urlparse
 
-APP_VERSION="3.11.2"
+STORAGE_SCAN_LOCK=threading.Lock()
+CONFIG_LOCK=threading.RLock()
+
+
+def config_transaction(function):
+    @functools.wraps(function)
+    def locked(*args,**kwargs):
+        with CONFIG_LOCK:
+            return function(*args,**kwargs)
+    return locked
+
+APP_VERSION="3.11.3"
 CFG=Path("/etc/pigway-pi-control.conf")
 STATUS=Path("/run/pigway-pi-control/status.json")
 TOKEN=Path("/etc/pigway-pi-control-api.token")
 ALLOWED_SECTIONS={"device","services","processes","alerts","timing","dark_mode","api","storage_names","storage_visible"}
 NAME_RE=re.compile(r"^[A-Za-z0-9_.-]{1,64}$")
 UNIT_RE=re.compile(r"^[A-Za-z0-9_.@:-]{1,128}\.service$")
-RGB_KEY_RE=re.compile(r"^(cpu|power|memory|storage|network|service|normal)_[rgb]$")
-FLASH_KEY_RE=re.compile(r"^(warning|critical|emergency|normal)_flash_(on|off)_ms$")
-NORMAL_MODES={"off","solid","flash","flow","breathe","marquee","rainbow","colorful"}
-EFFECT_COLORS={"red","green","blue","yellow","purple","cyan","white"}
 PROTECTED_UNITS={"pigway-pi-control-api.service"}
 
 def plugin_inventory():
@@ -36,6 +40,7 @@ def find_plugin(plugin_id):
     raise ValueError("unknown plugin")
 
 
+@config_transaction
 def plugin_operation(body):
     plugin_id=str(body.get("id",""))
     if not NAME_RE.fullmatch(plugin_id):raise ValueError("invalid plugin id")
@@ -66,7 +71,7 @@ def machine_identity():
     return {"id":identifier,"name":name,"hostname":socket.gethostname(),"local":True,"url":""}
 
 def load_cfg():
-    cfg=configparser.ConfigParser(); cfg.read(CFG)
+    cfg=configparser.ConfigParser(interpolation=None); cfg.read(CFG)
     return cfg
 
 def api_setting(key,default,cast=str):
@@ -91,36 +96,39 @@ def validate_value(section,key,value,existing):
     if section=="timing" and key in {"oled_refresh_interval","page_interval"}:raise ValueError("display timing is managed by the hardware plugin")
     if section not in {"services","processes"} and key not in existing.get(section,set()):
         raise ValueError(f"unsupported option: {section}.{key}")
-    if section=="hardware" and key=="cooling_hat_enabled":
-        if value.lower() not in ("true","false"): raise ValueError("cooling_hat_enabled must be true or false")
-    elif section=="led" and key=="alert_mode":
-        if value.lower() not in ("breathe","flash"): raise ValueError("alert_mode must be breathe or flash")
-    elif section=="led" and key=="custom_min_hold_ms":
-        if not 1500<=int(value)<=60000: raise ValueError("custom_min_hold_ms must be 1500..60000")
-    elif RGB_KEY_RE.fullmatch(key):
-        number=int(value)
-        if not 0<=number<=255: raise ValueError(f"{key} must be 0..255")
-    elif FLASH_KEY_RE.fullmatch(key):
-        number=int(value)
-        if not 50<=number<=60000: raise ValueError(f"{key} must be 50..60000")
-    elif section=="led" and key=="normal_mode":
-        if value.lower() not in NORMAL_MODES: raise ValueError("normal_mode must be off, solid, flash, flow, breathe, marquee, rainbow or colorful")
-    elif section=="led" and key=="normal_effect_speed":
-        if int(value) not in (1,2,3): raise ValueError("normal_effect_speed must be 1, 2 or 3")
-    elif section=="led" and key=="normal_effect_color":
-        if value.lower() not in EFFECT_COLORS: raise ValueError("normal_effect_color must be red, green, blue, yellow, purple, cyan or white")
-    elif section=="api" and key=="port":
-        number=int(value)
-        if not 1<=number<=65535: raise ValueError("api.port must be 1..65535")
-    elif section=="api" and key=="log_limit":
-        number=int(value)
-        if not 1<=number<=1000: raise ValueError("api.log_limit must be 1..1000")
-    elif section=="api" and key=="bind":
-        ipaddress.ip_address(value)
+    if section in {"alerts","timing"}:
+        number=float(value)
+        if not math.isfinite(number):raise ValueError(f"{section}.{key} must be finite")
+        low,high=(0,86400)
+        if section=="timing":
+            low=0 if key in {"startup_grace_seconds","network_failure_seconds"} else 0.2
+            high=3600
+        elif key in {"cpu_high","cpu_recover","memory_high","memory_recover","disk_high","disk_critical"}:low,high=0,100
+        elif key in {"temp_high","temp_critical"}:low,high=-50,150
+        elif key in {"wifi_weak","wifi_recover"}:low,high=-120,0
+        elif key=="ethernet_min_speed_mbps":low,high=0,1000000
+        if not low<=number<=high:raise ValueError(f"{section}.{key} must be {low}..{high}")
+        if key in {"cpu_high_seconds","memory_high_seconds","ethernet_min_speed_mbps"} and not value.isdecimal():
+            raise ValueError(f"{key} must be a nonnegative integer")
+    elif section=="dark_mode" and value.lower() not in {"on","off","true","false","0","1","yes","no"}:
+        raise ValueError("LED setting must be on or off")
+    elif section=="services" and not UNIT_RE.fullmatch(value):raise ValueError("invalid systemd service name")
+    elif section=="api" and key in {"port","log_limit"}:
+        limit=65535 if key=="port" else 1000
+        if not 1<=int(value)<=limit:raise ValueError(f"api.{key} must be 1..{limit}")
+    elif section=="api" and key=="bind":ipaddress.ip_address(value)
     elif section=="device" and key=="identifier" and value.lower()!="auto" and not NAME_RE.fullmatch(value):
         raise ValueError("device.identifier must be auto or 1..64 letters, numbers, dot, underscore or dash")
 
 
+def validate_candidate(parsed):
+    for lower,upper in (("cpu_recover","cpu_high"),("memory_recover","memory_high"),
+                        ("temp_high","temp_critical"),("disk_high","disk_critical"),("wifi_weak","wifi_recover")):
+        if parsed.has_option("alerts",lower) and parsed.has_option("alerts",upper):
+            if not parsed.getfloat("alerts",lower)<parsed.getfloat("alerts",upper):
+                raise ValueError(f"alerts.{lower} must be less than alerts.{upper}")
+
+
 def replace_ini_value(section,key,value):
     return replace_ini_text(CFG.read_text(),section,key,value)
 
@@ -139,13 +147,23 @@ def replace_ini_text(text,section,key,value):
     if value is not None:return text[:end].rstrip()+f"\n{key} = {value}\n\n"+text[end:].lstrip("\n")
     return text
 
+@config_transaction
 def save_ini_text(text,prefix="api"):
-    parsed=configparser.ConfigParser(); parsed.read_string(text)
-    backup=CFG.with_name(f"{CFG.name}.bak.{prefix}-{time.strftime('%Y%m%d-%H%M%S')}")
+    parsed=configparser.ConfigParser(interpolation=None); parsed.read_string(text)
+    validate_candidate(parsed)
+    backup=CFG.with_name(f"{CFG.name}.bak.{prefix}-{time.time_ns()}")
     shutil.copy2(CFG,backup)
-    temporary=CFG.with_suffix(".tmp"); temporary.write_text(text); temporary.chmod(0o644); temporary.replace(CFG)
+    temporary=None
+    try:
+        with tempfile.NamedTemporaryFile(mode="w",dir=CFG.parent,prefix=CFG.name+".",delete=False) as file:
+            temporary=Path(file.name);file.write(text)
+        temporary.chmod(0o644);temporary.replace(CFG)
+    finally:
+        if temporary is not None:temporary.unlink(missing_ok=True)
     return str(backup)
 
+
+@config_transaction
 def update_ini(updates):
     if not isinstance(updates,dict): raise ValueError("updates must be an object")
     text=CFG.read_text()
@@ -175,12 +193,7 @@ def update_ini(updates):
                 text=text[:start]+replacement+text[stop:]
             else:
                 text=text[:end].rstrip()+f"\n{key} = {value}\n\n"+text[end:].lstrip("\n")
-    parsed=configparser.ConfigParser(); parsed.read_string(text)
-    backup=CFG.with_name(f"{CFG.name}.bak.api-{time.strftime('%Y%m%d-%H%M%S')}")
-    shutil.copy2(CFG,backup)
-    temporary=CFG.with_suffix(".tmp")
-    temporary.write_text(text); temporary.chmod(0o644); temporary.replace(CFG)
-    return str(backup)
+    return save_ini_text(text,"api")
 
 def journal_query(since=None,until=None,severity="",event="",search="",sort="timestamp",order="desc",limit=100,offset=0):
     command=["journalctl","-u","pigway-pi-control.service","--no-pager","-o","json"]
@@ -244,6 +257,7 @@ def service_inventory():
         item["protected"]=item["unit"] in PROTECTED_UNITS
     return sorted(units.values(),key=lambda x:(not x["monitored"],x["unit"]))
 
+@config_transaction
 def edit_service_monitor(operation,name,target,note="",previous_name=""):
     if operation not in {"save","delete","pause","resume"}: raise ValueError("invalid monitor operation")
     if not NAME_RE.fullmatch(name): raise ValueError("invalid monitor name")
@@ -281,10 +295,7 @@ def edit_service_monitor(operation,name,target,note="",previous_name=""):
             change("service_monitor_disabled",previous_name,None)
         change("services",name,target); change("service_notes",name,note or None)
         if was_paused: change("service_monitor_disabled",name,"true")
-    parsed=configparser.ConfigParser(); parsed.read_string(text)
-    backup=CFG.with_name(f"{CFG.name}.bak.api-{time.strftime('%Y%m%d-%H%M%S')}")
-    shutil.copy2(CFG,backup)
-    temporary=CFG.with_suffix(".tmp"); temporary.write_text(text); temporary.chmod(0o644); temporary.replace(CFG)
+    backup=save_ini_text(text,"api")
     subprocess.run(["systemctl","restart","pigway-pi-control.service"],check=True,timeout=10)
     return str(backup)
 
@@ -298,6 +309,7 @@ def process_monitors():
         result.append({"name":name.upper(),"target":target,"up":up})
     return result
 
+@config_transaction
 def edit_process_monitor(operation,name,target="",previous_name=""):
     if operation not in {"save","delete"}: raise ValueError("invalid process monitor operation")
     if not NAME_RE.fullmatch(name): raise ValueError("invalid process monitor name")
@@ -311,7 +323,7 @@ def edit_process_monitor(operation,name,target="",previous_name=""):
     else:
         if previous_name and previous_name.lower()!=name.lower(): text=replace_ini_text(text,"processes",previous_name,None)
         text=replace_ini_text(text,"processes",name,target)
-    parsed=configparser.ConfigParser(); parsed.read_string(text)
+    parsed=configparser.ConfigParser(interpolation=None); parsed.read_string(text)
     backup=save_ini_text(text,"api")
     subprocess.run(["systemctl","restart","pigway-pi-control.service"],check=True,timeout=10)
     return backup
@@ -359,7 +371,10 @@ class Handler(BaseHTTPRequestHandler):
                 self.send_json(200,{"ok":True,"version":APP_VERSION,"status_available":STATUS.exists()});return
             if parsed.path=="/api/v1/status":
                 if not STATUS.exists(): self.send_json(503,{"error":"agent status unavailable"}); return
-                self.send_json(200,json.loads(STATUS.read_text())); return
+                state=json.loads(STATUS.read_text())
+                if not 0<=time.time()-state.get("timestamp",0)<=10:
+                    self.send_json(503,{"error":"agent status stale"});return
+                self.send_json(200,state); return
             if parsed.path=="/api/v1/logs":
                 query=parse_qs(parsed.query)
                 value=lambda key,default="":query.get(key,[default])[0]

app/plugin_api.py

@@ -4,6 +4,7 @@ from contextlib import closing
 import http.client
 import json
 import socket
+import time
 import threading
 import uuid
 from pathlib import Path
@@ -65,14 +66,15 @@ class Publisher:
     """One replaceable snapshot, not an unbounded queue of animation commands."""
     def __init__(self,plugins,source,log):
         self.plugins=plugins;self.source=source;self.log=log
-        self.session=str(uuid.uuid4());self.revision=0;self.snapshot=None
+        self.session=str(uuid.uuid4());self.revision=0;self.snapshot=None;self.published_at=None
         self.lock=threading.Lock();self.stop_event=threading.Event();self.status={}
         self.threads=[threading.Thread(target=self.run,args=(key,path),name='plugin-'+key,daemon=True)
                       for key,path in plugins.items()]
         for thread in self.threads:thread.start()
 
     def publish(self,state):
-        with self.lock:self.snapshot=copy.deepcopy(state)
+        with self.lock:
+            self.snapshot=copy.deepcopy(state);self.published_at=time.monotonic()
 
     def current_status(self):
         with self.lock:return copy.deepcopy(self.status)
@@ -80,8 +82,11 @@ class Publisher:
     def run(self,key,path):
         previous_error=None;revision=0;descriptor=None
         while not self.stop_event.is_set():
-            with self.lock:state=self.snapshot
-            if state is not None:
+            with self.lock:
+                state=self.snapshot
+                fresh=self.published_at is not None and time.monotonic()-self.published_at<3
+            # Never renew a hardware lease from a stalled monitoring loop.
+            if state is not None and fresh:
                 revision+=1
                 payload={'api_version':1,'source':self.source,'session':self.session,'revision':revision,
                          'ttl_seconds':10,'system':state['system'],'network':state['network'],'alerts':state['display']['alerts']}

app/storage_cache.py

@@ -174,7 +174,7 @@ class StorageCache:
             values = data['values']
             if all(isinstance(values[k], int) and not isinstance(values[k], bool) and values[k] >= 0 for k in ('total', 'used', 'free')) and values['total'] > 0 and values['used'] <= values['total']:
                 at = data['observed_at']
-                if isinstance(at, (int, float)) and 0 <= self.clock() - at < 90000:
+                if isinstance(at, (int, float)) and 0 <= self.clock() - at < 604800:
                     self.values, self.observed_at = values, at
                     if not self.error:self.next_update_at = at + 604800
         except (OSError, ValueError, KeyError, TypeError):