From 7b1065d8579a850a98faa747999439f03dcf8f80 Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Thu, 9 Jul 2026 17:46:22 +0500 Subject: [PATCH] fix(bundler): enforce version pin on bundled preset/extension installs (#3377) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 --------- Co-authored-by: Claude Fable 5 --- .../bundler/services/primitives.py | 55 ++++++++++-- tests/unit/test_bundler_primitives.py | 84 +++++++++++++++++++ 2 files changed, 133 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/bundler/services/primitives.py b/src/specify_cli/bundler/services/primitives.py index bd0d8ddee..229fb6137 100644 --- a/src/specify_cli/bundler/services/primitives.py +++ b/src/specify_cli/bundler/services/primitives.py @@ -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 ) diff --git a/tests/unit/test_bundler_primitives.py b/tests/unit/test_bundler_primitives.py index f662d22fc..9891e6f77 100644 --- a/tests/unit/test_bundler_primitives.py +++ b/tests/unit/test_bundler_primitives.py @@ -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