mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
* fix: prevent extension self-install from deleting source dir (#2990) `specify extension add <path> --dev --force` permanently deleted the extension directory without registering it when the source path resolved to the extension's own install location (`.specify/extensions/<id>`). 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 <noreply@anthropic.com> * fix: harden extension self-install guard --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1337,6 +1337,22 @@ class ExtensionManager:
|
||||
# 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(strict=False) == dest_dir.resolve(
|
||||
strict=False
|
||||
)
|
||||
except (OSError, RuntimeError):
|
||||
same_location = source_dir.absolute() == dest_dir.absolute()
|
||||
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/).
|
||||
@@ -1355,8 +1371,7 @@ class ExtensionManager:
|
||||
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)
|
||||
|
||||
|
||||
@@ -1118,6 +1118,56 @@ class TestExtensionManager:
|
||||
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_from_install_dir_is_rejected_when_resolve_fails(
|
||||
self, extension_dir, project_dir, monkeypatch
|
||||
):
|
||||
"""Resolution failures must not bypass the self-install guard."""
|
||||
manager = ExtensionManager(project_dir)
|
||||
|
||||
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
|
||||
install_dir = project_dir / ".specify" / "extensions" / "test-ext"
|
||||
|
||||
original_resolve = Path.resolve
|
||||
|
||||
def fail_resolve(self, *args, **kwargs):
|
||||
if self in {install_dir, manager.extensions_dir / "test-ext"}:
|
||||
raise OSError("cannot resolve path")
|
||||
return original_resolve(self, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(Path, "resolve", fail_resolve)
|
||||
|
||||
with pytest.raises(ValidationError, match="install destination"):
|
||||
manager.install_from_directory(
|
||||
install_dir, "0.1.0", register_commands=False, force=True
|
||||
)
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user