Skip to content

Commit 52bacea

Browse files
committed
fix: propagate extension removal refusal to bundle uninstall
_ExtensionKindManager.remove() discarded ExtensionManager.remove()'s bool result, so a tampered registry id or symlinked target that made the security guard refuse to act still let remove_bundle() record the component as uninstalled and drop the bundle's ownership metadata, leaving the extension on disk. Raise BundlerError when remove() returns False so the bundle removal fails instead of silently succeeding. Also add a regression test covering the previously-untested regular-file preset target rejection in PresetManager.remove().
1 parent fd86df8 commit 52bacea

3 files changed

Lines changed: 47 additions & 1 deletion

File tree

‎src/specify_cli/bundles/primitives.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,11 +313,16 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None:
313313

314314
def remove(self, component: ComponentRef) -> None:
315315
try:
316-
self._manager.remove(component.id)
316+
removed = self._manager.remove(component.id)
317317
except Exception as exc: # noqa: BLE001
318318
raise BundlerError(
319319
f"Failed to remove extension '{component.id}': {exc}"
320320
) from exc
321+
if not removed:
322+
raise BundlerError(
323+
f"Failed to remove extension '{component.id}': removal was "
324+
"refused (unsafe registry id or symlinked target)."
325+
)
321326

322327

323328
class _WorkflowKindManager:

‎tests/specify_cli/bundles/test_primitives.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,31 @@ def test_default_installer_threads_allow_network(tmp_path: Path):
7070
installer.install(tmp_path, _component("workflows"))
7171

7272

73+
def test_extension_remove_propagates_refusal_as_bundler_error(tmp_path: Path):
74+
"""A tampered registry/symlinked target must fail the bundle removal, not
75+
silently report success while leaving the extension installed on disk.
76+
77+
``ExtensionManager.remove()`` returns ``False`` when it refuses to act on
78+
an unsafe target; the bundle adapter must surface that refusal instead of
79+
discarding it, or ``remove_bundle()`` would record the component as
80+
uninstalled while it is still on disk.
81+
"""
82+
manager = primitive_manager("extensions", tmp_path)
83+
manager._manager.registry.add("test-ext", {"version": "1.0.0"})
84+
85+
ext_dir = tmp_path / ".specify" / "extensions" / "test-ext"
86+
ext_dir.parent.mkdir(parents=True, exist_ok=True)
87+
real_target = tmp_path / "real-target"
88+
real_target.mkdir()
89+
ext_dir.symlink_to(real_target, target_is_directory=True)
90+
91+
with pytest.raises(BundlerError, match="removal was refused"):
92+
manager.remove(_component("extensions", "test-ext"))
93+
94+
assert ext_dir.is_symlink()
95+
assert manager._manager.registry.is_installed("test-ext")
96+
97+
7398
@pytest.mark.parametrize("kind", ["presets", "extensions", "workflows", "steps"])
7499
def test_offline_refresh_explains_component_needs_network(tmp_path: Path, kind: str):
75100
installer = DefaultPrimitiveInstaller(allow_network=False)

‎tests/specify_cli/presets/test_manager.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,22 @@ def test_remove_refuses_dangling_symlinked_preset_dir(self, project_dir):
355355
assert pack_dir.is_symlink()
356356
assert manager.registry.is_installed("test-pack")
357357

358+
def test_remove_refuses_regular_file_preset_dir(self, project_dir):
359+
"""A regular file at the preset path must fail explicitly, not crash
360+
``shutil.rmtree`` or leave the registry mutated."""
361+
manager = PresetManager(project_dir)
362+
manager.registry.add("test-pack", {"version": "1.0.0"})
363+
364+
pack_dir = project_dir / ".specify" / "presets" / "test-pack"
365+
pack_dir.parent.mkdir(parents=True, exist_ok=True)
366+
pack_dir.write_text("not a directory")
367+
368+
result = manager.remove("test-pack")
369+
370+
assert result is False
371+
assert pack_dir.is_file()
372+
assert manager.registry.is_installed("test-pack")
373+
358374
def test_list_installed(self, project_dir, pack_dir):
359375
"""Test listing installed packs."""
360376
manager = PresetManager(project_dir)

0 commit comments

Comments
 (0)