Refuse to replace a file the user cannot write (Unix)

A rename needs only a writable folder, so a read-only file would be
replaced where writing it in place would fail. Before staging, the
existing file is opened for writing without truncating; when that fails
the save fails with a read-only error.
This commit is contained in:
aiongg
2026-10-12 20:23:17 +02:00
committed by skylar taylor-barrick
parent 658ca37c8e
commit d78b352004
2 changed files with 47 additions and 1 deletions
+29 -1
View File
@@ -156,7 +156,11 @@ pub fn atomic_write_custom(fs: &dyn AtomicFs, retry: RenameRetry, path: &Path, b
fn replace(fs: &dyn AtomicFs, retry: RenameRetry, path: &Path, fill: impl FnOnce(&mut File) -> io::Result<()>) -> io::Result<()> {
let target = resolve_links(path)?;
let (dir, name) = split(&target, path)?;
let existed = target.exists();
let existing = std::fs::metadata(&target).ok();
let existed = existing.is_some();
if existing.as_ref().is_some_and(|m| m.is_file()) {
check_writable(&target).map_err(|e| read_only_error(e, path))?;
}
let tmp = stage(fs, &dir, &name, path, existed, fill)?;
@@ -173,6 +177,19 @@ fn replace(fs: &dyn AtomicFs, retry: RenameRetry, path: &Path, fill: impl FnOnce
Ok(())
}
/// A rename replaces a file the user can't write (Unix only needs the folder to be writable), so
/// refuse, as writing it in place would. Opening for writing without truncating changes nothing.
#[cfg(unix)]
fn check_writable(target: &Path) -> io::Result<()> {
OpenOptions::new().write(true).open(target).map(drop)
}
/// Windows refuses to rename over a read-only file by itself.
#[cfg(not(unix))]
fn check_writable(_: &Path) -> io::Result<()> {
Ok(())
}
/// The folder `target` is in and its file name. `path` is the caller's path, for the error.
fn split(target: &Path, path: &Path) -> io::Result<(PathBuf, String)> {
let name = target.file_name().ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, format!("{}: not a file path", path.display())))?;
@@ -346,6 +363,17 @@ fn write_error(e: io::Error, path: &Path, dir: &Path, existed: bool) -> io::Erro
io::Error::new(e.kind(), msg)
}
fn read_only_error(e: io::Error, path: &Path) -> io::Error {
let kept = unchanged(true);
let msg = match e.kind() {
io::ErrorKind::PermissionDenied | io::ErrorKind::ReadOnlyFilesystem => {
format!("cannot save {}: the file is read-only ({e}){kept}", path.display())
}
_ => format!("cannot save {}: {e}{kept}", path.display()),
};
io::Error::new(e.kind(), msg)
}
fn rename_error(e: io::Error, path: &Path, existed: bool) -> io::Error {
let kept = unchanged(existed);
let msg = if is_cross_device(&e) {
+18
View File
@@ -152,3 +152,21 @@ fn symlink_loop_is_an_error() {
assert!(std::fs::symlink_metadata(d.0.join("a")).map(|m| m.file_type().is_symlink()).unwrap_or(false));
assert_no_temp(&d.0);
}
#[cfg(unix)]
#[test]
fn read_only_target_is_not_replaced() {
use std::os::unix::fs::PermissionsExt;
let d = TempDir::new("read_only");
let p = d.0.join("locked.txt");
std::fs::write(&p, ORIGINAL).ok();
std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o444)).ok();
// Root may write the file anyway; the check only means something for an ordinary user.
if std::fs::OpenOptions::new().write(true).open(&p).is_ok() {
return;
}
let msg = atomic_write(&p, b"new").err().map(|e| e.to_string()).unwrap_or_default();
assert!(msg.contains("read-only"), "{msg}");
assert_eq!(std::fs::read(&p).ok(), Some(ORIGINAL.to_vec()));
assert_no_temp(&d.0);
}