fix(installer): merge GRUB cmdline instead of overwriting it

configure_grub replaced the whole GRUB_CMDLINE_LINUX_DEFAULT line with a fixed string. A base install that had set cryptdevice=, resume=, zfs=, or any other boot-critical parameter lost it, and the grub-mkconfig that follows baked an unbootable config.

update_grub_cmdline now reads the current value and merges: every existing token survives, archsetup's tokens are added, and where both set the same key archsetup's value wins. A safety check refuses to write when any existing token's key would vanish from the merge, and the write goes through awk + mv so paths with slashes can't break the substitution. The block also backs up /etc/default/grub before editing, which the other system-file edits already did.

1 files changed, 121 insertions, 0 deletions

diff --git a/tests/installer-steps/test_grub_cmdline.py b/tests/installer-steps/test_grub_cmdline.py
new file mode 100644
index 0000000..8c0b5b0
--- /dev/null
+++ b/tests/installer-steps/test_grub_cmdline.py
@@ -0,0 +1,121 @@
+"""Test the GRUB cmdline merge.
+
+Bug (P2): configure_grub rewrote the whole GRUB_CMDLINE_LINUX_DEFAULT line
+with a fixed string. A base install that had set cryptdevice=/resume=/zfs=
+(or any other boot-critical token) lost it, and the following grub-mkconfig
+baked an unbootable config.
+
+The fix is a merge: every pre-existing token survives, archsetup's tokens are
+added, and where both set the same key archsetup's value wins. A safety
+assert refuses to write if any existing token's key would vanish.
+
+Method: sed-extract merge_grub_cmdline (pure) and update_grub_cmdline
+(file-level, run against temp grub files with a fake error_warn).
+
+Run from repo root:
+    python3 -m unittest tests.installer-steps.test_grub_cmdline
+"""
+
+import os
+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")
+
+EXTRACT = (
+    "source <(sed -n '/^merge_grub_cmdline() {{/,/^}}/p;"
+    "/^update_grub_cmdline() {{/,/^}}/p' \"{a}\")"
+).format(a=ARCHSETUP)
+
+
+def run(body):
+    script = f"logfile=/dev/null\nerror_warn() {{ echo \"WARN: $1\"; return 1; }}\n{EXTRACT}\n{body}\n"
+    return subprocess.run(["bash", "-c", script],
+                          capture_output=True, text=True, timeout=10)
+
+
+def merge(existing, desired):
+    r = run(f'merge_grub_cmdline "{existing}" "{desired}"')
+    return r.stdout.strip()
+
+
+class MergeGrubCmdline(unittest.TestCase):
+    DESIRED = "rw loglevel=2 quiet splash"
+
+    def test_empty_existing_yields_desired(self):
+        self.assertEqual(merge("", self.DESIRED), self.DESIRED)
+
+    def test_boot_critical_tokens_survive(self):
+        out = merge("cryptdevice=UUID=abc:root resume=/dev/nvme0n1p3 root=/dev/mapper/root",
+                    self.DESIRED)
+        for tok in ("cryptdevice=UUID=abc:root", "resume=/dev/nvme0n1p3",
+                    "root=/dev/mapper/root", "loglevel=2", "quiet", "splash"):
+            self.assertIn(tok, out.split(), f"{tok} missing from: {out}")
+
+    def test_same_key_desired_value_wins_once(self):
+        out = merge("loglevel=7 quiet", self.DESIRED).split()
+        self.assertIn("loglevel=2", out)
+        self.assertNotIn("loglevel=7", out)
+        self.assertEqual(out.count("loglevel=2"), 1)
+        self.assertEqual(out.count("quiet"), 1)
+
+    def test_zfs_token_survives(self):
+        out = merge("zfs=zroot/ROOT/default", self.DESIRED).split()
+        self.assertIn("zfs=zroot/ROOT/default", out)
+
+
+class UpdateGrubCmdline(unittest.TestCase):
+    def run_update(self, grub_body):
+        with tempfile.TemporaryDirectory() as d:
+            path = os.path.join(d, "grub")
+            with open(path, "w") as f:
+                f.write(grub_body)
+            r = run(f'update_grub_cmdline "{path}"; echo "RC=$?"')
+            with open(path) as f:
+                return r, f.read()
+
+    def line(self, content):
+        return [ln for ln in content.splitlines()
+                if ln.startswith('GRUB_CMDLINE_LINUX_DEFAULT=')]
+
+    def test_existing_tokens_preserved_in_file(self):
+        _, content = self.run_update(textwrap.dedent("""\
+            GRUB_TIMEOUT=5
+            GRUB_CMDLINE_LINUX_DEFAULT="loglevel=3 cryptdevice=UUID=abc:root resume=/dev/sda2"
+            GRUB_DISABLE_RECOVERY=true
+        """))
+        lines = self.line(content)
+        self.assertEqual(len(lines), 1)
+        val = lines[0]
+        self.assertIn("cryptdevice=UUID=abc:root", val)
+        self.assertIn("resume=/dev/sda2", val)
+        self.assertIn("loglevel=2", val)     # archsetup's value wins
+        self.assertNotIn("loglevel=3", val)
+        self.assertIn("quiet", val)
+        # other lines untouched
+        self.assertIn("GRUB_TIMEOUT=5", content)
+        self.assertIn("GRUB_DISABLE_RECOVERY=true", content)
+
+    def test_quoting_stays_a_single_pair(self):
+        _, content = self.run_update('GRUB_CMDLINE_LINUX_DEFAULT="quiet"\n')
+        val = self.line(content)[0]
+        self.assertEqual(val.count('"'), 2)
+        self.assertRegex(val, r'^GRUB_CMDLINE_LINUX_DEFAULT="[^"]*"$')
+
+    def test_commented_line_gets_active_line_appended(self):
+        _, content = self.run_update('#GRUB_CMDLINE_LINUX_DEFAULT="quiet"\n')
+        self.assertEqual(len(self.line(content)), 1)
+        self.assertIn('#GRUB_CMDLINE_LINUX_DEFAULT="quiet"', content)
+
+    def test_idempotent_on_rerun(self):
+        body = 'GRUB_CMDLINE_LINUX_DEFAULT="cryptdevice=UUID=abc:root"\n'
+        _, once = self.run_update(body)
+        _, twice = self.run_update(once)
+        self.assertEqual(once, twice)
+
+
+if __name__ == "__main__":
+    unittest.main()