tempfile: add new_with_modes and is_named methods

[champtar]

On filesystems that do not support O_TMPFILE (e.g. vfat), TempFile::new()
automatically falls back to a potentially world readable named temp file.

Add TempFile::new_with_modes() to create temp files with a chosen mode directly.
We pass 2 modes, one for unnamed tempfile (happy path, created with O_TMPFILE),
and one for named tempfile.

The final mode is still restricted by the process umask.

@cgwalters this will help improve cap-std-ext atomic* permissions handling

[champtar]

Still pretty new to rust, not sure if this is ok to use #[cfg(unix)] for the mode argument, but 'it works (TM)'

[cgwalters]

Suggested change

	let mode = match mode {
	Some(mode) => Mode::from(mode),
	None => Mode::from(0o666),
	};
	let mode = mode.unwrap_or(0o666).map(Mode::from);

would be slightly shorter.

Actually for internal APIs I'd say we should actually use rustic::fs::Mode as much as possible; in other words make this function take a Mode instead. (We can't for public APIs yet as we aren't depending on rustix there yet AFAIK)

[champtar]

Now using rustic::fs::Mode

[champtar]

This fails for Darwin

error[E0308]: mismatched types
   --> cap-tempfile/src/tempfile.rs:130:19
    |
130 |         opts.mode(mode.as_raw_mode());
    |              ---- ^^^^^^^^^^^^^^^^^^ expected `u32`, found `u16`
    |              |
    |              arguments to this method are incorrect
    |
note: method defined here
   --> /home/runner/work/cap-std/cap-std/cap-primitives/src/fs/open_options.rs:257:8
    |
257 |     fn mode(&mut self, mode: u32) -> &mut Self;
    |        ^^^^
help: you can convert a `u16` to a `u32`
    |
130 |         opts.mode(mode.as_raw_mode().into());
    |                                     +++++++

error[E0277]: the trait bound `Mode: From<u32>` is not satisfied
   --> cap-tempfile/src/tempfile.rs:159:56
    |
159 |         let (fd, name) = new_tempfile(dir, false, Some(rustix::fs::Mode::from(mode)))?;
    |                                                        ^^^^^^^^^^^^^^^^ the trait `From<u32>` is not implemented for `Mode`
    |
    = help: the trait `From<u32>` is not implemented for `Mode`
            but trait `From<u16>` is implemented for it
    = help: for that trait implementation, expected `u16`, found `u32`

[champtar]

Back to u32 for now

[cgwalters]

A bit unusual, how about 0o200 instead so we can at least write to the file?

[champtar]

Definitely unusual, but we can write to the fd as it was open with O_RDWR

[champtar]

I switched to passing 2 modes (for unnamed and named case) + adding is_named() method.
This will allow to keep the happy path (O_TMPFILE) simpler/faster in coreos/cap-std-ext#73

[champtar]

This one is ready for re-review

[cgwalters]

The first condition implies the second, which makes things feel backwards to me.

This all might be cleaner if we used a struct NewTempfileOpts instead - then there'd be conditionals in way fewer places (just the definition and uses, not needing to thread them through all the internal APIs)

[champtar]

The first condition implies the second, which makes things feel backwards to me.

I can reorder, i think I put unnamed_mode first because I use it first

This all might be cleaner if we used a struct NewTempfileOpts instead - then there'd be conditionals in way fewer places (just the definition and uses, not needing to thread them through all the internal APIs)

That would just change new_tempfile and new_tempfile_linux definition, and new_tempfile_linux doesn't need named_mode, so not much difference. I can do it in a separate commit if you want to see.

[cgwalters]

"anonymous" and "unnamed" mean the same thing, so how about anonymous_mode?

[champtar]

Then we would have named_mode and anonymous_mode
Looking at the man page (https://man7.org/linux/man-pages/man2/openat.2.html):

O_TMPFILE (since Linux 3.11)
              Create an unnamed temporary regular file.

I would replace anonymous by unnamed instead

[sunfishcode]

I apologize for the very late review here. I think we can fix the permissions of new_anonymous to not be readable or writable, so I've now submitted #403 to do that.

I'm hesitant to add a new_with_mode function to cap-tempfile itself as it doesn't correspond to anything in the regular tempfile crate. If tempfile doesn't have an interface for this, is it generally needed?

[champtar]

My goal is to do atomic writes, but have the temporary file not readable AND respect umask (optimize functions like https://docs.rs/cap-std-ext/latest/cap_std_ext/dirext/trait.CapStdExtDirExt.html#tymethod.atomic_replace_with)

When using O_TMPFILE we are already safe, so you can create the file directly with the wanted permissions and it'll apply the umask during creation without extra steps.
When you can't use O_TMPFILE, you set the permissions to 0o000, and then you need to somehow retrieve the umask (by reading /proc or creating a temp file just for that) to compute the permission.

The interest of new_with_modes + is_named is to avoid having to retrieve umask + call chmod in the O_TMPFILE case, that's it.

[sunfishcode]

@champtar It sounds like the same concerns would apply to the regular tempfile crate as well, right?

cap-tempfile here is aiming to be to be a cap- adaptation of the tempfile crate. If tempfile adds a new_with_mode option then it will make sense for cap-tempfile to do so.

[champtar]

@sunfishcode I completely forgot to answer you, and then today in something completely unrelated I had to fix a race condition, fixed by using atomic writes.

For devs to use atomic writes by default it needs to be easy to do and cheap.

cap-tempfile support replace with O_TMPFILE, tempfile doesn't and will not (Stebalien/tempfile#13), so new_with_mode will never be added to tempfile.

IMO being an adaptation means covering all the use cases of tempfile, but it doesn't prevent you from having extra features.