diff options
| author | Craig Jennings <c@cjennings.net> | 2026-04-26 01:08:37 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-04-26 01:08:37 -0500 |
| commit | bdda6b17c83e0e298c76bbaf146f082218389cd8 (patch) | |
| tree | 8bfa18cdd39e6b1969f6bc8a13d64a1e5482fd52 /installer/lib | |
| parent | 5d677360721ec3ea0f3624ca2a2f20e7241bfba8 (diff) | |
| download | archangel-bdda6b17c83e0e298c76bbaf146f082218389cd8.tar.gz archangel-bdda6b17c83e0e298c76bbaf146f082218389cd8.zip | |
fix: fail fast on missing ZFSBootMenu efibootmgr entry
The post-bootloader boot-order step in `configure_zfsbootmenu` parsed `efibootmgr` output through a `grep | head | grep -oP` chain with no null guards. If any link returned empty (the entry wasn't created, the label was different, or efibootmgr itself failed), the surrounding `if [[ -n "$bootnum" ]]` silently skipped, the install reported success, and the user rebooted into a machine that wouldn't boot ZFSBootMenu by default.
I replaced the chain with two pure helpers in `lib/common.sh`, `parse_efibootmgr_entry` and `parse_efibootmgr_bootorder`. The caller in `archangel` invokes them with explicit `|| error` guards on each parse stage. The helpers capture `efibootmgr` output once and reuse it (it was called twice before). The same hardening covers the BootOrder lookup at the adjacent line. It used to rely on the now-removed `bootnum` guard for safety.
The helpers are stdin-driven and use bash regex, so they're easy to test in bats without exercising the real efibootmgr binary. Added 9 unit tests across normal cases, hex-character boot numbers, multi-match selection, missing label, missing BootOrder line, empty input, and an empty label argument. The empty-label case would otherwise falsely match `BootCurrent` via the hex regex, capturing "C". The helper now guards it explicitly.
Verified manually against real efibootmgr output (GRUB entry at Boot0001, BootOrder 0006,0001,2001,2002,2003). Both helpers parsed correctly. VM integration not re-run for this small post-bootloader change. The next scheduled `make test-install` exercises the green path.
Diffstat (limited to 'installer/lib')
| -rw-r--r-- | installer/lib/common.sh | 24 |
1 files changed, 24 insertions, 0 deletions
diff --git a/installer/lib/common.sh b/installer/lib/common.sh index 8193b19..98220fa 100644 --- a/installer/lib/common.sh +++ b/installer/lib/common.sh @@ -236,6 +236,30 @@ install_dropin() { cat > "${dir}/${dropin_name}.conf" } +# Read efibootmgr output from stdin and echo the boot number of the +# first entry whose label contains $1. Returns 1 (with empty output) +# if the label is empty, no entry matches, or the matched line has no +# Boot[0-9A-F]+ prefix. The empty-label guard is important: an empty +# string would match every line, and a line like "BootCurrent: 0001" +# would falsely satisfy the Boot[hex]+ regex (capturing "C"). +parse_efibootmgr_entry() { + local label="$1" + [[ -z "$label" ]] && return 1 + local line + line=$(grep -F -m 1 "$label") || return 1 + [[ "$line" =~ Boot([0-9A-Fa-f]+) ]] || return 1 + echo "${BASH_REMATCH[1]}" +} + +# Read efibootmgr output from stdin and echo the comma-separated boot +# numbers from the BootOrder line, with whitespace stripped. Returns 1 +# (with empty output) if no BootOrder line is present. +parse_efibootmgr_bootorder() { + local line + line=$(grep "^BootOrder:") || return 1 + echo "${line#BootOrder:}" | tr -d ' ' +} + # List available disks (not in use) list_available_disks() { local disks=() |
