Skip to content

Commit 4a06016

Browse files
committed
oci: Empty /run and use /usr mtime for emptied directories
Empty /run in create_filesystem() when processing OCI images. This is a tmpfs at runtime and should always be empty in images. Emptying /run also works around podman/buildah's RUN --mount issue where cache mounts can leave incomplete directory entries in OCI tar layers, causing mtime inconsistencies. Container authors can now safely use: RUN rm -rf /var/cache/dnf && ln -sr /run/dnfcache /var/cache/dnf RUN --mount=type=cache,target=/run/dnfcache dnf install -y ... The /run directory is emptied for all OCI images, not just bootable ones, since it should always be empty regardless of boot configuration. Also change emptied directories in transform_for_boot() (/boot, /sysroot) to use /usr's mtime instead of 0, for consistency with how root directory metadata is handled via copy_root_metadata_from_usr(). Closes: #132 Assisted-by: OpenCode (Opus 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 4141129 commit 4a06016

5 files changed

Lines changed: 200 additions & 8 deletions

File tree

crates/composefs-boot/src/lib.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ pub mod selabel;
1414
pub mod uki;
1515
pub mod write_boot;
1616

17+
use std::ffi::OsStr;
18+
1719
use anyhow::Result;
1820

1921
use composefs::{fsverity::FsVerityHashValue, repository::Repository, tree::FileSystem};
@@ -67,9 +69,14 @@ impl<ObjectID: FsVerityHashValue> BootOps<ObjectID> for FileSystem<ObjectID> {
6769
repo: &Repository<ObjectID>,
6870
) -> Result<Vec<BootEntry<ObjectID>>> {
6971
let boot_entries = get_boot_resources(self, repo)?;
72+
73+
// Get /usr's mtime to use as the canonical mtime for emptied directories.
74+
// This matches how we handle the root directory in copy_root_metadata_from_usr().
75+
let usr_mtime = self.root.get_directory(OsStr::new("usr"))?.stat.st_mtim_sec;
76+
7077
for d in REQUIRED_TOPLEVEL_TO_EMPTY_DIRS {
7178
let d = self.root.get_directory_mut(d.as_ref())?;
72-
d.stat.st_mtim_sec = 0;
79+
d.stat.st_mtim_sec = usr_mtime;
7380
d.clear();
7481
}
7582

crates/composefs-oci/src/image.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,9 @@ pub fn create_filesystem<ObjectID: FsVerityHashValue>(
128128
}
129129
}
130130

131-
// Copy root metadata from /usr to ensure consistent digests across different
132-
// container runtimes and tar implementations.
133-
filesystem.copy_root_metadata_from_usr()?;
131+
// Apply OCI container transformations for consistent digests.
132+
// See https://github.com/containers/composefs-rs/issues/132
133+
filesystem.transform_for_oci()?;
134134

135135
Ok(filesystem)
136136
}

crates/composefs/src/fs.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,27 +400,29 @@ pub fn is_allowed_container_xattr(name: &OsStr) -> bool {
400400
/// Load a container root filesystem from the given path.
401401
///
402402
/// This is a convenience wrapper around [`read_filesystem_filtered`] that also
403-
/// copies metadata from `/usr` to the root directory.
403+
/// applies OCI container transformations via [`FileSystem::transform_for_oci`].
404404
///
405405
/// Equivalent to calling:
406406
/// ```ignore
407407
/// let mut fs = read_filesystem_filtered(dirfd, path, repo, is_allowed_container_xattr)?;
408-
/// fs.copy_root_metadata_from_usr()?;
408+
/// fs.transform_for_oci()?;
409409
/// ```
410410
///
411411
/// This is the recommended way to read a container filesystem because:
412412
/// - OCI container runtimes don't preserve root directory metadata from layer tars
413413
/// - Host xattrs (especially `security.selinux`) can leak into mounted filesystems
414+
/// - `/run` should be empty (it's a tmpfs at runtime)
415+
/// - Podman/buildah's `RUN --mount` can leave directory stubs
414416
///
415-
/// By filtering xattrs and copying `/usr` metadata to root, we ensure consistent
417+
/// By filtering xattrs and applying OCI transformations, we ensure consistent
416418
/// and reproducible composefs digests between build-time and install-time.
417419
pub fn read_container_root<ObjectID: FsVerityHashValue>(
418420
dirfd: impl AsFd,
419421
path: &Path,
420422
repo: Option<&Repository<ObjectID>>,
421423
) -> Result<FileSystem<ObjectID>> {
422424
let mut fs = read_filesystem_filtered(dirfd, path, repo, is_allowed_container_xattr)?;
423-
fs.copy_root_metadata_from_usr()?;
425+
fs.transform_for_oci()?;
424426
Ok(fs)
425427
}
426428

crates/composefs/src/generic_tree.rs

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,52 @@ impl<T> FileSystem<T> {
547547
stat.xattrs.borrow_mut().retain(|k, _| predicate(k));
548548
});
549549
}
550+
551+
/// Empties the `/run` directory if present, using `/usr`'s mtime.
552+
///
553+
/// `/run` is a tmpfs at runtime and should always be empty in container images.
554+
/// This also works around podman/buildah's `RUN --mount` behavior where bind
555+
/// mount targets leave directory stubs in the filesystem that shouldn't be
556+
/// part of the image content.
557+
///
558+
/// The mtime is set to match `/usr` for consistency with [`Self::copy_root_metadata_from_usr`].
559+
///
560+
/// NOTE: If changing this behavior, also update `doc/oci.md`.
561+
///
562+
/// # Errors
563+
///
564+
/// Returns an error if `/usr` does not exist (needed to get the mtime).
565+
pub fn canonicalize_run(&mut self) -> Result<(), ImageError> {
566+
if self.root.get_directory_opt(OsStr::new("run"))?.is_some() {
567+
let usr_mtime = self.root.get_directory(OsStr::new("usr"))?.stat.st_mtim_sec;
568+
let run_dir = self.root.get_directory_mut(OsStr::new("run"))?;
569+
run_dir.stat.st_mtim_sec = usr_mtime;
570+
run_dir.clear();
571+
}
572+
Ok(())
573+
}
574+
575+
/// Transforms the filesystem for OCI container image consistency.
576+
///
577+
/// This applies the standard transformations needed to ensure consistent
578+
/// composefs digests between build-time (mounted filesystem) and install-time
579+
/// (OCI tar layers) views:
580+
///
581+
/// 1. [`Self::copy_root_metadata_from_usr`] - copies `/usr` metadata to root directory
582+
/// 2. [`Self::canonicalize_run`] - empties `/run` directory
583+
///
584+
/// This is the recommended single entry point for OCI container processing.
585+
///
586+
/// NOTE: If changing this behavior, also update `doc/oci.md`.
587+
///
588+
/// # Errors
589+
///
590+
/// Returns an error if `/usr` does not exist.
591+
pub fn transform_for_oci(&mut self) -> Result<(), ImageError> {
592+
self.copy_root_metadata_from_usr()?;
593+
self.canonicalize_run()?;
594+
Ok(())
595+
}
550596
}
551597

552598
#[cfg(test)]
@@ -921,4 +967,93 @@ mod tests {
921967
assert_eq!(root_xattrs.len(), 1);
922968
assert!(root_xattrs.contains_key(OsStr::new("user.custom")));
923969
}
970+
971+
#[test]
972+
fn test_canonicalize_run() {
973+
let mut fs = FileSystem::<FileContents>::new(default_stat());
974+
975+
// Create /usr with specific mtime
976+
let usr_dir = Directory::new(stat_with_mtime(12345));
977+
fs.root
978+
.insert(OsStr::new("usr"), Inode::Directory(Box::new(usr_dir)));
979+
980+
// Create /run with content and different mtime
981+
let mut run_dir = Directory::new(stat_with_mtime(99999));
982+
run_dir.insert(OsStr::new("somefile"), Inode::Leaf(new_leaf_file(11111)));
983+
let mut subdir = Directory::new(stat_with_mtime(22222));
984+
subdir.insert(OsStr::new("nested"), Inode::Leaf(new_leaf_file(33333)));
985+
run_dir.insert(OsStr::new("subdir"), Inode::Directory(Box::new(subdir)));
986+
fs.root
987+
.insert(OsStr::new("run"), Inode::Directory(Box::new(run_dir)));
988+
989+
// Verify /run has content before
990+
assert_eq!(
991+
fs.root
992+
.get_directory(OsStr::new("run"))
993+
.unwrap()
994+
.entries
995+
.len(),
996+
2
997+
);
998+
999+
// Canonicalize
1000+
fs.canonicalize_run().unwrap();
1001+
1002+
// Verify /run is now empty with /usr's mtime
1003+
let run = fs.root.get_directory(OsStr::new("run")).unwrap();
1004+
assert!(run.entries.is_empty());
1005+
assert_eq!(run.stat.st_mtim_sec, 12345);
1006+
}
1007+
1008+
#[test]
1009+
fn test_canonicalize_run_no_run_dir() {
1010+
let mut fs = FileSystem::<FileContents>::new(default_stat());
1011+
1012+
// Create /usr but no /run
1013+
let usr_dir = Directory::new(stat_with_mtime(12345));
1014+
fs.root
1015+
.insert(OsStr::new("usr"), Inode::Directory(Box::new(usr_dir)));
1016+
1017+
// Should succeed without error
1018+
fs.canonicalize_run().unwrap();
1019+
}
1020+
1021+
#[test]
1022+
fn test_transform_for_oci() {
1023+
let mut fs = FileSystem::<FileContents>::new(default_stat());
1024+
1025+
// Create /usr with specific metadata
1026+
let usr_stat = Stat {
1027+
st_mode: 0o750,
1028+
st_uid: 100,
1029+
st_gid: 200,
1030+
st_mtim_sec: 54321,
1031+
xattrs: RefCell::new(BTreeMap::from([(
1032+
Box::from(OsStr::new("user.test")),
1033+
Box::from(b"val".as_slice()),
1034+
)])),
1035+
};
1036+
fs.root
1037+
.insert(OsStr::new("usr"), new_dir_inode_with_stat(usr_stat));
1038+
1039+
// Create /run with content
1040+
let mut run_dir = Directory::new(stat_with_mtime(99999));
1041+
run_dir.insert(OsStr::new("file"), Inode::Leaf(new_leaf_file(11111)));
1042+
fs.root
1043+
.insert(OsStr::new("run"), Inode::Directory(Box::new(run_dir)));
1044+
1045+
// Transform for OCI
1046+
fs.transform_for_oci().unwrap();
1047+
1048+
// Verify root metadata copied from /usr
1049+
assert_eq!(fs.root.stat.st_mode, 0o750);
1050+
assert_eq!(fs.root.stat.st_uid, 100);
1051+
assert_eq!(fs.root.stat.st_gid, 200);
1052+
assert_eq!(fs.root.stat.st_mtim_sec, 54321);
1053+
1054+
// Verify /run is emptied with /usr's mtime
1055+
let run = fs.root.get_directory(OsStr::new("run")).unwrap();
1056+
assert!(run.entries.is_empty());
1057+
assert_eq!(run.stat.st_mtim_sec, 54321);
1058+
}
9241059
}

doc/oci.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,51 @@ This ensures that:
7777
- Other host xattrs (overlayfs internals, etc.) don't pollute the image
7878

7979
See: https://github.com/containers/storage/pull/1608#issuecomment-1600915185
80+
81+
# The /run directory
82+
83+
When processing OCI images via `create_filesystem()`, the `/run` directory
84+
is emptied if present. This is a tmpfs at runtime and should always be
85+
empty in images. Its mtime is set to match `/usr` for consistency with
86+
how root directory metadata is handled.
87+
88+
This makes it possible to work around podman/buildah's `RUN --mount` issue where cache
89+
mounts can leave incomplete directory entries in OCI tar layers (directories
90+
without explicit tar entries inherit incorrect mtimes) by pointing all
91+
such mounts into `/run`, and then redirecting from their final location
92+
via e.g. symlinks into `/run`.
93+
94+
## Container build cache mounts
95+
96+
A practical implication of emptying `/run` is that container authors can
97+
use it for cache mounts without worrying about polluting the final image.
98+
99+
Instead of:
100+
```dockerfile
101+
RUN --mount=type=cache,target=/var/cache/dnf dnf install -y ...
102+
```
103+
104+
Consider:
105+
```dockerfile
106+
RUN rm -rf /var/cache/dnf && ln -sr /run/dnfcache /var/cache/dnf
107+
RUN --mount=type=cache,target=/run/dnfcache dnf install -y ...
108+
```
109+
110+
This avoids potential mtime inconsistencies in `/var/cache` while still
111+
benefiting from build caching.
112+
113+
See: https://github.com/containers/composefs-rs/issues/132
114+
115+
# Emptied directories for boot
116+
117+
When preparing a filesystem for boot via `transform_for_boot()`, certain
118+
additional directories are emptied because their contents should not be
119+
part of the final verified image:
120+
121+
- `/boot`: Contains the UKI which embeds the composefs digest, so including
122+
it would create a circular dependency
123+
- `/sysroot`: Only has content in ostree-container cases, and traversing
124+
it for SELinux labeling causes problems
125+
126+
These directories are emptied and their mtime is set to match `/usr` for
127+
consistency with how the root directory metadata is handled.

0 commit comments

Comments
 (0)