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

Writing changes file ownership #11988

Open
kirawi opened this issue Nov 2, 2024 · 3 comments
Open

Writing changes file ownership #11988

kirawi opened this issue Nov 2, 2024 · 3 comments
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@kirawi
Copy link
Member

kirawi commented Nov 2, 2024

Summary

Writing a file may change the file's ownership when writing.

Reproduction Steps

$ touch abc
$ ls -la abc
-rw-r--r-- 1 m00n m00n 0 Oct 28 23:03 abc
$ sudo helix abc
( Write the file with any content )
$ ls -la
-rw-r--r-- 1 root m00n 4 Oct 28 23:03 abc

Helix log

No response

Platform

All

Terminal Emulator

N/a

Installation Method

release

Helix Version

helix 24.7

@kirawi kirawi added C-bug Category: This is a bug A-helix-term Area: Helix term improvements labels Nov 2, 2024
@poliorcetics
Copy link
Contributor

I think the issue is

let mut dst = tokio::fs::File::create(&write_path).await?;
, where the file is created with the access rights, owner and group of the running helix process, not that of the target file

@kirawi
Copy link
Member Author

kirawi commented Nov 10, 2024

I don't think so. I believe it would open with the same file permissions if it already exists, and for backup files we already copy metadata on

// copy metadata and delete backup
let _ = tokio::task::spawn_blocking(move || {
let _ = copy_metadata(&backup, &write_path)
.map_err(|e| log::error!("Failed to copy metadata on write: {e}"));
let _ = std::fs::remove_file(backup)
.map_err(|e| log::error!("Failed to remove backup file on write: {e}"));
})

@kirawi
Copy link
Member Author

kirawi commented Nov 10, 2024

Actually you might have been right. This seems to be fixed by #11374

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

2 participants