Skip to content

Commit de18d21

Browse files
jinaparkdevclaude
andauthored
fix: prevent extension self-install from deleting source dir (#2990) (#2991)
* 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>
1 parent 75aee19 commit de18d21

2 files changed

Lines changed: 67 additions & 2 deletions

File tree

src/specify_cli/extensions.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,6 +1337,22 @@ def install_from_directory(
13371337
# Reject manifests that would shadow core commands or installed extensions.
13381338
self._validate_install_conflicts(manifest)
13391339

1340+
# Refuse to install an extension from its own install destination — with
1341+
# --force this would delete the source before copying it (issue #2990).
1342+
dest_dir = self.extensions_dir / manifest.id
1343+
try:
1344+
same_location = source_dir.resolve(strict=False) == dest_dir.resolve(
1345+
strict=False
1346+
)
1347+
except (OSError, RuntimeError):
1348+
same_location = source_dir.absolute() == dest_dir.absolute()
1349+
if same_location:
1350+
raise ValidationError(
1351+
f"Source path is the install destination for '{manifest.id}' "
1352+
f"({dest_dir}). Refusing to proceed to avoid deleting the "
1353+
f"extension. Install from a copy in a different location instead."
1354+
)
1355+
13401356
# Remove existing installation AFTER all validations pass so that a
13411357
# validation failure doesn't leave the user with a half-uninstalled
13421358
# extension (configs stranded in .backup/).
@@ -1355,8 +1371,7 @@ def install_from_directory(
13551371
backup_config_dir.unlink()
13561372
did_remove = self.remove(manifest.id)
13571373

1358-
# Install extension
1359-
dest_dir = self.extensions_dir / manifest.id
1374+
# Install extension (dest_dir computed above during self-install guard)
13601375
if dest_dir.exists():
13611376
shutil.rmtree(dest_dir)
13621377

tests/test_extensions.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,56 @@ def test_install_force_without_existing(self, extension_dir, project_dir):
11181118
assert manifest.id == "test-ext"
11191119
assert manager.registry.is_installed("test-ext")
11201120

1121+
def test_install_from_install_dir_is_rejected_without_data_loss(
1122+
self, extension_dir, project_dir
1123+
):
1124+
"""Installing from an extension's own install dir must fail without
1125+
deleting it (regression for issue #2990)."""
1126+
manager = ExtensionManager(project_dir)
1127+
1128+
# Install once so the extension lives at its install destination.
1129+
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
1130+
install_dir = project_dir / ".specify" / "extensions" / "test-ext"
1131+
assert install_dir.exists()
1132+
1133+
# Re-installing from that same directory with --force must be rejected.
1134+
with pytest.raises(ValidationError, match="install destination"):
1135+
manager.install_from_directory(
1136+
install_dir, "0.1.0", register_commands=False, force=True
1137+
)
1138+
1139+
# The directory and its contents must be left intact (no data loss).
1140+
assert install_dir.exists()
1141+
assert (install_dir / "extension.yml").exists()
1142+
assert (install_dir / "commands" / "hello.md").exists()
1143+
1144+
def test_install_from_install_dir_is_rejected_when_resolve_fails(
1145+
self, extension_dir, project_dir, monkeypatch
1146+
):
1147+
"""Resolution failures must not bypass the self-install guard."""
1148+
manager = ExtensionManager(project_dir)
1149+
1150+
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
1151+
install_dir = project_dir / ".specify" / "extensions" / "test-ext"
1152+
1153+
original_resolve = Path.resolve
1154+
1155+
def fail_resolve(self, *args, **kwargs):
1156+
if self in {install_dir, manager.extensions_dir / "test-ext"}:
1157+
raise OSError("cannot resolve path")
1158+
return original_resolve(self, *args, **kwargs)
1159+
1160+
monkeypatch.setattr(Path, "resolve", fail_resolve)
1161+
1162+
with pytest.raises(ValidationError, match="install destination"):
1163+
manager.install_from_directory(
1164+
install_dir, "0.1.0", register_commands=False, force=True
1165+
)
1166+
1167+
assert install_dir.exists()
1168+
assert (install_dir / "extension.yml").exists()
1169+
assert (install_dir / "commands" / "hello.md").exists()
1170+
11211171
def test_install_zip_force_reinstall(self, extension_dir, project_dir):
11221172
"""Test force-reinstalling from ZIP when already installed."""
11231173
import zipfile

0 commit comments

Comments
 (0)