From d83e45de625587b7b11e376a4d608854027a723e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 6 Feb 2024 17:01:34 +0100 Subject: [PATCH] fix: when obtaining locks, use mode 666 on Linux by default. Doing so helps with using `sudo` and others. --- gix-lock/src/acquire.rs | 28 ++++++++++++++++++++++++++-- gix-lock/tests/lock/file.rs | 10 ++++++++++ gix-lock/tests/lock/marker.rs | 10 ++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/gix-lock/src/acquire.rs b/gix-lock/src/acquire.rs index 2242a72e106..dd6cc83eadb 100644 --- a/gix-lock/src/acquire.rs +++ b/gix-lock/src/acquire.rs @@ -60,6 +60,8 @@ impl File { /// If `boundary_directory` is given, non-existing directories will be created automatically and removed in the case of /// a rollback. Otherwise the containing directory is expected to exist, even though the resource doesn't have to. /// + /// Note that permissions will be set to `0o666`, which usually results in `0o644` after passing a default umask, on Unix systems. + /// /// ### Warning of potential resource leak /// /// Please note that the underlying file will remain if destructors don't run, as is the case when interrupting the application. @@ -71,7 +73,11 @@ impl File { boundary_directory: Option, ) -> Result { let (lock_path, handle) = lock_with_mode(at_path.as_ref(), mode, boundary_directory, &|p, d, c| { - gix_tempfile::writable_at(p, d, c) + if let Some(permissions) = default_permissions() { + gix_tempfile::writable_at_with_permissions(p, d, c, permissions) + } else { + gix_tempfile::writable_at(p, d, c) + } })?; Ok(File { inner: handle, @@ -87,6 +93,8 @@ impl Marker { /// If `boundary_directory` is given, non-existing directories will be created automatically and removed in the case of /// a rollback. /// + /// Note that permissions will be set to `0o666`, which usually results in `0o644` after passing a default umask, on Unix systems. + /// /// ### Warning of potential resource leak /// /// Please note that the underlying file will remain if destructors don't run, as is the case when interrupting the application. @@ -98,7 +106,11 @@ impl Marker { boundary_directory: Option, ) -> Result { let (lock_path, handle) = lock_with_mode(at_path.as_ref(), mode, boundary_directory, &|p, d, c| { - gix_tempfile::mark_at(p, d, c) + if let Some(permissions) = default_permissions() { + gix_tempfile::mark_at_with_permissions(p, d, c, permissions) + } else { + gix_tempfile::mark_at(p, d, c) + } })?; Ok(Marker { created_from_file: false, @@ -169,6 +181,18 @@ fn add_lock_suffix(resource_path: &Path) -> PathBuf { )) } +fn default_permissions() -> Option { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + Some(std::fs::Permissions::from_mode(0o666)) + } + #[cfg(not(unix))] + { + None + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/gix-lock/tests/lock/file.rs b/gix-lock/tests/lock/file.rs index 0964e713365..369455b9055 100644 --- a/gix-lock/tests/lock/file.rs +++ b/gix-lock/tests/lock/file.rs @@ -105,6 +105,16 @@ mod acquire { assert_eq!(file.lock_path(), resource_lock); assert_eq!(file.resource_path(), resource); assert!(resource_lock.is_file()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let perms = resource_lock.metadata()?.permissions(); + assert_ne!( + perms.mode() & !0o170000, + 0o600, + "mode is more permissive now, even after passing the umask" + ); + } file.with_mut(|out| out.write_all(b"hello world"))?; assert_eq!(file.commit()?.0, resource, "returned and computed resource path match"); assert_eq!( diff --git a/gix-lock/tests/lock/marker.rs b/gix-lock/tests/lock/marker.rs index 6f27ed5caa7..02ebe0f84fa 100644 --- a/gix-lock/tests/lock/marker.rs +++ b/gix-lock/tests/lock/marker.rs @@ -71,6 +71,16 @@ mod commit { let dir = tempfile::tempdir()?; let resource = dir.path().join("the-resource"); let mark = gix_lock::Marker::acquire_to_hold_resource(resource, Fail::Immediately, None)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let perms = mark.lock_path().metadata()?.permissions(); + assert_ne!( + perms.mode() & !0o170000, + 0o600, + "mode is more permissive now, even after passing the umask" + ); + } let err = mark.commit().expect_err("should always fail"); assert_eq!(err.error.kind(), std::io::ErrorKind::Other); assert_eq!(