From a3294721e8369e4ca9a7f3efcf3653c1e409d46f Mon Sep 17 00:00:00 2001 From: Vas Zayarskiy <7261268+Staphylococcus@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:17:29 +0300 Subject: [PATCH] fix(updates): support real host upgrade layouts --- crates/lg-buddy/src/updates.rs | 138 ++++++++++++++++++++++- crates/lg-buddy/src/upgrade_preflight.rs | 70 ++++++++++-- 2 files changed, 195 insertions(+), 13 deletions(-) diff --git a/crates/lg-buddy/src/updates.rs b/crates/lg-buddy/src/updates.rs index 5eef597..c4c2347 100644 --- a/crates/lg-buddy/src/updates.rs +++ b/crates/lg-buddy/src/updates.rs @@ -2,6 +2,8 @@ use std::error::Error; use std::fmt; use std::fs::{self, OpenOptions}; use std::io::{self, Read, Write}; +#[cfg(unix)] +use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt}; use std::path::{Path, PathBuf}; use std::process; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -1084,18 +1086,19 @@ impl UpdateCacheStore for FileUpdateCacheStore { fn atomic_write_file(path: &Path, contents: &[u8]) -> io::Result<()> { if let Some(parent) = path.parent() { if !parent.as_os_str().is_empty() { - fs::create_dir_all(parent)?; + ensure_cache_parent(parent)?; } } let mut last_error = None; for attempt in 0..100 { let temp_path = atomic_temp_path(path, attempt); - let mut file = match OpenOptions::new() - .write(true) - .create_new(true) - .open(&temp_path) - { + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + options.mode(0o600); + + let mut file = match options.open(&temp_path) { Ok(file) => file, Err(err) if err.kind() == io::ErrorKind::AlreadyExists => { last_error = Some(err); @@ -1128,6 +1131,49 @@ fn atomic_write_file(path: &Path, contents: &[u8]) -> io::Result<()> { })) } +#[cfg(unix)] +fn ensure_cache_parent(parent: &Path) -> io::Result<()> { + let mut current = PathBuf::new(); + for component in parent.components() { + current.push(component.as_os_str()); + match fs::symlink_metadata(¤t) { + Ok(metadata) if metadata.file_type().is_dir() => {} + Ok(_) => { + return Err(io::Error::new( + io::ErrorKind::NotADirectory, + format!( + "cache path component `{}` is not a directory", + current.display() + ), + )) + } + Err(err) if err.kind() == io::ErrorKind::NotFound => { + match fs::DirBuilder::new().mode(0o700).create(¤t) { + Ok(()) => {} + Err(err) if err.kind() == io::ErrorKind::AlreadyExists => {} + Err(err) => return Err(err), + } + if !fs::symlink_metadata(¤t)?.file_type().is_dir() { + return Err(io::Error::new( + io::ErrorKind::NotADirectory, + format!( + "cache path component `{}` is not a directory", + current.display() + ), + )); + } + } + Err(err) => return Err(err), + } + } + Ok(()) +} + +#[cfg(not(unix))] +fn ensure_cache_parent(parent: &Path) -> io::Result<()> { + fs::create_dir_all(parent) +} + fn atomic_temp_path(path: &Path, attempt: u8) -> PathBuf { let file_name = path .file_name() @@ -1520,6 +1566,8 @@ mod tests { use std::fs; use std::io::{self, Read, Write}; use std::net::TcpListener; + #[cfg(unix)] + use std::os::unix::fs::PermissionsExt; use std::path::PathBuf; use std::process; use std::sync::{ @@ -1970,6 +2018,27 @@ mod tests { LOCK.get_or_init(|| Mutex::new(())) } + #[cfg(unix)] + struct UmaskGuard { + previous: libc::mode_t, + } + + #[cfg(unix)] + impl UmaskGuard { + fn set(mask: libc::mode_t) -> Self { + Self { + previous: unsafe { libc::umask(mask) }, + } + } + } + + #[cfg(unix)] + impl Drop for UmaskGuard { + fn drop(&mut self) { + unsafe { libc::umask(self.previous) }; + } + } + #[test] fn cache_path_resolver_prefers_xdg_cache_home() { let xdg_cache_home = PathBuf::from("/tmp/xdg-cache"); @@ -2122,6 +2191,63 @@ mod tests { fs::remove_dir_all(dir).expect("remove test temp dir"); } + #[cfg(unix)] + #[test] + fn file_cache_creates_private_path_and_file_under_group_writable_umask() { + const CHILD_ENV: &str = "LG_BUDDY_TEST_CACHE_PERMISSIONS_CHILD"; + if std::env::var_os(CHILD_ENV).is_none() { + let status = std::process::Command::new( + std::env::current_exe().expect("resolve current test executable"), + ) + .arg("file_cache_creates_private_path_and_file_under_group_writable_umask") + .arg("--nocapture") + .env(CHILD_ENV, "1") + .status() + .expect("run isolated cache-permissions regression"); + assert!(status.success(), "isolated cache-permissions test failed"); + return; + } + + let _guard = env_lock() + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let dir = unique_temp_dir("cache-permissions"); + let home = dir.join("home"); + fs::create_dir(&home).expect("create test home"); + fs::set_permissions(&home, fs::Permissions::from_mode(0o750)) + .expect("set test home permissions"); + let path = home + .join(".cache") + .join("lg-buddy") + .join("update-check.json"); + + let _umask = UmaskGuard::set(0o002); + FileUpdateCacheStore::new(path.clone()) + .save(&UpdateCheckCache::default()) + .expect("save cache"); + + for directory in [home.join(".cache"), home.join(".cache").join("lg-buddy")] { + assert_eq!( + fs::symlink_metadata(directory) + .expect("cache directory metadata") + .permissions() + .mode() + & 0o777, + 0o700 + ); + } + assert_eq!( + fs::symlink_metadata(path) + .expect("cache file metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); + + fs::remove_dir_all(dir).expect("remove test temp dir"); + } + #[test] fn cache_without_notification_state_loads_with_absent_notification() { let cache: UpdateCheckCache = serde_json::from_str( diff --git a/crates/lg-buddy/src/upgrade_preflight.rs b/crates/lg-buddy/src/upgrade_preflight.rs index 48fc304..298bb2d 100644 --- a/crates/lg-buddy/src/upgrade_preflight.rs +++ b/crates/lg-buddy/src/upgrade_preflight.rs @@ -920,14 +920,14 @@ impl<'a, F: FilesystemFacts> Checker<'a, F> { ); } InstallerPathPolicy::MutateDirectory => { - self.check_mutable_directory(path, &facts, check, 0o300); + self.check_mutable_directory(path, &facts, owner_uid, check, 0o300); } InstallerPathPolicy::RecursiveClear => { - self.check_mutable_directory(path, &facts, check, 0o300); + self.check_mutable_directory(path, &facts, owner_uid, check, 0o300); self.check_recursive_clear_mounts(path, check); } InstallerPathPolicy::ExactDropInDirectory { expected_entry } => { - self.check_mutable_directory(path, &facts, check, 0o700); + self.check_mutable_directory(path, &facts, owner_uid, check, 0o700); self.check_exact_directory(path, expected_entry, check); } InstallerPathPolicy::ReadableInput => self.check_permissions( @@ -1016,6 +1016,7 @@ impl<'a, F: FilesystemFacts> Checker<'a, F> { &mut self, path: &Path, facts: &PathFacts, + owner_uid: u32, check: &'static str, required_permissions: u32, ) { @@ -1035,15 +1036,21 @@ impl<'a, F: FilesystemFacts> Checker<'a, F> { "replace the mounted path with an ordinary installation directory before upgrading", ); } + let required_permissions = if owner_uid == 0 && facts.owner_uid == 0 { + required_permissions & !0o200 + } else { + required_permissions + }; self.check_permissions( path, facts, required_permissions, check, - if required_permissions & 0o400 != 0 { - "directory is not readable, writable, and searchable by its owner" - } else { - "directory is not writable and searchable by its owner" + match required_permissions { + 0o100 => "directory is not searchable by its owner", + 0o500 => "directory is not readable and searchable by its owner", + 0o700 => "directory is not readable, writable, and searchable by its owner", + _ => "directory is not writable and searchable by its owner", }, ); } @@ -1938,6 +1945,55 @@ mod tests { ); } + #[test] + fn root_owned_mutation_directory_may_rely_on_privileged_write_access() { + let fixture = InstalledFixture::new("root-owned-read-only-mode"); + let directory = fixture.facts.layout.system_path("/usr/bin"); + + let root_owned = OverriddenFilesystem { + path: directory.clone(), + owner_uid: Some(0), + mode: Some(0o555), + read_only: None, + mount_point: None, + }; + let mut root_checker = Checker::new(&root_owned); + root_checker.check_requirement( + &directory, + 0, + None, + InstallerPathPolicy::MutateDirectory, + "policy-contract", + ); + assert!( + root_checker.report.compatible(), + "{}", + root_checker.report.render() + ); + + let user_owned = OverriddenFilesystem { + path: directory.clone(), + owner_uid: Some(fixture.facts.user_owner_uid), + mode: Some(0o555), + read_only: None, + mount_point: None, + }; + let mut user_checker = Checker::new(&user_owned); + user_checker.check_requirement( + &directory, + fixture.facts.user_owner_uid, + None, + InstallerPathPolicy::MutateDirectory, + "policy-contract", + ); + assert_failure( + &user_checker.report, + "policy-contract", + &directory, + "not writable and searchable", + ); + } + #[test] fn initial_preflight_refuses_read_only_installation_paths() { let fixture = InstalledFixture::new("read-only");