Skip to content

Commit

Permalink
feat: Add Builder::permissions() method. (#273)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
Byron committed Feb 5, 2024
1 parent 184ab8f commit 4a05e47
Show file tree
Hide file tree
Showing 10 changed files with 192 additions and 21 deletions.
19 changes: 19 additions & 0 deletions src/dir/imp/any.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use crate::error::IoResultExt;
use crate::TempDir;
use std::path::PathBuf;
use std::{fs, io};

fn not_supported<T>(msg: &str) -> io::Result<T> {
Err(io::Error::new(io::ErrorKind::Other, msg))
}

pub fn create(path: PathBuf, permissions: Option<&std::fs::Permissions>) -> io::Result<TempDir> {
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(),
})
}
9 changes: 9 additions & 0 deletions src/dir/imp/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#[cfg(unix)]
mod unix;
#[cfg(unix)]
pub use unix::*;

#[cfg(not(unix))]
mod any;
#[cfg(not(unix))]
pub use any::*;
21 changes: 21 additions & 0 deletions src/dir/imp/unix.rs
Original file line number Diff line number Diff line change
@@ -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<TempDir> {
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(),
})
}
15 changes: 8 additions & 7 deletions src/dir.rs → src/dir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -468,10 +468,11 @@ impl Drop for TempDir {
}
}

pub(crate) fn create(path: PathBuf) -> io::Result<TempDir> {
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<TempDir> {
imp::create(path, permissions)
}

mod imp;
6 changes: 5 additions & 1 deletion src/file/imp/other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ fn not_supported<T>() -> io::Result<T> {
))
}

pub fn create_named(_path: &Path, _open_options: &mut OpenOptions) -> io::Result<File> {
pub fn create_named(
_path: &Path,
_open_options: &mut OpenOptions,
_permissions: Option<&std::fs::Permissions>,
) -> io::Result<File> {
not_supported()
}

Expand Down
17 changes: 12 additions & 5 deletions src/file/imp/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -19,12 +19,17 @@ use {
std::fs::hard_link,
};

pub fn create_named(path: &Path, open_options: &mut OpenOptions) -> io::Result<File> {
pub fn create_named(
path: &Path,
open_options: &mut OpenOptions,
#[cfg_attr(target_os = "wasi", allow(unused))] permissions: Option<&std::fs::Permissions>,
) -> io::Result<File> {
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)
Expand All @@ -40,7 +45,7 @@ fn create_unlinked(path: &Path) -> io::Result<File> {
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);
Expand All @@ -50,6 +55,7 @@ fn create_unlinked(path: &Path) -> io::Result<File> {
#[cfg(target_os = "linux")]
pub fn create(dir: &Path) -> io::Result<File> {
use rustix::{fs::OFlags, io::Errno};
use std::os::unix::fs::OpenOptionsExt;
OpenOptions::new()
.read(true)
.write(true)
Expand Down Expand Up @@ -77,7 +83,8 @@ fn create_unix(dir: &Path) -> io::Result<File> {
OsStr::new(".tmp"),
OsStr::new(""),
crate::NUM_RAND_CHARS,
|path| create_unlinked(&path),
None,
|path, _| create_unlinked(&path),
)
}

Expand Down
16 changes: 14 additions & 2 deletions src/file/imp/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,18 @@ fn to_utf16(s: &Path) -> Vec<u16> {
s.as_os_str().encode_wide().chain(iter::once(0)).collect()
}

pub fn create_named(path: &Path, open_options: &mut OpenOptions) -> io::Result<File> {
fn not_supported<T>(msg: &str) -> io::Result<T> {
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<File> {
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)
Expand All @@ -34,7 +45,8 @@ pub fn create(dir: &Path) -> io::Result<File> {
OsStr::new(".tmp"),
OsStr::new(""),
crate::NUM_RAND_CHARS,
|path| {
None,
|path, _permissions| {
OpenOptions::new()
.create_new(true)
.read(true)
Expand Down
3 changes: 2 additions & 1 deletion src/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1107,13 +1107,14 @@ impl<F: AsRawHandle> AsRawHandle for NamedTempFile<F> {
pub(crate) fn create_named(
mut path: PathBuf,
open_options: &mut OpenOptions,
permissions: Option<&std::fs::Permissions>,
) -> io::Result<NamedTempFile> {
// 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 {
Expand Down
102 changes: 99 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ pub struct Builder<'a, 'b> {
prefix: &'a OsStr,
suffix: &'b OsStr,
append: bool,
permissions: Option<std::fs::Permissions>,
}

impl<'a, 'b> Default for Builder<'a, 'b> {
Expand All @@ -206,6 +207,7 @@ impl<'a, 'b> Default for Builder<'a, 'b> {
prefix: OsStr::new(".tmp"),
suffix: OsStr::new(""),
append: false,
permissions: None,
}
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
},
)
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down
5 changes: 3 additions & 2 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ pub fn create_helper<R>(
prefix: &OsStr,
suffix: &OsStr,
random_len: usize,
mut f: impl FnMut(PathBuf) -> io::Result<R>,
permissions: Option<&std::fs::Permissions>,
mut f: impl FnMut(PathBuf, Option<&std::fs::Permissions>) -> io::Result<R>,
) -> io::Result<R> {
let num_retries = if random_len != 0 {
crate::NUM_RETRIES
Expand All @@ -30,7 +31,7 @@ pub fn create_helper<R>(

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.
Expand Down

0 comments on commit 4a05e47

Please sign in to comment.