Don't spawn long-running daemons on an undrained stdout PIPE; tear down NAT rules on AP stop

pi/debug_controller.py

@@ -21,6 +21,7 @@
 OPENOCD_SCRIPTS = os.environ.get(
     "OPENOCD_SCRIPTS", "/usr/local/share/openocd-esp32/scripts")
 OPENOCD_START_TIMEOUT = 5.0
+OPENOCD_LOG = os.environ.get("OPENOCD_LOG", "/tmp/openocd.log")
 
 # Per-chip OpenOCD board configs (USB JTAG built-in)
 BUILTIN_CONFIGS = {
@@ -421,14 +422,21 @@ def start(slot_label: str, slot: dict, gdb_port: int, telnet_port: int,
         cmd += ["-c", f"tcl port {tcl_port_for(gdb_port)}"]
         cmd += ["-c", "bindto 0.0.0.0"]
 
-        # Launch OpenOCD
+        # Launch OpenOCD. stdout goes to a file, never a PIPE: a pipe
+        # nobody drains fills up (~64KB) and then blocks the daemon in
+        # write(), silently wedging it — the failure mode that froze
+        # dnsmasq/hostapd in wifi_controller on a live bench 2026-07-18,
+        # and OpenOCD is just as chatty across long GDB sessions.
         try:
-            proc = subprocess.Popen(
-                cmd,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT,
-                text=True,
-            )
+            logf = open(OPENOCD_LOG, "ab")
+            try:
+                proc = subprocess.Popen(
+                    cmd,
+                    stdout=logf,
+                    stderr=subprocess.STDOUT,
+                )
+            finally:
+                logf.close()  # child keeps its own duplicated fd
         except FileNotFoundError:
             return {"ok": False,
                     "error": f"openocd not found at {OPENOCD_EXE}"}
@@ -437,11 +445,14 @@ def start(slot_label: str, slot: dict, gdb_port: int, telnet_port: int,
 
         # Wait for GDB port to open
         if not _wait_for_port(gdb_port):
-            # Read any output for error diagnosis
+            # Read the log tail for error diagnosis
             output = ""
             try:
                 proc.kill()
-                output = proc.stdout.read()[:500] if proc.stdout else ""
+                with open(OPENOCD_LOG, "rb") as f:
+                    f.seek(0, os.SEEK_END)
+                    f.seek(max(0, f.tell() - 500))
+                    output = f.read().decode(errors="replace")
             except Exception:
                 pass
             if probe:

pi/mqtt_controller.py

@@ -124,18 +124,32 @@ def start():
         with open(MOSQUITTO_CONF, "w") as f:
             f.write("\n".join(conf_lines) + "\n")
 
-        # Start mosquitto
-        _proc = subprocess.Popen(
-            ["mosquitto", "-c", MOSQUITTO_CONF],
-            stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
-        )
+        # Start mosquitto. stdout goes to a file, never a PIPE: a pipe
+        # nobody drains fills up (~64KB) and then blocks the daemon in
+        # write(), silently wedging it — the failure mode that froze
+        # dnsmasq/hostapd in wifi_controller on a live bench 2026-07-18.
+        stdout_log = os.path.join(WORK_DIR, "mosquitto.stdout.log")
+        logf = open(stdout_log, "ab")
+        try:
+            _proc = subprocess.Popen(
+                ["mosquitto", "-c", MOSQUITTO_CONF],
+                stdout=logf, stderr=subprocess.STDOUT,
+            )
+        finally:
+            logf.close()  # child keeps its own duplicated fd
 
         # Wait for it to initialise
         time.sleep(1.0)
         if _proc.poll() is not None:
-            out = _proc.stdout.read().decode(errors="replace")
             _active = False
-            raise RuntimeError(f"mosquitto failed to start: {out[:500]}")
+            try:
+                with open(stdout_log, "rb") as f:
+                    f.seek(0, os.SEEK_END)
+                    f.seek(max(0, f.tell() - 500))
+                    out = f.read().decode(errors="replace")
+            except OSError:
+                out = ""
+            raise RuntimeError(f"mosquitto failed to start: {out}")
 
         _active = True
         logger.info("MQTT broker started on port %d", MQTT_PORT)

pi/wifi_controller.py

@@ -262,6 +262,16 @@ def _flush_addr(iface: str = None):
         pass
 
 
+# (table, chain, rule-args) for the wlan0→eth0 NAT bridge — shared by
+# _enable_nat/_disable_nat so setup and teardown can't drift apart.
+_NAT_RULES = [
+    ("nat", "POSTROUTING", ["-s", AP_SUBNET, "-o", "eth0", "-j", "MASQUERADE"]),
+    ("filter", "FORWARD", ["-i", WLAN_IF, "-o", "eth0", "-j", "ACCEPT"]),
+    ("filter", "FORWARD", ["-i", "eth0", "-o", WLAN_IF, "-m", "state",
+                           "--state", "RELATED,ESTABLISHED", "-j", "ACCEPT"]),
+]
+
+
 def _enable_nat():
     """Bridge the wlan0 AP to the LAN/internet via NAT masquerade on eth0.
 
@@ -270,14 +280,7 @@ def _enable_nat():
     """
     subprocess.run(["sysctl", "-w", "net.ipv4.ip_forward=1"],
                    capture_output=True, check=False)
-    # (table, chain, rule-args) — added only if not already present (-C).
-    rules = [
-        ("nat", "POSTROUTING", ["-o", "eth0", "-j", "MASQUERADE"]),
-        ("filter", "FORWARD", ["-i", WLAN_IF, "-o", "eth0", "-j", "ACCEPT"]),
-        ("filter", "FORWARD", ["-i", "eth0", "-o", WLAN_IF, "-m", "state",
-                               "--state", "RELATED,ESTABLISHED", "-j", "ACCEPT"]),
-    ]
-    for table, chain, args in rules:
+    for table, chain, args in _NAT_RULES:
         base = ["iptables", "-t", table]
         exists = subprocess.run(base + ["-C", chain] + args,
                                 capture_output=True, check=False).returncode == 0
@@ -286,6 +289,24 @@ def _enable_nat():
                            capture_output=True, check=False)
 
 
+def _disable_nat():
+    """Remove the NAT bridge rules (best effort, idempotent).
+
+    Without this, one ap_start(internet=True) leaves the bridge in place
+    forever — every later plain ap_start would still give the DUT a path to
+    the LAN/internet, silently breaking the isolation an air-gapped test
+    assumes it has.
+    """
+    for table, chain, args in _NAT_RULES:
+        base = ["iptables", "-t", table]
+        for _ in range(10):  # bounded: -D can fail while -C still matches
+            if subprocess.run(base + ["-C", chain] + args,
+                              capture_output=True, check=False).returncode != 0:
+                break
+            subprocess.run(base + ["-D", chain] + args,
+                           capture_output=True, check=False)
+
+
 def ap_start(ssid, password="", channel=6, dns_logging=False, internet=False):
     """Start SoftAP on wlan0. Returns dict with ip.
 
@@ -439,6 +460,8 @@ def ap_start(ssid, password="", channel=6, dns_logging=False, internet=False):
 
         if internet:
             _enable_nat()
+        else:
+            _disable_nat()  # a previous internet=True AP must not leak isolation
 
         _ap_active = True
         _start_hostapd_watcher()
@@ -466,6 +489,7 @@ def _ap_stop_unlocked():
     _ap_dnsmasq_proc = None
     _kill_proc(_ap_hostapd_proc)
     _ap_hostapd_proc = None
+    _disable_nat()
 
     _ap_active = False
     _ap_ssid = ""