diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index 04624243b..29ae212ba 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -36,6 +36,7 @@ use crate::composefs_consts::{ use crate::spec::Bootloader; /// A parsed composefs command line +#[derive(Clone)] pub(crate) struct ComposefsCmdline { #[allow(dead_code)] pub insecure: bool, diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 9331db466..ad2989066 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -721,8 +721,11 @@ pub(crate) fn ensure_self_unshared_mount_namespace() -> Result<()> { /// This prepares the process for write operations (re-exec, mount namespace, etc). #[context("Initializing storage")] pub(crate) async fn get_storage() -> Result { + let env = crate::store::Environment::detect()?; + // Always call prepare_for_write() for write operations - it checks + // for container, root privileges, mount namespace setup, etc. prepare_for_write()?; - let r = BootedStorage::new() + let r = BootedStorage::new(env) .await? .ok_or_else(|| anyhow!("System not booted via bootc"))?; Ok(r) diff --git a/crates/lib/src/status.rs b/crates/lib/src/status.rs index 55e7146b3..e8b4192a2 100644 --- a/crates/lib/src/status.rs +++ b/crates/lib/src/status.rs @@ -369,7 +369,12 @@ pub(crate) fn get_status( } async fn get_host() -> Result { - let Some(storage) = BootedStorage::new().await? else { + let env = crate::store::Environment::detect()?; + if env.needs_mount_namespace() { + crate::cli::prepare_for_write()?; + } + + let Some(storage) = BootedStorage::new(env).await? else { // If we're not booted, then return a default. return Ok(Host::default()); }; diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index 38a91f585..c0ea340a5 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -97,6 +97,43 @@ pub struct BootedComposefs { /// Discriminated union representing the boot storage backend. /// +/// The runtime environment in which bootc is executing. +pub(crate) enum Environment { + /// System booted via ostree + OstreeBooted, + /// System booted via composefs + ComposefsBooted(ComposefsCmdline), + /// Running in a container + Container, + /// Other (not booted via bootc) + Other, +} + +impl Environment { + /// Detect the current runtime environment. + pub(crate) fn detect() -> Result { + if ostree_ext::container_utils::running_in_container() { + return Ok(Self::Container); + } + + if let Some(cmdline) = composefs_booted()? { + return Ok(Self::ComposefsBooted(cmdline.clone())); + } + + if ostree_booted()? { + return Ok(Self::OstreeBooted); + } + + Ok(Self::Other) + } + + /// Returns true if this environment requires entering a mount namespace + /// before loading storage (to avoid leaving /sysroot writable). + pub(crate) fn needs_mount_namespace(&self) -> bool { + matches!(self, Self::OstreeBooted | Self::ComposefsBooted(_)) + } +} + /// A system can boot via either ostree or composefs; this enum /// allows code to handle both cases while maintaining type safety. pub(crate) enum BootedStorageKind<'a> { @@ -105,54 +142,58 @@ pub(crate) enum BootedStorageKind<'a> { } impl BootedStorage { - /// Create a new booted storage accessor. - /// - /// This detects whether the system is booted via composefs or ostree - /// and initializes the appropriate storage backend. + /// Create a new booted storage accessor for the given environment. /// - /// Note: For write operations, callers should call `prepare_for_write()` - /// before calling this function to ensure the process is in the correct - /// mount namespace. - pub(crate) async fn new() -> Result> { + /// The caller must have already called `prepare_for_write()` if + /// `env.needs_mount_namespace()` is true. + pub(crate) async fn new(env: Environment) -> Result> { let physical_root = Dir::open_ambient_dir("/sysroot", cap_std::ambient_authority()) .context("Opening /sysroot")?; let run = Dir::open_ambient_dir("/run", cap_std::ambient_authority()).context("Opening /run")?; - let r = if let Some(cmdline) = composefs_booted()? { - let mut composefs = ComposefsRepository::open_path(&physical_root, COMPOSEFS)?; - if cmdline.insecure { - composefs.set_insecure(true); + let r = match env { + Environment::ComposefsBooted(cmdline) => { + let mut composefs = ComposefsRepository::open_path(&physical_root, COMPOSEFS)?; + if cmdline.insecure { + composefs.set_insecure(true); + } + let composefs = Arc::new(composefs); + + let storage = Storage { + physical_root, + run, + ostree: Default::default(), + composefs: OnceCell::from(composefs), + imgstore: Default::default(), + }; + + Some(Self { storage }) } - let composefs = Arc::new(composefs); - - let storage = Storage { - physical_root, - run, - ostree: Default::default(), - composefs: OnceCell::from(composefs), - imgstore: Default::default(), - }; - - Some(Self { storage }) - } else if ostree_booted()? { - let sysroot = ostree::Sysroot::new_default(); - sysroot.set_mount_namespace_in_use(); - let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?; - sysroot.load(gio::Cancellable::NONE)?; - - let storage = Storage { - physical_root, - run, - ostree: OnceCell::from(sysroot), - composefs: Default::default(), - imgstore: Default::default(), - }; - - Some(Self { storage }) - } else { - None + Environment::OstreeBooted => { + // The caller must have entered a private mount namespace before + // calling this function. This is because ostree's sysroot.load() will + // remount /sysroot as writable, and we call set_mount_namespace_in_use() + // to indicate we're in a mount namespace. Without actually being in a + // mount namespace, this would leave the global /sysroot writable. + + let sysroot = ostree::Sysroot::new_default(); + sysroot.set_mount_namespace_in_use(); + let sysroot = ostree_ext::sysroot::SysrootLock::new_from_sysroot(&sysroot).await?; + sysroot.load(gio::Cancellable::NONE)?; + + let storage = Storage { + physical_root, + run, + ostree: OnceCell::from(sysroot), + composefs: Default::default(), + imgstore: Default::default(), + }; + + Some(Self { storage }) + } + Environment::Container | Environment::Other => None, }; Ok(r) } diff --git a/tmt/tests/booted/readonly/001-test-status.nu b/tmt/tests/booted/readonly/001-test-status.nu index cabb4b77d..a4a119ae7 100644 --- a/tmt/tests/booted/readonly/001-test-status.nu +++ b/tmt/tests/booted/readonly/001-test-status.nu @@ -3,6 +3,22 @@ use tap.nu tap begin "verify bootc status output formats" +let st = bootc status --json | from json +# Detect composefs by checking if composefs field is present +let is_composefs = ($st.status.booted.composefs? != null) + +# FIXME: Should be mounting /sysroot readonly in composefs by default +if not $is_composefs { + # Verify /sysroot is not writable initially (read-only operations should not make it writable) + let is_writable = (do -i { /bin/test -w /sysroot } | complete | get exit_code) == 0 + assert (not $is_writable) "/sysroot should not be writable initially" + + # Double-check with findmnt + let mnt = (findmnt /sysroot -J | from json) + let opts = ($mnt.filesystems.0.options | split row ",") + assert ($opts | any { |o| $o == "ro" }) "/sysroot should be mounted read-only" +} + let st = bootc status --json | from json assert equal $st.apiVersion org.containers.bootc/v1 @@ -11,8 +27,6 @@ assert equal $st.apiVersion org.containers.bootc/v1 let st = bootc status --format=yaml | from yaml assert equal $st.apiVersion org.containers.bootc/v1 -# Detect composefs by checking if composefs field is present -let is_composefs = ($st.status.booted.composefs? != null) if not $is_composefs { assert ($st.status.booted.image.timestamp != null) } # else { TODO composefs: timestamp is not populated with composefs } @@ -23,12 +37,17 @@ if $ostree != null { let st = bootc status --json --booted | from json assert equal $st.apiVersion org.containers.bootc/v1 -# Detect composefs by checking if composefs field is present -let is_composefs = ($st.status.booted.composefs? != null) if not $is_composefs { assert ($st.status.booted.image.timestamp != null) } # else { TODO composefs: timestamp is not populated with composefs } assert (($st.status | get rollback | default null) == null) assert (($st.status | get staged | default null) == null) +# FIXME: See above re /sysroot ro +if not $is_composefs { + # Verify /sysroot is still not writable after bootc status (regression test for PR #1718) + let is_writable = (do -i { /bin/test -w /sysroot } | complete | get exit_code) == 0 + assert (not $is_writable) "/sysroot should remain read-only after bootc status" +} + tap ok