Merge pull request #1243 from cgwalters/cap-std-ext-walkdir

Update to use walk from cap-std-ext

Author: jeckersb

Cargo.lock

@@ -9,14 +9,15 @@ use std::collections::BTreeSet;
 use std::env::consts::ARCH;
 use std::fmt::Write as WriteFmt;
 use std::os::unix::ffi::OsStrExt;
+use std::path::Path;
 
 use anyhow::Result;
 use bootc_utils::PathQuotedDisplay;
 use camino::{Utf8Path, Utf8PathBuf};
 use cap_std::fs::Dir;
 use cap_std_ext::cap_std;
 use cap_std_ext::cap_std::fs::MetadataExt;
-use cap_std_ext::dirext::CapStdExtDirExt as _;
+use cap_std_ext::dirext::{CapStdExtDirExt as _, WalkConfiguration};
 use fn_error_context::context;
 use indoc::indoc;
 use linkme::distributed_slice;
@@ -338,30 +339,44 @@ UTF-8 filenames. Non-UTF8 filenames will cause a fatal error.
     check_utf8,
 );
 fn check_utf8(dir: &Dir) -> LintResult {
-    for entry in dir.entries()? {
-        let entry = entry?;
-        let name = entry.file_name();
-
-        let Some(strname) = &name.to_str() else {
-            // will escape nicely like "abc\xFFdéf"
-            return lint_err(format!("/: Found non-utf8 filename {name:?}"));
-        };
-
-        let ifmt = entry.file_type()?;
-        if ifmt.is_symlink() {
-            let target = dir.read_link_contents(&name)?;
-            if !target.to_str().is_some() {
-                return lint_err(format!("/{strname}: Found non-utf8 symlink target"));
+    let mut err = None;
+    dir.walk(
+        &WalkConfiguration::default()
+            .noxdev()
+            .path_base(Path::new("/")),
+        |e| -> std::io::Result<_> {
+            // Right now we stop iteration on the first non-UTF8 filename found.
+            // However in the future it'd make sense to handle multiple.
+            if err.is_some() {
+                return Ok(std::ops::ControlFlow::Break(()));
             }
-        } else if ifmt.is_dir() {
-            let Some(subdir) = dir.open_dir_noxdev(entry.file_name())? else {
-                continue;
+            let path = e.path;
+            let filename = e.filename;
+            let dirname = path.parent().unwrap_or(Path::new("/"));
+            if filename.to_str().is_none() {
+                // This escapes like "abc\xFFdéf"
+                err = Some(format!(
+                    "{}: Found non-utf8 filename {filename:?}",
+                    PathQuotedDisplay::new(&dirname)
+                ));
+                return Ok(std::ops::ControlFlow::Break(()));
             };
-            if let Err(err) = check_utf8(&subdir)? {
-                // Try to do the path pasting only in the event of an error
-                return lint_err(format!("/{strname}{err}"));
+
+            if e.file_type.is_symlink() {
+                let target = e.dir.read_link_contents(filename)?;
+                if !target.to_str().is_some() {
+                    err = Some(format!(
+                        "{}: Found non-utf8 symlink target",
+                        PathQuotedDisplay::new(&path)
+                    ));
+                    return Ok(std::ops::ControlFlow::Break(()));
+                }
             }
-        }
+            Ok(std::ops::ControlFlow::Continue(()))
+        },
+    )?;
+    if let Some(err) = err {
+        return lint_err(err);
     }
     lint_ok()
 }
@@ -875,7 +890,7 @@ mod tests {
         };
         assert_eq!(
             err.to_string(),
-            r#"/subdir/2/: Found non-utf8 filename "bad\xFFdir""#
+            r#"/subdir/2: Found non-utf8 filename "bad\xFFdir""#
         );
         root.remove_dir(baddir).unwrap(); // Get rid of the problem
         check_utf8(root).unwrap().unwrap(); // Check it

lib/src/lints.rs

@@ -1,7 +1,5 @@
-use std::{
-    os::fd::AsRawFd,
-    path::{Path, PathBuf},
-};
+use std::os::fd::AsRawFd;
+use std::path::Path;
 
 use anyhow::Result;
 use camino::Utf8Path;
@@ -147,8 +145,7 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
             cmd!(sh, "sudo {BASE_ARGS...} {image} bootc install to-existing-root --acknowledge-destructive {generic_inst_args...}").run()?;
             generic_post_install_verification()?;
             let root = &Dir::open_ambient_dir("/ostree", cap_std::ambient_authority()).unwrap();
-            let mut path = PathBuf::from(".");
-            crate::selinux::verify_selinux_recurse(root, &mut path, false)?;
+            crate::selinux::verify_selinux_recurse(root, false)?;
             Ok(())
         }),
         Trial::test("Install to non-default stateroot", move || {

tests-integration/src/install.rs

@@ -1,35 +1,38 @@
+use std::ffi::OsStr;
+use std::ops::ControlFlow;
 use std::os::fd::AsRawFd;
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 
 use anyhow::{Context, Result};
 use cap_std_ext::cap_std::fs::Dir;
+use cap_std_ext::dirext::{CapStdExtDirExt, WalkConfiguration};
+use rustix::path::DecInt;
 
-fn verify_selinux_label_exists(root: &Dir, path: &Path, warn: bool) -> Result<()> {
+fn verify_selinux_label_exists(d: &Dir, filename: &OsStr) -> Result<bool> {
     let mut buf = [0u8; 1024];
-    let fdpath = format!("/proc/self/fd/{}/", root.as_raw_fd());
-    let fdpath = &Path::new(&fdpath).join(path);
+    let mut fdpath = PathBuf::from("/proc/self/fd");
+    fdpath.push(DecInt::new(d.as_raw_fd()));
+    fdpath.push(filename);
     match rustix::fs::lgetxattr(fdpath, "security.selinux", &mut buf) {
         // Ignore EOPNOTSUPPORTED
-        Ok(_) | Err(rustix::io::Errno::OPNOTSUPP) => Ok(()),
-        Err(rustix::io::Errno::NODATA) if warn => {
-            eprintln!("No SELinux label found for: {path:?}");
-            Ok(())
-        }
-        Err(e) => Err(e).with_context(|| format!("Failed to look up context for {path:?}")),
+        Ok(_) | Err(rustix::io::Errno::OPNOTSUPP) => Ok(true),
+        Err(rustix::io::Errno::NODATA) => Ok(false),
+        Err(e) => Err(e.into()),
     }
 }
 
-pub(crate) fn verify_selinux_recurse(root: &Dir, path: &mut PathBuf, warn: bool) -> Result<()> {
-    for ent in root.read_dir(&path)? {
-        let ent = ent?;
-        let name = ent.file_name();
-        path.push(name);
-        verify_selinux_label_exists(root, &path, warn)?;
-        let file_type = ent.file_type()?;
-        if file_type.is_dir() {
-            verify_selinux_recurse(root, path, warn)?;
+pub(crate) fn verify_selinux_recurse(root: &Dir, warn: bool) -> Result<()> {
+    root.walk(&WalkConfiguration::default().noxdev(), |e| {
+        let exists = verify_selinux_label_exists(e.dir, e.filename)
+            .with_context(|| format!("Failed to look up context for {:?}", e.path))?;
+        if !exists {
+            if warn {
+                eprintln!("No SELinux label found for: {:?}", e.path);
+            } else {
+                anyhow::bail!("No SELinux label found for: {:?}", e.path);
+            }
         }
-        path.pop();
-    }
+        anyhow::Ok(ControlFlow::Continue(()))
+    })?;
     Ok(())
 }

tests-integration/src/selinux.rs

@@ -1,7 +1,5 @@
 //! Integration tests.
 
-use std::path::PathBuf;
-
 use camino::Utf8PathBuf;
 use cap_std_ext::cap_std::{self, fs::Dir};
 use clap::Parser;
@@ -53,8 +51,7 @@ fn main() {
         Opt::RunVM(opts) => runvm::run(opts),
         Opt::VerifySELinux { rootfs, warn } => {
             let root = &Dir::open_ambient_dir(&rootfs, cap_std::ambient_authority()).unwrap();
-            let mut path = PathBuf::from(".");
-            selinux::verify_selinux_recurse(root, &mut path, warn)
+            selinux::verify_selinux_recurse(root, warn)
         }
     };
     if let Err(e) = r {

tests-integration/src/tests-integration.rs

@@ -18,9 +18,8 @@ impl<'a> Display for PathQuotedDisplay<'a> {
             }
         }
         if let Ok(r) = shlex::bytes::try_quote(self.path.as_os_str().as_bytes()) {
-            if let Ok(s) = std::str::from_utf8(&r) {
-                return f.write_str(s);
-            }
+            let s = String::from_utf8_lossy(&r);
+            return f.write_str(&s);
         }
         // Should not happen really
         return Err(std::fmt::Error);
@@ -40,6 +39,8 @@ impl<'a> PathQuotedDisplay<'a> {
 
 #[cfg(test)]
 mod tests {
+    use std::ffi::OsStr;
+
     use super::*;
 
     #[test]
@@ -61,4 +62,12 @@ mod tests {
             assert_eq!(quoted, format!("{}", PathQuotedDisplay::new(&v)));
         }
     }
+
+    #[test]
+    fn test_nonutf8() {
+        let p = Path::new(OsStr::from_bytes(b"/foo/somenonutf8\xEE/bar"));
+        assert!(p.to_str().is_none());
+        let q = PathQuotedDisplay::new(&p).to_string();
+        assert_eq!(q, r#"'/foo/somenonutf8�/bar'"#);
+    }
 }