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.

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

src/uu/cp/src/cp.rs

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)

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

+ fix comments from review

github-actions · 2025-05-06T07:27:02Z

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)

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 force-pushed the selinux-cp branch from 02be974 to 3eae759 Compare May 5, 2025 10:06

cakebaker reviewed May 5, 2025

View reviewed changes

src/uu/cp/src/cp.rs Outdated Show resolved Hide resolved

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

sylvestre force-pushed the selinux-cp branch from b30bb31 to 510b845 Compare May 6, 2025 06:49

sylvestre and others added 4 commits May 6, 2025 08:52

selinux: add support in cp

e7fdd3d

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

74e72f5

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

b3a2b74

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

set_selinux_security_context: also display the error from the crate

5148ba1

+ fix comments from review

sylvestre force-pushed the selinux-cp branch from 510b845 to 5148ba1 Compare May 6, 2025 06:52

cakebaker merged commit c981767 into uutils:main May 6, 2025
70 checks passed

sylvestre deleted the selinux-cp branch May 6, 2025 08:44

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

github-actions bot commented May 5, 2025

github-actions bot commented May 6, 2025

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

cp: improve the selinux support #7878

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

github-actions bot commented May 5, 2025

github-actions bot commented May 6, 2025