Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New folders are created with 0755 permission #464

Open
dipterix opened this issue Aug 3, 2024 · 8 comments
Open

New folders are created with 0755 permission #464

dipterix opened this issue Aug 3, 2024 · 8 comments
Labels
bug an unexpected problem or unintended behavior

Comments

@dipterix
Copy link

dipterix commented Aug 3, 2024

I'm aware of the discussion here #293

Today I encountered this issue. I was running an R script on my server. I set my system default umask to be 077 (https://support.apple.com/en-us/101914), and Sys.umask() is "77", hoping that the code would respect that (I also have ACL set up on the project folder so normally only the code owner and user group permitted by ACL can run commands. However, packages depending on fs generated bunch of 0755 files (because my account has the admin (not root) privilege).

This incident raised my concerns. You see the people who wrote the code might not be the same group of people who actually run the code, especially for R package authors. Most people who use fs because it is a powerful substitute (indeed it is) to the base functions. It has almost no learning curve. People who are already familiar with base functions can easily transfer to fs with no significant differences

However, such "naive" assumption might cause problems. The code authors might not know how chmod, umask, ACL work (they don't need to). People who know the permissions might also give upthinking and rely on fs being consistent (since fs is too similar to base functions and too easy to use).

My suggestion is maybe fs always respect Sys.umask, and can be turned off via options. For most users this won't change their experience as most system default umask is 022.

Alternatively users can instruct whether to respect Sys.umask by wrapping the mode

fs::dir_create("dir", mode = "0777", umask = Sys.umask())

# or

fs::dir_create("dir", mode = fs::safe_mode("0777"))
safe_mode <- function(mode, umask = Sys.umask()) {
  ...
}

Then all the code that tries to change to mode to 0777 is constrained by user-controlled umask

@gaborcsardi
Copy link
Member

My suggestion is maybe fs always respect Sys.umask

It already does:

❯ Sys.umask()
[1] "22"fs::file_create("foo")
❯ fs::file_info("foo")
# A tibble: 1 × 18
  path       type     size permissions modification_time   user  group device_id
  <fs::path> <fct> <fs::b> <fs::perms> <dttm>              <chr> <chr>     <dbl>
1 foo        file        0 rw-r--r--   2024-08-04 08:54:33 gabowheel  16777234
# ℹ 10 more variables: hard_links <dbl>, special_device_id <dbl>, inode <dbl>,
#   block_size <dbl>, blocks <dbl>, flags <int>, generation <dbl>,
#   access_time <dttm>, change_time <dttm>, birth_time <dttm>
❯ Sys.umask("77")
❯ Sys.umask()
[1] "77"fs::file_create("bar")
❯ fs::file_info("bar")
# A tibble: 1 × 18
  path       type     size permissions modification_time   user  group device_id
  <fs::path> <fct> <fs::b> <fs::perms> <dttm>              <chr> <chr>     <dbl>
1 bar        file        0 rw-------   2024-08-04 08:55:05 gabowheel  16777234
# ℹ 10 more variables: hard_links <dbl>, special_device_id <dbl>, inode <dbl>,
#   block_size <dbl>, blocks <dbl>, flags <int>, generation <dbl>,
#   access_time <dttm>, change_time <dttm>, birth_time <dttm>

and can be turned off via options

and then use what umask to create files? There is always an umask, we need to set some mode when we create a file.

I guess I don't really understand your problem. Can you please

  1. show us a reproducible example of what you are seeing?
  2. tell us why you think it is not right?
  3. tell us what you think the correct behavior would be?
    Thanks!

@dipterix
Copy link
Author

dipterix commented Aug 4, 2024

I see. Indeed file_create respect that, but not dir_create.

Sys.umask("22")

# expect to umask to 0755
fs::dir_create("junk", mode = "0777")

# 0777 instead
fs::file_info("junk")
#> # A tibble: 1 × 18
#>   path       type     size permissions modification_time   user  group device_id
#>   <fs::path> <fct>   <fs:> <fs::perms> <dttm>              <chr> <chr>     <dbl>
#> 1 junk       direct…    64 rwxrwxrwx   2024-08-04 13:19:48 dipt… staff  16777232
#> # ℹ 10 more variables: hard_links <dbl>, special_device_id <dbl>, inode <dbl>,
#> #   block_size <dbl>, blocks <dbl>, flags <int>, generation <dbl>,
#> #   access_time <dttm>, change_time <dttm>, birth_time <dttm>

# clean up
fs::dir_delete("junk")

Created on 2024-08-04 with reprex v2.1.1

It should respect Sys.umask and set folder permissions to be rwxr-xr-x instead of rwxrwxrwx. The reason is because people who write fs::dir_create("junk", mode = "0777") might not be the same people who run the code.

> packageVersion("fs")
[1] ‘1.6.4’

@gaborcsardi
Copy link
Member

OK, so basically, this issue is about dir_create() not using the umask at all. I don't really understand why that is, I would think that we need to call mkdir(2) with the specified mode, and then the OS applies the umask automatically. I am not sure why dir_create() messing with the umask explicitly, but I am probably missing something.

In any case, this is potentially a bug.

@gaborcsardi gaborcsardi added the bug an unexpected problem or unintended behavior label Aug 4, 2024
@dipterix
Copy link
Author

dipterix commented Aug 4, 2024

If dir_create() respects umask at both R and OS level, then there is no need to mess with umask explicitly.

The reason why I had that proposal was because I thought the behavior (which you said could be a bug just now) had been purposely designed.

@Patrick330
Copy link

@dipterix I suspect your issue is occurring because the C function called by dir_create(), uv_fs_mkdir, creates all directories with a 777 mode and then applies chmod after. In your reprex above you create the directory with mode "0777" and get that mode as a result, without use of the umask. Could you try creating with a different mode to see if you continue to get a mode of "0777"?

I have a related issue, which I will write up shortly, once my system admins give me additional context on our ACL setup.

@dipterix
Copy link
Author

dipterix commented Aug 8, 2024

@Patrick330 Could you give me any instructions? I tried 0777, 0775, ... and even set umask to other values such as "37" in R. dir_create never respect those umasks. (Just in case this is what you mean by different modes)

Also I don't think it relates too much with ACL setup (may or may not be). I ran the reprex on my personal computer with default MacOS settings and it still produced the same result.

It's interesting to see uv_fs_mkdir creates directory with 0777 first and chmod later on here

fs/src/dir.cc

Line 25 in 714990b

int fd = uv_fs_mkdir(uv_default_loop(), &req, p, 0x777, NULL);

Should it calculate the proper mode before creating the folder?

@Patrick330
Copy link

When you create with a different mode, as in fs::dir_create("junk", mode = "0444"), do the folders created have the requested mode, or do they continue to have 0777? I was thinking perhaps the chmod call later was the issue?

@dipterix
Copy link
Author

dipterix commented Aug 8, 2024

Oh I see. It is 0444 if you explicitly specify the mode. I think the issue is different. The issue here is regardless of the mode code owner specifies, the created folder should respect system & program umask.

So if code owner creates a folder with 0777 permission, and users set umask to be 0022, then the newly created files should NEVER exceed the user-defined permission, that is the folder should be created with 0755.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants