From 2db3be91a2583c854bcaaaccb3037eddd9ab1f95 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 7 Apr 2026 12:12:02 +0530 Subject: [PATCH 1/4] composefs/boot: Get os_id from mounted EROFS Instead of reading the in memory filesystem to get /usr/lib/os-release get it from the mounted EROFS. This is also prep for providing backwards compatibility due to our newly introduced prefix `bootc_composefs-` where we'll need to create new boot entries and we can get the `os_id` from the mounted root Signed-off-by: Pragyan Poudyal --- crates/lib/src/bootc_composefs/boot.rs | 45 ++++++++------------------ 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 4e945c065..619a5b262 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -61,8 +61,8 @@ //! 1. **Primary**: New/upgraded deployment (default boot target) //! 2. **Secondary**: Currently booted deployment (rollback option) -use std::ffi::OsStr; use std::fs::create_dir_all; +use std::io::Read; use std::io::Write; use std::path::Path; @@ -451,41 +451,22 @@ fn write_bls_boot_entries_to_disk( } /// Parses /usr/lib/os-release and returns (id, title, version) -fn parse_os_release( - fs: &crate::store::ComposefsFilesystem, - repo: &crate::store::ComposefsRepository, -) -> Result, Option)>> { +/// Expects a referenece to the root of the filesystem, or the root +/// of a mounted EROFS +pub fn parse_os_release(root: &Dir) -> Result, Option)>> { // Every update should have its own /usr/lib/os-release - let (dir, fname) = fs - .root - .split(OsStr::new("/usr/lib/os-release")) - .context("Getting /usr/lib/os-release")?; - - let os_release = dir - .get_file_opt(fname) - .context("Getting /usr/lib/os-release")?; + let file = root + .open_optional("usr/lib/os-release") + .context("Opening usr/lib/os-release")?; - let Some(os_rel_file) = os_release else { + let Some(mut os_rel_file) = file else { return Ok(None); }; - let file_contents = match read_file(os_rel_file, repo) { - Ok(c) => c, - Err(e) => { - tracing::warn!("Could not read /usr/lib/os-release: {e:?}"); - return Ok(None); - } - }; - - let file_contents = match std::str::from_utf8(&file_contents) { - Ok(c) => c, - Err(e) => { - tracing::warn!("/usr/lib/os-release did not have valid UTF-8: {e}"); - return Ok(None); - } - }; + let mut file_contents = String::new(); + os_rel_file.read_to_string(&mut file_contents)?; - let parsed = OsReleaseInfo::parse(file_contents); + let parsed = OsReleaseInfo::parse(&file_contents); let os_id = parsed .get_value(&["ID"]) @@ -521,7 +502,7 @@ pub(crate) fn setup_composefs_bls_boot( ) -> Result { let id_hex = id.to_hex(); - let (root_path, esp_device, mut cmdline_refs, fs, bootloader) = match setup_type { + let (root_path, esp_device, mut cmdline_refs, _, bootloader) = match setup_type { BootSetupType::Setup((root_setup, state, postfetch, fs)) => { // root_setup.kargs has [root=UUID=, "rw"] let mut cmdline_options = Cmdline::new(); @@ -668,7 +649,7 @@ pub(crate) fn setup_composefs_bls_boot( let boot_digest = compute_boot_digest(usr_lib_modules_vmlinuz, &repo) .context("Computing boot digest")?; - let osrel = parse_os_release(fs, &repo)?; + let osrel = parse_os_release(mounted_erofs)?; let (os_id, title, version, sort_key) = match osrel { Some((id_str, title_opt, version_opt)) => ( From d7bcdc67ec9a25008b0b4a2bfef9ec41e535e7e1 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 7 Apr 2026 14:07:24 +0530 Subject: [PATCH 2/4] composefs: Handle backwads compatibility with older versions While finishing up GC, we had come up with the idea of prepending our boot binaries (UKI PEs, BLS directories) with a certain prefix and we ended up hard requiring these prefixes. If someone has an older version of bootc which they used to install their system with, then upgrade to a new version, many if not all of the important operations would cease to work. This basically handles the backwards compatibility of new binaries on older systems by prepending our custom prefix to all existing boot binaries Signed-off-by: Pragyan Poudyal --- .../backwards_compat/bcompat_boot.rs | 393 ++++++++++++++++++ .../bootc_composefs/backwards_compat/mod.rs | 1 + crates/lib/src/bootc_composefs/mod.rs | 1 + crates/lib/src/bootc_composefs/rollback.rs | 10 +- crates/lib/src/parsers/bls_config.rs | 23 +- crates/lib/src/parsers/grub_menuconfig.rs | 12 +- 6 files changed, 425 insertions(+), 15 deletions(-) create mode 100644 crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs create mode 100644 crates/lib/src/bootc_composefs/backwards_compat/mod.rs diff --git a/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs b/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs new file mode 100644 index 000000000..dd6e569c1 --- /dev/null +++ b/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs @@ -0,0 +1,393 @@ +use std::io::{Read, Write}; + +use crate::{ + bootc_composefs::{ + boot::{ + BOOTC_UKI_DIR, BootType, FILENAME_PRIORITY_PRIMARY, FILENAME_PRIORITY_SECONDARY, + get_efi_uuid_source, get_uki_name, parse_os_release, type1_entry_conf_file_name, + }, + rollback::{rename_exchange_bls_entries, rename_exchange_user_cfg}, + status::{get_bootloader, get_sorted_grub_uki_boot_entries, get_sorted_type1_boot_entries}, + }, + composefs_consts::{ + ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_TYPE, STATE_DIR_RELATIVE, TYPE1_BOOT_DIR_PREFIX, + TYPE1_ENT_PATH_STAGED, UKI_NAME_PREFIX, USER_CFG_STAGED, + }, + parsers::bls_config::{BLSConfig, BLSConfigType}, + spec::Bootloader, + store::{BootedComposefs, Storage}, +}; +use anyhow::{Context, Result}; +use camino::Utf8PathBuf; +use cap_std_ext::{cap_std::fs::Dir, dirext::CapStdExtDirExt}; +use cfsctl::composefs_boot::bootloader::{EFI_ADDON_DIR_EXT, EFI_EXT}; +use fn_error_context::context; +use ocidir::cap_std::ambient_authority; +use rustix::fs::{RenameFlags, fsync, renameat_with}; + +/// Represents a pending rename operation to be executed atomically +#[derive(Debug)] +struct PendingRename { + old_name: String, + new_name: String, +} + +/// Transaction context for managing atomic renames (both files and directories) +#[derive(Debug)] +struct RenameTransaction { + operations: Vec, +} + +impl RenameTransaction { + fn new() -> Self { + Self { + operations: Vec::new(), + } + } + + fn add_operation(&mut self, old_name: String, new_name: String) { + self.operations.push(PendingRename { old_name, new_name }); + } + + /// Execute all renames atomically in the provided directory + /// If any operation fails, attempt to rollback all completed operations + /// + /// We currently only have two entries at max, so this is quite unlikely to fail... + #[context("Executing rename transactions")] + fn execute_transaction(&self, target_dir: &Dir) -> Result<()> { + let mut completed_operations = Vec::new(); + + for op in &self.operations { + match renameat_with( + target_dir, + &op.old_name, + target_dir, + &op.new_name, + RenameFlags::empty(), + ) { + Ok(()) => { + completed_operations.push(op); + tracing::debug!("Renamed {} -> {}", op.old_name, op.new_name); + } + Err(e) => { + // Attempt rollback of completed operations + for completed_op in completed_operations.iter().rev() { + if let Err(rollback_err) = renameat_with( + target_dir, + &completed_op.new_name, + target_dir, + &completed_op.old_name, + RenameFlags::empty(), + ) { + tracing::error!( + "Rollback failed for {} -> {}: {}", + completed_op.new_name, + completed_op.old_name, + rollback_err + ); + } + } + + return Err(e).context(format!("Failed to rename {}", op.old_name)); + } + } + } + + Ok(()) + } +} + +/// Plan EFI binary renames and populate the transaction +/// The actual renames are deferred to the transaction +#[context("Planning EFI renames")] +fn plan_efi_binary_renames( + esp: &Dir, + digest: &str, + rename_transaction: &mut RenameTransaction, +) -> Result<()> { + let bootc_uki_dir = esp.open_dir(BOOTC_UKI_DIR)?; + + for entry in bootc_uki_dir.entries_utf8()? { + let entry = entry?; + let filename = entry.file_name()?; + + if filename.starts_with(UKI_NAME_PREFIX) { + continue; + } + + if !filename.ends_with(EFI_EXT) && !filename.ends_with(EFI_ADDON_DIR_EXT) { + continue; + } + + if !filename.contains(digest) { + continue; + } + + let new_name = format!("{UKI_NAME_PREFIX}{filename}"); + rename_transaction.add_operation(filename.to_string(), new_name); + } + + Ok(()) +} + +/// Plan BLS directory renames and populate the transaction +/// The actual renames are deferred to the transaction +#[context("Planning BLS directory renames")] +fn plan_bls_entry_rename(binaries_dir: &Dir, entry_to_fix: &str) -> Result> { + for entry in binaries_dir.entries_utf8()? { + let entry = entry?; + let filename = entry.file_name()?; + + // We don't really put any files here, but just in case + if !entry.file_type()?.is_dir() { + continue; + } + + if filename != entry_to_fix { + continue; + } + + let new_name = format!("{TYPE1_BOOT_DIR_PREFIX}{filename}"); + return Ok(Some(new_name)); + } + + Ok(None) +} + +#[context("Staging BLS entry changes")] +fn stage_bls_entry_changes( + storage: &Storage, + boot_dir: &Dir, + entries: &Vec, + booted_cfs: &BootedComposefs, +) -> Result<(RenameTransaction, Vec<(String, BLSConfig)>)> { + let mut rename_transaction = RenameTransaction::new(); + + let root = Dir::open_ambient_dir("/", ambient_authority())?; + let osrel = parse_os_release(&root)?; + + let os_id = osrel + .as_ref() + .map(|(s, _, _)| s.as_str()) + .unwrap_or("bootc"); + + // to not add duplicate transactions since we share BLS entries + // across deployements + let mut fixed = vec![]; + let mut new_bls_entries = vec![]; + + for entry in entries { + let (digest, has_prefix) = entry.boot_artifact_info()?; + let digest = digest.to_string(); + + if has_prefix { + continue; + } + + let mut new_entry = entry.clone(); + + let conf_filename = if *booted_cfs.cmdline.digest == digest { + type1_entry_conf_file_name(os_id, new_entry.version(), FILENAME_PRIORITY_PRIMARY) + } else { + type1_entry_conf_file_name(os_id, new_entry.version(), FILENAME_PRIORITY_SECONDARY) + }; + + match &mut new_entry.cfg_type { + BLSConfigType::NonEFI { linux, initrd, .. } => { + let new_name = + plan_bls_entry_rename(&storage.bls_boot_binaries_dir()?, &digest)? + .ok_or_else(|| anyhow::anyhow!("Directory for entry {digest} not found"))?; + + // We don't want this multiple times in the rename_transaction if it was already + // "fixed" + if !fixed.contains(&digest) { + rename_transaction.add_operation(digest.clone(), new_name.clone()); + } + + *linux = linux.as_str().replace(&digest, &new_name).into(); + *initrd = initrd + .iter_mut() + .map(|path| path.as_str().replace(&digest, &new_name).into()) + .collect(); + } + + BLSConfigType::EFI { efi, .. } => { + // boot_dir in case of UKI is the ESP + plan_efi_binary_renames(&boot_dir, &digest, &mut rename_transaction)?; + *efi = Utf8PathBuf::from("/") + .join(BOOTC_UKI_DIR) + .join(get_uki_name(&digest)); + } + + _ => anyhow::bail!("Unknown BLS config type"), + } + + new_bls_entries.push((conf_filename, new_entry)); + fixed.push(digest.into()); + } + + Ok((rename_transaction, new_bls_entries)) +} + +fn create_staged_bls_entries(boot_dir: &Dir, entries: &Vec<(String, BLSConfig)>) -> Result<()> { + boot_dir.create_dir_all(TYPE1_ENT_PATH_STAGED)?; + let staged_entries = boot_dir.open_dir(TYPE1_ENT_PATH_STAGED)?; + + for (filename, new_entry) in entries { + staged_entries.atomic_write(filename, new_entry.to_string().as_bytes())?; + } + + fsync(staged_entries.reopen_as_ownedfd()?).context("fsync") +} + +fn get_boot_type(storage: &Storage, booted_cfs: &BootedComposefs) -> Result { + let mut config = String::new(); + + let origin_path = Utf8PathBuf::from(STATE_DIR_RELATIVE) + .join(&*booted_cfs.cmdline.digest) + .join(format!("{}.origin", booted_cfs.cmdline.digest)); + + storage + .physical_root + .open(origin_path) + .context("Opening origin file")? + .read_to_string(&mut config) + .context("Reading origin file")?; + + let origin = tini::Ini::from_string(&config) + .with_context(|| format!("Failed to parse origin as ini"))?; + + let boot_type = match origin.get::(ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_TYPE) { + Some(s) => BootType::try_from(s.as_str())?, + None => anyhow::bail!("{ORIGIN_KEY_BOOT} not found"), + }; + + Ok(boot_type) +} + +fn handle_bls_conf( + storage: &Storage, + booted_cfs: &BootedComposefs, + boot_dir: &Dir, + is_uki: bool, +) -> Result<()> { + let entries = get_sorted_type1_boot_entries(boot_dir, true)?; + let (rename_transaction, new_bls_entries) = + stage_bls_entry_changes(storage, boot_dir, &entries, booted_cfs)?; + + if rename_transaction.operations.is_empty() { + tracing::debug!("Nothing to do"); + return Ok(()); + } + + create_staged_bls_entries(boot_dir, &new_bls_entries)?; + + let binaries_dir = if is_uki { + let esp = storage.require_esp()?; + let uki_dir = esp.fd.open_dir(BOOTC_UKI_DIR).context("Opening UKI dir")?; + + uki_dir + } else { + storage.bls_boot_binaries_dir()? + }; + + // execute all EFI PE renames atomically before the final exchange + rename_transaction + .execute_transaction(&binaries_dir) + .context("Failed to execute EFI binary rename transaction")?; + + fsync(binaries_dir.reopen_as_ownedfd()?)?; + + let loader_dir = boot_dir.open_dir("loader").context("Opening loader dir")?; + rename_exchange_bls_entries(&loader_dir)?; + + Ok(()) +} + +/// Goes through the ESP and prepends every UKI/Addon with our custom prefix +/// Goes through the BLS entries and prepends our custom prefix +#[context("Prepending custom prefix to EFI and BLS entries")] +pub(crate) async fn prepend_custom_prefix( + storage: &Storage, + booted_cfs: &BootedComposefs, +) -> Result<()> { + let boot_dir = storage.require_boot_dir()?; + + let bootloader = get_bootloader()?; + + match get_boot_type(storage, booted_cfs)? { + BootType::Bls => { + handle_bls_conf(storage, booted_cfs, boot_dir, false)?; + } + + BootType::Uki => match bootloader { + Bootloader::Grub => { + let esp = storage.require_esp()?; + + let mut buf = String::new(); + let menuentries = get_sorted_grub_uki_boot_entries(boot_dir, &mut buf)?; + + let mut new_menuentries = vec![]; + let mut rename_transaction = RenameTransaction::new(); + + for entry in menuentries { + let (digest, has_prefix) = entry.boot_artifact_info()?; + let digest = digest.to_string(); + + if has_prefix { + continue; + } + + plan_efi_binary_renames(&esp.fd, &digest, &mut rename_transaction)?; + + let new_path = Utf8PathBuf::from("/") + .join(BOOTC_UKI_DIR) + .join(get_uki_name(&digest)); + + let mut new_entry = entry.clone(); + new_entry.body.chainloader = new_path.into(); + + new_menuentries.push(new_entry); + } + + if rename_transaction.operations.is_empty() { + tracing::debug!("Nothing to do"); + return Ok(()); + } + + let grub_dir = boot_dir.open_dir("grub2").context("opening boot/grub2")?; + + grub_dir + .atomic_replace_with(USER_CFG_STAGED, |f| -> std::io::Result<_> { + f.write_all(get_efi_uuid_source().as_bytes())?; + + for entry in new_menuentries { + f.write_all(entry.to_string().as_bytes())?; + } + + Ok(()) + }) + .with_context(|| format!("Writing to {USER_CFG_STAGED}"))?; + + let esp = storage.require_esp()?; + let uki_dir = esp.fd.open_dir(BOOTC_UKI_DIR).context("Opening UKI dir")?; + + // execute all EFI PE renames atomically before the final exchange + rename_transaction + .execute_transaction(&uki_dir) + .context("Failed to execute EFI binary rename transaction")?; + + fsync(uki_dir.reopen_as_ownedfd()?)?; + rename_exchange_user_cfg(&grub_dir)?; + } + + Bootloader::Systemd => { + handle_bls_conf(storage, booted_cfs, boot_dir, true)?; + } + + Bootloader::None => unreachable!("Checked at install time"), + }, + }; + + Ok(()) +} diff --git a/crates/lib/src/bootc_composefs/backwards_compat/mod.rs b/crates/lib/src/bootc_composefs/backwards_compat/mod.rs new file mode 100644 index 000000000..38fa99683 --- /dev/null +++ b/crates/lib/src/bootc_composefs/backwards_compat/mod.rs @@ -0,0 +1 @@ +pub(crate) mod bcompat_boot; diff --git a/crates/lib/src/bootc_composefs/mod.rs b/crates/lib/src/bootc_composefs/mod.rs index 0660cdcd7..766083c58 100644 --- a/crates/lib/src/bootc_composefs/mod.rs +++ b/crates/lib/src/bootc_composefs/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod backwards_compat; pub(crate) mod boot; pub(crate) mod delete; pub(crate) mod digest; diff --git a/crates/lib/src/bootc_composefs/rollback.rs b/crates/lib/src/bootc_composefs/rollback.rs index 3da3b5dbc..106919f04 100644 --- a/crates/lib/src/bootc_composefs/rollback.rs +++ b/crates/lib/src/bootc_composefs/rollback.rs @@ -25,22 +25,22 @@ use crate::{ /// Atomically rename exchange grub user.cfg with the staged version /// Performed as the last step in rollback/update/switch operation #[context("Atomically exchanging user.cfg")] -pub(crate) fn rename_exchange_user_cfg(entries_dir: &Dir) -> Result<()> { +pub(crate) fn rename_exchange_user_cfg(grub2_dir: &Dir) -> Result<()> { tracing::debug!("Atomically exchanging {USER_CFG_STAGED} and {USER_CFG}"); renameat_with( - &entries_dir, + &grub2_dir, USER_CFG_STAGED, - &entries_dir, + &grub2_dir, USER_CFG, RenameFlags::EXCHANGE, ) .context("renameat")?; tracing::debug!("Removing {USER_CFG_STAGED}"); - rustix::fs::unlinkat(&entries_dir, USER_CFG_STAGED, AtFlags::empty()).context("unlinkat")?; + rustix::fs::unlinkat(&grub2_dir, USER_CFG_STAGED, AtFlags::empty()).context("unlinkat")?; tracing::debug!("Syncing to disk"); - let entries_dir = entries_dir + let entries_dir = grub2_dir .reopen_as_ownedfd() .context("Reopening entries dir as owned fd")?; diff --git a/crates/lib/src/parsers/bls_config.rs b/crates/lib/src/parsers/bls_config.rs index 0988b0d61..fb6f2207c 100644 --- a/crates/lib/src/parsers/bls_config.rs +++ b/crates/lib/src/parsers/bls_config.rs @@ -15,7 +15,7 @@ use uapi_version::Version; use crate::bootc_composefs::status::ComposefsCmdline; use crate::composefs_consts::{TYPE1_BOOT_DIR_PREFIX, UKI_NAME_PREFIX}; -#[derive(Debug, PartialEq, Eq, Default)] +#[derive(Debug, PartialEq, Eq, Default, Clone)] pub enum BLSConfigType { EFI { /// The path to the EFI binary, usually a UKI @@ -38,7 +38,7 @@ pub enum BLSConfigType { /// The boot loader should present the available boot menu entries to the user in a sorted list. /// The list should be sorted by the `sort-key` field, if it exists, otherwise by the `machine-id` field. /// If multiple entries have the same `sort-key` (or `machine-id`), they should be sorted by the `version` field in descending order. -#[derive(Debug, Eq, PartialEq, Default)] +#[derive(Debug, Eq, PartialEq, Default, Clone)] #[non_exhaustive] pub(crate) struct BLSConfig { /// The title of the boot entry, to be displayed in the boot menu. @@ -212,6 +212,17 @@ impl BLSConfig { /// The names are stripped of our custom prefix and suffixes, so this returns the /// verity digest part of the name pub(crate) fn boot_artifact_name(&self) -> Result<&str> { + Ok(self.boot_artifact_info()?.0) + } + + /// Returns name of UKI in case of EFI config + /// Returns name of the directory containing Kernel + Initrd in case of Non-EFI config + /// + /// The names are stripped of our custom prefix and suffixes, so this returns the + /// verity digest part of the name as the first value + /// + /// The second value is a boolean indicating whether it found our custom prefix or not + pub(crate) fn boot_artifact_info(&self) -> Result<(&str, bool)> { match &self.cfg_type { BLSConfigType::EFI { efi } => { let file_name = efi @@ -228,8 +239,8 @@ impl BLSConfig { // For backwards compatibility, we don't make this prefix mandatory match without_suffix.strip_prefix(UKI_NAME_PREFIX) { - Some(no_prefix) => Ok(no_prefix), - None => Ok(without_suffix), + Some(no_prefix) => Ok((no_prefix, true)), + None => Ok((without_suffix, false)), } } @@ -244,8 +255,8 @@ impl BLSConfig { // For backwards compatibility, we don't make this prefix mandatory match dir_name.strip_prefix(TYPE1_BOOT_DIR_PREFIX) { - Some(dir_name_no_prefix) => Ok(dir_name_no_prefix), - None => Ok(dir_name), + Some(dir_name_no_prefix) => Ok((dir_name_no_prefix, true)), + None => Ok((dir_name, false)), } } diff --git a/crates/lib/src/parsers/grub_menuconfig.rs b/crates/lib/src/parsers/grub_menuconfig.rs index 90d7e2ee2..cd7954f24 100644 --- a/crates/lib/src/parsers/grub_menuconfig.rs +++ b/crates/lib/src/parsers/grub_menuconfig.rs @@ -20,7 +20,7 @@ use crate::{ }; /// Body content of a GRUB menuentry containing parsed commands. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Clone)] pub(crate) struct MenuentryBody<'a> { /// Kernel modules to load pub(crate) insmod: Vec<&'a str>, @@ -76,7 +76,7 @@ impl<'a> From> for MenuentryBody<'a> { } /// A complete GRUB menuentry with title and body commands. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Clone)] pub(crate) struct MenuEntry<'a> { /// Display title (supports escaped quotes) pub(crate) title: String, @@ -128,6 +128,10 @@ impl<'a> MenuEntry<'a> { /// The names are stripped of our custom prefix and suffixes, so this returns /// the verity digest part of the name pub(crate) fn boot_artifact_name(&self) -> Result { + Ok(self.boot_artifact_info()?.0) + } + + pub(crate) fn boot_artifact_info(&self) -> Result<(String, bool)> { let chainloader_path = Utf8PathBuf::from(&self.body.chainloader); let file_name = chainloader_path.file_name().ok_or_else(|| { @@ -147,8 +151,8 @@ impl<'a> MenuEntry<'a> { // For backwards compatibility, we don't make this prefix mandatory match without_suffix.strip_prefix(UKI_NAME_PREFIX) { - Some(no_prefix) => Ok(no_prefix.into()), - None => Ok(without_suffix.into()), + Some(no_prefix) => Ok((no_prefix.into(), true)), + None => Ok((without_suffix.into(), false)), } } } From f6cd249bbabe607b0d907143445881abd844043f Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 7 Apr 2026 14:47:02 +0530 Subject: [PATCH 3/4] composefs: Check for meta.json Check if the repo has meta.json file and if not apply our fix of prepending custom prefix to our bootloader entries and boot binaries Signed-off-by: Pragyan Poudyal --- .../backwards_compat/bcompat_boot.rs | 29 ++++++++++--------- crates/lib/src/store/mod.rs | 13 ++++++++- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs b/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs index dd6e569c1..a27dbc7aa 100644 --- a/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs +++ b/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs @@ -7,7 +7,10 @@ use crate::{ get_efi_uuid_source, get_uki_name, parse_os_release, type1_entry_conf_file_name, }, rollback::{rename_exchange_bls_entries, rename_exchange_user_cfg}, - status::{get_bootloader, get_sorted_grub_uki_boot_entries, get_sorted_type1_boot_entries}, + status::{ + ComposefsCmdline, get_bootloader, get_sorted_grub_uki_boot_entries, + get_sorted_type1_boot_entries, + }, }, composefs_consts::{ ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_TYPE, STATE_DIR_RELATIVE, TYPE1_BOOT_DIR_PREFIX, @@ -15,7 +18,7 @@ use crate::{ }, parsers::bls_config::{BLSConfig, BLSConfigType}, spec::Bootloader, - store::{BootedComposefs, Storage}, + store::Storage, }; use anyhow::{Context, Result}; use camino::Utf8PathBuf; @@ -159,7 +162,7 @@ fn stage_bls_entry_changes( storage: &Storage, boot_dir: &Dir, entries: &Vec, - booted_cfs: &BootedComposefs, + cfs_cmdline: &ComposefsCmdline, ) -> Result<(RenameTransaction, Vec<(String, BLSConfig)>)> { let mut rename_transaction = RenameTransaction::new(); @@ -186,7 +189,7 @@ fn stage_bls_entry_changes( let mut new_entry = entry.clone(); - let conf_filename = if *booted_cfs.cmdline.digest == digest { + let conf_filename = if *cfs_cmdline.digest == digest { type1_entry_conf_file_name(os_id, new_entry.version(), FILENAME_PRIORITY_PRIMARY) } else { type1_entry_conf_file_name(os_id, new_entry.version(), FILENAME_PRIORITY_SECONDARY) @@ -240,12 +243,12 @@ fn create_staged_bls_entries(boot_dir: &Dir, entries: &Vec<(String, BLSConfig)>) fsync(staged_entries.reopen_as_ownedfd()?).context("fsync") } -fn get_boot_type(storage: &Storage, booted_cfs: &BootedComposefs) -> Result { +fn get_boot_type(storage: &Storage, cfs_cmdline: &ComposefsCmdline) -> Result { let mut config = String::new(); let origin_path = Utf8PathBuf::from(STATE_DIR_RELATIVE) - .join(&*booted_cfs.cmdline.digest) - .join(format!("{}.origin", booted_cfs.cmdline.digest)); + .join(&*cfs_cmdline.digest) + .join(format!("{}.origin", cfs_cmdline.digest)); storage .physical_root @@ -267,13 +270,13 @@ fn get_boot_type(storage: &Storage, booted_cfs: &BootedComposefs) -> Result Result<()> { let entries = get_sorted_type1_boot_entries(boot_dir, true)?; let (rename_transaction, new_bls_entries) = - stage_bls_entry_changes(storage, boot_dir, &entries, booted_cfs)?; + stage_bls_entry_changes(storage, boot_dir, &entries, cfs_cmdline)?; if rename_transaction.operations.is_empty() { tracing::debug!("Nothing to do"); @@ -309,15 +312,15 @@ fn handle_bls_conf( #[context("Prepending custom prefix to EFI and BLS entries")] pub(crate) async fn prepend_custom_prefix( storage: &Storage, - booted_cfs: &BootedComposefs, + cfs_cmdline: &ComposefsCmdline, ) -> Result<()> { let boot_dir = storage.require_boot_dir()?; let bootloader = get_bootloader()?; - match get_boot_type(storage, booted_cfs)? { + match get_boot_type(storage, cfs_cmdline)? { BootType::Bls => { - handle_bls_conf(storage, booted_cfs, boot_dir, false)?; + handle_bls_conf(storage, cfs_cmdline, boot_dir, false)?; } BootType::Uki => match bootloader { @@ -382,7 +385,7 @@ pub(crate) async fn prepend_custom_prefix( } Bootloader::Systemd => { - handle_bls_conf(storage, booted_cfs, boot_dir, true)?; + handle_bls_conf(storage, cfs_cmdline, boot_dir, true)?; } Bootloader::None => unreachable!("Checked at install time"), diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index 4f0cf4190..346837068 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -39,6 +39,7 @@ use rustix::fs::Mode; use cfsctl::composefs; use composefs::fsverity::Sha512HashValue; +use crate::bootc_composefs::backwards_compat::bcompat_boot::prepend_custom_prefix; use crate::bootc_composefs::boot::{EFI_LINUX, mount_esp}; use crate::bootc_composefs::status::{ComposefsCmdline, composefs_booted, get_bootloader}; use crate::lsm; @@ -210,6 +211,10 @@ impl BootedStorage { Bootloader::None => unreachable!("Checked at install time"), }; + let meta_json = physical_root + .open_dir(COMPOSEFS)? + .open_optional("meta.json")?; + let storage = Storage { physical_root, physical_root_path: Utf8PathBuf::from("/sysroot"), @@ -217,10 +222,16 @@ impl BootedStorage { boot_dir: Some(boot_dir), esp: Some(esp_mount), ostree: Default::default(), - composefs: OnceCell::from(composefs), + composefs: OnceCell::from(composefs.clone()), imgstore: Default::default(), }; + if meta_json.is_none() { + let cmdline = composefs_booted()? + .ok_or_else(|| anyhow::anyhow!("Could not get booted composefs cmdline"))?; + prepend_custom_prefix(&storage, &cmdline).await?; + } + Some(Self { storage }) } Environment::OstreeBooted => { From 256a16d90480b17187465e3c9fa90f84d7290ab1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 31 Mar 2026 21:32:38 +0000 Subject: [PATCH 4/4] composefs: Add harness and unit tests for shared boot entry GC Add some basic infra to mock up enough of an installed root to use in unit tests - specifically targeted for the bootloader logic. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters --- crates/lib/src/bootc_composefs/gc.rs | 529 ++++++++++++++- crates/lib/src/bootc_composefs/status.rs | 57 +- crates/lib/src/lib.rs | 3 + crates/lib/src/parsers/bls_config.rs | 85 +++ crates/lib/src/parsers/grub_menuconfig.rs | 106 ++- crates/lib/src/testutils.rs | 776 ++++++++++++++++++++++ 6 files changed, 1534 insertions(+), 22 deletions(-) create mode 100644 crates/lib/src/testutils.rs diff --git a/crates/lib/src/bootc_composefs/gc.rs b/crates/lib/src/bootc_composefs/gc.rs index 0aa0f9edc..475d3959f 100644 --- a/crates/lib/src/bootc_composefs/gc.rs +++ b/crates/lib/src/bootc_composefs/gc.rs @@ -15,7 +15,7 @@ use crate::{ bootc_composefs::{ boot::{BOOTC_UKI_DIR, BootType, get_type1_dir_name, get_uki_addon_dir_name, get_uki_name}, delete::{delete_image, delete_staged, delete_state_dir}, - status::{get_composefs_status, get_imginfo, list_bootloader_entries}, + status::{BootloaderEntry, get_composefs_status, get_imginfo, list_bootloader_entries}, }, composefs_consts::{STATE_DIR_RELATIVE, TYPE1_BOOT_DIR_PREFIX, UKI_NAME_PREFIX}, store::{BootedComposefs, Storage}, @@ -182,6 +182,26 @@ fn delete_uki(storage: &Storage, uki_id: &str, dry_run: bool) -> Result<()> { Ok(()) } +/// Find boot binaries on disk that are not referenced by any bootloader entry. +/// +/// We compare against `boot_artifact_name` (the directory/file name on disk) +/// rather than `fsverity` (the composefs= cmdline digest), because a shared +/// entry's directory name may belong to a different deployment than the one +/// whose composefs digest is in the BLS options line. +fn unreferenced_boot_binaries<'a>( + boot_binaries: &'a [BootBinary], + bootloader_entries: &[BootloaderEntry], +) -> Vec<&'a BootBinary> { + boot_binaries + .iter() + .filter(|bin| { + !bootloader_entries + .iter() + .any(|entry| entry.boot_artifact_name == bin.1) + }) + .collect() +} + /// 1. List all bootloader entries /// 2. List all EROFS images /// 3. List all state directories @@ -224,24 +244,8 @@ pub(crate) async fn composefs_gc( tracing::debug!("bootloader_entries: {bootloader_entries:?}"); tracing::debug!("boot_binaries: {boot_binaries:?}"); - // Bootloader entry is deleted, but the binary (UKI/kernel+initrd) still exists - let unreferenced_boot_binaries = boot_binaries - .iter() - .filter(|bin_path| { - // We reuse kernel + initrd if they're the same for two deployments - // We don't want to delete the (being deleted) deployment's kernel + initrd - // if it's in use by any other deployment - // - // filter the ones that are not referenced by any bootloader entry - !bootloader_entries - .iter() - // We compare the name of directory containing the binary instead of comparing the - // fsverity digest. This is because a shared entry might differing directory - // name and fsverity digest in the cmdline. And since we want to GC the actual - // binaries, we compare with the directory name - .any(|boot_entry| boot_entry.boot_artifact_name == bin_path.1) - }) - .collect::>(); + let unreferenced_boot_binaries = + unreferenced_boot_binaries(&boot_binaries, &bootloader_entries); tracing::debug!("unreferenced_boot_binaries: {unreferenced_boot_binaries:?}"); @@ -355,3 +359,490 @@ pub(crate) async fn composefs_gc( Ok(gc_result) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::bootc_composefs::status::list_type1_entries; + use crate::testutils::{ChangeType, TestRoot}; + + /// Reproduce the shared-entry GC bug from issue #2102. + /// + /// Scenario with both shared and non-shared kernels: + /// + /// 1. Install deployment A (kernel K1, boot dir "A") + /// 2. Upgrade to B, same kernel → shares A's boot dir + /// 3. Upgrade to C, new kernel K2 → gets its own boot dir "C" + /// 4. Upgrade to D, same kernel as C → shares C's boot dir + /// + /// After GC of A (the creator of boot dir used by B): + /// - A's boot dir must still exist (B references it) + /// - C's boot dir must still exist (D references it) + /// + /// The old code compared `fsverity` instead of `boot_artifact_name`, + /// which would incorrectly mark A's boot dir as unreferenced once A's + /// BLS entry is gone — even though B still points its linux/initrd + /// paths at A's directory. + #[test] + fn test_gc_shared_boot_binaries_not_deleted() -> anyhow::Result<()> { + let mut root = TestRoot::new()?; + let digest_a = root.current().verity.clone(); + + // B shares A's kernel (userspace-only change) + root.upgrade(1, ChangeType::Userspace)?; + + // C gets a new kernel + root.upgrade(2, ChangeType::Kernel)?; + let digest_c = root.current().verity.clone(); + + // D shares C's kernel (userspace-only change) + root.upgrade(3, ChangeType::Userspace)?; + let digest_d = root.current().verity.clone(); + + // Now GC deployment A — the one that *created* the shared boot dir + root.gc_deployment(&digest_a)?; + + // At this point only C (secondary) and D (primary) have BLS entries. + // But A's boot binary directory is still on disk because B used to + // share it and we haven't cleaned up boot binaries yet — that's + // what the GC filter decides. + let boot_dir = root.boot_dir()?; + + // Collect what's on disk: two boot dirs (A's and C's) + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + assert_eq!( + on_disk.len(), + 2, + "should have A's and C's boot dirs on disk" + ); + + // Collect what the BLS entries reference + let bls_entries = list_type1_entries(&boot_dir)?; + assert_eq!(bls_entries.len(), 2, "D (primary) + C (secondary)"); + + // The fix: unreferenced_boot_binaries uses boot_artifact_name. + // D's boot_artifact_name points to C's dir, C's points to itself. + // A's boot dir is NOT referenced by any current BLS entry's + // boot_artifact_name (B was the one referencing it, and B is no + // longer in the BLS entries either). + let unreferenced = unreferenced_boot_binaries(&on_disk, &bls_entries); + + // A's boot dir IS unreferenced (only B used it, and B isn't in BLS anymore) + assert_eq!(unreferenced.len(), 1); + assert_eq!(unreferenced[0].1, digest_a); + + // C's boot dir is still referenced (by both C and D via boot_artifact_name) + assert!( + !unreferenced.iter().any(|b| b.1 == digest_c), + "C's boot dir must not be unreferenced" + ); + + // Now the more dangerous scenario: GC C, the creator of the boot + // dir that D shares. After this, remaining deployments are [B, D]. + // B still shares A's boot dir, D still shares C's boot dir. + root.gc_deployment(&digest_c)?; + + let mut on_disk_2 = Vec::new(); + collect_type1_boot_binaries(&root.boot_dir()?, &mut on_disk_2)?; + // A's dir + C's dir still on disk (boot binary cleanup hasn't run) + assert_eq!(on_disk_2.len(), 2); + + let bls_entries_2 = list_type1_entries(&root.boot_dir()?)?; + // D (primary) + B (secondary) + assert_eq!(bls_entries_2.len(), 2); + + let entry_d = bls_entries_2 + .iter() + .find(|e| e.fsverity == digest_d) + .unwrap(); + assert_eq!( + entry_d.boot_artifact_name, digest_c, + "D shares C's boot dir" + ); + + let unreferenced_2 = unreferenced_boot_binaries(&on_disk_2, &bls_entries_2); + + // Both boot dirs are still referenced: + // - A's dir via B's boot_artifact_name + // - C's dir via D's boot_artifact_name + assert!( + unreferenced_2.is_empty(), + "no boot dirs should be unreferenced when both are shared" + ); + + // Prove the old buggy logic would fail: if we compared fsverity + // instead of boot_artifact_name, BOTH dirs would be wrongly + // unreferenced. Neither A nor C has a BLS entry with matching + // fsverity — only B (verity=B) and D (verity=D) exist, but their + // boot dirs are named after A and C respectively. + let buggy_unreferenced: Vec<_> = on_disk_2 + .iter() + .filter(|bin| !bls_entries_2.iter().any(|e| e.fsverity == bin.1)) + .collect(); + assert_eq!( + buggy_unreferenced.len(), + 2, + "old fsverity-based logic would incorrectly GC both boot dirs" + ); + + Ok(()) + } + + /// Verify that list_type1_entries correctly parses legacy (unprefixed) BLS + /// entries. This is the code path that composefs_gc actually uses to find + /// bootloader entries, so it's critical that it handles both layouts. + #[test] + fn test_list_type1_entries_handles_legacy_bls() -> anyhow::Result<()> { + let mut root = TestRoot::new_legacy()?; + let digest_a = root.current().verity.clone(); + + root.upgrade(1, ChangeType::Userspace)?; + let digest_b = root.current().verity.clone(); + + let boot_dir = root.boot_dir()?; + let bls_entries = list_type1_entries(&boot_dir)?; + + assert_eq!(bls_entries.len(), 2, "Should find both BLS entries"); + + // boot_artifact_name should return the raw digest (no prefix) + // because the legacy entries don't have the prefix + for entry in &bls_entries { + assert_eq!( + entry.boot_artifact_name, digest_a, + "Both entries should reference A's boot dir (shared kernel)" + ); + } + + // fsverity should differ between the two entries + let verity_set: std::collections::HashSet<&str> = + bls_entries.iter().map(|e| e.fsverity.as_str()).collect(); + assert!(verity_set.contains(digest_a.as_str())); + assert!(verity_set.contains(digest_b.as_str())); + + Ok(()) + } + + /// Legacy (unprefixed) boot dirs are invisible to collect_type1_boot_binaries, + /// which only looks for the `bootc_composefs-` prefix. This test verifies + /// that the GC scanner does not see unprefixed directories. + /// + /// This is the problem that PR #2128 solves by migrating legacy entries + /// to the prefixed format before any GC or status operations run. + #[test] + fn test_legacy_boot_dirs_invisible_to_gc_scanner() -> anyhow::Result<()> { + let root = TestRoot::new_legacy()?; + + // The legacy layout creates a boot dir without the prefix + let boot_dir = root.boot_dir()?; + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + + // collect_type1_boot_binaries requires the prefix — legacy dirs + // are invisible to it + assert!( + on_disk.is_empty(), + "Legacy (unprefixed) boot dirs should not be found by collect_type1_boot_binaries" + ); + + Ok(()) + } + + /// After migration from legacy to prefixed layout, GC should work + /// correctly — the boot binary directories become visible and + /// the BLS entries reference them properly. + #[test] + fn test_gc_works_after_legacy_migration() -> anyhow::Result<()> { + let mut root = TestRoot::new_legacy()?; + let digest_a = root.current().verity.clone(); + + // B shares A's kernel (userspace-only change) + root.upgrade(1, ChangeType::Userspace)?; + + // C gets a new kernel + root.upgrade(2, ChangeType::Kernel)?; + + // Simulate the migration that PR #2128 performs + root.migrate_to_prefixed()?; + + // Now GC should see both boot dirs + let boot_dir = root.boot_dir()?; + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + assert_eq!(on_disk.len(), 2, "Should see A's and C's boot dirs"); + + // BLS entries should correctly reference boot artifact names + let bls_entries = list_type1_entries(&boot_dir)?; + assert_eq!(bls_entries.len(), 2); + + // No boot dirs should be unreferenced (all are in use) + let unreferenced = unreferenced_boot_binaries(&on_disk, &bls_entries); + assert!( + unreferenced.is_empty(), + "All boot dirs should be referenced after migration" + ); + + // GC deployment A (the one that created the shared boot dir) + root.gc_deployment(&digest_a)?; + + let boot_dir = root.boot_dir()?; + let bls_entries = list_type1_entries(&boot_dir)?; + assert_eq!(bls_entries.len(), 2, "B (secondary) + C (primary)"); + + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + assert_eq!(on_disk.len(), 2, "Both boot dirs still on disk"); + + let unreferenced = unreferenced_boot_binaries(&on_disk, &bls_entries); + // A's boot dir is still referenced by B + assert!( + unreferenced.is_empty(), + "A's boot dir should still be referenced by B after migration" + ); + + Ok(()) + } + + /// Test the full upgrade cycle with shared kernels after migration: + /// install (legacy) → migrate → upgrade → GC. + /// + /// This verifies that GC correctly handles a system that was originally + /// installed with old bootc, migrated, and then upgraded with new bootc. + #[test] + fn test_gc_post_migration_upgrade_cycle() -> anyhow::Result<()> { + let mut root = TestRoot::new_legacy()?; + let digest_a = root.current().verity.clone(); + + // B shares A's kernel (still legacy) + root.upgrade(1, ChangeType::Userspace)?; + + // Simulate migration + root.migrate_to_prefixed()?; + + // Now upgrade with new bootc (creates prefixed entries) + root.upgrade(2, ChangeType::Kernel)?; + let digest_c = root.current().verity.clone(); + + // D shares C's kernel + root.upgrade(3, ChangeType::Userspace)?; + let digest_d = root.current().verity.clone(); + + // GC all old deployments, keeping only C and D + root.gc_deployment(&digest_a)?; + + let boot_dir = root.boot_dir()?; + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + + let bls_entries = list_type1_entries(&boot_dir)?; + assert_eq!(bls_entries.len(), 2, "D (primary) + C (secondary)"); + + let unreferenced = unreferenced_boot_binaries(&on_disk, &bls_entries); + // A's boot dir is unreferenced (B is gone, only C and D remain) + assert_eq!( + unreferenced.len(), + 1, + "A's boot dir should be unreferenced after GC of A and B is evicted" + ); + assert_eq!(unreferenced[0].1, digest_a); + + // C's boot dir must still be referenced by D + assert!( + !unreferenced.iter().any(|b| b.1 == digest_c), + "C's boot dir must still be referenced by D" + ); + + // Verify D shares C's boot dir + let entry_d = bls_entries + .iter() + .find(|e| e.fsverity == digest_d) + .expect("D should have a BLS entry"); + assert_eq!( + entry_d.boot_artifact_name, digest_c, + "D should share C's boot dir" + ); + + Ok(()) + } + + /// Test deep transitive sharing: A → B → C → D all share A's boot dir + /// via successive userspace-only upgrades. When we GC A (the creator + /// of the boot dir), the dir must be kept because the remaining + /// deployments still reference it. + /// + /// This tests that boot_dir_verity propagates correctly through + /// a chain of userspace-only upgrades and that the GC filter handles + /// the case where no remaining deployment's fsverity matches the + /// boot directory name. + #[test] + fn test_gc_deep_transitive_sharing_chain() -> anyhow::Result<()> { + let mut root = TestRoot::new()?; + let digest_a = root.current().verity.clone(); + + // B, C, D all share A's kernel via userspace-only upgrades + root.upgrade(1, ChangeType::Userspace)?; + root.upgrade(2, ChangeType::Userspace)?; + root.upgrade(3, ChangeType::Userspace)?; + let digest_d = root.current().verity.clone(); + + // Only one boot dir on disk (all share A's) + let boot_dir = root.boot_dir()?; + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + assert_eq!(on_disk.len(), 1, "All deployments share one boot dir"); + assert_eq!(on_disk[0].1, digest_a, "The boot dir belongs to A"); + + // BLS entries: D (primary) + C (secondary), both referencing A's dir + let bls_entries = list_type1_entries(&boot_dir)?; + assert_eq!(bls_entries.len(), 2); + for entry in &bls_entries { + assert_eq!( + entry.boot_artifact_name, digest_a, + "All entries reference A's boot dir" + ); + } + + // GC deployment A (the creator of the shared boot dir) + root.gc_deployment(&digest_a)?; + + let boot_dir = root.boot_dir()?; + let bls_entries = list_type1_entries(&boot_dir)?; + // D (primary) + C (secondary) — A was already evicted from BLS + assert_eq!(bls_entries.len(), 2); + + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + + let unreferenced = unreferenced_boot_binaries(&on_disk, &bls_entries); + assert!( + unreferenced.is_empty(), + "A's boot dir must stay — C and D still reference it" + ); + + // Now also GC B and C, leaving only D + let digest_b = crate::testutils::fake_digest_version(1); + let digest_c = crate::testutils::fake_digest_version(2); + root.gc_deployment(&digest_b)?; + root.gc_deployment(&digest_c)?; + + // D is the only deployment left + let boot_dir = root.boot_dir()?; + let bls_entries = list_type1_entries(&boot_dir)?; + assert_eq!(bls_entries.len(), 1, "Only D remains"); + assert_eq!(bls_entries[0].fsverity, digest_d); + assert_eq!( + bls_entries[0].boot_artifact_name, digest_a, + "D still references A's boot dir" + ); + + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + let unreferenced = unreferenced_boot_binaries(&on_disk, &bls_entries); + assert!( + unreferenced.is_empty(), + "A's boot dir must survive — D is the last deployment and still uses it" + ); + + Ok(()) + } + + /// Verify that boot_artifact_info().1 (has_prefix) is the correct + /// signal for identifying entries that need migration, and that the + /// GC filter works correctly at each stage of the migration pipeline. + /// + /// This exercises the API that stage_bls_entry_changes() in PR #2128 + /// uses to decide which entries to migrate. + #[test] + fn test_boot_artifact_info_drives_migration_decisions() -> anyhow::Result<()> { + use crate::bootc_composefs::status::get_sorted_type1_boot_entries; + + let mut root = TestRoot::new_legacy()?; + let digest_a = root.current().verity.clone(); + + root.upgrade(1, ChangeType::Userspace)?; + root.upgrade(2, ChangeType::Kernel)?; + + // -- Pre-migration: all entries lack the prefix -- + let boot_dir = root.boot_dir()?; + let raw_entries = get_sorted_type1_boot_entries(&boot_dir, true)?; + assert_eq!(raw_entries.len(), 2); + + let needs_migration: Vec<_> = raw_entries + .iter() + .filter(|e| !e.boot_artifact_info().unwrap().1) + .collect(); + assert_eq!( + needs_migration.len(), + 2, + "All legacy entries should need migration (has_prefix=false)" + ); + + // GC scanner can't see the boot dirs (no prefix on disk) + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + assert!(on_disk.is_empty(), "Legacy dirs invisible before migration"); + + // -- Migrate -- + root.migrate_to_prefixed()?; + + // -- Post-migration: all entries have the prefix -- + let boot_dir = root.boot_dir()?; + let raw_entries = get_sorted_type1_boot_entries(&boot_dir, true)?; + assert_eq!(raw_entries.len(), 2); + + let needs_migration: Vec<_> = raw_entries + .iter() + .filter(|e| !e.boot_artifact_info().unwrap().1) + .collect(); + assert!( + needs_migration.is_empty(), + "No entries should need migration after migrate_to_prefixed()" + ); + + // GC scanner can now see the boot dirs + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + assert_eq!(on_disk.len(), 2, "Both dirs visible after migration"); + + // GC filter correctly identifies all dirs as referenced + let bls_entries = list_type1_entries(&boot_dir)?; + let unreferenced = unreferenced_boot_binaries(&on_disk, &bls_entries); + assert!( + unreferenced.is_empty(), + "All dirs referenced after migration" + ); + + // -- Upgrade with new bootc (prefixed from creation) -- + root.upgrade(3, ChangeType::Kernel)?; + + let boot_dir = root.boot_dir()?; + let raw_entries = get_sorted_type1_boot_entries(&boot_dir, true)?; + // All entries (both migrated and new) should have the prefix + for entry in &raw_entries { + let (_, has_prefix) = entry.boot_artifact_info()?; + assert!( + has_prefix, + "All entries should have prefix after migration + upgrade" + ); + } + + // GC should now see 3 boot dirs: A's, C's (from upgrade 2), and + // the new one from upgrade 3 + let mut on_disk = Vec::new(); + collect_type1_boot_binaries(&boot_dir, &mut on_disk)?; + assert_eq!(on_disk.len(), 3, "Three boot dirs on disk"); + + // Only 2 BLS entries (primary + secondary), so one dir is unreferenced + let bls_entries = list_type1_entries(&boot_dir)?; + assert_eq!(bls_entries.len(), 2); + let unreferenced = unreferenced_boot_binaries(&on_disk, &bls_entries); + assert_eq!( + unreferenced.len(), + 1, + "A's boot dir should be unreferenced (B evicted from BLS)" + ); + assert_eq!(unreferenced[0].1, digest_a); + + Ok(()) + } +} diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index 58a93638a..5ca710395 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -283,7 +283,7 @@ fn get_sorted_type1_boot_entries_helper( Ok(all_configs) } -fn list_type1_entries(boot_dir: &Dir) -> Result> { +pub(crate) fn list_type1_entries(boot_dir: &Dir) -> Result> { // Type1 Entry let boot_entries = get_sorted_type1_boot_entries(boot_dir, true)?; @@ -1130,4 +1130,59 @@ mod tests { let result = ComposefsCmdline::find_in_cmdline(&cmdline); assert!(result.is_none()); } + + use crate::testutils::fake_digest_version; + + /// Test that staged entries are also collected by list_type1_entries. + /// This is important for GC to not delete staged deployments' boot binaries. + #[test] + fn test_list_type1_entries_includes_staged() -> Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + + let digest_active = fake_digest_version(0); + let digest_staged = fake_digest_version(1); + + let active_entry = format!( + r#" + title Active Deployment + version 2 + sort-key 1 + linux /boot/bootc_composefs-{digest_active}/vmlinuz + initrd /boot/bootc_composefs-{digest_active}/initramfs.img + options root=UUID=abc123 rw composefs={digest_active} + "# + ); + + let staged_entry = format!( + r#" + title Staged Deployment + version 3 + sort-key 0 + linux /boot/bootc_composefs-{digest_staged}/vmlinuz + initrd /boot/bootc_composefs-{digest_staged}/initramfs.img + options root=UUID=abc123 rw composefs={digest_staged} + "# + ); + + tempdir.create_dir_all("loader/entries")?; + tempdir.create_dir_all("loader/entries.staged")?; + tempdir.atomic_write("loader/entries/active.conf", active_entry)?; + tempdir.atomic_write("loader/entries.staged/staged.conf", staged_entry)?; + + let result = list_type1_entries(&tempdir)?; + assert_eq!(result.len(), 2); + + let verity_set: std::collections::HashSet<&str> = + result.iter().map(|e| e.fsverity.as_str()).collect(); + assert!( + verity_set.contains(digest_active.as_str()), + "Should contain active entry" + ); + assert!( + verity_set.contains(digest_staged.as_str()), + "Should contain staged entry" + ); + + Ok(()) + } } diff --git a/crates/lib/src/lib.rs b/crates/lib/src/lib.rs index 558ca8718..e248139f9 100644 --- a/crates/lib/src/lib.rs +++ b/crates/lib/src/lib.rs @@ -96,6 +96,9 @@ mod task; mod ukify; mod utils; +#[cfg(test)] +pub(crate) mod testutils; + #[cfg(feature = "docgen")] mod cli_json; diff --git a/crates/lib/src/parsers/bls_config.rs b/crates/lib/src/parsers/bls_config.rs index fb6f2207c..d86bd508e 100644 --- a/crates/lib/src/parsers/bls_config.rs +++ b/crates/lib/src/parsers/bls_config.rs @@ -741,4 +741,89 @@ mod tests { .contains("missing file name") ); } + + #[test] + fn test_boot_artifact_name_unknown_type() { + let config = BLSConfig { + cfg_type: BLSConfigType::Unknown, + version: "1".to_string(), + ..Default::default() + }; + + let result = config.boot_artifact_name(); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("unknown config type") + ); + } + #[test] + fn test_boot_artifact_name_efi_nested_path() -> Result<()> { + let efi_path = Utf8PathBuf::from("/EFI/Linux/bootc/bootc_composefs-deadbeef01234567.efi"); + let config = BLSConfig { + cfg_type: BLSConfigType::EFI { efi: efi_path }, + version: "1".to_string(), + ..Default::default() + }; + + assert_eq!(config.boot_artifact_name()?, "deadbeef01234567"); + Ok(()) + } + + #[test] + fn test_boot_artifact_name_non_efi_deep_path() -> Result<()> { + // Realistic Type1 path: /boot/bootc_composefs-/vmlinuz + let digest = "7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6"; + let linux_path = Utf8PathBuf::from(format!("/boot/bootc_composefs-{digest}/vmlinuz")); + let config = BLSConfig { + cfg_type: BLSConfigType::NonEFI { + linux: linux_path, + initrd: vec![], + options: None, + }, + version: "1".to_string(), + ..Default::default() + }; + + assert_eq!(config.boot_artifact_name()?, digest); + Ok(()) + } + + /// Test boot_artifact_name from parsed EFI config + #[test] + fn test_boot_artifact_name_from_parsed_efi_config() -> Result<()> { + let digest = "f7415d75017a12a387a39d2281e033a288fc15775108250ef70a01dcadb93346"; + let input = format!( + r#" + title Fedora UKI + version 1 + efi /EFI/Linux/bootc/bootc_composefs-{digest}.efi + sort-key bootc-fedora-0 + "# + ); + + let config = parse_bls_config(&input)?; + assert_eq!(config.boot_artifact_name()?, digest); + assert_eq!(config.get_verity()?, digest); + Ok(()) + } + + /// Test that Non-EFI boot_artifact_name fails when linux path has no parent + #[test] + fn test_boot_artifact_name_non_efi_no_parent() { + let config = BLSConfig { + cfg_type: BLSConfigType::NonEFI { + linux: Utf8PathBuf::from("vmlinuz"), + initrd: vec![], + options: None, + }, + version: "1".to_string(), + ..Default::default() + }; + + let result = config.boot_artifact_name(); + assert!(result.is_err()); + } } diff --git a/crates/lib/src/parsers/grub_menuconfig.rs b/crates/lib/src/parsers/grub_menuconfig.rs index cd7954f24..b01e87e54 100644 --- a/crates/lib/src/parsers/grub_menuconfig.rs +++ b/crates/lib/src/parsers/grub_menuconfig.rs @@ -583,7 +583,7 @@ mod test { fn test_menuentry_boot_artifact_name_success() { let body = MenuentryBody { insmod: vec!["fat", "chain"], - chainloader: "/EFI/bootc_composefs/bootc_composefs-abcd1234.efi".to_string(), + chainloader: "/EFI/Linux/bootc/bootc_composefs-abcd1234.efi".to_string(), search: "--no-floppy --set=root --fs-uuid test", version: 0, extra: vec![], @@ -625,7 +625,7 @@ mod test { fn test_menuentry_boot_artifact_name_missing_suffix() { let body = MenuentryBody { insmod: vec!["fat", "chain"], - chainloader: "/EFI/bootc_composefs/bootc_composefs-abcd1234".to_string(), + chainloader: "/EFI/Linux/bootc/bootc_composefs-abcd1234".to_string(), search: "--no-floppy --set=root --fs-uuid test", version: 0, extra: vec![], @@ -645,4 +645,106 @@ mod test { .contains("missing expected suffix") ); } + + #[test] + fn test_menuentry_boot_artifact_name_empty_chainloader() { + let body = MenuentryBody { + insmod: vec![], + chainloader: "".to_string(), + search: "", + version: 0, + extra: vec![], + }; + + let entry = MenuEntry { + title: "Empty".to_string(), + body, + }; + + let result = entry.boot_artifact_name(); + assert!(result.is_err()); + } + + /// Test that boot_artifact_name and get_verity return the same value + /// for a standard UKI entry. + /// + /// Note: GRUB/UKI entries always have matching boot_artifact_name and + /// get_verity because both derive from the same chainloader path. The + /// shared-entry divergence (where boot_artifact_name != get_verity) only + /// applies to Type1 BLS entries, which have separate linux path and + /// composefs= cmdline parameter. + #[test] + fn test_menuentry_boot_artifact_name_matches_get_verity() { + let digest = "f7415d75017a12a387a39d2281e033a288fc15775108250ef70a01dcadb93346"; + + let body = MenuentryBody { + insmod: vec!["fat", "chain"], + chainloader: format!("/EFI/Linux/bootc/bootc_composefs-{digest}.efi"), + search: "--no-floppy --set=root --fs-uuid test", + version: 0, + extra: vec![], + }; + + let entry = MenuEntry { + title: "Test".to_string(), + body, + }; + + let artifact_name = entry.boot_artifact_name().unwrap(); + let verity = entry.get_verity().unwrap(); + assert_eq!(artifact_name, verity); + assert_eq!(artifact_name, digest); + } + + /// Test boot_artifact_name with realistic full-length hex digest + #[test] + fn test_menuentry_boot_artifact_name_full_digest() { + let digest = "7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6"; + + let body = MenuentryBody { + insmod: vec!["fat", "chain"], + chainloader: format!("/EFI/Linux/bootc/bootc_composefs-{digest}.efi"), + search: "--no-floppy --set=root --fs-uuid \"${EFI_PART_UUID}\"", + version: 0, + extra: vec![], + }; + + let entry = MenuEntry { + title: format!("Fedora Bootc UKI: ({digest})"), + body, + }; + + assert_eq!(entry.boot_artifact_name().unwrap(), digest); + } + + /// Test boot_artifact_name via MenuEntry::new constructor + #[test] + fn test_menuentry_new_boot_artifact_name() { + let uki_id = "abc123def456"; + let entry = MenuEntry::new("Fedora 42", uki_id); + + assert_eq!(entry.boot_artifact_name().unwrap(), uki_id); + assert_eq!(entry.get_verity().unwrap(), uki_id); + } + + /// Test boot_artifact_name from a parsed grub config + #[test] + fn test_menuentry_boot_artifact_name_from_parsed() { + let digest = "7e11ac46e3e022053e7226a20104ac656bf72d1a84e3a398b7cce70e9df188b6"; + let menuentry = format!( + r#" + menuentry "Fedora 42: ({digest})" {{ + insmod fat + insmod chain + search --no-floppy --set=root --fs-uuid "${{EFI_PART_UUID}}" + chainloader /EFI/Linux/bootc/bootc_composefs-{digest}.efi + }} + "# + ); + + let result = parse_grub_menuentry_file(&menuentry).unwrap(); + assert_eq!(result.len(), 1); + assert_eq!(result[0].boot_artifact_name().unwrap(), digest); + assert_eq!(result[0].get_verity().unwrap(), digest); + } } diff --git a/crates/lib/src/testutils.rs b/crates/lib/src/testutils.rs new file mode 100644 index 000000000..e6fdf767a --- /dev/null +++ b/crates/lib/src/testutils.rs @@ -0,0 +1,776 @@ +//! Test infrastructure for simulating a composefs BLS Type1 sysroot. +//! +//! Provides [`TestRoot`] which creates a realistic sysroot filesystem layout +//! suitable for unit-testing the GC, status, and boot entry logic without +//! requiring a real booted system. + +use std::sync::Arc; + +use anyhow::{Context, Result}; +use cap_std_ext::cap_std::{self, fs::Dir}; +use cap_std_ext::cap_tempfile; +use cap_std_ext::dirext::CapStdExtDirExt; + +use crate::bootc_composefs::boot::{ + FILENAME_PRIORITY_PRIMARY, FILENAME_PRIORITY_SECONDARY, get_type1_dir_name, primary_sort_key, + secondary_sort_key, type1_entry_conf_file_name, +}; +use crate::composefs_consts::{ + ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_DIGEST, ORIGIN_KEY_BOOT_TYPE, STATE_DIR_RELATIVE, + TYPE1_BOOT_DIR_PREFIX, TYPE1_ENT_PATH, +}; +use crate::parsers::bls_config::{BLSConfig, parse_bls_config}; +use crate::store::ComposefsRepository; + +use ostree_ext::container::deploy::ORIGIN_CONTAINER; + +/// Return a deterministic SHA-256 hex digest for a test build version. +/// +/// Computes `sha256("build-{n}")`, producing a realistic 64-char hex digest +/// that is stable across runs. +pub(crate) fn fake_digest_version(n: u32) -> String { + let hash = openssl::hash::hash( + openssl::hash::MessageDigest::sha256(), + format!("build-{n}").as_bytes(), + ) + .expect("sha256"); + hex::encode(hash) +} + +/// What changed in an upgrade. +#[derive(Clone, Copy, Debug)] +#[allow(dead_code)] +pub(crate) enum ChangeType { + /// Only userspace changed; kernel+initrd are identical, so the new + /// deployment shares the previous deployment's boot binary directory. + Userspace, + /// The kernel (and/or initrd) changed, so the new deployment gets + /// its own boot binary directory. + Kernel, + /// Both userspace and kernel changed. New boot binary directory and + /// new composefs image. + Both, +} + +/// Controls whether TestRoot writes boot entries in the current (prefixed) +/// or legacy (unprefixed) format. +/// +/// Older versions of bootc didn't prefix boot binary directories or UKI +/// filenames with `bootc_composefs-`. PR #2128 adds a migration path that +/// renames these on first run. This enum lets tests simulate both layouts. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[allow(dead_code)] +pub(crate) enum LayoutMode { + /// Current layout: directories named `bootc_composefs-`, + /// BLS linux paths reference the prefixed directory name. + Current, + /// Legacy layout: directories named with just the raw ``, + /// BLS linux paths reference the unprefixed directory name. + /// This simulates a system installed with an older bootc. + Legacy, +} + +/// Metadata for a single simulated deployment. +#[derive(Clone, Debug)] +pub(crate) struct DeploymentMeta { + /// The deployment's composefs verity digest (what goes in `composefs=`). + pub verity: String, + /// SHA256 digest of the vmlinuz+initrd pair. + pub boot_digest: String, + /// The name of the boot binary directory (verity portion only, no prefix). + /// This equals `verity` for the deployment that created the directory, + /// but may point to a different deployment's directory for shared entries. + pub boot_dir_verity: String, + /// The container image reference stored in the origin file. + pub imgref: String, + /// OS identifier for BLS entry naming. + pub os_id: String, + /// Version string for BLS entry. + pub version: String, +} + +/// A simulated composefs BLS Type1 sysroot for testing. +/// +/// Creates the filesystem layout that the GC, status, and boot entry code +/// expects: +/// +/// ```text +/// / +/// ├── composefs/ # composefs repo (objects/, images/, streams/) +/// │ └── images/ +/// │ └── # one file per deployed image +/// ├── state/deploy/ +/// │ └── / +/// │ ├── .origin +/// │ └── etc/ +/// └── boot/ +/// ├── bootc_composefs-/ +/// │ ├── vmlinuz +/// │ └── initrd +/// └── loader/entries/ +/// └── *.conf +/// ``` +pub(crate) struct TestRoot { + /// The root Dir — equivalent to `Storage.physical_root`. + /// Also owns the tempdir lifetime. + root: cap_tempfile::TempDir, + /// Deployments added so far, in order. + deployments: Vec, + /// Composefs repository handle. + repo: Arc, + /// Whether to write entries in the current (prefixed) or legacy format. + layout: LayoutMode, +} + +impl TestRoot { + /// Create a new test sysroot with one initial deployment (the "install"). + /// + /// The deployment gets: + /// - An EROFS image entry in `composefs/images/` + /// - A state directory with a `.origin` file + /// - A boot binary directory with vmlinuz + initrd + /// - A primary BLS Type1 entry in `loader/entries/` + pub fn new() -> Result { + Self::with_layout(LayoutMode::Current) + } + + /// Create a new test sysroot using the legacy (unprefixed) layout. + /// + /// This simulates a system installed with an older version of bootc + /// that didn't prefix boot binary directories with `bootc_composefs-`. + /// Useful for testing the backwards compatibility migration from PR #2128. + #[allow(dead_code)] + pub fn new_legacy() -> Result { + Self::with_layout(LayoutMode::Legacy) + } + + /// Create a test sysroot with the specified layout mode. + fn with_layout(layout: LayoutMode) -> Result { + let root = cap_tempfile::tempdir(cap_std::ambient_authority())?; + + // Create the composefs repo directory structure + root.create_dir_all("composefs") + .context("Creating composefs/")?; + root.create_dir_all("composefs/images") + .context("Creating composefs/images/")?; + + // Create the state directory + root.create_dir_all(STATE_DIR_RELATIVE) + .context("Creating state/deploy/")?; + + // Create the boot directory with loader/entries + root.create_dir_all(&format!("boot/{TYPE1_ENT_PATH}")) + .context("Creating boot/loader/entries/")?; + + // Open the composefs repo + let repo_dir = root.open_dir("composefs")?; + let mut repo = + ComposefsRepository::open_path(&repo_dir, ".").context("Opening composefs repo")?; + repo.set_insecure(true); + + let mut test_root = Self { + root, + deployments: Vec::new(), + repo: Arc::new(repo), + layout, + }; + + // Add an initial deployment (version 0) + let meta = DeploymentMeta { + verity: fake_digest_version(0), + boot_digest: fake_digest_version(0), + boot_dir_verity: fake_digest_version(0), + imgref: "oci:quay.io/test/image:latest".into(), + os_id: "fedora".into(), + version: "42.20250101.0".into(), + }; + test_root.add_deployment(&meta, true)?; + + Ok(test_root) + } + + /// Access the root directory (equivalent to `Storage.physical_root`). + #[allow(dead_code)] + pub fn root(&self) -> &Dir { + &self.root + } + + /// Access the boot directory (equivalent to `Storage.boot_dir`). + pub fn boot_dir(&self) -> Result { + self.root.open_dir("boot").context("Opening boot/") + } + + /// Access the composefs repository. + #[allow(dead_code)] + pub fn repo(&self) -> &Arc { + &self.repo + } + + /// The most recently added deployment. + pub fn current(&self) -> &DeploymentMeta { + self.deployments.last().expect("at least one deployment") + } + + /// All deployments, oldest first. + #[allow(dead_code)] + pub fn deployments(&self) -> &[DeploymentMeta] { + &self.deployments + } + + /// Simulate an upgrade: adds a new deployment as the primary boot entry. + /// + /// The previous primary becomes the secondary. `change` controls whether + /// the kernel changed: + /// + /// - [`ChangeType::Userspace`]: only userspace changed, so the new + /// deployment shares the previous deployment's boot binary directory. + /// This is the scenario that triggers the GC bug from issue #2102. + /// - [`ChangeType::Kernel`]: the kernel+initrd changed, so a new boot + /// binary directory is created. + pub fn upgrade(&mut self, version: u32, change: ChangeType) -> Result<&DeploymentMeta> { + let prev = self.current().clone(); + + let new_verity = fake_digest_version(version); + let (boot_dir_verity, boot_digest) = match change { + ChangeType::Userspace => (prev.boot_dir_verity.clone(), prev.boot_digest.clone()), + ChangeType::Kernel | ChangeType::Both => { + let new_boot_digest = fake_digest_version(version); + (new_verity.clone(), new_boot_digest) + } + }; + + let meta = DeploymentMeta { + verity: new_verity, + boot_digest, + boot_dir_verity, + imgref: prev.imgref.clone(), + os_id: prev.os_id.clone(), + version: format!("4{}.20250201.0", self.deployments.len() + 1), + }; + + self.add_deployment(&meta, false)?; + + // Rewrite loader/entries/ to have the new primary + old secondary + self.rewrite_bls_entries()?; + + Ok(self.deployments.last().unwrap()) + } + + /// Simulate GC of the oldest deployment: removes its EROFS image, state + /// dir, and BLS entry, but leaves boot binaries alone (the real GC + /// decides whether to remove them based on `boot_artifact_name`). + pub fn gc_deployment(&mut self, verity: &str) -> Result<()> { + // Remove EROFS image + let images_dir = self.root.open_dir("composefs/images")?; + images_dir + .remove_file(verity) + .with_context(|| format!("Removing image {verity}"))?; + + // Remove state directory + self.root + .remove_dir_all(format!("{STATE_DIR_RELATIVE}/{verity}")) + .with_context(|| format!("Removing state dir for {verity}"))?; + + // Remove from our tracking list + self.deployments.retain(|d| d.verity != verity); + + // Rewrite BLS entries for remaining deployments + self.rewrite_bls_entries()?; + + Ok(()) + } + + /// Add a deployment: creates image, state dir, boot binaries, and BLS entry. + fn add_deployment(&mut self, meta: &DeploymentMeta, is_initial: bool) -> Result<()> { + self.write_erofs_image(&meta.verity)?; + self.write_state_dir(meta)?; + self.write_boot_binaries(&meta.boot_dir_verity)?; + + self.deployments.push(meta.clone()); + + if is_initial { + self.rewrite_bls_entries()?; + } + + Ok(()) + } + + /// Create a placeholder file in composefs/images/ for this deployment. + fn write_erofs_image(&self, verity: &str) -> Result<()> { + let images_dir = self.root.open_dir("composefs/images")?; + images_dir.atomic_write(verity, b"erofs-placeholder")?; + Ok(()) + } + + /// Create the state directory with a .origin file. + fn write_state_dir(&self, meta: &DeploymentMeta) -> Result<()> { + let state_path = format!("{STATE_DIR_RELATIVE}/{}", meta.verity); + self.root.create_dir_all(format!("{state_path}/etc"))?; + + // tini merges items under the same section name, so the repeated + // .section(ORIGIN_KEY_BOOT) calls produce a single [boot] section + // with both keys. This matches how state.rs writes the origin file. + let origin = tini::Ini::new() + .section("origin") + .item( + ORIGIN_CONTAINER, + format!("ostree-unverified-image:{}", meta.imgref), + ) + .section(ORIGIN_KEY_BOOT) + .item(ORIGIN_KEY_BOOT_TYPE, "bls") + .section(ORIGIN_KEY_BOOT) + .item(ORIGIN_KEY_BOOT_DIGEST, &meta.boot_digest); + + let state_dir = self.root.open_dir(&state_path)?; + state_dir.atomic_write( + format!("{}.origin", meta.verity), + origin.to_string().as_bytes(), + )?; + + Ok(()) + } + + /// Return the boot binary directory name for a given verity digest, + /// respecting the current layout mode. + fn boot_binary_dir_name(&self, boot_dir_verity: &str) -> String { + match self.layout { + LayoutMode::Current => get_type1_dir_name(boot_dir_verity), + LayoutMode::Legacy => boot_dir_verity.to_string(), + } + } + + /// Create the boot binary directory with vmlinuz + initrd. + /// Skips if the directory already exists (shared entry case). + fn write_boot_binaries(&self, boot_dir_verity: &str) -> Result<()> { + let dir_name = self.boot_binary_dir_name(boot_dir_verity); + let path = format!("boot/{dir_name}"); + + if self.root.exists(&path) { + return Ok(()); + } + + self.root.create_dir_all(&path)?; + let boot_bin_dir = self.root.open_dir(&path)?; + boot_bin_dir.atomic_write("vmlinuz", b"fake-kernel")?; + boot_bin_dir.atomic_write("initrd", b"fake-initrd")?; + Ok(()) + } + + /// Rewrite the BLS entries in loader/entries/ to match current deployments. + /// + /// The last deployment is primary, the second-to-last (if any) is secondary. + fn rewrite_bls_entries(&self) -> Result<()> { + let entries_dir = self.root.open_dir(&format!("boot/{TYPE1_ENT_PATH}"))?; + + // Remove all existing .conf files + for entry in entries_dir.entries()? { + let entry = entry?; + let name = entry.file_name(); + if name.to_string_lossy().ends_with(".conf") { + entries_dir.remove_file(name)?; + } + } + + let n = self.deployments.len(); + if n == 0 { + return Ok(()); + } + + // Primary = most recent deployment + let primary = &self.deployments[n - 1]; + let primary_conf = self.build_bls_config(primary, true); + let primary_fname = + type1_entry_conf_file_name(&primary.os_id, &primary.version, FILENAME_PRIORITY_PRIMARY); + entries_dir.atomic_write(&primary_fname, primary_conf.as_bytes())?; + + // Secondary = previous deployment (if exists) + if n >= 2 { + let secondary = &self.deployments[n - 2]; + let secondary_conf = self.build_bls_config(secondary, false); + let secondary_fname = type1_entry_conf_file_name( + &secondary.os_id, + &secondary.version, + FILENAME_PRIORITY_SECONDARY, + ); + entries_dir.atomic_write(&secondary_fname, secondary_conf.as_bytes())?; + } + + Ok(()) + } + + /// Build a BLS .conf file body for a deployment. + fn build_bls_config(&self, meta: &DeploymentMeta, is_primary: bool) -> String { + let dir_name = self.boot_binary_dir_name(&meta.boot_dir_verity); + let sort_key = if is_primary { + primary_sort_key(&meta.os_id) + } else { + secondary_sort_key(&meta.os_id) + }; + + format!( + "title {os_id} {version}\n\ + version {version}\n\ + sort-key {sort_key}\n\ + linux /boot/{dir_name}/vmlinuz\n\ + initrd /boot/{dir_name}/initrd\n\ + options root=UUID=test-uuid rw composefs={verity}\n", + os_id = meta.os_id, + version = meta.version, + sort_key = sort_key, + dir_name = dir_name, + verity = meta.verity, + ) + } + + /// Parse the current BLS entries from disk and return them. + #[allow(dead_code)] + pub fn read_bls_entries(&self) -> Result> { + let boot_dir = self.boot_dir()?; + let entries_dir = boot_dir.open_dir(TYPE1_ENT_PATH)?; + + let mut configs = Vec::new(); + for entry in entries_dir.entries()? { + let entry = entry?; + let name = entry.file_name(); + if !name.to_string_lossy().ends_with(".conf") { + continue; + } + let contents = entries_dir.read_to_string(&name)?; + configs.push(parse_bls_config(&contents)?); + } + + configs.sort(); + Ok(configs) + } + + /// List EROFS image names present in composefs/images/. + #[allow(dead_code)] + pub fn list_images(&self) -> Result> { + let images_dir = self.root.open_dir("composefs/images")?; + let mut names = Vec::new(); + for entry in images_dir.entries()? { + let entry = entry?; + let name = entry.file_name(); + names.push(name.to_string_lossy().into_owned()); + } + names.sort(); + Ok(names) + } + + /// List state directory names present in state/deploy/. + #[allow(dead_code)] + pub fn list_state_dirs(&self) -> Result> { + let state = self.root.open_dir(STATE_DIR_RELATIVE)?; + let mut names = Vec::new(); + for entry in state.entries()? { + let entry = entry?; + if entry.file_type()?.is_dir() { + names.push(entry.file_name().to_string_lossy().into_owned()); + } + } + names.sort(); + Ok(names) + } + + /// List boot binary directories (stripped of any prefix). + /// + /// In `Current` mode, strips `TYPE1_BOOT_DIR_PREFIX`; in `Legacy` mode, + /// returns directory names that look like hex digests directly. + #[allow(dead_code)] + pub fn list_boot_binaries(&self) -> Result> { + let boot_dir = self.boot_dir()?; + let mut names = Vec::new(); + for entry in boot_dir.entries()? { + let entry = entry?; + if !entry.file_type()?.is_dir() { + continue; + } + let name = entry.file_name().to_string_lossy().into_owned(); + // Skip non-boot directories like "loader" + if name == "loader" { + continue; + } + match self.layout { + LayoutMode::Current => { + if let Some(verity) = name.strip_prefix(TYPE1_BOOT_DIR_PREFIX) { + names.push(verity.to_string()); + } + } + LayoutMode::Legacy => { + // Legacy dirs are just the raw hex digest (64 chars). + // Only include entries that look like hex digests to + // avoid accidentally counting "loader" or other dirs. + if name.len() == 64 && name.chars().all(|c| c.is_ascii_hexdigit()) { + names.push(name); + } + } + } + } + names.sort(); + Ok(names) + } + + /// Simulate the backwards compatibility migration: rename all legacy + /// (unprefixed) boot binary directories to use the `bootc_composefs-` + /// prefix, and rewrite BLS entries to reference the new paths. + /// + /// This mirrors what `prepend_custom_prefix()` from PR #2128 does. + #[allow(dead_code)] + pub fn migrate_to_prefixed(&mut self) -> Result<()> { + anyhow::ensure!( + self.layout == LayoutMode::Legacy, + "migrate_to_prefixed only makes sense for legacy layouts" + ); + + let boot_dir = self.boot_dir()?; + + // Rename all unprefixed boot binary directories + let mut to_rename = Vec::new(); + for entry in boot_dir.entries()? { + let entry = entry?; + if !entry.file_type()?.is_dir() { + continue; + } + let name = entry.file_name().to_string_lossy().into_owned(); + if name == "loader" { + continue; + } + // Rename directories that look like bare hex digests + // (the legacy format). This is intentionally simplified + // compared to the real migration in PR #2128 which also + // handles UKI PE files and GRUB configs. + if !name.starts_with(TYPE1_BOOT_DIR_PREFIX) + && name.len() == 64 + && name.chars().all(|c| c.is_ascii_hexdigit()) + { + to_rename.push(name); + } + } + + for old_name in &to_rename { + let new_name = format!("{TYPE1_BOOT_DIR_PREFIX}{old_name}"); + rustix::fs::renameat(&boot_dir, old_name.as_str(), &boot_dir, new_name.as_str()) + .with_context(|| format!("Renaming {old_name} -> {new_name}"))?; + } + + // Switch to current mode and rewrite BLS entries + self.layout = LayoutMode::Current; + self.rewrite_bls_entries()?; + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Smoke test: verify TestRoot creates a valid sysroot layout. + #[test] + fn test_initial_install() -> Result<()> { + let root = TestRoot::new()?; + + let depl = root.current(); + assert_eq!(depl.verity, fake_digest_version(0)); + + // All three storage areas should have exactly one entry + assert_eq!(root.list_images()?.len(), 1); + assert_eq!(root.list_state_dirs()?.len(), 1); + assert_eq!(root.list_boot_binaries()?.len(), 1); + + // BLS entry should round-trip through the parser correctly + let entries = root.read_bls_entries()?; + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].get_verity()?, depl.verity); + assert_eq!(entries[0].boot_artifact_name()?, depl.verity); + + Ok(()) + } + + /// Verify that the legacy layout creates unprefixed boot directories + /// and BLS entries that reference unprefixed paths. + #[test] + fn test_legacy_layout_creates_unprefixed_dirs() -> Result<()> { + let root = TestRoot::new_legacy()?; + let depl = root.current(); + + // Boot binary directory should be the raw digest, no prefix + let boot_dir = root.boot_dir()?; + let expected_dir = &depl.verity; + assert!( + boot_dir.exists(expected_dir), + "Legacy layout should create unprefixed dir {expected_dir}" + ); + + // The prefixed version should NOT exist + let prefixed_dir = format!("{TYPE1_BOOT_DIR_PREFIX}{}", depl.verity); + assert!( + !boot_dir.exists(&prefixed_dir), + "Legacy layout should NOT create prefixed dir {prefixed_dir}" + ); + + // BLS entry should parse and return the correct verity digest. + // boot_artifact_name() handles legacy entries by returning the raw + // dir name when the prefix is absent. + let entries = root.read_bls_entries()?; + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].get_verity()?, depl.verity); + assert_eq!(entries[0].boot_artifact_name()?, depl.verity); + + Ok(()) + } + + /// Verify that legacy layout with multiple deployments (shared kernel + /// scenario) works correctly. + #[test] + fn test_legacy_layout_shared_kernel() -> Result<()> { + let mut root = TestRoot::new_legacy()?; + let digest_a = root.current().verity.clone(); + + // B shares A's kernel + root.upgrade(1, ChangeType::Userspace)?; + let digest_b = root.current().verity.clone(); + + // Should still have only one boot binary dir (shared) + let boot_bins = root.list_boot_binaries()?; + assert_eq!(boot_bins.len(), 1); + assert_eq!(boot_bins[0], digest_a); + + // Both BLS entries should reference A's boot dir via boot_artifact_name + let entries = root.read_bls_entries()?; + assert_eq!(entries.len(), 2); + for entry in &entries { + assert_eq!( + entry.boot_artifact_name()?, + digest_a, + "Both entries should point to A's boot dir" + ); + } + + // But they should have different composefs= verity digests + let verity_set: std::collections::HashSet = + entries.iter().map(|e| e.get_verity().unwrap()).collect(); + assert!(verity_set.contains(&digest_a)); + assert!(verity_set.contains(&digest_b)); + + Ok(()) + } + + /// Verify that migrate_to_prefixed renames directories and rewrites + /// BLS entries correctly. + #[test] + fn test_migrate_to_prefixed() -> Result<()> { + let mut root = TestRoot::new_legacy()?; + let digest_a = root.current().verity.clone(); + + // Add a second deployment with a new kernel + root.upgrade(1, ChangeType::Kernel)?; + let digest_b = root.current().verity.clone(); + + // Before migration: unprefixed dirs + let boot_dir = root.boot_dir()?; + assert!(boot_dir.exists(&digest_a)); + assert!(boot_dir.exists(&digest_b)); + + // Perform migration + root.migrate_to_prefixed()?; + + // After migration: prefixed dirs + let boot_dir = root.boot_dir()?; + let prefixed_a = format!("{TYPE1_BOOT_DIR_PREFIX}{digest_a}"); + let prefixed_b = format!("{TYPE1_BOOT_DIR_PREFIX}{digest_b}"); + assert!( + boot_dir.exists(&prefixed_a), + "After migration, {prefixed_a} should exist" + ); + assert!( + boot_dir.exists(&prefixed_b), + "After migration, {prefixed_b} should exist" + ); + assert!( + !boot_dir.exists(&digest_a), + "After migration, unprefixed {digest_a} should be gone" + ); + assert!( + !boot_dir.exists(&digest_b), + "After migration, unprefixed {digest_b} should be gone" + ); + + // BLS entries should now reference the prefixed paths + let entries = root.read_bls_entries()?; + assert_eq!(entries.len(), 2); + for entry in &entries { + let artifact = entry.boot_artifact_name()?; + assert!( + artifact == digest_a || artifact == digest_b, + "boot_artifact_name should strip the prefix and return the digest" + ); + } + + Ok(()) + } + + /// Verify that boot_artifact_info() returns has_prefix=false for legacy + /// entries, which is the signal the migration code uses to decide what + /// needs renaming. After migration, has_prefix should be true. + #[test] + fn test_boot_artifact_info_prefix_detection() -> Result<()> { + let mut root = TestRoot::new_legacy()?; + let digest_a = root.current().verity.clone(); + root.upgrade(1, ChangeType::Kernel)?; + + // Legacy entries: boot_artifact_info should report has_prefix=false + let entries = root.read_bls_entries()?; + for entry in &entries { + let (digest, has_prefix) = entry.boot_artifact_info()?; + assert!( + !has_prefix, + "Legacy entry for {digest} should have has_prefix=false" + ); + } + + // Migrate to prefixed + root.migrate_to_prefixed()?; + + // Current entries: boot_artifact_info should report has_prefix=true + let entries = root.read_bls_entries()?; + for entry in &entries { + let (digest, has_prefix) = entry.boot_artifact_info()?; + assert!( + has_prefix, + "Migrated entry for {digest} should have has_prefix=true" + ); + } + + // boot_artifact_name() should return the same digest in both cases + let migrated_digests: std::collections::HashSet<&str> = entries + .iter() + .map(|e| e.boot_artifact_name().unwrap()) + .collect(); + assert!(migrated_digests.contains(digest_a.as_str())); + + Ok(()) + } + + /// Verify that boot_artifact_info() works correctly in a shared-kernel + /// scenario with legacy layout. Both entries should report has_prefix=false + /// and the same boot_artifact_name (the shared directory). + #[test] + fn test_boot_artifact_info_shared_kernel_legacy() -> Result<()> { + let mut root = TestRoot::new_legacy()?; + let digest_a = root.current().verity.clone(); + + root.upgrade(1, ChangeType::Userspace)?; + + let entries = root.read_bls_entries()?; + assert_eq!(entries.len(), 2); + + for entry in &entries { + let (digest, has_prefix) = entry.boot_artifact_info()?; + assert!(!has_prefix, "Legacy shared entry should have no prefix"); + assert_eq!(digest, digest_a, "Both should share A's boot dir"); + } + + Ok(()) + } +}