Merge pull request #328 from naturallaw777/copilot/fix-sovran-hosts-update-runtime-dependency

fix(local-domain-loopback): replace raw /etc script with writeShellApplication, declare explicit runtimeInputs

app/tests/test_local_domain_loopback_nix.py

@@ -0,0 +1,130 @@
+"""Structural regression tests for modules/core/local-domain-loopback.nix.
+
+Verifies that the sovran-hosts-update helper:
+  - is built as a pkgs.writeShellApplication with explicit runtimeInputs
+    (gawk, gnugrep, coreutils);
+  - uses ``lib.getExe hostsUpdateScript`` for both the systemd ExecStart and
+    the activation script so that both contexts share the same Nix-store
+    executable;
+  - does NOT point ExecStart at the raw /etc path;
+  - includes element-calling in the supported domain list;
+  - uses ``awk -v`` for safe marker-variable passing rather than interpolating
+    marker text directly into the awk program;
+  - retains the domain validation regex (idempotency / injection prevention);
+  - exposes /etc/sovran-hosts-update.sh as a symlink via ``source =`` (not a
+    second raw ``text =`` body);
+  - does NOT rely on environment.systemPackages for the helper's dependencies.
+"""
+
+import unittest
+from pathlib import Path
+
+NIX_FILE = (
+    Path(__file__).resolve().parents[2]
+    / "modules"
+    / "core"
+    / "local-domain-loopback.nix"
+)
+
+
+class LocalDomainLoopbackNixStructureTests(unittest.TestCase):
+    def setUp(self):
+        self.source = NIX_FILE.read_text()
+
+    # ── writeShellApplication and explicit runtimeInputs ────────────────────
+
+    def test_uses_write_shell_application(self):
+        self.assertIn("pkgs.writeShellApplication", self.source)
+
+    def test_runtime_inputs_includes_coreutils(self):
+        self.assertIn("pkgs.coreutils", self.source)
+
+    def test_runtime_inputs_includes_gawk(self):
+        self.assertIn("pkgs.gawk", self.source)
+
+    def test_runtime_inputs_includes_gnugrep(self):
+        self.assertIn("pkgs.gnugrep", self.source)
+
+    def test_runtime_inputs_block_present(self):
+        self.assertIn("runtimeInputs", self.source)
+
+    # ── Both execution paths use lib.getExe ─────────────────────────────────
+
+    def test_exec_start_uses_lib_get_exe(self):
+        """systemd ExecStart must reference the Nix-store executable."""
+        self.assertIn("ExecStart = lib.getExe hostsUpdateScript", self.source)
+
+    def test_activation_script_uses_lib_get_exe(self):
+        """Activation text must call the same Nix-store executable."""
+        self.assertIn("${lib.getExe hostsUpdateScript}", self.source)
+
+    def test_exec_start_does_not_point_to_etc_path(self):
+        """ExecStart must NOT use the raw /etc path (which lacks a deterministic PATH)."""
+        self.assertNotIn('ExecStart = "/etc/sovran-hosts-update.sh"', self.source)
+
+    # ── /etc symlink uses source =, not a second text = body ────────────────
+
+    def test_etc_entry_uses_source_not_text(self):
+        """The /etc/sovran-hosts-update.sh entry must be a symlink (source =),
+        not a second raw script body (text =)."""
+        self.assertIn(
+            'environment.etc."sovran-hosts-update.sh".source', self.source
+        )
+
+    def test_etc_source_points_to_get_exe(self):
+        self.assertIn(
+            'environment.etc."sovran-hosts-update.sh".source = lib.getExe hostsUpdateScript',
+            self.source,
+        )
+
+    # ── element-calling domain is supported ─────────────────────────────────
+
+    def test_element_calling_domain_key_present(self):
+        self.assertIn("element-calling", self.source)
+
+    # ── Robust awk -v variable passing ──────────────────────────────────────
+
+    def test_awk_uses_dash_v_for_begin_marker(self):
+        """awk must receive the begin marker via -v, not by shell interpolation."""
+        self.assertIn('awk -v begin=', self.source)
+
+    def test_awk_uses_dash_v_for_end_marker(self):
+        self.assertIn('-v end=', self.source)
+
+    def test_awk_does_not_interpolate_marker_into_program(self):
+        """The old pattern interpolated $BEGIN_MARKER directly into the awk source."""
+        self.assertNotIn('/$BEGIN_MARKER', self.source)
+        self.assertNotIn('/$END_MARKER', self.source)
+
+    # ── Domain validation ────────────────────────────────────────────────────
+
+    def test_domain_validation_regex_present(self):
+        """The hostname validation regex must still be present for injection prevention."""
+        self.assertIn("grep -qE", self.source)
+        self.assertIn("[a-zA-Z0-9]", self.source)
+
+    def test_invalid_domain_warning_present(self):
+        self.assertIn("skipping invalid domain value", self.source)
+
+    # ── No environment.systemPackages reliance ───────────────────────────────
+
+    def test_no_environment_system_packages_for_helper(self):
+        """The helper's tools are declared via runtimeInputs; the module must
+        not add them to environment.systemPackages."""
+        self.assertNotIn("environment.systemPackages", self.source)
+
+    # ── Idempotency: existing Sovran block is removed before rewriting ───────
+
+    def test_existing_block_removal_logic_present(self):
+        """awk strip of the managed block must be present for idempotency."""
+        self.assertIn("skip=1", self.source)
+        self.assertIn("skip=0", self.source)
+
+    # ── Activation script warns on failure rather than silently swallowing ───
+
+    def test_activation_script_emits_warning_on_failure(self):
+        self.assertIn("warning: sovran-hosts-update", self.source)
+
+
+if __name__ == "__main__":
+    unittest.main()

modules/core/local-domain-loopback.nix

@@ -32,22 +32,31 @@
 # regenerated by the system activation script.  The ``system.activationScripts``
 # hook below converts it to a writable file each time the system is activated
 # (i.e. after every ``nixos-rebuild switch``) and then injects the Sovran block.
-# The same script is also run by the ``sovran-hosts-update.service`` unit so
-# that the Hub can trigger it immediately after saving a domain without
-# requiring a full rebuild.
+# The same wrapped Nix-store executable is reused by both the activation hook
+# and the ``sovran-hosts-update.service`` unit, ensuring a deterministic runtime
+# PATH in every execution context.
 
-{
-  # ── Helper script (stored in the Nix store, never reads /var/lib at eval) ──
+let
+  # ── Wrapped Nix-store executable ──────────────────────────────────────────
+  # Built with pkgs.writeShellApplication so that all required runtime tools
+  # (awk, grep, coreutils) are declared explicitly and injected into PATH by
+  # Nix.  Both the systemd service and the activation hook reference this same
+  # store-path executable — there is no second raw script body.
+  hostsUpdateScript = pkgs.writeShellApplication {
+    name = "sovran-hosts-update";
 
-  environment.systemPackages = [ pkgs.coreutils ];
+    # Declare every external command the script calls.  These packages are
+    # added to the script's runtime PATH by writeShellApplication; nothing from
+    # the system PATH is relied upon.
+    runtimeInputs = [
+      pkgs.coreutils   # readlink, cp, mv, chmod, mktemp, rm, tr, head
+      pkgs.gawk        # awk
+      pkgs.gnugrep     # grep
+    ];
 
-  environment.etc."sovran-hosts-update.sh" = {
-    mode = "0755";
     text = ''
-      #!/bin/sh
       # Regenerate the Sovran-managed loopback block in /etc/hosts.
       # Safe to run multiple times — idempotent.
-      set -eu
 
       DOMAINS_DIR="/var/lib/domains"
       HOSTS_FILE="/etc/hosts"
@@ -66,13 +75,14 @@
 
       # ── Step 2: remove any existing Sovran block ──────────────────────────
       # Use a temp file so the operation is atomic.
+      # awk -v passes marker strings safely without shell interpolation.
       TMP=$(mktemp "$HOSTS_FILE.XXXXXX")
       trap 'rm -f "$TMP"' EXIT
-      awk "
-        /^$BEGIN_MARKER\$/ { skip=1; next }
-        /^$END_MARKER\$/   { skip=0; next }
+      awk -v begin="$BEGIN_MARKER" -v end="$END_MARKER" '
+        $0 == begin { skip=1; next }
+        $0 == end   { skip=0; next }
         !skip
-      " "$HOSTS_FILE" > "$TMP"
+      ' "$HOSTS_FILE" > "$TMP"
 
       # ── Step 3: collect valid configured service domains ──────────────────
       # NOTE: The hostname validation regex below must stay in sync with
@@ -110,6 +120,14 @@
     '';
   };
 
+in
+{
+  # ── /etc/sovran-hosts-update.sh — operator discoverability symlink ─────────
+  # Retain the familiar /etc path so administrators can inspect or manually
+  # invoke the helper.  The target is the wrapped Nix-store executable, so
+  # there is no second raw script body to keep in sync.
+  environment.etc."sovran-hosts-update.sh".source = lib.getExe hostsUpdateScript;
+
   # ── Systemd service ────────────────────────────────────────────────────────
 
   systemd.services.sovran-hosts-update = {
@@ -126,18 +144,24 @@
     serviceConfig = {
       Type = "oneshot";
       RemainAfterExit = true;
-      ExecStart = "/etc/sovran-hosts-update.sh";
+      # Point directly at the wrapped Nix-store executable, not the /etc path.
+      ExecStart = lib.getExe hostsUpdateScript;
     };
   };
 
   # ── Activation script (runs after every nixos-rebuild switch) ─────────────
   # This ensures the loopback block survives rebuilds that restore the /etc/hosts
   # symlink.  The "users" and "etc" scripts must complete first.
+  # The same wrapped Nix-store executable used by the systemd service is
+  # referenced here, guaranteeing identical runtime dependencies in both
+  # execution contexts.
 
   system.activationScripts.sovranDomainLoopback = {
     text = ''
-      if [ -x /etc/sovran-hosts-update.sh ] && [ -d /var/lib/domains ]; then
-        /etc/sovran-hosts-update.sh || true
+      if [ -d /var/lib/domains ]; then
+        if ! ${lib.getExe hostsUpdateScript}; then
+          echo "warning: sovran-hosts-update: failed to update /etc/hosts loopback entries" >&2
+        fi
       fi
     '';
     deps = [ "etc" "users" ];