fix(installer): eight fixes from an overnight bug-hunt and its review

I squashed these because the per-bug reasoning lives in todo.org, which this commit carries.

Two could cost a machine. configure_initramfs_hook swapped the udev hook for systemd on a LUKS root, leaving a standalone encrypt hook under an init that never runs it. The rebuild succeeds and the installer exits clean, then the root won't unlock at the next boot. trim_firmware ran pacman -Rdd against twelve firmware packages behind a DMI gate reading product_name, where "Framework" never appears. That left it dead on the hardware it targets, and dangerous to fix the obvious way: this machine is a Framework Desktop whose Ryzen iGPU needs the amdgpu firmware. It refuses on PCI modalias evidence now.

wipedisk discarded before it checked. blkdiscard ran with -f, which disables the exclusive open, so picking the wrong disk destroyed a live filesystem and then reported that nothing had happened.

Four more are smaller. The NVIDIA preflight aborted dwm and headless installs over a driver floor they never need. zfs-replicate exited 0 after every dataset failed. Unattended installs blocked on two prompts, and the first fix for that inherited a [Y/n] default into passwordless console login. A fresh install left the dotfiles repo permanently dirty.

The review found a pattern worth more than any single fix. Helpers had thorough tests and none proved they were called. Deleting the call left five suites green, including the guard on that pacman -Rdd. CALL_SITES now pins nine caller/callee pairs.

The suite runs 341 tests at exit 0, with no new shellcheck findings. I proved every guard by deleting it and watching the intended test go red.

1 files changed, 178 insertions, 0 deletions

diff --git a/tests/installer-steps/test_switch_udev_hook.py b/tests/installer-steps/test_switch_udev_hook.py
new file mode 100644
index 0000000..54969e6
--- /dev/null
+++ b/tests/installer-steps/test_switch_udev_hook.py
@@ -0,0 +1,178 @@
+"""Test the udev -> systemd initramfs hook swap.
+
+configure_initramfs_hook swaps the `udev` initramfs hook for `systemd` so fsck
+output routes through systemd. mkinitcpio's `systemd` hook symlinks /init to
+systemd, which never executes the ash `run_hook` scripts the busybox init
+drives -- so any busybox-only hook left in HOOKS is installed into the image
+and then never runs.
+
+`encrypt` is the boot-critical member of that set: it is what unlocks a LUKS
+root, and its `cryptdevice=` cmdline parameter (which merge_grub_cmdline
+already treats as boot-critical) is read by nothing else. mkinitcpio has no
+conflict check, so `mkinitcpio -P` succeeds, archsetup reports success, and the
+machine fails to unlock its root at the next boot. The systemd equivalent is
+`sd-encrypt` driven by `rd.luks.*`, and converting between the two means
+rewriting the kernel cmdline against the volume's UUID -- so the step refuses
+the swap rather than attempting a migration.
+
+Method: sed-extract switch_udev_hook_to_systemd + hooks_need_busybox_init from
+the real `archsetup`, point them at a temp mkinitcpio.conf, and fake
+backup_system_file/display/error_warn.
+
+Run from repo root:
+    python3 -m unittest tests.installer-steps.test_switch_udev_hook
+"""
+
+import os
+import re
+import subprocess
+import tempfile
+import textwrap
+import unittest
+
+REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
+ARCHSETUP = os.path.join(REPO_ROOT, "archsetup")
+
+# The two HOOKS lines Arch's own mkinitcpio.conf documents for an encrypted
+# root -- the busybox pairing and the systemd pairing. They are the fixtures
+# that matter, because the bug is turning the first into neither.
+BUSYBOX_ENCRYPT = (
+    "HOOKS=(base udev microcode modconf keyboard keymap consolefont block "
+    "mdadm_udev encrypt filesystems fsck)\n"
+)
+SYSTEMD_ENCRYPT = (
+    "HOOKS=(base systemd autodetect microcode modconf kms keyboard sd-vconsole "
+    "sd-encrypt block filesystems fsck)\n"
+)
+PLAIN_UDEV = "HOOKS=(base udev autodetect microcode modconf block filesystems fsck)\n"
+
+
+def run(conf_body, missing_conf=False):
+    with tempfile.TemporaryDirectory() as d:
+        conf = os.path.join(d, "mkinitcpio.conf")
+        if not missing_conf:
+            with open(conf, "w") as f:
+                f.write(conf_body)
+        script = textwrap.dedent(f"""\
+            logfile=/dev/null
+            action=""
+            display() {{ echo "DISPLAY: $2"; }}
+            backup_system_file() {{ echo "BACKUP: $1" >> "{d}/backup.log"; }}
+            error_warn() {{ echo "WARN: $1"; return 1; }}
+            source <(sed -n '/^hooks_need_busybox_init() {{/,/^}}/p;/^switch_udev_hook_to_systemd() {{/,/^}}/p' "{ARCHSETUP}")
+            switch_udev_hook_to_systemd "{conf}"
+            echo "RC=$?"
+            echo "CONF:[$(cat "{conf}" 2>/dev/null)]"
+            [ -f "{d}/backup.log" ] && cat "{d}/backup.log"
+            exit 0
+        """)
+        return subprocess.run(
+            ["bash", "-c", script], capture_output=True, text=True, timeout=10,
+        )
+
+
+def rc_of(r):
+    """The step's exact return code. Substring-matching "RC=1" in stdout would
+    also match "RC=127" (the code bash gives a missing function), so a broken
+    extraction would read as a passing test."""
+    m = re.search(r"^RC=(\d+)$", r.stdout, re.M)
+    assert m, "no RC line in output: %r / %r" % (r.stdout, r.stderr)
+    return int(m.group(1))
+
+
+class SwitchUdevHookToSystemd(unittest.TestCase):
+    # ------------------------------------------------------------ normal ----
+    def test_plain_udev_is_swapped_and_reports_change(self):
+        r = run(PLAIN_UDEV)
+        self.assertIn("HOOKS=(base systemd autodetect", r.stdout)
+        self.assertNotIn("udev", r.stdout.split("CONF:[")[1])
+        self.assertEqual(rc_of(r), 0, "a conf that changed must report success")
+        self.assertIn("BACKUP:", r.stdout, "the conf must be backed up before the edit")
+
+    def test_already_systemd_is_left_alone_and_reports_no_change(self):
+        body = "HOOKS=(base systemd autodetect block filesystems fsck)\n"
+        r = run(body)
+        self.assertIn(body.strip(), r.stdout)
+        self.assertEqual(rc_of(r), 1,
+                         "an unchanged conf must not report a change (no needless rebuild)")
+
+    # ---------------------------------------------------------- boundary ----
+    def test_busybox_encrypt_hook_blocks_the_swap(self):
+        # THE BUG: swapping here leaves `encrypt` under a systemd init that
+        # never runs it, so the LUKS root never unlocks and the machine will
+        # not boot.
+        r = run(BUSYBOX_ENCRYPT)
+        self.assertIn(" udev ", r.stdout, "udev must survive next to a busybox encrypt hook")
+        self.assertIn(" encrypt ", r.stdout)
+        self.assertNotIn("systemd", r.stdout.split("CONF:[")[1])
+        self.assertEqual(rc_of(r), 1)
+        self.assertNotIn("BACKUP:", r.stdout, "a declined swap must not touch the file")
+
+    def test_tab_separated_encrypt_hook_blocks_the_swap(self):
+        # The token class was [( ] / [ )], which is a literal space and does
+        # not include a tab. mkinitcpio's HOOKS is a bash array literal, and
+        # bash word-splits on IFS -- tabs included -- so a tab-separated HOOKS
+        # line is valid, loads correctly, and slipped straight past the guard
+        # into the exact unbootable state it exists to prevent.
+        r = run("HOOKS=(base udev block\tencrypt\tfilesystems fsck)\n")
+        self.assertEqual(rc_of(r), 1, "a tab-separated encrypt must block the swap")
+        self.assertIn("encrypt", r.stdout)
+        self.assertNotIn("systemd", r.stdout.split("CONF:[")[1])
+        self.assertNotIn("BACKUP:", r.stdout)
+
+    def test_tab_separated_sd_encrypt_is_still_not_the_busybox_hook(self):
+        # Widening the class must not widen what counts as the busybox hook:
+        # sd-encrypt IS the systemd one and has to keep swapping.
+        r = run("HOOKS=(base\tsystemd\tsd-encrypt\tfilesystems)\n")
+        self.assertNotIn("BACKUP:", r.stdout)   # already systemd, nothing to do
+        self.assertEqual(rc_of(r), 1)
+
+    def test_tab_separated_udev_without_encrypt_still_swaps(self):
+        r = run("HOOKS=(base\tudev\tblock\tfilesystems\tfsck)\n")
+        self.assertEqual(rc_of(r), 0, "a tab-separated conf with no encrypt must swap")
+        self.assertIn("systemd", r.stdout.split("CONF:[")[1])
+
+    def test_plymouth_encrypt_hook_blocks_the_swap(self):
+        r = run("HOOKS=(base udev block plymouth-encrypt filesystems)\n")
+        self.assertIn(" udev ", r.stdout)
+        self.assertEqual(rc_of(r), 1)
+
+    def test_sd_encrypt_is_not_mistaken_for_the_busybox_hook(self):
+        # sd-encrypt IS the systemd hook -- it must not be read as a reason to
+        # refuse. (No udev here, so the swap is a no-op either way; what this
+        # pins is that the substring match does not over-trigger.)
+        r = run(SYSTEMD_ENCRYPT)
+        self.assertIn("sd-encrypt", r.stdout)
+        self.assertNotIn("HOOKS carries encrypt", r.stdout)
+
+    def test_encrypt_as_last_token_is_detected(self):
+        r = run("HOOKS=(base udev block encrypt)\n")
+        self.assertIn(" udev ", r.stdout)
+        self.assertEqual(rc_of(r), 1)
+
+    def test_encrypt_as_first_token_is_detected(self):
+        r = run("HOOKS=(encrypt udev block)\n")
+        self.assertIn("(encrypt udev block)", r.stdout)
+        self.assertEqual(rc_of(r), 1)
+
+    def test_only_the_hooks_line_is_rewritten(self):
+        # A commented example line mentioning udev must survive untouched.
+        body = "# HOOKS=(base udev block filesystems fsck)\n" + PLAIN_UDEV
+        r = run(body)
+        self.assertIn("# HOOKS=(base udev block filesystems fsck)", r.stdout)
+        self.assertIn("HOOKS=(base systemd autodetect", r.stdout)
+
+    # ------------------------------------------------------------- error ----
+    def test_conf_without_a_hooks_line_is_a_no_op(self):
+        r = run("MODULES=(nvme)\n")
+        self.assertEqual(rc_of(r), 1)
+        self.assertIn("MODULES=(nvme)", r.stdout)
+
+    def test_missing_conf_is_a_no_op_not_a_crash(self):
+        r = run("", missing_conf=True)
+        self.assertEqual(rc_of(r), 1)
+        self.assertEqual(r.returncode, 0)
+
+
+if __name__ == "__main__":
+    unittest.main()