diff --git a/Cargo.lock b/Cargo.lock index d022dc163a6..bdf659a431d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2928,6 +2928,7 @@ dependencies = [ "gix-path 0.10.14", "gix-worktree 0.39.0", "io-close", + "rustix", "thiserror 2.0.3", ] diff --git a/gix-worktree-state/Cargo.toml b/gix-worktree-state/Cargo.toml index deefbdf6b8e..5f5a2b960bb 100644 --- a/gix-worktree-state/Cargo.toml +++ b/gix-worktree-state/Cargo.toml @@ -29,3 +29,9 @@ gix-filter = { version = "^0.17.0", path = "../gix-filter" } io-close = "0.3.7" thiserror = "2.0.0" bstr = { version = "1.3.0", default-features = false } + +[target.'cfg(unix)'.dependencies] +rustix = { version = "0.38.20", default-features = false, features = [ + "std", + "fs", +] } diff --git a/gix-worktree-state/src/checkout/chunk.rs b/gix-worktree-state/src/checkout/chunk.rs index 7f23722f59e..92663ae48d6 100644 --- a/gix-worktree-state/src/checkout/chunk.rs +++ b/gix-worktree-state/src/checkout/chunk.rs @@ -177,7 +177,6 @@ where // We process each key and do as the filter process tells us, while collecting data about the overall progress. let keys: BTreeSet<_> = delayed_filter_results.iter().map(|d| d.key.clone()).collect(); let mut unknown_paths = Vec::new(); - let mut rela_path_as_path = Default::default(); for key in keys { loop { let rela_paths = ctx.filters.driver_state_mut().list_delayed_paths(&key)?; @@ -229,10 +228,7 @@ where entry::finalize_entry( delayed.entry, write.inner.into_inner().map_err(std::io::IntoInnerError::into_error)?, - set_executable_after_creation.then(|| { - rela_path_as_path = gix_path::from_bstr(delayed.entry_path); - rela_path_as_path.as_ref() - }), + set_executable_after_creation, )?; delayed_files += 1; files.fetch_add(1, Ordering::Relaxed); diff --git a/gix-worktree-state/src/checkout/entry.rs b/gix-worktree-state/src/checkout/entry.rs index aff4ac01032..8df37947113 100644 --- a/gix-worktree-state/src/checkout/entry.rs +++ b/gix-worktree-state/src/checkout/entry.rs @@ -135,7 +135,7 @@ where }; // For possibly existing, overwritten files, we must change the file mode explicitly. - finalize_entry(entry, file, set_executable_after_creation.then_some(dest))?; + finalize_entry(entry, file, set_executable_after_creation)?; num_bytes } gix_index::entry::Mode::SYMLINK => { @@ -275,20 +275,16 @@ pub(crate) fn open_file( try_op_or_unlink(path, overwrite_existing, |p| options.open(p)).map(|f| (f, set_executable_after_creation)) } -/// Close `file` and store its stats in `entry`, possibly setting `file` executable depending on `set_executable_after_creation`. -#[cfg_attr(windows, allow(unused_variables))] +/// Close `file` and store its stats in `entry`, possibly setting `file` executable. pub(crate) fn finalize_entry( entry: &mut gix_index::Entry, file: std::fs::File, - set_executable_after_creation: Option<&Path>, + #[cfg_attr(windows, allow(unused_variables))] set_executable_after_creation: bool, ) -> Result<(), crate::checkout::Error> { // For possibly existing, overwritten files, we must change the file mode explicitly. #[cfg(unix)] - if let Some(path) = set_executable_after_creation { - 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)?; - } + if set_executable_after_creation { + set_executable(&file)?; } // 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,102 +293,104 @@ pub(crate) fn finalize_entry( Ok(()) } +/// Use `fstat` and `fchmod` on a file descriptor to make a regular file executable. +/// +/// See `let_readers_execute` for the exact details of how the mode is transformed. #[cfg(unix)] -fn set_mode_executable(mut perm: std::fs::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) +fn set_executable(file: &std::fs::File) -> Result<(), std::io::Error> { + let old_raw_mode = rustix::fs::fstat(file)?.st_mode; + let new_mode = let_readers_execute(old_raw_mode); + rustix::fs::fchmod(file, new_mode)?; + Ok(()) +} + +/// Given the st_mode of a regular file, compute the mode with executable bits safely added. +/// +/// Currently this adds executable bits for whoever has read bits already. It doesn't use the umask. +/// Set-user-ID and set-group-ID bits are unset for safety. The sticky bit is also unset. +/// +/// This returns only mode bits, not file type. The return value can be passed to chmod or fchmod. +#[cfg(unix)] +fn let_readers_execute(mut raw_mode: rustix::fs::RawMode) -> rustix::fs::Mode { + assert_eq!( + raw_mode & 0o170000, + 0o100000, + "bug in caller if not from a regular file" + ); + raw_mode &= 0o777; // Clear type, non-rwx mode bits (setuid, setgid, sticky). + raw_mode |= (raw_mode & 0o444) >> 2; // Let readers also execute. + rustix::fs::Mode::from_bits(raw_mode).expect("all bits recognized") } #[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() { + fn let_readers_execute() { 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)), + (0o100755, 0o755), + (0o100644, 0o755), + (0o100750, 0o750), + (0o100640, 0o750), + (0o100700, 0o700), + (0o100600, 0o700), + (0o100775, 0o775), + (0o100664, 0o775), + (0o100770, 0o770), + (0o100660, 0o770), + (0o100764, 0o775), + (0o100760, 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)), + (0o100674, 0o775), + (0o100670, 0o770), + (0o100000, 0o000), + (0o100400, 0o500), + (0o100440, 0o550), + (0o100444, 0o555), + (0o100462, 0o572), + (0o100242, 0o252), + (0o100167, 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), + (0o104755, 0o755), + (0o104644, 0o755), + (0o102755, 0o755), + (0o102644, 0o755), + (0o101755, 0o755), + (0o101644, 0o755), + (0o106755, 0o755), + (0o106644, 0o755), + (0o104750, 0o750), + (0o104640, 0o750), + (0o102750, 0o750), + (0o102640, 0o750), + (0o101750, 0o750), + (0o101640, 0o750), + (0o106750, 0o750), + (0o106640, 0o750), + (0o107644, 0o755), + (0o107000, 0o000), + (0o106400, 0o500), + (0o102462, 0o572), ]; - 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()); + for (st_mode, raw_expected) in cases { + let expected = rustix::fs::Mode::from_bits(raw_expected).expect("expected mode is a mode"); + let actual = super::let_readers_execute(st_mode); assert_eq!( - actual, - expected, - "{old_mode:06o} should become {}, became {}", - pretty(expected), - pretty(actual) + actual, expected, + "{st_mode:06o} should become {expected:04o}, became {actual:04o}" ); } } + + #[test] + #[should_panic] + fn let_readers_execute_panics_on_directory() { + super::let_readers_execute(0o040644); + } + + #[test] + #[should_panic] + fn let_readers_execute_should_panic_on_symlink() { + super::let_readers_execute(0o120644); + } }