store: Preserve /sysroot readonly for read-only operations

PR #1718 introduced a regression where /sysroot was left writable after
running `bootc status`. This occurred because BootedStorage::new()
unconditionally calls set_mount_namespace_in_use(), which tells ostree
it can safely remount /sysroot as writable. When sysroot.load() is called
without actually being in a mount namespace, it leaves the global /sysroot
writable.

Fix by introducing an Environment enum that detects the runtime environment
(ostree, composefs, container, or other) early in the execution flow. Callers
now detect the environment and call prepare_for_write() if needed before
constructing BootedStorage. This ensures a single call to prepare_for_write()
per execution path and eliminates the previous layering violation where storage
code called into CLI code.

The Environment abstraction also makes it clearer when mount namespace
setup is required and provides a foundation for future environment-specific
behavior.

Assisted-by: Claude Code (Sonnet 4.5)
Signed-off-by: Colin Walters <[email protected]>

Author: cgwalters

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