ostree-ext: override root dir metadata on merge commit

Dockerfile

@@ -422,6 +422,9 @@ if test "${boot_type}" = "uki"; then
   kver=$(ls /run/kernel)
   /run/packaging/finalize-uki /run/sealed-uki/out "$kver"
 fi
+# TODO: drop once https://src.fedoraproject.org/rpms/filesystem/pull-request/44 or equivalent
+# is widely available. For now, manually ensure that /usr has the correct permissions
+chmod 0555 /usr
 EORUN
 # And finally, test our linting
 # lint: allow non-tmpfs - we want to detect leaked files in /run and /tmp

crates/ostree-ext/src/container/store.rs

@@ -158,7 +158,9 @@ use oci_spec::image::{
 };
 use ocidir::oci_spec::distribution::Reference;
 use ostree::prelude::{Cast, FileEnumeratorExt, FileExt, ToVariant};
-use ostree::{gio, glib};
+use ostree::{DirMetaParsed, gio, glib};
+use rustix::fs::{Gid, Mode, OFlags, ResolveFlags, Uid};
+use rustix::io::Errno;
 use std::collections::{BTreeMap, BTreeSet, HashMap};
 use std::fmt::Write as _;
 use std::iter::FromIterator;
@@ -642,6 +644,178 @@ fn cleanup_root(root: &Dir) -> Result<()> {
     Ok(())
 }
 
+/// Collect the dirmeta checksum for every directory in the given dir, keyed by path
+/// relative to the commit root. Traverses over child directories recursively.
+fn collect_dirmeta_checksums(
+    dir: &ostree::RepoFile,
+    path: &Utf8Path,
+    out: &mut HashMap<Utf8PathBuf, glib::GString>,
+    cancellable: Option<&gio::Cancellable>,
+) -> Result<()> {
+    dir.ensure_resolved()?;
+
+    let checksum = dir
+        .tree_get_metadata_checksum()
+        .ok_or_else(|| anyhow!("Missing metadata checksum for {path}"))?;
+    out.insert(path.to_owned(), checksum);
+
+    // Recurse over child directories, without following symlinks
+    let query_flags = gio::FileQueryInfoFlags::NOFOLLOW_SYMLINKS;
+    let files =
+        dir.enumerate_children("standard::name,standard::type", query_flags, cancellable)?;
+    while let Some(child_info) = files.next_file(cancellable)? {
+        if child_info.file_type() != gio::FileType::Directory {
+            continue;
+        }
+        let name = child_info.name();
+        let name = name.to_str().expect("UTF-8 ostree name");
+        let child = files
+            .child(&child_info)
+            .downcast::<ostree::RepoFile>()
+            .expect("downcast");
+        collect_dirmeta_checksums(&child, &path.join(name), out, cancellable)?;
+    }
+
+    Ok(())
+}
+
+/// Apply directory mode and ownership from an ostree DirMeta to a filesystem
+/// directory.
+///
+/// We intentionally skip xattrs here: the SELinux relabelling pass in
+/// `write_dfd_to_mtree` overwrites `security.selinux` on every directory,
+/// and no other directory xattrs are meaningful in practice.
+fn apply_dirmeta(
+    dir: impl rustix::fd::AsFd,
+    metadata: &ostree::DirMetaParsed,
+    should_fchown: bool,
+) -> Result<()> {
+    if should_fchown {
+        rustix::fs::fchown(
+            &dir,
+            Some(Uid::from_raw(metadata.uid)),
+            Some(Gid::from_raw(metadata.gid)),
+        )
+        .context("fchown")?;
+    }
+    rustix::fs::fchmod(&dir, Mode::from_raw_mode(metadata.mode)).context("fchmod")?;
+    Ok(())
+}
+
+/// A `UnionFiles` checkout keeps the directory metadata from the first layer
+/// that created the directory, so later layers that only change a directory's
+/// mode or ownership (e.g. `RUN chmod 0555 /usr`) are silently lost.
+/// OCI semantics (and composefs-rs) require last-layer-wins, so we reapply
+/// directory metadata after all layers have been checked out.
+///
+/// For efficiency, we first collect only the cheap dirmeta *checksums* from
+/// each layer. Only directories whose final checksum differs from the base
+/// (first) layer need to be updated, which should let us avoid having to
+/// process the vast majority of directories.
+fn apply_layer_dirmeta(
+    repo: &ostree::Repo,
+    base_commit: Option<&str>,
+    layer_commits: &[LayerRef],
+    root: &Dir,
+    should_fchown: bool,
+    cancellable: Option<&gio::Cancellable>,
+) -> Result<()> {
+    // Collect the base layer's dirmeta checksums so we can skip directories
+    // whose metadata doesn't differ from this layer
+    let base_checksums = match base_commit {
+        Some(base) => {
+            let mut base_map = HashMap::new();
+            let (base_root, _) = repo.read_commit(base, cancellable)?;
+            let base_root = base_root.downcast::<ostree::RepoFile>().unwrap();
+            collect_dirmeta_checksums(&base_root, Utf8Path::new("."), &mut base_map, cancellable)
+                .context("Reading base directory metadata checksums")?;
+            base_map
+        }
+        None => HashMap::new(),
+    };
+
+    // Walk the derived layers in order, where the last writer for a given path
+    // wins via HashMap::insert.
+    let mut final_checksums: HashMap<Utf8PathBuf, glib::GString> = HashMap::new();
+    for lc in layer_commits {
+        let (commit_root, _) = repo.read_commit(&lc.commit, cancellable)?;
+        let commit_root = commit_root
+            .downcast::<ostree::RepoFile>()
+            .expect("downcast");
+        collect_dirmeta_checksums(
+            &commit_root,
+            Utf8Path::new("."),
+            &mut final_checksums,
+            cancellable,
+        )
+        .with_context(|| format!("Reading directory metadata checksums of {}", lc.commit))?;
+    }
+
+    // Only parse + apply directories whose checksum differs from the base.
+    for (path, checksum) in &final_checksums {
+        if base_checksums
+            .get(path)
+            .is_some_and(|s| s.as_str() == checksum.as_str())
+        {
+            tracing::debug!("Skipping {path}, metadata checksum is the same as the base layer");
+            continue;
+        }
+        tracing::debug!("Applying directory metadata for {path}");
+
+        let variant = repo
+            .load_variant(ostree::ObjectType::DirMeta, checksum)
+            .with_context(|| format!("Loading dirmeta {checksum} for {path}"))?;
+        let parsed = ostree::DirMetaParsed::from_variant(&variant)
+            .map_err(|e| anyhow!("Parsing dirmeta for {path}: {e}"))?;
+
+        let flags = OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC;
+        let resolve = ResolveFlags::BENEATH | ResolveFlags::NO_SYMLINKS;
+        let fd = match rustix::fs::openat2(root, path.as_std_path(), flags, Mode::empty(), resolve)
+        {
+            Ok(fd) => fd,
+            // The directory was removed by a later layer, or it / a parent
+            // was replaced by a non-directory.
+            Err(Errno::NOENT | Errno::NOTDIR | Errno::LOOP) => {
+                continue;
+            }
+            Err(e) => return Err(e).with_context(|| format!("Getting FD for {path}")),
+        };
+        apply_dirmeta(fd, &parsed, should_fchown)
+            .with_context(|| format!("Updating metadata for {path}"))?;
+    }
+    Ok(())
+}
+
+/// Copy the `/usr` directory mode and ownership to the root, matching
+/// composefs-rs, since OCI images don't reliably include root dir metadata.
+/// This should be called before the merged tree is committed, so the root
+/// still gets its own SELinux label.
+///
+/// We intentionally don't copy xattrs here: the SELinux relabelling pass
+/// in `write_dfd_to_mtree` will assign the correct `root_t` label to `/`.
+///
+/// See:
+///   - <https://github.com/coreos/fedora-coreos-tracker/issues/2238>
+///   - <https://github.com/coreos/fedora-coreos-tracker/issues/2238#issuecomment-6082074286>
+///   - <https://github.com/bootc-dev/bootc/pull/2571>
+fn normalize_root_dirmeta(root: &Dir, should_fchown: bool) -> Result<()> {
+    let usr = root.symlink_metadata("usr").context("looking up /usr")?;
+    if !usr.is_dir() {
+        anyhow::bail!("/usr is not a directory");
+    }
+    // `root` may be an `O_PATH` fd, on which fchmod/fchown fail with EBADF.
+    let flags = OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC;
+    let rootfd = rustix::fs::openat(root, ".", flags, Mode::empty()).context("opening root")?;
+
+    let metadata = DirMetaParsed {
+        uid: usr.uid(),
+        gid: usr.gid(),
+        mode: usr.mode(),
+        xattrs: vec![],
+    };
+    apply_dirmeta(rootfd, &metadata, should_fchown).context("applying /usr metadata to root")
+}
+
 impl ImageImporter {
     /// The metadata key used in ostree commit metadata to serialize
     const CACHED_KEY_MANIFEST_DIGEST: &'static str = "ostree-ext.cached.manifest-digest";
@@ -1431,6 +1605,20 @@ impl ImageImporter {
 
         cleanup_root(&root_dir)?;
 
+        // If we are running as root, we should fchown to apply uid/gid
+        let should_fchown = checkout_mode == ostree::RepoCheckoutMode::None;
+        apply_layer_dirmeta(
+            repo,
+            base_commit,
+            layer_commits,
+            &root_dir,
+            should_fchown,
+            cancellable,
+        )
+        .context("applying layer directory metadata")?;
+        normalize_root_dirmeta(&root_dir, should_fchown)
+            .context("normalizing root dir metadata")?;
+
         let mt = ostree::MutableTree::new();
         repo.write_dfd_to_mtree(
             (*td).as_raw_fd(),

crates/ostree-ext/tests/it/main.rs

@@ -60,6 +60,47 @@ fn assert_err_contains<T>(r: Result<T>, s: impl AsRef<str>) {
     }
 }
 
+/// Verify that a commit's root directory mode/uid/gid matches `/usr`.
+fn assert_root_dirmeta_matches_usr(repo: &ostree::Repo, commit: &str) {
+    let root = repo.read_commit(commit, gio::Cancellable::NONE).unwrap().0;
+    let root = root.downcast::<ostree::RepoFile>().unwrap();
+    root.ensure_resolved().unwrap();
+
+    let usr = root.resolve_relative_path("usr");
+    let usr = usr.downcast::<ostree::RepoFile>().unwrap();
+    usr.ensure_resolved().unwrap();
+
+    let queryflags = gio::FileQueryInfoFlags::NOFOLLOW_SYMLINKS;
+    let attrs = "unix::mode,unix::uid,unix::gid";
+    let root_info = root
+        .query_info(attrs, queryflags, gio::Cancellable::NONE)
+        .unwrap();
+    let usr_info = usr
+        .query_info(attrs, queryflags, gio::Cancellable::NONE)
+        .unwrap();
+
+    let root_mode = root_info.attribute_uint32("unix::mode");
+    let usr_mode = usr_info.attribute_uint32("unix::mode");
+    assert_eq!(
+        root_mode, usr_mode,
+        "root mode '{root_mode:#o}' should match /usr mode '{usr_mode:#o}'"
+    );
+
+    let root_uid = root_info.attribute_uint32("unix::uid");
+    let usr_uid = usr_info.attribute_uint32("unix::uid");
+    assert_eq!(
+        root_uid, usr_uid,
+        "root uid '{root_uid}' should match /usr uid '{usr_uid}'"
+    );
+
+    let root_gid = root_info.attribute_uint32("unix::gid");
+    let usr_gid = usr_info.attribute_uint32("unix::gid");
+    assert_eq!(
+        root_gid, usr_gid,
+        "root gid '{root_gid}' should match /usr gid '{usr_gid}'"
+    );
+}
+
 static TEST_REGISTRY: LazyLock<String> =
     LazyLock::new(|| match std::env::var_os("TEST_REGISTRY") {
         Some(t) => t.to_str().unwrap().to_owned(),
@@ -709,6 +750,7 @@ async fn test_export_as_container_nonderived() -> Result<()> {
     let reimported = fixture.must_import(&dest).await?;
     let reimport_ls = fixture::ostree_ls(fixture.destrepo(), &reimported.merge_commit).unwrap();
     similar_asserts::assert_eq!(initimport_ls, reimport_ls);
+    assert_root_dirmeta_matches_usr(fixture.destrepo(), &reimported.merge_commit);
     Ok(())
 }
 
@@ -1024,6 +1066,69 @@ async fn test_export_etc_remapping() -> Result<()> {
     Ok(())
 }
 
+/// Verify that a derived layer that changes `/usr` mode (e.g. `RUN chmod 0555 /usr`)
+/// is preserved through the merge commit.
+#[tokio::test]
+async fn test_derived_layer_usr_mode() -> Result<()> {
+    if !check_skopeo() {
+        return Ok(());
+    }
+    let fixture = Fixture::new_v1()?;
+    let src_imgref = fixture.export_container().await.unwrap().0;
+
+    // Build a derived image with a layer that changes /usr to mode 0555.
+    let derived_path = &fixture.path.join("derived-usr-mode.oci");
+    oci_clone(src_imgref.name.as_str(), derived_path)
+        .await
+        .unwrap();
+    ostree_ext::integrationtest::generate_derived_oci_from_tar(
+        derived_path,
+        |w| {
+            let mut layer_tar = tar::Builder::new(w);
+            let mut h = tar::Header::new_gnu();
+            h.set_uid(0);
+            h.set_gid(0);
+            h.set_size(0);
+            h.set_mode(0o555);
+            h.set_entry_type(tar::EntryType::Directory);
+            layer_tar.append_data(&mut h, "usr", &mut std::io::empty())?;
+            layer_tar.finish()?;
+            Ok(())
+        },
+        None,
+        None,
+    )?;
+
+    let derived_imgref = ImageReference {
+        transport: Transport::OciDir,
+        name: derived_path.to_string(),
+    };
+    let imp = fixture.must_import(&derived_imgref).await?;
+
+    // Verify /usr has mode 0555 in the merge commit.
+    let root = fixture
+        .destrepo()
+        .read_commit(&imp.merge_commit, gio::Cancellable::NONE)?
+        .0;
+    let usr = root.resolve_relative_path("usr");
+    let usr = usr.downcast::<ostree::RepoFile>().unwrap();
+    usr.ensure_resolved().unwrap();
+
+    let info = usr.query_info(
+        "unix::mode",
+        gio::FileQueryInfoFlags::NOFOLLOW_SYMLINKS,
+        gio::Cancellable::NONE,
+    )?;
+    let mode = info.attribute_uint32("unix::mode") & 0o7777; // mask to get only permissions bits
+    assert_eq!(
+        mode, 0o555,
+        "/usr mode should be 0555 after derived layer, got '{mode:#o}'"
+    );
+
+    assert_root_dirmeta_matches_usr(fixture.destrepo(), &imp.merge_commit);
+    Ok(())
+}
+
 #[tokio::test]
 async fn test_unencapsulate_unbootable() -> Result<()> {
     if !check_skopeo() {
@@ -1653,6 +1758,7 @@ async fn test_non_ostree() -> Result<()> {
         similar_asserts::assert_eq!(&src_manifest, &dest_manifest);
     }
     assert_eq!(imp.manifest_digest, src_digest);
+    assert_root_dirmeta_matches_usr(fixture.destrepo(), &imp.merge_commit);
     Ok(())
 }
 

tmt/tests/booted/readonly/040-test-baseconfigs.nu

@@ -21,9 +21,8 @@ for config in $configs {
             let fstype = $mnt.filesystems.0.fstype
             assert equal $fstype "overlay" "/etc should be mounted as overlay when etc-transient is active"
 
-            print "# Checking / has 755 permissions (overlay upper dir must not block traversal)"
-            let perms = (stat -c "%a" / | str trim)
-            assert equal $perms "755" "/ should have 755 permissions"
+            # root permission checks done in 055-test-root-dirmeta
+            # (overlay upper dir must not block traversal)
 
             print "# Checking /etc is writable (transient overlay)"
             let result = (do { ^touch /etc/bootc-baseconfig-test } | complete)
@@ -36,9 +35,7 @@ for config in $configs {
             let fstype = $mnt.filesystems.0.fstype
             assert equal $fstype "overlay" "/ should be mounted as overlay when root-transient is active"
 
-            print "# Checking / has 755 permissions"
-            let perms = (stat -c "%a" / | str trim)
-            assert equal $perms "755" "/ should have 755 permissions"
+            # root permission checks done in 055-test-root-dirmeta
 
             print "# Checking / is writable (transient overlay)"
             let result = (do { ^touch /bootc-baseconfig-root-test } | complete)

tmt/tests/booted/readonly/055-test-root-dirmeta.nu

@@ -0,0 +1,18 @@
+# Verify that the root dir's uid, gid, and permissions match /usr.
+#
+# See: https://github.com/coreos/fedora-coreos-tracker/issues/2238
+use std assert
+use tap.nu
+
+tap begin "verify root dir metadata matches /usr"
+
+let root_meta = (stat -c "mode: %a, gid: %g, uid: %u" / | str trim)
+let usr_meta = (stat -c "mode: %a, gid: %g, uid: %u" /usr | str trim)
+
+print $"# root: ($root_meta)"
+print $"# /usr: ($usr_meta)"
+
+assert equal $root_meta $usr_meta $"root dir metadata \(($root_meta)\) should match /usr \(($usr_meta)\)"
+assert equal $root_meta "mode: 555, gid: 0, uid: 0" $"root dir metadata \(($root_meta)\) should match expected \(mode: 555, gid: 0, uid: 0\)"
+
+tap ok