diff --git a/NEWS.md b/NEWS.md index adee14bd..5f35d413 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## Unreleased +- Fixed: Restore now sets Unix user/group ownership on symlinks and directories. Previously, only file ownership was restored. (Setting file ownership typically requires restoring as root.) + - Performance: Also keep a cache of the existence of blocks that have not yet been read. - Changed: The format and keys written by `--metrics-json` has changed. diff --git a/src/owner/unix.rs b/src/owner/unix.rs index 6147e9ab..aa414260 100644 --- a/src/owner/unix.rs +++ b/src/owner/unix.rs @@ -15,17 +15,15 @@ //! Unix implementation of file ownership. use std::io; -use std::os::unix::fs::MetadataExt; +use std::os::unix::fs::{lchown, MetadataExt}; use std::sync::Mutex; use std::{fs, path::Path}; use lazy_static::lazy_static; -use nix::errno::Errno; -use nix::unistd; use uzers::{Groups, Users, UsersCache}; use super::Owner; -use crate::{Error, Result}; +use crate::Result; lazy_static! { static ref USERS_CACHE: Mutex = Mutex::new(UsersCache::new()); @@ -44,34 +42,29 @@ impl From<&fs::Metadata> for Owner { } } -#[mutants::skip] // TODO: Test that at least groups are restored! +#[mutants::skip] // TODO: Difficult to test as non-root but we could at least test that at least groups are restored! pub(crate) fn set_owner(owner: &Owner, path: &Path) -> Result<()> { let users_cache = USERS_CACHE.lock().unwrap(); let uid_opt = owner .user .as_ref() .and_then(|user| users_cache.get_user_by_name(&user)) - .map(|user| user.uid()) - .map(unistd::Uid::from_raw); + .map(|user| user.uid()); let gid_opt = owner .group .as_ref() .and_then(|group| users_cache.get_group_by_name(&group)) - .map(|group| group.gid()) - .map(unistd::Gid::from_raw); + .map(|group| group.gid()); drop(users_cache); // TODO: use `std::os::unix::fs::chown(path, uid, gid)?;` once stable - match unistd::chown(path, uid_opt, gid_opt) { + match lchown(path, uid_opt, gid_opt) { Ok(()) => Ok(()), - Err(Errno::EPERM) => { + Err(err) if err.kind() == io::ErrorKind::PermissionDenied => { // If the restore is not run as root (or with special capabilities) // then we probably can't set ownership, and there's no point // complaining Ok(()) } - Err(errno) => Err(Error::SetOwner { - path: path.to_path_buf(), - source: io::Error::from_raw_os_error(errno as i32), - }), + Err(err) => Err(err.into()), } } diff --git a/src/restore.rs b/src/restore.rs index 9ce5ffe7..104b9047 100644 --- a/src/restore.rs +++ b/src/restore.rs @@ -111,6 +111,7 @@ pub fn restore( path, unix_mode: entry.unix_mode(), mtime: entry.mtime(), + owner: entry.owner().clone(), }) } Kind::File => { @@ -163,6 +164,7 @@ struct DirDeferral { path: PathBuf, unix_mode: UnixMode, mtime: OffsetDateTime, + owner: Owner, } fn apply_deferrals(deferrals: &[DirDeferral]) -> Result { @@ -171,8 +173,13 @@ fn apply_deferrals(deferrals: &[DirDeferral]) -> Result { path, unix_mode, mtime, + owner, } in deferrals { + if let Err(err) = owner.set_owner(path) { + error!(?path, ?err, "Error restoring ownership"); + stats.errors += 1; + } if let Err(err) = unix_mode.set_permissions(path) { error!(?path, ?err, "Failed to set directory permissions"); stats.errors += 1; @@ -254,7 +261,8 @@ fn restore_file( } #[cfg(unix)] -fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result<()> { +fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result { + let mut stats = RestoreStats::default(); use std::os::unix::fs as unix_fs; if let Some(ref target) = entry.symlink_target() { if let Err(source) = unix_fs::symlink(target, path) { @@ -263,6 +271,10 @@ fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result<()> { source, }); } + if let Err(err) = &entry.owner().set_owner(path) { + error!(?path, ?err, "Error restoring ownership"); + stats.errors += 1; + } let mtime = entry.mtime().to_file_time(); if let Err(source) = set_symlink_file_times(path, mtime, mtime) { return Err(Error::RestoreModificationTime { @@ -272,15 +284,16 @@ fn restore_symlink(path: &Path, entry: &IndexEntry) -> Result<()> { } } else { error!(apath = ?entry.apath(), "No target in symlink entry"); + stats.errors += 1; } - Ok(()) + Ok(stats) } #[cfg(not(unix))] #[mutants::skip] -fn restore_symlink(_restore_path: &Path, entry: &IndexEntry) -> Result<()> { +fn restore_symlink(_restore_path: &Path, entry: &IndexEntry) -> Result { // TODO: Add a test with a canned index containing a symlink, and expect // it cannot be restored on Windows and can be on Unix. warn!("Can't restore symlinks on non-Unix: {}", entry.apath()); - Ok(()) + Ok(RestoreStats::default()) }