aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-04-27 13:00:26 -0500
committerCraig Jennings <c@cjennings.net>2026-04-27 13:00:26 -0500
commit26f3f823ac17940a1b0153619f6140f45d856e33 (patch)
treebdb02b6fc0c110ce6e59ffd379521068a446c45a
parent8c69aaaff13da3b7d1d24ed34975e8c5b30409e6 (diff)
downloadarchangel-26f3f823ac17940a1b0153619f6140f45d856e33.tar.gz
archangel-26f3f823ac17940a1b0153619f6140f45d856e33.zip
refactor: verify GRUB_CMDLINE_LINUX seds via prepend_grub_cmdline_linux helper
Audited the ~10 silent sed -i sites in the installer against the verification-after pattern that landed for sshd_config last session. Triaged each by failure mode. The two GRUB_CMDLINE_LINUX seds in lib/btrfs.sh have a real silent-failure risk. If /etc/default/grub is missing or malformed and the sed pattern doesn't match, nothing happens. The kernel boots without cryptdevice=. The system can't unlock LUKS at boot. Added prepend_grub_cmdline_linux to lib/common.sh. Same shape as enable_sshd_root_login (sed, then grep, then error if the line wasn't modified). Replaced the two inline seds with helper calls. The HOOKS= seds in installer/archangel and lib/btrfs.sh (six total) don't need verification. A missing HOOKS= line makes mkinitcpio -P fail loudly downstream, so silent-replace failure can't reach a booted system. Added a one-line audit-rationale comment at each of the three locations so the next reader doesn't re-litigate the decision. The FILES= sed at lib/btrfs.sh:213 already self-heals via a sed-then-grep-then-append pattern, so no behavior change there. Filed a separate follow-up to lift that pattern into a named helper for clarity. Bats: 142 → 146. Four new tests in test_common.bats cover normal (empty cmdline, existing cmdline preserved, other lines preserved) and error (missing GRUB_CMDLINE_LINUX line). Lint clean.
-rwxr-xr-xinstaller/archangel3
-rw-r--r--installer/lib/btrfs.sh11
-rw-r--r--installer/lib/common.sh18
-rw-r--r--testing-strategy.org2
-rw-r--r--tests/unit/test_common.bats64
5 files changed, 95 insertions, 3 deletions
diff --git a/installer/archangel b/installer/archangel
index 12f1cd0..479b53a 100755
--- a/installer/archangel
+++ b/installer/archangel
@@ -896,6 +896,9 @@ EOF
# This ensures NVMe, AHCI, and other storage drivers are always included
# - Remove fsck: ZFS doesn't use it, avoids confusing error messages
# - Add zfs: required for ZFS root boot
+ # No sed verification needed: a missing HOOKS= line makes mkinitcpio -P
+ # fail loudly downstream, so silent-replace failure can't reach a booted
+ # system. (Audited 2026-04-27 against silent-sed pattern.)
sed -i 's/^HOOKS=.*/HOOKS=(base udev microcode modconf kms keyboard keymap consolefont block zfs filesystems)/' /mnt/etc/mkinitcpio.conf
# Get the installed kernel version (not the running kernel)
diff --git a/installer/lib/btrfs.sh b/installer/lib/btrfs.sh
index 09127b2..f704fd7 100644
--- a/installer/lib/btrfs.sh
+++ b/installer/lib/btrfs.sh
@@ -204,6 +204,8 @@ configure_luks_initramfs() {
# Add encrypt hook before filesystems (configure_btrfs_initramfs overwrites
# this with the final hook list, using sd-encrypt for multi-disk setups)
+ # No sed verification needed: a missing HOOKS= line makes mkinitcpio -P
+ # fail loudly downstream. (Audited 2026-04-27 against silent-sed pattern.)
sed -i 's/^HOOKS=.*/HOOKS=(base udev microcode modconf kms keyboard keymap consolefont block encrypt filesystems fsck)/' \
/mnt/etc/mkinitcpio.conf
@@ -247,7 +249,8 @@ configure_luks_grub() {
info "Testing mode: adding cryptkey parameter for automated unlock"
fi
- sed -i "s|^GRUB_CMDLINE_LINUX=\"|GRUB_CMDLINE_LINUX=\"cryptdevice=UUID=$uuid:$LUKS_MAPPER_NAME:allow-discards ${cryptkey_param}|" \
+ prepend_grub_cmdline_linux \
+ "cryptdevice=UUID=$uuid:$LUKS_MAPPER_NAME:allow-discards ${cryptkey_param}" \
/mnt/etc/default/grub
info "GRUB configured with cryptdevice parameter and cryptodisk enabled."
@@ -613,7 +616,8 @@ EOF
cryptkey_param="cryptkey=rootfs:$LUKS_KEYFILE "
info "Testing mode: adding cryptkey parameter for automated unlock"
fi
- sed -i "s|^GRUB_CMDLINE_LINUX=\"|GRUB_CMDLINE_LINUX=\"cryptdevice=UUID=$uuid:$LUKS_MAPPER_NAME:allow-discards ${cryptkey_param}|" \
+ prepend_grub_cmdline_linux \
+ "cryptdevice=UUID=$uuid:$LUKS_MAPPER_NAME:allow-discards ${cryptkey_param}" \
/mnt/etc/default/grub
info "Added cryptdevice parameter for LUKS partition."
fi
@@ -844,6 +848,9 @@ EOF
# Configure hooks for btrfs
# Include encrypt hook if LUKS is enabled, btrfs hook if multi-device
+ # No sed verification needed on the four HOOKS= seds below: a missing
+ # HOOKS= line makes mkinitcpio -P fail loudly downstream. (Audited
+ # 2026-04-27 against silent-sed pattern.)
local num_disks=${#SELECTED_DISKS[@]}
local luks_enabled="no"
[[ "$NO_ENCRYPT" != "yes" && -n "$LUKS_PASSPHRASE" ]] && luks_enabled="yes"
diff --git a/installer/lib/common.sh b/installer/lib/common.sh
index 3040799..dfeb245 100644
--- a/installer/lib/common.sh
+++ b/installer/lib/common.sh
@@ -302,3 +302,21 @@ enable_sshd_root_login() {
grep -q '^PermitRootLogin yes$' "$config_file" \
|| error "PermitRootLogin not set in $config_file (no matching line to replace)"
}
+
+#############################
+# GRUB Configuration
+#############################
+
+# Prepend a string just inside the GRUB_CMDLINE_LINUX="..." quotes in
+# /etc/default/grub. Errors if the line isn't present in the file.
+# Silently doing nothing here would leave the kernel without the
+# parameter — for cryptdevice= that means the system can't unlock the
+# root partition at boot, so we want a loud failure during install
+# rather than an unbootable system after first reboot.
+prepend_grub_cmdline_linux() {
+ local addition="$1"
+ local config_file="$2"
+ sed -i "s|^GRUB_CMDLINE_LINUX=\"|GRUB_CMDLINE_LINUX=\"${addition}|" "$config_file"
+ grep -qF "GRUB_CMDLINE_LINUX=\"${addition}" "$config_file" \
+ || error "GRUB_CMDLINE_LINUX not modified in $config_file (line missing or pattern unmatched)"
+}
diff --git a/testing-strategy.org b/testing-strategy.org
index b008cc1..6078631 100644
--- a/testing-strategy.org
+++ b/testing-strategy.org
@@ -61,7 +61,7 @@ Current coverage lives in =tests/unit/=:
| File | What it covers |
|------+----------------|
-| =test_common.bats= | =command_exists=, =require_command=, =info=/=warn=/=error=, =enable_color=, =log=, =prompt_password=, =pacstrap_packages=, =install_dropin= |
+| =test_common.bats= | =command_exists=, =require_command=, =info=/=warn=/=error=, =enable_color=, =log=, =prompt_password=, =pacstrap_packages=, =install_dropin=, =parse_efibootmgr_*=, =EFI_DIR=, =enable_sshd_root_login=, =prepend_grub_cmdline_linux= |
| =test_config.bats= | =parse_args=, =load_config=, =validate_config=, =validate_filesystem=, =check_config=, default values pinned in config.sh |
| =test_raid.bats= | =raid_valid_levels_for_count=, =raid_is_valid=, =raid_usable_bytes=, =raid_fault_tolerance= |
| =test_disk.bats= | =get_efi_partition=, =get_root_partition=, =partition_disks= (orchestration shape) |
diff --git a/tests/unit/test_common.bats b/tests/unit/test_common.bats
index 9d267ab..48efd15 100644
--- a/tests/unit/test_common.bats
+++ b/tests/unit/test_common.bats
@@ -405,3 +405,67 @@ Boot0001* ZFSBootMenu"
! grep -q 'PermitRootLogin' "$f"
rm -f "$f"
}
+
+#############################
+# prepend_grub_cmdline_linux
+#############################
+# prepend_grub_cmdline_linux prepends a string to GRUB_CMDLINE_LINUX
+# in /etc/default/grub. Errors loudly if the line isn't present, since
+# silently doing nothing would leave the kernel without the parameter
+# (e.g. cryptdevice= for LUKS — a missing prefix means the system
+# can't unlock the root partition at boot).
+
+@test "prepend_grub_cmdline_linux prepends to an empty GRUB_CMDLINE_LINUX" {
+ local f
+ f=$(mktemp)
+ printf '%s\n' 'GRUB_CMDLINE_LINUX=""' > "$f"
+
+ prepend_grub_cmdline_linux "cryptdevice=UUID=abc123:root " "$f"
+
+ grep -qF 'GRUB_CMDLINE_LINUX="cryptdevice=UUID=abc123:root "' "$f"
+ rm -f "$f"
+}
+
+@test "prepend_grub_cmdline_linux preserves text already inside GRUB_CMDLINE_LINUX" {
+ local f
+ f=$(mktemp)
+ printf '%s\n' 'GRUB_CMDLINE_LINUX="quiet splash"' > "$f"
+
+ prepend_grub_cmdline_linux "cryptdevice=UUID=abc:root " "$f"
+
+ grep -qF 'GRUB_CMDLINE_LINUX="cryptdevice=UUID=abc:root quiet splash"' "$f"
+ rm -f "$f"
+}
+
+@test "prepend_grub_cmdline_linux preserves other lines in /etc/default/grub" {
+ local f
+ f=$(mktemp)
+ printf '%s\n' \
+ 'GRUB_DEFAULT=0' \
+ 'GRUB_TIMEOUT=5' \
+ 'GRUB_CMDLINE_LINUX=""' \
+ 'GRUB_DISABLE_RECOVERY=true' > "$f"
+
+ prepend_grub_cmdline_linux "cryptdevice=UUID=abc:root " "$f"
+
+ grep -qF 'GRUB_DEFAULT=0' "$f"
+ grep -qF 'GRUB_TIMEOUT=5' "$f"
+ grep -qF 'GRUB_CMDLINE_LINUX="cryptdevice=UUID=abc:root "' "$f"
+ grep -qF 'GRUB_DISABLE_RECOVERY=true' "$f"
+ rm -f "$f"
+}
+
+@test "prepend_grub_cmdline_linux errors when GRUB_CMDLINE_LINUX line is absent" {
+ local f
+ f=$(mktemp)
+ printf '%s\n' \
+ 'GRUB_DEFAULT=0' \
+ 'GRUB_TIMEOUT=5' > "$f"
+
+ error() { echo "ERROR: $*" >&2; return 1; }
+ run prepend_grub_cmdline_linux "cryptdevice=UUID=abc:root " "$f"
+ [ "$status" -ne 0 ]
+ [[ "$output" == *"GRUB_CMDLINE_LINUX"* ]]
+ ! grep -q 'cryptdevice' "$f"
+ rm -f "$f"
+}