From 9a09195d49e130a97fea53c9b4bd48ae0c90b6ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EB=B0=95=EC=A7=84=EC=95=84?= Date: Tue, 16 Jun 2026 16:38:47 +0900 Subject: [PATCH] fix: prevent extension self-install from deleting source dir (#2990) `specify extension add --dev --force` permanently deleted the extension directory without registering it when the source path resolved to the extension's own install location (`.specify/extensions/`). With `--force`, `install_from_directory()` removed the existing installation (the source) and then `shutil.copytree()` tried to copy from the now-deleted directory, destroying it and crashing. Add a guard that fails fast with a clear ValidationError when the resolved source path equals the install destination, before any destructive operation runs. Includes a regression test asserting the directory and its contents survive. Co-Authored-By: Claude Opus 4.8 --- src/specify_cli/extensions.py | 17 +++++++++++++++-- tests/test_extensions.py | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index db53b7997f..64fe5257f3 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1333,6 +1333,20 @@ def install_from_directory( # Reject manifests that would shadow core commands or installed extensions. self._validate_install_conflicts(manifest) + # Refuse to install an extension from its own install destination — with + # --force this would delete the source before copying it (issue #2990). + dest_dir = self.extensions_dir / manifest.id + try: + same_location = source_dir.resolve() == dest_dir.resolve() + except OSError: + same_location = False + if same_location: + raise ValidationError( + f"Source path is the install destination for '{manifest.id}' " + f"({dest_dir}). Refusing to proceed to avoid deleting the " + f"extension. Install from a copy in a different location instead." + ) + # Remove existing installation AFTER all validations pass so that a # validation failure doesn't leave the user with a half-uninstalled # extension (configs stranded in .backup/). @@ -1351,8 +1365,7 @@ def install_from_directory( backup_config_dir.unlink() did_remove = self.remove(manifest.id) - # Install extension - dest_dir = self.extensions_dir / manifest.id + # Install extension (dest_dir computed above during self-install guard) if dest_dir.exists(): shutil.rmtree(dest_dir) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 1d05e1c2c4..390efba05c 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -1118,6 +1118,29 @@ def test_install_force_without_existing(self, extension_dir, project_dir): assert manifest.id == "test-ext" assert manager.registry.is_installed("test-ext") + def test_install_from_install_dir_is_rejected_without_data_loss( + self, extension_dir, project_dir + ): + """Installing from an extension's own install dir must fail without + deleting it (regression for issue #2990).""" + manager = ExtensionManager(project_dir) + + # Install once so the extension lives at its install destination. + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + install_dir = project_dir / ".specify" / "extensions" / "test-ext" + assert install_dir.exists() + + # Re-installing from that same directory with --force must be rejected. + with pytest.raises(ValidationError, match="install destination"): + manager.install_from_directory( + install_dir, "0.1.0", register_commands=False, force=True + ) + + # The directory and its contents must be left intact (no data loss). + assert install_dir.exists() + assert (install_dir / "extension.yml").exists() + assert (install_dir / "commands" / "hello.md").exists() + def test_install_zip_force_reinstall(self, extension_dir, project_dir): """Test force-reinstalling from ZIP when already installed.""" import zipfile