From efdad46dc3589a46c9be68a32cab965a03cc2d49 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 1 Oct 2026 12:19:06 +0530 Subject: [PATCH 1/4] grubcfg: Handle symlink that lead outside of `grub2` We have cases where `grub2/grub.cfg -> ../loader/grub.cfg` and since we moved to cap-std APIs, the metadata gathering for the file fails since the target leads to outside the filesystem (which is the directory `grub2` in this case). To fix, use `std::fs` APIs to get metadata Use `GRUBCONFIG_FILE_MODE` constant instead of raw 0600 mode Signed-off-by: Pragyan Poudyal --- src/aleph.rs | 2 ++ src/grubconfigs.rs | 50 +++++++++++++++++++++++++++++++++++++--------- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/aleph.rs b/src/aleph.rs index 783260a7..3ef8ffd4 100644 --- a/src/aleph.rs +++ b/src/aleph.rs @@ -12,6 +12,7 @@ use anyhow::{Context, Result}; use chrono::prelude::*; +use fn_error_context::context; use serde::{Deserialize, Serialize}; use std::fs::File; use std::path::Path; @@ -36,6 +37,7 @@ const ALEPH_PATHS: &[&str] = &[ "sysroot/.bootc-aleph.json", ]; +#[context("Getting aleph version")] pub(crate) fn get_aleph_version(root: &Path) -> Result> { for aleph_path in ALEPH_PATHS { let path = &root.join(aleph_path); diff --git a/src/grubconfigs.rs b/src/grubconfigs.rs index f06180e8..f76a2e4a 100644 --- a/src/grubconfigs.rs +++ b/src/grubconfigs.rs @@ -174,12 +174,19 @@ fn write_grubenv(grubdir: &Dir) -> Result<()> { .run_inherited_with_cmd_context() } -#[context("Ensure file permissions are 0600")] +#[context("Ensure file permissions are {GRUBCONFIG_FILE_MODE:04o}")] fn ensure_file_permissions(target_dir: &Dir, target_file: &str) -> Result<()> { - let metadata = match target_dir.metadata(target_file) { + use std::os::unix::fs::PermissionsExt; + + // grub.cfg may be a symlink pointing outside this Dir cap_std refuses to traverse + // such symlinks via openat, so resolve through /proc/self/fd + let absolute_path = format!("/proc/self/fd/{}/{target_file}", target_dir.as_raw_fd()); + + let metadata = match std::fs::metadata(&absolute_path) { Ok(metadata) => metadata, Err(e) if e.kind() == std::io::ErrorKind::NotFound => { // Silently return if file doesn't exist + log::debug!("File {target_file} not found"); return Ok(()); } Err(e) => return Err(e).context(format!("Reading {} metadata", target_file)), @@ -187,14 +194,21 @@ fn ensure_file_permissions(target_dir: &Dir, target_file: &str) -> Result<()> { let mode = metadata.permissions().mode() & 0o777; if mode != GRUBCONFIG_FILE_MODE { - target_dir - .set_permissions(target_file, Permissions::from_mode(GRUBCONFIG_FILE_MODE)) - .with_context(|| format!("Setting {} permissions to 0600", target_file))?; + std::fs::set_permissions( + absolute_path, + std::fs::Permissions::from_mode(GRUBCONFIG_FILE_MODE), + ) + .with_context(|| { + format!( + "Setting {} permissions to {GRUBCONFIG_FILE_MODE:04o}", + target_file + ) + })?; } Ok(()) } -#[context("Ensure grub files permissions are 0600")] +#[context("Ensure grub files permissions are {GRUBCONFIG_FILE_MODE:04o}")] pub(crate) fn ensure_grub_permissions(grub_dir: &Dir) -> Result<()> { for file_name in GRUB_FILES.iter() { ensure_file_permissions(&grub_dir, file_name) @@ -251,7 +265,7 @@ mod tests { { let metadata = td.metadata("grubenv")?; let mode = metadata.permissions().mode() & 0o777; - assert_eq!(mode, 0o600); + assert_eq!(mode, GRUBCONFIG_FILE_MODE); } Ok(()) } @@ -285,11 +299,29 @@ mod tests { // Verify grubenv was fixed to 0o600 let metadata = grub_dir.metadata("grubenv")?; - assert_eq!(metadata.permissions().mode() & 0o777, 0o600); + assert_eq!(metadata.permissions().mode() & 0o777, GRUBCONFIG_FILE_MODE); // Verify grub.cfg was fixed 0o600 let metadata = grub_dir.metadata("grub.cfg")?; - assert_eq!(metadata.permissions().mode() & 0o777, 0o600); + assert_eq!(metadata.permissions().mode() & 0o777, GRUBCONFIG_FILE_MODE); + + use std::os::unix::fs::PermissionsExt; + + // Test it with symlinks pointing outside the grub2 dir + // Create the symlink target outside the grub2 dir, with wrong perms + let external_target = tdp.join("external-grub.cfg"); + std::fs::write(&external_target, "external grub.cfg content")?; + std::fs::set_permissions(&external_target, std::fs::Permissions::from_mode(0o644))?; + + grub_dir.remove_file("grub.cfg")?; + std::os::unix::fs::symlink(&external_target, grub.join("grub.cfg"))?; + + ensure_grub_permissions(&grub_dir)?; + + // Verify grub.cfg was fixed 0o600 + let metadata = std::fs::metadata(&external_target)?; + assert_eq!(metadata.permissions().mode() & 0o777, GRUBCONFIG_FILE_MODE); + Ok(()) } } From 6969be576cc518159343d05f2a415d5c9f115fb3 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 1 Oct 2026 14:18:27 +0530 Subject: [PATCH 2/4] ci: Skip composefs tests for C10S bootc 1.16.14 (in copr) uses composefs.digest= cmdline, but since we don't rebuild the initramfs, we never include that digest as a condition for bootc-initramfs-setup to run which fails the VM startup Disable C10S tests for composefs, for now Signed-off-by: Pragyan Poudyal --- .github/workflows/ci.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a9c3ab2f..b5be61c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -161,4 +161,10 @@ jobs: run: | set -xeuo pipefail sudo ./scripts/test-uefi-vm-boot.sh "$IMG_NAME" "ostree" - sudo ./scripts/test-uefi-vm-boot.sh "$IMG_NAME" "composefs" + + # TODO: Skip composefs tests for C10S as bootc 1.16.3 uses composefs.digest= + # cmdline, but for some reason that cmdline is not mentioned in + # bootc-root-setup.service + if [[ "${{ matrix.stream }}" != "10" ]]; then + sudo ./scripts/test-uefi-vm-boot.sh "$IMG_NAME" "composefs" + fi From 570d12e850f82322b1f3be5b3e26ea26ecfefaea Mon Sep 17 00:00:00 2001 From: cgwalters-bot Date: Tue, 22 Sep 2026 22:27:29 -0400 Subject: [PATCH 3/4] cli: Reuse DVerb for `bootupctl backend` `bootupctl backend` had its own copy of the `bootupd` verbs that had to be kept in sync by hand, and its variants were each individually hidden. Share the one enum so both entrypoints get the same verbs and help text. Prep for un-hiding `backend`. Generated-by: AI --- src/cli/bootupctl.rs | 24 ++---------------------- src/cli/bootupd.rs | 19 +++++++++++++------ 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/src/cli/bootupctl.rs b/src/cli/bootupctl.rs index 7f2d7507..2fc7ebf5 100644 --- a/src/cli/bootupctl.rs +++ b/src/cli/bootupctl.rs @@ -52,7 +52,7 @@ pub enum CtlVerb { // FIXME(lucab): drop this after refreshing // https://github.com/coreos/fedora-coreos-config/pull/595 #[clap(name = "backend", hide = true, subcommand)] - Backend(CtlBackend), + Backend(super::bootupd::DVerb), #[clap(name = "status", about = "Show components status")] Status(StatusOpts), #[clap(name = "update", about = "Update all components")] @@ -69,17 +69,6 @@ pub enum CtlVerb { MigrateStaticGrubConfig, } -#[derive(Debug, Parser)] -pub enum CtlBackend { - #[clap(name = "generate-update-metadata", hide = true)] - Generate(super::bootupd::GenerateOpts), - #[clap(name = "install", hide = true)] - Install(super::bootupd::InstallOpts), - #[clap(hide = true)] - #[cfg(efi_arch)] - SetDefaultBootloader(super::bootupd::DefaultBootloaderOpts), -} - #[derive(Debug, Parser)] pub struct StatusOpts { /// If there are updates available, output `Updates available: ` to standard output; @@ -123,16 +112,7 @@ impl CtlCommand { CtlVerb::Update(opts) => Self::run_update(opts), CtlVerb::AdoptAndUpdate(opts) => Self::run_adopt_and_update(opts), CtlVerb::Validate => Self::run_validate(), - CtlVerb::Backend(CtlBackend::Generate(opts)) => { - super::bootupd::DCommand::run_generate_meta(opts) - } - CtlVerb::Backend(CtlBackend::Install(opts)) => { - super::bootupd::DCommand::run_install(opts) - } - #[cfg(efi_arch)] - CtlVerb::Backend(CtlBackend::SetDefaultBootloader(opts)) => { - super::bootupd::DCommand::set_default_bootloader(opts) - } + CtlVerb::Backend(verb) => verb.run(), CtlVerb::MigrateStaticGrubConfig => Self::run_migrate_static_grub_config(), } } diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index a9b8951b..f7949791 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -108,15 +108,22 @@ pub struct DefaultBootloaderOpts { pub(crate) bootloader: Bootloader, } +impl DVerb { + /// Run this verb; shared by `bootupd` and `bootupctl backend`. + pub(crate) fn run(self) -> Result<()> { + match self { + DVerb::Install(opts) => DCommand::run_install(opts), + DVerb::GenerateUpdateMetadata(opts) => DCommand::run_generate_meta(opts), + #[cfg(efi_arch)] + DVerb::SetDefaultBootloader(opts) => DCommand::set_default_bootloader(opts), + } + } +} + impl DCommand { /// Run CLI application. pub fn run(self) -> Result<()> { - match self.cmd { - DVerb::Install(opts) => Self::run_install(opts), - DVerb::GenerateUpdateMetadata(opts) => Self::run_generate_meta(opts), - #[cfg(efi_arch)] - DVerb::SetDefaultBootloader(opts) => Self::set_default_bootloader(opts), - } + self.cmd.run() } /// Runner for `generate-install-metadata` verb. From a5fa13ee31e8ada2cabbb44d9c1c6c4c3fc492d7 Mon Sep 17 00:00:00 2001 From: cgwalters-bot Date: Tue, 22 Sep 2026 22:28:13 -0400 Subject: [PATCH 4/4] cli: Un-hide `bootupctl backend` and document its verbs Hiding `backend` was meant to keep the client side less confusing, but in practice hidden commands cause more confusion than they avoid: they are used by image build tooling and referenced in our own docs, yet `--help` gives no hint they exist or what they are for. Show it, listed after the verbs for a booted system, and give it and its verbs help text that explains they operate on an OS tree or target root at build/install time, pointing users who want to update a booted system at `update` and `adopt-and-update`. Add a test so the verbs stay visible and documented. Note this reverses the intent of the deleted FIXME(lucab) comment, which planned to eventually drop `backend` from bootupctl once coreos/fedora-coreos-config#595 was refreshed. Since image build tooling still relies on it, making it a documented interface seems better, but that is a call for the maintainers. Related: #1119 Generated-by: AI --- src/cli/bootupctl.rs | 12 ++++++++---- src/cli/bootupd.rs | 17 ++++++++++++++--- src/cli/mod.rs | 17 +++++++++++++++++ 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/cli/bootupctl.rs b/src/cli/bootupctl.rs index 2fc7ebf5..f462b2c8 100644 --- a/src/cli/bootupctl.rs +++ b/src/cli/bootupctl.rs @@ -49,10 +49,6 @@ impl CtlCommand { /// CLI sub-commands. #[derive(Debug, Parser)] pub enum CtlVerb { - // FIXME(lucab): drop this after refreshing - // https://github.com/coreos/fedora-coreos-config/pull/595 - #[clap(name = "backend", hide = true, subcommand)] - Backend(super::bootupd::DVerb), #[clap(name = "status", about = "Show components status")] Status(StatusOpts), #[clap(name = "update", about = "Update all components")] @@ -61,6 +57,14 @@ pub enum CtlVerb { AdoptAndUpdate(AdoptAndUpdateOpts), #[clap(name = "validate", about = "Validate system state")] Validate, + /// Low-level commands for building and installing OS images + /// + /// These act on an OS tree or target root rather than on the running + /// system, and are intended for image build and installation tooling + /// (e.g. `bootc install` or disk image builders). To update the + /// bootloader of a booted system, use `update` or `adopt-and-update`. + #[clap(name = "backend", subcommand)] + Backend(super::bootupd::DVerb), #[clap( name = "migrate-static-grub-config", hide = true, diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index f7949791..4d57637f 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -36,10 +36,21 @@ impl DCommand { /// CLI sub-commands. #[derive(Debug, Parser)] pub enum DVerb { - #[clap(name = "generate-update-metadata", about = "Generate metadata")] + /// Generate update metadata from the bootloader files in an OS tree + /// + /// Run this at image build time. It records the bootloader component + /// versions (e.g. from the RPM database) under /usr/lib/bootupd/updates, + /// which `install` and `update` later use as their source. + #[clap(name = "generate-update-metadata")] GenerateUpdateMetadata(GenerateOpts), - #[clap(name = "install", about = "Install components")] + /// Install bootloader components to a target root + /// + /// Used when creating a disk image or installing a system: copies the + /// update payload from the source root into the target's ESP and/or + /// boot partition, and installs to the target block device(s) as needed. + #[clap(name = "install")] Install(InstallOpts), + /// Set the default bootloader on EFI systems #[cfg(efi_arch)] SetDefaultBootloader(DefaultBootloaderOpts), } @@ -94,7 +105,7 @@ pub(crate) struct InstallOpts { #[derive(Debug, Parser)] pub struct GenerateOpts { - /// Physical root mountpoint + /// Root of the OS tree (only `/` is currently supported) #[clap(value_parser)] sysroot: Option, } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 02212b56..6b505797 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -58,6 +58,23 @@ mod tests { bootupd::DCommand::command().debug_assert(); } + #[test] + fn backend_is_documented() { + use clap::CommandFactory; + let ctl = bootupctl::CtlCommand::command(); + let backend = ctl.find_subcommand("backend").unwrap(); + assert!(!backend.is_hide_set()); + assert!(backend.get_about().is_some()); + for verb in backend.get_subcommands() { + assert!(!verb.is_hide_set(), "{} is hidden", verb.get_name()); + assert!( + verb.get_about().is_some(), + "{} has no help", + verb.get_name() + ); + } + } + #[test] fn test_multicall_dispatch() { {