From 4a05e47d3b912ef90054efb64b56132640dffb96 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 5 Feb 2024 19:17:32 +0100 Subject: [PATCH] feat: Add `Builder::permissions()` method. (#273) * feat: Add `Builder::permissions()` method. With it it's possible to change the default mode of tempfiles on unix systems. The example also doesn't hide the fact that it is currently only useful on Unix, but as the ecosystem matures, thanks to the usage of `std::fs::Permissions` it should be able grow into more platforms over time. * Use `permissions` field of `Builder` on Unix to affect file mode The implementation favors the least invasive solution even though its forced to pull platform dependent code up. The alternative would have been to alter the method signatures of `create_named()` on all platforms. * Respect module boundaries and make all platforms permissions aware. This way, permissions can be passed and handled by platform specific code, which avoid having to use platform dependent code in the top-level. * Fail on Windows if somebody tries to set permissions there. That way, there will be no surprises as would be the case with doing nothing. * Add support for setting permissions on directories as well. --- src/dir/imp/any.rs | 19 +++++++ src/dir/imp/mod.rs | 9 ++++ src/dir/imp/unix.rs | 21 ++++++++ src/{dir.rs => dir/mod.rs} | 15 +++--- src/file/imp/other.rs | 6 ++- src/file/imp/unix.rs | 17 +++++-- src/file/imp/windows.rs | 16 +++++- src/file/mod.rs | 3 +- src/lib.rs | 102 +++++++++++++++++++++++++++++++++++-- src/util.rs | 5 +- 10 files changed, 192 insertions(+), 21 deletions(-) create mode 100644 src/dir/imp/any.rs create mode 100644 src/dir/imp/mod.rs create mode 100644 src/dir/imp/unix.rs rename src/{dir.rs => dir/mod.rs} (98%) diff --git a/src/dir/imp/any.rs b/src/dir/imp/any.rs new file mode 100644 index 000000000..015f9f87d --- /dev/null +++ b/src/dir/imp/any.rs @@ -0,0 +1,19 @@ +use crate::error::IoResultExt; +use crate::TempDir; +use std::path::PathBuf; +use std::{fs, io}; + +fn not_supported(msg: &str) -> io::Result { + Err(io::Error::new(io::ErrorKind::Other, msg)) +} + +pub fn create(path: PathBuf, permissions: Option<&std::fs::Permissions>) -> io::Result { + if permissions.map_or(false, |p| p.readonly()) { + return not_supported("changing permissions is not supported on this platform"); + } + fs::create_dir(&path) + .with_err_path(|| &path) + .map(|_| TempDir { + path: path.into_boxed_path(), + }) +} diff --git a/src/dir/imp/mod.rs b/src/dir/imp/mod.rs new file mode 100644 index 000000000..26d0a2273 --- /dev/null +++ b/src/dir/imp/mod.rs @@ -0,0 +1,9 @@ +#[cfg(unix)] +mod unix; +#[cfg(unix)] +pub use unix::*; + +#[cfg(not(unix))] +mod any; +#[cfg(not(unix))] +pub use any::*; diff --git a/src/dir/imp/unix.rs b/src/dir/imp/unix.rs new file mode 100644 index 000000000..47dd125c1 --- /dev/null +++ b/src/dir/imp/unix.rs @@ -0,0 +1,21 @@ +use crate::error::IoResultExt; +use crate::TempDir; +use std::io; +use std::path::PathBuf; + +pub fn create(path: PathBuf, permissions: Option<&std::fs::Permissions>) -> io::Result { + let mut dir_options = std::fs::DirBuilder::new(); + #[cfg(not(target_os = "wasi"))] + { + use std::os::unix::fs::{DirBuilderExt, PermissionsExt}; + if let Some(p) = permissions { + dir_options.mode(p.mode()); + } + } + dir_options + .create(&path) + .with_err_path(|| &path) + .map(|_| TempDir { + path: path.into_boxed_path(), + }) +} diff --git a/src/dir.rs b/src/dir/mod.rs similarity index 98% rename from src/dir.rs rename to src/dir/mod.rs index 1b79be445..db70dd52e 100644 --- a/src/dir.rs +++ b/src/dir/mod.rs @@ -12,7 +12,7 @@ use std::ffi::OsStr; use std::fs::remove_dir_all; use std::mem; use std::path::{self, Path, PathBuf}; -use std::{fmt, fs, io}; +use std::{fmt, io}; use crate::error::IoResultExt; use crate::Builder; @@ -468,10 +468,11 @@ impl Drop for TempDir { } } -pub(crate) fn create(path: PathBuf) -> io::Result { - fs::create_dir(&path) - .with_err_path(|| &path) - .map(|_| TempDir { - path: path.into_boxed_path(), - }) +pub(crate) fn create( + path: PathBuf, + permissions: Option<&std::fs::Permissions>, +) -> io::Result { + imp::create(path, permissions) } + +mod imp; diff --git a/src/file/imp/other.rs b/src/file/imp/other.rs index 8721d2da6..bba36712a 100644 --- a/src/file/imp/other.rs +++ b/src/file/imp/other.rs @@ -9,7 +9,11 @@ fn not_supported() -> io::Result { )) } -pub fn create_named(_path: &Path, _open_options: &mut OpenOptions) -> io::Result { +pub fn create_named( + _path: &Path, + _open_options: &mut OpenOptions, + _permissions: Option<&std::fs::Permissions>, +) -> io::Result { not_supported() } diff --git a/src/file/imp/unix.rs b/src/file/imp/unix.rs index 1f179276f..5f2cb5dcb 100644 --- a/src/file/imp/unix.rs +++ b/src/file/imp/unix.rs @@ -4,7 +4,7 @@ use std::fs::{self, File, OpenOptions}; use std::io; cfg_if::cfg_if! { if #[cfg(not(target_os = "wasi"))] { - use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; + use std::os::unix::fs::MetadataExt; } else { #[cfg(feature = "nightly")] use std::os::wasi::fs::MetadataExt; @@ -19,12 +19,17 @@ use { std::fs::hard_link, }; -pub fn create_named(path: &Path, open_options: &mut OpenOptions) -> io::Result { +pub fn create_named( + path: &Path, + open_options: &mut OpenOptions, + #[cfg_attr(target_os = "wasi", allow(unused))] permissions: Option<&std::fs::Permissions>, +) -> io::Result { open_options.read(true).write(true).create_new(true); #[cfg(not(target_os = "wasi"))] { - open_options.mode(0o600); + use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + open_options.mode(permissions.map(|p| p.mode()).unwrap_or(0o600)); } open_options.open(path) @@ -40,7 +45,7 @@ fn create_unlinked(path: &Path) -> io::Result { path = &tmp; } - let f = create_named(path, &mut OpenOptions::new())?; + let f = create_named(path, &mut OpenOptions::new(), None)?; // don't care whether the path has already been unlinked, // but perhaps there are some IO error conditions we should send up? let _ = fs::remove_file(path); @@ -50,6 +55,7 @@ fn create_unlinked(path: &Path) -> io::Result { #[cfg(target_os = "linux")] pub fn create(dir: &Path) -> io::Result { use rustix::{fs::OFlags, io::Errno}; + use std::os::unix::fs::OpenOptionsExt; OpenOptions::new() .read(true) .write(true) @@ -77,7 +83,8 @@ fn create_unix(dir: &Path) -> io::Result { OsStr::new(".tmp"), OsStr::new(""), crate::NUM_RAND_CHARS, - |path| create_unlinked(&path), + None, + |path, _| create_unlinked(&path), ) } diff --git a/src/file/imp/windows.rs b/src/file/imp/windows.rs index 9df65f9e8..bed62090f 100644 --- a/src/file/imp/windows.rs +++ b/src/file/imp/windows.rs @@ -19,7 +19,18 @@ fn to_utf16(s: &Path) -> Vec { s.as_os_str().encode_wide().chain(iter::once(0)).collect() } -pub fn create_named(path: &Path, open_options: &mut OpenOptions) -> io::Result { +fn not_supported(msg: &str) -> io::Result { + Err(io::Error::new(io::ErrorKind::Other, msg)) +} + +pub fn create_named( + path: &Path, + open_options: &mut OpenOptions, + permissions: Option<&std::fs::Permissions>, +) -> io::Result { + if permissions.map_or(false, |p| p.readonly()) { + return not_supported("changing permissions is not supported on this platform"); + } open_options .create_new(true) .read(true) @@ -34,7 +45,8 @@ pub fn create(dir: &Path) -> io::Result { OsStr::new(".tmp"), OsStr::new(""), crate::NUM_RAND_CHARS, - |path| { + None, + |path, _permissions| { OpenOptions::new() .create_new(true) .read(true) diff --git a/src/file/mod.rs b/src/file/mod.rs index 8a28343e0..f6950f383 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -1107,13 +1107,14 @@ impl AsRawHandle for NamedTempFile { pub(crate) fn create_named( mut path: PathBuf, open_options: &mut OpenOptions, + permissions: Option<&std::fs::Permissions>, ) -> io::Result { // Make the path absolute. Otherwise, changing directories could cause us to // delete the wrong file. if !path.is_absolute() { path = env::current_dir()?.join(path) } - imp::create_named(&path, open_options) + imp::create_named(&path, open_options, permissions) .with_err_path(|| path.clone()) .map(|file| NamedTempFile { path: TempPath { diff --git a/src/lib.rs b/src/lib.rs index 4b6371d49..2072f7a12 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -197,6 +197,7 @@ pub struct Builder<'a, 'b> { prefix: &'a OsStr, suffix: &'b OsStr, append: bool, + permissions: Option, } impl<'a, 'b> Default for Builder<'a, 'b> { @@ -206,6 +207,7 @@ impl<'a, 'b> Default for Builder<'a, 'b> { prefix: OsStr::new(".tmp"), suffix: OsStr::new(""), append: false, + permissions: None, } } } @@ -399,6 +401,89 @@ impl<'a, 'b> Builder<'a, 'b> { self } + /// The permissions to create the tempfile or [tempdir](Self::tempdir) with. + /// This allows to them differ from the default mode of `0o600` on Unix. + /// + /// # Security + /// + /// By default, the permissions of tempfiles on unix are set for it to be + /// readable and writable by the owner only, yielding the greatest amount + /// of security. + /// As this method allows to widen the permissions, security would be + /// reduced in such cases. + /// + /// # Platform Notes + /// ## Unix + /// + /// The actual permission bits set on the tempfile or tempdir will be affected by the + /// `umask` applied by the underlying syscall. + /// + /// + /// ## Windows and others + /// + /// This setting is unsupported and trying to set a file or directory read-only + /// will cause an error to be returned.. + /// + /// # Examples + /// + /// Create a named temporary file that is world-readable. + /// + /// ``` + /// # use std::io; + /// # fn main() { + /// # if let Err(_) = run() { + /// # ::std::process::exit(1); + /// # } + /// # } + /// # fn run() -> Result<(), io::Error> { + /// # use tempfile::Builder; + /// #[cfg(unix)] + /// { + /// use std::os::unix::fs::PermissionsExt; + /// let all_read_write = std::fs::Permissions::from_mode(0o666); + /// let tempfile = Builder::new().permissions(all_read_write).tempfile()?; + /// let actual_permissions = tempfile.path().metadata()?.permissions(); + /// assert_ne!( + /// actual_permissions.mode() & !0o170000, + /// 0o600, + /// "we get broader permissions than the default despite umask" + /// ); + /// } + /// # Ok(()) + /// # } + /// ``` + /// + /// Create a named temporary directory that is restricted to the owner. + /// + /// ``` + /// # use std::io; + /// # fn main() { + /// # if let Err(_) = run() { + /// # ::std::process::exit(1); + /// # } + /// # } + /// # fn run() -> Result<(), io::Error> { + /// # use tempfile::Builder; + /// #[cfg(unix)] + /// { + /// use std::os::unix::fs::PermissionsExt; + /// let owner_rwx = std::fs::Permissions::from_mode(0o700); + /// let tempdir = Builder::new().permissions(owner_rwx).tempdir()?; + /// let actual_permissions = tempdir.path().metadata()?.permissions(); + /// assert_eq!( + /// actual_permissions.mode() & !0o170000, + /// 0o700, + /// "we get the narrow permissions we asked for" + /// ); + /// } + /// # Ok(()) + /// # } + /// ``` + pub fn permissions(&mut self, permissions: std::fs::Permissions) -> &mut Self { + self.permissions = Some(permissions); + self + } + /// Create the named temporary file. /// /// # Security @@ -473,7 +558,10 @@ impl<'a, 'b> Builder<'a, 'b> { self.prefix, self.suffix, self.random_len, - |path| file::create_named(path, OpenOptions::new().append(self.append)), + self.permissions.as_ref(), + |path, permissions| { + file::create_named(path, OpenOptions::new().append(self.append), permissions) + }, ) } @@ -545,7 +633,14 @@ impl<'a, 'b> Builder<'a, 'b> { dir = &storage; } - util::create_helper(dir, self.prefix, self.suffix, self.random_len, dir::create) + util::create_helper( + dir, + self.prefix, + self.suffix, + self.random_len, + self.permissions.as_ref(), + dir::create, + ) } /// Attempts to create a temporary file (or file-like object) using the @@ -690,7 +785,8 @@ impl<'a, 'b> Builder<'a, 'b> { self.prefix, self.suffix, self.random_len, - move |path| { + None, + move |path, _permissions| { Ok(NamedTempFile::from_parts( f(&path)?, TempPath::from_path(path), diff --git a/src/util.rs b/src/util.rs index d426ba3d7..8c04953a3 100644 --- a/src/util.rs +++ b/src/util.rs @@ -20,7 +20,8 @@ pub fn create_helper( prefix: &OsStr, suffix: &OsStr, random_len: usize, - mut f: impl FnMut(PathBuf) -> io::Result, + permissions: Option<&std::fs::Permissions>, + mut f: impl FnMut(PathBuf, Option<&std::fs::Permissions>) -> io::Result, ) -> io::Result { let num_retries = if random_len != 0 { crate::NUM_RETRIES @@ -30,7 +31,7 @@ pub fn create_helper( for _ in 0..num_retries { let path = base.join(tmpname(prefix, suffix, random_len)); - return match f(path) { + return match f(path, permissions) { Err(ref e) if e.kind() == io::ErrorKind::AlreadyExists && num_retries > 1 => continue, // AddrInUse can happen if we're creating a UNIX domain socket and // the path already exists.