From 5148ba12d6b7b4237c81bf0eebf3249db5929c75 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 26 Apr 2025 15:21:12 +0200 Subject: [PATCH] set_selinux_security_context: also display the error from the crate + fix comments from review --- src/uu/cp/src/copydir.rs | 4 +--- src/uu/cp/src/cp.rs | 15 ++++++++------- tests/by-util/test_cp.rs | 15 +++++---------- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/uu/cp/src/copydir.rs b/src/uu/cp/src/copydir.rs index 3137a2053..d2e367c5c 100644 --- a/src/uu/cp/src/copydir.rs +++ b/src/uu/cp/src/copydir.rs @@ -436,7 +436,6 @@ pub(crate) fn copy_directory( &entry.source_absolute, &entry.local_to_target, &options.attributes, - options, )?; } } @@ -467,7 +466,6 @@ pub(crate) fn copy_directory( &entry.source_absolute, &entry.local_to_target, &options.attributes, - options, )?; } } @@ -478,7 +476,7 @@ pub(crate) fn copy_directory( let dest = target.join(root.file_name().unwrap()); for (x, y) in aligned_ancestors(root, dest.as_path()) { if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) { - copy_attributes(&src, y, &options.attributes, options)?; + copy_attributes(&src, y, &options.attributes)?; } } } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index fcdeca7f8..200a903aa 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1492,7 +1492,7 @@ fn copy_source( if options.parents { for (x, y) in aligned_ancestors(source, dest.as_path()) { if let Ok(src) = canonicalize(x, MissingHandling::Normal, ResolveMode::Physical) { - copy_attributes(&src, y, &options.attributes, options)?; + copy_attributes(&src, y, &options.attributes)?; } } } @@ -1640,12 +1640,10 @@ fn copy_extended_attrs(source: &Path, dest: &Path) -> CopyResult<()> { } /// Copy the specified attributes from one path to another. -#[allow(unused_variables)] pub(crate) fn copy_attributes( source: &Path, dest: &Path, attributes: &Attributes, - options: &Options, ) -> CopyResult<()> { let context = &*format!("{} -> {}", source.quote(), dest.quote()); let source_metadata = fs::symlink_metadata(source).context(context)?; @@ -2442,7 +2440,7 @@ fn copy_file( if options.dereference(source_in_command_line) { if let Ok(src) = canonicalize(source, MissingHandling::Normal, ResolveMode::Physical) { if src.exists() { - copy_attributes(&src, dest, &options.attributes, options)?; + copy_attributes(&src, dest, &options.attributes)?; } } } else if source_is_stream && source.exists() { @@ -2450,15 +2448,18 @@ fn copy_file( // like anonymous pipes. Thus, we can't really copy its // attributes. However, this is already handled in the stream // copy function (see `copy_stream` under platform/linux.rs). - copy_attributes(source, dest, &options.attributes, options)?; } else { - copy_attributes(source, dest, &options.attributes, options)?; + copy_attributes(source, dest, &options.attributes)?; } #[cfg(feature = "selinux")] if options.set_selinux_context && uucore::selinux::is_selinux_enabled() { // Set the given selinux permissions on the copied file. - uucore::selinux::set_selinux_security_context(dest, options.context.as_ref())?; + if let Err(e) = + uucore::selinux::set_selinux_security_context(dest, options.context.as_ref()) + { + return Err(Error::Error(format!("SELinux error: {}", e))); + } } copied_files.insert( diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 515d7dab4..9fc2b05a7 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -6292,9 +6292,7 @@ fn test_cp_selinux() { assert!( selinux_perm.contains("unconfined_u"), - "Expected '{}' not found in getfattr output:\n{}", - "foo", - selinux_perm + "Expected 'foo' not found in getfattr output:\n{selinux_perm}" ); at.remove(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); } @@ -6342,9 +6340,7 @@ fn test_cp_preserve_selinux() { let selinux_perm_dest = get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_DEST)); assert!( selinux_perm_dest.contains("unconfined_u"), - "Expected '{}' not found in getfattr output:\n{}", - "foo", - selinux_perm_dest + "Expected 'foo' not found in getfattr output:\n{selinux_perm_dest}" ); assert_eq!( get_getfattr_output(&at.plus_as_string(TEST_HELLO_WORLD_SOURCE)), @@ -6373,8 +6369,8 @@ fn test_cp_preserve_selinux_admin_context() { at.touch(TEST_HELLO_WORLD_SOURCE); // Get the default SELinux context for the destination file path - // on Debian/Ubuntu, this program is provided by the selinux-utils package - // on Fedora/RHEL, this program is provided by the libselinux-devel package + // On Debian/Ubuntu, this program is provided by the selinux-utils package + // On Fedora/RHEL, this program is provided by the libselinux-devel package let output = std::process::Command::new("matchpathcon") .arg(at.plus_as_string(TEST_HELLO_WORLD_DEST)) .output() @@ -6432,7 +6428,6 @@ fn test_cp_selinux_context_priority() { let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; - // Create two different files at.write(TEST_HELLO_WORLD_SOURCE, "source content"); // First, set a known context on source file (only if system supports it) @@ -6642,7 +6637,7 @@ fn test_cp_preserve_context_root() { } // Copy the file with preserved context - // Only works at root + // Only works as root if let Ok(result) = run_ucmd_as_root(&scene, &["--preserve=context", source_file, dest_file]) { let src_ctx = get_getfattr_output(&at.plus_as_string(source_file)); let dest_ctx = get_getfattr_output(&at.plus_as_string(dest_file));