diff --git a/crates/soar-cli/src/main.rs b/crates/soar-cli/src/main.rs index 177ebf6b..d6b94c77 100644 --- a/crates/soar-cli/src/main.rs +++ b/crates/soar-cli/src/main.rs @@ -1,5 +1,7 @@ use std::{ - env, fs, + env, + ffi::OsString, + fs, io::Read, os::unix::fs::PermissionsExt as _, process::Command, @@ -21,8 +23,8 @@ use progress::{create_download_job, handle_download_progress, spawn_event_handle use remove::remove_packages; use run::run_package; use soar_config::config::{ - self, enable_system_mode, generate_default_config, get_config, set_current_profile, Config, - CONFIG_PATH, + self, enable_system_mode, generate_default_config, get_config, path_env_var, + set_current_profile, Config, CONFIG_PATH, PATH_ENV_SUFFIXES, }; use soar_core::{ error::{ErrorContext, SoarError}, @@ -209,7 +211,27 @@ fn handle_system_mode(command: &cli::Commands) -> SoarResult<()> { escalation_cmd ); - let status = Command::new(escalation_cmd) + // sudo and doas both reset the environment, so the system overrides are + // handed to the privileged process explicitly. Without this, a read-only + // `--system` command and a privileged one would resolve different trees. + let forwarded: Vec = PATH_ENV_SUFFIXES + .iter() + .filter_map(|suffix| { + let var = path_env_var(suffix, true); + env::var_os(&var).map(|value| { + let mut assignment = OsString::from(format!("{var}=")); + assignment.push(value); + assignment + }) + }) + .collect(); + + let mut command = Command::new(escalation_cmd); + if !forwarded.is_empty() { + command.arg("env").args(&forwarded); + } + + let status = command .arg(¤t_exe) .args(&args) .status() @@ -534,13 +556,14 @@ async fn handle_cli() -> SoarResult<()> { if utils::json_enabled() { json_output::emit(&paths); } else { - info!("SOAR_CONFIG={}", paths.config); - info!("SOAR_PACKAGES_CONFIG={}", paths.packages_config); - info!("SOAR_BIN={}", paths.bin); - info!("SOAR_DB={}", paths.db); - info!("SOAR_CACHE={}", paths.cache); - info!("SOAR_PACKAGES={}", paths.packages); - info!("SOAR_REPOSITORIES={}", paths.repositories); + let name = |suffix| path_env_var(suffix, config.is_system()); + info!("{}={}", name("CONFIG"), paths.config); + info!("{}={}", name("PACKAGES_CONFIG"), paths.packages_config); + info!("{}={}", name("BIN"), paths.bin); + info!("{}={}", name("DB"), paths.db); + info!("{}={}", name("CACHE"), paths.cache); + info!("{}={}", name("PACKAGES"), paths.packages); + info!("{}={}", name("REPOSITORIES"), paths.repositories); } } #[cfg(feature = "self")] diff --git a/crates/soar-config/src/config.rs b/crates/soar-config/src/config.rs index 67e3efd9..cce98a35 100644 --- a/crates/soar-config/src/config.rs +++ b/crates/soar-config/src/config.rs @@ -8,7 +8,8 @@ use std::{ use documented::{Documented, DocumentedFields}; use serde::{Deserialize, Serialize}; use soar_utils::{ - path::{is_safe_component, resolve_path, xdg_config_home, xdg_data_home}, + error::PathResult, + path::{is_safe_component, resolve_path_with, xdg_config_home, xdg_data_home}, system::platform, }; use toml_edit::DocumentMut; @@ -36,15 +37,15 @@ pub struct Config { pub repositories: Vec, /// Path to the local cache directory. - /// Default: $SOAR_ROOT/cache + /// Default: root_path/cache pub cache_path: Option, /// Path where the Soar package database is stored. - /// Default: $SOAR_ROOT/db + /// Default: root_path/db pub db_path: Option, /// Directory where binary symlinks are placed. - /// Default: $SOAR_ROOT/bin + /// Default: root_path/bin pub bin_path: Option, /// Directory where desktop files are stored. @@ -52,11 +53,11 @@ pub struct Config { pub desktop_path: Option, /// Path to the local clone of all repositories. - /// Default: $SOAR_ROOT/repos + /// Default: root_path/repos pub repositories_path: Option, /// Portable dirs path - /// Default: $SOAR_ROOT/portable-dirs + /// Default: root_path/portable-dirs pub portable_dirs: Option, /// If true, enables parallel downloading of packages. @@ -150,23 +151,24 @@ pub fn is_system_mode() -> bool { /// Enable system mode and set appropriate paths /// -/// Both config files move to the system location. An explicit `SOAR_CONFIG` or -/// `SOAR_PACKAGES_CONFIG` names the file to use whatever the mode, so it is -/// left alone. +/// Both config files move to the system location, discarding whatever the +/// user's `SOAR_CONFIG` and `SOAR_PACKAGES_CONFIG` named. `SOAR_SYSTEM_CONFIG` +/// and `SOAR_SYSTEM_PACKAGES_CONFIG` name the system files instead. pub fn enable_system_mode() { let mut system_mode = SYSTEM_MODE.write().unwrap(); *system_mode = true; drop(system_mode); - if std::env::var_os("SOAR_CONFIG").is_none() { - let mut config_path = CONFIG_PATH.write().unwrap(); - *config_path = PathBuf::from("/etc/soar/config.toml"); - } + let mut config_path = CONFIG_PATH.write().unwrap(); + *config_path = path_env("CONFIG", true) + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from("/etc/soar/config.toml")); + drop(config_path); - if std::env::var_os("SOAR_PACKAGES_CONFIG").is_none() { - let mut packages_config_path = crate::packages::PACKAGES_CONFIG_PATH.write().unwrap(); - *packages_config_path = PathBuf::from("/etc/soar/packages.toml"); - } + let mut packages_config_path = crate::packages::PACKAGES_CONFIG_PATH.write().unwrap(); + *packages_config_path = path_env("PACKAGES_CONFIG", true) + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from("/etc/soar/packages.toml")); } /// Get the system root path @@ -174,6 +176,68 @@ pub fn system_root() -> PathBuf { PathBuf::from("/opt/soar") } +/// Suffixes of the path override variables. +/// +/// User mode reads `SOAR_` and system mode reads +/// `SOAR_SYSTEM_`, so a user's exported variables never redirect the +/// system tree. +pub const PATH_ENV_SUFFIXES: &[&str] = &[ + "ROOT", + "BIN", + "DB", + "CACHE", + "PACKAGES", + "REPOSITORIES", + "PORTABLE_DIRS", + "DESKTOP", + "CONFIG", + "PACKAGES_CONFIG", +]; + +/// Name of the variable that overrides `suffix` in the given mode. +pub fn path_env_var(suffix: &str, system_mode: bool) -> String { + if system_mode { + format!("SOAR_SYSTEM_{suffix}") + } else { + format!("SOAR_{suffix}") + } +} + +/// Value of the path override for `suffix` in the given mode, if it is set. +pub fn path_env(suffix: &str, system_mode: bool) -> Option { + std::env::var(path_env_var(suffix, system_mode)).ok() +} + +/// Name the given mode reads in place of `var`. +/// +/// A `SOAR_*` reference inside a system config means the system variable, so +/// a system path never resolves through a user's environment. +pub fn mode_env_name(var: &str, system_mode: bool) -> String { + match var.strip_prefix("SOAR_") { + Some(suffix) if system_mode && !var.starts_with("SOAR_SYSTEM_") => { + format!("SOAR_SYSTEM_{suffix}") + } + _ => var.to_string(), + } +} + +/// Resolves a configured path, reading `SOAR_*` references from the variables +/// the given mode owns. +pub(crate) fn resolve_mode_path(path: &str, system_mode: bool) -> PathResult { + resolve_path_with(path, |var| mode_env_name(var, system_mode)) +} + +/// Root directory of the tree the given mode owns. +fn resolve_soar_root(system_mode: bool) -> String { + path_env("ROOT", system_mode).unwrap_or_else(|| { + if system_mode { + system_root().display().to_string() + } else { + format!("{}/soar", xdg_data_home().display()) + } + }) +} + pub fn init() -> Result<()> { let config = Config::new()?; let mut global_config = CONFIG.write().unwrap(); @@ -234,13 +298,8 @@ impl Config { #[allow(deprecated)] pub fn default_config>(selected_repos: &[T]) -> Self { trace!("creating default configuration"); - let soar_root = if is_system_mode() { - std::env::var("SOAR_ROOT").unwrap_or_else(|_| system_root().display().to_string()) - } else { - std::env::var("SOAR_ROOT") - .unwrap_or_else(|_| format!("{}/soar", xdg_data_home().display())) - }; - trace!(soar_root = soar_root, "resolved SOAR_ROOT"); + let soar_root = resolve_soar_root(is_system_mode()); + trace!(soar_root = soar_root, "resolved soar root"); let default_profile = Profile { root_path: soar_root.clone(), @@ -332,13 +391,8 @@ impl Config { "creating default configuration for system_mode={}", system_mode ); - let soar_root = if system_mode { - std::env::var("SOAR_ROOT").unwrap_or_else(|_| system_root().display().to_string()) - } else { - std::env::var("SOAR_ROOT") - .unwrap_or_else(|_| format!("{}/soar", xdg_data_home().display())) - }; - trace!(soar_root = soar_root, "resolved SOAR_ROOT"); + let soar_root = resolve_soar_root(system_mode); + trace!(soar_root = soar_root, "resolved soar root"); let default_profile = Profile { root_path: soar_root.clone(), @@ -520,13 +574,13 @@ impl Config { } pub fn get_bin_path(&self) -> Result { - if let Ok(env_path) = std::env::var("SOAR_BIN") { - return Ok(resolve_path(&env_path)?); + if let Some(env_path) = path_env("BIN", self.system_mode) { + return Ok(resolve_mode_path(&env_path, self.system_mode)?); } if let Some(bin_path) = &self.bin_path { - return Ok(resolve_path(bin_path)?); + return Ok(resolve_mode_path(bin_path, self.system_mode)?); } - self.default_profile()?.get_bin_path() + self.default_profile()?.get_bin_path(self.system_mode) } /// Shells whose completions should be linked. @@ -548,11 +602,11 @@ impl Config { } pub fn get_desktop_path(&self) -> Result { - if let Ok(env_path) = std::env::var("SOAR_DESKTOP") { - return Ok(resolve_path(&env_path)?); + if let Some(env_path) = path_env("DESKTOP", self.system_mode) { + return Ok(resolve_mode_path(&env_path, self.system_mode)?); } if let Some(desktop_path) = &self.desktop_path { - return Ok(resolve_path(desktop_path)?); + return Ok(resolve_mode_path(desktop_path, self.system_mode)?); } Ok(soar_utils::path::desktop_dir(self.system_mode)) } @@ -586,52 +640,55 @@ impl Config { } pub fn get_db_path(&self) -> Result { - if let Ok(env_path) = std::env::var("SOAR_DB") { - return Ok(resolve_path(&env_path)?); + if let Some(env_path) = path_env("DB", self.system_mode) { + return Ok(resolve_mode_path(&env_path, self.system_mode)?); } if let Some(soar_db) = &self.db_path { - return Ok(resolve_path(soar_db)?); + return Ok(resolve_mode_path(soar_db, self.system_mode)?); } - self.default_profile()?.get_db_path() + self.default_profile()?.get_db_path(self.system_mode) } pub fn get_packages_path(&self, profile_name: Option) -> Result { - if let Ok(env_path) = std::env::var("SOAR_PACKAGES") { - return Ok(resolve_path(&env_path)?); + if let Some(env_path) = path_env("PACKAGES", self.system_mode) { + return Ok(resolve_mode_path(&env_path, self.system_mode)?); } let profile_name = profile_name.unwrap_or_else(get_current_profile); - self.get_profile(&profile_name)?.get_packages_path() + self.get_profile(&profile_name)? + .get_packages_path(self.system_mode) } pub fn get_cache_path(&self) -> Result { - if let Ok(env_path) = std::env::var("SOAR_CACHE") { - return Ok(resolve_path(&env_path)?); + if let Some(env_path) = path_env("CACHE", self.system_mode) { + return Ok(resolve_mode_path(&env_path, self.system_mode)?); } if let Some(soar_cache) = &self.cache_path { - return Ok(resolve_path(soar_cache)?); + return Ok(resolve_mode_path(soar_cache, self.system_mode)?); } - self.get_profile(&get_current_profile())?.get_cache_path() + self.get_profile(&get_current_profile())? + .get_cache_path(self.system_mode) } pub fn get_repositories_path(&self) -> Result { - if let Ok(env_path) = std::env::var("SOAR_REPOSITORIES") { - return Ok(resolve_path(&env_path)?); + if let Some(env_path) = path_env("REPOSITORIES", self.system_mode) { + return Ok(resolve_mode_path(&env_path, self.system_mode)?); } if let Some(repositories_path) = &self.repositories_path { - return Ok(resolve_path(repositories_path)?); + return Ok(resolve_mode_path(repositories_path, self.system_mode)?); } - self.default_profile()?.get_repositories_path() + self.default_profile()? + .get_repositories_path(self.system_mode) } pub fn get_portable_dirs(&self) -> Result { - if let Ok(env_path) = std::env::var("SOAR_PORTABLE_DIRS") { - return Ok(resolve_path(&env_path)?); + if let Some(env_path) = path_env("PORTABLE_DIRS", self.system_mode) { + return Ok(resolve_mode_path(&env_path, self.system_mode)?); } if let Some(portable_dirs) = &self.portable_dirs { - return Ok(resolve_path(portable_dirs)?); + return Ok(resolve_mode_path(portable_dirs, self.system_mode)?); } - self.default_profile()?.get_portable_dirs() + self.default_profile()?.get_portable_dirs(self.system_mode) } pub fn get_repository(&self, repo_name: &str) -> Option<&Repository> { @@ -943,4 +1000,110 @@ mod tests { assert_eq!(bin_path, PathBuf::from("/custom/bin")); }); } + + #[test] + fn a_user_path_variable_does_not_reach_the_system_tree() { + let vars = vec![("SOAR_BIN", "/custom/bin"), ("SOAR_ROOT", "/custom/root")]; + with_env(vars, || { + let config = Config::default_config_for_mode::<&str>(&[], true); + assert_eq!(config.get_bin_path().unwrap(), system_root().join("bin")); + assert_eq!( + config.default_profile().unwrap().root_path, + system_root().display().to_string() + ); + }); + } + + #[test] + fn the_system_variables_override_the_system_tree() { + let vars = vec![ + ("SOAR_SYSTEM_ROOT", "/srv/soar"), + ("SOAR_SYSTEM_BIN", "/usr/local/bin"), + ]; + with_env(vars, || { + let config = Config::default_config_for_mode::<&str>(&[], true); + assert_eq!( + config.get_bin_path().unwrap(), + PathBuf::from("/usr/local/bin") + ); + assert_eq!(config.get_db_path().unwrap(), PathBuf::from("/srv/soar/db")); + }); + } + + /// Covers the profile fall-through: a config that declares only a root + /// leaves every path to `Profile`, which is where the mode used to come + /// from the global flag rather than the config. + #[test] + fn a_system_config_resolves_its_profile_root_without_the_user_environment() { + let dir = std::env::temp_dir().join(format!("soar-cfg-{}", std::process::id())); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("config.toml"); + fs::write( + &path, + "default_profile = \"default\"\nrepositories = []\n\n[profile.default]\nroot_path = \"/srv/soar\"\n", + ) + .unwrap(); + + with_env(vec![("SOAR_ROOT", "/home/me/leak")], || { + let config = Config::new_for_mode(&path, true).unwrap(); + assert_eq!(config.get_db_path().unwrap(), PathBuf::from("/srv/soar/db")); + assert_eq!( + config.get_bin_path().unwrap(), + PathBuf::from("/srv/soar/bin") + ); + assert_eq!( + config.get_packages_path(None).unwrap(), + PathBuf::from("/srv/soar/packages") + ); + }); + + with_env(vec![("SOAR_SYSTEM_ROOT", "/srv/other")], || { + let config = Config::new_for_mode(&path, true).unwrap(); + assert_eq!( + config.get_db_path().unwrap(), + PathBuf::from("/srv/other/db") + ); + }); + + with_env(vec![("SOAR_ROOT", "/home/me/mine")], || { + let config = Config::new_for_mode(&path, false).unwrap(); + assert_eq!( + config.get_db_path().unwrap(), + PathBuf::from("/home/me/mine/db") + ); + }); + + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn a_soar_reference_inside_a_config_value_follows_the_mode() { + let vars = vec![ + ("SOAR_ROOT", "/home/me/leak"), + ("SOAR_SYSTEM_ROOT", "/srv/soar"), + ]; + with_env(vars, || { + let mut system = Config::default_config_for_mode::<&str>(&[], true); + system.db_path = Some("$SOAR_ROOT/db".to_string()); + assert_eq!(system.get_db_path().unwrap(), PathBuf::from("/srv/soar/db")); + + let mut user = Config::default_config_for_mode::<&str>(&[], false); + user.db_path = Some("$SOAR_ROOT/db".to_string()); + assert_eq!( + user.get_db_path().unwrap(), + PathBuf::from("/home/me/leak/db") + ); + }); + } + + #[test] + fn the_system_variables_do_not_reach_the_user_tree() { + with_env(vec![("SOAR_SYSTEM_BIN", "/usr/local/bin")], || { + let config = Config::default_config_for_mode::<&str>(&[], false); + assert_ne!( + config.get_bin_path().unwrap(), + PathBuf::from("/usr/local/bin") + ); + }); + } } diff --git a/crates/soar-config/src/profile.rs b/crates/soar-config/src/profile.rs index ce0e08b5..d1d78828 100644 --- a/crates/soar-config/src/profile.rs +++ b/crates/soar-config/src/profile.rs @@ -2,9 +2,11 @@ use std::path::PathBuf; use documented::{Documented, DocumentedFields}; use serde::{Deserialize, Serialize}; -use soar_utils::path::resolve_path; -use crate::error::Result; +use crate::{ + config::{path_env, resolve_mode_path}, + error::Result, +}; /// A profile defines a local package store and its configuration. #[derive(Clone, Deserialize, Serialize, Documented, DocumentedFields)] @@ -21,39 +23,45 @@ pub struct Profile { } impl Profile { - pub(crate) fn get_bin_path(&self) -> Result { - Ok(self.get_root_path()?.join("bin")) + pub(crate) fn get_bin_path(&self, system_mode: bool) -> Result { + Ok(self.get_root_path(system_mode)?.join("bin")) } - pub(crate) fn get_db_path(&self) -> Result { - Ok(self.get_root_path()?.join("db")) + pub(crate) fn get_db_path(&self, system_mode: bool) -> Result { + Ok(self.get_root_path(system_mode)?.join("db")) } - pub fn get_packages_path(&self) -> Result { + /// Directory holding this profile's packages. + pub fn get_packages_path(&self, system_mode: bool) -> Result { if let Some(ref packages_path) = self.packages_path { - Ok(resolve_path(packages_path)?) + Ok(resolve_mode_path(packages_path, system_mode)?) } else { - Ok(self.get_root_path()?.join("packages")) + Ok(self.get_root_path(system_mode)?.join("packages")) } } - pub fn get_cache_path(&self) -> Result { - Ok(self.get_root_path()?.join("cache")) + /// Directory holding this profile's download cache. + pub fn get_cache_path(&self, system_mode: bool) -> Result { + Ok(self.get_root_path(system_mode)?.join("cache")) } - pub(crate) fn get_repositories_path(&self) -> Result { - Ok(self.get_root_path()?.join("repos")) + pub(crate) fn get_repositories_path(&self, system_mode: bool) -> Result { + Ok(self.get_root_path(system_mode)?.join("repos")) } - pub(crate) fn get_portable_dirs(&self) -> Result { - Ok(self.get_root_path()?.join("portable-dirs")) + pub(crate) fn get_portable_dirs(&self, system_mode: bool) -> Result { + Ok(self.get_root_path(system_mode)?.join("portable-dirs")) } - pub fn get_root_path(&self) -> Result { - if let Ok(env_path) = std::env::var("SOAR_ROOT") { - return Ok(resolve_path(&env_path)?); + /// Root of this profile's tree. + /// + /// The mode is a parameter rather than the global flag so a config built + /// for one mode never resolves through the other mode's variables. + pub fn get_root_path(&self, system_mode: bool) -> Result { + if let Some(env_path) = path_env("ROOT", system_mode) { + return Ok(resolve_mode_path(&env_path, system_mode)?); } - Ok(resolve_path(&self.root_path)?) + Ok(resolve_mode_path(&self.root_path, system_mode)?) } } @@ -80,7 +88,7 @@ mod tests { packages_path: Some("/custom/packages".to_string()), }; - let path = profile.get_packages_path().unwrap(); + let path = profile.get_packages_path(false).unwrap(); assert!(path.ends_with("packages")); } @@ -91,7 +99,7 @@ mod tests { packages_path: None, }; - let path = profile.get_packages_path().unwrap(); + let path = profile.get_packages_path(false).unwrap(); assert!(path.ends_with("packages")); } @@ -103,7 +111,7 @@ mod tests { packages_path: None, }; - let path = profile.get_root_path().unwrap(); + let path = profile.get_root_path(false).unwrap(); assert_eq!(path, PathBuf::from("/custom/root")); }); } diff --git a/crates/soar-config/src/test_utils.rs b/crates/soar-config/src/test_utils.rs index 900a4f2a..9f54e9f4 100644 --- a/crates/soar-config/src/test_utils.rs +++ b/crates/soar-config/src/test_utils.rs @@ -1,8 +1,14 @@ +/// Serializes the tests that mutate the process environment. +#[cfg(test)] +static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + #[cfg(test)] pub fn with_env(vars: Vec<(&str, &str)>, f: F) where F: FnOnce(), { + let _guard = ENV_LOCK.lock().unwrap_or_else(|err| err.into_inner()); + let old_vars: Vec<_> = vars .iter() .map(|(k, _)| (*k, std::env::var(k).ok())) diff --git a/crates/soar-core/src/utils.rs b/crates/soar-core/src/utils.rs index 237543d8..d05e597a 100644 --- a/crates/soar-core/src/utils.rs +++ b/crates/soar-core/src/utils.rs @@ -33,7 +33,7 @@ pub fn setup_required_paths() -> Result<()> { } for profile in config.profile.values() { - let packages_path = profile.get_packages_path()?; + let packages_path = profile.get_packages_path(config.is_system())?; if !packages_path.exists() { fs::create_dir_all(&packages_path).with_context(|| { format!("creating packages directory {}", packages_path.display()) diff --git a/crates/soar-utils/src/path.rs b/crates/soar-utils/src/path.rs index 4173e264..459c61b8 100644 --- a/crates/soar-utils/src/path.rs +++ b/crates/soar-utils/src/path.rs @@ -63,13 +63,22 @@ pub fn is_safe_component(name: &str) -> bool { /// } /// ``` pub fn resolve_path(path: &str) -> PathResult { + resolve_path_with(path, |var| var.to_string()) +} + +/// Resolves a path, mapping every environment variable name through `rename` +/// before it is looked up. +/// +/// Callers that own a namespaced set of variables use this to redirect the +/// references a path contains without rewriting the path itself. +pub fn resolve_path_with(path: &str, rename: impl Fn(&str) -> String) -> PathResult { let path = path.trim(); if path.is_empty() { return Err(PathError::Empty); } - let resolved = expand_variables(path)?; + let resolved = expand_variables(path, &rename)?; let path_buf = PathBuf::from(resolved); if path_buf.is_absolute() { @@ -179,7 +188,7 @@ pub fn icons_dir(system: bool) -> PathBuf { } } -fn expand_variables(path: &str) -> PathResult { +fn expand_variables(path: &str, rename: &impl Fn(&str) -> String) -> PathResult { let mut result = String::with_capacity(path.len()); let mut chars = path.chars().peekable(); @@ -189,13 +198,13 @@ fn expand_variables(path: &str) -> PathResult { if chars.peek() == Some(&'{') { chars.next(); let var_name = consume_until(&mut chars, '}')?; - expand_env_var(&var_name, &mut result, path)?; + expand_env_var(&var_name, &mut result, path, rename)?; } else { let var_name = consume_var_name(&mut chars); if var_name.is_empty() { result.push('$'); } else { - expand_env_var(&var_name, &mut result, path)?; + expand_env_var(&var_name, &mut result, path, rename)?; } } } @@ -239,17 +248,23 @@ fn consume_var_name(chars: &mut std::iter::Peekable) -> String var_name } -fn expand_env_var(var_name: &str, result: &mut String, original: &str) -> PathResult<()> { - match var_name { +fn expand_env_var( + var_name: &str, + result: &mut String, + original: &str, + rename: &impl Fn(&str) -> String, +) -> PathResult<()> { + let var_name = rename(var_name); + match var_name.as_str() { "HOME" => result.push_str(&home_dir().to_string_lossy()), "XDG_CONFIG_HOME" => result.push_str(&xdg_config_home().to_string_lossy()), "XDG_DATA_HOME" => result.push_str(&xdg_data_home().to_string_lossy()), "XDG_CACHE_HOME" => result.push_str(&xdg_cache_home().to_string_lossy()), _ => { - let value = env::var(var_name).map_err(|_| { + let value = env::var(&var_name).map_err(|_| { PathError::MissingEnvVar { input: original.into(), - var: var_name.into(), + var: var_name.clone(), } })?; result.push_str(&value); @@ -260,6 +275,10 @@ fn expand_env_var(var_name: &str, result: &mut String, original: &str) -> PathRe #[cfg(test)] mod tests { + fn no_rename(var: &str) -> String { + var.to_string() + } + #[test] fn test_is_safe_component() { assert!(super::is_safe_component("clipcat")); @@ -285,7 +304,7 @@ mod tests { fn test_expand_variables_simple() { env::set_var("TEST_VAR", "test_value"); - let result = expand_variables("$TEST_VAR/path").unwrap(); + let result = expand_variables("$TEST_VAR/path", &no_rename).unwrap(); assert_eq!(result, "test_value/path"); env::remove_var("TEST_VAR"); @@ -295,7 +314,7 @@ mod tests { fn test_expand_variables_braces() { env::set_var("TEST_VAR_BRACES", "test_value"); - let result = expand_variables("${TEST_VAR_BRACES}/path").unwrap(); + let result = expand_variables("${TEST_VAR_BRACES}/path", &no_rename).unwrap(); assert_eq!(result, "test_value/path"); env::remove_var("TEST_VAR_BRACES"); @@ -305,7 +324,7 @@ mod tests { fn test_expand_variables_missing_braces() { env::set_var("TEST_VAR_MISSING_BRACES", "test_value"); - let result = expand_variables("${TEST_VAR_MISSING_BRACES"); + let result = expand_variables("${TEST_VAR_MISSING_BRACES", &no_rename); assert!(result.is_err()); env::remove_var("TEST_VAR_MISSING_BRACES"); @@ -313,7 +332,7 @@ mod tests { #[test] fn test_expand_variables_missing_var() { - let result = expand_variables("$THIS_VAR_DOESNT_EXIST"); + let result = expand_variables("$THIS_VAR_DOESNT_EXIST", &no_rename); assert!(result.is_err()); } @@ -363,6 +382,34 @@ mod tests { env::remove_var("HOME"); } + #[test] + #[serial] + fn resolve_path_with_renames_the_variable_it_reads() { + env::set_var("SOAR_RENAMED_ROOT", "/srv/soar"); + env::set_var("SOAR_ROOT", "/home/me"); + + let rename = |var: &str| format!("SOAR_RENAMED_{}", &var[5..]); + + assert_eq!( + resolve_path_with("$SOAR_ROOT/db", rename).unwrap(), + PathBuf::from("/srv/soar/db") + ); + assert_eq!( + resolve_path_with("${SOAR_ROOT}/db", rename).unwrap(), + PathBuf::from("/srv/soar/db") + ); + assert_eq!( + resolve_path("$SOAR_ROOT/db").unwrap(), + PathBuf::from("/home/me/db") + ); + + let missing = resolve_path_with("$SOAR_ROOT/db", |_| "SOAR_ABSENT".to_string()); + assert!(missing.is_err()); + + env::remove_var("SOAR_RENAMED_ROOT"); + env::remove_var("SOAR_ROOT"); + } + #[test] #[serial] fn test_resolve_path() { @@ -421,29 +468,32 @@ mod tests { env::set_var("HOME", "/tmp/home"); // Dollar at the end - assert_eq!(expand_variables("path/$").unwrap(), "path/$"); + assert_eq!(expand_variables("path/$", &no_rename).unwrap(), "path/$"); // Dollar with invalid char assert_eq!( - expand_variables("path/$!invalid").unwrap(), + expand_variables("path/$!invalid", &no_rename).unwrap(), "path/$!invalid" ); // Multiple variables env::set_var("VAR1", "val1"); env::set_var("VAR2", "val2"); - assert_eq!(expand_variables("$VAR1/${VAR2}").unwrap(), "val1/val2"); + assert_eq!( + expand_variables("$VAR1/${VAR2}", &no_rename).unwrap(), + "val1/val2" + ); env::remove_var("VAR1"); env::remove_var("VAR2"); // Tilde expansion let home_str = home_dir().to_string_lossy().to_string(); assert_eq!( - expand_variables("~/path").unwrap(), + expand_variables("~/path", &no_rename).unwrap(), format!("{}/path", home_str) ); - assert_eq!(expand_variables("~").unwrap(), home_str); - assert_eq!(expand_variables("a/~/b").unwrap(), "a/~/b"); + assert_eq!(expand_variables("~", &no_rename).unwrap(), home_str); + assert_eq!(expand_variables("a/~/b", &no_rename).unwrap(), "a/~/b"); env::remove_var("HOME"); } @@ -474,19 +524,25 @@ mod tests { env::remove_var("XDG_CACHE_HOME"); let mut result = String::new(); - expand_env_var("HOME", &mut result, "$HOME").unwrap(); + expand_env_var("HOME", &mut result, "$HOME", &no_rename).unwrap(); assert_eq!(result, "/tmp/home"); result.clear(); - expand_env_var("XDG_CONFIG_HOME", &mut result, "$XDG_CONFIG_HOME").unwrap(); + expand_env_var( + "XDG_CONFIG_HOME", + &mut result, + "$XDG_CONFIG_HOME", + &no_rename, + ) + .unwrap(); assert_eq!(result, "/tmp/home/.config"); result.clear(); - expand_env_var("XDG_DATA_HOME", &mut result, "$XDG_DATA_HOME").unwrap(); + expand_env_var("XDG_DATA_HOME", &mut result, "$XDG_DATA_HOME", &no_rename).unwrap(); assert_eq!(result, "/tmp/home/.local/share"); result.clear(); - expand_env_var("XDG_CACHE_HOME", &mut result, "$XDG_CACHE_HOME").unwrap(); + expand_env_var("XDG_CACHE_HOME", &mut result, "$XDG_CACHE_HOME", &no_rename).unwrap(); assert_eq!(result, "/tmp/home/.cache"); env::remove_var("HOME"); diff --git a/docs/cli-reference.md b/docs/cli-reference.md index c36328ac..ae960227 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -209,6 +209,48 @@ sudo soar --system install docker | `SOAR_NIGHTLY` | Force nightly update channel (self update) | `export SOAR_NIGHTLY=1` | | `SOAR_RELEASE` | Force stable update channel (self update) | `export SOAR_RELEASE=1` | +### System mode + +With `--system`, Soar reads the `SOAR_SYSTEM_`-prefixed variant of every path +variable above and ignores the unprefixed one, so an exported `SOAR_ROOT` never +redirects the system tree into your home: + +| User mode | System mode | Default in system mode | +|-----------|-------------|------------------------| +| `SOAR_CONFIG` | `SOAR_SYSTEM_CONFIG` | `/etc/soar/config.toml` | +| `SOAR_PACKAGES_CONFIG` | `SOAR_SYSTEM_PACKAGES_CONFIG` | `/etc/soar/packages.toml` | +| `SOAR_ROOT` | `SOAR_SYSTEM_ROOT` | `/opt/soar` | +| `SOAR_BIN` | `SOAR_SYSTEM_BIN` | `/opt/soar/bin` | +| `SOAR_DB` | `SOAR_SYSTEM_DB` | `/opt/soar/db` | +| `SOAR_CACHE` | `SOAR_SYSTEM_CACHE` | `/opt/soar/cache` | +| `SOAR_PACKAGES` | `SOAR_SYSTEM_PACKAGES` | `/opt/soar/packages` | +| `SOAR_REPOSITORIES` | `SOAR_SYSTEM_REPOSITORIES` | `/opt/soar/repos` | +| `SOAR_PORTABLE_DIRS` | `SOAR_SYSTEM_PORTABLE_DIRS` | `/opt/soar/portable-dirs` | +| `SOAR_DESKTOP` | `SOAR_SYSTEM_DESKTOP` | `/usr/local/share/applications` | + +A `$SOAR_*` reference inside a system config file follows the same rule, so +`db_path = "$SOAR_ROOT/db"` in `/etc/soar/config.toml` reads `SOAR_SYSTEM_ROOT`. + +The `SOAR_SYSTEM_*` values are forwarded across the `sudo` or `doas` escalation, +so a read-only `--system` command and a privileged one always resolve to the +same tree. Forwarding makes the escalation `sudo env VAR=... soar ...`, which a +sudoers rule that whitelists the `soar` binary by path will reject. + +Do not whitelist `/usr/bin/env` to work around that. A sudoers rule permitting +`env` grants arbitrary root, because `sudo env /bin/sh` then becomes a root +shell. Use one of these instead: + +- Leave the `SOAR_SYSTEM_*` variables unset in the calling shell and put the + system paths in `/etc/soar/config.toml`. The config file needs no forwarding, + and with nothing to forward Soar invokes the binary directly. +- Become root first, with `sudo -i` or equivalent, and run `soar --system` from + that shell. No escalation happens, so the root shell's own environment is + read directly. + +Sudoers environment settings such as `env_keep` do not help here. The wrapper is +added whenever a `SOAR_SYSTEM_*` variable is set in the calling shell, before +sudo is involved at all. + ## See Also - [Configuration](./configuration.md) for the configuration file reference diff --git a/docs/configuration.md b/docs/configuration.md index a0ea7dad..918aac7a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -192,6 +192,12 @@ soar -c /path/to/config.toml [subcommand] Environment variables take precedence over configuration file settings and profile paths. ::: +::: warning +System mode (`--system`) reads the `SOAR_SYSTEM_`-prefixed variant of each path +variable, such as `SOAR_SYSTEM_ROOT` and `SOAR_SYSTEM_CONFIG`, and ignores the +unprefixed ones. See the [CLI reference](./cli-reference.md#system-mode). +::: + ## Common Issues ### Invalid TOML Syntax diff --git a/docs/declarative.md b/docs/declarative.md index b68711e4..3932a876 100644 --- a/docs/declarative.md +++ b/docs/declarative.md @@ -714,6 +714,10 @@ export SOAR_PACKAGES_CONFIG=/path/to/my-packages.toml soar apply ``` +In system mode the variable is `SOAR_SYSTEM_PACKAGES_CONFIG`, defaulting to +`/etc/soar/packages.toml`. `SOAR_PACKAGES_CONFIG` is ignored there, so a user's +file never becomes the machine's file by accident. + ## Best Practices - **Version Pinning**: Pin versions for production tools, use `*` for development diff --git a/docs/health.md b/docs/health.md index d4dfc06a..e9491c3c 100644 --- a/docs/health.md +++ b/docs/health.md @@ -109,6 +109,9 @@ This command displays: - `SOAR_PACKAGES`: packages directory path. - `SOAR_REPOSITORIES`: repository directory path. +In system mode the same command reports the `SOAR_SYSTEM_`-prefixed names, which +are the ones that mode reads. + These environment variables can be set to override Soar's default paths and behavior. For example: diff --git a/docs/profiles.md b/docs/profiles.md index 3a30b417..0e711fd6 100644 --- a/docs/profiles.md +++ b/docs/profiles.md @@ -19,7 +19,7 @@ root_path = "/path/to/profile/root" packages_path = "/path/to/packages" # Optional ``` -- **`root_path`** (required): Root directory for the profile. Defaults to `~/.local/share/soar` or `$SOAR_ROOT/soar` if not in system mode. +- **`root_path`** (required): Root directory for the profile. Defaults to `$XDG_DATA_HOME/soar`, or `/opt/soar` in system mode. - **`packages_path`** (optional): Custom location for package storage. If not set, defaults to `/packages`. ### Path Resolution Priority @@ -98,6 +98,10 @@ The `SOAR_ROOT` environment variable overrides the `root_path` setting for any p SOAR_ROOT=/tmp/test-soar soar install neovim ``` +In system mode the variable is `SOAR_SYSTEM_ROOT`. `SOAR_ROOT` is ignored there, +including inside a config value such as `db_path = "$SOAR_ROOT/db"`, which a +system config reads as `SOAR_SYSTEM_ROOT`. + ### System Mode Use the `--system` flag (`-S`) to operate in system-wide mode. This changes the config location to `/etc/soar/config.toml` and typically requires root privileges: @@ -163,4 +167,4 @@ Profiles provide simple, file-based environment isolation: - **Path Priority**: Environment variables > global config overrides > profile-computed paths - **Computed Paths**: bin, db, cache, repos, portable-dirs are automatically derived from `root_path` unless overridden - **System Mode**: Use `--system` flag for system-wide installations -- **Environment Override**: `SOAR_ROOT` overrides profile `root_path` at runtime +- **Environment Override**: `SOAR_ROOT` overrides profile `root_path` at runtime, and `SOAR_SYSTEM_ROOT` does the same in system mode