From f1909ef6d1122ed9aff15d0fbab9297f6ce63d24 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 7 Feb 2025 15:13:06 -0500 Subject: [PATCH 1/2] Set 777 permissions on locked files --- crates/uv-fs/src/lib.rs | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/crates/uv-fs/src/lib.rs b/crates/uv-fs/src/lib.rs index 989ede8d36a88..2242e9f031c38 100644 --- a/crates/uv-fs/src/lib.rs +++ b/crates/uv-fs/src/lib.rs @@ -599,7 +599,7 @@ impl LockedFile { path: impl AsRef, resource: impl Display, ) -> Result { - let file = fs_err::File::create(path.as_ref())?; + let file = Self::create(path)?; let resource = resource.to_string(); Self::lock_file_blocking(file, &resource) } @@ -610,10 +610,31 @@ impl LockedFile { path: impl AsRef, resource: impl Display, ) -> Result { - let file = fs_err::File::create(path.as_ref())?; + let file = Self::create(path)?; let resource = resource.to_string(); tokio::task::spawn_blocking(move || Self::lock_file_blocking(file, &resource)).await? } + + #[cfg(unix)] + fn create(path: impl AsRef) -> std::io::Result { + use fs_err::os::unix::fs::OpenOptionsExt; + + fs_err::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .mode(0o777) + .open(path.as_ref()) + } + + #[cfg(not(unix))] + fn create(path: impl AsRef) -> std::io::Result { + fs_err::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .open(path.as_ref()) + } } impl Drop for LockedFile { From d5f19cec6d9c3cf113685a78dd5eb4eeac3d57c9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 7 Feb 2025 15:49:05 -0500 Subject: [PATCH 2/2] Try umask --- crates/uv-fs/src/lib.rs | 44 +++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/crates/uv-fs/src/lib.rs b/crates/uv-fs/src/lib.rs index 2242e9f031c38..aecac9eaea580 100644 --- a/crates/uv-fs/src/lib.rs +++ b/crates/uv-fs/src/lib.rs @@ -1,6 +1,7 @@ -use fs2::FileExt; use std::fmt::Display; use std::path::{Path, PathBuf}; + +use fs2::FileExt; use tempfile::NamedTempFile; use tracing::{debug, error, info, trace, warn}; @@ -616,15 +617,46 @@ impl LockedFile { } #[cfg(unix)] - fn create(path: impl AsRef) -> std::io::Result { - use fs_err::os::unix::fs::OpenOptionsExt; + fn create(path: impl AsRef) -> Result { + use std::os::unix::fs::PermissionsExt; - fs_err::OpenOptions::new() + // If path already exists, return it. + if let Ok(file) = fs_err::OpenOptions::new() .read(true) .write(true) - .create(true) - .mode(0o777) .open(path.as_ref()) + { + return Ok(file); + } + + // Otherwise, create a temporary file with 777 permissions. We must set + // permissions _after_ creating the file, to override the `umask`. + let file = if let Some(parent) = path.as_ref().parent() { + NamedTempFile::new_in(parent)? + } else { + NamedTempFile::new()? + }; + if let Err(err) = file + .as_file() + .set_permissions(std::fs::Permissions::from_mode(0o777)) + { + warn!("Failed to set permissions on temporary file: {err}"); + } + + // Try to move the file to path, but if path exists now, just open path + match file.persist_noclobber(path.as_ref()) { + Ok(file) => Ok(fs_err::File::from_parts(file, path.as_ref())), + Err(err) => { + if err.error.kind() == std::io::ErrorKind::AlreadyExists { + fs_err::OpenOptions::new() + .read(true) + .write(true) + .open(path.as_ref()) + } else { + Err(err.error) + } + } + } } #[cfg(not(unix))]