From 029bd902db3d9c9b0b58cb5230b1e84fffd9f4e9 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 27 Jun 2025 15:52:42 +0200 Subject: [PATCH 1/3] ls: fix the GNU test tests/ls/selinux.sh --- src/uu/ls/Cargo.toml | 2 +- src/uu/ls/src/ls.rs | 11 ++-- tests/by-util/test_ls.rs | 112 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 116 insertions(+), 9 deletions(-) diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index ffb90a9403c..2ce0cc3fdb7 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -52,4 +52,4 @@ name = "ls" path = "src/main.rs" [features] -feat_selinux = ["selinux"] +feat_selinux = ["selinux", "uucore/selinux"] diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7ab752a1593..572daa61e5a 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1900,11 +1900,7 @@ impl PathData { None => OnceCell::new(), }; - let security_context = if config.context { - get_security_context(config, &p_buf, must_dereference) - } else { - String::new() - }; + let security_context = get_security_context(config, &p_buf, must_dereference); Self { md: OnceCell::new(), @@ -3339,7 +3335,10 @@ fn get_security_context(config: &Config, p_buf: &Path, must_dereference: bool) - Err(err) => { // The Path couldn't be dereferenced, so return early and set exit code 1 // to indicate a minor error - show!(LsError::IOErrorContext(p_buf.to_path_buf(), err, false)); + // Only show error when context display is requested to avoid duplicate messages + if config.context { + show!(LsError::IOErrorContext(p_buf.to_path_buf(), err, false)); + } return substitute_string; } Ok(_md) => (), diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index af5fb71b135..07bc3cb7193 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4263,8 +4263,7 @@ fn test_ls_context_long() { let line: Vec<_> = result.stdout_str().split(' ').collect(); assert!(line[0].ends_with('.')); assert!(line[4].starts_with("unconfined_u")); - let s: Vec<_> = line[4].split(':').collect(); - assert!(s.len() == 4); + validate_selinux_context(line[4]); } } @@ -4298,6 +4297,115 @@ fn test_ls_context_format() { } } +/// Helper function to validate `SELinux` context format +/// Returns the context parts if valid, panics with descriptive message if invalid +#[cfg(feature = "feat_selinux")] +fn validate_selinux_context(context: &str) { + assert!( + context.contains(':'), + "Expected SELinux context format (user:role:type:level), got: {}", + context + ); + + let context_parts: Vec<&str> = context.split(':').collect(); + assert_eq!( + context_parts.len(), + 4, + "SELinux context should have 4 components separated by colons, got: {}", + context + ); +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_selinux_context_format() { + if !uucore::selinux::is_selinux_enabled() { + println!("test skipped: Kernel has no support for SElinux context"); + return; + } + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.touch("file"); + at.symlink_file("file", "link"); + + // Test that ls -lnZ properly shows the context + for file in ["file", "link"] { + let result = scene.ucmd().args(&["-lnZ", file]).succeeds(); + let output = result.stdout_str(); + + let lines: Vec<&str> = output.lines().collect(); + assert!(!lines.is_empty(), "Output should contain at least one line"); + + let first_line = lines[0]; + let parts: Vec<&str> = first_line.split_whitespace().collect(); + assert!(parts.len() >= 6, "Line should have at least 6 fields"); + + // The 5th field (0-indexed position 4) should contain the SELinux context + // Format: permissions links owner group context size date time name + let context = parts[4]; + validate_selinux_context(context); + } +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_selinux_context_indicator() { + if !uucore::selinux::is_selinux_enabled() { + println!("test skipped: Kernel has no support for SElinux context"); + return; + } + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + at.touch("file"); + at.symlink_file("file", "link"); + + // Test that ls -l shows "." indicator for files with SELinux contexts + for file in ["file", "link"] { + let result = scene.ucmd().args(&["-l", file]).succeeds(); + let output = result.stdout_str(); + + // The 11th character should be "." indicating SELinux context + // -rw-rw-r--. (permissions + context indicator) + let lines: Vec<&str> = output.lines().collect(); + assert!(!lines.is_empty(), "Output should contain at least one line"); + + let first_line = lines[0]; + let chars: Vec = first_line.chars().collect(); + assert!( + chars.len() >= 11, + "Line should be at least 11 characters long" + ); + + // The 11th character (0-indexed position 10) should be "." for SELinux context + assert_eq!( + chars[10], '.', + "Expected '.' indicator for SELinux context in position 11, got '{}' in line: {}", + chars[10], first_line + ); + } + + // Test that ls -lnZ properly shows the context + for file in ["file", "link"] { + let result = scene.ucmd().args(&["-lnZ", file]).succeeds(); + let output = result.stdout_str(); + + let lines: Vec<&str> = output.lines().collect(); + assert!(!lines.is_empty(), "Output should contain at least one line"); + + let first_line = lines[0]; + let parts: Vec<&str> = first_line.split_whitespace().collect(); + assert!(parts.len() >= 6, "Line should have at least 6 fields"); + + // The 5th field (0-indexed position 4) should contain the SELinux context + // Format: permissions links owner group context size date time name + validate_selinux_context(parts[4]); + } +} + #[test] #[allow(non_snake_case)] fn test_ls_a_A() { From 6b7d25b0140c03618631d376a9e2480371908f3e Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 2 Jul 2025 10:13:05 +0200 Subject: [PATCH 2/3] Remove old comment Co-authored-by: Daniel Hofstetter --- tests/by-util/test_ls.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 07bc3cb7193..4286abd3df7 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4298,7 +4298,6 @@ fn test_ls_context_format() { } /// Helper function to validate `SELinux` context format -/// Returns the context parts if valid, panics with descriptive message if invalid #[cfg(feature = "feat_selinux")] fn validate_selinux_context(context: &str) { assert!( From 1f84584af6fdc5f0050419324097b75f161d7f74 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 2 Jul 2025 10:13:14 +0200 Subject: [PATCH 3/3] simpli Co-authored-by: Daniel Hofstetter --- tests/by-util/test_ls.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 4286abd3df7..af6b2dd18ac 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4306,9 +4306,8 @@ fn validate_selinux_context(context: &str) { context ); - let context_parts: Vec<&str> = context.split(':').collect(); assert_eq!( - context_parts.len(), + context.split(':').count(), 4, "SELinux context should have 4 components separated by colons, got: {}", context