From f64bfe8c1d1b6e168725aa0a33dfbb43737c1bfd Mon Sep 17 00:00:00 2001 From: tims <0xtimsb@gmail.com> Date: Fri, 20 Dec 2024 03:46:01 +0530 Subject: [PATCH] linux: Fix saving file with root ownership (#22045) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #13585 Currently, saving files with `root` ownership or `root` as the group throws a `Permission denied (os error 13). Please try again.` error. This PR fixes the issue on Linux by prompting the user for a password and saving the file with elevated privileges. It uses `pkexec` (Polkit), which is by default available on GNOME, KDE, and most Linux systems. I haven't implemented this for macOS as I don't have a device to test it on. This implementation is similar to how Vscode handles it. Except, they don't show custom message. **Working**: When file saving fails due to a `PermissionDenied` error, we create a temporary file in the same directory as the target file and writes the data to this temporary file. After, the contents of this file are copied to the original file using the `tee` command instead of `cp` or `mv`. This ensures that the ownership and permissions of the original file are preserved. This command is executed using `pkexec` which will prompt user for their password. **Custom Message**: The message displayed to the user in the prompt is automatically retrieved from the `org.zed.app.policy` file, which is located at `/usr/share/polkit-1/actions/`. This file should be installed during the setup process. While the policy file is optional, omitting it will cause the user to see the underlying command being executed rather than a user-friendly message. Currently, VSCode does not display the user-friendly message. The policy file must specify a unique binary, ensuring that only that binary can use the policy file. It cannot be as generic as a `/bin/bash`, as any software using bash to prompt will end up showing Zed’s custom message. To address this, we will create a custom bash script, as simple as the following, placed in `/usr/bin/zed/elevate.sh`. The script should have root ownership and should not reside in the home directory, since the policy file cannot resolve `$HOME`. ```sh #!/bin/bash eval "$@" ``` *IMPORTANT NOTE* Since copying the policy file and our script requires sudo privileges, the installation script will now prompt for the password at very end. Only on Linux, if `pexec` is installed. Screenshots: KDE with policy file: ![Screenshot from 2024-12-15 22-13-06](https://github.com/user-attachments/assets/b8bb7565-85df-4c95-bb10-82e50acf9b56) Gnome with policy file: ![Screenshot from 2024-12-15 22-21-48](https://github.com/user-attachments/assets/83d15056-a2bd-41d9-a01d-9b8954260381) Gnome without policy file: ![image](https://github.com/user-attachments/assets/66c39d02-eed4-4f09-886f-621b6d37ff43) VSCode: ![image](https://github.com/user-attachments/assets/949dc470-c3df-4e2f-8cc6-31babaee1d18) User declines the permission request: ![image](https://github.com/user-attachments/assets/c5cbf056-f6f9-43a8-8d88-f2b0597e14d6) Release Notes: - Fixed file saving with root ownership on Linux. --- Cargo.lock | 2 + crates/fs/Cargo.toml | 3 + crates/fs/src/fs.rs | 141 +++++++++++++++++++++++++++++++++++-------- docs/src/linux.md | 10 +++ 4 files changed, 132 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9964dd56e574d..6b7c849a25b7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4792,11 +4792,13 @@ dependencies = [ "rope", "serde", "serde_json", + "shlex", "smol", "tempfile", "text", "time", "util", + "which 6.0.3", "windows 0.58.0", ] diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index fe145a6be589f..f31f378ea22bd 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -47,9 +47,12 @@ windows.workspace = true [target.'cfg(any(target_os = "linux", target_os = "freebsd"))'.dependencies] ashpd.workspace = true +which.workspace = true +shlex.workspace = true [dev-dependencies] gpui = { workspace = true, features = ["test-support"] } [features] test-support = ["gpui/test-support", "git/test-support"] + diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 0a0d2be6cdaec..be9f3f4e0f488 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -10,6 +10,8 @@ use git::GitHostingProviderRegistry; #[cfg(any(target_os = "linux", target_os = "freebsd"))] use ashpd::desktop::trash; #[cfg(any(target_os = "linux", target_os = "freebsd"))] +use smol::process::Command; +#[cfg(any(target_os = "linux", target_os = "freebsd"))] use std::fs::File; #[cfg(unix)] use std::os::fd::AsFd; @@ -514,24 +516,7 @@ impl Fs for RealFs { async fn atomic_write(&self, path: PathBuf, data: String) -> Result<()> { smol::unblock(move || { - let mut tmp_file = if cfg!(any(target_os = "linux", target_os = "freebsd")) { - // Use the directory of the destination as temp dir to avoid - // invalid cross-device link error, and XDG_CACHE_DIR for fallback. - // See https://github.com/zed-industries/zed/pull/8437 for more details. - NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir())) - } else if cfg!(target_os = "windows") { - // If temp dir is set to a different drive than the destination, - // we receive error: - // - // failed to persist temporary file: - // The system cannot move the file to a different disk drive. (os error 17) - // - // So we use the directory of the destination as a temp dir to avoid it. - // https://github.com/zed-industries/zed/issues/16571 - NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir())) - } else { - NamedTempFile::new() - }?; + let mut tmp_file = create_temp_file(&path)?; tmp_file.write_all(data.as_bytes())?; tmp_file.persist(path)?; Ok::<(), anyhow::Error>(()) @@ -546,13 +531,43 @@ impl Fs for RealFs { if let Some(path) = path.parent() { self.create_dir(path).await?; } - let file = smol::fs::File::create(path).await?; - let mut writer = smol::io::BufWriter::with_capacity(buffer_size, file); - for chunk in chunks(text, line_ending) { - writer.write_all(chunk.as_bytes()).await?; + match smol::fs::File::create(path).await { + Ok(file) => { + let mut writer = smol::io::BufWriter::with_capacity(buffer_size, file); + for chunk in chunks(text, line_ending) { + writer.write_all(chunk.as_bytes()).await?; + } + writer.flush().await?; + Ok(()) + } + Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { + if cfg!(any(target_os = "linux", target_os = "freebsd")) { + let target_path = path.to_path_buf(); + let temp_file = smol::unblock(move || create_temp_file(&target_path)).await?; + + let temp_path = temp_file.into_temp_path(); + let temp_path_for_write = temp_path.to_path_buf(); + + let async_file = smol::fs::OpenOptions::new() + .write(true) + .open(&temp_path) + .await?; + + let mut writer = smol::io::BufWriter::with_capacity(buffer_size, async_file); + + for chunk in chunks(text, line_ending) { + writer.write_all(chunk.as_bytes()).await?; + } + writer.flush().await?; + + write_to_file_as_root(temp_path_for_write, path.to_path_buf()).await + } else { + // Todo: Implement for Mac and Windows + Err(e.into()) + } + } + Err(e) => Err(e.into()), } - writer.flush().await?; - Ok(()) } async fn canonicalize(&self, path: &Path) -> Result { @@ -1999,6 +2014,84 @@ fn chunks(rope: &Rope, line_ending: LineEnding) -> impl Iterator { }) } +fn create_temp_file(path: &Path) -> Result { + let temp_file = if cfg!(any(target_os = "linux", target_os = "freebsd")) { + // Use the directory of the destination as temp dir to avoid + // invalid cross-device link error, and XDG_CACHE_DIR for fallback. + // See https://github.com/zed-industries/zed/pull/8437 for more details. + NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir()))? + } else if cfg!(target_os = "windows") { + // If temp dir is set to a different drive than the destination, + // we receive error: + // + // failed to persist temporary file: + // The system cannot move the file to a different disk drive. (os error 17) + // + // So we use the directory of the destination as a temp dir to avoid it. + // https://github.com/zed-industries/zed/issues/16571 + NamedTempFile::new_in(path.parent().unwrap_or(paths::temp_dir()))? + } else { + NamedTempFile::new()? + }; + + Ok(temp_file) +} + +#[cfg(target_os = "macos")] +async fn write_to_file_as_root(_temp_file_path: PathBuf, _target_file_path: PathBuf) -> Result<()> { + unimplemented!("write_to_file_as_root is not implemented") +} + +#[cfg(target_os = "windows")] +async fn write_to_file_as_root(_temp_file_path: PathBuf, _target_file_path: PathBuf) -> Result<()> { + unimplemented!("write_to_file_as_root is not implemented") +} + +#[cfg(any(target_os = "linux", target_os = "freebsd"))] +async fn write_to_file_as_root(temp_file_path: PathBuf, target_file_path: PathBuf) -> Result<()> { + use shlex::try_quote; + use std::os::unix::fs::PermissionsExt; + use which::which; + + let pkexec_path = smol::unblock(|| which("pkexec")) + .await + .map_err(|_| anyhow::anyhow!("pkexec not found in PATH"))?; + + let script_file = smol::unblock(move || { + let script_file = tempfile::Builder::new() + .prefix("write-to-file-as-root-") + .tempfile_in(paths::temp_dir())?; + + writeln!( + script_file.as_file(), + "#!/usr/bin/env sh\nset -eu\ncat \"{}\" > \"{}\"", + try_quote(&temp_file_path.to_string_lossy())?, + try_quote(&target_file_path.to_string_lossy())? + )?; + + let mut perms = script_file.as_file().metadata()?.permissions(); + perms.set_mode(0o700); // rwx------ + script_file.as_file().set_permissions(perms)?; + + Result::<_>::Ok(script_file) + }) + .await?; + + let script_path = script_file.into_temp_path(); + + let output = Command::new(&pkexec_path) + .arg("--disable-internal-agent") + .arg(&script_path) + .output() + .await?; + + if !output.status.success() { + return Err(anyhow::anyhow!("Failed to write to file as root")); + } + + Ok(()) +} + pub fn normalize_path(path: &Path) -> PathBuf { let mut components = path.components().peekable(); let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() { diff --git a/docs/src/linux.md b/docs/src/linux.md index 6e62300ec82e6..f87de5b5bdbac 100644 --- a/docs/src/linux.md +++ b/docs/src/linux.md @@ -170,3 +170,13 @@ rm ~/.local/zed.app/lib/libcrypto.so.1.1 ``` This will force zed to fallback to the system `libssl` and `libcrypto` libraries. + +### Editing files requiring root access + +When you try to edit files that require root access, Zed requires `pkexec` (part of polkit) to handle authentication prompts. + +Polkit comes pre-installed with most desktop environments like GNOME and KDE. If you're using a minimal system and polkit is not installed, you can install it with: + +- Ubuntu/Debian: `sudo apt install policykit-1` +- Fedora: `sudo dnf install polkit` +- Arch Linux: `sudo pacman -S polkit`