diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 6d78354809..086a9841c6 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -2186,10 +2186,24 @@ def _matches_source_config_baseline(config_name: str) -> bool: "a regular file or remove it — then reinstall." ) if cfg_file.is_file(): - stranded_configs[cfg_file.name] = ( - cfg_file.read_bytes(), - cfg_file.stat().st_mode, - ) + # A kept config that cannot be read or stat'ed must not + # crash the reinstall with a raw OSError — and must not + # reach the rmtree below unrescued. Like the symlink + # guard above, reject while dest_dir is untouched so the + # preserved bytes are never lost. + try: + stranded_configs[cfg_file.name] = ( + cfg_file.read_bytes(), + cfg_file.stat().st_mode, + ) + except OSError as exc: + raise ValidationError( + "Preserved extension config for " + f"'{manifest.id}' cannot be read " + f"({cfg_file.name}) in {dest_dir}: {exc}. " + "Resolve manually — fix its permissions or " + "remove it — then reinstall." + ) from exc if stranded_configs and not staging_is_complete: # Write a durable backup outside dest_dir before any diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 3d9146d52b..f72543949e 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -1601,6 +1601,57 @@ def test_reinstall_with_symlinked_config_rejects_install( assert external_target.read_text() == "model: linked-model\n" assert not manager.registry.is_installed("test-ext") + def test_reinstall_with_unreadable_kept_config_aborts_with_guidance( + self, extension_dir, project_dir, monkeypatch + ): + """An unreadable kept config must abort reinstall, not crash it. + + The sibling symlink guard four lines above raises ``ValidationError`` + with resolution guidance, but the rescue read itself + (``cfg_file.read_bytes()``/``stat()``) had no boundary, so a kept + config that cannot be read (permission or I/O error) crashed the + reinstall with a raw ``OSError``. It must reject the reinstall while + dest_dir is untouched so the preserved bytes are never rescued + half-read or lost to the rmtree below. + """ + manager = ExtensionManager(project_dir) + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + 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("model: custom-model\nmax_iterations: 99\n") + kept_bytes = config_file.read_bytes() + + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + assert config_file.is_file() + + # Simulate a kept config that can no longer be read (e.g. a + # permission or I/O error) without touching real permissions so the + # test also runs on platforms where chmod is a no-op. + original_read_bytes = Path.read_bytes + + def failing_read_bytes(self_path, *args, **kwargs): + if self_path == config_file: + raise PermissionError(13, "Permission denied") + return original_read_bytes(self_path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_bytes", failing_read_bytes) + + with pytest.raises(ValidationError, match="cannot be read"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # The kept config survives untouched; nothing was rescued half-read. + monkeypatch.undo() + assert config_file.read_bytes() == kept_bytes + assert not manager.registry.is_installed("test-ext") + def test_retry_with_symlinked_live_config_aborts_and_preserves_both( self, extension_dir, project_dir, monkeypatch ):