Get 'test' working properly with acls and supplementary groups

.vscode/cspell.dictionaries/workspace.wordlist.txt

@@ -152,7 +152,9 @@ vmsplice
 
 # * vars/libc
 COMFOLLOW
+EACCESS
 EXDEV
+FDCWD
 FILENO
 FTSENT
 HOSTSIZE
@@ -197,7 +199,10 @@ blocksize
 canonname
 chroot
 dlsym
+eaccess
+euidaccess
 execvp
+faccessat
 fdatasync
 freeaddrinfo
 getaddrinfo

src/uu/test/src/test.rs

@@ -258,26 +258,69 @@ enum PathCondition {
 
 #[cfg(not(windows))]
 fn path(path: &OsStr, condition: &PathCondition) -> bool {
-    use std::fs::Metadata;
+    use std::ffi::CString;
+    use std::os::unix::ffi::OsStrExt;
     use std::os::unix::fs::FileTypeExt;
 
     const S_ISUID: u32 = 0o4000;
     const S_ISGID: u32 = 0o2000;
     const S_ISVTX: u32 = 0o1000;
 
-    enum Permission {
-        Read = 0o4,
-        Write = 0o2,
-        Execute = 0o1,
-    }
+    // Helper function to check file access permissions.
+    // Uses platform-specific access functions that properly handle supplementary groups
+    // and ACLs.
+    let check_access = |path: &OsStr, mode: i32| -> bool {
+        let path_bytes = path.as_bytes();
+        let Ok(c_path) = CString::new(path_bytes) else {
+            return false; // Path contains null byte
+        };
 
-    let perm = |metadata: Metadata, p: Permission| {
-        if geteuid() == metadata.uid() {
-            metadata.mode() & ((p as u32) << 6) != 0
-        } else if getegid() == metadata.gid() {
-            metadata.mode() & ((p as u32) << 3) != 0
-        } else {
-            metadata.mode() & (p as u32) != 0
+        unsafe {
+            #[cfg(any(
+                target_os = "linux",
+                target_os = "hurd",
+                target_os = "cygwin",
+                target_os = "solaris",
+                target_os = "illumos"
+            ))]
+            {
+                libc::euidaccess(c_path.as_ptr(), mode) == 0
+            }
+
+            #[cfg(target_os = "freebsd")]
+            {
+                libc::eaccess(c_path.as_ptr(), mode) == 0
+            }
+
+            #[cfg(any(
+                target_os = "macos",
+                target_os = "ios",
+                target_os = "netbsd",
+                target_os = "openbsd",
+                target_os = "dragonfly"
+            ))]
+            {
+                libc::faccessat(libc::AT_FDCWD, c_path.as_ptr(), mode, libc::AT_EACCESS) == 0
+            }
+
+            // Fallback for other OSes
+            #[cfg(not(any(
+                target_os = "linux",
+                target_os = "hurd",
+                target_os = "cygwin",
+                target_os = "solaris",
+                target_os = "illumos",
+                target_os = "freebsd",
+                target_os = "macos",
+                target_os = "ios",
+                target_os = "netbsd",
+                target_os = "openbsd",
+                target_os = "dragonfly"
+            )))]
+            {
+                // fallback: use regular access()
+                libc::access(c_path.as_ptr(), mode) == 0
+            }
         }
     };
 
@@ -308,12 +351,12 @@ fn path(path: &OsStr, condition: &PathCondition) -> bool {
         PathCondition::Sticky => metadata.mode() & S_ISVTX != 0,
         PathCondition::UserOwns => metadata.uid() == geteuid(),
         PathCondition::Fifo => file_type.is_fifo(),
-        PathCondition::Readable => perm(metadata, Permission::Read),
+        PathCondition::Readable => check_access(path, libc::R_OK),
         PathCondition::Socket => file_type.is_socket(),
         PathCondition::NonEmpty => metadata.size() > 0,
         PathCondition::UserIdFlag => metadata.mode() & S_ISUID != 0,
-        PathCondition::Writable => perm(metadata, Permission::Write),
-        PathCondition::Executable => perm(metadata, Permission::Execute),
+        PathCondition::Writable => check_access(path, libc::W_OK),
+        PathCondition::Executable => check_access(path, libc::X_OK),
     }
 }
 

tests/by-util/test_test.rs

@@ -1034,3 +1034,218 @@ fn test_unary_op_as_literal_in_three_arg_form() {
     new_ucmd!().args(&["-f", "=", "a"]).fails_with_code(1);
     new_ucmd!().args(&["-f", "=", "a", "-o", "b"]).succeeds();
 }
+
+// Only test platforms supporting setfacl.
+#[test]
+#[cfg(any(target_os = "linux", target_os = "cygwin"))]
+fn test_permission_with_acl() {
+    // Test that permission checks (-r, -w, -x) correctly handle ACL (Access Control List)
+    // permissions, not just traditional Unix permissions. This requires using euidaccess()
+    // or equivalent, not manual permission bit checking.
+    //
+    // This test requires:
+    // - Running as a non-root user
+    // - sudo access
+    // - setfacl command available
+    use std::process::Command;
+
+    let scene = TestScenario::new(util_name!());
+
+    let is_root = scene.cmd("whoami").run().stdout_str().trim() == "root";
+
+    // Check if setfacl is available
+    if Command::new("which").arg("setfacl").output().is_err() {
+        return;
+    }
+
+    // Skip if we cannot run non-interactive sudo and we're not already root.
+    let sudo_ok = if is_root {
+        true
+    } else {
+        Command::new("sudo")
+            .env("LC_ALL", "C")
+            .args(["-E", "--non-interactive", "whoami"])
+            .output()
+            .ok()
+            .is_some_and(|output| output.status.success() && output.stdout == b"root\n")
+    };
+    if !sudo_ok {
+        return;
+    }
+
+    // Get current username for ACL
+    let Ok(username_output) = Command::new("whoami").output() else {
+        return;
+    };
+    let username = String::from_utf8_lossy(&username_output.stdout)
+        .trim()
+        .to_string();
+
+    let test_path = scene.fixtures.plus("test_acl_dir");
+    let test_path_str = test_path.to_str().unwrap();
+
+    // Create directory with root:root ownership and 750 permissions (no access for others)
+    let install_result = if is_root {
+        scene
+            .cmd("install")
+            .args(&["-d", "-m", "750", "-o", "root", "-g", "root", test_path_str])
+            .run()
+    } else {
+        scene
+            .cmd("sudo")
+            .args(&[
+                "-E",
+                "--non-interactive",
+                "install",
+                "-d",
+                "-m",
+                "750",
+                "-o",
+                "root",
+                "-g",
+                "root",
+                test_path_str,
+            ])
+            .run()
+    };
+
+    if !install_result.succeeded() {
+        return;
+    }
+
+    // Grant ACL permissions to current user (read + execute)
+    let setfacl_result = if is_root {
+        scene
+            .cmd("setfacl")
+            .args(&["-m", &format!("u:{username}:rx"), test_path_str])
+            .run()
+    } else {
+        scene
+            .cmd("sudo")
+            .args(&[
+                "-E",
+                "--non-interactive",
+                "setfacl",
+                "-m",
+                &format!("u:{username}:rx"),
+                test_path_str,
+            ])
+            .run()
+    };
+
+    if !setfacl_result.succeeded() {
+        // Clean up and skip test
+        let _ = if is_root {
+            scene.cmd("rm").args(&["-rf", test_path_str]).run()
+        } else {
+            scene
+                .cmd("sudo")
+                .args(&["-E", "--non-interactive", "rm", "-rf", test_path_str])
+                .run()
+        };
+        return;
+    }
+
+    // Test that -r correctly detects directory as readable (via ACL)
+    scene.ucmd().args(&["-r", test_path_str]).succeeds();
+
+    // Test that -x correctly detects directory as executable (via ACL)
+    scene.ucmd().args(&["-x", test_path_str]).succeeds();
+
+    // Test that -w correctly detects directory as NOT writable (ACL grants r-x, not w)
+    scene.ucmd().args(&["!", "-w", test_path_str]).succeeds();
+
+    // Clean up
+    let _ = if is_root {
+        scene.cmd("rm").args(&["-rf", test_path_str]).run()
+    } else {
+        scene
+            .cmd("sudo")
+            .args(&["-E", "--non-interactive", "rm", "-rf", test_path_str])
+            .run()
+    };
+}
+
+#[test]
+#[cfg(any(
+    target_os = "linux",
+    target_os = "hurd",
+    target_os = "solaris",
+    target_os = "illumos",
+    target_os = "freebsd",
+    target_os = "macos",
+    target_os = "netbsd",
+    target_os = "openbsd",
+    target_os = "dragonfly"
+))]
+fn test_permission_with_supplementary_group() {
+    // Test that permission checks (-r, -w, -x) correctly consider supplementary groups,
+    // not just the primary group (egid). See #9147
+    //
+    // This test requires:
+    // - Running as a non-root user
+    // - User must have at least one supplementary group
+    // - sudo access to create test files/dirs with specific ownership
+    use std::process::Command;
+
+    let scene = TestScenario::new(util_name!());
+
+    // Must be run as non-root user (otherwise owner permissions would apply)
+    if scene.cmd("whoami").run().stdout_str().trim() == "root" {
+        return;
+    }
+
+    // Get user's supplementary groups
+    let Ok(groups_output) = Command::new("id").arg("-nG").output() else {
+        return;
+    };
+
+    let groups_str = String::from_utf8_lossy(&groups_output.stdout);
+    let groups: Vec<&str> = groups_str.split_whitespace().collect();
+
+    // Need at least 2 groups (primary + at least one supplementary)
+    if groups.len() < 2 {
+        return;
+    }
+
+    let supplementary_group = groups[1];
+    let test_path = scene.fixtures.plus("test_supp_group_dir");
+    let test_path_str = test_path.to_str().unwrap();
+
+    // Try to create directory with root owner and supplementary group, mode 750
+    let install_result = scene
+        .cmd("sudo")
+        .args(&[
+            "-E",
+            "--non-interactive",
+            "install",
+            "-d",
+            "-m",
+            "750",
+            "-o",
+            "root",
+            "-g",
+            supplementary_group,
+            test_path_str,
+        ])
+        .run();
+
+    if !install_result.succeeded() {
+        return;
+    }
+
+    // Test that -r correctly detects directory as readable (via supplementary group)
+    scene.ucmd().args(&["-r", test_path_str]).succeeds();
+
+    // Test that -x correctly detects directory as executable (via supplementary group)
+    scene.ucmd().args(&["-x", test_path_str]).succeeds();
+
+    // Test that -w correctly detects directory as NOT writable (group has r-x, not w)
+    scene.ucmd().args(&["!", "-w", test_path_str]).succeeds();
+
+    // Clean up
+    let _ = scene
+        .cmd("sudo")
+        .args(&["-E", "--non-interactive", "rm", "-rf", test_path_str])
+        .run();
+}