diff --git a/gix-worktree-state/src/checkout/entry.rs b/gix-worktree-state/src/checkout/entry.rs index bf37fd9aad2..7e84b93a74c 100644 --- a/gix-worktree-state/src/checkout/entry.rs +++ b/gix-worktree-state/src/checkout/entry.rs @@ -1,6 +1,6 @@ -use std::borrow::Cow; use std::{ - fs::OpenOptions, + borrow::Cow, + fs::{OpenOptions, Permissions}, io::Write, path::{Path, PathBuf}, }; @@ -286,10 +286,10 @@ pub(crate) fn finalize_entry( // For possibly existing, overwritten files, we must change the file mode explicitly. #[cfg(unix)] if let Some(path) = set_executable_after_creation { - use std::os::unix::fs::PermissionsExt; - let mut perm = std::fs::symlink_metadata(path)?.permissions(); - perm.set_mode(0o777); - std::fs::set_permissions(path, perm)?; + let old_perm = std::fs::symlink_metadata(path)?.permissions(); + if let Some(new_perm) = set_mode_executable(old_perm) { + std::fs::set_permissions(path, new_perm)?; + } } // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. // revisit this once there is a bug to fix. @@ -297,3 +297,103 @@ pub(crate) fn finalize_entry( file.close()?; Ok(()) } + +#[cfg(unix)] +fn set_mode_executable(mut perm: Permissions) -> Option { + use std::os::unix::fs::PermissionsExt; + let mut mode = perm.mode(); + if mode & 0o170000 != 0o100000 { + return None; // Stop if we don't have a regular file anymore. + } + mode &= 0o777; // Clear non-rwx bits (setuid, setgid, sticky). + mode |= (mode & 0o444) >> 2; // Let readers also execute. + perm.set_mode(mode); + Some(perm) +} + +#[cfg(all(test, unix))] +mod tests { + fn pretty(maybe_mode: Option) -> String { + match maybe_mode { + Some(mode) => format!("Some({mode:04o})"), + None => "None".into(), + } + } + + #[test] + fn set_mode_executable() { + let cases = [ + // Common cases: + (0o100755, Some(0o755)), + (0o100644, Some(0o755)), + (0o100750, Some(0o750)), + (0o100640, Some(0o750)), + (0o100700, Some(0o700)), + (0o100600, Some(0o700)), + (0o100775, Some(0o775)), + (0o100664, Some(0o775)), + (0o100770, Some(0o770)), + (0o100660, Some(0o770)), + (0o100764, Some(0o775)), + (0o100760, Some(0o770)), + // Less common: + (0o100674, Some(0o775)), + (0o100670, Some(0o770)), + (0o100000, Some(0o000)), + (0o100400, Some(0o500)), + (0o100440, Some(0o550)), + (0o100444, Some(0o555)), + (0o100462, Some(0o572)), + (0o100242, Some(0o252)), + (0o100167, Some(0o177)), + // With set-user-ID, set-group-ID, and sticky bits: + (0o104755, Some(0o755)), + (0o104644, Some(0o755)), + (0o102755, Some(0o755)), + (0o102644, Some(0o755)), + (0o101755, Some(0o755)), + (0o101644, Some(0o755)), + (0o106755, Some(0o755)), + (0o106644, Some(0o755)), + (0o104750, Some(0o750)), + (0o104640, Some(0o750)), + (0o102750, Some(0o750)), + (0o102640, Some(0o750)), + (0o101750, Some(0o750)), + (0o101640, Some(0o750)), + (0o106750, Some(0o750)), + (0o106640, Some(0o750)), + (0o107644, Some(0o755)), + (0o107000, Some(0o000)), + (0o106400, Some(0o500)), + (0o102462, Some(0o572)), + // Where it was replaced with a directory due to a race: + (0o040755, None), + (0o040644, None), + (0o040600, None), + (0o041755, None), + (0o041644, None), + (0o046644, None), + // Where it was replaced with a symlink due to a race: + (0o120777, None), + (0o120644, None), + // Where it was replaced with some other non-regular file due to a race: + (0o140644, None), + (0o060644, None), + (0o020644, None), + (0o010644, None), + ]; + for (old_mode, expected) in cases { + use std::os::unix::fs::PermissionsExt; + let old_perm = std::fs::Permissions::from_mode(old_mode); + let actual = super::set_mode_executable(old_perm).map(|perm| perm.mode()); + assert_eq!( + actual, + expected, + "{old_mode:06o} should become {}, became {}", + pretty(expected), + pretty(actual) + ); + } + } +} diff --git a/gix-worktree-state/tests/state/checkout.rs b/gix-worktree-state/tests/state/checkout.rs index c496d04688b..f69a5d748f1 100644 --- a/gix-worktree-state/tests/state/checkout.rs +++ b/gix-worktree-state/tests/state/checkout.rs @@ -220,9 +220,32 @@ fn overwriting_files_and_lone_directories_works() -> crate::Result { let meta = std::fs::symlink_metadata(exe)?; assert!(meta.is_file()); + #[cfg(unix)] if opts.fs.executable_bit { - #[cfg(unix)] - assert_eq!(meta.mode() & 0o700, 0o700, "the executable bit is set where supported"); + let mode = meta.mode(); + assert_eq!( + mode & 0o100, + 0o100, + "executable bit set where supported ({:04o} & {:04o} = {:04o} should be {:04o})", + mode, + 0o100, + mode & 0o100, + 0o100 + ); + let umask_write = gix_testtools::umask() & 0o222; + assert_eq!( + mode & umask_write, + 0, + "no excessive write bits are set ({:04o} & {:04o} = {:04o} should be {:04o})", + mode, + umask_write, + mode & umask_write, + 0 + ); + assert_ne!( + umask_write, 0, + "test not meaningful unless runner umask restricts some writes" + ); } assert_eq!( diff --git a/tests/tools/src/lib.rs b/tests/tools/src/lib.rs index e3ce748cc5c..cb91c0b87c6 100644 --- a/tests/tools/src/lib.rs +++ b/tests/tools/src/lib.rs @@ -944,6 +944,19 @@ pub fn size_ok(actual_size: usize, expected_64_bit_size: usize) -> bool { return actual_size <= expected_64_bit_size; } +/// Get the umask in a way that is safe, but may be too slow for use outside of tests. +#[cfg(unix)] +pub fn umask() -> u32 { + let output = std::process::Command::new("/bin/sh") + .args(["-c", "umask"]) + .output() + .expect("can execute `sh -c umask`"); + assert!(output.status.success(), "`sh -c umask` failed"); + assert!(output.stderr.is_empty(), "`sh -c umask` unexpected message"); + let text = output.stdout.to_str().expect("valid Unicode").trim(); + u32::from_str_radix(text, 8).expect("parses as octal number") +} + #[cfg(test)] mod tests { use super::*; diff --git a/tests/tools/src/main.rs b/tests/tools/src/main.rs index bfe68bbe59c..99d944e32af 100644 --- a/tests/tools/src/main.rs +++ b/tests/tools/src/main.rs @@ -1,4 +1,5 @@ use std::{fs, io, io::prelude::*, path::PathBuf}; + fn mess_in_the_middle(path: PathBuf) -> io::Result<()> { let mut file = fs::OpenOptions::new().read(false).write(true).open(path)?; file.seek(io::SeekFrom::Start(file.metadata()?.len() / 2))?; @@ -6,11 +7,19 @@ fn mess_in_the_middle(path: PathBuf) -> io::Result<()> { Ok(()) } +#[cfg(unix)] +fn umask() -> io::Result<()> { + println!("{:04o}", gix_testtools::umask()); + Ok(()) +} + fn main() -> Result<(), Box> { let mut args = std::env::args().skip(1); let scmd = args.next().expect("sub command"); match &*scmd { "mess-in-the-middle" => mess_in_the_middle(PathBuf::from(args.next().expect("path to file to mess with")))?, + #[cfg(unix)] + "umask" => umask()?, _ => unreachable!("Unknown subcommand: {}", scmd), }; Ok(()) diff --git a/tests/tools/tests/umask.rs b/tests/tools/tests/umask.rs new file mode 100644 index 00000000000..0a4cc04250b --- /dev/null +++ b/tests/tools/tests/umask.rs @@ -0,0 +1,19 @@ +#[test] +#[cfg(unix)] +#[cfg_attr(not(target_os = "linux"), ignore = "The test itself uses /proc")] +fn umask() { + use std::fs::File; + use std::io::{BufRead, BufReader}; + + use bstr::ByteSlice; + // Check against the umask obtained via a less portable but also completely safe method. + let less_portable = BufReader::new(File::open("/proc/self/status").expect("can open")) + .split(b'\n') + .find_map(|line| line.expect("can read").strip_prefix(b"Umask:\t").map(ToOwned::to_owned)) + .expect("has umask line") + .to_str() + .expect("umask line is valid UTF-8") + .to_owned(); + let more_portable = format!("{:04o}", gix_testtools::umask()); + assert_eq!(more_portable, less_portable); +}