Pre-RFC: std::fs::Permissions - writeable()

The standard library has a function readonly() in the std::fs::Permissions crate. But this function doesn't indicate if we have the right to write inside a file.

See the following code:

use std::fs::File;

fn main() -> std::io::Result<()> {
    let f = File::open("/bin/ls")?;
    let readonly = f.metadata()?.permissions().readonly();

    println!("Is `/bin/ls` readonly ?");
    
    match readonly {
        true  => println!("Yes"),
        false => println!("No"),
    }
    Ok(())
}
❯ cargo run -q
Is `/bin/ls` readonly ?
No

❯ ls -l /bin/ls
-rwxr-xr-x 1 root root 138216 févr.  8  2024 /bin/ls

root has the right to write (rwx), so the file is not considered as readonly.

Proposition

I suggest adding a function writeable() inside the std::fs::Permissions crate.

This function would return a bool :

  • true if we have the right to write.
  • false if we don't have the right to write.

Reasoning and drawback

Drawback

The drawback of adding this function is that users might try to check if they have the permission of the file before writing to it.
Like in this forum discussion : How to test std::fs::File has write permission - #4 by bigdogs - help - The Rust Programming Language Forum.

A better pattern is to write to the file, and handle any error to avoid TOCTOU race conditions.

Advantage

Rust already forces the user to handle the error. Which mostly mitigates the TOCTOU race conditions.

On the flip side, there are legitimate reasons to check if we have the right to write inside a file.

In my case, I am writing an hexadecimal editor. It would be nice to inform the user that he doesn't have the possibility to write inside the file.

Vim does it when we try to edit a file without the correct permissions. This is a common problem, and it would be nice to have a cross-platform way to check if we have the rights for this operation.

Workaround

It's always possible of writing inside a file, and restore it. But such an operation would change the date of the last modification despite no changes being performed.

The way forward would be to create an ACP (API Change Proposal) for the added function. I expect you'll need to argue whether the functionality is retrievable on Windows as well as on Unixes. Furthermore, I think this would mean that creating a Permissions value would require checking the current user, access control lists, and group membership (or at least .writable() would), which seems surprisingly involved.

While it's not an ahead of time indicator, you can catch an IO error of kind PermissionDenied when trying to edit a file to inform the user that it's not writable due to permissions.

Or you could use PermissionsExt to get and check the permission bits directly.

The only way to do this check accurately is to try to open the file for writing. Even if you did manually reimplement the exact same rules the OS uses for permission checks (which may or may not be possible — in some cases you can have the ability to write to a file but not the ability to know what its ACL is) the information you got would be subject to an unavoidable TOCTOU race.

(For extra complication, you might have the ability to replace the file (with rename) even if you cannot open it for writing; those two capabilities are semi-independent. This is one of the reasons why sometimes :w in vi will tell you the file is read-only but :w! will succeed.)

For @0xfalafel's purposes, the Right Thing is to attempt to open the file for update (O_RDWR | O_CREAT on Unix, I don't know the Windows equivalent off the top of my head) when the user first accesses it, catch a failure due to lack of write access, and retry in read-only mode. If the second open succeeds, show the read-only indicator. If the first succeeds, keep it open and use that fd / handle when saving edits.

The Rust stdlib shouldn't add mechanisms that encourage people to get this wrong.

8 Likes

Just to discourage "oh, I can do that", other things that can interfere:

  • the file may be under a readonly mount
  • security hooks (e.g., SELinux)
  • complicated FUSE logic
  • temporal (file locks)

And that's just LInux. Windows ACLs are…quite an interesting beast that I don't think anyone can truly emulate once one starts getting Really Fancy™ in the permission editing dialogs.

8 Likes

The permission read-only methods are mistakes in my opinion. They exist to expose the Windows read-only flag and in that process, make a mess for Unix (because Unix has no concept of read-only flag). Writeable doesn't correspond to a flag on any system, and therefore doesn't make sense to add. As others have pointed out, it will always be incorrect in some way, and I don't think we need to add even more ways to handle the file system incorrectly.

1 Like

I think you have that backwards. It was intended to expose the Unix mode and thought the Windows read-only attribute was close enough to their Unix read-only behaviour despite not being related to permissions.

We can do something like AccessCheck.or, on recent verions of Windows, just use the EffectiveAccess field of FILE_STAT_INFORMATION. The trouble is that on Windows the Permissions struct is based on file attributes and not permissions.

We could of course add an extra field to it but I personally wouldn't be keen on that because AccessCheck is relatively slow and is usually the wrong thing to do. It's almost always better to do something like:

You can test effective permissions with eaccess (considers the processes user and group and also file-system ro mount). Same probably also exists for Windows, but opening the file and handling the error is the correct way.

The Permissions API is also lacking and the only function also doesn't belong there, as the read only flag is not a permission. And is inconsistent between Unix and Windows.

1 Like

Access and eaccess can lead to TOCTOU issues though, so for most purposes they are a bad idea. As far as I know the editor use case is really the only valid one.

However, from reading the man page for eaccess it isn't clear to me if it also checks things like selinux/apparmor, seccomp, POSIX ACL, NFS ACL, Samba ACL or just the modes.

It is almost certainly wrong in some cases (unless it internally opens for writing like the suggestion above). And it leads to TOCTOU.

1 Like

Sounds kind of like the readonly method should just be deprecated as being the wrong abstraction.

6 Likes

Yea it's not really worth checking permissions, and I don't know any application which does it this way, except that some apps may check if their working dir is "writable", but that's it.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.