Skip to content

Commit

Permalink
fix: when obtaining locks, use mode 666 on Linux by default.
Browse files Browse the repository at this point in the history
Doing so helps with using `sudo` and others.
  • Loading branch information
Byron committed Feb 6, 2024
1 parent 45ef4ff commit d83e45d
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 2 deletions.
28 changes: 26 additions & 2 deletions gix-lock/src/acquire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -71,7 +73,11 @@ impl File {
boundary_directory: Option<PathBuf>,
) -> Result<File, Error> {
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,
Expand All @@ -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.
Expand All @@ -98,7 +106,11 @@ impl Marker {
boundary_directory: Option<PathBuf>,
) -> Result<Marker, Error> {
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,
Expand Down Expand Up @@ -169,6 +181,18 @@ fn add_lock_suffix(resource_path: &Path) -> PathBuf {
))
}

fn default_permissions() -> Option<std::fs::Permissions> {
#[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::*;
Expand Down
10 changes: 10 additions & 0 deletions gix-lock/tests/lock/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
10 changes: 10 additions & 0 deletions gix-lock/tests/lock/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down

0 comments on commit d83e45d

Please sign in to comment.