mirror of
https://github.com/github/spec-kit.git
synced 2026-08-15 14:02:04 +02:00
fix(extensions): reject reinstall when a kept config cannot be read (#3960)
The keep-config rescue branch of install_from_directory() reads each preserved config with bare read_bytes()/stat() calls, so a kept config that cannot be read (permission or I/O error) crashed the reinstall with a raw OSError. The sibling symlink guard four lines above already rejects with a ValidationError and resolution guidance for the same reason: bytes that cannot be safely rescued must not reach the rmtree below. Wrap the read and raise ValidationError with guidance, while dest_dir is still untouched so the preserved bytes are never lost. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -2186,10 +2186,24 @@ class ExtensionManager:
|
||||
"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
|
||||
|
||||
@@ -1601,6 +1601,57 @@ class TestExtensionManager:
|
||||
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
|
||||
):
|
||||
|
||||
Reference in New Issue
Block a user