fix: harden variant switching, symlink cleanup and path checks

crates/soar-cli/src/health.rs

@@ -99,6 +99,19 @@ pub async fn display_health(ctx: &SoarContext) -> SoarResult<()> {
     Ok(())
 }
 
+pub fn remove_broken_symlinks(ctx: &SoarContext) -> SoarResult<()> {
+    let removed = health::remove_broken_symlinks(ctx)?;
+
+    if removed.is_empty() {
+        info!("No broken symlinks found.");
+    }
+    for path in &removed {
+        info!("Removed broken symlink: {}", path.display());
+    }
+
+    Ok(())
+}
+
 pub async fn remove_broken_packages(ctx: &SoarContext) -> SoarResult<()> {
     let report = health::remove_broken_packages(ctx).await?;
 

crates/soar-cli/src/main.rs

@@ -13,7 +13,7 @@ use apply::apply_packages;
 use clap::{CommandFactory, Parser};
 use cli::Args;
 use download::{DownloadContext, create_regex_patterns, download};
-use health::{display_health, remove_broken_packages};
+use health::{display_health, remove_broken_packages, remove_broken_symlinks};
 use indicatif::ProgressBar;
 use install::install_packages;
 use json2db::json_to_db;
@@ -29,7 +29,7 @@ use soar_config::config::{
 use soar_core::{
     SoarResult,
     error::{ErrorContext, SoarError},
-    utils::{cleanup_cache, remove_broken_symlinks, setup_required_paths},
+    utils::{cleanup_cache, setup_required_paths},
 };
 use soar_dl::http_client::configure_http_client;
 use soar_events::EventSinkHandle;
@@ -673,7 +673,7 @@ async fn handle_cli() -> SoarResult<()> {
                         cleanup_cache()?;
                     }
                     if unspecified || broken_symlinks {
-                        remove_broken_symlinks()?;
+                        remove_broken_symlinks(&ctx)?;
                     }
                     if unspecified || broken {
                         remove_broken_packages(&ctx).await?;

crates/soar-cli/src/use.rs

@@ -50,12 +50,13 @@ pub async fn use_alternate_package(ctx: &SoarContext, name: &str) -> SoarResult<
         );
     }
 
-    if variants.len() == 1 {
-        return Ok(());
-    }
-
-    let selection = get_valid_selection(variants.len())?;
-    switch::switch_variant(ctx, name, selection).await?;
+    // A sole variant can still be unlinked, and switching to it links it again.
+    let selection = match variants.as_slice() {
+        [only] if only.is_active => return Ok(()),
+        [_] => 0,
+        _ => get_valid_selection(variants.len())?,
+    };
+    switch::switch_variant(ctx, variants[selection].package.id).await?;
 
     info!("Switched to {}", variants[selection].package.pkg_name);
 

crates/soar-core/src/package/install.rs

@@ -3,7 +3,7 @@ use std::{
     env, fs,
     io::{Read, Write},
     os::unix::fs::PermissionsExt,
-    path::{Path, PathBuf},
+    path::{Component, Path, PathBuf},
     process::Command,
     thread::sleep,
     time::Duration,
@@ -316,7 +316,7 @@ fn is_safe_relative(path: &str) -> bool {
         && !Path::new(path).is_absolute()
         && Path::new(path)
             .components()
-            .all(|c| matches!(c, std::path::Component::Normal(_)))
+            .all(|c| matches!(c, Component::Normal(_)))
 }
 
 /// Every regular file under `dir`, skipping the staging directory.
@@ -428,16 +428,17 @@ fn is_elf(path: &Path) -> bool {
 }
 
 /// Early validation of relative paths before download.
-/// Rejects paths containing `..` or absolute paths.
+/// Rejects absolute paths and paths with a `..` component.
 fn validate_relative_path(relative_path: &str, path_type: &str) -> SoarResult<()> {
-    if Path::new(relative_path).is_absolute() {
+    let path = Path::new(relative_path);
+    if path.is_absolute() {
         return Err(SoarError::Custom(format!(
             "{} '{}' must be a relative path, not absolute",
             path_type, relative_path
         )));
     }
 
-    if relative_path.contains("..") {
+    if path.components().any(|c| c == Component::ParentDir) {
         return Err(SoarError::Custom(format!(
             "{} '{}' contains path traversal components",
             path_type, relative_path
@@ -1613,7 +1614,7 @@ mod tests {
 
     use super::{
         Package, apply_file_layout, calculate_checksum, clear_for_promotion, install_extras,
-        relative_to, resolve_source,
+        relative_to, resolve_source, validate_relative_path,
     };
 
     /// Runs `install_extras`, which never waits on anything, to completion.
@@ -1679,6 +1680,19 @@ mod tests {
         );
     }
 
+    #[test]
+    fn extract_paths_reject_only_parent_components() {
+        for ok in ["foo..bar", "a/..b", "..a/b", "./bin", "a/b"] {
+            assert!(validate_relative_path(ok, "extract_root").is_ok(), "{ok}");
+        }
+        for bad in ["..", "../x", "a/../b", "a/..", "/abs"] {
+            assert!(
+                validate_relative_path(bad, "extract_root").is_err(),
+                "{bad}"
+            );
+        }
+    }
+
     #[test]
     fn alias_in_another_directory_climbs_out() {
         assert_eq!(

crates/soar-core/src/package/url.rs

@@ -49,8 +49,7 @@ impl UrlPackage {
     /// - `ghcr.io/org/repo@sha256:digest`
     /// - `ghcr.io/org/repo` (implies :latest)
     pub fn is_ghcr(input: &str) -> bool {
-        let input = input.trim().to_lowercase();
-        input.starts_with("ghcr.io/")
+        strip_ghcr_prefix(input.trim()).is_some()
     }
 
     /// Check if input is either a URL or GHCR reference.
@@ -128,17 +127,12 @@ impl UrlPackage {
     ) -> SoarResult<Self> {
         let reference = reference.trim();
 
-        if !Self::is_ghcr(reference) {
+        let Some(path) = strip_ghcr_prefix(reference) else {
             return Err(SoarError::Custom(format!(
                 "Invalid GHCR reference: {}",
                 reference
             )));
-        }
-
-        let path = reference
-            .strip_prefix("ghcr.io/")
-            .or_else(|| reference.strip_prefix("GHCR.IO/"))
-            .unwrap_or(reference);
+        };
 
         let (package, tag) = if let Some((pkg, digest)) = path.split_once('@') {
             (pkg, digest.to_string())
@@ -165,7 +159,7 @@ impl UrlPackage {
         Self::validate_names(&pkg_name, pkg_id.as_deref(), pkg_family.as_deref())?;
 
         Ok(Self {
-            url: reference.to_string(),
+            url: format!("ghcr.io/{path}"),
             pkg_id,
             pkg_family,
             pkg_name,
@@ -294,6 +288,15 @@ impl UrlPackage {
     }
 }
 
+/// The rest of `input` after a `ghcr.io/` prefix in any letter case.
+fn strip_ghcr_prefix(input: &str) -> Option<&str> {
+    const PREFIX: &str = "ghcr.io/";
+    input
+        .get(..PREFIX.len())
+        .filter(|p| p.eq_ignore_ascii_case(PREFIX))
+        .map(|_| &input[PREFIX.len()..])
+}
+
 /// Extract the family from a URL: host plus the first two path segments.
 ///
 /// Examples:
@@ -685,6 +688,18 @@ mod tests {
         assert!(pkg.is_ghcr);
     }
 
+    #[test]
+    fn ghcr_prefix_is_case_insensitive() {
+        let pkg =
+            UrlPackage::from_ghcr("Ghcr.Io/pkgforge/soar:v0.8.1", None, None, None, None).unwrap();
+
+        assert_eq!(pkg.url, "ghcr.io/pkgforge/soar:v0.8.1");
+        assert_eq!(pkg.pkg_name, "soar");
+        assert_eq!(pkg.pkg_family.as_deref(), Some("pkgforge.soar"));
+        assert!(UrlPackage::is_ghcr("gHcR.iO/org/repo"));
+        assert!(!UrlPackage::is_ghcr("ghcr.i"));
+    }
+
     #[test]
     fn test_ghcr_with_digest() {
         let ghcr = "ghcr.io/org/repo@sha256:deadbeef1234567890";

crates/soar-core/src/utils.rs

@@ -7,10 +7,6 @@ use std::{
 };
 
 use soar_config::config::get_config;
-use soar_utils::{
-    error::FileSystemResult,
-    fs::{safe_remove, walk_dir},
-};
 use tracing::info;
 
 use crate::error::{ErrorContext, SoarError};
@@ -58,33 +54,6 @@ pub fn cleanup_cache() -> Result<()> {
     Ok(())
 }
 
-fn remove_action(path: &Path) -> FileSystemResult<()> {
-    if path.is_symlink() && !path.exists() {
-        safe_remove(path)?;
-        info!("Removed broken symlink: {}", path.display());
-    }
-    Ok(())
-}
-
-/// Removes broken symlinks from bin, desktop, and icons directories.
-pub fn remove_broken_symlinks() -> Result<()> {
-    let mut soar_files_action = |path: &Path| -> FileSystemResult<()> {
-        if let Some(filename) = path.file_stem().and_then(|s| s.to_str())
-            && filename.ends_with("-soar")
-        {
-            return remove_action(path);
-        }
-        Ok(())
-    };
-
-    let config = get_config();
-    walk_dir(&config.get_bin_path()?, &mut remove_action)?;
-    walk_dir(&config.get_desktop_path()?, &mut soar_files_action)?;
-    walk_dir(config.get_icons_path(), &mut soar_files_action)?;
-
-    Ok(())
-}
-
 /// Gets the extract directory path for a given base directory.
 pub fn get_extract_dir<P: AsRef<Path>>(base_dir: P) -> PathBuf {
     let base_dir = base_dir.as_ref();

crates/soar-operations/src/health.rs

@@ -1,4 +1,7 @@
-use std::path::PathBuf;
+use std::{
+    io::ErrorKind,
+    path::{Path, PathBuf},
+};
 
 use soar_core::{SoarResult, package::remove::PackageRemover};
 use soar_db::repository::core::CoreRepository;
@@ -128,15 +131,26 @@ pub fn remove_broken_symlinks(ctx: &SoarContext) -> SoarResult<Vec<PathBuf>> {
     let broken = get_broken_symlinks(ctx)?;
 
     let mut removed = Vec::new();
-    for path in &broken {
-        if std::fs::remove_file(path).is_ok() {
-            removed.push(path.clone());
+    for path in broken {
+        // The link may have been repaired since the scan.
+        if is_dangling(&path) && std::fs::remove_file(&path).is_ok() {
+            removed.push(path);
         }
     }
 
     Ok(removed)
 }
 
+/// Whether `path` is a symlink whose target does not exist.
+///
+/// Any other failure to read the target, such as a permission error, leaves
+/// the link alone, since its target may well be there.
+fn is_dangling(path: &Path) -> bool {
+    path.is_symlink()
+        && std::fs::metadata(path)
+            .is_err_and(|e| matches!(e.kind(), ErrorKind::NotFound | ErrorKind::NotADirectory))
+}
+
 fn get_broken_packages(ctx: &SoarContext) -> SoarResult<Vec<BrokenPackage>> {
     let diesel_db = ctx.diesel_core_db()?;
     let broken = diesel_db.with_conn(CoreRepository::list_broken)?;
@@ -161,22 +175,19 @@ fn get_broken_symlinks(ctx: &SoarContext) -> SoarResult<Vec<PathBuf>> {
     // something is wrong when nothing is.
     let bin_path = config.get_bin_path()?;
     if bin_path.is_dir() {
-        walk_dir(
-            &bin_path,
-            &mut |path: &std::path::Path| -> FileSystemResult<()> {
-                if !path.exists() {
-                    broken.push(path.to_path_buf());
-                }
-                Ok(())
-            },
-        )?;
+        walk_dir(&bin_path, &mut |path: &Path| -> FileSystemResult<()> {
+            if is_dangling(path) {
+                broken.push(path.to_path_buf());
+            }
+            Ok(())
+        })?;
     }
 
     let desktop_path = config.get_desktop_path()?;
-    let mut soar_check = |path: &std::path::Path| -> FileSystemResult<()> {
+    let mut soar_check = |path: &Path| -> FileSystemResult<()> {
         if let Some(filename) = path.file_stem().and_then(|s| s.to_str())
             && filename.ends_with("-soar")
-            && !path.exists()
+            && is_dangling(path)
         {
             broken.push(path.to_path_buf());
         }
@@ -193,3 +204,38 @@ fn get_broken_symlinks(ctx: &SoarContext) -> SoarResult<Vec<PathBuf>> {
 
     Ok(broken)
 }
+
+#[cfg(test)]
+mod tests {
+    use std::{
+        fs,
+        os::unix::fs::{PermissionsExt, symlink},
+    };
+
+    use super::*;
+
+    #[test]
+    fn only_links_to_missing_targets_are_dangling() {
+        let dir = tempfile::tempdir().unwrap();
+        let p = |name: &str| dir.path().join(name);
+        fs::write(p("file"), "").unwrap();
+        symlink(p("file"), p("good")).unwrap();
+        symlink(p("gone"), p("missing")).unwrap();
+        symlink(p("file/below"), p("through-file")).unwrap();
+
+        assert!(!is_dangling(&p("file")));
+        assert!(!is_dangling(&p("good")));
+        assert!(is_dangling(&p("missing")));
+        assert!(is_dangling(&p("through-file")));
+
+        fs::create_dir(p("locked")).unwrap();
+        fs::write(p("locked/target"), "").unwrap();
+        symlink(p("locked/target"), p("unreadable")).unwrap();
+        fs::set_permissions(p("locked"), fs::Permissions::from_mode(0o000)).unwrap();
+        // Root reads through any permission, so the case cannot arise there.
+        if fs::metadata(p("unreadable")).is_err() {
+            assert!(!is_dangling(&p("unreadable")));
+        }
+        fs::set_permissions(p("locked"), fs::Permissions::from_mode(0o755)).unwrap();
+    }
+}

crates/soar-operations/src/progress.rs

@@ -18,7 +18,7 @@ const PROGRESS_INTERVAL: Duration = Duration::from_millis(100);
 /// `progress_callback` parameter.
 ///
 /// soar-dl reports every read, so chunk progress is passed on at most once
-/// per [`PROGRESS_INTERVAL`], plus the chunk that completes the download.
+/// per `PROGRESS_INTERVAL`, plus the chunk that completes the download.
 pub fn create_progress_bridge(
     events: EventSinkHandle,
     op_id: OperationId,