store: Preserve /sysroot readonly for read-only operations

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,

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<crate::store::BootedStorage> {
+    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)

crates/lib/src/status.rs

@@ -369,7 +369,12 @@ pub(crate) fn get_status(
 }
 
 async fn get_host() -> Result<Host> {
-    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());
     };

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<Self> {
+        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<Option<Self>> {
+    /// The caller must have already called `prepare_for_write()` if
+    /// `env.needs_mount_namespace()` is true.
+    pub(crate) async fn new(env: Environment) -> Result<Option<Self>> {
         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)
     }

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