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

cargo add changes file permissions of Cargo.toml to 600 #13896

Closed
stevenengler opened this issue May 10, 2024 · 2 comments · Fixed by #13898
Closed

cargo add changes file permissions of Cargo.toml to 600 #13896

stevenengler opened this issue May 10, 2024 · 2 comments · Fixed by #13898
Labels
A-filesystem Area: issues with filesystems C-bug Category: bug Command-add Command-remove S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@stevenengler
Copy link
Contributor

stevenengler commented May 10, 2024

Problem

When you run cargo add, it changes the file permissions of Cargo.toml to 600 (user read+write only). This is a little bit painful when you're building the code as a different user than the user writing the code, for example if you're running the code in a container. This applies to cargo remove as well. I tested this behaviour on Cargo 1.78.0 and nightly.

Steps

$ mkdir cargo-add-test && cd cargo-add-test
$ cargo init
$ ls -l
-rw-rw-r-- 1 user user   85 May  9 23:09 Cargo.toml
drwxrwxr-x 2 user user 4096 May  9 23:09 src
$ cargo add libc
$ ls -l
-rw-rw-r-- 1 user user  376 May  9 23:09 Cargo.lock
-rw------- 1 user user  102 May  9 23:09 Cargo.toml
drwxrwxr-x 2 user user 4096 May  9 23:09 src
$ umask
0002

Possible Solution(s)

I'm guessing that Cargo replaces the existing file with a new file, and this is why the permissions change. In this case, I think it would be better to create the file with the original user/group/world permissions (ignoring things like suid permissions), ideally bypassing the umask as well.

Edit: It appears to use the tempfile crate, which uses 600 by default on Linux.

pub fn write_atomic<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> Result<()> {
let path = path.as_ref();
let mut tmp = TempFileBuilder::new()
.prefix(path.file_name().unwrap())
.tempfile_in(path.parent().unwrap())?;
tmp.write_all(contents.as_ref())?;
tmp.persist(path)?;
Ok(())
}

Notes

No response

Version

cargo 1.80.0-nightly (0ca60e940 2024-05-08)
release: 1.80.0-nightly
commit-hash: 0ca60e940821c311c9b25a6423b59ccdbcea218f
commit-date: 2024-05-08
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.3 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Ubuntu 20.4.0 (focal) [64-bit]
@stevenengler stevenengler added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels May 10, 2024
@weihanglo
Copy link
Member

Thanks for the report. You guessed it correctly. It is because Cargo switched to atomic write since #12744 in 1.75.0. Open to a fix.
(Cross-platform permission handling is not easy to me though 😅.)

@weihanglo weihanglo added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review A-filesystem Area: issues with filesystems Command-add Command-remove and removed S-triage Status: This issue is waiting on initial triage. labels May 10, 2024
@stevenengler
Copy link
Contributor Author

(Cross-platform permission handling is not easy to me though 😅.)

Yeah it's not for me either :) But tempfile doesn't support permissions on Windows, so I made #13898 which is unix-only.

@bors bors closed this as completed in 0ea330d May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-filesystem Area: issues with filesystems C-bug Category: bug Command-add Command-remove S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants