fix: validate .backup root on force reinstall, abort preset force reinstall on refused removal, keep update refusal non-destructive

Author: chelsealong

src/specify_cli/extensions/__init__.py

@@ -2159,6 +2159,11 @@ def install_from_directory(
             # Clear any stale backup from a previous remove so that only the
             # backup produced by the current remove() call is restored later.
             backup_config_dir = self.extensions_dir / ".backup" / manifest.id
+            if backup_config_dir.parent.is_symlink():
+                raise ExtensionError(
+                    f"Refusing to reinstall '{manifest.id}': "
+                    f"'.backup' is a symlink"
+                )
             # Check is_symlink first: is_dir() follows symlinks so a
             # symlink-to-directory would pass, but rmtree() raises on them.
             if backup_config_dir.is_symlink():

src/specify_cli/extensions/_command_update_transaction.py

@@ -521,6 +521,7 @@ def backup_extension_skills(skill_names, *, skills_dir=None):
                     # 7. Remove old extension (handles command file cleanup and registry removal)
                     installation_modified = True
                     if manager.remove(extension_id, keep_config=True) is False:
+                        installation_modified = False
                         raise RuntimeError(
                             f"Could not safely remove existing '{extension_id}'"
                         )

src/specify_cli/presets/_manager.py

@@ -395,7 +395,11 @@ def install_from_directory(
                     f"Preset '{manifest.id}' is already installed. "
                     f"Use 'specify preset remove {manifest.id}' first."
                 )
-            self.remove(manifest.id)
+            if not self.remove(manifest.id):
+                raise PresetError(
+                    f"Refusing to reinstall '{manifest.id}': existing "
+                    f"installation could not be safely removed"
+                )
 
         dest_dir = self.presets_dir / manifest.id
         if dest_dir.exists():

tests/specify_cli/presets/test_manager.py

@@ -1,6 +1,7 @@
 """Tests for preset installation and removal in specify_cli.presets._manager."""
 
 import json
+import shutil
 import tarfile
 import zipfile
 from pathlib import Path
@@ -335,6 +336,19 @@ def test_remove_refuses_symlinked_preset_dir(self, project_dir):
         assert pack_dir.is_symlink()
         assert manager.registry.is_installed("test-pack")
 
+    def test_force_reinstall_aborts_when_removal_refused(self, project_dir, pack_dir):
+        """install_from_directory(force=True) must abort if remove() refuses."""
+        manager = PresetManager(project_dir)
+        manager.install_from_directory(pack_dir, "0.1.5")
+        installed = project_dir / ".specify" / "presets" / "test-pack"
+        real_target = project_dir.parent / "real-target"
+        shutil.move(str(installed), str(real_target))
+        installed.symlink_to(real_target, target_is_directory=True)
+
+        with pytest.raises(PresetError, match="could not be safely removed"):
+            manager.install_from_directory(pack_dir, "0.1.5", force=True)
+        assert (real_target / "preset.yml").exists()
+
     def test_remove_refuses_dangling_symlinked_preset_dir(self, project_dir):
         """A dangling symlink must fail explicitly rather than be silently skipped.
 

tests/test_extensions.py

@@ -3337,6 +3337,23 @@ def test_force_reinstall_aborts_when_removal_refused(self, extension_dir, projec
             )
         assert (real_target / "extension.yml").exists()
 
+    def test_force_reinstall_refuses_symlinked_backup_root(self, extension_dir, project_dir):
+        """Stale-backup cleanup must not delete through a symlinked .backup root."""
+        manager = ExtensionManager(project_dir)
+        manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
+        outside = project_dir.parent / "outside-backup"
+        (outside / "test-ext").mkdir(parents=True)
+        (outside / "test-ext" / "keep.txt").write_text("keep")
+        (project_dir / ".specify" / "extensions" / ".backup").symlink_to(
+            outside, target_is_directory=True
+        )
+
+        with pytest.raises(ExtensionError, match="symlink"):
+            manager.install_from_directory(
+                extension_dir, "0.1.0", register_commands=False, force=True
+            )
+        assert (outside / "test-ext" / "keep.txt").exists()
+
     def test_remove_refuses_symlinked_backup_dir(self, extension_dir, project_dir):
         """Config backups must not be redirected through a symlinked destination."""
         manager = ExtensionManager(project_dir)