Address review comments: refactor SMACK helpers and rename SELinux flags

ChrisDryden · ChrisDryden · commit f5db3c9b86eb · 2026-01-05T11:41:12.000-05:00
diff --git a/src/uu/id/locales/en-US.ftl b/src/uu/id/locales/en-US.ftl
@@ -18,7 +18,7 @@ id-error-names-real-ids-require-flags = printing only names or real IDs requires
 id-error-zero-not-permitted-default = option --zero not permitted in default format
 id-error-cannot-print-context-with-user = cannot print security context when user specified
 id-error-cannot-get-context = can't get process context
-id-error-context-selinux-only = --context (-Z) works only on an SELinux-enabled kernel
+id-error-context-security-only = --context (-Z) works only on an SELinux/SMACK-enabled kernel
 id-error-no-such-user = { $user }: no such user
 id-error-cannot-find-group-name = cannot find name for group ID { $gid }
 id-error-cannot-find-user-name = cannot find name for user ID { $uid }
diff --git a/src/uu/id/locales/fr-FR.ftl b/src/uu/id/locales/fr-FR.ftl
@@ -18,7 +18,7 @@ id-error-names-real-ids-require-flags = l'affichage des noms uniquement ou des I
 id-error-zero-not-permitted-default = l'option --zero n'est pas autorisée dans le format par défaut
 id-error-cannot-print-context-with-user = impossible d'afficher le contexte de sécurité quand un utilisateur est spécifié
 id-error-cannot-get-context = impossible d'obtenir le contexte du processus
-id-error-context-selinux-only = --context (-Z) ne fonctionne que sur un noyau avec SELinux activé
+id-error-context-security-only = --context (-Z) ne fonctionne que sur un noyau avec SELinux/SMACK activé
 id-error-no-such-user = { $user } : utilisateur inexistant
 id-error-cannot-find-group-name = impossible de trouver le nom pour l'ID de groupe { $gid }
 id-error-cannot-find-user-name = impossible de trouver le nom pour l'ID utilisateur { $uid }
diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs
@@ -210,7 +210,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         // Neither SELinux nor SMACK supported
         return Err(USimpleError::new(
             1,
-            translate!("id-error-context-selinux-only"),
+            translate!("id-error-context-security-only"),
         ));
     }
 
diff --git a/src/uu/mkdir/src/mkdir.rs b/src/uu/mkdir/src/mkdir.rs
@@ -27,7 +27,7 @@ mod options {
     pub const PARENTS: &str = "parents";
     pub const VERBOSE: &str = "verbose";
     pub const DIRS: &str = "dirs";
-    pub const SELINUX: &str = "z";
+    pub const SECURITY_CONTEXT: &str = "z";
     pub const CONTEXT: &str = "context";
 }
 
@@ -42,8 +42,8 @@ pub struct Config<'a> {
     /// Print message for each created directory.
     pub verbose: bool,
 
-    /// Set `SELinux` security context.
-    pub set_selinux_context: bool,
+    /// Set security context (SELinux/SMACK).
+    pub set_security_context: bool,
 
     /// Specific `SELinux` context.
     pub context: Option<&'a String>,
@@ -79,7 +79,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let recursive = matches.get_flag(options::PARENTS);
 
     // Extract the SELinux related flags and options
-    let set_selinux_context = matches.get_flag(options::SELINUX);
+    let set_security_context = matches.get_flag(options::SECURITY_CONTEXT);
     let context = matches.get_one::<String>(options::CONTEXT);
 
     match get_mode(&matches) {
@@ -88,7 +88,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
                 recursive,
                 mode,
                 verbose,
-                set_selinux_context: set_selinux_context || context.is_some(),
+                set_security_context: set_security_context || context.is_some(),
                 context,
             };
             exec(dirs, &config)
@@ -129,7 +129,7 @@ pub fn uu_app() -> Command {
                 .action(ArgAction::SetTrue),
         )
         .arg(
-            Arg::new(options::SELINUX)
+            Arg::new(options::SECURITY_CONTEXT)
                 .short('Z')
                 .help(translate!("mkdir-help-selinux"))
                 .action(ArgAction::SetTrue),
@@ -292,7 +292,7 @@ fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<(
 
             // Apply SELinux context if requested
             #[cfg(feature = "selinux")]
-            if config.set_selinux_context && uucore::selinux::is_selinux_enabled() {
+            if config.set_security_context && uucore::selinux::is_selinux_enabled() {
                 if let Err(e) = uucore::selinux::set_selinux_security_context(path, config.context)
                 {
                     let _ = std::fs::remove_dir(path);
@@ -302,13 +302,8 @@ fn create_single_dir(path: &Path, is_parent: bool, config: &Config) -> UResult<(
 
             // Apply SMACK context if requested
             #[cfg(feature = "smack")]
-            if config.set_selinux_context && uucore::smack::is_smack_enabled() {
-                if let Some(ctx) = config.context {
-                    if let Err(e) = uucore::smack::set_smack_label_for_path(path, ctx) {
-                        let _ = std::fs::remove_dir(path);
-                        return Err(USimpleError::new(1, e.to_string()));
-                    }
-                }
+            if config.set_security_context {
+                uucore::smack::set_smack_label_for_new_dir(path, config.context)?;
             }
             Ok(())
         }
diff --git a/src/uu/mkfifo/src/mkfifo.rs b/src/uu/mkfifo/src/mkfifo.rs
@@ -16,7 +16,7 @@ use uucore::{format_usage, show};
 
 mod options {
     pub static MODE: &str = "mode";
-    pub static SELINUX: &str = "Z";
+    pub static SECURITY_CONTEXT: &str = "Z";
     pub static CONTEXT: &str = "context";
     pub static FIFO: &str = "fifo";
 }
@@ -62,10 +62,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         #[cfg(feature = "selinux")]
         {
             // Extract the SELinux related flags and options
-            let set_selinux_context = matches.get_flag(options::SELINUX);
+            let set_security_context = matches.get_flag(options::SECURITY_CONTEXT);
             let context = matches.get_one::<String>(options::CONTEXT);
 
-            if set_selinux_context || context.is_some() {
+            if set_security_context || context.is_some() {
                 use std::path::Path;
                 if let Err(e) =
                     uucore::selinux::set_selinux_security_context(Path::new(&f), context)
@@ -79,17 +79,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         // Apply SMACK context if requested
         #[cfg(feature = "smack")]
         {
-            let set_smack_context = matches.get_flag(options::SELINUX);
+            let set_security_context = matches.get_flag(options::SECURITY_CONTEXT);
             let context = matches.get_one::<String>(options::CONTEXT);
-
-            if (set_smack_context || context.is_some()) && uucore::smack::is_smack_enabled() {
-                if let Some(ctx) = context {
-                    use std::path::Path;
-                    if let Err(e) = uucore::smack::set_smack_label_for_path(Path::new(&f), ctx) {
-                        let _ = fs::remove_file(&f);
-                        return Err(USimpleError::new(1, e.to_string()));
-                    }
-                }
+            if set_security_context || context.is_some() {
+                uucore::smack::set_smack_label_for_new_file(&f, context)?;
             }
         }
     }
@@ -112,7 +105,7 @@ pub fn uu_app() -> Command {
                 .value_name("MODE"),
         )
         .arg(
-            Arg::new(options::SELINUX)
+            Arg::new(options::SECURITY_CONTEXT)
                 .short('Z')
                 .help(translate!("mkfifo-help-selinux"))
                 .action(ArgAction::SetTrue),
diff --git a/src/uu/mknod/src/mknod.rs b/src/uu/mknod/src/mknod.rs
@@ -23,7 +23,7 @@ mod options {
     pub const TYPE: &str = "type";
     pub const MAJOR: &str = "major";
     pub const MINOR: &str = "minor";
-    pub const SELINUX: &str = "z";
+    pub const SECURITY_CONTEXT: &str = "z";
     pub const CONTEXT: &str = "context";
 }
 
@@ -54,10 +54,10 @@ pub struct Config<'a> {
 
     pub dev: dev_t,
 
-    /// Set `SELinux` security context.
-    pub set_selinux_context: bool,
+    /// Set security context (SELinux/SMACK).
+    pub set_security_context: bool,
 
-    /// Specific `SELinux` context.
+    /// Specific security context (SELinux/SMACK).
     pub context: Option<&'a String>,
 }
 
@@ -88,7 +88,7 @@ fn mknod(file_name: &str, config: Config) -> i32 {
 
         // Apply SELinux context if requested
         #[cfg(feature = "selinux")]
-        if config.set_selinux_context {
+        if config.set_security_context {
             if let Err(e) = uucore::selinux::set_selinux_security_context(
                 std::path::Path::new(file_name),
                 config.context,
@@ -102,16 +102,10 @@ fn mknod(file_name: &str, config: Config) -> i32 {
 
         // Apply SMACK context if requested
         #[cfg(feature = "smack")]
-        if config.set_selinux_context && uucore::smack::is_smack_enabled() {
-            if let Some(ctx) = config.context {
-                if let Err(e) =
-                    uucore::smack::set_smack_label_for_path(std::path::Path::new(file_name), ctx)
-                {
-                    // if it fails, delete the file
-                    let _ = std::fs::remove_file(file_name);
-                    eprintln!("{}: {}", uucore::util_name(), e);
-                    return 1;
-                }
+        if config.set_security_context {
+            if let Err(e) = uucore::smack::set_smack_label_for_new_file(file_name, config.context) {
+                eprintln!("{}: {}", uucore::util_name(), e);
+                return 1;
             }
         }
 
@@ -139,8 +133,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         .get_one::<String>("name")
         .expect("Missing argument 'NAME'");
 
-    // Extract the SELinux related flags and options
-    let set_selinux_context = matches.get_flag(options::SELINUX);
+    // Extract the security context related flags and options
+    let set_security_context = matches.get_flag(options::SECURITY_CONTEXT);
     let context = matches.get_one::<String>(options::CONTEXT);
 
     let dev = match (
@@ -168,7 +162,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         mode,
         use_umask,
         dev,
-        set_selinux_context: set_selinux_context || context.is_some(),
+        set_security_context: set_security_context || context.is_some(),
         context,
     };
 
@@ -219,7 +213,7 @@ pub fn uu_app() -> Command {
                 .value_parser(value_parser!(u64)),
         )
         .arg(
-            Arg::new(options::SELINUX)
+            Arg::new(options::SECURITY_CONTEXT)
                 .short('Z')
                 .help(translate!("mknod-help-selinux"))
                 .action(ArgAction::SetTrue),
diff --git a/src/uucore/src/lib/features/smack.rs b/src/uucore/src/lib/features/smack.rs
@@ -13,7 +13,7 @@ use std::sync::OnceLock;
 
 use thiserror::Error;
 
-use crate::error::{UError, strip_errno};
+use crate::error::{UError, USimpleError, strip_errno};
 use crate::translate;
 
 #[derive(Debug, Error)]
@@ -72,13 +72,9 @@ pub fn set_smack_label_for_self(label: &str) -> Result<(), SmackError> {
         return Err(SmackError::SmackNotEnabled);
     }
 
-    let label_owned = label.to_string();
     fs::File::create("/proc/self/attr/current")
-        .map_err(|e| SmackError::LabelSetFailure(label_owned.clone(), e))?
-        .write_all(label.as_bytes())
-        .map_err(|e| SmackError::LabelSetFailure(label_owned, e))?;
-
-    Ok(())
+        .and_then(|mut f| f.write_all(label.as_bytes()))
+        .map_err(|e| SmackError::LabelSetFailure(label.to_string(), e))
 }
 
 /// Gets the SMACK label for a filesystem path via xattr.
@@ -106,3 +102,35 @@ pub fn set_smack_label_for_path(path: &Path, label: &str) -> Result<(), SmackErr
     xattr::set(path, "security.SMACK64", label.as_bytes())
         .map_err(|e| SmackError::LabelSetFailure(label.to_string(), e))
 }
+
+/// Sets SMACK label for a file, removing it on failure.
+pub fn set_smack_label_for_new_file(
+    path: impl AsRef<Path>,
+    context: Option<&String>,
+) -> Result<(), Box<dyn UError>> {
+    let Some(ctx) = context else { return Ok(()) };
+    if !is_smack_enabled() {
+        return Ok(());
+    }
+    let path = path.as_ref();
+    set_smack_label_for_path(path, ctx).map_err(|e| {
+        let _ = fs::remove_file(path);
+        USimpleError::new(1, e.to_string())
+    })
+}
+
+/// Sets SMACK label for a directory, removing it on failure.
+pub fn set_smack_label_for_new_dir(
+    path: impl AsRef<Path>,
+    context: Option<&String>,
+) -> Result<(), Box<dyn UError>> {
+    let Some(ctx) = context else { return Ok(()) };
+    if !is_smack_enabled() {
+        return Ok(());
+    }
+    let path = path.as_ref();
+    set_smack_label_for_path(path, ctx).map_err(|e| {
+        let _ = fs::remove_dir(path);
+        USimpleError::new(1, e.to_string())
+    })
+}

Original file line number	Diff line number	Diff line change
`@@ -210,7 +210,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {`
`210`	`210`	`// Neither SELinux nor SMACK supported`
`211`	`211`	`return Err(USimpleError::new(`
`212`	`212`	`1,`
`213`		`- translate!("id-error-context-selinux-only"),`
	`213`	`+ translate!("id-error-context-security-only"),`
`214`	`214`	`));`
`215`	`215`	`}`
`216`	`216`