From 531629e1c9f5d2abfc660dd933ad6be1fcdc78cf Mon Sep 17 00:00:00 2001 From: archnode Date: Wed, 6 May 2026 15:21:55 +0200 Subject: [PATCH 1/4] #103: Add elevation when directory access is denied. --- src/actions.rs | 18 ++--- src/deploy.rs | 173 +++++++++++++++++++++++++++++++++++++++++++--- src/difference.rs | 15 ++-- src/filesystem.rs | 159 ++++++++++++++++++++++++++++++++++++------ 4 files changed, 324 insertions(+), 41 deletions(-) diff --git a/src/actions.rs b/src/actions.rs index c2a6887..138bc9d 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -115,7 +115,7 @@ pub fn delete_symlink( info!("{} symlink {:?} -> {:?}", "[-]".red(), source, target); let comparison = fs - .compare_symlink(source, target) + .compare_symlink(source, target, &None) .context("detect symlink's current state")?; debug!("Current state: {}", comparison); @@ -170,7 +170,7 @@ pub fn delete_template( info!("{} template {:?} -> {:?}", "[-]".red(), source, target); let comparison = fs - .compare_template(target, cache) + .compare_template(target, cache, &None) .context("detect templated file's current state")?; debug!("Current state: {}", comparison); @@ -249,7 +249,7 @@ pub fn create_symlink( ); let comparison = fs - .compare_symlink(source, &target.target) + .compare_symlink(source, &target.target, &target.owner) .context("detect symlink's current state")?; debug!("Current state: {}", comparison); @@ -321,7 +321,7 @@ pub fn create_template( ); let comparison = fs - .compare_template(&target.target, cache) + .compare_template(&target.target, cache, &target.owner) .context("detect templated file's current state")?; debug!("Current state: {}", comparison); @@ -404,7 +404,7 @@ pub fn update_symlink( debug!("Updating symlink {:?} -> {:?}...", source, target.target); let comparison = fs - .compare_symlink(source, &target.target) + .compare_symlink(source, &target.target, &target.owner) .context("detect symlink's current state")?; debug!("Current state: {}", comparison); @@ -472,7 +472,7 @@ pub fn update_template( ) -> Result { debug!("Updating template {:?} -> {:?}...", source, target.target); let comparison = fs - .compare_template(&target.target, cache) + .compare_template(&target.target, cache, &target.owner) .context("detect templated file's current state")?; debug!("Current state: {}", comparison); @@ -482,6 +482,7 @@ pub fn update_template( difference::print_template_diff( source, target, + fs, handlebars, variables, diff_context_lines, @@ -525,6 +526,7 @@ pub fn update_template( difference::print_template_diff( source, target, + fs, handlebars, variables, diff_context_lines, @@ -538,7 +540,7 @@ pub fn update_template( TemplateComparison::Changed => { // At this point, we're not sure if there's a difference between the rendered source // and target, only that the target has been modified in some way. - let diff = generate_template_diff(source, target, handlebars, variables, false) + let diff = generate_template_diff(source, target, fs, handlebars, variables, false) .context("diff source and target")?; if diff_nonempty(&diff) { error!( @@ -576,7 +578,7 @@ pub(crate) fn perform_template_deploy( variables: &Variables, ) -> Result<()> { let file_contents = fs - .read_to_string(source) + .read_to_string(source, &None) .context("read template source file")?; let file_contents = match target { Some(t) => t.apply_actions(file_contents), diff --git a/src/deploy.rs b/src/deploy.rs index 5901e67..9180754 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -707,9 +707,13 @@ mod test { // create_symlink fs.expect_compare_symlink() .times(1) - .with(function(path_eq("a_in")), function(path_eq("a_out"))) + .with( + function(path_eq("a_in")), + function(path_eq("a_out")), + eq(None), + ) .in_sequence(&mut seq) - .returning(|_, _| Ok(SymlinkComparison::OnlySourceExists)); + .returning(|_, _, _| Ok(SymlinkComparison::OnlySourceExists)); fs.expect_create_dir_all() .times(1) .with(function(path_eq("")), eq(None)) // parent of a_out @@ -731,9 +735,10 @@ mod test { .with( function(path_eq("b_out")), function(path_eq("cache/b_cache")), + eq(None), ) .in_sequence(&mut seq) - .returning(|_, _| Ok(TemplateComparison::BothMissing)); + .returning(|_, _, _| Ok(TemplateComparison::BothMissing)); fs.expect_create_dir_all() .times(1) .with(function(path_eq("")), eq(None)) // parent of b_out @@ -741,9 +746,9 @@ mod test { .returning(|_, _| Ok(())); fs.expect_read_to_string() .times(1) - .with(function(path_eq("b_in"))) + .with(function(path_eq("b_in")), eq(None)) .in_sequence(&mut seq) - .returning(|_| Ok("Hello!".into())); + .returning(|_, _| Ok("Hello!".into())); fs.expect_create_dir_all() .times(1) .with(function(path_eq("cache")), eq(None)) @@ -814,9 +819,13 @@ mod test { // create_symlink fs.expect_compare_symlink() .times(1) - .with(function(path_eq("a_in")), function(path_eq("a_out"))) + .with( + function(path_eq("a_in")), + function(path_eq("a_out")), + eq(None), + ) .in_sequence(&mut seq) - .returning(|_, _| Ok(SymlinkComparison::Changed)); + .returning(|_, _, _| Ok(SymlinkComparison::Changed)); // create_template fs.expect_compare_template() @@ -824,9 +833,10 @@ mod test { .with( function(path_eq("b_out")), function(path_eq("cache/b_cache")), + eq(None), ) .in_sequence(&mut seq) - .returning(|_, _| Ok(TemplateComparison::Changed)); + .returning(|_, _, _| Ok(TemplateComparison::Changed)); // Reality let mut runner = actions::RealActionRunner::new( @@ -853,4 +863,151 @@ mod test { .unwrap() ); } + + #[test] + fn low_level_symlink_owner_passes_owner_to_compare() { + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + let owner = Some(crate::config::UnixUser::Name("root".into())); + let target = SymbolicTarget { + target: PathBuf::from("owned_out"), + owner: owner.clone(), + recurse: None, + condition: None, + }; + + fs.expect_compare_symlink() + .times(1) + .with( + function(path_eq("owned_in")), + function(path_eq("owned_out")), + eq(owner.clone()), + ) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(SymlinkComparison::OnlySourceExists)); + fs.expect_create_dir_all() + .times(1) + .with(function(path_eq("")), eq(owner.clone())) + .in_sequence(&mut seq) + .returning(|_, _| Ok(())); + fs.expect_make_symlink() + .times(1) + .with( + function(path_eq("owned_out")), + function(path_eq("owned_in")), + eq(owner.clone()), + ) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + + assert!( + runner + .create_symlink(&PathBuf::from("owned_in"), &target) + .unwrap() + ); + } + + #[test] + fn low_level_update_template_reads_owned_target_via_filesystem() { + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + + let opt = Options::default(); + let handlebars = handlebars::Handlebars::new(); + let variables = toml::map::Map::new(); + let owner = Some(crate::config::UnixUser::Name("root".into())); + let target = TemplateTarget { + target: PathBuf::from("owned_out"), + owner: owner.clone(), + append: None, + prepend: None, + condition: None, + }; + + fs.expect_compare_template() + .times(1) + .with( + function(path_eq("owned_out")), + function(path_eq("cache/owned_cache")), + eq(owner.clone()), + ) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(TemplateComparison::Changed)); + fs.expect_read_to_string() + .times(1) + .with(function(path_eq("owned_in")), eq(None)) + .in_sequence(&mut seq) + .returning(|_, _| Ok("Hello!".into())); + fs.expect_read_to_string() + .times(1) + .with(function(path_eq("owned_out")), eq(owner.clone())) + .in_sequence(&mut seq) + .returning(|_, _| Ok("Hello!".into())); + fs.expect_read_to_string() + .times(1) + .with(function(path_eq("owned_in")), eq(None)) + .in_sequence(&mut seq) + .returning(|_, _| Ok("Hello!".into())); + fs.expect_create_dir_all() + .times(1) + .with(function(path_eq("cache")), eq(None)) + .in_sequence(&mut seq) + .returning(|_, _| Ok(())); + fs.expect_write() + .times(1) + .with( + function(path_eq("cache/owned_cache")), + eq(String::from("Hello!")), + ) + .in_sequence(&mut seq) + .returning(|_, _| Ok(())); + fs.expect_copy_file() + .times(1) + .with( + function(path_eq("cache/owned_cache")), + function(path_eq("owned_out")), + eq(owner.clone()), + ) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + fs.expect_copy_permissions() + .times(1) + .with( + function(path_eq("owned_in")), + function(path_eq("owned_out")), + eq(owner), + ) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(())); + + let mut runner = actions::RealActionRunner::new( + &mut fs, + &handlebars, + &variables, + opt.force, + opt.diff_context_lines, + ); + + assert!( + runner + .update_template( + &PathBuf::from("owned_in"), + &PathBuf::from("cache/owned_cache"), + &target, + ) + .unwrap() + ); + } } diff --git a/src/difference.rs b/src/difference.rs index b553a16..ce6e883 100644 --- a/src/difference.rs +++ b/src/difference.rs @@ -3,10 +3,10 @@ use crossterm::style::Stylize; use handlebars::Handlebars; use std::cmp::{max, min}; -use std::fs; use std::path::Path; use crate::config::{TemplateTarget, Variables}; +use crate::filesystem::Filesystem; pub type Diff = Vec>; pub type HunkDiff = Vec<(usize, usize, Diff)>; @@ -14,12 +14,13 @@ pub type HunkDiff = Vec<(usize, usize, Diff)>; pub fn print_template_diff( source: &Path, target: &TemplateTarget, + filesystem: &mut dyn Filesystem, handlebars: &Handlebars<'_>, variables: &Variables, diff_context_lines: usize, ) { if log_enabled!(log::Level::Info) { - match generate_template_diff(source, target, handlebars, variables, true) { + match generate_template_diff(source, target, filesystem, handlebars, variables, true) { Ok(diff) => { if diff_nonempty(&diff) { info!( @@ -44,18 +45,22 @@ pub fn print_template_diff( pub fn generate_template_diff( source: &Path, target: &TemplateTarget, + filesystem: &mut dyn Filesystem, handlebars: &Handlebars<'_>, variables: &Variables, source_to_target: bool, ) -> Result { - let file_contents = fs::read_to_string(source).context("read template source file")?; + let file_contents = filesystem + .read_to_string(source, &None) + .context("read template source file")?; let file_contents = target.apply_actions(file_contents); let rendered = handlebars .render_template(&file_contents, variables) .context("render template")?; - let target_contents = - fs::read_to_string(&target.target).context("read template target file")?; + let target_contents = filesystem + .read_to_string(&target.target, &target.owner) + .context("read template target file")?; let diff_result = if source_to_target { diff::lines(&target_contents, &rendered) diff --git a/src/filesystem.rs b/src/filesystem.rs index 6a7c7be..fe26a70 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -6,6 +6,8 @@ use serde::ser::Serialize; use std::collections::BTreeMap; use std::fs::{self, File}; use std::io::{self, ErrorKind, Read}; +#[cfg(unix)] +use std::os::unix::ffi::OsStringExt; use std::path::{Path, PathBuf}; #[cfg(unix)] use std::process::Command; @@ -44,16 +46,26 @@ where #[cfg_attr(test, mockall::automock)] pub trait Filesystem { /// Check state of expected symlink on disk - fn compare_symlink(&mut self, source: &Path, link: &Path) -> Result; + fn compare_symlink( + &mut self, + source: &Path, + link: &Path, + owner: &Option, + ) -> Result; /// Check state of expected symbolic link on disk - fn compare_template(&mut self, target: &Path, cache: &Path) -> Result; + fn compare_template( + &mut self, + target: &Path, + cache: &Path, + owner: &Option, + ) -> Result; /// Removes a file or folder, elevating privileges if needed fn remove_file(&mut self, path: &Path) -> Result<()>; /// Read contents of file into a string - fn read_to_string(&mut self, path: &Path) -> Result; + fn read_to_string(&mut self, path: &Path, owner: &Option) -> Result; /// Write string to file, without elevating privileges fn write(&mut self, path: &Path, content: String) -> Result<()>; @@ -101,7 +113,12 @@ impl RealFilesystem { #[cfg(windows)] impl Filesystem for RealFilesystem { - fn compare_symlink(&mut self, source: &Path, link: &Path) -> Result { + fn compare_symlink( + &mut self, + source: &Path, + link: &Path, + _owner: &Option, + ) -> Result { let source_state = get_file_state(source).context("get source state")?; trace!("Source state: {:#?}", source_state); let link_state = get_file_state(link).context("get link state")?; @@ -110,7 +127,12 @@ impl Filesystem for RealFilesystem { compare_symlink(source, source_state, link_state) } - fn compare_template(&mut self, target: &Path, cache: &Path) -> Result { + fn compare_template( + &mut self, + target: &Path, + cache: &Path, + _owner: &Option, + ) -> Result { let target_state = get_file_state(target).context("get state of target")?; trace!("Target state: {:#?}", target_state); let cache_state = get_file_state(cache).context("get state of cache")?; @@ -128,7 +150,7 @@ impl Filesystem for RealFilesystem { } } - fn read_to_string(&mut self, path: &Path) -> Result { + fn read_to_string(&mut self, path: &Path, _owner: &Option) -> Result { fs::read_to_string(path).context("read from file") } @@ -265,19 +287,72 @@ impl RealFilesystem { let process_uid = unsafe { libc::geteuid() }; Ok(file_uid == process_uid) } + + fn get_file_state(&mut self, path: &Path, owner: &Option) -> Result { + match try_get_file_state(path) { + Ok(state) => Ok(state), + Err(e) if e.kind() == ErrorKind::PermissionDenied && owner.is_some() => { + self.get_file_state_as_root(path) + } + Err(e) => Err(e).context("read contents of file that isn't symbolic or directory"), + } + } + + fn get_file_state_as_root(&mut self, path: &Path) -> Result { + let output = self + .sudo(format!("detecting file state for {path:?} as root")) + .arg("sh") + .arg("-c") + .arg( + "if [ -L \"$1\" ]; then printf 'symlink\\0'; readlink -- \"$1\"; elif [ -d \"$1\" ]; then printf 'directory\\0'; elif [ -e \"$1\" ]; then printf 'file\\0'; cat -- \"$1\"; else printf 'missing\\0'; fi", + ) + .arg("sh") + .arg(path) + .output() + .context("run sudo file state probe")?; + + anyhow::ensure!(output.status.success(), "sudo file state probe failed"); + parse_sudo_file_state(output.stdout) + } + + fn read_to_string_as_root(&mut self, path: &Path) -> Result { + let output = self + .sudo(format!("reading file {path:?} as root")) + .arg("cat") + .arg(path) + .output() + .context("run sudo cat")?; + + anyhow::ensure!(output.status.success(), "sudo cat failed"); + String::from_utf8(output.stdout) + .map_err(|e| io::Error::new(ErrorKind::InvalidData, e)) + .context("read from file as root") + } } #[cfg(unix)] impl Filesystem for RealFilesystem { - fn compare_symlink(&mut self, source: &Path, link: &Path) -> Result { + fn compare_symlink( + &mut self, + source: &Path, + link: &Path, + owner: &Option, + ) -> Result { let source_state = get_file_state(source).context("get source state")?; - let link_state = get_file_state(link).context("get link state")?; + let link_state = self.get_file_state(link, owner).context("get link state")?; compare_symlink(source, source_state, link_state) } - fn compare_template(&mut self, target: &Path, cache: &Path) -> Result { - let target_state = get_file_state(target).context("get state of target")?; + fn compare_template( + &mut self, + target: &Path, + cache: &Path, + owner: &Option, + ) -> Result { + let target_state = self + .get_file_state(target, owner) + .context("get state of target")?; let cache_state = get_file_state(cache).context("get state of cache")?; Ok(compare_template(target_state, cache_state)) @@ -311,8 +386,14 @@ impl Filesystem for RealFilesystem { } } - fn read_to_string(&mut self, path: &Path) -> Result { - fs::read_to_string(path).context("read from file") + fn read_to_string(&mut self, path: &Path, owner: &Option) -> Result { + match fs::read_to_string(path) { + Ok(contents) => Ok(contents), + Err(e) if e.kind() == ErrorKind::PermissionDenied && owner.is_some() => { + self.read_to_string_as_root(path) + } + Err(e) => Err(e).context("read from file"), + } } fn write(&mut self, path: &Path, content: String) -> Result<()> { @@ -555,7 +636,12 @@ impl DryRunFilesystem { } impl Filesystem for DryRunFilesystem { - fn compare_symlink(&mut self, source: &Path, link: &Path) -> Result { + fn compare_symlink( + &mut self, + source: &Path, + link: &Path, + _owner: &Option, + ) -> Result { let source_state = if let Some(state) = self.file_states.get(source) { debug!("Cached (probably not actual) source state: {:?}", state); state.clone() @@ -576,7 +662,12 @@ impl Filesystem for DryRunFilesystem { compare_symlink(source, source_state, link_state) } - fn compare_template(&mut self, target: &Path, cache: &Path) -> Result { + fn compare_template( + &mut self, + target: &Path, + cache: &Path, + _owner: &Option, + ) -> Result { let target_state = if let Some(state) = self.file_states.get(target) { debug!("Cached (probably not actual) target state: {:?}", state); state.clone() @@ -603,7 +694,7 @@ impl Filesystem for DryRunFilesystem { Ok(()) } - fn read_to_string(&mut self, path: &Path) -> Result { + fn read_to_string(&mut self, path: &Path, _owner: &Option) -> Result { debug!("Reading contents of file {:?}", path); match self.get_state(path).context("get file state")? { FileState::File(s) => Ok(s.context("invalid utf-8 in template source")?), @@ -692,7 +783,7 @@ impl Filesystem for DryRunFilesystem { // === Comparisons === -fn get_file_state(path: &Path) -> Result { +fn try_get_file_state(path: &Path) -> io::Result { if let Ok(target) = fs::read_link(path) { return Ok(FileState::SymbolicLink(target)); } @@ -705,7 +796,34 @@ fn get_file_state(path: &Path) -> Result { Ok(f) => Ok(FileState::File(Some(f))), Err(e) if e.kind() == ErrorKind::InvalidData => Ok(FileState::File(None)), Err(e) if e.kind() == ErrorKind::NotFound => Ok(FileState::Missing), - Err(e) => Err(e).context("read contents of file that isn't symbolic or directory")?, + Err(e) => Err(e), + } +} + +fn get_file_state(path: &Path) -> Result { + try_get_file_state(path).context("read contents of file that isn't symbolic or directory") +} + +#[cfg(unix)] +fn parse_sudo_file_state(output: Vec) -> Result { + let separator = output + .iter() + .position(|byte| *byte == b'\0') + .context("parse sudo file state probe output")?; + let (kind, payload) = output.split_at(separator); + let payload = &payload[1..]; + + match kind { + b"symlink" => Ok(FileState::SymbolicLink(PathBuf::from( + std::ffi::OsString::from_vec(payload.to_vec()), + ))), + b"directory" => Ok(FileState::Directory), + b"file" => match String::from_utf8(payload.to_vec()) { + Ok(contents) => Ok(FileState::File(Some(contents))), + Err(_) => Ok(FileState::File(None)), + }, + b"missing" => Ok(FileState::Missing), + _ => anyhow::bail!("unexpected sudo file state probe output"), } } @@ -897,7 +1015,7 @@ mod test { fs.write(&PathBuf::from("test"), "hello world!".into()) .unwrap(); assert_eq!( - fs.read_to_string(&PathBuf::from("test")).unwrap(), + fs.read_to_string(&PathBuf::from("test"), &None).unwrap(), "hello world!" ); } @@ -932,7 +1050,8 @@ mod test { assert_eq!( fs.compare_template( &PathBuf::from("target_dir/target"), - &PathBuf::from("cache_dir/cache") + &PathBuf::from("cache_dir/cache"), + &None, ) .unwrap(), TemplateComparison::BothMissing @@ -943,7 +1062,7 @@ mod test { // perform_template_deploy assert_eq!( - fs.read_to_string(&PathBuf::from("source")).unwrap(), + fs.read_to_string(&PathBuf::from("source"), &None).unwrap(), "{{name}}" ); let rendered = String::from("John"); From 8432eff9ce7b24ca1db5f60903efff8f4fba2596 Mon Sep 17 00:00:00 2001 From: archnode Date: Wed, 6 May 2026 15:55:00 +0200 Subject: [PATCH 2/4] #103: Add elevation to symlink comparison. --- src/filesystem.rs | 103 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index fe26a70..7e7daf3 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -315,6 +315,41 @@ impl RealFilesystem { parse_sudo_file_state(output.stdout) } + fn resolve_symlink_destination( + &mut self, + link: &Path, + owner: &Option, + ) -> Result> { + match real_path(link) { + Ok(path) => Ok(Some(path)), + Err(e) if e.kind() == ErrorKind::PermissionDenied && owner.is_some() => { + self.resolve_symlink_destination_as_root(link) + } + Err(e) if e.kind() == ErrorKind::NotFound => Ok(None), + Err(e) => Err(e).context("get real path of existing symlink"), + } + } + + fn resolve_symlink_destination_as_root(&mut self, path: &Path) -> Result> { + let output = self + .sudo(format!("resolving existing symlink {path:?} as root")) + .arg("sh") + .arg("-c") + .arg( + "if resolved=$(realpath -- \"$1\" 2>/dev/null); then printf 'resolved\\0%s' \"$resolved\"; elif [ -L \"$1\" ] && [ ! -e \"$1\" ]; then printf 'missing\\0'; else exit 1; fi", + ) + .arg("sh") + .arg(path) + .output() + .context("run sudo symlink resolution probe")?; + + anyhow::ensure!( + output.status.success(), + "sudo symlink resolution probe failed" + ); + parse_sudo_resolved_path(output.stdout) + } + fn read_to_string_as_root(&mut self, path: &Path) -> Result { let output = self .sudo(format!("reading file {path:?} as root")) @@ -341,7 +376,24 @@ impl Filesystem for RealFilesystem { let source_state = get_file_state(source).context("get source state")?; let link_state = self.get_file_state(link, owner).context("get link state")?; - compare_symlink(source, source_state, link_state) + Ok(match (source_state, link_state) { + (FileState::Missing, FileState::SymbolicLink(_)) => SymlinkComparison::OnlyTargetExists, + (_, FileState::SymbolicLink(_)) => { + let expected_target = real_path(source).context("get real path of source")?; + match self + .resolve_symlink_destination(link, owner) + .context("resolve symlink target")? + { + Some(actual_target) if actual_target == expected_target => { + SymlinkComparison::Identical + } + Some(_) | None => SymlinkComparison::Changed, + } + } + (FileState::Missing, FileState::Missing) => SymlinkComparison::BothMissing, + (_, FileState::Missing) => SymlinkComparison::OnlySourceExists, + _ => SymlinkComparison::TargetNotSymlink, + }) } fn compare_template( @@ -827,6 +879,24 @@ fn parse_sudo_file_state(output: Vec) -> Result { } } +#[cfg(unix)] +fn parse_sudo_resolved_path(output: Vec) -> Result> { + let separator = output + .iter() + .position(|byte| *byte == b'\0') + .context("parse sudo symlink resolution probe output")?; + let (kind, payload) = output.split_at(separator); + let payload = &payload[1..]; + + match kind { + b"resolved" => Ok(Some(PathBuf::from(std::ffi::OsString::from_vec( + payload.to_vec(), + )))), + b"missing" => Ok(None), + _ => anyhow::bail!("unexpected sudo symlink resolution probe output"), + } +} + #[derive(Debug, PartialEq, Eq)] pub enum SymlinkComparison { Identical, @@ -998,6 +1068,17 @@ pub fn platform_dunce(path: &Path) -> PathBuf { #[cfg(test)] mod test { use super::*; + #[cfg(unix)] + use std::time::{SystemTime, UNIX_EPOCH}; + + #[cfg(unix)] + fn unique_test_path(name: &str) -> PathBuf { + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("clock before unix epoch") + .as_nanos(); + std::env::temp_dir().join(format!("dotter-{name}-{}-{unique}", std::process::id())) + } #[test] fn simple_remove() { @@ -1130,4 +1211,24 @@ mod test { fs.copy_file(&PathBuf::from("link"), &PathBuf::from("link2"), &None) .unwrap_err(); } + + #[cfg(unix)] + #[test] + fn compare_symlink_accepts_relative_target_to_same_file() { + let root = unique_test_path("relative-symlink"); + let source_dir = root.join("source"); + let link_dir = root.join("links"); + std::fs::create_dir_all(&source_dir).unwrap(); + std::fs::create_dir_all(&link_dir).unwrap(); + + let source = source_dir.join("file.txt"); + let link = link_dir.join("file.txt"); + std::fs::write(&source, "hello").unwrap(); + std::os::unix::fs::symlink(PathBuf::from("../source/file.txt"), &link).unwrap(); + + let result = RealFilesystem::new(true).compare_symlink(&source, &link, &None); + std::fs::remove_dir_all(&root).unwrap(); + + assert_eq!(result.unwrap(), SymlinkComparison::Identical); + } } From 36660f82a9f055c71739841be4641e6227a9506a Mon Sep 17 00:00:00 2001 From: archnode Date: Wed, 6 May 2026 16:06:49 +0200 Subject: [PATCH 3/4] #103: Add elevation to dry run. --- src/filesystem.rs | 333 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 232 insertions(+), 101 deletions(-) diff --git a/src/filesystem.rs b/src/filesystem.rs index 7e7daf3..656987a 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -267,18 +267,7 @@ impl RealFilesystem { } fn sudo(&mut self, goal: impl AsRef) -> Command { - if !self.sudo_occurred { - warn!("Elevating permissions ({})", goal.as_ref()); - if !log_enabled!(log::Level::Debug) { - warn!( - "To see more than the first time elevated permissions are used, use verbosity 2 or more (-vv)" - ); - } - self.sudo_occurred = true; - } else { - debug!("Elevating permissions ({})", goal.as_ref()); - } - Command::new("sudo") + sudo_command(&mut self.sudo_occurred, goal) } fn is_owned_by_user(path: &Path) -> Result { @@ -289,30 +278,7 @@ impl RealFilesystem { } fn get_file_state(&mut self, path: &Path, owner: &Option) -> Result { - match try_get_file_state(path) { - Ok(state) => Ok(state), - Err(e) if e.kind() == ErrorKind::PermissionDenied && owner.is_some() => { - self.get_file_state_as_root(path) - } - Err(e) => Err(e).context("read contents of file that isn't symbolic or directory"), - } - } - - fn get_file_state_as_root(&mut self, path: &Path) -> Result { - let output = self - .sudo(format!("detecting file state for {path:?} as root")) - .arg("sh") - .arg("-c") - .arg( - "if [ -L \"$1\" ]; then printf 'symlink\\0'; readlink -- \"$1\"; elif [ -d \"$1\" ]; then printf 'directory\\0'; elif [ -e \"$1\" ]; then printf 'file\\0'; cat -- \"$1\"; else printf 'missing\\0'; fi", - ) - .arg("sh") - .arg(path) - .output() - .context("run sudo file state probe")?; - - anyhow::ensure!(output.status.success(), "sudo file state probe failed"); - parse_sudo_file_state(output.stdout) + get_file_state_with_owner(&mut self.sudo_occurred, path, owner) } fn resolve_symlink_destination( @@ -320,48 +286,7 @@ impl RealFilesystem { link: &Path, owner: &Option, ) -> Result> { - match real_path(link) { - Ok(path) => Ok(Some(path)), - Err(e) if e.kind() == ErrorKind::PermissionDenied && owner.is_some() => { - self.resolve_symlink_destination_as_root(link) - } - Err(e) if e.kind() == ErrorKind::NotFound => Ok(None), - Err(e) => Err(e).context("get real path of existing symlink"), - } - } - - fn resolve_symlink_destination_as_root(&mut self, path: &Path) -> Result> { - let output = self - .sudo(format!("resolving existing symlink {path:?} as root")) - .arg("sh") - .arg("-c") - .arg( - "if resolved=$(realpath -- \"$1\" 2>/dev/null); then printf 'resolved\\0%s' \"$resolved\"; elif [ -L \"$1\" ] && [ ! -e \"$1\" ]; then printf 'missing\\0'; else exit 1; fi", - ) - .arg("sh") - .arg(path) - .output() - .context("run sudo symlink resolution probe")?; - - anyhow::ensure!( - output.status.success(), - "sudo symlink resolution probe failed" - ); - parse_sudo_resolved_path(output.stdout) - } - - fn read_to_string_as_root(&mut self, path: &Path) -> Result { - let output = self - .sudo(format!("reading file {path:?} as root")) - .arg("cat") - .arg(path) - .output() - .context("run sudo cat")?; - - anyhow::ensure!(output.status.success(), "sudo cat failed"); - String::from_utf8(output.stdout) - .map_err(|e| io::Error::new(ErrorKind::InvalidData, e)) - .context("read from file as root") + resolve_symlink_destination_with_owner(&mut self.sudo_occurred, link, owner) } } @@ -439,13 +364,7 @@ impl Filesystem for RealFilesystem { } fn read_to_string(&mut self, path: &Path, owner: &Option) -> Result { - match fs::read_to_string(path) { - Ok(contents) => Ok(contents), - Err(e) if e.kind() == ErrorKind::PermissionDenied && owner.is_some() => { - self.read_to_string_as_root(path) - } - Err(e) => Err(e).context("read from file"), - } + read_to_string_with_owner(&mut self.sudo_occurred, path, owner) } fn write(&mut self, path: &Path, content: String) -> Result<()> { @@ -661,6 +580,8 @@ impl Filesystem for RealFilesystem { // == Dry run Filesystem == pub struct DryRunFilesystem { file_states: BTreeMap, + #[cfg(unix)] + sudo_occurred: bool, } #[derive(Debug, Clone, PartialEq)] @@ -676,15 +597,36 @@ impl DryRunFilesystem { pub fn new() -> DryRunFilesystem { DryRunFilesystem { file_states: BTreeMap::new(), + #[cfg(unix)] + sudo_occurred: false, } } - fn get_state(&mut self, path: &Path) -> Result { + fn get_state(&mut self, path: &Path, owner: &Option) -> Result { match self.file_states.get(path) { Some(state) => Ok(state.clone()), - None => get_file_state(path), + None => { + #[cfg(unix)] + { + get_file_state_with_owner(&mut self.sudo_occurred, path, owner) + } + #[cfg(not(unix))] + { + let _ = owner; + get_file_state(path) + } + } } } + + #[cfg(unix)] + fn resolve_symlink_destination( + &mut self, + link: &Path, + owner: &Option, + ) -> Result> { + resolve_symlink_destination_with_owner(&mut self.sudo_occurred, link, owner) + } } impl Filesystem for DryRunFilesystem { @@ -692,13 +634,13 @@ impl Filesystem for DryRunFilesystem { &mut self, source: &Path, link: &Path, - _owner: &Option, + owner: &Option, ) -> Result { let source_state = if let Some(state) = self.file_states.get(source) { debug!("Cached (probably not actual) source state: {:?}", state); state.clone() } else { - let state = get_file_state(source).context("get source state")?; + let state = self.get_state(source, &None).context("get source state")?; debug!("Source state: {:?}", state); state }; @@ -706,25 +648,61 @@ impl Filesystem for DryRunFilesystem { debug!("Cached (probably not actual) link state: {:?}", state); state.clone() } else { - let state = get_file_state(link).context("get link state")?; + let state = self.get_state(link, owner).context("get link state")?; debug!("Link state: {:?}", state); state }; - compare_symlink(source, source_state, link_state) + Ok(match (source_state, link_state) { + (FileState::Missing, FileState::SymbolicLink(_)) => SymlinkComparison::OnlyTargetExists, + (_, FileState::SymbolicLink(target)) => { + let expected_target = real_path(source).context("get real path of source")?; + let actual_target = if self.file_states.contains_key(link) { + if target == source { + Some(expected_target.clone()) + } else if target == expected_target { + Some(target) + } else { + None + } + } else { + #[cfg(unix)] + { + self.resolve_symlink_destination(link, owner) + .context("resolve symlink target")? + } + #[cfg(not(unix))] + { + resolve_symlink_destination(link).context("resolve symlink target")? + } + }; + + match actual_target { + Some(actual_target) if actual_target == expected_target => { + SymlinkComparison::Identical + } + Some(_) | None => SymlinkComparison::Changed, + } + } + (FileState::Missing, FileState::Missing) => SymlinkComparison::BothMissing, + (_, FileState::Missing) => SymlinkComparison::OnlySourceExists, + _ => SymlinkComparison::TargetNotSymlink, + }) } fn compare_template( &mut self, target: &Path, cache: &Path, - _owner: &Option, + owner: &Option, ) -> Result { let target_state = if let Some(state) = self.file_states.get(target) { debug!("Cached (probably not actual) target state: {:?}", state); state.clone() } else { - let state = get_file_state(target).context("get state of target")?; + let state = self + .get_state(target, owner) + .context("get state of target")?; debug!("Target state: {:?}", state); state }; @@ -732,7 +710,7 @@ impl Filesystem for DryRunFilesystem { debug!("Cached (probably not actual) cache state: {:?}", state); state.clone() } else { - let state = get_file_state(cache).context("get state of cache")?; + let state = self.get_state(cache, &None).context("get state of cache")?; debug!("Cache state: {:?}", state); state }; @@ -746,9 +724,9 @@ impl Filesystem for DryRunFilesystem { Ok(()) } - fn read_to_string(&mut self, path: &Path, _owner: &Option) -> Result { + fn read_to_string(&mut self, path: &Path, owner: &Option) -> Result { debug!("Reading contents of file {:?}", path); - match self.get_state(path).context("get file state")? { + match self.get_state(path, owner).context("get file state")? { FileState::File(s) => Ok(s.context("invalid utf-8 in template source")?), _ => anyhow::bail!("writing to non-file"), } @@ -794,10 +772,13 @@ impl Filesystem for DryRunFilesystem { "Copying file {:?} -> {:?} (target owned by {:?})", source, target, owner ); - match self.get_state(source).context("get state of source file")? { + match self + .get_state(source, &None) + .context("get state of source file")? + { FileState::File(content) => { if self - .get_state(target.parent().context("get parent of target")?) + .get_state(target.parent().context("get parent of target")?, &None) .context("get state of target's parent")? == FileState::Directory { @@ -835,6 +816,134 @@ impl Filesystem for DryRunFilesystem { // === Comparisons === +#[cfg(unix)] +fn sudo_command(sudo_occurred: &mut bool, goal: impl AsRef) -> Command { + if !*sudo_occurred { + warn!("Elevating permissions ({})", goal.as_ref()); + if !log_enabled!(log::Level::Debug) { + warn!( + "To see more than the first time elevated permissions are used, use verbosity 2 or more (-vv)" + ); + } + *sudo_occurred = true; + } else { + debug!("Elevating permissions ({})", goal.as_ref()); + } + Command::new("sudo") +} + +#[cfg(unix)] +fn get_file_state_with_owner( + sudo_occurred: &mut bool, + path: &Path, + owner: &Option, +) -> Result { + match try_get_file_state(path) { + Ok(state) => Ok(state), + Err(e) if e.kind() == ErrorKind::PermissionDenied && owner.is_some() => { + get_file_state_as_root(sudo_occurred, path) + } + Err(e) => Err(e).context("read contents of file that isn't symbolic or directory"), + } +} + +#[cfg(unix)] +fn get_file_state_as_root(sudo_occurred: &mut bool, path: &Path) -> Result { + let output = sudo_command(sudo_occurred, format!("detecting file state for {path:?} as root")) + .arg("sh") + .arg("-c") + .arg( + "if [ -L \"$1\" ]; then printf 'symlink\\0'; readlink -- \"$1\"; elif [ -d \"$1\" ]; then printf 'directory\\0'; elif [ -e \"$1\" ]; then printf 'file\\0'; cat -- \"$1\"; else printf 'missing\\0'; fi", + ) + .arg("sh") + .arg(path) + .output() + .context("run sudo file state probe")?; + + anyhow::ensure!(output.status.success(), "sudo file state probe failed"); + parse_sudo_file_state(output.stdout) +} + +#[cfg(not(unix))] +fn resolve_symlink_destination(path: &Path) -> Result> { + match real_path(path) { + Ok(path) => Ok(Some(path)), + Err(e) if e.kind() == ErrorKind::NotFound => Ok(None), + Err(e) => Err(e).context("get real path of existing symlink"), + } +} + +#[cfg(unix)] +fn resolve_symlink_destination_with_owner( + sudo_occurred: &mut bool, + path: &Path, + owner: &Option, +) -> Result> { + match real_path(path) { + Ok(path) => Ok(Some(path)), + Err(e) if e.kind() == ErrorKind::PermissionDenied && owner.is_some() => { + resolve_symlink_destination_as_root(sudo_occurred, path) + } + Err(e) if e.kind() == ErrorKind::NotFound => Ok(None), + Err(e) => Err(e).context("get real path of existing symlink"), + } +} + +#[cfg(unix)] +fn resolve_symlink_destination_as_root( + sudo_occurred: &mut bool, + path: &Path, +) -> Result> { + let output = sudo_command( + sudo_occurred, + format!("resolving existing symlink {path:?} as root"), + ) + .arg("sh") + .arg("-c") + .arg( + "if resolved=$(realpath -- \"$1\" 2>/dev/null); then printf 'resolved\\0%s' \"$resolved\"; elif [ -L \"$1\" ] && [ ! -e \"$1\" ]; then printf 'missing\\0'; else exit 1; fi", + ) + .arg("sh") + .arg(path) + .output() + .context("run sudo symlink resolution probe")?; + + anyhow::ensure!( + output.status.success(), + "sudo symlink resolution probe failed" + ); + parse_sudo_resolved_path(output.stdout) +} + +#[cfg(unix)] +fn read_to_string_with_owner( + sudo_occurred: &mut bool, + path: &Path, + owner: &Option, +) -> Result { + match fs::read_to_string(path) { + Ok(contents) => Ok(contents), + Err(e) if e.kind() == ErrorKind::PermissionDenied && owner.is_some() => { + read_to_string_as_root(sudo_occurred, path) + } + Err(e) => Err(e).context("read from file"), + } +} + +#[cfg(unix)] +fn read_to_string_as_root(sudo_occurred: &mut bool, path: &Path) -> Result { + let output = sudo_command(sudo_occurred, format!("reading file {path:?} as root")) + .arg("cat") + .arg(path) + .output() + .context("run sudo cat")?; + + anyhow::ensure!(output.status.success(), "sudo cat failed"); + String::from_utf8(output.stdout) + .map_err(|e| io::Error::new(ErrorKind::InvalidData, e)) + .context("read from file as root") +} + fn try_get_file_state(path: &Path) -> io::Result { if let Ok(target) = fs::read_link(path) { return Ok(FileState::SymbolicLink(target)); @@ -922,6 +1031,7 @@ impl std::fmt::Display for SymlinkComparison { } } +#[cfg(windows)] fn compare_symlink( source_path: &Path, source_state: FileState, @@ -1107,15 +1217,16 @@ mod test { fs.create_dir_all(&PathBuf::from("/home/user/.config"), &None) .unwrap(); assert_eq!( - fs.get_state(&PathBuf::from("/home")).unwrap(), + fs.get_state(&PathBuf::from("/home"), &None).unwrap(), FileState::Directory ); assert_eq!( - fs.get_state(&PathBuf::from("/home/user")).unwrap(), + fs.get_state(&PathBuf::from("/home/user"), &None).unwrap(), FileState::Directory ); assert_eq!( - fs.get_state(&PathBuf::from("/home/user/.config")).unwrap(), + fs.get_state(&PathBuf::from("/home/user/.config"), &None) + .unwrap(), FileState::Directory ); } @@ -1231,4 +1342,24 @@ mod test { assert_eq!(result.unwrap(), SymlinkComparison::Identical); } + + #[cfg(unix)] + #[test] + fn dry_run_compare_symlink_accepts_relative_target_to_same_file() { + let root = unique_test_path("dry-run-relative-symlink"); + let source_dir = root.join("source"); + let link_dir = root.join("links"); + std::fs::create_dir_all(&source_dir).unwrap(); + std::fs::create_dir_all(&link_dir).unwrap(); + + let source = source_dir.join("file.txt"); + let link = link_dir.join("file.txt"); + std::fs::write(&source, "hello").unwrap(); + std::os::unix::fs::symlink(PathBuf::from("../source/file.txt"), &link).unwrap(); + + let result = DryRunFilesystem::new().compare_symlink(&source, &link, &None); + std::fs::remove_dir_all(&root).unwrap(); + + assert_eq!(result.unwrap(), SymlinkComparison::Identical); + } } From 24435cff82c105c50689763f5fce8f3aa5d96d96 Mon Sep 17 00:00:00 2001 From: archnode Date: Thu, 7 May 2026 11:14:22 +0200 Subject: [PATCH 4/4] Add owner to cache and escalation on symlink delete. --- src/actions.rs | 27 +++++++----- src/config.rs | 106 +++++++++++++++++++++++++++++++++++++++++++++- src/deploy.rs | 106 +++++++++++++++++++++++++++++++++++++++++++--- src/filesystem.rs | 20 ++++++++- 4 files changed, 240 insertions(+), 19 deletions(-) diff --git a/src/actions.rs b/src/actions.rs index 138bc9d..a6ef22c 100644 --- a/src/actions.rs +++ b/src/actions.rs @@ -5,13 +5,13 @@ use anyhow::{Context, Result}; use crossterm::style::Stylize; use handlebars::Handlebars; -use crate::config::{SymbolicTarget, TemplateTarget, Variables}; +use crate::config::{CachedSymlinkTarget, SymbolicTarget, TemplateTarget, Variables}; use crate::difference::{self, diff_nonempty, generate_template_diff, print_diff}; use crate::filesystem::{Filesystem, SymlinkComparison, TemplateComparison}; #[cfg_attr(test, mockall::automock)] pub trait ActionRunner { - fn delete_symlink(&mut self, source: &Path, target: &Path) -> Result; + fn delete_symlink(&mut self, source: &Path, target: &CachedSymlinkTarget) -> Result; fn delete_template(&mut self, source: &Path, cache: &Path, target: &Path) -> Result; fn create_symlink(&mut self, source: &Path, target: &SymbolicTarget) -> Result; fn create_template( @@ -56,7 +56,7 @@ impl<'a> RealActionRunner<'a> { } impl ActionRunner for RealActionRunner<'_> { - fn delete_symlink(&mut self, source: &Path, target: &Path) -> Result { + fn delete_symlink(&mut self, source: &Path, target: &CachedSymlinkTarget) -> Result { delete_symlink(source, target, self.fs, self.force) } fn delete_template(&mut self, source: &Path, cache: &Path, target: &Path) -> Result { @@ -108,44 +108,49 @@ impl ActionRunner for RealActionRunner<'_> { /// Returns true if symlink should be deleted from cache pub fn delete_symlink( source: &Path, - target: &Path, + target: &CachedSymlinkTarget, fs: &mut dyn Filesystem, force: bool, ) -> Result { - info!("{} symlink {:?} -> {:?}", "[-]".red(), source, target); + info!( + "{} symlink {:?} -> {:?}", + "[-]".red(), + source, + target.target + ); let comparison = fs - .compare_symlink(source, target, &None) + .compare_symlink(source, &target.target, &target.owner) .context("detect symlink's current state")?; debug!("Current state: {}", comparison); match comparison { SymlinkComparison::Identical | SymlinkComparison::OnlyTargetExists => { debug!("Performing deletion"); - perform_symlink_target_deletion(fs, target) + perform_symlink_target_deletion(fs, &target.target) .context("perform symlink target deletion")?; Ok(true) } SymlinkComparison::OnlySourceExists | SymlinkComparison::BothMissing => { warn!( "Deleting symlink {:?} -> {:?} but target doesn't exist. Removing from cache anyways.", - source, target + source, target.target ); Ok(true) } SymlinkComparison::Changed | SymlinkComparison::TargetNotSymlink if force => { warn!( "Deleting symlink {:?} -> {:?} but {}. Forcing.", - source, target, comparison + source, target.target, comparison ); - perform_symlink_target_deletion(fs, target) + perform_symlink_target_deletion(fs, &target.target) .context("perform symlink target deletion")?; Ok(true) } SymlinkComparison::Changed | SymlinkComparison::TargetNotSymlink => { error!( "Deleting {:?} -> {:?} but {}. Skipping.", - source, target, comparison + source, target.target, comparison ); Ok(false) } diff --git a/src/config.rs b/src/config.rs index aef922b..5cdfef4 100644 --- a/src/config.rs +++ b/src/config.rs @@ -45,6 +45,27 @@ pub struct TemplateTarget { pub condition: Option, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] +#[serde(from = "CachedSymlinkTargetRepr", into = "CachedSymlinkTargetRepr")] +pub struct CachedSymlinkTarget { + pub target: PathBuf, + pub owner: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] +#[serde(untagged)] +enum CachedSymlinkTargetRepr { + Path(PathBuf), + Target(CachedSymlinkTargetInner), +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] +#[serde(deny_unknown_fields)] +struct CachedSymlinkTargetInner { + target: PathBuf, + owner: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] #[serde(from = "FileTargetOuterRepr", into = "FileTargetOuterRepr")] pub enum FileTarget { @@ -208,7 +229,9 @@ pub fn load_configuration( #[derive(Debug, Serialize, Deserialize, Default, Clone)] #[serde(deny_unknown_fields)] pub struct Cache { - pub symlinks: BTreeMap, + #[serde(default)] + pub symlinks: BTreeMap, + #[serde(default)] pub templates: BTreeMap, } @@ -501,6 +524,52 @@ impl> From for TemplateTarget { } } +impl From for CachedSymlinkTarget { + fn from(input: CachedSymlinkTargetRepr) -> Self { + match input { + CachedSymlinkTargetRepr::Path(target) => Self { + target, + owner: None, + }, + CachedSymlinkTargetRepr::Target(target) => Self { + target: target.target, + owner: target.owner, + }, + } + } +} + +impl From for CachedSymlinkTargetRepr { + fn from(input: CachedSymlinkTarget) -> Self { + if input.owner.is_none() { + Self::Path(input.target) + } else { + Self::Target(CachedSymlinkTargetInner { + target: input.target, + owner: input.owner, + }) + } + } +} + +impl> From for CachedSymlinkTarget { + fn from(input: T) -> Self { + Self { + target: input.into(), + owner: None, + } + } +} + +impl From<&SymbolicTarget> for CachedSymlinkTarget { + fn from(input: &SymbolicTarget) -> Self { + Self { + target: input.target.clone(), + owner: input.owner.clone(), + } + } +} + impl SymbolicTarget { pub fn into_template(self) -> TemplateTarget { TemplateTarget { @@ -852,4 +921,39 @@ mod test { &FileTarget::Symbolic(PathBuf::from("~/.SliverBodacious").into()) ); } + + #[test] + fn deserialize_legacy_symlink_cache_entry() { + let cache: Cache = toml::from_str( + r#" + [symlinks] + source = "target" + "#, + ) + .unwrap(); + + assert_eq!( + cache.symlinks.get(&PathBuf::from("source")), + Some(&CachedSymlinkTarget::from("target")) + ); + } + + #[test] + fn deserialize_symlink_cache_entry_with_owner() { + let cache: Cache = toml::from_str( + r#" + [symlinks] + source = { target = "target", owner = "root" } + "#, + ) + .unwrap(); + + assert_eq!( + cache.symlinks.get(&PathBuf::from("source")), + Some(&CachedSymlinkTarget { + target: PathBuf::from("target"), + owner: Some(UnixUser::Name("root".into())), + }) + ); + } } diff --git a/src/deploy.rs b/src/deploy.rs index 9180754..4a3c29f 100644 --- a/src/deploy.rs +++ b/src/deploy.rs @@ -7,7 +7,7 @@ use std::path::PathBuf; use crate::actions::{self, ActionRunner, RealActionRunner}; use crate::args::Options; -use crate::config::{self, Cache, FileTarget, SymbolicTarget, TemplateTarget}; +use crate::config::{self, Cache, CachedSymlinkTarget, FileTarget, SymbolicTarget, TemplateTarget}; use crate::display_error; use crate::filesystem::{self, Filesystem, load_file}; use crate::handlebars_helpers::create_new_handlebars; @@ -265,7 +265,7 @@ fn run_deploy( let existing_symlinks: BTreeSet<(PathBuf, PathBuf)> = cache .symlinks .iter() - .map(|(k, v)| (k.clone(), v.clone())) + .map(|(k, v)| (k.clone(), v.target.clone())) .collect(); let existing_templates: BTreeSet<(PathBuf, PathBuf)> = cache .templates @@ -288,8 +288,9 @@ fn run_deploy( for (source, target) in existing_symlinks.difference(&desired_symlinks.keys().cloned().collect()) { + let cached_target = cache.symlinks.get(source).unwrap(); execute_action( - runner.delete_symlink(source, target), + runner.delete_symlink(source, cached_target), || resulting_cache.symlinks.remove(source), || format!("delete symlink {source:?} -> {target:?}"), &mut suggest_force, @@ -323,7 +324,7 @@ fn run_deploy( || { resulting_cache .symlinks - .insert(source.clone(), target_path.clone()) + .insert(source.clone(), CachedSymlinkTarget::from(*target)) }, || format!("create symlink {source:?} -> {target_path:?}"), &mut suggest_force, @@ -361,7 +362,11 @@ fn run_deploy( .unwrap(); execute_action( runner.update_symlink(source, target), - || (), + || { + resulting_cache + .symlinks + .insert(source.clone(), CachedSymlinkTarget::from(*target)); + }, || format!("update symlink {source:?} -> {target_path:?}"), &mut suggest_force, &mut error_occurred, @@ -558,7 +563,10 @@ mod test { runner .expect_delete_symlink() .times(1) - .with(function(path_eq("a_in")), function(path_eq("a_out_old"))) + .with( + function(path_eq("a_in")), + eq(CachedSymlinkTarget::from("a_out_old")), + ) .in_sequence(&mut seq) .returning(|_, _| Ok(true)); runner @@ -588,6 +596,58 @@ mod test { assert_eq!(cache.templates.len(), 0); } + #[test] + fn high_level_update_symlink_refreshes_cached_owner() { + let a_out: SymbolicTarget = SymbolicTarget { + target: PathBuf::from("a_out"), + owner: Some(crate::config::UnixUser::Name("root".into())), + recurse: None, + condition: None, + }; + + let desired_symlinks = maplit::btreemap! { + PathBuf::from("a_in") => a_out.clone() + }; + + let mut runner = actions::MockActionRunner::new(); + let mut seq = mockall::Sequence::new(); + let mut cache = Cache { + symlinks: maplit::btreemap! { + PathBuf::from("a_in") => CachedSymlinkTarget::from("a_out") + }, + templates: BTreeMap::new(), + }; + + runner + .expect_update_symlink() + .times(1) + .with(function(path_eq("a_in")), eq(a_out)) + .in_sequence(&mut seq) + .returning(|_, _| Ok(true)); + + let (suggest_force, error_occurred) = run_deploy( + &mut runner, + &desired_symlinks, + &BTreeMap::new(), + &mut cache, + &Options { + cache_directory: "cache".into(), + force: false, + ..Options::default() + }, + ); + + assert!(!suggest_force); + assert!(!error_occurred); + assert_eq!( + cache.symlinks.get(&PathBuf::from("a_in")), + Some(&CachedSymlinkTarget { + target: PathBuf::from("a_out"), + owner: Some(crate::config::UnixUser::Name("root".into())), + }) + ); + } + #[test] fn high_level_change_type() { // Setup @@ -864,6 +924,40 @@ mod test { ); } + #[test] + fn low_level_delete_symlink_uses_cached_owner() { + let mut fs = crate::filesystem::MockFilesystem::new(); + let mut seq = mockall::Sequence::new(); + let target = CachedSymlinkTarget { + target: PathBuf::from("owned_out"), + owner: Some(crate::config::UnixUser::Name("root".into())), + }; + + fs.expect_compare_symlink() + .times(1) + .with( + function(path_eq("owned_in")), + function(path_eq("owned_out")), + eq(target.owner.clone()), + ) + .in_sequence(&mut seq) + .returning(|_, _, _| Ok(SymlinkComparison::Identical)); + fs.expect_remove_file() + .times(1) + .with(function(path_eq("owned_out"))) + .in_sequence(&mut seq) + .returning(|_| Ok(())); + fs.expect_delete_parents() + .times(1) + .with(function(path_eq("owned_out")), eq(false)) + .in_sequence(&mut seq) + .returning(|_, _| Ok(())); + + assert!( + actions::delete_symlink(&PathBuf::from("owned_in"), &target, &mut fs, false).unwrap() + ); + } + #[test] fn low_level_symlink_owner_passes_owner_to_compare() { let mut fs = crate::filesystem::MockFilesystem::new(); diff --git a/src/filesystem.rs b/src/filesystem.rs index 656987a..594d0d1 100644 --- a/src/filesystem.rs +++ b/src/filesystem.rs @@ -336,7 +336,25 @@ impl Filesystem for RealFilesystem { } fn remove_file(&mut self, path: &Path) -> Result<()> { - let metadata = path.symlink_metadata().context("get metadata")?; + let metadata = match path.symlink_metadata() { + Ok(metadata) => metadata, + Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { + let success = self + .sudo(format!("removing file {path:?} as root")) + .arg("rm") + .arg("-r") + .arg(path) + .spawn() + .context("spawn sudo rm command")? + .wait() + .context("wait for sudo rm command")? + .success(); + + anyhow::ensure!(success, "sudo rm command failed"); + return Ok(()); + } + Err(e) => return Err(e).context("get metadata"), + }; let result = if metadata.is_dir() { std::fs::remove_dir_all(path) } else {