diff --git a/steward/core/plugin_manager.py b/steward/core/plugin_manager.py index 3f68510..42023e2 100644 --- a/steward/core/plugin_manager.py +++ b/steward/core/plugin_manager.py @@ -3,8 +3,8 @@ from __future__ import annotations import hashlib import logging import os +import shutil import sys -import tempfile import zipfile from pathlib import Path from typing import TYPE_CHECKING @@ -290,19 +290,108 @@ def load_plugins(app: "Quart") -> None: logger.info("Plugin %r loaded (v%s)", name, meta.get("version", "?")) +# Bounds on what an install may bring in: the download itself, and what it +# unpacks to (a small zip can expand to fill the disk). +MAX_PLUGIN_DOWNLOAD_BYTES = 50 * 1024 * 1024 +MAX_PLUGIN_UNPACKED_BYTES = 200 * 1024 * 1024 + + +class PluginInstallError(Exception): + """An install refused or failed; the message is shown to the admin.""" + + +def check_plugin_name(name: str) -> None: + """A plugin is imported by its folder name, so the name must be a plain + Python identifier — which also keeps it from naming a path.""" + if not (name.isascii() and name.isidentifier()): + raise PluginInstallError(f"{name!r} is not a valid plugin name") + + +def verify_checksum(content: bytes, expected_sha256: str) -> None: + """Refuse a download whose sha256 is missing from the catalog or differs.""" + if not expected_sha256: + raise PluginInstallError( + "The catalog lists no checksum for this plugin, so Steward cannot " + "tell it is the file the catalog meant. Not installed.") + actual = hashlib.sha256(content).hexdigest() + if actual != expected_sha256.strip().lower(): + raise PluginInstallError(f"Checksum mismatch: expected {expected_sha256}, got {actual}") + + +def extract_plugin(content: bytes, name: str, dest: Path) -> None: + """Unpack a plugin zip into `dest`, which must not exist yet. + + One top-level folder whose name starts with the plugin name is stripped: + name/ (clean plugin zip) + name-v1.0.0/ (release tag archive) + otherwise the zip is unpacked as it stands. Every member must land inside + `dest` — an absolute path or a `..` that climbs out refuses the whole zip — + and the unpacked total is capped. + """ + import io + root = dest.resolve() + with zipfile.ZipFile(io.BytesIO(content)) as zf: + infos = zf.infolist() + if sum(i.file_size for i in infos) > MAX_PLUGIN_UNPACKED_BYTES: + raise PluginInstallError( + f"Unpacks to more than {MAX_PLUGIN_UNPACKED_BYTES // (1024 * 1024)} MB") + top_level = {i.filename.split("/")[0] for i in infos} + prefix = "" + if len(top_level) == 1: + candidate = next(iter(top_level)) + if candidate.lower().startswith(name.lower()) and any( + i.filename.startswith(candidate + "/") for i in infos): + prefix = candidate + "/" + planned: list[tuple[zipfile.ZipInfo, Path]] = [] + for info in infos: + if prefix and not info.filename.startswith(prefix): + continue + rel = info.filename[len(prefix):] + if not rel: + continue + target = (root / rel).resolve() + if not target.is_relative_to(root) or target == root: + raise PluginInstallError(f"Refused: {info.filename!r} points outside the plugin folder") + planned.append((info, target)) + root.mkdir(parents=True) + for info, target in planned: + if info.is_dir(): + target.mkdir(parents=True, exist_ok=True) + continue + target.parent.mkdir(parents=True, exist_ok=True) + with zf.open(info) as src, open(target, "wb") as out: + shutil.copyfileobj(src, out) + + +async def _download(url: str) -> bytes: + import httpx + chunks: list[bytes] = [] + size = 0 + async with httpx.AsyncClient(timeout=60, follow_redirects=True) as client: + async with client.stream("GET", url) as resp: + resp.raise_for_status() + async for chunk in resp.aiter_bytes(): + size += len(chunk) + if size > MAX_PLUGIN_DOWNLOAD_BYTES: + raise PluginInstallError( + f"Download is larger than {MAX_PLUGIN_DOWNLOAD_BYTES // (1024 * 1024)} MB") + chunks.append(chunk) + return b"".join(chunks) + + async def download_and_install_plugin( app: "Quart", name: str, download_url: str, expected_sha256: str, ) -> tuple[bool, str]: - """Download a plugin zip, verify its checksum, and extract it to PLUGIN_DIR. + """Download a plugin zip named by the catalog, verify its checksum, and + unpack it into the install dir, replacing any earlier copy whole. - Returns (success, message). Does NOT hot-reload — call hot_reload_plugin() - after this to activate the plugin without a restart (new plugins only). + The URL and checksum must come from the catalog entry, never from the + request. Returns (success, message). Does NOT hot-reload — call + hot_reload_plugin() after this to activate a new plugin without a restart. """ - import httpx - # Downloads always land in the external (writable, persistent) install dir, # never the read-only bundled dir. plugin_dir = Path(app.config["PLUGIN_INSTALL_DIR"]).resolve() @@ -314,68 +403,46 @@ async def download_and_install_plugin( sys.path.insert(0, plugin_dir_str) try: + check_plugin_name(name) + if not download_url: + raise PluginInstallError("The catalog lists no download URL for this plugin") logger.info("Downloading plugin %r from %s", name, download_url) - async with httpx.AsyncClient(timeout=60, follow_redirects=True) as client: - resp = await client.get(download_url) - resp.raise_for_status() - content = resp.content - except Exception as exc: - msg = f"Download failed: {exc}" - logger.error("Plugin %r: %s", name, msg) - return False, msg + try: + content = await _download(download_url) + except PluginInstallError: + raise + except Exception as exc: + raise PluginInstallError(f"Download failed: {exc}") from exc + verify_checksum(content, expected_sha256) - # Verify checksum if provided - if expected_sha256: - actual = hashlib.sha256(content).hexdigest() - if actual != expected_sha256.lower(): - msg = f"Checksum mismatch: expected {expected_sha256}, got {actual}" - logger.error("Plugin %r: %s", name, msg) - return False, msg - - # Extract the zip - try: - with tempfile.TemporaryDirectory() as tmpdir: - zip_path = Path(tmpdir) / f"{name}.zip" - zip_path.write_bytes(content) - with zipfile.ZipFile(zip_path) as zf: - members = zf.namelist() - # Collect unique top-level names (dirs and loose files) - top_level = {m.split("/")[0] for m in members} - - dest = plugin_dir / name - dest.mkdir(parents=True, exist_ok=True) - - # Detect a single top-level directory to strip. Handles: - # name/ (clean plugin zip) - # name-v1.0.0/ (GitHub release tag archive) - # steward-plugins-main/name/ (whole-repo archive) - # Strategy: if there is exactly one top-level item and it is a - # directory whose name starts with the plugin name (case-insensitive), - # strip it. Otherwise extract flat into dest. - prefix: str | None = None - if len(top_level) == 1: - candidate = list(top_level)[0] - if candidate.lower().startswith(name.lower()): - prefix = candidate + "/" - - for member in members: - if prefix: - if not member.startswith(prefix): - continue - rel = member[len(prefix):] - else: - rel = member - if not rel or rel.endswith("/"): - if rel: - (dest / rel.rstrip("/")).mkdir(parents=True, exist_ok=True) - continue - target = dest / rel - target.parent.mkdir(parents=True, exist_ok=True) - target.write_bytes(zf.read(member)) - except Exception as exc: - msg = f"Extraction failed: {exc}" - logger.error("Plugin %r: %s", name, msg) - return False, msg + # Unpack beside the live copy, then swap: a failed or refused zip + # leaves the installed plugin untouched, and an update drops files the + # new version no longer ships. + plugin_dir.mkdir(parents=True, exist_ok=True) + dest = plugin_dir / name + staging = plugin_dir / f".{name}.installing" + old = plugin_dir / f".{name}.replaced" + for leftover in (staging, old): + shutil.rmtree(leftover, ignore_errors=True) + try: + extract_plugin(content, name, staging) + except PluginInstallError: + raise + except Exception as exc: + raise PluginInstallError(f"Extraction failed: {exc}") from exc + try: + if dest.exists(): + dest.rename(old) + staging.rename(dest) + except OSError as exc: + if old.exists() and not dest.exists(): + old.rename(dest) # put the previous version back + raise PluginInstallError(f"Could not move the plugin into place: {exc}") from exc + shutil.rmtree(old, ignore_errors=True) + except PluginInstallError as exc: + shutil.rmtree(plugin_dir / f".{name}.installing", ignore_errors=True) + logger.error("Plugin %r: %s", name, exc) + return False, str(exc) # Run migrations for the newly installed plugin. # Include all plugin migration dirs so Alembic can resolve any previously-stamped diff --git a/steward/settings/routes.py b/steward/settings/routes.py index de1de43..a513009 100644 --- a/steward/settings/routes.py +++ b/steward/settings/routes.py @@ -820,23 +820,34 @@ async def plugins_catalog(): @settings_bp.post("/plugins/install/") @require_role(UserRole.admin) async def install_plugin(name: str): - """Download and install a plugin from the catalog, then hot-reload it.""" - form = await request.form - download_url = form.get("download_url", "").strip() - checksum = form.get("checksum_sha256", "").strip() + """Download and install a plugin from the catalog, then hot-reload it. - if not download_url: + Only the plugin's name comes from the request. The download URL and + checksum are read from the configured catalogs, so a request cannot point + Steward at a file of its own choosing. + """ + from steward.core.plugin_index import fetch_catalog + from steward.core.plugin_manager import download_and_install_plugin, hot_reload_plugin + + async with current_app.db_sessionmaker() as db: + settings = await get_all_settings(db) + repo_urls = [r["url"] for r in _get_plugin_repos(settings) if r.get("url")] + entry = next((p for p in await fetch_catalog(repo_urls) if p.name == name), None) + if entry is None: return await render_template( "settings/_install_result.html", name=name, success=False, - message="No download URL provided.", + message="No configured plugin repository lists this plugin.", ) - from steward.core.plugin_manager import download_and_install_plugin, hot_reload_plugin - ok, msg = await download_and_install_plugin( - current_app._get_current_object(), name, download_url, checksum + current_app._get_current_object(), entry.name, entry.download_url, entry.checksum_sha256 ) + await log_audit( + current_app, session.get("user_id"), session.get("username", ""), + "plugin.install" if ok else "plugin.install_failed", entity_type="plugin", + entity_id=name, detail={"version": entry.version, "url": entry.download_url, + "message": msg}) if not ok: return await render_template( "settings/_install_result.html", diff --git a/steward/templates/settings/_catalog_partial.html b/steward/templates/settings/_catalog_partial.html index 3c7da8e..0637f0b 100644 --- a/steward/templates/settings/_catalog_partial.html +++ b/steward/templates/settings/_catalog_partial.html @@ -56,7 +56,14 @@ class="btn btn-ghost btn-sm" style="font-size:0.78rem;">Source {% endif %} - {% if update_available %} + {% if (update_available or not is_installed) and not plugin.checksum_sha256 %} + {# Steward installs only what it can verify against the catalog's checksum #} + + Can't {{ "update" if update_available else "install" }}: the catalog lists no checksum + + + {% elif update_available %} {# Installed but outdated — offer update (download + restart) #}
{{ csrf_field() }} - -
@@ -91,8 +96,6 @@ hx-target="#catalog-result-{{ plugin.name }}" hx-swap="outerHTML" style="margin:0;">{{ csrf_field() }} - - {% endif %} diff --git a/tests/core/test_plugin_install.py b/tests/core/test_plugin_install.py new file mode 100644 index 0000000..b15bd8b --- /dev/null +++ b/tests/core/test_plugin_install.py @@ -0,0 +1,116 @@ +"""Plugin installs fetch only what the catalog names, verify it, and unpack it +only inside the plugin's own folder.""" +import hashlib +import io +import zipfile +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + +import pytest + +from steward.core import plugin_manager as pm + + +def make_zip(files: dict[str, str]) -> bytes: + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + for name, text in files.items(): + zf.writestr(name, text) + return buf.getvalue() + + +def sha(content: bytes) -> str: + return hashlib.sha256(content).hexdigest() + + +@pytest.mark.parametrize("name", ["..", "a/b", "evil-name", "", "x.y", "ümlaut"]) +def test_name_must_be_an_identifier(name): + with pytest.raises(pm.PluginInstallError): + pm.check_plugin_name(name) + + +def test_checksum_is_required_and_compared(): + content = b"zip" + with pytest.raises(pm.PluginInstallError, match="no checksum"): + pm.verify_checksum(content, "") + with pytest.raises(pm.PluginInstallError, match="mismatch"): + pm.verify_checksum(content, "0" * 64) + pm.verify_checksum(content, sha(content).upper()) + + +def test_extract_strips_the_plugin_folder(tmp_path): + content = make_zip({"demo-v1.2/__init__.py": "x = 1", "demo-v1.2/sub/a.txt": "a"}) + pm.extract_plugin(content, "demo", tmp_path / "demo") + assert (tmp_path / "demo" / "__init__.py").read_text() == "x = 1" + assert (tmp_path / "demo" / "sub" / "a.txt").read_text() == "a" + + +@pytest.mark.parametrize("member", ["../escape.py", "demo/../../escape.py", "/abs.py"]) +def test_extract_refuses_members_outside_the_folder(tmp_path, member): + content = make_zip({"__init__.py": "", member: "boom"}) + with pytest.raises(pm.PluginInstallError, match="outside"): + pm.extract_plugin(content, "demo", tmp_path / "plugins" / "demo") + assert not (tmp_path / "escape.py").exists() + assert not (tmp_path / "plugins" / "escape.py").exists() + assert not (tmp_path / "plugins" / "demo").exists() # nothing half-written + + +def test_extract_refuses_an_oversized_unpack(tmp_path, monkeypatch): + monkeypatch.setattr(pm, "MAX_PLUGIN_UNPACKED_BYTES", 10) + with pytest.raises(pm.PluginInstallError, match="Unpacks to more than"): + pm.extract_plugin(make_zip({"__init__.py": "x" * 11}), "demo", tmp_path / "demo") + + +def _app(tmp_path): + return SimpleNamespace(config={"PLUGIN_INSTALL_DIR": str(tmp_path), "PLUGIN_DIRS": [str(tmp_path)], + "DATABASE_URL": ""}) + + +@pytest.mark.asyncio +async def test_update_replaces_the_folder_whole(tmp_path): + old = tmp_path / "demo" + old.mkdir() + (old / "gone.py").write_text("old") + content = make_zip({"demo/__init__.py": "new"}) + with patch.object(pm, "_download", new=AsyncMock(return_value=content)): + ok, msg = await pm.download_and_install_plugin(_app(tmp_path), "demo", "https://x/demo.zip", sha(content)) + assert ok, msg + assert (old / "__init__.py").read_text() == "new" + assert not (old / "gone.py").exists() + assert sorted(p.name for p in tmp_path.iterdir()) == ["demo"] + + +@pytest.mark.asyncio +async def test_a_refused_download_leaves_the_installed_copy(tmp_path): + old = tmp_path / "demo" + old.mkdir() + (old / "__init__.py").write_text("old") + content = make_zip({"demo/__init__.py": "new"}) + with patch.object(pm, "_download", new=AsyncMock(return_value=content)): + ok, msg = await pm.download_and_install_plugin(_app(tmp_path), "demo", "https://x/demo.zip", "") + assert not ok and "no checksum" in msg + assert (old / "__init__.py").read_text() == "old" + assert sorted(p.name for p in tmp_path.iterdir()) == ["demo"] + + +@pytest.mark.asyncio +async def test_route_takes_url_and_checksum_from_the_catalog(client): + async with client.session_transaction() as sess: + sess.update(user_id="u", user_role="admin", username="admin", _csrf="t") + entry = SimpleNamespace(name="demo", version="1.0", download_url="https://catalog/demo.zip", + checksum_sha256="abc") + install = AsyncMock(return_value=(False, "stopped here")) + with patch("steward.settings.routes.get_all_settings", new=AsyncMock(return_value={})), \ + patch("steward.settings.routes._get_plugin_repos", return_value=[{"url": "https://catalog/"}]), \ + patch("steward.core.plugin_index.fetch_catalog", new=AsyncMock(return_value=[entry])), \ + patch("steward.core.plugin_manager.download_and_install_plugin", new=install), \ + patch("steward.settings.routes.log_audit", new=AsyncMock()): + resp = await client.post("/settings/plugins/install/demo", headers={"X-CSRF-Token": "t"}, form={ + "download_url": "https://attacker/evil.zip", "checksum_sha256": "dead"}) + assert resp.status_code == 200 + assert install.await_args.args[1:] == ("demo", "https://catalog/demo.zip", "abc") + + install.reset_mock() + resp = await client.post("/settings/plugins/install/other", headers={"X-CSRF-Token": "t"}) + assert "No configured plugin repository lists this plugin" in await resp.get_data(as_text=True) + install.assert_not_awaited()