cp: improve the selinux support #7878

sylvestre · 2025-05-03T16:53:06Z

No description provided.

github-actions · 2025-05-03T17:29:49Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

cakebaker · 2025-05-04T12:03:16Z

The code currently fails due to the changes in #7845:

error[E0277]: `?` couldn't convert the error to `Error`
    --> src/uu/cp/src/cp.rs:2461:86
     |
2251 | ) -> CopyResult<()> {
     |      -------------- expected `Error` because of this
...
2461 |         uucore::selinux::set_selinux_security_context(dest, options.context.as_ref())?;
     |         -----------------------------------------------------------------------------^ the trait `From<SeLinuxError>` is not implemented for `Error`
     |         |
     |         this can't be annotated with `?` because it has type `Result<_, SeLinuxError>`

github-actions · 2025-05-04T15:51:44Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

Copilot

Pull Request Overview

This PR improves SELinux support across the codebase by updating error messages, refactoring the SELinux error enum, and adding new command-line options for SELinux context handling. Key changes include:

Updating expected error message substrings in SELinux-related tests.
Refactoring the SELinux error enum (now SeLinuxError) with improved error variants and messages.
Adding new SELinux-related command-line options in the cp utility and updating Cargo.toml dependency configurations.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/by-util/test_mknod.rs	Adjust expected error string casing for SELinux errors
tests/by-util/test_mkfifo.rs	Adjust expected error string casing for SELinux errors
tests/by-util/test_mkdir.rs	Update expected SELinux error message to reflect new error variant
src/uucore/src/lib/features/selinux.rs	Refactor SELinux error enum and error mapping using thiserror
src/uu/mkdir/src/mkdir.rs	Update error reporting when setting SELinux context in directory creation
src/uu/cp/src/cp.rs	Add SELinux context flags and update feature flag usage for SELinux
src/uu/cp/Cargo.toml	Update SELinux dependency configuration to include thiserror

Comments suppressed due to low confidence (4)

tests/by-util/test_mknod.rs:189

Ensure that the expected error message substring matches the actual output case consistently across tests. If the change to lowercase is intentional, verify that all related tests follow this convention.

.stderr_contains("Failed to");

tests/by-util/test_mkfifo.rs:162

Ensure the expected error message substring now reflects the updated casing ('failed to') consistently with the changes in other SELinux tests.

.stderr_contains("Failed to");

tests/by-util/test_mkdir.rs:413

The expected error message has been updated to 'Failed to set default file creation context to 'testtest':'; ensure that the test expectation is aligned with the new error message generated by the updated SELinux handling.

.stderr_contains("failed to set SELinux security context:");

src/uucore/src/lib/features/selinux.rs:26

[nitpick] The error variant names 'ContextSetFailure' and 'ContextConversionFailure' are very similar and could be confusing; consider renaming one for clearer differentiation between failures during context conversion versus setting.

ContextConversionFailure(String, String),

cakebaker · 2025-05-05T07:44:31Z

Can you please rebase? The PR currently contains commits from a different PR.

… code. + Add test for the selinux changes with context SLASHLogin Improves the coverage of tests/cp/cp-a-selinux.sh

github-actions · 2025-05-05T10:41:25Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

cakebaker · 2025-05-05T14:27:26Z

tests/by-util/test_cp.rs

+            "Expected '{}' not found in getfattr output:\n{}",
+            "foo",
+            selinux_perm


Suggested change

"Expected '{}' not found in getfattr output:\n{}",

"foo",

selinux_perm

"Expected 'foo' not found in getfattr output:\n{selinux_perm}"

cakebaker · 2025-05-05T14:32:52Z

tests/by-util/test_cp.rs

+            "Expected '{}' not found in getfattr output:\n{}",
+            "foo",
+            selinux_perm_dest


Suggested change

"Expected '{}' not found in getfattr output:\n{}",

"foo",

selinux_perm_dest

"Expected 'foo' not found in getfattr output:\n{selinux_perm_dest}"

cakebaker · 2025-05-05T14:38:19Z

tests/by-util/test_cp.rs

+    // 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


Suggested change

// 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

// 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.

cakebaker · 2025-05-05T14:51:21Z

tests/by-util/test_cp.rs

+    // Create two different files
+    at.write(TEST_HELLO_WORLD_SOURCE, "source content");


Either the comment is incorrect or the code. There is only one call of at.write().

cakebaker · 2025-05-05T15:05:08Z

tests/by-util/test_cp.rs

+    }
+
+    // Copy the file with preserved context
+    // Only works at root


Suggested change

// Only works at root

// Only works as root

cakebaker · 2025-05-05T15:24:42Z

src/uu/cp/src/cp.rs

 pub(crate) fn copy_attributes(
    source: &Path,
    dest: &Path,
    attributes: &Attributes,
+    options: &Options,


What's the reason for adding this unused parameter?

carryover from the previous work, I might be able to remove it

+ fix comments from review

github-actions · 2025-05-05T17:37:43Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

sylvestre force-pushed the selinux-cp branch from 6184c5a to b895615 Compare May 3, 2025 16:53

sylvestre requested review from cakebaker and Copilot May 4, 2025 15:55

Copilot AI reviewed May 4, 2025

View reviewed changes

sylvestre and others added 3 commits May 5, 2025 12:06

selinux: add support in cp

07a2be9

cp: add -Z flag & add --context=[CTX] flag

2b104a2

cp/selinx: improve the support of --preserve-context and simplify the…

4325fd1

… code. + Add test for the selinux changes with context SLASHLogin Improves the coverage of tests/cp/cp-a-selinux.sh

sylvestre force-pushed the selinux-cp branch from 02be974 to 3eae759 Compare May 5, 2025 10:06

cakebaker reviewed May 5, 2025

View reviewed changes

set_selinux_security_context: also display the error from the crate

b30bb31

+ fix comments from review

sylvestre force-pushed the selinux-cp branch from 3eae759 to b30bb31 Compare May 5, 2025 17:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cp: improve the selinux support #7878

cp: improve the selinux support #7878

sylvestre commented May 3, 2025

github-actions bot commented May 3, 2025

cakebaker commented May 4, 2025

github-actions bot commented May 4, 2025

Copilot AI left a comment

cakebaker commented May 5, 2025

github-actions bot commented May 5, 2025

cakebaker May 5, 2025

cakebaker May 5, 2025

cakebaker May 5, 2025

cakebaker May 5, 2025

cakebaker May 5, 2025

cakebaker May 5, 2025 •

edited

Loading

sylvestre May 5, 2025

github-actions bot commented May 5, 2025

		// Create two different files
		at.write(TEST_HELLO_WORLD_SOURCE, "source content");

cp: improve the selinux support #7878

Are you sure you want to change the base?

cp: improve the selinux support #7878

Conversation

sylvestre commented May 3, 2025

github-actions bot commented May 3, 2025

cakebaker commented May 4, 2025

github-actions bot commented May 4, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

cakebaker commented May 5, 2025

github-actions bot commented May 5, 2025

cakebaker May 5, 2025

Choose a reason for hiding this comment

cakebaker May 5, 2025

Choose a reason for hiding this comment

cakebaker May 5, 2025

Choose a reason for hiding this comment

cakebaker May 5, 2025

Choose a reason for hiding this comment

cakebaker May 5, 2025

Choose a reason for hiding this comment

cakebaker May 5, 2025 • edited Loading

Choose a reason for hiding this comment

sylvestre May 5, 2025

Choose a reason for hiding this comment

github-actions bot commented May 5, 2025

cakebaker May 5, 2025 •

edited

Loading