fix: block path traversal via spec name/version in persistence

src/metaseed/specs/persistence.py

@@ -12,16 +12,39 @@
 
 import shutil
 from collections.abc import Callable
+from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
 from metaseed.specs.builder import SpecBuilder
 
 if TYPE_CHECKING:
-    from pathlib import Path
-
     from metaseed.specs.schema import ProfileSpec
 
 
+def _specs_subpath(*parts: str) -> Path:
+    """Resolve a path under the custom-specs dir, rejecting escapes.
+
+    ``name`` and ``version`` components flow from user/model input into
+    filesystem paths (and, on delete, into ``shutil.rmtree``). A component like
+    ``../../.config/autostart`` or an absolute path would otherwise escape the
+    specs directory. Resolve the full path and require it to stay within the
+    (resolved) base; also reject empty components.
+
+    Raises:
+        ValueError: If any component is empty or the result escapes the base.
+    """
+    base = get_custom_specs_dir()
+    for part in parts:
+        if not part or not str(part).strip():
+            raise ValueError("path component cannot be empty")
+    candidate = base.joinpath(*parts).resolve()
+    if not candidate.is_relative_to(base.resolve()):
+        raise ValueError(
+            f"path component escapes the specs directory: {parts!r}"
+        )
+    return candidate
+
+
 def get_custom_specs_dir() -> Path:
     """Return the directory for user-created specs (created if needed).
 
@@ -65,7 +88,7 @@ def save_spec(spec: ProfileSpec, name: str | None = None) -> Path:
             f"Please choose a different name."
         )
 
-    version_dir = get_custom_specs_dir() / safe_name / spec.version
+    version_dir = _specs_subpath(safe_name, spec.version)
     version_dir.mkdir(parents=True, exist_ok=True)
 
     profile_path = version_dir / "profile.yaml"
@@ -149,12 +172,13 @@ def delete_user_spec(name: str, version: str | None = None) -> bool:
     if not loader.is_user_defined(name):
         raise ValueError(f"Cannot delete built-in specification: {name}")
 
-    profile_dir = get_custom_specs_dir() / name
+    # Containment-check before any rmtree: name/version must not escape the base.
+    profile_dir = _specs_subpath(name)
     if not profile_dir.exists():
         return False
 
     if version:
-        version_dir = profile_dir / version
+        version_dir = _specs_subpath(name, version)
         if not version_dir.exists():
             return False
         shutil.rmtree(version_dir)

tests/test_specs/test_persistence_security.py

@@ -0,0 +1,102 @@
+"""Security regression tests: spec name/version must not escape the specs dir.
+
+``save_spec`` interpolated ``spec.version`` and ``delete_user_spec`` interpolated
+``name``/``version`` into filesystem paths without sanitization — a crafted value
+(``../../.config/autostart``, an absolute path) escaped the custom-specs directory
+(a chosen-directory write, and a destructive ``rmtree`` on delete). The fix routes
+both through ``_specs_subpath`` (resolve + containment check).
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from metaseed.specs import persistence
+from metaseed.specs.loader import SpecLoader
+
+
+@pytest.fixture
+def specs_base(tmp_path, monkeypatch):
+    base = tmp_path / "custom_specs"
+    base.mkdir()
+    monkeypatch.setattr(persistence, "get_custom_specs_dir", lambda: base)
+    return base
+
+
+def _spec():
+    spec = SpecLoader(profile="miappe").load_profile(version="1.2", profile="miappe")
+    spec.name = "mytestprofile"
+    return spec
+
+
+class TestSaveSpecTraversal:
+    def test_relative_traversal_version_blocked(self, specs_base, tmp_path):
+        spec = _spec()
+        spec.version = "../../outside_evil"
+        with pytest.raises(ValueError):
+            persistence.save_spec(spec, name="mytestprofile")
+        assert not (tmp_path / "outside_evil").exists()
+
+    def test_absolute_version_blocked(self, specs_base, tmp_path):
+        spec = _spec()
+        # An absolute path outside the specs base must be rejected.
+        spec.version = str(tmp_path / "evil_abs")
+        with pytest.raises(ValueError):
+            persistence.save_spec(spec, name="mytestprofile")
+
+    def test_empty_version_blocked(self, specs_base):
+        spec = _spec()
+        spec.version = "   "
+        with pytest.raises(ValueError):
+            persistence.save_spec(spec, name="mytestprofile")
+
+    def test_valid_version_saves_within_base(self, specs_base):
+        spec = _spec()
+        spec.version = "1.0"
+        path = persistence.save_spec(spec, name="mytestprofile")
+        assert path.is_relative_to(specs_base)
+        assert path.name == "profile.yaml"
+        assert path.read_text(encoding="utf-8")
+
+
+class TestDeleteSpecTraversal:
+    def test_version_traversal_does_not_rmtree_outside(
+        self, specs_base, tmp_path, monkeypatch
+    ):
+        # A sibling dir outside the base that must survive.
+        victim = tmp_path / "victim"
+        victim.mkdir()
+        (victim / "keep.txt").write_text("x")
+        # Create the real profile dir so delete reaches the version check.
+        spec = _spec()
+        spec.version = "1.0"
+        persistence.save_spec(spec, name="mytestprofile")
+        # Force the user-defined gate so the containment guard is what fires.
+        monkeypatch.setattr(
+            "metaseed.specs.loader.SpecLoader.is_user_defined", lambda self, name: True
+        )
+        with pytest.raises(ValueError):
+            persistence.delete_user_spec("mytestprofile", "../../victim")
+        assert victim.exists() and (victim / "keep.txt").exists()
+
+    def test_name_traversal_does_not_rmtree_outside(
+        self, specs_base, tmp_path, monkeypatch
+    ):
+        victim = tmp_path / "victim2"
+        victim.mkdir()
+        (victim / "keep.txt").write_text("x")
+        monkeypatch.setattr(
+            "metaseed.specs.loader.SpecLoader.is_user_defined", lambda self, name: True
+        )
+        with pytest.raises(ValueError):
+            persistence.delete_user_spec("../../victim2")
+        assert victim.exists() and (victim / "keep.txt").exists()
+
+    def test_valid_delete_roundtrip(self, specs_base, monkeypatch):
+        spec = _spec()
+        spec.version = "1.0"
+        persistence.save_spec(spec, name="mytestprofile")
+        monkeypatch.setattr(
+            "metaseed.specs.loader.SpecLoader.is_user_defined", lambda self, name: True
+        )
+        assert persistence.delete_user_spec("mytestprofile", "1.0") is True