diff --git a/src/specify_cli/bundles/primitives.py b/src/specify_cli/bundles/primitives.py index b32342e68d..2da9db5a95 100644 --- a/src/specify_cli/bundles/primitives.py +++ b/src/specify_cli/bundles/primitives.py @@ -152,7 +152,7 @@ def __init__(self, project_root: Path, allow_network: bool) -> None: def is_installed(self, component: ComponentRef) -> bool: try: - return self._manager.get_pack(component.id) is not None + return self._manager.registry.is_installed(component.id) except Exception: # noqa: BLE001 return False @@ -222,11 +222,16 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: def remove(self, component: ComponentRef) -> None: try: - self._manager.remove(component.id) + removed = self._manager.remove(component.id) except Exception as exc: # noqa: BLE001 raise BundlerError( f"Failed to remove preset '{component.id}': {exc}" ) from exc + if not removed: + raise BundlerError( + f"Failed to remove preset '{component.id}': removal was " + "refused (unsafe registry id or unexpected on-disk target)." + ) class _ExtensionKindManager: @@ -313,11 +318,16 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: def remove(self, component: ComponentRef) -> None: try: - self._manager.remove(component.id) + removed = self._manager.remove(component.id) except Exception as exc: # noqa: BLE001 raise BundlerError( f"Failed to remove extension '{component.id}': {exc}" ) from exc + if not removed: + raise BundlerError( + f"Failed to remove extension '{component.id}': removal was " + "refused (unsafe registry id or unexpected on-disk target)." + ) class _WorkflowKindManager: diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index e4b9e7de9d..909a00bfcc 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -317,7 +317,7 @@ def _validate(self): ) # Validate extension ID format - if not re.match(r"^[a-z0-9-]+$", ext["id"]): + if not re.fullmatch(r"[a-z0-9-]+", ext["id"]): raise ValidationError( f"Invalid extension ID '{ext['id']}': " "must be lowercase alphanumeric with hyphens only" @@ -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(): @@ -2168,6 +2173,11 @@ def install_from_directory( elif backup_config_dir.exists(): backup_config_dir.unlink() did_remove = self.remove(manifest.id) + if not did_remove: + raise ExtensionError( + f"Refusing to reinstall '{manifest.id}': existing " + f"installation could not be safely removed" + ) # Load and validate .extensionignore BEFORE reading/creating the rescue # staging directory (and thus before deleting dest_dir). The loader can @@ -2960,6 +2970,35 @@ def remove(self, extension_id: str, keep_config: bool = False) -> bool: if not self.registry.is_installed(extension_id): return False + # A registered extension_id is trusted only as far as the registry + # file itself is trustworthy; validate it as a single, well-formed + # path component before it is used to construct removal/backup + # targets, and refuse a target that is not a real directory (e.g. a + # symlink planted to redirect the deletion elsewhere). + if not VALID_EXTENSION_ARTIFACT_NAME_PATTERN.fullmatch(extension_id): + return False + extension_dir = self.extensions_dir / extension_id + if extension_dir.is_symlink() or ( + extension_dir.exists() and not extension_dir.is_dir() + ): + return False + backup_root = self.extensions_dir / ".backup" + backup_dir = backup_root / extension_id + if not keep_config and extension_dir.exists(): + if ( + backup_root.is_symlink() + or backup_dir.is_symlink() + or (backup_root.exists() and not backup_root.is_dir()) + or (backup_dir.exists() and not backup_dir.is_dir()) + or any( + (backup_dir / f.name).is_symlink() + or (backup_dir / f.name).is_dir() + for f in list(extension_dir.glob("*-config.yml")) + + list(extension_dir.glob("*-config.local.yml")) + ) + ): + return False + # Get registered commands and skills before removal metadata = self.registry.get(extension_id) registered_commands = ( @@ -2972,8 +3011,6 @@ def remove(self, extension_id: str, keep_config: bool = False) -> bool: else: registered_skills = [] - extension_dir = self.extensions_dir / extension_id - # Unregister commands from all AI agents if registered_commands: registrar = CommandRegistrar() @@ -3007,7 +3044,6 @@ def remove(self, extension_id: str, keep_config: bool = False) -> bool: if extension_dir.exists(): # Use subdirectory per extension to avoid name accumulation # (e.g., jira-jira-config.yml on repeated remove/install cycles) - backup_dir = self.extensions_dir / ".backup" / extension_id backup_dir.mkdir(parents=True, exist_ok=True) # Backup both primary and local override config files diff --git a/src/specify_cli/extensions/_command_update_transaction.py b/src/specify_cli/extensions/_command_update_transaction.py index b5dd995c1c..5e259d9716 100644 --- a/src/specify_cli/extensions/_command_update_transaction.py +++ b/src/specify_cli/extensions/_command_update_transaction.py @@ -520,7 +520,11 @@ def backup_extension_skills(skill_names, *, skills_dir=None): # 7. Remove old extension (handles command file cleanup and registry removal) installation_modified = True - manager.remove(extension_id, keep_config=True) + if manager.remove(extension_id, keep_config=True) is False: + installation_modified = False + raise RuntimeError( + f"Could not safely remove existing '{extension_id}'" + ) # 8. Install new version _ = manager.install_from_zip( diff --git a/src/specify_cli/presets/_manager.py b/src/specify_cli/presets/_manager.py index 33274dfdfc..149a6d3f1e 100644 --- a/src/specify_cli/presets/_manager.py +++ b/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(): @@ -640,6 +644,17 @@ def remove(self, pack_id: str) -> bool: if not self.registry.is_installed(pack_id): return False + # A registered pack_id is trusted only as far as the registry file + # itself is trustworthy; validate it as a single, well-formed path + # component before it is used to construct the removal target, and + # refuse a target that is not a real directory (e.g. a symlink + # planted to redirect the deletion elsewhere). + if not PresetResolver._is_safe_registry_id(pack_id): + return False + pack_dir = self.presets_dir / pack_id + if pack_dir.is_symlink() or (pack_dir.exists() and not pack_dir.is_dir()): + return False + metadata = self.registry.get(pack_id) # Restore original skills when preset is removed registered_skills = metadata.get("registered_skills", []) if metadata else [] @@ -668,7 +683,6 @@ def remove(self, pack_id: str) -> bool: fallback_agent=fallback_agent, ) registered_commands = metadata.get("registered_commands", {}) if metadata else {} - pack_dir = self.presets_dir / pack_id # Record which historical agents this preset's registered_commands # actually targeted, *before* any filtering below, so post-removal diff --git a/src/specify_cli/presets/_manifest.py b/src/specify_cli/presets/_manifest.py index b5a7ff03c3..e75ecb7966 100644 --- a/src/specify_cli/presets/_manifest.py +++ b/src/specify_cli/presets/_manifest.py @@ -117,7 +117,7 @@ def _validate(self): ) # Validate pack ID format - if not re.match(r'^[a-z0-9-]+$', pack["id"]): + if not re.fullmatch(r'[a-z0-9-]+', pack["id"]): raise PresetValidationError( f"Invalid preset ID '{pack['id']}': " "must be lowercase alphanumeric with hyphens only" diff --git a/tests/specify_cli/bundles/test_primitives.py b/tests/specify_cli/bundles/test_primitives.py index a3b4d83f45..472face0d3 100644 --- a/tests/specify_cli/bundles/test_primitives.py +++ b/tests/specify_cli/bundles/test_primitives.py @@ -70,6 +70,67 @@ def test_default_installer_threads_allow_network(tmp_path: Path): installer.install(tmp_path, _component("workflows")) +def test_extension_remove_propagates_refusal_as_bundler_error(tmp_path: Path): + """A tampered registry/symlinked target must fail the bundle removal, not + silently report success while leaving the extension installed on disk. + + ``ExtensionManager.remove()`` returns ``False`` when it refuses to act on + an unsafe target; the bundle adapter must surface that refusal instead of + discarding it, or ``remove_bundle()`` would record the component as + uninstalled while it is still on disk. + """ + manager = primitive_manager("extensions", tmp_path) + manager._manager.registry.add("test-ext", {"version": "1.0.0"}) + + ext_dir = tmp_path / ".specify" / "extensions" / "test-ext" + ext_dir.parent.mkdir(parents=True, exist_ok=True) + real_target = tmp_path / "real-target" + real_target.mkdir() + ext_dir.symlink_to(real_target, target_is_directory=True) + + with pytest.raises(BundlerError, match="removal was refused"): + manager.remove(_component("extensions", "test-ext")) + + assert ext_dir.is_symlink() + assert manager._manager.registry.is_installed("test-ext") + + +def test_preset_remove_propagates_refusal_as_bundler_error(tmp_path: Path): + """Mirrors ``test_extension_remove_propagates_refusal_as_bundler_error``: + ``PresetManager.remove()`` also returns ``False`` on an unsafe target, and + the bundle adapter must surface that refusal rather than discarding it. + """ + manager = primitive_manager("presets", tmp_path) + manager._manager.registry.add("test-preset", {"version": "1.0.0"}) + + pack_dir = tmp_path / ".specify" / "presets" / "test-preset" + pack_dir.parent.mkdir(parents=True, exist_ok=True) + real_target = tmp_path / "real-target" + real_target.mkdir() + pack_dir.symlink_to(real_target, target_is_directory=True) + + with pytest.raises(BundlerError, match="removal was refused"): + manager.remove(_component("presets", "test-preset")) + + assert pack_dir.is_symlink() + assert manager._manager.registry.is_installed("test-preset") + + +def test_preset_is_installed_uses_registry_for_corrupt_symlink_target(tmp_path: Path): + """``remove_bundle()`` only calls ``remove()`` when ``is_installed()`` is + true, so a registered preset behind a symlink must still count as installed. + """ + manager = primitive_manager("presets", tmp_path) + manager._manager.registry.add("test-preset", {"version": "1.0.0"}) + pack_dir = tmp_path / ".specify" / "presets" / "test-preset" + pack_dir.parent.mkdir(parents=True, exist_ok=True) + real_target = tmp_path / "real-target" + real_target.mkdir() + pack_dir.symlink_to(real_target, target_is_directory=True) + + assert manager.is_installed(_component("presets", "test-preset")) + + @pytest.mark.parametrize("kind", ["presets", "extensions", "workflows", "steps"]) def test_offline_refresh_explains_component_needs_network(tmp_path: Path, kind: str): installer = DefaultPrimitiveInstaller(allow_network=False) diff --git a/tests/specify_cli/extensions/test_command_update_transaction.py b/tests/specify_cli/extensions/test_command_update_transaction.py index 28efb46833..c67188302b 100644 --- a/tests/specify_cli/extensions/test_command_update_transaction.py +++ b/tests/specify_cli/extensions/test_command_update_transaction.py @@ -2228,3 +2228,25 @@ def fail_partial_skill_backup(src, dst, *args, **kwargs): assert first_support.read_text(encoding="utf-8") == "FIRST SUPPORT" assert second_support.read_text(encoding="utf-8") == "SECOND SUPPORT" assert _update_backup_dirs(project_dir) == [] + + + +def test_extension_update_aborts_when_removal_refused(project_dir, monkeypatch): + """A refused remove() (returns False) must abort the update before install.""" + monkeypatch.chdir(project_dir) + monkeypatch.setattr(ExtensionManager, "list_installed", lambda self: [{"id": "test-ext", "name": "Test Ext", "version": "1.0.0"}]) + monkeypatch.setattr(ExtensionRegistry, "get", lambda self, ext_id: {"version": "1.0.0", "enabled": True}) + mock_zip = project_dir / "mock.zip" + _write_update_zip(mock_zip) + monkeypatch.setattr(ExtensionCatalog, "download_extension", lambda self, ext_id: mock_zip) + monkeypatch.setattr(ExtensionCatalog, "get_extension_info", lambda self, ext_id: {"id": "test-ext", "name": "Test Ext", "version": "1.1.0", "download_url": "https://example.com/ext.zip"}) + monkeypatch.setattr(ExtensionManager, "remove", lambda self, ext_id, keep_config=False: False) + installs = [] + monkeypatch.setattr(ExtensionManager, "install_from_zip", lambda *a, **k: installs.append(1)) + monkeypatch.setattr("typer.confirm", lambda _: True) + + result = runner.invoke(app, ["extension", "update", "test-ext"], obj={"project_root": project_dir}) + + assert result.exit_code == 1 + assert "Could not safely remove" in result.output + assert installs == [] diff --git a/tests/specify_cli/presets/test_manager.py b/tests/specify_cli/presets/test_manager.py index 91e0805593..3528578485 100644 --- a/tests/specify_cli/presets/test_manager.py +++ b/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 @@ -291,6 +292,107 @@ def test_remove_nonexistent(self, project_dir): result = manager.remove("nonexistent") assert result is False + def test_remove_rejects_unsafe_registry_id(self, project_dir): + """A tampered registry entry must not reach path construction. + + The registry file is user-editable JSON; an id like ``../outside`` + would otherwise let ``remove()`` build a deletion target outside + ``.specify/presets``. + """ + manager = PresetManager(project_dir) + unsafe_id = "../outside-target" + manager.registry.add(unsafe_id, {"version": "1.0.0"}) + assert manager.registry.is_installed(unsafe_id) + + outside_target = project_dir / ".specify" / "outside-target" + outside_target.mkdir() + (outside_target / "keep.txt").write_text("do not delete") + + result = manager.remove(unsafe_id) + + assert result is False + assert outside_target.exists() + assert (outside_target / "keep.txt").exists() + # Refused before the registry entry was mutated. + assert manager.registry.is_installed(unsafe_id) + + def test_remove_refuses_symlinked_preset_dir(self, project_dir): + """A symlinked preset directory must fail explicitly, not be deleted.""" + manager = PresetManager(project_dir) + manager.registry.add("test-pack", {"version": "1.0.0"}) + + real_target = project_dir.parent / "real-target" + real_target.mkdir() + (real_target / "important.txt").write_text("do not delete") + + pack_dir = project_dir / ".specify" / "presets" / "test-pack" + pack_dir.symlink_to(real_target, target_is_directory=True) + + result = manager.remove("test-pack") + + assert result is False + assert real_target.exists() + assert (real_target / "important.txt").exists() + 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_registered_preset_with_missing_dir(self, project_dir): + """A valid registered preset whose directory is already gone is removed.""" + manager = PresetManager(project_dir) + manager.registry.add("test-pack", {"version": "1.0.0"}) + + assert manager.remove("test-pack") is True + assert not manager.registry.is_installed("test-pack") + + def test_remove_refuses_dangling_symlinked_preset_dir(self, project_dir): + """A dangling symlink must fail explicitly rather than be silently skipped. + + ``Path.exists()`` follows the link and returns False for a broken + symlink, so a guard gated on ``exists()`` would let this through. + """ + manager = PresetManager(project_dir) + manager.registry.add("test-pack", {"version": "1.0.0"}) + + pack_dir = project_dir / ".specify" / "presets" / "test-pack" + pack_dir.parent.mkdir(parents=True, exist_ok=True) + missing_target = project_dir.parent / "does-not-exist" + pack_dir.symlink_to(missing_target, target_is_directory=True) + + result = manager.remove("test-pack") + + assert result is False + assert pack_dir.is_symlink() + assert manager.registry.is_installed("test-pack") + + def test_remove_refuses_regular_file_preset_dir(self, project_dir): + """A regular file at the preset path must fail explicitly, not crash + ``shutil.rmtree`` or leave the registry mutated.""" + manager = PresetManager(project_dir) + manager.registry.add("test-pack", {"version": "1.0.0"}) + + pack_dir = project_dir / ".specify" / "presets" / "test-pack" + pack_dir.parent.mkdir(parents=True, exist_ok=True) + pack_dir.write_text("not a directory") + + result = manager.remove("test-pack") + + assert result is False + assert pack_dir.is_file() + assert manager.registry.is_installed("test-pack") + def test_list_installed(self, project_dir, pack_dir): """Test listing installed packs.""" manager = PresetManager(project_dir) diff --git a/tests/specify_cli/presets/test_manifest.py b/tests/specify_cli/presets/test_manifest.py index 27ac723daa..e2fb3bfd89 100644 --- a/tests/specify_cli/presets/test_manifest.py +++ b/tests/specify_cli/presets/test_manifest.py @@ -199,9 +199,10 @@ def test_missing_pack_id(self, temp_dir, valid_pack_data): with pytest.raises(PresetValidationError, match="Missing preset.id"): PresetManifest(manifest_path) - def test_invalid_pack_id_format(self, temp_dir, valid_pack_data): - """Test invalid pack ID format.""" - valid_pack_data["preset"]["id"] = "Invalid_ID" + @pytest.mark.parametrize("bad_id", ["Invalid_ID", "test-pack\n"]) + def test_invalid_pack_id_format(self, temp_dir, valid_pack_data, bad_id): + """Test invalid pack ID format (incl. trailing newline).""" + valid_pack_data["preset"]["id"] = bad_id manifest_path = temp_dir / "preset.yml" with open(manifest_path, 'w') as f: yaml.dump(valid_pack_data, f) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index b512742389..8a2a35a999 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -384,11 +384,12 @@ def test_invalid_utf8_bytes_raises_validation_error(self, temp_dir): with pytest.raises(ValidationError, match="not valid UTF-8"): ExtensionManifest(manifest_path) - def test_invalid_extension_id(self, temp_dir, valid_manifest_data): - """Test manifest with invalid extension ID format.""" + @pytest.mark.parametrize("bad_id", ["Invalid_ID", "test-ext\n"]) + def test_invalid_extension_id(self, temp_dir, valid_manifest_data, bad_id): + """Test manifest with invalid extension ID format (incl. trailing newline).""" import yaml - valid_manifest_data["extension"]["id"] = "Invalid_ID" # Uppercase not allowed + valid_manifest_data["extension"]["id"] = bad_id manifest_path = temp_dir / "extension.yml" with open(manifest_path, 'w') as f: @@ -3277,6 +3278,241 @@ def test_config_backup_on_remove(self, extension_dir, project_dir): assert backup_file.exists() assert backup_file.read_text() == "test: config" + def test_remove_rejects_unsafe_registry_id(self, project_dir): + """A tampered registry entry must not reach path construction. + + The registry file is user-editable JSON; an id like ``../outside`` + would otherwise let ``remove()`` build a deletion target outside + ``.specify/extensions``. + """ + manager = ExtensionManager(project_dir) + unsafe_id = "../outside-target" + manager.registry.add(unsafe_id, {"version": "1.0.0"}) + assert manager.registry.is_installed(unsafe_id) + + outside_target = project_dir / ".specify" / "outside-target" + outside_target.mkdir() + (outside_target / "keep.txt").write_text("do not delete") + + result = manager.remove(unsafe_id) + + assert result is False + assert outside_target.exists() + assert (outside_target / "keep.txt").exists() + # Refused before the registry entry was mutated. + assert manager.registry.is_installed(unsafe_id) + + def test_remove_refuses_symlinked_extension_dir(self, project_dir): + """A symlinked extension directory must fail explicitly, not be deleted.""" + manager = ExtensionManager(project_dir) + manager.registry.add("test-ext", {"version": "1.0.0"}) + + real_target = project_dir.parent / "real-target" + real_target.mkdir() + (real_target / "important.txt").write_text("do not delete") + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + ext_dir.symlink_to(real_target, target_is_directory=True) + + result = manager.remove("test-ext") + + assert result is False + assert real_target.exists() + assert (real_target / "important.txt").exists() + assert ext_dir.is_symlink() + assert manager.registry.is_installed("test-ext") + + def test_force_reinstall_aborts_when_removal_refused(self, extension_dir, project_dir): + """install(force=True) must abort if remove() refuses a symlinked target.""" + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + real_target = project_dir.parent / "real-target" + shutil.move(str(ext_dir), str(real_target)) + ext_dir.symlink_to(real_target, target_is_directory=True) + + with pytest.raises(ExtensionError, match="could not be safely removed"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False, force=True + ) + 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) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("test: config") + + backup_root = project_dir / ".specify" / "extensions" / ".backup" + backup_root.mkdir(parents=True, exist_ok=True) + outside_target = project_dir.parent / "outside-backup" + outside_target.mkdir() + (backup_root / "test-ext").symlink_to(outside_target, target_is_directory=True) + + result = manager.remove("test-ext", keep_config=False) + + assert result is False + assert ext_dir.exists() + assert config_file.exists() + assert not (outside_target / "test-ext-config.yml").exists() + assert manager.registry.is_installed("test-ext") + + def test_remove_refuses_regular_file_backup_path(self, extension_dir, project_dir): + """A non-directory at the backup path must not cause partial removal.""" + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + backup_root = project_dir / ".specify" / "extensions" / ".backup" + backup_root.mkdir(parents=True, exist_ok=True) + (backup_root / "test-ext").write_text("not a directory") + + assert manager.remove("test-ext", keep_config=False) is False + assert ext_dir.exists() + assert manager.registry.is_installed("test-ext") + + def test_remove_refuses_symlinked_backup_config_file( + self, extension_dir, project_dir + ): + """A symlink at ``.backup//`` must not be written through.""" + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + (ext_dir / "test-ext-config.yml").write_text("a: 1\n") + backup_dir = ext_dir.parent / ".backup" / "test-ext" + backup_dir.mkdir(parents=True) + outside = project_dir.parent / "outside-target.yml" + outside.write_text("original") + (backup_dir / "test-ext-config.yml").symlink_to(outside) + + assert manager.remove("test-ext", keep_config=False) is False + assert outside.read_text() == "original" + assert ext_dir.exists() + assert manager.registry.is_installed("test-ext") + + def test_remove_refuses_directory_backup_config_path( + self, extension_dir, project_dir + ): + """A directory at ``.backup//`` must not be copied into.""" + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + (ext_dir / "test-ext-config.yml").write_text("a: 1\n") + backup_cfg = ext_dir.parent / ".backup" / "test-ext" / "test-ext-config.yml" + backup_cfg.mkdir(parents=True) + + assert manager.remove("test-ext", keep_config=False) is False + assert list(backup_cfg.iterdir()) == [] + assert ext_dir.exists() + assert manager.registry.is_installed("test-ext") + + def test_remove_refuses_regular_file_extension_dir(self, project_dir): + """A regular file at the extension path is preserved and not unregistered.""" + manager = ExtensionManager(project_dir) + manager.registry.add("test-ext", {"version": "1.0.0"}) + ext_path = project_dir / ".specify" / "extensions" / "test-ext" + ext_path.parent.mkdir(parents=True, exist_ok=True) + ext_path.write_text("not a directory") + + assert manager.remove("test-ext") is False + assert ext_path.read_text() == "not a directory" + assert manager.registry.is_installed("test-ext") + + def test_remove_refuses_dangling_symlinked_extension_dir(self, project_dir): + """A dangling symlink must fail explicitly rather than be silently skipped. + + ``Path.exists()`` follows the link and returns False for a broken + symlink, so a guard gated on ``exists()`` would let this through. + """ + manager = ExtensionManager(project_dir) + manager.registry.add("test-ext", {"version": "1.0.0"}) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + ext_dir.parent.mkdir(parents=True, exist_ok=True) + missing_target = project_dir.parent / "does-not-exist" + ext_dir.symlink_to(missing_target, target_is_directory=True) + + result = manager.remove("test-ext") + + assert result is False + assert ext_dir.is_symlink() + assert manager.registry.is_installed("test-ext") + + def test_remove_refuses_symlinked_backup_root(self, extension_dir, project_dir): + """A symlinked ``.backup`` parent must not redirect config backups.""" + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("test: config") + + outside_target = project_dir.parent / "outside-backup-root" + outside_target.mkdir() + backup_root = project_dir / ".specify" / "extensions" / ".backup" + backup_root.symlink_to(outside_target, target_is_directory=True) + + result = manager.remove("test-ext", keep_config=False) + + assert result is False + assert ext_dir.exists() + assert config_file.exists() + assert not (outside_target / "test-ext" / "test-ext-config.yml").exists() + assert manager.registry.is_installed("test-ext") + + def test_remove_keep_config_ignores_symlinked_backup_root( + self, extension_dir, project_dir + ): + """``keep_config=True`` never writes a backup, so an unrelated symlink + at ``.backup`` must not block removal. + """ + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + outside_target = project_dir.parent / "outside-backup-root-keep-config" + outside_target.mkdir() + backup_root = project_dir / ".specify" / "extensions" / ".backup" + backup_root.symlink_to(outside_target, target_is_directory=True) + + result = manager.remove("test-ext", keep_config=True) + + assert result is True + assert not manager.registry.is_installed("test-ext") + + def test_remove_rejects_id_with_trailing_newline(self, project_dir): + """``fullmatch`` must be used so a trailing newline cannot slip past ``$``.""" + manager = ExtensionManager(project_dir) + unsafe_id = "test-ext\n" + manager.registry.add(unsafe_id, {"version": "1.0.0"}) + assert manager.registry.is_installed(unsafe_id) + + result = manager.remove(unsafe_id) + + assert result is False + assert manager.registry.is_installed(unsafe_id) + # ===== CommandRegistrar Tests =====