fix: harden dependency path validation

CHANGELOG.md

@@ -8,11 +8,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-### Added
+## [0.8.2] - 2026-03-19
+
+### Security
 
-- JFrog Artifactory VCS repository support — explicit FQDN, transparent proxy (`ARTIFACTORY_BASE_URL`), air-gapped mode (`ARTIFACTORY_ONLY=1`), multi-format archive URLs, zip path traversal protection (#354)
+- Harden dependency path validation (#364)
+
+### Added
 
-## [0.8.2] - 2026-03-18
+- JFrog Artifactory VCS repository support (#354)
 
 ### Fixed
 

src/apm_cli/commands/install.py

@@ -11,6 +11,7 @@
 from ..utils.console import _rich_error, _rich_info, _rich_success, _rich_warning
 from ..utils.diagnostics import DiagnosticCollector
 from ..utils.github_host import default_host, is_valid_fqdn
+from ..utils.path_security import safe_rmtree
 from ._helpers import (
     _create_minimal_apm_yml,
     _get_default_config,
@@ -802,7 +803,10 @@ def _copy_local_package(dep_ref, install_path, project_root):
     # Ensure parent exists and clean target (always re-copy for local deps)
     install_path.parent.mkdir(parents=True, exist_ok=True)
     if install_path.exists():
-        shutil.rmtree(install_path)
+        # install_path is already validated by get_install_path() (Layer 2),
+        # but use safe_rmtree for defense-in-depth.
+        apm_modules_dir = install_path.parent.parent  # _local/<name> → apm_modules
+        safe_rmtree(install_path, apm_modules_dir)
 
     shutil.copytree(local, install_path, dirs_exist_ok=False, symlinks=True)
     return install_path

src/apm_cli/commands/prune.py

@@ -7,6 +7,7 @@
 import click
 
 from ..utils.console import _rich_error, _rich_info, _rich_success, _rich_warning
+from ..utils.path_security import PathTraversalError, safe_rmtree
 from ._helpers import _build_expected_install_paths, _scan_installed_packages
 
 # APM Dependencies
@@ -80,7 +81,7 @@ def prune(ctx, dry_run):
             path_parts = org_repo_name.split("/")
             pkg_path = apm_modules_dir.joinpath(*path_parts)
             try:
-                shutil.rmtree(pkg_path)
+                safe_rmtree(pkg_path, apm_modules_dir)
                 _rich_info(f"+ Removed {org_repo_name}")
                 removed_count += 1
                 pruned_keys.append(org_repo_name)

src/apm_cli/commands/uninstall.py

@@ -8,6 +8,7 @@
 import click
 
 from ..utils.console import _rich_error, _rich_info, _rich_success, _rich_warning
+from ..utils.path_security import PathTraversalError, safe_rmtree
 
 # APM Dependencies
 try:
@@ -210,6 +211,9 @@ def _parse_dependency_entry(dep_entry):
                 try:
                     dep_ref = _parse_dependency_entry(package)
                     package_path = dep_ref.get_install_path(apm_modules_dir)
+                except (PathTraversalError,) as e:
+                    _rich_error(f"x Refusing to remove {package}: {e}")
+                    continue
                 except (ValueError, TypeError, AttributeError, KeyError):
                     # Fallback for invalid format: use raw path segments
                     package_str = package if isinstance(package, str) else str(package)
@@ -221,7 +225,7 @@ def _parse_dependency_entry(dep_entry):
 
                 if package_path.exists():
                     try:
-                        shutil.rmtree(package_path)
+                        safe_rmtree(package_path, apm_modules_dir)
                         _rich_info(f"+ Removed {package} from apm_modules/")
                         removed_from_modules += 1
                         deleted_pkg_paths.append(package_path)
@@ -306,7 +310,7 @@ def _find_transitive_orphans(lockfile, removed_urls):
 
                     if orphan_path.exists():
                         try:
-                            shutil.rmtree(orphan_path)
+                            safe_rmtree(orphan_path, apm_modules_dir)
                             _rich_info(f"+ Removed transitive dependency {orphan_key} from apm_modules/")
                             removed_from_modules += 1
                             deleted_orphan_paths.append(orphan_path)

src/apm_cli/deps/github_downloader.py

@@ -1566,6 +1566,9 @@ def download_subdirectory_package(self, dep_ref: DependencyReference, target_pat
 
             # Check if subdirectory exists
             source_subdir = temp_clone_path / subdir_path
+            # Security: ensure subdirectory resolves within the cloned repo
+            from ..utils.path_security import ensure_path_within
+            ensure_path_within(source_subdir, temp_clone_path)
             if not source_subdir.exists():
                 raise RuntimeError(f"Subdirectory '{subdir_path}' not found in repository")
 

src/apm_cli/models/dependency.py

@@ -7,6 +7,7 @@
 from pathlib import Path
 from typing import Any, Dict, List, Optional
 
+from ..utils.path_security import PathTraversalError, ensure_path_within
 from ..utils.github_host import (
     default_host,
     is_artifactory_path,
@@ -293,43 +294,75 @@ def get_install_path(self, apm_modules_dir: Path) -> Path:
 
         Returns:
             Path: Absolute path to the package installation directory
+
+        Raises:
+            PathTraversalError: If the computed path escapes apm_modules_dir
         """
         if self.is_local and self.local_path:
             pkg_dir_name = Path(self.local_path).name
-            return apm_modules_dir / "_local" / pkg_dir_name
+            if pkg_dir_name in ('', '.', '..'):
+                raise PathTraversalError(
+                    f"Invalid local package path '{self.local_path}': "
+                    f"basename must not be empty, '.', or '..'"
+                )
+            result = apm_modules_dir / "_local" / pkg_dir_name
+            ensure_path_within(result, apm_modules_dir)
+            return result
 
         repo_parts = self.repo_url.split("/")
+
+        # Security: reject traversal in repo_url segments (catches lockfile injection)
+        if any(seg in ('.', '..') for seg in repo_parts):
+            raise PathTraversalError(
+                f"Invalid repo_url '{self.repo_url}': "
+                f"path segments must not be '.' or '..'"
+            )
+
+        # Security: reject traversal in virtual_path (catches lockfile injection)
+        if self.virtual_path and any(
+            seg in ('.', '..') for seg in self.virtual_path.replace('\\', '/').split('/')
+        ):
+            raise PathTraversalError(
+                f"Invalid virtual_path '{self.virtual_path}': "
+                f"path segments must not be '.' or '..'"
+            )
+        result: Path | None = None
 
         if self.is_virtual:
             # Subdirectory packages (like Claude Skills) should use natural path structure
             if self.is_virtual_subdirectory():
                 # Use repo path + subdirectory path
                 if self.is_azure_devops() and len(repo_parts) >= 3:
                     # ADO: org/project/repo/subdir
-                    return apm_modules_dir / repo_parts[0] / repo_parts[1] / repo_parts[2] / self.virtual_path
+                    result = apm_modules_dir / repo_parts[0] / repo_parts[1] / repo_parts[2] / self.virtual_path
                 elif len(repo_parts) >= 2:
                     # owner/repo/subdir or group/subgroup/repo/subdir
-                    return apm_modules_dir.joinpath(*repo_parts, self.virtual_path)
+                    result = apm_modules_dir.joinpath(*repo_parts, self.virtual_path)
             else:
                 # Virtual file/collection: use sanitized package name (flattened)
                 package_name = self.get_virtual_package_name()
                 if self.is_azure_devops() and len(repo_parts) >= 3:
                     # ADO: org/project/virtual-pkg-name
-                    return apm_modules_dir / repo_parts[0] / repo_parts[1] / package_name
+                    result = apm_modules_dir / repo_parts[0] / repo_parts[1] / package_name
                 elif len(repo_parts) >= 2:
                     # owner/virtual-pkg-name (use first segment as namespace)
-                    return apm_modules_dir / repo_parts[0] / package_name
+                    result = apm_modules_dir / repo_parts[0] / package_name
         else:
             # Regular package: use full repo path
             if self.is_azure_devops() and len(repo_parts) >= 3:
                 # ADO: org/project/repo
-                return apm_modules_dir / repo_parts[0] / repo_parts[1] / repo_parts[2]
+                result = apm_modules_dir / repo_parts[0] / repo_parts[1] / repo_parts[2]
             elif len(repo_parts) >= 2:
                 # owner/repo or group/subgroup/repo (generic hosts)
-                return apm_modules_dir.joinpath(*repo_parts)
+                result = apm_modules_dir.joinpath(*repo_parts)
 
-        # Fallback: join all parts
-        return apm_modules_dir.joinpath(*repo_parts)
+        if result is None:
+            # Fallback: join all parts
+            result = apm_modules_dir.joinpath(*repo_parts)
+
+        # Security: ensure the computed path stays within apm_modules/
+        ensure_path_within(result, apm_modules_dir)
+        return result
 
     @staticmethod
     def _normalize_ssh_protocol_url(url: str) -> str:
@@ -428,7 +461,13 @@ def parse_from_dict(cls, entry: dict) -> "DependencyReference":
         if sub_path is not None:
             if not isinstance(sub_path, str) or not sub_path.strip():
                 raise ValueError("'path' field must be a non-empty string")
-            sub_path = sub_path.strip().strip('/')
+            # Normalize backslashes to forward slashes for cross-platform safety
+            sub_path = sub_path.replace('\\', '/').strip().strip('/')
+            # Security: reject path traversal
+            if any(seg in ('.', '..') for seg in sub_path.split('/')):
+                raise PathTraversalError(
+                    f"Invalid path '{sub_path}': path segments must not be '.' or '..'"
+                )
 
         # Parse the git URL using the standard parser
         dep = cls.parse(git_url)
@@ -633,6 +672,14 @@ def parse(cls, dependency_str: str) -> "DependencyReference":
 
                 # Extract virtual path (base repo is derived later)
                 virtual_path = '/'.join(path_segments[min_base_segments:])
+
+                # Security: reject path traversal in virtual path
+                # Normalize backslashes for cross-platform safety
+                vp_check = virtual_path.replace('\\', '/')
+                if any(seg in ('.', '..') for seg in vp_check.split('/')):
+                    raise PathTraversalError(
+                        f"Invalid virtual path '{virtual_path}': path segments must not be '.' or '..'"
+                    )
 
                 # Virtual package types (validated later during download):
                 # 1. Collections: /collections/ in path
@@ -807,6 +854,10 @@ def parse(cls, dependency_str: str) -> "DependencyReference":
                 # ADO project names may contain spaces
                 allowed_pattern = r'^[a-zA-Z0-9._\- ]+$' if is_ado_host else r'^[a-zA-Z0-9._-]+$'
                 for part in uparts:
+                    if part in ('.', '..'):
+                        raise PathTraversalError(
+                            f"Invalid repository path component: '{part}' is a traversal sequence"
+                        )
                     if not re.match(allowed_pattern, part.rstrip('.git')):
                         raise ValueError(f"Invalid repository path component: {part}")
 

src/apm_cli/utils/path_security.py

@@ -0,0 +1,58 @@
+"""Centralised path-security helpers for APM CLI.
+
+Every filesystem operation whose target is derived from user-controlled
+input (dependency strings, ``virtual_path``, ``apm.yml`` fields) **must**
+pass through one of these guards before touching the disk.
+
+Design
+------
+* ``ensure_path_within`` is the single predicate — resolves both paths and
+  asserts containment via ``Path.is_relative_to``.
+* ``safe_rmtree`` wraps ``shutil.rmtree`` with an ``ensure_path_within``
+  check so callers get a drop-in replacement.
+* ``PathTraversalError`` is a ``ValueError`` subclass for clear error
+  semantics and easy ``except`` targeting.
+"""
+
+from __future__ import annotations
+
+import shutil
+from pathlib import Path
+
+
+class PathTraversalError(ValueError):
+    """Raised when a computed path escapes its expected base directory."""
+
+
+def ensure_path_within(path: Path, base_dir: Path) -> Path:
+    """Resolve *path* and assert it lives inside *base_dir*.
+
+    Returns the resolved path on success.  Raises
+    :class:`PathTraversalError` if the resolved path escapes *base_dir*.
+
+    This is intentionally strict: symlinks are resolved so that a link
+    pointing outside the base is caught as well.
+    """
+    resolved = path.resolve()
+    resolved_base = base_dir.resolve()
+    try:
+        if not resolved.is_relative_to(resolved_base):
+            raise PathTraversalError(
+                f"Path '{path}' resolves to '{resolved}' which is outside "
+                f"the allowed base directory '{resolved_base}'"
+            )
+    except (TypeError, ValueError) as exc:
+        raise PathTraversalError(
+            f"Cannot verify containment of '{path}' within '{base_dir}': {exc}"
+        ) from exc
+    return resolved
+
+
+def safe_rmtree(path: Path, base_dir: Path) -> None:
+    """Remove *path* only if it resolves within *base_dir*.
+
+    Drop-in replacement for ``shutil.rmtree(path)`` at sites where the
+    target is derived from user-controlled input.
+    """
+    ensure_path_within(path, base_dir)
+    shutil.rmtree(path)