fix: harden shared manifest and upgrade refresh

Author: PascalThuet

src/specify_cli/__init__.py

@@ -2926,7 +2926,6 @@ def integration_upgrade(
             script_type=selected_script,
             raw_options=raw_options,
         )
-        new_manifest.save()
         settings = _with_integration_setting(
             current,
             key,
@@ -2935,16 +2934,23 @@ def integration_upgrade(
             raw_options=raw_options,
             parsed_options=parsed_options,
         )
+        if installed_key == key:
+            try:
+                _refresh_shared_templates(
+                    project_root,
+                    invoke_separator=_invoke_separator_for_integration(
+                        integration, {"integration_settings": settings}, key, parsed_options
+                    ),
+                    force=force,
+                )
+            except (ValueError, OSError) as exc:
+                raise _SharedTemplateRefreshError(
+                    f"Failed to refresh shared templates for '{key}': {exc}"
+                ) from exc
+        new_manifest.save()
         _write_integration_json(project_root, installed_key, installed_keys, settings)
         if installed_key == key:
             _update_init_options_for_integration(project_root, integration, script_type=selected_script)
-            _refresh_shared_templates(
-                project_root,
-                invoke_separator=_invoke_separator_for_integration(
-                    integration, {"integration_settings": settings}, key, parsed_options
-                ),
-                force=force,
-            )
     except Exception as exc:
         # Don't teardown — setup overwrites in-place, so teardown would
         # delete files that were working before the upgrade.  Just report.

src/specify_cli/integrations/manifest.py

@@ -11,6 +11,7 @@
 import hashlib
 import json
 import os
+import tempfile
 from datetime import datetime, timezone
 from pathlib import Path
 from typing import Any
@@ -47,6 +48,59 @@ def _validate_rel_path(rel: Path, root: Path) -> Path:
     return resolved
 
 
+def _manifest_path_label(root: Path, path: Path) -> str:
+    try:
+        return path.relative_to(root).as_posix()
+    except ValueError:
+        return path.as_posix()
+
+
+def _ensure_safe_manifest_directory(root: Path, directory: Path) -> None:
+    """Create a manifest directory without following symlinked parents."""
+    root_resolved = root.resolve()
+    try:
+        rel = directory.relative_to(root)
+    except ValueError:
+        label = _manifest_path_label(root, directory)
+        raise ValueError(f"Integration manifest directory escapes project root: {label}") from None
+
+    current = root
+    for part in rel.parts:
+        current = current / part
+        label = _manifest_path_label(root, current)
+        if current.is_symlink():
+            raise ValueError(f"Refusing to use symlinked integration manifest directory: {label}")
+        if current.exists():
+            if not current.is_dir():
+                raise ValueError(f"Integration manifest directory path is not a directory: {label}")
+            try:
+                current.resolve().relative_to(root_resolved)
+            except (OSError, ValueError):
+                raise ValueError(f"Integration manifest directory escapes project root: {label}") from None
+            continue
+        current.mkdir()
+        try:
+            current.resolve().relative_to(root_resolved)
+        except (OSError, ValueError):
+            raise ValueError(f"Integration manifest directory escapes project root: {label}") from None
+
+
+def _ensure_safe_manifest_destination(root: Path, path: Path) -> None:
+    """Refuse manifest writes that would escape the project or follow symlinks."""
+    root_resolved = root.resolve()
+    _ensure_safe_manifest_directory(root, path.parent)
+    label = _manifest_path_label(root, path)
+    if path.is_symlink():
+        raise ValueError(f"Refusing to overwrite symlinked integration manifest path: {label}")
+    if path.exists():
+        if not path.is_file():
+            raise ValueError(f"Integration manifest path is not a file: {label}")
+        try:
+            path.resolve().relative_to(root_resolved)
+        except (OSError, ValueError):
+            raise ValueError(f"Integration manifest path escapes project root: {label}") from None
+
+
 class IntegrationManifest:
     """Tracks files installed by a single integration.
 
@@ -217,8 +271,19 @@ def save(self) -> Path:
             "files": self._files,
         }
         path = self.manifest_path
-        path.parent.mkdir(parents=True, exist_ok=True)
-        path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8")
+        content = json.dumps(data, indent=2) + "\n"
+        _ensure_safe_manifest_destination(self.project_root, path)
+        fd, temp_name = tempfile.mkstemp(prefix=f".{path.name}.", dir=path.parent)
+        temp_path = Path(temp_name)
+        try:
+            with os.fdopen(fd, "w", encoding="utf-8") as fh:
+                fh.write(content)
+            temp_path.chmod(0o644)
+            _ensure_safe_manifest_destination(self.project_root, path)
+            os.replace(temp_path, path)
+        finally:
+            if temp_path.exists():
+                temp_path.unlink()
         return path
 
     @classmethod

tests/integrations/test_cli.py

@@ -373,6 +373,38 @@ def test_shared_infra_refuses_symlinked_specify_directory_before_mkdir(self, tmp
         assert not (outside / "scripts").exists()
         assert not (outside / "templates").exists()
 
+    @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable")
+    def test_shared_infra_refuses_symlinked_shared_manifest(self, tmp_path):
+        """Shared infra manifest saves must not follow destination symlinks."""
+        from specify_cli.shared_infra import install_shared_infra
+
+        project = tmp_path / "symlink-shared-manifest-test"
+        project.mkdir()
+        integrations_dir = project / ".specify" / "integrations"
+        integrations_dir.mkdir(parents=True)
+
+        outside = tmp_path / "outside-manifest.json"
+        outside.write_text("# outside\n", encoding="utf-8")
+        os.symlink(outside, integrations_dir / "speckit.manifest.json")
+
+        core_pack = tmp_path / "core-pack"
+        templates_src = core_pack / "templates"
+        templates_src.mkdir(parents=True)
+        (templates_src / "plan-template.md").write_text("# plan\n", encoding="utf-8")
+
+        with pytest.raises(ValueError, match="symlinked integration manifest"):
+            install_shared_infra(
+                project,
+                "sh",
+                version="test",
+                core_pack=core_pack,
+                repo_root=tmp_path / "unused",
+                console=_NoopConsole(),
+                force=True,
+            )
+
+        assert outside.read_text(encoding="utf-8") == "# outside\n"
+
     @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable")
     def test_shared_template_refresh_preflights_before_writing(self, tmp_path):
         """Template refresh validates all destinations before writing any file."""

tests/integrations/test_integration_subcommand.py

@@ -967,6 +967,34 @@ def test_upgrade_invalid_manifest_reports_cli_error(self, tmp_path):
         assert "manifest" in result.output
         assert "unreadable" in result.output
 
+    def test_upgrade_does_not_persist_state_when_template_refresh_fails(self, tmp_path, monkeypatch):
+        project = _init_project(tmp_path, "claude")
+        int_json = project / ".specify" / "integration.json"
+        init_options = project / ".specify" / "init-options.json"
+        manifest_path = project / ".specify" / "integrations" / "claude.manifest.json"
+
+        before_state = json.loads(int_json.read_text(encoding="utf-8"))
+        before_options = json.loads(init_options.read_text(encoding="utf-8"))
+        before_manifest = manifest_path.read_text(encoding="utf-8")
+
+        import specify_cli
+
+        def fail_refresh(*args, **kwargs):
+            raise ValueError("refuse refresh")
+
+        monkeypatch.setattr(specify_cli, "_refresh_shared_templates", fail_refresh)
+
+        result = _run_in_project(project, [
+            "integration", "upgrade", "claude",
+            "--force",
+        ])
+
+        assert result.exit_code != 0
+        assert "Failed to refresh shared templates" in result.output
+        assert json.loads(int_json.read_text(encoding="utf-8")) == before_state
+        assert json.loads(init_options.read_text(encoding="utf-8")) == before_options
+        assert manifest_path.read_text(encoding="utf-8") == before_manifest
+
     def test_upgrade_non_default_keeps_default_template_invocations(self, tmp_path):
         project = _init_project(tmp_path, "gemini")
         template = project / ".specify" / "templates" / "plan-template.md"