From 8b33259927afc56a518e80a3c7615a37da0911bf Mon Sep 17 00:00:00 2001 From: John Eckersberg Date: Thu, 12 Feb 2026 10:51:56 -0500 Subject: [PATCH] store: Centralize composefs directory creation with mode 0700 The install-time composefs directory creation in repo.rs used create_dir_all() which relies on the process umask for permissions, potentially creating /sysroot/composefs with overly permissive modes and leaking information. Centralize the directory creation into a new ensure_composefs_dir() helper in store/mod.rs that explicitly sets mode 0700. Both the install-time path (repo.rs) and the runtime lazy-init path (Storage::get_ensure_composefs) now use this single helper. The helper also always updates permissions on existing directories, so systems installed with an older version of bootc will have their composefs directory permissions corrected on upgrade. Also removes #[allow(dead_code)] from COMPOSEFS_MODE since it is now actively used, and adds unit tests verifying the directory permissions, idempotency, and correction of pre-existing wrong permissions. Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: John Eckersberg --- crates/lib/src/bootc_composefs/repo.rs | 4 +- crates/lib/src/store/mod.rs | 92 ++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 9 deletions(-) diff --git a/crates/lib/src/bootc_composefs/repo.rs b/crates/lib/src/bootc_composefs/repo.rs index 0c497fcdc..7f7ce9777 100644 --- a/crates/lib/src/bootc_composefs/repo.rs +++ b/crates/lib/src/bootc_composefs/repo.rs @@ -26,9 +26,7 @@ pub(crate) async fn initialize_composefs_repository( ) -> Result<(String, impl FsVerityHashValue)> { let rootfs_dir = &root_setup.physical_root; - rootfs_dir - .create_dir_all("composefs") - .context("Creating dir composefs")?; + crate::store::ensure_composefs_dir(rootfs_dir)?; let repo = open_composefs_repo(rootfs_dir)?; diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index 260fe96b5..7a516bae4 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -24,7 +24,9 @@ use anyhow::{Context, Result}; use bootc_mount::tempmount::TempMount; use camino::Utf8PathBuf; use cap_std_ext::cap_std; -use cap_std_ext::cap_std::fs::{Dir, DirBuilder, DirBuilderExt as _}; +use cap_std_ext::cap_std::fs::{ + Dir, DirBuilder, DirBuilderExt as _, Permissions, PermissionsExt as _, +}; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; @@ -51,8 +53,29 @@ pub const SYSROOT: &str = "sysroot"; /// The toplevel composefs directory path pub const COMPOSEFS: &str = "composefs"; -#[allow(dead_code)] -pub const COMPOSEFS_MODE: Mode = Mode::from_raw_mode(0o700); + +/// The mode for the composefs directory; this is intentionally restrictive +/// to avoid leaking information. +pub(crate) const COMPOSEFS_MODE: Mode = Mode::from_raw_mode(0o700); + +/// Ensure the composefs directory exists in the given physical root +/// with the correct permissions (mode 0700). +pub(crate) fn ensure_composefs_dir(physical_root: &Dir) -> Result<()> { + let mut db = DirBuilder::new(); + db.mode(COMPOSEFS_MODE.as_raw_mode()); + physical_root + .ensure_dir_with(COMPOSEFS, &db) + .context("Creating composefs directory")?; + // Always update permissions, in case the directory pre-existed + // with incorrect mode (e.g. from an older version of bootc). + physical_root + .set_permissions( + COMPOSEFS, + Permissions::from_mode(COMPOSEFS_MODE.as_raw_mode()), + ) + .context("Setting composefs directory permissions")?; + Ok(()) +} /// The path to the bootc root directory, relative to the physical /// system root @@ -399,9 +422,7 @@ impl Storage { return Ok(Arc::clone(composefs)); } - let mut db = DirBuilder::new(); - db.mode(COMPOSEFS_MODE.as_raw_mode()); - self.physical_root.ensure_dir_with(COMPOSEFS, &db)?; + ensure_composefs_dir(&self.physical_root)?; // Bootstrap verity off of the ostree state. In practice this means disabled by // default right now. @@ -430,3 +451,62 @@ impl Storage { .context("update_timestamps") } } + +#[cfg(test)] +mod tests { + use super::*; + + /// The raw mode returned by metadata includes file type bits (S_IFDIR, + /// etc.) in addition to permission bits. This constant masks to only + /// the permission bits (owner/group/other rwx). + const PERMS: Mode = Mode::from_raw_mode(0o777); + + #[test] + fn test_ensure_composefs_dir_mode() -> Result<()> { + use cap_std_ext::cap_primitives::fs::PermissionsExt as _; + + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + + let assert_mode = || -> Result<()> { + let perms = td.metadata(COMPOSEFS)?.permissions(); + let mode = Mode::from_raw_mode(perms.mode()); + assert_eq!(mode & PERMS, COMPOSEFS_MODE); + Ok(()) + }; + + ensure_composefs_dir(&td)?; + assert_mode()?; + + // Calling again should be a no-op (ensure is idempotent) + ensure_composefs_dir(&td)?; + assert_mode()?; + + Ok(()) + } + + #[test] + fn test_ensure_composefs_dir_fixes_existing() -> Result<()> { + use cap_std_ext::cap_primitives::fs::PermissionsExt as _; + + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + + // Create with overly permissive mode (simulating old bootc behavior) + let mut db = DirBuilder::new(); + db.mode(0o755); + td.create_dir_with(COMPOSEFS, &db)?; + + // Verify it starts with wrong permissions + let perms = td.metadata(COMPOSEFS)?.permissions(); + let mode = Mode::from_raw_mode(perms.mode()); + assert_eq!(mode & PERMS, Mode::from_raw_mode(0o755)); + + // ensure_composefs_dir should fix the permissions + ensure_composefs_dir(&td)?; + + let perms = td.metadata(COMPOSEFS)?.permissions(); + let mode = Mode::from_raw_mode(perms.mode()); + assert_eq!(mode & PERMS, COMPOSEFS_MODE); + + Ok(()) + } +}