Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace users with nix crate #9610

Merged
merged 2 commits into from
Jul 5, 2023
Merged

Replace users with nix crate #9610

merged 2 commits into from
Jul 5, 2023

Conversation

nibon7
Copy link
Contributor

@nibon7 nibon7 commented Jul 5, 2023

Description

The users crate hasn't been updated for a long time, this PR tries to replace users with nix.
See advisory page for additional details.

@WindSoilder

User-Facing Changes

Tests + Formatting

After Submitting

Copy link
Member

@sholderbach sholderbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for really cleaning up our dependency tree @nibon7 !

This change looks good to me! It doesn't seem like we need the maybe a bit more polished API of the users crate.

For the fn get_user_groups() @WindSoilder vendored in #9531 we would still need some additional logic to the FFI code provided by nix (that had a RUSTSEC advisory on its own a while back nix-rust/nix#1541). So I think we can keep the direct libc code for now.

If we would really want to get rid of the users dependency we would need to vendor the code of the is-root crate (hasn't been changed for 2 years. Actually complicated logic of this single file crate is building around winapi)

@nibon7
Copy link
Contributor Author

nibon7 commented Jul 5, 2023

If we would really want to get rid of the users dependency we would need to vendor the code of the is-root crate (hasn't been changed for 2 years. Actually complicated logic of this single file crate is building around winapi)

Can't agree more, and the unix branch of is_root could be replaced by nix::unistd::Uid::current().is_root()

@sholderbach
Copy link
Member

If you don't mind @nibon7 I would land this one now and then we can consider the is-root replacement.

@sholderbach sholderbach merged commit 687b0e1 into nushell:main Jul 5, 2023
@nibon7
Copy link
Contributor Author

nibon7 commented Jul 5, 2023

then we can consider the is-root replacement.

I write my own with windows crate, or we could find one in crates.io

diff --git a/Cargo.lock b/Cargo.lock
index 69568f71c..330c82ae2 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1909,16 +1909,6 @@ dependencies = [
  "once_cell",
 ]
 
-[[package]]
-name = "is-root"
-version = "0.1.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "04a4202a60e86f1c9702706bb42270dadd333f2db7810157563c86f17af3c873"
-dependencies = [
- "users",
- "winapi",
-]
-
 [[package]]
 name = "is-terminal"
 version = "0.4.7"
@@ -2777,7 +2767,6 @@ dependencies = [
  "htmlescape",
  "indexmap",
  "indicatif",
- "is-root",
  "itertools",
  "libc",
  "log",
@@ -5586,16 +5575,6 @@ dependencies = [
  "percent-encoding",
 ]
 
-[[package]]
-name = "users"
-version = "0.10.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "aa4227e95324a443c9fcb06e03d4d85e91aabe9a5a02aa818688b6918b6af486"
-dependencies = [
- "libc",
- "log",
-]
-
 [[package]]
 name = "utf-8"
 version = "0.7.6"
diff --git a/crates/nu-command/Cargo.toml b/crates/nu-command/Cargo.toml
index a7226f28c..46b903eca 100644
--- a/crates/nu-command/Cargo.toml
+++ b/crates/nu-command/Cargo.toml
@@ -51,7 +51,6 @@ fs_extra = "1.3"
 htmlescape = "0.3"
 indexmap = { version = "1.7", features = ["serde-1"] }
 indicatif = "0.17"
-is-root = "0.1"
 itertools = "0.10"
 log = "0.4"
 lscolors = { version = "0.14", default-features = false, features = ["nu-ansi-term"] }
@@ -111,7 +110,12 @@ optional = true
 version = "3.0"
 
 [target.'cfg(windows)'.dependencies.windows]
-features = ["Win32_Foundation", "Win32_Storage_FileSystem", "Win32_System_SystemServices"]
+features = ["Win32_Foundation",
+	"Win32_Storage_FileSystem",
+	"Win32_System_SystemServices",
+	"Win32_Security",
+	"Win32_System_Threading"
+]
 version = "0.48"
 
 [features]
diff --git a/crates/nu-command/src/experimental/is_admin.rs b/crates/nu-command/src/experimental/is_admin.rs
index 2fe379765..787d638f0 100644
--- a/crates/nu-command/src/experimental/is_admin.rs
+++ b/crates/nu-command/src/experimental/is_admin.rs
@@ -1,4 +1,3 @@
-use is_root::is_root;
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::{
@@ -48,3 +47,44 @@ impl Command for IsAdmin {
         ]
     }
 }
+
+#[cfg(unix)]
+fn is_root() -> bool {
+    nix::unistd::Uid::current().is_root()
+}
+
+#[cfg(windows)]
+fn is_root() -> bool {
+    use windows::Win32::{
+        Foundation::{CloseHandle, FALSE, INVALID_HANDLE_VALUE},
+        Security::{GetTokenInformation, TokenElevation, TOKEN_ELEVATION, TOKEN_QUERY},
+        System::Threading::{GetCurrentProcess, OpenProcessToken},
+    };
+
+    let mut handle = INVALID_HANDLE_VALUE;
+    let mut elevated = false;
+
+    unsafe {
+        if OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut handle) != FALSE {
+            let mut elevation = TOKEN_ELEVATION { TokenIsElevated: 0 };
+            let mut size = std::mem::size_of::<TOKEN_ELEVATION>() as u32;
+
+            if GetTokenInformation(
+                handle,
+                TokenElevation,
+                Some(&mut elevation as *mut TOKEN_ELEVATION as *mut _),
+                size,
+                &mut size,
+            ) != FALSE
+            {
+                elevated = elevation.TokenIsElevated != 0;
+            }
+        }
+
+        if handle != INVALID_HANDLE_VALUE {
+            CloseHandle(handle);
+        }
+    }
+
+    elevated
+}

@nibon7 nibon7 deleted the users branch July 5, 2023 22:24
@WindSoilder
Copy link
Collaborator

@nibon7 Thanks! It looks really good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants