mirror of
https://github.com/github/spec-kit.git
synced 2026-07-10 09:44:41 +08:00
fix(bundler): enforce version pin on bundled preset/extension installs (#3377)
* fix(bundler): enforce version pin on bundled preset/extension installs The bundled install branch of _PresetKindManager/_ExtensionKindManager called install_from_directory and returned before _assert_pinned_version, so a bundle manifest pinning e.g. 2.0.0 would silently install the bundled asset's own version (1.0.0) — the pin was only enforced on the catalog path. _WorkflowKindManager already enforces it unconditionally. Read the bundled asset's declared version from its manifest (best-effort; None => cannot enforce, matching the catalog 'advertises no version' escape hatch) and call _assert_pinned_version before install, in both bundled branches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bundler): address review on bundled version-pin check - Make _assert_pinned_version's error source-agnostic ('resolved version' / 'the source') so bundled preset.yml/extension.yml mismatches read correctly, not just catalog ones. - Type-guard _bundled_manifest_version: only a non-empty string version is usable; missing/non-string/whitespace -> None ('cannot enforce'). - Add bundled-preset success-path test (matching pin + version=None both proceed to install_from_directory), mirroring the extension test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -33,12 +33,13 @@ DEFAULT_PRIORITY = 10
|
||||
def _assert_pinned_version(
|
||||
kind: str, component_id: str, pinned: str | None, advertised: object
|
||||
) -> None:
|
||||
"""Refuse to install when the catalog version differs from the manifest pin.
|
||||
"""Refuse to install when the resolved version differs from the manifest pin.
|
||||
|
||||
Bundle manifests pin component versions for reproducibility; installing
|
||||
whatever the active catalog currently serves would silently violate the
|
||||
pin. When the catalog advertises no version we cannot enforce the pin, so
|
||||
installation proceeds (the catalog, not the bundler, owns that gap).
|
||||
whatever the resolved source (catalog *or* bundled asset) provides would
|
||||
silently violate the pin. When the source advertises no version we cannot
|
||||
enforce the pin, so installation proceeds (the source, not the bundler,
|
||||
owns that gap).
|
||||
"""
|
||||
if not pinned or advertised is None:
|
||||
return
|
||||
@@ -54,11 +55,35 @@ def _assert_pinned_version(
|
||||
if not matches:
|
||||
raise BundlerError(
|
||||
f"{kind} '{component_id}' is pinned to version {pinned} in the bundle "
|
||||
f"manifest, but the active catalog serves {actual}. Update the bundle's "
|
||||
"pinned version or the catalog before installing."
|
||||
f"manifest, but the resolved version is {actual}. Update the bundle's "
|
||||
"pinned version or the source before installing."
|
||||
)
|
||||
|
||||
|
||||
def _bundled_manifest_version(manifest_path: Path, root_key: str) -> str | None:
|
||||
"""Best-effort read of a bundled asset's declared version from its manifest.
|
||||
|
||||
Returns ``None`` when the manifest is missing/unreadable/invalid, which
|
||||
``_assert_pinned_version`` treats as "cannot enforce" (proceed) — matching
|
||||
the catalog "advertises no version" escape hatch.
|
||||
"""
|
||||
try:
|
||||
import yaml
|
||||
|
||||
data = yaml.safe_load(manifest_path.read_text(encoding="utf-8"))
|
||||
if isinstance(data, dict):
|
||||
section = data.get(root_key)
|
||||
if isinstance(section, dict):
|
||||
version = section.get("version")
|
||||
# Only a non-empty string is a usable version; anything else
|
||||
# (missing / non-string / whitespace) means "cannot enforce".
|
||||
if isinstance(version, str) and version.strip():
|
||||
return version
|
||||
except Exception: # noqa: BLE001 - unreadable/invalid manifest: skip pin
|
||||
return None
|
||||
return None
|
||||
|
||||
|
||||
class _KindManager(Protocol):
|
||||
def is_installed(self, component: ComponentRef) -> bool: ...
|
||||
|
||||
@@ -134,6 +159,15 @@ class _PresetKindManager:
|
||||
|
||||
bundled = _locate_bundled_preset(component.id)
|
||||
if bundled is not None:
|
||||
# Enforce the manifest pin against the bundled asset's own version,
|
||||
# mirroring the catalog path below (the bundled path previously
|
||||
# skipped the pin entirely).
|
||||
_assert_pinned_version(
|
||||
"Preset",
|
||||
component.id,
|
||||
component.version,
|
||||
_bundled_manifest_version(bundled / "preset.yml", "preset"),
|
||||
)
|
||||
self._manager.install_from_directory(bundled, speckit_version, priority)
|
||||
return
|
||||
|
||||
@@ -198,6 +232,15 @@ class _ExtensionKindManager:
|
||||
|
||||
bundled = _locate_bundled_extension(component.id)
|
||||
if bundled is not None:
|
||||
# Enforce the manifest pin against the bundled asset's own version,
|
||||
# mirroring the catalog path below (the bundled path previously
|
||||
# skipped the pin entirely).
|
||||
_assert_pinned_version(
|
||||
"Extension",
|
||||
component.id,
|
||||
component.version,
|
||||
_bundled_manifest_version(bundled / "extension.yml", "extension"),
|
||||
)
|
||||
self._manager.install_from_directory(
|
||||
bundled, speckit_version, priority=priority
|
||||
)
|
||||
|
||||
@@ -131,3 +131,87 @@ def test_preset_install_preserves_explicit_zero_priority(tmp_path: Path, monkeyp
|
||||
|
||||
# An explicit priority of 0 must be passed through, not replaced by default.
|
||||
assert calls["priority"] == 0
|
||||
|
||||
|
||||
def _write_manifest(path: Path, root_key: str, version: str) -> Path:
|
||||
path.mkdir(parents=True, exist_ok=True)
|
||||
(path / f"{root_key}.yml").write_text(
|
||||
f"{root_key}:\n id: x\n version: {version}\n", encoding="utf-8"
|
||||
)
|
||||
return path
|
||||
|
||||
|
||||
def test_bundled_extension_pin_mismatch_refuses(tmp_path: Path, monkeypatch):
|
||||
"""A bundled extension whose version != the manifest pin must be refused
|
||||
(the bundled path previously skipped the pin the catalog path enforces)."""
|
||||
import specify_cli._assets as assets
|
||||
from specify_cli.extensions import ExtensionManager
|
||||
|
||||
bundled = _write_manifest(tmp_path / "ext", "extension", "1.0.0")
|
||||
monkeypatch.setattr(assets, "_locate_bundled_extension", lambda cid: bundled)
|
||||
called: list = []
|
||||
monkeypatch.setattr(
|
||||
ExtensionManager, "install_from_directory",
|
||||
lambda self, *a, **k: called.append(a),
|
||||
)
|
||||
|
||||
manager = primitive_manager("extensions", tmp_path, allow_network=False)
|
||||
with pytest.raises(BundlerError, match="pinned to version 2.0.0"):
|
||||
manager.install(ComponentRef(kind="extensions", id="my-ext", version="2.0.0"))
|
||||
assert called == [] # install must not proceed
|
||||
|
||||
|
||||
def test_bundled_extension_pin_match_installs(tmp_path: Path, monkeypatch):
|
||||
import specify_cli._assets as assets
|
||||
from specify_cli.extensions import ExtensionManager
|
||||
|
||||
bundled = _write_manifest(tmp_path / "ext", "extension", "1.0.0")
|
||||
monkeypatch.setattr(assets, "_locate_bundled_extension", lambda cid: bundled)
|
||||
called: list = []
|
||||
monkeypatch.setattr(
|
||||
ExtensionManager, "install_from_directory",
|
||||
lambda self, *a, **k: called.append(a),
|
||||
)
|
||||
|
||||
manager = primitive_manager("extensions", tmp_path, allow_network=False)
|
||||
# matching pin, and unpinned, both install cleanly
|
||||
manager.install(ComponentRef(kind="extensions", id="my-ext", version="1.0.0"))
|
||||
manager.install(ComponentRef(kind="extensions", id="my-ext", version=None))
|
||||
assert len(called) == 2
|
||||
|
||||
|
||||
def test_bundled_preset_pin_mismatch_refuses(tmp_path: Path, monkeypatch):
|
||||
import specify_cli._assets as assets
|
||||
from specify_cli.presets import PresetManager
|
||||
|
||||
bundled = _write_manifest(tmp_path / "preset", "preset", "1.0.0")
|
||||
monkeypatch.setattr(assets, "_locate_bundled_preset", lambda cid: bundled)
|
||||
called: list = []
|
||||
monkeypatch.setattr(
|
||||
PresetManager, "install_from_directory",
|
||||
lambda self, *a, **k: called.append(a),
|
||||
)
|
||||
|
||||
manager = primitive_manager("presets", tmp_path, allow_network=False)
|
||||
with pytest.raises(BundlerError, match="pinned to version 2.0.0"):
|
||||
manager.install(ComponentRef(kind="presets", id="my-preset", version="2.0.0"))
|
||||
assert called == []
|
||||
|
||||
|
||||
def test_bundled_preset_pin_match_installs(tmp_path: Path, monkeypatch):
|
||||
import specify_cli._assets as assets
|
||||
from specify_cli.presets import PresetManager
|
||||
|
||||
bundled = _write_manifest(tmp_path / "preset", "preset", "1.0.0")
|
||||
monkeypatch.setattr(assets, "_locate_bundled_preset", lambda cid: bundled)
|
||||
called: list = []
|
||||
monkeypatch.setattr(
|
||||
PresetManager, "install_from_directory",
|
||||
lambda self, *a, **k: called.append(a),
|
||||
)
|
||||
|
||||
manager = primitive_manager("presets", tmp_path, allow_network=False)
|
||||
# matching pin, and unpinned, both proceed to install
|
||||
manager.install(ComponentRef(kind="presets", id="my-preset", version="1.0.0"))
|
||||
manager.install(ComponentRef(kind="presets", id="my-preset", version=None))
|
||||
assert len(called) == 2
|
||||
|
||||
Reference in New Issue
Block a user