Use safe nix API instead of libc (#12315)

# Description
Where possible, this PR replaces usages of raw `libc` bindings to
instead use safe interfaces from the `nix` crate. Where not possible,
the `libc` version reexported through `nix` was used instead of having a
separate `libc` dependency.

Cargo.lock

@@ -2867,7 +2867,6 @@ dependencies = [
  "indexmap",
  "indicatif",
  "itertools 0.12.0",
- "libc",
  "log",
  "lscolors",
  "md-5",

crates/nu-command/Cargo.toml

@@ -106,9 +106,8 @@ winreg = { workspace = true }
 uucore = { workspace = true, features = ["mode"] }
 
 [target.'cfg(unix)'.dependencies]
-libc = { workspace = true }
 umask = { workspace = true }
-nix = { workspace = true, default-features = false, features = ["user", "resource"] }
+nix = { workspace = true, default-features = false, features = ["user", "resource", "pthread"] }
 
 [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
 procfs = { workspace = true }

crates/nu-command/src/debug/info.rs

@@ -71,7 +71,7 @@ impl LazySystemInfoRecord {
     ) -> Result<Value, ShellError> {
         let pid = Pid::from(std::process::id() as usize);
         match column {
-            "thread_id" => Ok(Value::int(get_thread_id(), self.span)),
+            "thread_id" => Ok(Value::int(get_thread_id() as i64, self.span)),
             "pid" => Ok(Value::int(pid.as_u32() as i64, self.span)),
             "ppid" => {
                 // only get information requested
@@ -261,13 +261,13 @@ impl<'a, F: Fn() -> RefreshKind> From<(Option<&'a System>, F)> for SystemOpt<'a>
     }
 }
 
-fn get_thread_id() -> i64 {
+fn get_thread_id() -> u64 {
-    #[cfg(target_family = "windows")]
+    #[cfg(windows)]
     {
-        unsafe { windows::Win32::System::Threading::GetCurrentThreadId() as i64 }
+        unsafe { windows::Win32::System::Threading::GetCurrentThreadId().into() }
     }
-    #[cfg(not(target_family = "windows"))]
+    #[cfg(unix)]
     {
-        unsafe { libc::pthread_self() as i64 }
+        nix::sys::pthread::pthread_self() as u64
     }
 }

crates/nu-command/src/filesystem/cd.rs

@@ -1,16 +1,14 @@
-#[cfg(unix)]
-use libc::gid_t;
 use nu_engine::{command_prelude::*, current_dir};
 use std::path::Path;
-
-// For checking whether we have permission to cd to a directory
 #[cfg(unix)]
-mod file_permissions {
+use {
-    pub type Mode = u32;
+    crate::filesystem::util::users,
-    pub const USER_EXECUTE: Mode = libc::S_IXUSR as Mode;
+    nix::{
-    pub const GROUP_EXECUTE: Mode = libc::S_IXGRP as Mode;
+        sys::stat::{mode_t, Mode},
-    pub const OTHER_EXECUTE: Mode = libc::S_IXOTH as Mode;
+        unistd::{Gid, Uid},
-}
+    },
+    std::os::unix::fs::MetadataExt,
+};
 
 // The result of checking whether we have permission to cd to a directory
 #[derive(Debug)]
@@ -170,26 +168,22 @@ fn have_permission(dir: impl AsRef<Path>) -> PermissionResult<'static> {
 
 #[cfg(unix)]
 fn have_permission(dir: impl AsRef<Path>) -> PermissionResult<'static> {
-    use crate::filesystem::util::users;
-
     match dir.as_ref().metadata() {
         Ok(metadata) => {
-            use std::os::unix::fs::MetadataExt;
+            let mode = Mode::from_bits_truncate(metadata.mode() as mode_t);
-            let bits = metadata.mode();
-            let has_bit = |bit| bits & bit == bit;
             let current_user_uid = users::get_current_uid();
-            if current_user_uid == 0 {
+            if current_user_uid.is_root() {
                 return PermissionResult::PermissionOk;
             }
             let current_user_gid = users::get_current_gid();
-            let owner_user = metadata.uid();
+            let owner_user = Uid::from_raw(metadata.uid());
-            let owner_group = metadata.gid();
+            let owner_group = Gid::from_raw(metadata.gid());
             match (
                 current_user_uid == owner_user,
                 current_user_gid == owner_group,
             ) {
                 (true, _) => {
-                    if has_bit(file_permissions::USER_EXECUTE) {
+                    if mode.contains(Mode::S_IXUSR) {
                         PermissionResult::PermissionOk
                     } else {
                         PermissionResult::PermissionDenied(
@@ -198,7 +192,7 @@ fn have_permission(dir: impl AsRef<Path>) -> PermissionResult<'static> {
                     }
                 }
                 (false, true) => {
-                    if has_bit(file_permissions::GROUP_EXECUTE) {
+                    if mode.contains(Mode::S_IXGRP) {
                         PermissionResult::PermissionOk
                     } else {
                         PermissionResult::PermissionDenied(
@@ -207,8 +201,8 @@ fn have_permission(dir: impl AsRef<Path>) -> PermissionResult<'static> {
                     }
                 }
                 (false, false) => {
-                    if has_bit(file_permissions::OTHER_EXECUTE)
+                    if mode.contains(Mode::S_IXOTH)
-                        || (has_bit(file_permissions::GROUP_EXECUTE)
+                        || (mode.contains(Mode::S_IXGRP)
                             && any_group(current_user_gid, owner_group))
                     {
                         PermissionResult::PermissionOk
@@ -225,24 +219,19 @@ fn have_permission(dir: impl AsRef<Path>) -> PermissionResult<'static> {
 }
 
 #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "android"))]
-fn any_group(_current_user_gid: gid_t, owner_group: u32) -> bool {
+fn any_group(_current_user_gid: Gid, owner_group: Gid) -> bool {
-    use crate::filesystem::util::users;
+    users::current_user_groups()
-    let Some(user_groups) = users::current_user_groups() else {
+        .unwrap_or_default()
-        return false;
+        .contains(&owner_group)
-    };
-    user_groups.iter().any(|gid| gid.as_raw() == owner_group)
 }
 
 #[cfg(all(
     unix,
     not(any(target_os = "linux", target_os = "freebsd", target_os = "android"))
 ))]
-fn any_group(current_user_gid: gid_t, owner_group: u32) -> bool {
+fn any_group(current_user_gid: Gid, owner_group: Gid) -> bool {
-    use crate::filesystem::util::users;
-
     users::get_current_username()
         .and_then(|name| users::get_user_groups(&name, current_user_gid))
         .unwrap_or_default()
-        .into_iter()
+        .contains(&owner_group)
-        .any(|gid| gid.as_raw() == owner_group)
 }

crates/nu-command/src/filesystem/ls.rs

@@ -511,6 +511,7 @@ pub(crate) fn dir_entry_dict(
             {
                 use crate::filesystem::util::users;
                 use std::os::unix::fs::MetadataExt;
+
                 let mode = md.permissions().mode();
                 record.push(
                     "mode",
@@ -525,19 +526,19 @@ pub(crate) fn dir_entry_dict(
 
                 record.push(
                     "user",
-                    if let Some(user) = users::get_user_by_uid(md.uid()) {
+                    if let Some(user) = users::get_user_by_uid(md.uid().into()) {
                         Value::string(user.name, span)
                     } else {
-                        Value::int(md.uid() as i64, span)
+                        Value::int(md.uid().into(), span)
                     },
                 );
 
                 record.push(
                     "group",
-                    if let Some(group) = users::get_group_by_gid(md.gid()) {
+                    if let Some(group) = users::get_group_by_gid(md.gid().into()) {
                         Value::string(group.name, span)
                     } else {
-                        Value::int(md.gid() as i64, span)
+                        Value::int(md.gid().into(), span)
                     },
                 );
             }

crates/nu-command/src/filesystem/util.rs

@@ -92,57 +92,40 @@ pub fn is_older(src: &Path, dst: &Path) -> Option<bool> {
 
 #[cfg(unix)]
 pub mod users {
-    use libc::{gid_t, uid_t};
     use nix::unistd::{Gid, Group, Uid, User};
 
-    pub fn get_user_by_uid(uid: uid_t) -> Option<User> {
+    pub fn get_user_by_uid(uid: Uid) -> Option<User> {
-        User::from_uid(Uid::from_raw(uid)).ok().flatten()
+        User::from_uid(uid).ok().flatten()
     }
 
-    pub fn get_group_by_gid(gid: gid_t) -> Option<Group> {
+    pub fn get_group_by_gid(gid: Gid) -> Option<Group> {
-        Group::from_gid(Gid::from_raw(gid)).ok().flatten()
+        Group::from_gid(gid).ok().flatten()
     }
 
-    pub fn get_current_uid() -> uid_t {
+    pub fn get_current_uid() -> Uid {
-        Uid::current().as_raw()
+        Uid::current()
     }
 
-    pub fn get_current_gid() -> gid_t {
+    pub fn get_current_gid() -> Gid {
-        Gid::current().as_raw()
+        Gid::current()
     }
 
     #[cfg(not(any(target_os = "linux", target_os = "freebsd", target_os = "android")))]
     pub fn get_current_username() -> Option<String> {
-        User::from_uid(Uid::current())
+        get_user_by_uid(get_current_uid()).map(|user| user.name)
-            .ok()
-            .flatten()
-            .map(|user| user.name)
     }
 
     #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "android"))]
     pub fn current_user_groups() -> Option<Vec<Gid>> {
-        // SAFETY:
+        if let Ok(mut groups) = nix::unistd::getgroups() {
-        // if first arg is 0 then it ignores second argument and returns number of groups present for given user.
+            groups.sort_unstable_by_key(|id| id.as_raw());
-        let ngroups = unsafe { libc::getgroups(0, core::ptr::null::<gid_t> as *mut _) };
+            groups.dedup();
-        let mut buff: Vec<gid_t> = vec![0; ngroups as usize];
+            Some(groups)
-
-        // SAFETY:
-        // buff is the size of ngroups and  getgroups reads max ngroups elements into buff
-        let found = unsafe { libc::getgroups(ngroups, buff.as_mut_ptr()) };
-
-        if found < 0 {
-            None
         } else {
-            buff.truncate(found as usize);
+            None
-            buff.sort_unstable();
-            buff.dedup();
-            buff.into_iter()
-                .filter_map(|i| get_group_by_gid(i as gid_t))
-                .map(|group| group.gid)
-                .collect::<Vec<_>>()
-                .into()
         }
     }
+
     /// Returns groups for a provided user name and primary group id.
     ///
     /// # libc functions used
@@ -159,19 +142,19 @@ pub mod users {
     /// }
     /// ```
     #[cfg(not(any(target_os = "linux", target_os = "freebsd", target_os = "android")))]
-    pub fn get_user_groups(username: &str, gid: gid_t) -> Option<Vec<Gid>> {
+    pub fn get_user_groups(username: &str, gid: Gid) -> Option<Vec<Gid>> {
+        use nix::libc::{c_int, gid_t};
         use std::ffi::CString;
+
         // MacOS uses i32 instead of gid_t in getgrouplist for unknown reasons
         #[cfg(target_os = "macos")]
         let mut buff: Vec<i32> = vec![0; 1024];
         #[cfg(not(target_os = "macos"))]
         let mut buff: Vec<gid_t> = vec![0; 1024];
 
-        let Ok(name) = CString::new(username.as_bytes()) else {
+        let name = CString::new(username).ok()?;
-            return None;
-        };
 
-        let mut count = buff.len() as libc::c_int;
+        let mut count = buff.len() as c_int;
 
         // MacOS uses i32 instead of gid_t in getgrouplist for unknown reasons
         // SAFETY:
@@ -182,11 +165,19 @@ pub mod users {
         // The capacity for `*groups` is passed in as `*ngroups` which is the buffer max length/capacity (as we initialize with 0)
         // Following reads from `*groups`/`buff` will only happen after `buff.truncate(*ngroups)`
         #[cfg(target_os = "macos")]
-        let res =
+        let res = unsafe {
-            unsafe { libc::getgrouplist(name.as_ptr(), gid as i32, buff.as_mut_ptr(), &mut count) };
+            nix::libc::getgrouplist(
+                name.as_ptr(),
+                gid.as_raw() as i32,
+                buff.as_mut_ptr(),
+                &mut count,
+            )
+        };
 
         #[cfg(not(target_os = "macos"))]
-        let res = unsafe { libc::getgrouplist(name.as_ptr(), gid, buff.as_mut_ptr(), &mut count) };
+        let res = unsafe {
+            nix::libc::getgrouplist(name.as_ptr(), gid.as_raw(), buff.as_mut_ptr(), &mut count)
+        };
 
         if res < 0 {
             None
@@ -196,11 +187,13 @@ pub mod users {
             buff.dedup();
             // allow trivial cast: on macos i is i32, on linux it's already gid_t
             #[allow(trivial_numeric_casts)]
-            buff.into_iter()
+            Some(
-                .filter_map(|i| get_group_by_gid(i as gid_t))
+                buff.into_iter()
-                .map(|group| group.gid)
+                    .map(|id| Gid::from_raw(id as gid_t))
-                .collect::<Vec<_>>()
+                    .filter_map(get_group_by_gid)
-                .into()
+                    .map(|group| group.gid)
+                    .collect(),
+            )
         }
     }
 }

crates/nu-command/src/system/run_external.rs

@@ -533,44 +533,38 @@ impl ExternalCommand {
                 // Create a thread to wait for an exit code.
                 thread::Builder::new()
                     .name("exit code waiter".into())
-                    .spawn(move || {
+                    .spawn(move || match child.as_mut().wait() {
-                        match child.as_mut().wait() {
+                        Err(err) => Err(ShellError::ExternalCommand {
-                            Err(err) => Err(ShellError::ExternalCommand {
+                            label: "External command exited with error".into(),
-                                label: "External command exited with error".into(),
+                            help: err.to_string(),
-                                help: err.to_string(),
+                            span,
-                                span
+                        }),
-                            }),
+                        Ok(x) => {
-                            Ok(x) => {
+                            #[cfg(unix)]
-                                #[cfg(unix)]
+                            {
-                                {
+                                use nix::sys::signal::Signal;
-                                    use nu_ansi_term::{Color, Style};
+                                use nu_ansi_term::{Color, Style};
-                                    use std::ffi::CStr;
+                                use std::os::unix::process::ExitStatusExt;
-                                    use std::os::unix::process::ExitStatusExt;
 
-                                    if x.core_dumped() {
+                                if x.core_dumped() {
-                                        let cause = x.signal().and_then(|sig| unsafe {
+                                    let cause = x
-                                            // SAFETY: We should be the first to call `char * strsignal(int sig)`
+                                        .signal()
-                                            let sigstr_ptr = libc::strsignal(sig);
+                                        .and_then(|sig| {
-                                            if sigstr_ptr.is_null() {
+                                            Signal::try_from(sig).ok().map(Signal::as_str)
-                                                return None;
+                                        })
-                                            }
+                                        .unwrap_or("Something went wrong");
-
-                                            // SAFETY: The pointer points to a valid non-null string
-                                            let sigstr = CStr::from_ptr(sigstr_ptr);
-                                            sigstr.to_str().map(String::from).ok()
-                                        });
-
-                                        let cause = cause.as_deref().unwrap_or("Something went wrong");
 
                                     let style = Style::new().bold().on(Color::Red);
-                                    eprintln!(
+                                    let message = format!(
-                                        "{}",
+                                        "{cause}: child process '{commandname}' core dumped"
-                                        style.paint(format!(
-                                            "{cause}: oops, process '{commandname}' core dumped"
-                                        ))
                                     );
-                                    let _ = exit_code_tx.send(Value::error (
+                                    eprintln!("{}", style.paint(&message));
-                                        ShellError::ExternalCommand { label: "core dumped".to_string(), help: format!("{cause}: child process '{commandname}' core dumped"), span: head },
+                                    let _ = exit_code_tx.send(Value::error(
+                                        ShellError::ExternalCommand {
+                                            label: "core dumped".into(),
+                                            help: message,
+                                            span: head,
+                                        },
                                         head,
                                     ));
                                     return Ok(());
@@ -585,8 +579,8 @@ impl ExternalCommand {
                             }
                             Ok(())
                         }
-                    }
+                    })
-                }).map_err(|e| e.into_spanned(head))?;
+                    .map_err(|e| e.into_spanned(head))?;
 
                 let exit_code_receiver = ValueReceiver::new(exit_code_rx);
 

crates/nu-system/src/foreground.rs

@@ -120,7 +120,7 @@ mod foreground_pgroup {
     /// Currently only intended to access `tcsetpgrp` and `tcgetpgrp` with the I/O safe `nix`
     /// interface.
     pub unsafe fn stdin_fd() -> impl AsFd {
-        unsafe { BorrowedFd::borrow_raw(libc::STDIN_FILENO) }
+        unsafe { BorrowedFd::borrow_raw(nix::libc::STDIN_FILENO) }
     }
 
     pub fn prepare_command(external_command: &mut Command, existing_pgrp: u32) {