set_selinux_security_context: also display the error from the crate

sylvestre · sylvestre · commit acf4129a10f0 · 2025-04-26T15:21:29.000+02:00
+ fix comments from review
diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs
@@ -298,10 +298,7 @@ fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {
                 if let Err(e) = uucore::selinux::set_selinux_security_context(path, config.context)
                 {
                     let _ = std::fs::remove_dir(path);
-                    return Err(USimpleError::new(
-                        1,
-                        format!("failed to set SELinux security context: {}", e),
-                    ));
+                    return Err(USimpleError::new(1, e.to_string()));
                 }
             }
 
diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml
@@ -114,7 +114,7 @@ proc-info = ["tty", "walkdir"]
 quoting-style = []
 ranges = []
 ringbuffer = []
-selinux = ["dep:selinux"]
+selinux = ["dep:selinux", "thiserror"]
 signals = []
 sum = [
   "digest",
diff --git a/src/uucore/src/lib/features/selinux.rs b/src/uucore/src/lib/features/selinux.rs
@@ -13,27 +13,27 @@ pub enum SeLinuxError {
     #[error("SELinux is not enabled on this system")]
     SELinuxNotEnabled,
 
-    #[error("Failed to open the file")]
-    FileOpenFailure,
+    #[error("Failed to open the file: {0}")]
+    FileOpenFailure(String),
 
-    #[error("Failed to retrieve the security context")]
-    ContextRetrievalFailure,
+    #[error("Failed to retrieve the security context: {0}")]
+    ContextRetrievalFailure(String),
 
-    #[error("failed to set default file creation context to {0}")]
-    ContextSetFailure(String),
+    #[error("failed to set default file creation context2 to '{0}': {1}")]
+    ContextSetFailure(String, String),
 
-    #[error("Invalid context string or conversion failure")]
-    ContextConversionFailure,
+    #[error("failed to set default file creation context3 to '{0}': {1}")]
+    ContextConversionFailure(String, String),
 }
 
 impl From<SeLinuxError> for i32 {
     fn from(error: SeLinuxError) -> i32 {
         match error {
             SeLinuxError::SELinuxNotEnabled => 1,
-            SeLinuxError::FileOpenFailure => 2,
-            SeLinuxError::ContextRetrievalFailure => 3,
-            SeLinuxError::ContextSetFailure(_) => 4,
-            SeLinuxError::ContextConversionFailure => 5,
+            SeLinuxError::FileOpenFailure(_) => 2,
+            SeLinuxError::ContextRetrievalFailure(_) => 3,
+            SeLinuxError::ContextSetFailure(_, _) => 4,
+            SeLinuxError::ContextConversionFailure(_, _) => 5,
         }
     }
 }
@@ -98,26 +98,29 @@ pub fn set_selinux_security_context(
 
     if let Some(ctx_str) = context {
         // Create a CString from the provided context string
-        let c_context = std::ffi::CString::new(ctx_str.as_str())
-            .map_err(|_| SeLinuxError::ContextConversionFailure)?;
+        let c_context = std::ffi::CString::new(ctx_str.as_str()).map_err(|e| {
+            SeLinuxError::ContextConversionFailure(ctx_str.to_string(), e.to_string())
+        })?;
 
         // Convert the CString into an SELinux security context
-        let security_context = selinux::OpaqueSecurityContext::from_c_str(&c_context)
-            .map_err(|_| SeLinuxError::ContextConversionFailure)?;
+        let security_context =
+            selinux::OpaqueSecurityContext::from_c_str(&c_context).map_err(|e| {
+                SeLinuxError::ContextConversionFailure(ctx_str.to_string(), e.to_string())
+            })?;
 
         // Set the provided security context on the specified path
         SecurityContext::from_c_str(
-            &security_context
-                .to_c_string()
-                .map_err(|_| SeLinuxError::ContextConversionFailure)?,
+            &security_context.to_c_string().map_err(|e| {
+                SeLinuxError::ContextConversionFailure(ctx_str.to_string(), e.to_string())
+            })?,
             false,
         )
         .set_for_path(path, false, false)
-        .map_err(|_| SeLinuxError::ContextSetFailure(ctx_str.to_string()))
+        .map_err(|e| SeLinuxError::ContextSetFailure(ctx_str.to_string(), e.to_string()))
     } else {
         // If no context provided, set the default SELinux context for the path
         SecurityContext::set_default_for_path(path)
-            .map_err(|_| SeLinuxError::ContextSetFailure("".to_string()))
+            .map_err(|e| SeLinuxError::ContextSetFailure(String::new(), e.to_string()))
     }
 }
 
@@ -156,28 +159,29 @@ pub fn set_selinux_security_context(
 ///         }
 ///     },
 ///     Err(SeLinuxError::SELinuxNotEnabled) => println!("SELinux is not enabled on this system"),
-///     Err(SeLinuxError::FileOpenFailure) => println!("Failed to open the file"),
-///     Err(SeLinuxError::ContextRetrievalFailure) => println!("Failed to retrieve the security context"),
-///     Err(SeLinuxError::ContextConversionFailure) => println!("Failed to convert the security context to a string"),
+///     Err(SeLinuxError::FileOpenFailure(e)) => println!("Failed to open the file: {}", e),
+///     Err(SeLinuxError::ContextRetrievalFailure(e)) => println!("Failed to retrieve the security context: {}", e),
+///     Err(SeLinuxError::ContextConversionFailure(ctx, e)) => println!("Failed to convert context '{}': {}", ctx, e),
+///     Err(SeLinuxError::ContextSetFailure(ctx, e)) => println!("Failed to set context '{}': {}", ctx, e),
 /// }
 /// ```
 pub fn get_selinux_security_context(path: &Path) -> Result<String, SeLinuxError> {
     if !is_selinux_enabled() {
         return Err(SeLinuxError::SELinuxNotEnabled);
     }
 
-    let f = std::fs::File::open(path).map_err(|_| SeLinuxError::FileOpenFailure)?;
+    let f = std::fs::File::open(path).map_err(|e| SeLinuxError::FileOpenFailure(e.to_string()))?;
 
     // Get the security context of the file
     let context = match SecurityContext::of_file(&f, false) {
         Ok(Some(ctx)) => ctx,
         Ok(None) => return Ok(String::new()), // No context found, return empty string
-        Err(_) => return Err(SeLinuxError::ContextRetrievalFailure),
+        Err(e) => return Err(SeLinuxError::ContextRetrievalFailure(e.to_string())),
     };
 
     let context_c_string = context
         .to_c_string()
-        .map_err(|_| SeLinuxError::ContextConversionFailure)?;
+        .map_err(|e| SeLinuxError::ContextConversionFailure(String::new(), e.to_string()))?;
 
     if let Some(c_str) = context_c_string {
         // Convert the C string to a Rust String
@@ -204,19 +208,16 @@ mod tests {
             assert!(true, "Successfully set SELinux context");
         } else {
             let err = result.unwrap_err();
-            let valid_errors = [
-                "SELinux is not enabled on this system",
-                &format!(
-                    "Failed to set default context: selinux_lsetfilecon_default() failed on path '{}'",
-                    path.display()
-                ),
-            ];
-
-            assert!(
-                valid_errors.contains(&err.as_str()),
-                "Unexpected error message: {}",
-                err
-            );
+            match err {
+                SeLinuxError::SELinuxNotEnabled => {
+                    assert!(true, "SELinux is not enabled, which is a valid error case");
+                }
+                _ => {
+                    println!("Error setting context: {}", err);
+                    // Only assert for cases where we expect specific error messages
+                    // other errors can happen depending on the SELinux configuration
+                }
+            }
         }
     }
 
@@ -230,10 +231,18 @@ mod tests {
         let result = set_selinux_security_context(path, Some(&invalid_context));
 
         assert!(result.is_err());
-        assert_eq!(
-            result.unwrap_err(),
-            "Invalid context string (contains null bytes)"
-        );
+        if let Err(err) = result {
+            match err {
+                SeLinuxError::ContextConversionFailure(ctx, msg) => {
+                    assert_eq!(ctx, "invalid\0context");
+                    assert!(
+                        msg.contains("nul byte"),
+                        "Error message should mention nul byte"
+                    );
+                }
+                _ => panic!("Expected ContextConversionFailure error but got: {}", err),
+            }
+        }
     }
 
     #[test]
@@ -267,12 +276,23 @@ mod tests {
             // Valid error types
             match err {
                 SeLinuxError::SELinuxNotEnabled => assert!(true, "SELinux not supported"),
-                SeLinuxError::ContextRetrievalFailure => assert!(true, "Context retrieval failure"),
-                SeLinuxError::ContextConversionFailure => {
-                    assert!(true, "Context conversion failure")
+                SeLinuxError::ContextRetrievalFailure(e) => {
+                    println!("Context retrieval failure: {}", e);
+                    assert!(true, "Context retrieval failure");
+                }
+                SeLinuxError::ContextConversionFailure(ctx, e) => {
+                    println!("Context conversion failure for '{}': {}", ctx, e);
+                    assert!(true, "Context conversion failure");
                 }
-                SeLinuxError::FileOpenFailure => {
-                    panic!("File open failure occurred despite file being created")
+                SeLinuxError::ContextSetFailure(ctx, e) => {
+                    println!("Context set failure for '{}': {}", ctx, e);
+                    assert!(true, "Context set failure");
+                }
+                SeLinuxError::FileOpenFailure(e) => {
+                    panic!(
+                        "File open failure occurred despite file being created: {}",
+                        e
+                    );
                 }
             }
         }
@@ -285,9 +305,16 @@ mod tests {
         let result = get_selinux_security_context(path);
 
         assert!(result.is_err());
-        assert!(
-            matches!(result.unwrap_err(), SeLinuxError::FileOpenFailure),
-            "Expected file open error for nonexistent file"
-        );
+        if let Err(err) = result {
+            match err {
+                SeLinuxError::FileOpenFailure(e) => {
+                    assert!(
+                        e.contains("No such file"),
+                        "Error should mention file not found"
+                    );
+                }
+                _ => panic!("Expected FileOpenFailure error but got: {}", err),
+            }
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -298,10 +298,7 @@ fn create_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<()> {`
`298`	`298`	`if let Err(e) = uucore::selinux::set_selinux_security_context(path, config.context)`
`299`	`299`	`{`
`300`	`300`	`let _ = std::fs::remove_dir(path);`
`301`		`- return Err(USimpleError::new(`
`302`		`- 1,`
`303`		`- format!("failed to set SELinux security context: {}", e),`
`304`		`- ));`
	`301`	`+ return Err(USimpleError::new(1, e.to_string()));`
`305`	`302`	`}`
`306`	`303`	`}`
`307`	`304`