Merge pull request containers#680 from cgwalters/fix-hardlinked-sysro…

…ot-absolute tar: Handle absolute hardlinked paths to sysroot
cgwalters · Nov 4, 2024 · 40431e8 · 40431e8
2 parents 003039d + f3c1c15
commit 40431e8
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 2 deletions.
diff --git a/lib/src/tar/write.rs b/lib/src/tar/write.rs
@@ -272,6 +272,8 @@ pub(crate) fn filter_tar(
         let header = entry.header();
         let path = entry.path()?;
         let path: &Utf8Path = (&*path).try_into()?;
+        // Force all paths to relative
+        let path = path.strip_prefix("/").unwrap_or(path);
 
         let is_modified = header.mtime().unwrap_or_default() > 0;
         let is_regular = header.entry_type() == tar::EntryType::Regular;
@@ -302,6 +304,8 @@ pub(crate) fn filter_tar(
                 .link_name()?
                 .ok_or_else(|| anyhow!("Invalid empty hardlink"))?;
             let target: &Utf8Path = (&*target).try_into()?;
+            // Canonicalize to a relative path
+            let target = path.strip_prefix("/").unwrap_or(target);
             // If this is a hardlink into /sysroot...
             if target.strip_prefix(crate::tar::REPO_PREFIX).is_ok() {
                 // And we found a previously processed modified file there

diff --git a/lib/tests/it/main.rs b/lib/tests/it/main.rs
@@ -1132,7 +1132,16 @@ async fn test_container_var_content() -> Result<()> {
 }
 
 #[tokio::test]
-async fn test_container_etc_hardlinked() -> Result<()> {
+async fn test_container_etc_hardlinked_absolute() -> Result<()> {
+    test_container_etc_hardlinked(true).await
+}
+
+#[tokio::test]
+async fn test_container_etc_hardlinked_relative() -> Result<()> {
+    test_container_etc_hardlinked(false).await
+}
+
+async fn test_container_etc_hardlinked(absolute: bool) -> Result<()> {
     let fixture = Fixture::new_v1()?;
 
     let imgref = fixture.export_container().await.unwrap().0;
@@ -1174,6 +1183,7 @@ async fn test_container_etc_hardlinked() -> Result<()> {
             // Another case where we have /etc/dnf.conf and a hardlinked /ostree/repo/objects
             // link into it - in this case we should ignore the hardlinked one.
             let testdata = "hardlinked into object store";
+            let mut h = tar::Header::new_ustar();
             h.set_mode(0o644);
             h.set_mtime(42);
             h.set_size(testdata.len().try_into().unwrap());
@@ -1186,7 +1196,20 @@ async fn test_container_etc_hardlinked() -> Result<()> {
             h.set_entry_type(tar::EntryType::Link);
             h.set_mtime(42);
             h.set_size(0);
-            layer_tar.append_link(&mut h.clone(), "sysroot/ostree/repo/objects/45/7279b28b541ca20358bec8487c81baac6a3d5ed3cea019aee675137fab53cb.file", "etc/dnf.conf")?;
+            let path = "sysroot/ostree/repo/objects/45/7279b28b541ca20358bec8487c81baac6a3d5ed3cea019aee675137fab53cb.file";
+            let target = "etc/dnf.conf";
+            if absolute {
+                let ustarname = &mut h.as_ustar_mut().unwrap().name;
+                // The tar crate doesn't let us set absolute paths in tar archives, so we bypass
+                // it and just write to the path buffer directly.
+                assert!(path.len() < ustarname.len());
+                ustarname[0..path.len()].copy_from_slice(path.as_bytes());
+                h.set_link_name(target)?;
+                h.set_cksum();
+                layer_tar.append(&mut h.clone(), std::io::empty())?;
+            } else {
+                layer_tar.append_link(&mut h.clone(), path, target)?;
+            }
             layer_tar.finish()?;
             Ok(())
         },
@@ -1221,6 +1244,10 @@ async fn test_container_etc_hardlinked() -> Result<()> {
     bar.ensure_resolved()?;
     assert_eq!(foo.checksum(), bar.checksum());
 
+    let dnfconf = r.resolve_relative_path("usr/etc/dnf.conf");
+    let dnfconf: &ostree::RepoFile = dnfconf.downcast_ref::<ostree::RepoFile>().unwrap();
+    dnfconf.ensure_resolved()?;
+
     Ok(())
 }