From 826f1cdc4280b8ee77a3e9720fb9a025dbea88ad Mon Sep 17 00:00:00 2001 From: lrnselfreliance <33263708+lrnselfreliance@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:26:18 -0600 Subject: [PATCH] The drive wizard never offers an EFI system partition (#833) On a Debian install the onboarding wizard offered the EFI system partition (unlabeled vfat at /boot/efi) as the primary WROLPi drive; choosing it would mount the boot partition over /media/wrolpi. Candidates were filtered by the "EFI" label only, and the Debian installer leaves the ESP unlabeled. /boot/efi is now an excluded mountpoint, and partitions with the EFI system partition type (GPT GUID or MBR 0xef, from lsblk PARTTYPE) are excluded even when unmounted, e.g. on a second internal disk. Co-authored-by: Claude Opus 5.5 --- controller/lib/disks.py | 4 ++- controller/lib/onboarding.py | 8 +++++- controller/test/test_disks.py | 16 ++++++++++++ controller/test/test_onboarding.py | 42 ++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/controller/lib/disks.py b/controller/lib/disks.py index 83ac0338..5059ca3b 100644 --- a/controller/lib/disks.py +++ b/controller/lib/disks.py @@ -48,6 +48,7 @@ class BlockDevice: uuid: Optional[str] model: Optional[str] is_wrolpi_drive: bool = False + parttype: Optional[str] = None # GPT type GUID, or MBR type like "0xef". def get_block_devices() -> list[BlockDevice]: @@ -60,7 +61,7 @@ def get_block_devices() -> list[BlockDevice]: if is_docker_mode(): return [] - cmd = ["lsblk", "-J", "-o", "NAME,PATH,SIZE,TYPE,FSTYPE,MOUNTPOINT,LABEL,UUID,MODEL"] + cmd = ["lsblk", "-J", "-o", "NAME,PATH,SIZE,TYPE,FSTYPE,MOUNTPOINT,LABEL,UUID,MODEL,PARTTYPE"] try: result = subprocess.run(cmd, capture_output=True, text=True, timeout=10) @@ -83,6 +84,7 @@ def get_block_devices() -> list[BlockDevice]: label=dev.get("label"), uuid=dev.get("uuid"), model=dev.get("model") or parent_model, + parttype=dev.get("parttype"), )) # Process children (partitions) for child in dev.get("children", []): diff --git a/controller/lib/onboarding.py b/controller/lib/onboarding.py index 5b4a3657..977d9a02 100644 --- a/controller/lib/onboarding.py +++ b/controller/lib/onboarding.py @@ -36,11 +36,15 @@ logger = logging.getLogger(__name__) TEMP_MOUNT_PATH = "/media/wrolpi_temp_onboarding" # System mountpoints that should never be candidates -EXCLUDED_MOUNTPOINTS = {"/", "/boot", "/boot/firmware"} +EXCLUDED_MOUNTPOINTS = {"/", "/boot", "/boot/efi", "/boot/firmware"} # Partition labels that should never be candidates (case-insensitive) EXCLUDED_LABELS = {"efi", "efi system partition"} +# EFI system partition types (GPT GUID, MBR id); these partitions are often +# unlabeled, so the label check alone misses them. +EFI_PARTTYPES = {"c12a7328-f81f-11d2-ba4b-00a0c93ec93b", "0xef"} + def get_onboarding_candidates() -> list[dict]: """ @@ -65,6 +69,8 @@ def get_onboarding_candidates() -> list[dict]: # Skip EFI system partitions if d.label and d.label.lower() in EXCLUDED_LABELS: continue + if d.parttype and d.parttype.lower() in EFI_PARTTYPES: + continue candidates.append({ "path": d.path, "name": d.name, diff --git a/controller/test/test_disks.py b/controller/test/test_disks.py index 2b24ae5e..67918477 100644 --- a/controller/test/test_disks.py +++ b/controller/test/test_disks.py @@ -98,6 +98,22 @@ class TestGetBlockDevices: assert result[0].fstype == "ext4" assert result[0].model == "Samsung SSD" + def test_reads_partition_type(self): + """The partition type identifies an EFI system partition whatever its label or mount.""" + lsblk_output = json.dumps({"blockdevices": [{ + "name": "sda", "path": "/dev/sda", "size": "500G", "type": "disk", "children": [{ + "name": "sda1", "path": "/dev/sda1", "size": "512M", "type": "part", "fstype": "vfat", + "parttype": "c12a7328-f81f-11d2-ba4b-00a0c93ec93b", + }], + }]}) + with mock.patch("controller.lib.disks.is_docker_mode", return_value=False): + with mock.patch("subprocess.run") as mock_run: + mock_run.return_value = mock.Mock(returncode=0, stdout=lsblk_output) + result = get_block_devices() + + assert "PARTTYPE" in mock_run.call_args.args[0][-1].split(",") + assert result[0].parttype == "c12a7328-f81f-11d2-ba4b-00a0c93ec93b" + def test_filters_unsupported_filesystems(self): """Should only return devices with supported filesystems.""" lsblk_output = json.dumps({ diff --git a/controller/test/test_onboarding.py b/controller/test/test_onboarding.py index 93136ad1..b16fc101 100644 --- a/controller/test/test_onboarding.py +++ b/controller/test/test_onboarding.py @@ -151,6 +151,48 @@ class TestGetOnboardingCandidates: assert len(candidates) == 0 + def test_excludes_unlabeled_efi_at_boot_efi(self, mock_docker_mode): + """The Debian installer's EFI partition has no label and is mounted at /boot/efi; offering it as the + library drive would mount the boot partition over /media/wrolpi.""" + from controller.lib.disks import BlockDevice + + devices = [ + BlockDevice( + name="nvme0n1p1", path="/dev/nvme0n1p1", size="976M", fstype="vfat", + mountpoint="/boot/efi", label=None, uuid="D019-F19C", model="QEMU NVMe Ctrl", + ), + ] + with mock.patch("controller.lib.onboarding.get_block_devices", return_value=devices): + candidates = get_onboarding_candidates() + + assert candidates == [] + + @pytest.mark.parametrize("parttype", [ + "c12a7328-f81f-11d2-ba4b-00a0c93ec93b", # GPT EFI System Partition. + "C12A7328-F81F-11D2-BA4B-00A0C93EC93B", + "0xef", # MBR EFI System Partition. + ]) + def test_excludes_unmounted_efi_by_partition_type(self, mock_docker_mode, parttype): + """An unmounted, unlabeled EFI partition (e.g. on a second internal disk) is excluded by its type, + while an ordinary vfat USB drive is still offered.""" + from controller.lib.disks import BlockDevice + + devices = [ + BlockDevice( + name="sdb1", path="/dev/sdb1", size="100M", fstype="vfat", + mountpoint=None, label=None, uuid="AAAA-BBBB", model="Old Disk", parttype=parttype, + ), + BlockDevice( + name="sdc1", path="/dev/sdc1", size="64G", fstype="vfat", + mountpoint=None, label="USB", uuid="CCCC-DDDD", model="Flash", + parttype="ebd0a0a2-b9e5-4433-87c0-68b6b72699c7", # GPT Microsoft basic data. + ), + ] + with mock.patch("controller.lib.onboarding.get_block_devices", return_value=devices): + candidates = get_onboarding_candidates() + + assert [c["path"] for c in candidates] == ["/dev/sdc1"] + def test_excludes_temp_onboarding_mount(self, mock_docker_mode): """Drives mounted at the temp onboarding path should be excluded.""" from controller.lib.disks import BlockDevice