Prevent extension config and download path escapes

Extension catalog commands accepted parsed YAML roots without first checking that they were mappings, so corrupted configs could surface raw AttributeError tracebacks. URL-based extension downloads also derived their cache filename from user input, which let path separators escape the intended cache directory.

The command layer now validates catalog config roots before calling get(), and URL downloads use an unnamed temporary filename created inside the downloads cache directory. Regression tests cover both corrupted catalog roots and traversal-looking extension names.

Author: darion-yaphet

src/specify_cli/extensions/_commands.py

@@ -10,6 +10,7 @@
 
 import os
 import shutil
+import tempfile
 import zipfile
 from pathlib import Path
 from typing import Optional
@@ -58,6 +59,27 @@ def _display_project_path(*args, **kwargs):
     return _f(*args, **kwargs)
 
 
+def _load_catalog_command_config(project_root: Path, config_path: Path) -> dict:
+    """Load extension catalog CLI config with user-facing shape errors."""
+    try:
+        config = yaml.safe_load(config_path.read_text(encoding="utf-8"))
+    except Exception as e:
+        config_label = _display_project_path(project_root, config_path)
+        console.print(f"[red]Error:[/red] Failed to read {config_label}: {e}")
+        raise typer.Exit(1)
+
+    if config is None:
+        return {}
+    if not isinstance(config, dict):
+        config_label = _display_project_path(project_root, config_path)
+        console.print(
+            f"[red]Error:[/red] Invalid catalog config {config_label}: "
+            "expected a YAML mapping at the root."
+        )
+        raise typer.Exit(1)
+    return config
+
+
 def _resolve_installed_extension(
     argument: str,
     installed_extensions: list,
@@ -293,12 +315,7 @@ def catalog_add(
 
     # Load existing config
     if config_path.exists():
-        try:
-            config = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {}
-        except Exception as e:
-            config_label = _display_project_path(project_root, config_path)
-            console.print(f"[red]Error:[/red] Failed to read {config_label}: {e}")
-            raise typer.Exit(1)
+        config = _load_catalog_command_config(project_root, config_path)
     else:
         config = {}
 
@@ -345,11 +362,7 @@ def catalog_remove(
         console.print("[red]Error:[/red] No catalog config found. Nothing to remove.")
         raise typer.Exit(1)
 
-    try:
-        config = yaml.safe_load(config_path.read_text(encoding="utf-8")) or {}
-    except Exception:
-        console.print("[red]Error:[/red] Failed to read catalog config.")
-        raise typer.Exit(1)
+    config = _load_catalog_command_config(project_root, config_path)
 
     catalogs = config.get("catalogs", [])
     if not isinstance(catalogs, list):
@@ -463,7 +476,13 @@ def extension_add(
                 # Download ZIP to temp location
                 download_dir = project_root / ".specify" / "extensions" / ".cache" / "downloads"
                 download_dir.mkdir(parents=True, exist_ok=True)
-                zip_path = download_dir / f"{extension}-url-download.zip"
+                with tempfile.NamedTemporaryFile(
+                    prefix="extension-url-download-",
+                    suffix=".zip",
+                    dir=download_dir,
+                    delete=False,
+                ) as download_file:
+                    zip_path = Path(download_file.name)
 
                 try:
                     from specify_cli.authentication.http import open_url as _open_url

tests/integrations/test_cli.py

@@ -1315,6 +1315,36 @@ def test_catalog_config_output_uses_posix_paths(self, tmp_path):
         assert extension_list.exit_code == 0, extension_list.output
         assert "Config: .specify/extension-catalogs.yml" in extension_list.output
 
+    def test_extension_catalog_add_rejects_non_mapping_config_root(self, tmp_path):
+        project = self._make_project(tmp_path)
+        cfg_path = project / ".specify" / "extension-catalogs.yml"
+        cfg_path.write_text("- not\n- a\n- mapping\n", encoding="utf-8")
+
+        result = self._invoke([
+            "extension", "catalog", "add",
+            "https://example.com/extension-catalog.yml",
+            "--name", "demo-extensions",
+        ], project)
+
+        assert result.exit_code == 1, result.output
+        output = _normalize_cli_output(result.output)
+        assert "Invalid catalog config .specify/extension-catalogs.yml" in output
+        assert "expected a YAML mapping at the root" in output
+        assert "AttributeError" not in output
+
+    def test_extension_catalog_remove_rejects_non_mapping_config_root(self, tmp_path):
+        project = self._make_project(tmp_path)
+        cfg_path = project / ".specify" / "extension-catalogs.yml"
+        cfg_path.write_text("- not\n- a\n- mapping\n", encoding="utf-8")
+
+        result = self._invoke(["extension", "catalog", "remove", "demo"], project)
+
+        assert result.exit_code == 1, result.output
+        output = _normalize_cli_output(result.output)
+        assert "Invalid catalog config .specify/extension-catalogs.yml" in output
+        assert "expected a YAML mapping at the root" in output
+        assert "AttributeError" not in output
+
     # -- search ------------------------------------------------------------
 
     def test_search_lists_all(self, tmp_path, monkeypatch):

tests/test_extensions.py

@@ -4871,6 +4871,58 @@ def test_add_from_url_cancel_exits_cleanly(self, tmp_path):
         assert result.exit_code == 0
         assert "Cancelled" in result.output
 
+    def test_add_from_url_uses_cache_tempfile_for_untrusted_extension_name(self, tmp_path):
+        """The extension argument must not control the downloaded ZIP path."""
+        import io
+        from types import SimpleNamespace
+        from typer.testing import CliRunner
+        from unittest.mock import patch
+        from specify_cli import app
+
+        class FakeResponse(io.BytesIO):
+            def __enter__(self):
+                return self
+
+            def __exit__(self, exc_type, exc, tb):
+                return False
+
+        project_dir = tmp_path / "test-project"
+        project_dir.mkdir()
+        (project_dir / ".specify").mkdir()
+        downloads_dir = project_dir / ".specify" / "extensions" / ".cache" / "downloads"
+        installed = {}
+
+        def fake_install_from_zip(self_obj, zip_path, speckit_version, priority=10, force=False):
+            captured_path = Path(zip_path)
+            installed["zip_path"] = captured_path
+            installed["zip_bytes"] = captured_path.read_bytes()
+            return SimpleNamespace(
+                id="escape",
+                name="Escape Test",
+                version="1.0.0",
+                description="Test extension",
+                warnings=[],
+                commands=[],
+                hooks=[],
+            )
+
+        runner = CliRunner()
+        with patch.object(Path, "cwd", return_value=project_dir), \
+             patch("typer.confirm", return_value=True), \
+             patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(b"zip-bytes")), \
+             patch.object(ExtensionManager, "install_from_zip", fake_install_from_zip):
+            result = runner.invoke(
+                app,
+                ["extension", "add", "../outside", "--from", "https://example.com/ext.zip"],
+                catch_exceptions=True,
+            )
+
+        assert result.exit_code == 0
+        assert installed["zip_bytes"] == b"zip-bytes"
+        assert installed["zip_path"].resolve().is_relative_to(downloads_dir.resolve())
+        assert installed["zip_path"].name.startswith("extension-url-download-")
+        assert not installed["zip_path"].exists()
+
 
 class TestDownloadExtensionBundled:
     """Tests for download_extension handling of bundled extensions."""