diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index a1e5b2b71..a143cd393 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -1803,6 +1803,46 @@ pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> { Ok(()) } +/// Require that a directory contains only mount points recursively. +/// Returns Ok(()) if all entries in the directory tree are either: +/// - Mount points (on different filesystems) +/// - Directories that themselves contain only mount points (recursively) +/// - The lost+found directory +/// +/// Returns an error if any non-mount entry is found. +/// +/// This handles cases like /var containing /var/lib (not a mount) which contains +/// /var/lib/containers (a mount point). +#[context("Requiring directory contains only mount points")] +fn require_dir_contains_only_mounts(parent_fd: &Dir, dir_name: &str) -> Result<()> { + let Some(dir_fd) = parent_fd.open_dir_noxdev(dir_name)? else { + // The directory itself is a mount point + return Ok(()); + }; + + if dir_fd.entries()?.next().is_none() { + anyhow::bail!("Found empty directory: {dir_name}"); + } + + for entry in dir_fd.entries()? { + let entry = DirEntryUtf8::from_cap_std(entry?); + let entry_name = entry.file_name()?; + + if entry_name == LOST_AND_FOUND { + continue; + } + + let etype = entry.file_type()?; + if etype == FileType::dir() && dir_fd.open_dir_noxdev(&entry_name)?.is_some() { + require_dir_contains_only_mounts(&dir_fd, &entry_name)?; + } else { + anyhow::bail!("Found entry in {dir_name}: {entry_name}"); + } + } + + Ok(()) +} + #[context("Verifying empty rootfs")] fn require_empty_rootdir(rootfs_fd: &Dir) -> Result<()> { for e in rootfs_fd.entries()? { @@ -1811,17 +1851,11 @@ fn require_empty_rootdir(rootfs_fd: &Dir) -> Result<()> { if name == LOST_AND_FOUND { continue; } - // There must be a boot directory (that is empty) - if name == BOOT { - let mut entries = rootfs_fd.read_dir(BOOT)?; - if let Some(e) = entries.next() { - let e = DirEntryUtf8::from_cap_std(e?); - let name = e.file_name()?; - if matches!(name.as_str(), LOST_AND_FOUND | crate::bootloader::EFI_DIR) { - continue; - } - anyhow::bail!("Non-empty boot directory, found {name}"); - } + + // Check if this entry is a directory + let etype = e.file_type()?; + if etype == FileType::dir() { + require_dir_contains_only_mounts(rootfs_fd, &name)?; } else { anyhow::bail!("Non-empty root filesystem; found {name:?}"); } @@ -2573,4 +2607,79 @@ UUID=boot-uuid /boot ext4 defaults 0 0 Ok(()) } + + #[test] + fn test_require_dir_contains_only_mounts() -> Result<()> { + // Test 1: Empty directory should fail (not a mount point) + { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + td.create_dir("empty")?; + assert!(require_dir_contains_only_mounts(&td, "empty").is_err()); + } + + // Test 2: Directory with only lost+found should succeed (lost+found is ignored) + { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + td.create_dir_all("var/lost+found")?; + assert!(require_dir_contains_only_mounts(&td, "var").is_ok()); + } + + // Test 3: Directory with a regular file should fail + { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + td.create_dir("var")?; + td.write("var/test.txt", b"content")?; + assert!(require_dir_contains_only_mounts(&td, "var").is_err()); + } + + // Test 4: Nested directory structure with a file should fail + { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + td.create_dir_all("var/lib/containers")?; + td.write("var/lib/containers/storage.db", b"data")?; + assert!(require_dir_contains_only_mounts(&td, "var").is_err()); + } + + // Test 5: boot directory with grub should fail (grub2 is not a mount and contains files) + { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + td.create_dir_all("boot/grub2")?; + td.write("boot/grub2/grub.cfg", b"config")?; + assert!(require_dir_contains_only_mounts(&td, "boot").is_err()); + } + + // Test 6: Nested empty directories should fail (empty directories are not mount points) + { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + td.create_dir_all("var/lib/containers")?; + td.create_dir_all("var/log/journal")?; + assert!(require_dir_contains_only_mounts(&td, "var").is_err()); + } + + // Test 7: Directory with lost+found and a file should fail (lost+found is ignored, but file is not allowed) + { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + td.create_dir_all("var/lost+found")?; + td.write("var/data.txt", b"content")?; + assert!(require_dir_contains_only_mounts(&td, "var").is_err()); + } + + // Test 8: Directory with a symlink should fail + { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + td.create_dir("var")?; + td.symlink_contents("../usr/lib", "var/lib")?; + assert!(require_dir_contains_only_mounts(&td, "var").is_err()); + } + + // Test 9: Deeply nested directory with a file should fail + { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + td.create_dir_all("var/lib/containers/storage/overlay")?; + td.write("var/lib/containers/storage/overlay/file.txt", b"data")?; + assert!(require_dir_contains_only_mounts(&td, "var").is_err()); + } + + Ok(()) + } } diff --git a/crates/tests-integration/src/install.rs b/crates/tests-integration/src/install.rs index aeef12c8d..8487c0354 100644 --- a/crates/tests-integration/src/install.rs +++ b/crates/tests-integration/src/install.rs @@ -93,6 +93,143 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments) cmd!(sh, "sudo {BASE_ARGS...} -v {tmpdisk}:/disk {image} bootc install to-disk --via-loopback /disk").run()?; Ok(()) }), + Trial::test( + "install to-filesystem with separate /var mount", + move || { + let sh = &xshell::Shell::new()?; + reset_root(sh, image)?; + + // Create work directory for the test + let tmpd = sh.create_temp_dir()?; + let work_dir = tmpd.path(); + + // Create a disk image with partitions for root and var + let disk_img = work_dir.join("disk.img"); + let size = 12 * 1024 * 1024 * 1024; + let disk_file = std::fs::File::create(&disk_img)?; + disk_file.set_len(size)?; + drop(disk_file); + + // Setup loop device + let loop_dev = cmd!(sh, "sudo losetup -f --show {disk_img}") + .read()? + .trim() + .to_string(); + + // Helper closure for cleanup + let cleanup = |sh: &Shell, loop_dev: &str, target: &str| { + // Unmount filesystems + let _ = cmd!(sh, "sudo umount -R {target}").ignore_status().run(); + // Deactivate LVM + let _ = cmd!(sh, "sudo vgchange -an BL").ignore_status().run(); + let _ = cmd!(sh, "sudo vgremove -f BL").ignore_status().run(); + // Detach loop device + let _ = cmd!(sh, "sudo losetup -d {loop_dev}").ignore_status().run(); + }; + + // Create partition table + if let Err(e) = (|| -> Result<()> { + cmd!(sh, "sudo parted -s {loop_dev} mklabel gpt").run()?; + // Create BIOS boot partition (for GRUB on GPT) + cmd!(sh, "sudo parted -s {loop_dev} mkpart primary 1MiB 2MiB").run()?; + cmd!(sh, "sudo parted -s {loop_dev} set 1 bios_grub on").run()?; + // Create EFI partition + cmd!( + sh, + "sudo parted -s {loop_dev} mkpart primary fat32 2MiB 202MiB" + ) + .run()?; + cmd!(sh, "sudo parted -s {loop_dev} set 2 esp on").run()?; + // Create boot partition + cmd!( + sh, + "sudo parted -s {loop_dev} mkpart primary ext4 202MiB 1226MiB" + ) + .run()?; + // Create LVM partition + cmd!(sh, "sudo parted -s {loop_dev} mkpart primary 1226MiB 100%").run()?; + + // Reload partition table + cmd!(sh, "sudo partprobe {loop_dev}").run()?; + std::thread::sleep(std::time::Duration::from_secs(2)); + + let loop_part2 = format!("{}p2", loop_dev); // EFI + let loop_part3 = format!("{}p3", loop_dev); // Boot + let loop_part4 = format!("{}p4", loop_dev); // LVM + + // Create filesystems on boot partitions + cmd!(sh, "sudo mkfs.vfat -F32 {loop_part2}").run()?; + cmd!(sh, "sudo mkfs.ext4 -F {loop_part3}").run()?; + + // Setup LVM + cmd!(sh, "sudo pvcreate {loop_part4}").run()?; + cmd!(sh, "sudo vgcreate BL {loop_part4}").run()?; + + // Create logical volumes + cmd!(sh, "sudo lvcreate -L 4G -n var02 BL").run()?; + cmd!(sh, "sudo lvcreate -L 5G -n root02 BL").run()?; + + // Create filesystems on logical volumes + cmd!(sh, "sudo mkfs.ext4 -F /dev/BL/var02").run()?; + cmd!(sh, "sudo mkfs.ext4 -F /dev/BL/root02").run()?; + + // Get UUIDs + let root_uuid = cmd!(sh, "sudo blkid -s UUID -o value /dev/BL/root02") + .read()? + .trim() + .to_string(); + let boot_uuid = cmd!(sh, "sudo blkid -s UUID -o value {loop_part2}") + .read()? + .trim() + .to_string(); + + // Mount the partitions + let target_dir = work_dir.join("target"); + std::fs::create_dir_all(&target_dir)?; + let target = target_dir.to_str().unwrap(); + + cmd!(sh, "sudo mount /dev/BL/root02 {target}").run()?; + cmd!(sh, "sudo mkdir -p {target}/boot").run()?; + cmd!(sh, "sudo mount {loop_part3} {target}/boot").run()?; + cmd!(sh, "sudo mkdir -p {target}/boot/efi").run()?; + cmd!(sh, "sudo mount {loop_part2} {target}/boot/efi").run()?; + + // Critical: Mount /var as a separate partition + cmd!(sh, "sudo mkdir -p {target}/var").run()?; + cmd!(sh, "sudo mount /dev/BL/var02 {target}/var").run()?; + + // Run bootc install to-filesystem + // This should succeed and handle the separate /var mount correctly + // Mount the target at /target inside the container for simplicity + cmd!( + sh, + "sudo {BASE_ARGS...} -v {target}:/target -v /dev:/dev {image} bootc install to-filesystem --karg=root=UUID={root_uuid} --root-mount-spec=UUID={root_uuid} --boot-mount-spec=UUID={boot_uuid} /target" + ) + .run()?; + + // Verify the installation succeeded + // Check that bootc created the necessary files + cmd!(sh, "sudo test -d {target}/ostree").run()?; + cmd!(sh, "sudo test -d {target}/ostree/repo").run()?; + // Verify bootloader was installed + cmd!(sh, "sudo test -d {target}/boot/grub2").run()?; + + Ok(()) + })() { + let target = work_dir.join("target"); + let target_str = target.to_str().unwrap(); + cleanup(sh, &loop_dev, target_str); + return Err(e.into()); + } + + // Clean up on success + let target = work_dir.join("target"); + let target_str = target.to_str().unwrap(); + cleanup(sh, &loop_dev, target_str); + + Ok(()) + }, + ), Trial::test( "replace=alongside with ssh keys and a karg, and SELinux disabled", move || {