From 8ea2367b44fc370d30460557f7fbaf485902af3d Mon Sep 17 00:00:00 2001 From: Jakub Baranowski Date: Tue, 1 Sep 2026 19:35:34 +0200 Subject: [PATCH] fix(extensions): install bundled extension updates from the local package Bundled extensions (agent-context, git, assess) have no download URL, so `specify extension update` could offer a version bump it then failed to install: step 5 unconditionally called catalog.download_extension(), which errors out for catalog entries without a URL (#4345). Resolve the update source for bundled extensions from the copy shipped with the running spec-kit release instead: - `_bundled_update_source()` locates the local bundled copy and parses its manifest version. - `_archive_extension_directory()` packages that copy as a ZIP so the update flows through the identical hardened archive pipeline (bounded extraction, manifest preflight, ID/version checks, backup/rollback) rather than growing a second install path. Symlinks are never followed into the archive. - When the local copy lags the catalog (or is missing), the update is blocked with an explicit "upgrade spec-kit, then rerun" message instead of installing an intermediate version or crashing; when the local copy is newer than the catalog, it installs the local version. Tests pin the install-from-local-copy route, every blocked-update branch, the newer-local-copy case, archive content/symlink behavior, and execute-bit restoration through the archive install route (POSIX-only; install_from_directory's trailing ensure_executable_scripts() re-establishes modes that ZIP extraction drops). Part 1 of the series requested in review on #4351; refs #4345. Assisted-by: Claude Code (model: claude-fable-5) Co-Authored-By: Claude Fable 5 --- docs/reference/extensions.md | 2 + src/specify_cli/extensions/_commands.py | 110 +++++++++++- tests/test_extension_content_staleness.py | 134 ++++++++++++++ tests/test_extensions.py | 206 ++++++++++++++++++++++ 4 files changed, 446 insertions(+), 6 deletions(-) create mode 100644 tests/test_extension_content_staleness.py diff --git a/docs/reference/extensions.md b/docs/reference/extensions.md index 8de2c18c86..0473e72008 100644 --- a/docs/reference/extensions.md +++ b/docs/reference/extensions.md @@ -75,6 +75,8 @@ specify extension update [] Updates a specific extension, or all installed extensions if no name is given. +Bundled extensions (such as `agent-context` and `git`) have no download URL; their updates install from the copy shipped with the running spec-kit release. When the catalog advertises a newer version than your spec-kit release ships, the update is reported as requiring a spec-kit upgrade first. + ## Enable / Disable an Extension ```bash diff --git a/src/specify_cli/extensions/_commands.py b/src/specify_cli/extensions/_commands.py index 7f7933e934..f5125794c2 100644 --- a/src/specify_cli/extensions/_commands.py +++ b/src/specify_cli/extensions/_commands.py @@ -15,9 +15,12 @@ import stat import tempfile from pathlib import Path -from typing import Optional +from typing import Optional, TYPE_CHECKING from uuid import uuid4 +if TYPE_CHECKING: + from packaging.version import Version + import typer import yaml from rich.markup import escape as _escape_markup @@ -106,6 +109,58 @@ def _command_safe_id(raw_id: object, placeholder: str = "") -> str return placeholder +def _bundled_update_source(ext_id: str) -> tuple[Path, Version] | tuple[None, None]: + """Locate the local bundled copy of *ext_id* and its parsed version. + + Bundled extensions have no download URL, so an update can only come + from the copy shipped with the running spec-kit release — which may + lag the version the catalog on main advertises. Returns + ``(path, Version)`` when a valid local copy exists, ``(None, None)`` + otherwise. + """ + from . import ExtensionManifest, ValidationError + from packaging import version as pkg_version + + bundled_dir = _locate_bundled_extension(ext_id) + if bundled_dir is None: + return None, None + try: + manifest = ExtensionManifest(bundled_dir / "extension.yml") + return bundled_dir, pkg_version.Version(manifest.version) + except (ValidationError, pkg_version.InvalidVersion, OSError): + return None, None + + +def _archive_extension_directory(source_dir: Path) -> Path: + """Package an extension directory as a ZIP archive for the update flow. + + The update pipeline validates and installs archives (bounded + extraction, manifest preflight, ID/version checks, backup/rollback), + so a locally bundled extension is fed through that identical hardened + path rather than growing a second install code path. The caller + deletes the archive after the update, the same as a downloaded one. + """ + import zipfile + + fd, tmp_name = tempfile.mkstemp(prefix="speckit-bundled-update-", suffix=".zip") + try: + with os.fdopen(fd, "wb") as archive_file: + with zipfile.ZipFile(archive_file, "w", zipfile.ZIP_DEFLATED) as zf: + for path in sorted(source_dir.rglob("*")): + # Never follow symlinks: is_file() follows the target + # and ZipFile.write() reads its bytes, which would turn + # an out-of-tree target into a regular archive member + # before the hardened extractor ever sees it. + if path.is_symlink(): + continue + if path.is_file(): + zf.write(path, path.relative_to(source_dir).as_posix()) + except BaseException: + Path(tmp_name).unlink(missing_ok=True) + raise + return Path(tmp_name) + + def _refresh_events_and_warn(project_root: Path) -> None: """Refresh native event config and surface failures (R3). @@ -1622,6 +1677,7 @@ def extension_update( console.print("🔄 Checking for updates...\n") updates_available = [] + blocked_updates = [] for ext_id in extensions_to_update: safe_ext_id = _escape_markup(str(ext_id)) @@ -1658,20 +1714,55 @@ def extension_update( continue if catalog_version > installed_version: + download_url = ext_info.get("download_url") + bundled_dir = None + available_version = catalog_version + if ext_info.get("bundled") and not download_url: + # Bundled extensions cannot be downloaded; the update has + # to come from the copy shipped with the running spec-kit + # release, which may lag the catalog on main (#4345). + bundled_dir, bundled_version = _bundled_update_source(ext_id) + # Block whenever the local copy lags the catalog, not + # just when it lags the installation: installing an + # intermediate version would leave the project behind + # the catalog while reporting success, contrary to the + # documented "upgrade spec-kit first" behavior. + if bundled_dir is None or bundled_version < catalog_version: + local_desc = ( + f"only ships v{bundled_version}" + if bundled_dir is not None + else "does not ship a local copy" + ) + console.print( + f"⚠ {safe_ext_id}: v{catalog_version} is available, but this " + f"spec-kit release {local_desc} — upgrade spec-kit, then rerun " + f"'specify extension update'" + ) + blocked_updates.append(ext_id) + continue + available_version = bundled_version updates_available.append( { "id": ext_id, "name": ext_info.get("name", ext_id), # Display name for status messages "installed": str(installed_version), - "available": str(catalog_version), - "download_url": ext_info.get("download_url"), + "available": str(available_version), + "download_url": download_url, + "bundled_dir": bundled_dir, } ) else: console.print(f"✓ {safe_ext_id}: Up to date (v{installed_version})") if not updates_available: - console.print("\n[green]All extensions are up to date![/green]") + if blocked_updates: + console.print( + "\n[yellow]Update(s) exist but require a newer spec-kit " + "release — upgrade spec-kit, then rerun " + "'specify extension update'.[/yellow]" + ) + else: + console.print("\n[green]All extensions are up to date![/green]") raise typer.Exit(0) # Show available updates @@ -1968,8 +2059,15 @@ def backup_extension_skills(skill_names, *, skills_dir=None): if ext_hooks: backup_hooks[hook_name] = ext_hooks - # 5. Download new version - archive_path = catalog.download_extension(extension_id) + # 5. Acquire the new version. Bundled extensions install from + # the copy shipped with the running spec-kit release (they + # have no download URL); everything else downloads. Both are + # packaged as archives so the identical validation, + # backup/rollback, and install pipeline below applies. + if update.get("bundled_dir") is not None: + archive_path = _archive_extension_directory(update["bundled_dir"]) + else: + archive_path = catalog.download_extension(extension_id) try: # 6. Validate the archive and extension ID before modifying # the existing installation. The shared extractor applies diff --git a/tests/test_extension_content_staleness.py b/tests/test_extension_content_staleness.py new file mode 100644 index 0000000000..f76e041869 --- /dev/null +++ b/tests/test_extension_content_staleness.py @@ -0,0 +1,134 @@ +"""Tests for the bundled-extension local update route (#4345). + +Bundled extensions have no download URL, so `specify extension update` +installs them from the copy shipped with the running spec-kit release, +packaged by `_archive_extension_directory` into the same hardened +archive pipeline that downloaded updates use. These tests pin that +packaging step and its round trip through the archive installer. +""" + +from __future__ import annotations + +import os + +import pytest +import yaml +from pathlib import Path + +from specify_cli.extensions import ExtensionManager + + +def _create_extension_source( + base_dir: Path, name: str = "test-ext", version: str = "1.0.0" +) -> Path: + """Create a minimal installable extension source directory.""" + ext_dir = base_dir / name + ext_dir.mkdir(parents=True, exist_ok=True) + + manifest = { + "schema_version": "1.0", + "extension": { + "id": "test-ext", + "name": "Test Extension", + "version": version, + "description": "A test extension", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.test-ext.hello", + "file": "commands/hello.md", + "description": "Test command", + } + ] + }, + } + + (ext_dir / "extension.yml").write_text(yaml.dump(manifest, sort_keys=False)) + commands_dir = ext_dir / "commands" + commands_dir.mkdir(exist_ok=True) + (commands_dir / "hello.md").write_text("---\ndescription: Test\n---\n\n$ARGUMENTS\n") + scripts_dir = ext_dir / "scripts" + scripts_dir.mkdir(exist_ok=True) + (scripts_dir / "run.sh").write_text("#!/bin/sh\necho hello\n") + (ext_dir / "test-ext-config.yml").write_text("setting: default\n") + return ext_dir + + +def _make_project(tmp_path: Path) -> Path: + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".claude" / "skills").mkdir(parents=True) + return project_dir + + +class TestArchiveExtensionDirectory: + def test_archive_contains_regular_files_only(self, tmp_path): + import zipfile + + from specify_cli.extensions._commands import _archive_extension_directory + + ext_dir = _create_extension_source(tmp_path) + archive_path = _archive_extension_directory(ext_dir) + try: + with zipfile.ZipFile(archive_path) as zf: + names = set(zf.namelist()) + assert "extension.yml" in names + assert "commands/hello.md" in names + finally: + archive_path.unlink() + + def test_archive_never_follows_symlinks(self, tmp_path): + """A symlink in the source must not pull out-of-tree bytes into the + archive before the hardened extractor sees it.""" + import zipfile + + from specify_cli.extensions._commands import _archive_extension_directory + + ext_dir = _create_extension_source(tmp_path) + outside = tmp_path / "outside.txt" + outside.write_text("external bytes\n") + try: + (ext_dir / "scripts" / "link.txt").symlink_to(outside) + except OSError: + pytest.skip("symlink creation requires privileges on this platform") + + archive_path = _archive_extension_directory(ext_dir) + try: + with zipfile.ZipFile(archive_path) as zf: + names = set(zf.namelist()) + assert "scripts/link.txt" not in names + finally: + archive_path.unlink() + + @pytest.mark.skipif( + os.name == "nt", reason="POSIX execute bits do not exist on Windows" + ) + def test_archive_route_restores_script_execute_bits(self, tmp_path): + """safe_extract_archive writes members without their recorded ZIP + modes, so the archive install route depends on install_from_directory's + trailing ensure_executable_scripts() call to keep documented + `.specify/extensions//scripts/*.sh` invocations executable. Pin + that round trip so removing the restoration would fail here instead + of surfacing as `Permission denied` after a bundled update.""" + from specify_cli.extensions._commands import _archive_extension_directory + + project_dir = _make_project(tmp_path) + source = _create_extension_source(tmp_path) + (source / "scripts" / "run.sh").chmod(0o755) + + archive_path = _archive_extension_directory(source) + try: + ExtensionManager(project_dir).install_from_zip(archive_path, "0.1.0") + finally: + archive_path.unlink() + + installed_script = ( + project_dir / ".specify" / "extensions" / "test-ext" / "scripts" / "run.sh" + ) + assert installed_script.is_file() + assert installed_script.stat().st_mode & 0o100, ( + "execute bit lost through the archive install route" + ) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 6642da2b09..aec32dc4ba 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -9190,6 +9190,212 @@ def fake_install_from_zip(self_obj, _zip_path, speckit_version): ).read_text() assert restored_config_content == original_config_content + def test_update_installs_bundled_extension_from_local_copy(self, tmp_path): + """A bundled extension (no download URL) updates from the copy shipped + with the running spec-kit release instead of failing at download (#4345).""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".claude" / "skills").mkdir(parents=True) + + manager = ExtensionManager(project_dir) + v1_dir = self._create_extension_source(tmp_path, "1.0.0") + manager.install_from_directory(v1_dir, "0.1.0") + v2_dir = self._create_extension_source(tmp_path, "2.0.0") + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(ExtensionCatalog, "get_extension_info", return_value={ + "id": "test-ext", + "name": "Test Extension", + "version": "2.0.0", + "bundled": True, + "_install_allowed": True, + }), \ + patch( + "specify_cli._locate_bundled_extension", return_value=v2_dir + ), \ + patch.object( + ExtensionCatalog, + "download_extension", + side_effect=AssertionError("bundled update must not download"), + ): + result = runner.invoke( + app, ["extension", "update", "test-ext"], input="y\n", catch_exceptions=True + ) + + flat = " ".join(result.output.split()) + assert result.exit_code == 0, result.output + assert "Updated to v2.0.0" in flat + assert ExtensionManager(project_dir).registry.get("test-ext")["version"] == "2.0.0" + + def test_update_bundled_blocked_when_local_copy_lags_catalog(self, tmp_path): + """When the catalog advertises a newer version than the running release + bundles, the update is reported as requiring a spec-kit upgrade instead + of being offered and then failing.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".claude" / "skills").mkdir(parents=True) + + manager = ExtensionManager(project_dir) + v1_dir = self._create_extension_source(tmp_path, "1.0.0") + manager.install_from_directory(v1_dir, "0.1.0") + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(ExtensionCatalog, "get_extension_info", return_value={ + "id": "test-ext", + "name": "Test Extension", + "version": "2.0.0", + "bundled": True, + "_install_allowed": True, + }), \ + patch( + "specify_cli._locate_bundled_extension", return_value=v1_dir + ): + result = runner.invoke( + app, ["extension", "update", "test-ext"], catch_exceptions=True + ) + + flat = " ".join(result.output.split()) + assert result.exit_code == 0, result.output + assert "only ships v1.0.0" in flat + assert "upgrade spec-kit" in flat + assert "Update these extensions?" not in flat + assert "All extensions are up to date!" not in flat + assert ExtensionManager(project_dir).registry.get("test-ext")["version"] == "1.0.0" + + def test_update_bundled_blocked_when_local_copy_is_intermediate_version(self, tmp_path): + """A bundled copy newer than the installation but older than the + catalog must be blocked, not installed: an intermediate version would + leave the project lagging the catalog while reporting success.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".claude" / "skills").mkdir(parents=True) + + manager = ExtensionManager(project_dir) + v1_dir = self._create_extension_source(tmp_path, "1.0.0") + manager.install_from_directory(v1_dir, "0.1.0") + v2_dir = self._create_extension_source(tmp_path, "2.0.0") + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(ExtensionCatalog, "get_extension_info", return_value={ + "id": "test-ext", + "name": "Test Extension", + "version": "3.0.0", + "bundled": True, + "_install_allowed": True, + }), \ + patch( + "specify_cli._locate_bundled_extension", return_value=v2_dir + ), \ + patch.object( + ExtensionCatalog, + "download_extension", + side_effect=AssertionError("blocked bundled update must not download"), + ): + result = runner.invoke( + app, ["extension", "update", "test-ext"], catch_exceptions=True + ) + + flat = " ".join(result.output.split()) + assert result.exit_code == 0, result.output + assert "only ships v2.0.0" in flat + assert "upgrade spec-kit" in flat + assert "Update these extensions?" not in flat + assert ExtensionManager(project_dir).registry.get("test-ext")["version"] == "1.0.0" + + def test_update_installs_bundled_copy_newer_than_catalog(self, tmp_path): + """A dev/source checkout can ship a copy newer than the fetched + catalog advertises; the local copy is offered and installed.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".claude" / "skills").mkdir(parents=True) + + manager = ExtensionManager(project_dir) + v1_dir = self._create_extension_source(tmp_path, "1.0.0") + manager.install_from_directory(v1_dir, "0.1.0") + v3_dir = self._create_extension_source(tmp_path, "3.0.0") + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(ExtensionCatalog, "get_extension_info", return_value={ + "id": "test-ext", + "name": "Test Extension", + "version": "2.0.0", + "bundled": True, + "_install_allowed": True, + }), \ + patch( + "specify_cli._locate_bundled_extension", return_value=v3_dir + ): + result = runner.invoke( + app, ["extension", "update", "test-ext"], input="y\n", catch_exceptions=True + ) + + flat = " ".join(result.output.split()) + assert result.exit_code == 0, result.output + assert "Updated to v3.0.0" in flat + assert ExtensionManager(project_dir).registry.get("test-ext")["version"] == "3.0.0" + + def test_update_bundled_blocked_when_no_local_copy_exists(self, tmp_path): + """A bundled catalog entry with no locally shipped copy points at a + spec-kit upgrade instead of failing the update at download time.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".claude" / "skills").mkdir(parents=True) + + manager = ExtensionManager(project_dir) + v1_dir = self._create_extension_source(tmp_path, "1.0.0") + manager.install_from_directory(v1_dir, "0.1.0") + + with patch.object(Path, "cwd", return_value=project_dir), \ + patch.object(ExtensionCatalog, "get_extension_info", return_value={ + "id": "test-ext", + "name": "Test Extension", + "version": "2.0.0", + "bundled": True, + "_install_allowed": True, + }), \ + patch( + "specify_cli._locate_bundled_extension", return_value=None + ): + result = runner.invoke( + app, ["extension", "update", "test-ext"], catch_exceptions=True + ) + + flat = " ".join(result.output.split()) + assert result.exit_code == 0, result.output + assert "does not ship a local copy" in flat + assert "upgrade spec-kit" in flat + assert ExtensionManager(project_dir).registry.get("test-ext")["version"] == "1.0.0" + def test_update_failure_rolls_back_registry_hooks_and_commands(self, tmp_path, monkeypatch): """Failed update should restore original registry, hooks, and command files.""" from typer.testing import CliRunner