From 3becfac66dc569e71d0f6085fb56e92cfa6d626a Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Fri, 24 Jul 2026 12:19:04 -0500 Subject: 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. --- tests/installer-steps/test_switch_udev_hook.py | 178 +++++++++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 tests/installer-steps/test_switch_udev_hook.py (limited to 'tests/installer-steps/test_switch_udev_hook.py') 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() -- cgit v1.2.3