From c1b40ef819a1bd325e3b134e01d9e1838fffee0e Mon Sep 17 00:00:00 2001
From: atlas <atlas@hyperhive>
Date: Sun, 31 May 2026 21:03:56 +0200
Subject: [PATCH] network: fix dead assertion + document HIVE_NETWORK_SUBNET
 host-address
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two argus review fixups:

1. Move isolateContainers assertion to an unconditional `lib.mkIf
   cfg.isolateContainers` arm. The prior placement inside
   `mkIf (enable && isolateContainers)` was dead code — the assertion
   could never fire because both flags were already true by the time the
   block activated. Now `isolateContainers=true; enable=false;` raises
   a NixOS assertion error at eval time.

2. Add comment on HIVE_NETWORK_SUBNET noting it carries the host-side
   bridge IP (e.g. "10.42.0.1/24"), not the canonical network address
   ("10.42.0.0/24"). Rust consumer must normalize via bitwise AND before
   subnet membership checks or address arithmetic.
---
 nix/modules/hive-network.nix | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/nix/modules/hive-network.nix b/nix/modules/hive-network.nix
index 8e91e45b..8da7fe17 100644
--- a/nix/modules/hive-network.nix
+++ b/nix/modules/hive-network.nix
@@ -185,18 +185,13 @@ in
       };
     })
 
-    # Container network isolation (#14 v1). Ships as a separate overlay
-    # on top of the base bridge config (which stays unconditional) so
-    # operators can stand the bridge + resolver up first, validate
-    # everything, then flip isolation on independently.
-    (lib.mkIf (cfg.enable && cfg.isolateContainers) {
+    # Guard: isolateContainers requires the bridge to be up. This arm is
+    # unconditional on cfg.enable so the assertion fires even when an
+    # operator sets isolateContainers=true but forgets enable=true (the
+    # combined-condition arm below would silently do nothing in that case).
+    (lib.mkIf cfg.isolateContainers {
       assertions = [
         {
-          # Isolation without the bridge is a no-op: agents would get
-          # private netns but no reachable gateway. The assertion on
-          # `cfg.enable` above already gates the bridge, but making the
-          # dependency explicit here avoids confusing "bridge up, no
-          # isolation" vs "isolation on, no bridge" states.
           assertion = cfg.enable;
           message = ''
             services.hyperhive.network.isolateContainers = true requires
@@ -205,6 +200,13 @@ in
           '';
         }
       ];
+    })
+
+    # Container network isolation (#14 v1). Ships as a separate overlay
+    # on top of the base bridge config (which stays unconditional) so
+    # operators can stand the bridge + resolver up first, validate
+    # everything, then flip isolation on independently.
+    (lib.mkIf (cfg.enable && cfg.isolateContainers) {
 
       # IP forwarding — agents need to route through the bridge to reach
       # the internet. NixOS firewall's `nat.enable` sets this too, but
@@ -254,6 +256,12 @@ in
       # merges contributions from all modules that set it, so this
       # cross-module injection is idiomatic and doesn't require a
       # dedicated option in hive-c0re.nix.
+      #
+      # Note: HIVE_NETWORK_SUBNET is the host-side bridge IP + prefix
+      # length (e.g. "10.42.0.1/24"), not the canonical network address
+      # ("10.42.0.0/24"). The Rust side must normalize (bitwise AND the
+      # IP with the mask) before using it for subnet membership checks or
+      # address arithmetic.
       systemd.services.hive-c0re.environment = {
         HIVE_NETWORK_ISOLATION = "1";
         HIVE_NETWORK_BRIDGE = cfg.bridgeName;