diff --git a/.vscode/cspell.dictionaries/workspace.wordlist.txt b/.vscode/cspell.dictionaries/workspace.wordlist.txt index 28c468d4f9c..b0d3b5f074a 100644 --- a/.vscode/cspell.dictionaries/workspace.wordlist.txt +++ b/.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 diff --git a/src/uu/test/src/test.rs b/src/uu/test/src/test.rs index 0e4e809d760..08c362819d8 100644 --- a/src/uu/test/src/test.rs +++ b/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), } } diff --git a/tests/by-util/test_test.rs b/tests/by-util/test_test.rs index 21ea1893e99..19aebbe6576 100644 --- a/tests/by-util/test_test.rs +++ b/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(); +}