From 670e9d8af7ce1bfca76fe04dbde6dcd52fa69188 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Wed, 5 Aug 2026 13:36:20 -0400 Subject: [PATCH] security: signature lifecycle, update-path consent gates, download hardening Audit remediation (install/signing surface): - sig verdict reset on every install path (no stale verified badge) - registry update route mirrors install consent gates (unreviewed 404, unsigned-first-party/untrusted 409 until acknowledged, bad sig 400); update offers carry installed_source_type; frontend risk dialog gains update branch - first_party entries without verified pinned-key signature require acknowledge (community reviewed exempt, review-stamp-bound) - downloads: https-only with per-hop redirect revalidation, private/ link-local IP rejection, 100MB zip + 64KB sig caps, github token https-only - extraction: declared+actual 250MB decompression cap with cleanup - install/update audit logs record the signature verdict --- backend/app/api/marketplace.py | 28 +-- backend/app/api/plugins.py | 56 ++++- backend/app/services/plugin_service.py | 283 +++++++++++++++++------ backend/app/services/registry_service.py | 65 ++++++ backend/tests/test_extension_signing.py | 253 ++++++++++++++++++++ backend/tests/test_plugin_preview.py | 5 + backend/tests/test_registry.py | 12 +- frontend/src/pages/Marketplace.jsx | 33 ++- frontend/src/services/api/plugins.js | 5 +- 9 files changed, 634 insertions(+), 106 deletions(-) diff --git a/backend/app/api/marketplace.py b/backend/app/api/marketplace.py index f5e0cbae..75a9c4d8 100644 --- a/backend/app/api/marketplace.py +++ b/backend/app/api/marketplace.py @@ -62,31 +62,11 @@ def install_registry(slug): body = request.get_json(silent=True) or {} trust = (entry or {}).get('trust', 'unreviewed') acknowledged = body.get('acknowledge_risk') is True - reason = None - if entry is not None and not acknowledged: - if trust == 'unreviewed': - reason = 'unreviewed' - elif not entry.get('sha256'): - reason = 'unverified' - elif entry.get('signature') and not entry.get('publisher_trusted'): - # Signed, but by a publisher key this panel doesn't pin (plan 55). - # Consent-overridable like unsigned; a BAD signature is never - # overridable — install_registry_extension hard-fails on it below. - reason = 'untrusted_key' + # Consent gate (shared with the update path — audits M2/M3): + # unreviewed (dev-stage) > unverified (no checksum) > signature gate + # (untrusted key, or unsigned first-party = possible downgrade). + reason, message = registry_service.consent_gate(entry, acknowledged) if reason: - message = ( - 'This community extension has not been reviewed by the ServerKit ' - 'maintainers; installing it runs unreviewed code with full panel ' - 'privileges.' - if reason == 'unreviewed' else - 'This extension has no pinned checksum, so the panel cannot ' - 'verify the artifact it would install.' - if reason == 'unverified' else - f"This extension is signed, but its publisher key " - f"'{entry.get('publisher_key_id')}' is not pinned by this panel, " - f"so the signature cannot be verified. Install only if you trust " - f"the source." - ) return jsonify({ 'error': f'{message} Resend with acknowledge_risk: true to proceed.', 'trust': trust, diff --git a/backend/app/api/plugins.py b/backend/app/api/plugins.py index c1bb8ec4..4383fd22 100644 --- a/backend/app/api/plugins.py +++ b/backend/app/api/plugins.py @@ -82,12 +82,18 @@ def install_plugin(): url, user_id=user.id, expected_sha256=expected_sha256, expected_signature=expected_signature, expected_key_id=expected_key_id, ) + sig = (plugin.config or {}).get('_signature') or {} AuditService.log( action=AuditLog.ACTION_RESOURCE_CREATE, user_id=user.id, target_type='plugin', target_id=plugin.id, - details={'name': plugin.name, 'version': plugin.version, 'url': url} + details={ + 'name': plugin.name, 'version': plugin.version, 'url': url, + # Forensics (audit I7): the signature verdict of the bytes + # that just landed (verified / untrusted_key / unsigned). + 'signature': sig.get('status') or 'unsigned', + } ) return jsonify(plugin.to_dict()), 201 except ValueError as e: @@ -616,20 +622,62 @@ def list_updates(): @plugins_bp.route('//update', methods=['POST']) @jwt_required() def update_plugin_route(plugin_id): - """Update an installed plugin to the registry version (reinstall in place).""" + """Update an installed plugin to the registry version (reinstall in place). + + The update re-downloads from the registry, so it passes the SAME consent + gates as a fresh registry install (audit M2): hidden unreviewed entries + 404; unreviewed / unverified / untrusted-key / unsigned-first-party + entries 409 until resent with ``acknowledge_risk: true``; a bad signature + is always a hard 400 (the service verifies at install). + """ user = get_current_user() if not user or not user.is_admin: return jsonify({'error': 'Admin access required'}), 403 - from app.services.plugin_service import update_plugin + from app.services.plugin_service import get_plugin, update_plugin + from app.services import registry_service + plugin = get_plugin(plugin_id) + if not plugin: + return jsonify({'error': 'Plugin not found'}), 404 + + entry = registry_service.get_entry(plugin.slug) + if not entry: + return jsonify({ + 'error': f"No registry entry for '{plugin.slug}' — cannot update " + f"from the registry." + }), 400 + # Hidden means not updatable either: unreviewed entries only exist in + # development contexts (see registry_service._show_unreviewed). + if entry.get('trust') == 'unreviewed' \ + and not registry_service._show_unreviewed(): + return jsonify({'error': 'Extension not found in the registry'}), 404 + + body = request.get_json(silent=True) or {} + acknowledged = body.get('acknowledge_risk') is True + reason, message = registry_service.consent_gate(entry, acknowledged) + if reason: + return jsonify({ + 'error': f'{message} Resend with acknowledge_risk: true to proceed.', + 'trust': entry.get('trust'), + 'reason': reason, + 'requires_acknowledgment': True, + }), 409 + try: plugin = update_plugin(plugin_id, user_id=user.id) + sig = (plugin.config or {}).get('_signature') or {} AuditService.log( action=AuditLog.ACTION_RESOURCE_UPDATE, user_id=user.id, target_type='plugin', target_id=plugin.id, - details={'name': plugin.name, 'version': plugin.version, 'source': 'registry-update'}, + details={ + 'name': plugin.name, 'version': plugin.version, + 'source': 'registry-update', + # Forensics (audit I7): the signature verdict of the bytes + # that just landed. + 'signature': sig.get('status') or 'unsigned', + }, ) return jsonify(plugin.to_dict()) except ValueError as e: diff --git a/backend/app/services/plugin_service.py b/backend/app/services/plugin_service.py index 3204fc37..fc66e039 100644 --- a/backend/app/services/plugin_service.py +++ b/backend/app/services/plugin_service.py @@ -169,8 +169,9 @@ def _github_token(): def _is_github_host(url): """True for github.com / api.github.com / codeload.github.com URLs — the only - hosts the token is ever attached to (never leak it to third-party mirrors).""" - return bool(re.match(r'^https?://(?:[a-z0-9-]+\.)?github\.com/', url or '', re.I)) + hosts the token is ever attached to (never leak it to third-party mirrors, + and never over plain http — audit L6).""" + return bool(re.match(r'^https://(?:[a-z0-9-]+\.)?github\.com/', url or '', re.I)) def _github_api_headers(accept='application/vnd.github+json'): @@ -305,6 +306,103 @@ def _download_zip(url): return _download_resolved(resolved) +# ── Download safety (audit L4): scheme/host checks, redirect re-validation, +# and byte caps, shared by the zip and detached-signature fetches. +_MAX_DOWNLOAD_BYTES = 100 * 1024 * 1024 # extension zip cap +_MAX_SIGNATURE_BYTES = 64 * 1024 # detached .minisig cap +_MAX_EXTRACT_BYTES = 250 * 1024 * 1024 # decompression cap (audit L5) +_MAX_REDIRECTS = 5 + + +def _ip_is_public(ip): + import ipaddress + addr = ipaddress.ip_address(ip) + return not (addr.is_private or addr.is_loopback or addr.is_link_local + or addr.is_multicast or addr.is_reserved or addr.is_unspecified) + + +def _assert_url_safe(url): + """Reject anything that isn't an https URL to a public host (SSRF guard). + + Rules: https everywhere; http only for loopback dev hosts + (http://localhost:8000/ext.zip while developing); loopback targets are + always allowed (a same-host registry is legitimate), but RFC1918 / + link-local / reserved targets are refused by default — a panel must never + follow an attacker-supplied URL into the internal network or the cloud + metadata service. Operators with a private-zone registry can opt out with + SERVERKIT_ALLOW_PRIVATE_DOWNLOADS=1. + """ + import ipaddress + import socket + from urllib.parse import urlparse + + parsed = urlparse(url or '') + scheme = (parsed.scheme or '').lower() + if scheme not in ('http', 'https'): + raise ValueError( + f"Unsupported URL scheme {scheme!r} — extension downloads must be https://") + host = parsed.hostname + if not host: + raise ValueError('Download URL has no host') + try: + ips = {info[4][0] for info in socket.getaddrinfo(host, None)} + except Exception as e: + raise ValueError(f'Could not resolve download host {host!r}: {e}') + if all(ipaddress.ip_address(ip).is_loopback for ip in ips): + return # loopback dev host — http or https both fine + if scheme == 'http': + raise ValueError( + 'Plain-http extension downloads are only allowed for loopback dev hosts') + if os.environ.get('SERVERKIT_ALLOW_PRIVATE_DOWNLOADS', '').lower() in ('1', 'true', 'yes'): + return + bad = sorted(ip for ip in ips if not _ip_is_public(ip)) + if bad: + raise ValueError( + f'Download host {host!r} resolves to a non-public address ({bad[0]}) ' + f'— refusing to fetch (SSRF guard). Set ' + f'SERVERKIT_ALLOW_PRIVATE_DOWNLOADS=1 to allow private-zone registries.') + + +def _safe_get(url, timeout, headers, max_bytes): + """GET ``url`` with per-hop SSRF re-validation (redirects are followed + manually so each hop is re-checked), a hard byte cap, and the GitHub + token re-computed per hop (never leaked cross-host). Returns bytes.""" + from urllib.parse import urljoin + current = url + for _ in range(_MAX_REDIRECTS + 1): + _assert_url_safe(current) + hop_headers = dict(headers or {}) + token = _github_token() + if token and _is_github_host(current): + hop_headers['Authorization'] = f'Bearer {token}' + resp = requests.get(current, timeout=timeout, stream=True, + headers=hop_headers, allow_redirects=False) + try: + # status_code check rather than is_redirect: equivalent semantics, + # and tolerant of minimal test doubles. + if resp.status_code in (301, 302, 303, 307, 308): + location = resp.headers.get('Location') + if not location: + raise ValueError('Redirect without a Location header') + current = urljoin(current, location) + continue + resp.raise_for_status() + buf = io.BytesIO() + for chunk in resp.iter_content(chunk_size=8192): + buf.write(chunk) + if buf.tell() > max_bytes: + raise ValueError( + f'Download exceeds the {max_bytes // (1024 * 1024)} MB cap ' + f'— aborting (possible zip bomb or misconfigured source)') + buf.seek(0) + return buf.getvalue() + finally: + close = getattr(resp, 'close', None) + if callable(close): + close() + raise ValueError(f'Too many redirects fetching {url}') + + def _download_resolved(resolved): """Download an already-resolved zip URL and return a BytesIO buffer.""" logger.info(f'Downloading plugin from: {resolved}') @@ -312,18 +410,9 @@ def _download_resolved(resolved): 'Accept': 'application/octet-stream', 'User-Agent': 'ServerKit-Plugin-Installer/1.0', } - # Attach the token only for GitHub hosts (private assets / rate limit). - token = _github_token() - if token and _is_github_host(resolved): - headers['Authorization'] = f'Bearer {token}' - resp = requests.get(resolved, timeout=120, stream=True, headers=headers) - resp.raise_for_status() - - buf = io.BytesIO() - for chunk in resp.iter_content(chunk_size=8192): - buf.write(chunk) - buf.seek(0) - return buf + data = _safe_get(resolved, timeout=120, headers=headers, + max_bytes=_MAX_DOWNLOAD_BYTES) + return io.BytesIO(data) def _find_manifest(zf): @@ -580,9 +669,8 @@ def install_from_url(url, user_id=None, expected_sha256=None, force=False, plugin = _install_from_buffer( buf, source_url=url, source_type=source_type or 'url', user_id=user_id, force=force, - hot_load=hot_load, + hot_load=hot_load, sig_result=sig_result, ) - _record_signature_status(plugin, sig_result) return plugin @@ -620,13 +708,9 @@ def _fetch_detached_signature(resolved_url): 'Accept': 'text/plain, application/octet-stream', 'User-Agent': 'ServerKit-Plugin-Installer/1.0', } - token = _github_token() - if token and _is_github_host(resolved_url): - headers['Authorization'] = f'Bearer {token}' - resp = requests.get(resolved_url + '.minisig', timeout=15, headers=headers) - if resp.status_code != 200: - return None - text = (resp.text or '').strip() + data = _safe_get(resolved_url + '.minisig', timeout=15, headers=headers, + max_bytes=_MAX_SIGNATURE_BYTES) + text = data.decode('utf-8', errors='replace').strip() return text or None except Exception as e: logger.debug(f'No detached signature at {resolved_url}.minisig: {e}') @@ -805,7 +889,7 @@ def install_from_zip(zip_bytes, user_id=None, source_name=None): def _install_from_buffer(buf, source_url, source_type, user_id=None, force=False, - hot_load=True): + hot_load=True, sig_result=None): """Shared install pipeline: takes a seekable BytesIO containing a zip, extracts it into the panel's plugin dirs, and registers / hot-loads the resulting blueprint. All public install_* helpers funnel here so @@ -814,6 +898,12 @@ def _install_from_buffer(buf, source_url, source_type, user_id=None, force=False `force=True` allows reinstalling over an active plugin (the update path); normal installs still refuse to clobber an active install. `hot_load=False` skips the immediate blueprint registration (the boot repair pass, #48). + + `sig_result` is the signature verdict produced by the caller (only + install_from_url can produce one — local/upload/builtin installs have no + detached signature). It is stamped on EVERY successful install so a + reinstall via a different source can never inherit a stale 'verified' + badge from an earlier signed install (audit M1). """ _ensure_dirs() @@ -904,53 +994,91 @@ def _install_from_buffer(buf, source_url, source_type, user_id=None, force=False if has_frontend and os.path.exists(frontend_dest): shutil.rmtree(frontend_dest) - for member in zf.namelist(): - # Strip the GitHub zipball prefix - rel_path = member[len(prefix):] if prefix else member - if not rel_path or rel_path.endswith('/'): - continue + # Decompression cap (audit L5): refuse archives that expand past + # _MAX_EXTRACT_BYTES — a small zip can declare or write gigabytes + # (zip bomb). Pre-check the declared sizes (cheap, catches honest + # bombs before writing a byte), then count actual bytes written + # (the declared size can lie). On abort, remove the partial dest + # dirs so no half-extracted plugin is left behind. + declared_total = sum( + info.file_size for info in zf.infolist() if not info.is_dir()) + if declared_total > _MAX_EXTRACT_BYTES: + raise ValueError( + f'Archive expands to {declared_total // (1024 * 1024)} MB, ' + f'over the {_MAX_EXTRACT_BYTES // (1024 * 1024)} MB cap — ' + f'refusing to extract (possible zip bomb)') + extracted_total = 0 - if rel_path.startswith('backend/'): - # Zip Slip defense: _safe_extract_path rejects absolute - # paths, .. segments, and anything that resolves outside - # backend_dest after normalization. - out_path = _safe_extract_path( - backend_dest, rel_path[len('backend/'):] - ) - os.makedirs(os.path.dirname(out_path), exist_ok=True) - with zf.open(member) as src, open(out_path, 'wb') as dst: - dst.write(src.read()) + def _write_member(src, dst): + nonlocal extracted_total + while True: + chunk = src.read(1024 * 1024) + if not chunk: + break + extracted_total += len(chunk) + if extracted_total > _MAX_EXTRACT_BYTES: + raise ValueError( + f'Archive expands past the ' + f'{_MAX_EXTRACT_BYTES // (1024 * 1024)} MB cap — ' + f'refusing to extract (possible zip bomb)') + dst.write(chunk) - elif rel_path.startswith('frontend/'): - out_path = _safe_extract_path( - frontend_dest, rel_path[len('frontend/'):] - ) - os.makedirs(os.path.dirname(out_path), exist_ok=True) - with zf.open(member) as src, open(out_path, 'wb') as dst: - dst.write(src.read()) + try: + for member in zf.namelist(): + # Strip the GitHub zipball prefix + rel_path = member[len(prefix):] if prefix else member + if not rel_path or rel_path.endswith('/'): + continue - elif rel_path == 'requirements.txt': - # Plugin Python deps are sandboxed by default: installing - # them runs pip with the backend's privileges, which means - # arbitrary code via setup.py hooks. Operators must opt in - # by setting SERVERKIT_ALLOW_PLUGIN_PIP=1. - req_content = zf.read(member).decode('utf-8') - if req_content.strip() and os.environ.get( - 'SERVERKIT_ALLOW_PLUGIN_PIP', '' - ).lower() in ('1', 'true', 'yes'): - _install_requirements(req_content, slug) - elif req_content.strip(): - # Persist the requirements file so the admin can review - # and install it manually if they want. - req_out = _safe_extract_path(backend_dest, 'requirements.txt') - os.makedirs(os.path.dirname(req_out), exist_ok=True) - with open(req_out, 'w', encoding='utf-8') as f: - f.write(req_content) - logger.warning( - f"Plugin {slug} ships requirements.txt; skipping install " - f"(set SERVERKIT_ALLOW_PLUGIN_PIP=1 to enable). " - f"File saved to {req_out} for manual review." + if rel_path.startswith('backend/'): + # Zip Slip defense: _safe_extract_path rejects absolute + # paths, .. segments, and anything that resolves outside + # backend_dest after normalization. + out_path = _safe_extract_path( + backend_dest, rel_path[len('backend/'):] ) + os.makedirs(os.path.dirname(out_path), exist_ok=True) + with zf.open(member) as src, open(out_path, 'wb') as dst: + _write_member(src, dst) + + elif rel_path.startswith('frontend/'): + out_path = _safe_extract_path( + frontend_dest, rel_path[len('frontend/'):] + ) + os.makedirs(os.path.dirname(out_path), exist_ok=True) + with zf.open(member) as src, open(out_path, 'wb') as dst: + _write_member(src, dst) + + elif rel_path == 'requirements.txt': + # Plugin Python deps are sandboxed by default: installing + # them runs pip with the backend's privileges, which means + # arbitrary code via setup.py hooks. Operators must opt in + # by setting SERVERKIT_ALLOW_PLUGIN_PIP=1. + req_content = zf.read(member).decode('utf-8') + if req_content.strip() and os.environ.get( + 'SERVERKIT_ALLOW_PLUGIN_PIP', '' + ).lower() in ('1', 'true', 'yes'): + _install_requirements(req_content, slug) + elif req_content.strip(): + # Persist the requirements file so the admin can review + # and install it manually if they want. + req_out = _safe_extract_path(backend_dest, 'requirements.txt') + os.makedirs(os.path.dirname(req_out), exist_ok=True) + with open(req_out, 'w', encoding='utf-8') as f: + f.write(req_content) + logger.warning( + f"Plugin {slug} ships requirements.txt; skipping install " + f"(set SERVERKIT_ALLOW_PLUGIN_PIP=1 to enable). " + f"File saved to {req_out} for manual review." + ) + except ValueError: + # Extraction aborted (zip bomb cap) — drop the partial dest dirs so + # no half-extracted plugin is left behind, then re-raise into the + # outer error handler (marks the row STATUS_ERROR). + for dest in (backend_dest, frontend_dest): + if os.path.isdir(dest): + shutil.rmtree(dest, ignore_errors=True) + raise # Also write the manifest into the backend plugin dir for runtime access. # The trailing newline matters: these manifests are tracked files that @@ -989,6 +1117,10 @@ def _install_from_buffer(buf, source_url, source_type, user_id=None, force=False _record_frontend_hashes(plugin, manifest, frontend_dest) plugin.status = InstalledPlugin.STATUS_ACTIVE db.session.commit() + # Stamp the signature verdict on every successful install (None for + # local/upload/builtin sources → 'unsigned'), so a reinstall via a + # different source never keeps a stale 'verified' badge. + _record_signature_status(plugin, sig_result) # Try to register the blueprint immediately (hot-load) if has_backend and entry_point and hot_load: @@ -1401,6 +1533,9 @@ def uninstall_plugin(plugin_id, purge=False): try: from app.services import extension_lifecycle extension_lifecycle.remove_jobs(plugin, manifest) + # Audit F1: seam registrations (backup kinds, event types, template + # providers) must not keep executing after uninstall. + extension_lifecycle.unregister_capabilities(plugin) if purge: extension_lifecycle.purge_models(plugin) from app.plugins_sdk.store_sdk import purge as purge_store @@ -1454,6 +1589,8 @@ def enable_plugin(plugin_id): try: from app.services import extension_lifecycle extension_lifecycle.resume_jobs(plugin, plugin.manifest or {}) + # Audit F4: re-register the core seams disable tore down (idempotent). + extension_lifecycle.register_capabilities(plugin, plugin.manifest or {}) except Exception as e: logger.warning(f'Could not resume jobs for {plugin.slug}: {e}') return plugin @@ -1472,6 +1609,9 @@ def disable_plugin(plugin_id): try: from app.services import extension_lifecycle extension_lifecycle.pause_jobs(plugin, plugin.manifest or {}) + # Audit F1: seam registrations (backup kinds, event types, template + # providers) must not keep executing while disabled. + extension_lifecycle.unregister_capabilities(plugin) except Exception as e: logger.warning(f'Could not pause jobs for {plugin.slug}: {e}') return plugin @@ -1850,6 +1990,9 @@ def install_builtin_extension(slug, user_id=None): if existing: existing.status = InstalledPlugin.STATUS_ACTIVE db.session.commit() + # A builtin re-install carries no signature — never keep a + # verdict stamped by some earlier install of the slug. + _record_signature_status(existing, None) return existing plugin = _seed_flagship_row(entry) try: @@ -2059,7 +2202,10 @@ def check_for_updates(): Update v1 is registry-driven: a plugin whose slug is in the registry with a higher version than installed is flagged. Panel-version compatibility is - surfaced so the UI can gray out an update the panel can't run yet. + surfaced so the UI can gray out an update the panel can't run yet. The + installed plugin's ``source_type`` is included so callers can see the + provenance the update would cross (audit M2 — a registry entry only ever + matches on slug; the update route's consent gate is the enforcement). """ from app.services import registry_service from app.utils.version import compare_versions, version_satisfies, get_panel_version @@ -2080,6 +2226,7 @@ def check_for_updates(): panel, entry.get('min_panel_version'), entry.get('max_panel_version') ), 'source': entry.get('source'), + 'installed_source_type': p.source_type, }) return out diff --git a/backend/app/services/registry_service.py b/backend/app/services/registry_service.py index af48683c..60da5ed1 100644 --- a/backend/app/services/registry_service.py +++ b/backend/app/services/registry_service.py @@ -293,3 +293,68 @@ def list_catalog(include_bundled=False): def registry_source_label(): return _cache.get('source') + + +def signature_consent_reason(entry): + """The signature-side consent reason for an index entry, or None. + + Shared by the registry install gate and the update gate (audits M2/M3): + + - a signature pinned to a panel-trusted publisher key → no consent needed + (install verifies it cryptographically; a bad one hard-fails there). + - signed by an unpinned key → 'untrusted_key'. + - a FIRST-PARTY entry without a verifiable signature → 'unsigned': + first-party releases are expected to be signed, so anything less is a + possible downgrade and requires explicit consent. + - community REVIEWED entries stay exempt: the review stamp already binds + a maintainer's verdict to the exact sha256 of the artifact, which is + the pre-signature trust mechanism (see docs/EXTENSIONS.md). + """ + if entry.get('signature'): + if entry.get('publisher_trusted'): + return None + return 'untrusted_key' + if entry.get('first_party'): + return 'unsigned' + return None + + +_GATE_MESSAGES = { + 'unreviewed': ( + 'This community extension has not been reviewed by the ServerKit ' + 'maintainers; installing it runs unreviewed code with full panel ' + 'privileges.'), + 'unverified': ( + 'This extension has no pinned checksum, so the panel cannot ' + 'verify the artifact it would install.'), + # 'untrusted_key' is formatted inline (interpolates the key id). + 'unsigned': ( + 'This first-party release is not signed with a publisher key ' + 'pinned by this panel, so the panel cannot verify its origin. It ' + 'may be a legitimate older release — or a downgrade attempt.'), +} + + +def consent_gate(entry, acknowledged): + """The shared install/update consent decision (audits M2/M3). + + Returns ``(reason, message)`` — reason is None when the action may + proceed without explicit acknowledgment. Order: unreviewed (dev-stage) + > unverified (no checksum) > signature gate (untrusted key / unsigned + first-party).""" + if not entry or acknowledged: + return None, None + trust = entry.get('trust', 'unreviewed') + if trust == 'unreviewed': + reason = 'unreviewed' + elif not entry.get('sha256'): + reason = 'unverified' + else: + reason = signature_consent_reason(entry) + if reason == 'untrusted_key': + return reason, ( + f"This extension is signed, but its publisher key " + f"'{entry.get('publisher_key_id')}' is not pinned by this panel, " + f"so the signature cannot be verified. Install only if you trust " + f"the source.") + return reason, _GATE_MESSAGES.get(reason) diff --git a/backend/tests/test_extension_signing.py b/backend/tests/test_extension_signing.py index 58b621e2..0b548212 100644 --- a/backend/tests/test_extension_signing.py +++ b/backend/tests/test_extension_signing.py @@ -518,3 +518,256 @@ def test_install_endpoint_rejects_bad_signature( assert resp.status_code == 400 assert 'Signature verification failed' in resp.get_json()['error'] assert InstalledPlugin.query.filter_by(slug='signed-ext').first() is None + + +# --------------------------------------------------------------------------- # +# Audit M1 — a reinstall via another source must not keep a stale verdict +# --------------------------------------------------------------------------- # + +def test_unsigned_reinstall_clears_verified_badge( + app, plugin_dirs, tmp_path, monkeypatch): + """Signed URL install → verified. Reinstalling the same slug via upload + (no signature possible) must re-stamp the verdict, never leave a forged + 'verified' badge behind.""" + priv = Ed25519PrivateKey.generate() + _pin_keys(tmp_path, monkeypatch, [('test-pub', priv, 'Test Publisher')]) + zip_bytes = _make_plugin_zip() + sig = signing_service.sign_bytes(zip_bytes, priv) + monkeypatch.setattr(plugin_service, '_download_zip', lambda url: io.BytesIO(zip_bytes)) + + plugin = plugin_service.install_from_url( + 'https://x/signed-ext.zip', expected_signature=sig, expected_key_id='test-pub') + assert plugin.config['_signature']['status'] == 'verified' + + # Same slug, upload path (force, as the update flow does). + plugin = plugin_service._install_from_buffer( + io.BytesIO(zip_bytes), source_url='uploaded.zip', source_type='upload', + force=True) + db.session.refresh(plugin) + assert plugin.config['_signature'] == {'status': 'unsigned'} + + +def test_local_reinstall_clears_verified_badge( + app, plugin_dirs, tmp_path, monkeypatch): + """Same as above but through the public install_from_zip upload helper.""" + priv = Ed25519PrivateKey.generate() + _pin_keys(tmp_path, monkeypatch, [('test-pub', priv, 'Test Publisher')]) + zip_bytes = _make_plugin_zip() + sig = signing_service.sign_bytes(zip_bytes, priv) + monkeypatch.setattr(plugin_service, '_download_zip', lambda url: io.BytesIO(zip_bytes)) + + plugin = plugin_service.install_from_url( + 'https://x/signed-ext.zip', expected_signature=sig, expected_key_id='test-pub') + assert plugin.config['_signature']['status'] == 'verified' + + # Uninstall then reinstall via upload (the ordinary re-install path). + plugin_service.uninstall_plugin(plugin.id) + plugin = plugin_service.install_from_zip(zip_bytes, source_name='signed-ext.zip') + assert plugin.config['_signature'] == {'status': 'unsigned'} + + +# --------------------------------------------------------------------------- # +# Audit M2 — the update route passes the same consent gates as install +# --------------------------------------------------------------------------- # + +def _install_v1(app, plugin_dirs, monkeypatch, slug='upd-ext'): + """Install v1 of a plugin so the update route has a row to work on.""" + v1 = _make_plugin_zip(slug=slug, version='1.0.0') + monkeypatch.setattr(plugin_service, '_download_zip', lambda url: io.BytesIO(v1)) + return plugin_service.install_from_url(f'https://x/{slug}.zip') + + +def _seed_update_entry(monkeypatch, slug, version='2.0.0', **overrides): + base = { + 'slug': slug, 'display_name': 'Upd', 'version': version, + 'source': f'https://x/{slug}.zip', + 'min_panel_version': '0.0.1', + } + base.update(overrides) + monkeypatch.setattr(registry_service, '_cache', { + 'ts': 9e18, 'source': 'test', + 'entries': [registry_service._normalize(base)], + }) + + +def test_update_route_hidden_unreviewed_is_404( + app, client, auth_headers, plugin_dirs, monkeypatch): + plugin = _install_v1(app, plugin_dirs, monkeypatch) + _seed_update_entry(monkeypatch, 'upd-ext', sha256='d' * 64) # → unreviewed + monkeypatch.setattr(registry_service, '_show_unreviewed', lambda: False) + + resp = client.post(f'/api/v1/plugins/{plugin.id}/update', headers=auth_headers) + assert resp.status_code == 404 + + +def test_update_route_unsigned_first_party_409_until_acknowledged( + app, client, auth_headers, plugin_dirs, monkeypatch): + plugin = _install_v1(app, plugin_dirs, monkeypatch) + v2 = _make_plugin_zip(slug='upd-ext', version='2.0.0') + _seed_update_entry(monkeypatch, 'upd-ext', + sha256=hashlib.sha256(v2).hexdigest(), first_party=True) + monkeypatch.setattr(plugin_service, '_download_zip', lambda url: io.BytesIO(v2)) + + resp = client.post(f'/api/v1/plugins/{plugin.id}/update', headers=auth_headers) + assert resp.status_code == 409 + data = resp.get_json() + assert data['requires_acknowledgment'] is True + assert data['reason'] == 'unsigned' + + resp = client.post(f'/api/v1/plugins/{plugin.id}/update', + headers=auth_headers, json={'acknowledge_risk': True}) + assert resp.status_code == 200 + assert resp.get_json()['version'] == '2.0.0' + + +def test_update_route_signed_installs_without_ack( + app, client, auth_headers, plugin_dirs, tmp_path, monkeypatch): + priv = Ed25519PrivateKey.generate() + _pin_keys(tmp_path, monkeypatch, [('test-pub', priv, 'Test Publisher')]) + plugin = _install_v1(app, plugin_dirs, monkeypatch) + v2 = _make_plugin_zip(slug='upd-ext', version='2.0.0') + _seed_update_entry( + monkeypatch, 'upd-ext', sha256=hashlib.sha256(v2).hexdigest(), + first_party=True, + signature=signing_service.sign_bytes(v2, priv), + publisher_key_id='test-pub') + monkeypatch.setattr(plugin_service, '_download_zip', lambda url: io.BytesIO(v2)) + + resp = client.post(f'/api/v1/plugins/{plugin.id}/update', headers=auth_headers) + assert resp.status_code == 200 + assert resp.get_json()['version'] == '2.0.0' + + +def test_update_route_bad_signature_is_400( + app, client, auth_headers, plugin_dirs, tmp_path, monkeypatch): + priv = Ed25519PrivateKey.generate() + _pin_keys(tmp_path, monkeypatch, [('test-pub', priv, 'Test Publisher')]) + plugin = _install_v1(app, plugin_dirs, monkeypatch) + v2 = _make_plugin_zip(slug='upd-ext', version='2.0.0') + bad_sig = signing_service.sign_bytes(b'other bytes', priv) + _seed_update_entry( + monkeypatch, 'upd-ext', sha256=hashlib.sha256(v2).hexdigest(), + first_party=True, signature=bad_sig, publisher_key_id='test-pub') + monkeypatch.setattr(plugin_service, '_download_zip', lambda url: io.BytesIO(v2)) + + resp = client.post(f'/api/v1/plugins/{plugin.id}/update', headers=auth_headers) + assert resp.status_code == 400 + assert 'Signature verification failed' in resp.get_json()['error'] + # The installed v1 row was not clobbered. + assert InstalledPlugin.query.get(plugin.id).version == '1.0.0' + + +# --------------------------------------------------------------------------- # +# Audit L4 — download SSRF guards + byte caps +# --------------------------------------------------------------------------- # + +def test_download_rejects_non_http_schemes(app, plugin_dirs): + with pytest.raises(ValueError, match='Failed to download plugin'): + plugin_service.install_from_url('file:///etc/passwd') + with pytest.raises(ValueError, match='scheme'): + plugin_service._assert_url_safe('file:///etc/passwd') + + +def test_download_rejects_redirect_to_link_local(app, monkeypatch): + """A 302 into the cloud metadata service must be refused on the second hop + (redirects are followed manually and re-checked).""" + + class FakeRedirect: + status_code = 302 + headers = {'Location': 'http://169.254.169.254/latest/meta-data'} + def close(self): pass + + monkeypatch.setattr(plugin_service.requests, 'get', + lambda *a, **k: FakeRedirect()) + import socket as _socket + monkeypatch.setattr(_socket, 'getaddrinfo', + lambda host, port, *a, **k: [(2, 1, 6, '', ('93.184.216.34', 0))]) + + with pytest.raises(ValueError, match='Plain-http|non-public'): + plugin_service._download_resolved('https://example.com/ext.zip') + + +def test_download_rejects_private_and_plain_http_targets(app): + with pytest.raises(ValueError, match='Plain-http'): + plugin_service._assert_url_safe('http://192.168.1.10/ext.zip') + with pytest.raises(ValueError, match='non-public'): + plugin_service._assert_url_safe('https://192.168.1.10/ext.zip') + with pytest.raises(ValueError, match='non-public'): + plugin_service._assert_url_safe('https://169.254.169.254/latest') + # Loopback dev hosts stay allowed (http and https). + plugin_service._assert_url_safe('http://127.0.0.1:8000/ext.zip') + plugin_service._assert_url_safe('http://localhost:8000/ext.zip') + + +def test_download_aborts_past_byte_cap(app, monkeypatch): + monkeypatch.setattr(plugin_service, '_MAX_DOWNLOAD_BYTES', 1024) + + class FakeResp: + status_code = 200 + def raise_for_status(self): pass + def iter_content(self, chunk_size=8192): + return [b'x' * 1024, b'x' * 1024] + def close(self): pass + + monkeypatch.setattr(plugin_service.requests, 'get', lambda *a, **k: FakeResp()) + import socket as _socket + monkeypatch.setattr(_socket, 'getaddrinfo', + lambda host, port, *a, **k: [(2, 1, 6, '', ('93.184.216.34', 0))]) + + with pytest.raises(ValueError, match='cap'): + plugin_service._download_resolved('https://example.com/ext.zip') + + +# --------------------------------------------------------------------------- # +# Audit L5 — decompression cap +# --------------------------------------------------------------------------- # + +def test_extraction_cap_aborts_and_leaves_no_residue( + app, plugin_dirs, monkeypatch): + # Tiny cap so the declared-size pre-check trips on an ordinary plugin zip. + monkeypatch.setattr(plugin_service, '_MAX_EXTRACT_BYTES', 16) + zip_bytes = _make_plugin_zip() + + with pytest.raises(ValueError, match='zip bomb|cap'): + plugin_service.install_from_zip(zip_bytes) + + plugin = InstalledPlugin.query.filter_by(slug='signed-ext').first() + assert plugin is not None and plugin.status == InstalledPlugin.STATUS_ERROR + assert not (plugin_dirs['backend'] / 'signed-ext').exists() + assert not (plugin_dirs['frontend'] / 'signed-ext').exists() + + +# --------------------------------------------------------------------------- # +# Audit L6 — the GitHub token never travels over plain http +# --------------------------------------------------------------------------- # + +def test_github_host_matching_is_https_only(): + assert plugin_service._is_github_host('https://github.com/o/r') is True + assert plugin_service._is_github_host('https://api.github.com/x') is True + assert plugin_service._is_github_host('http://github.com/o/r') is False + assert plugin_service._is_github_host('https://github.com.evil.test/') is False + + +# --------------------------------------------------------------------------- # +# Audit I7 — the install/update audit-log entries carry the signature verdict +# --------------------------------------------------------------------------- # + +def test_install_audit_log_records_signature_verdict( + app, client, auth_headers, plugin_dirs, tmp_path, monkeypatch): + from app.models.audit_log import AuditLog + priv = Ed25519PrivateKey.generate() + _pin_keys(tmp_path, monkeypatch, [('test-pub', priv, 'Test Publisher')]) + zip_bytes = _make_plugin_zip() + sig = signing_service.sign_bytes(zip_bytes, priv) + monkeypatch.setattr(plugin_service, '_download_zip', lambda url: io.BytesIO(zip_bytes)) + + resp = client.post('/api/v1/plugins/install', headers=auth_headers, json={ + 'url': 'https://x/signed-ext.zip', + 'signature': sig, + 'publisher_key_id': 'test-pub', + }) + assert resp.status_code == 201 + row = (AuditLog.query.filter_by(target_type='plugin') + .order_by(AuditLog.id.desc()).first()) + assert row is not None + assert row.get_details().get('signature') == 'verified' diff --git a/backend/tests/test_plugin_preview.py b/backend/tests/test_plugin_preview.py index decb0eb4..81a95833 100644 --- a/backend/tests/test_plugin_preview.py +++ b/backend/tests/test_plugin_preview.py @@ -190,6 +190,11 @@ def test_download_token_only_for_github_hosts(monkeypatch): return FakeResp() monkeypatch.setattr(plugin_service.requests, 'get', fake_get) + # The SSRF guard resolves hosts before fetching; stub DNS to a public IP + # so this test stays offline (its subject is token scoping, not DNS). + import socket as _socket + monkeypatch.setattr(_socket, 'getaddrinfo', + lambda host, port, *a, **k: [(2, 1, 6, '', ('93.184.216.34', 0))]) plugin_service._download_resolved('https://github.com/o/r/releases/download/v1/x.zip') plugin_service._download_resolved('https://evil.example.com/x.zip') diff --git a/backend/tests/test_registry.py b/backend/tests/test_registry.py index e1badd16..ac179b0f 100644 --- a/backend/tests/test_registry.py +++ b/backend/tests/test_registry.py @@ -459,10 +459,20 @@ def test_install_gate_passes_with_acknowledge_risk(app, client, auth_headers, fa assert resp.status_code == 201 -def test_install_gate_unaffected_for_first_party(app, client, auth_headers, fake_install, monkeypatch): +def test_install_gate_unsigned_first_party_requires_ack(app, client, auth_headers, fake_install, monkeypatch): + # Audit M3: a first-party entry without a verifiable signature is a + # possible downgrade — consent-gated like untrusted_key. _seed_cache(monkeypatch, _entry(first_party=True)) resp = client.post('/api/v1/marketplace/registry/community-ext/install', headers=auth_headers) + assert resp.status_code == 409 + data = resp.get_json() + assert data['requires_acknowledgment'] is True + assert data['reason'] == 'unsigned' + assert 'first-party' in data['error'] + + resp = client.post('/api/v1/marketplace/registry/community-ext/install', + headers=auth_headers, json={'acknowledge_risk': True}) assert resp.status_code == 201 diff --git a/frontend/src/pages/Marketplace.jsx b/frontend/src/pages/Marketplace.jsx index 61b88073..798570d1 100644 --- a/frontend/src/pages/Marketplace.jsx +++ b/frontend/src/pages/Marketplace.jsx @@ -318,8 +318,13 @@ const Marketplace = () => { const confirmRiskyInstall = () => { const slug = riskTarget?.slug; + const updatePluginId = riskTarget?.updatePluginId; setRiskTarget(null); - if (slug) handleRegistryInstall(slug, true); + if (updatePluginId) { + handlePluginUpdate(updatePluginId, true); + } else if (slug) { + handleRegistryInstall(slug, true); + } }; // Land on Installed after a manual install so the new row (and its @@ -350,15 +355,21 @@ const Marketplace = () => { } }; - const handlePluginUpdate = async (pluginId) => { + const handlePluginUpdate = async (pluginId, acknowledgeRisk = false) => { if (busyPlugin) return; setBusyPlugin({ id: pluginId, action: 'update' }); try { - const result = await api.updatePlugin(pluginId); + const result = await api.updatePlugin( + pluginId, acknowledgeRisk ? { acknowledge_risk: true } : undefined); toast.success(`Extension "${result.display_name}" updated to v${result.version}.`); await loadExtensions(); } catch (err) { - toast.error(err.message || 'Extension update failed'); + // 409 consent gate (audit M2) — same acknowledge flow as installs. + if (err.status === 409 && err.data?.requires_acknowledgment) { + setRiskTarget({ updatePluginId: pluginId, reason: err.data?.reason || 'unsigned' }); + } else { + toast.error(err.message || 'Extension update failed'); + } } finally { setBusyPlugin(null); } @@ -593,13 +604,15 @@ const Marketplace = () => { ? 'Install without checksum verification?' : riskTarget.reason === 'untrusted_key' ? 'Install with an unverifiable signature?' - : 'Install unreviewed extension?'} + : riskTarget.reason === 'unsigned' + ? `${riskTarget.updatePluginId ? 'Update' : 'Install'} an unsigned release?` + : 'Install unreviewed extension?'} size="sm" footer={ <> } @@ -613,8 +626,12 @@ const Marketplace = () => { ? 'This extension is signed, but the publisher key is not ' + 'pinned by this panel, so the signature cannot be ' + 'verified. Treat it as unsigned.' - : 'This is a community extension whose exact code has not ' - + 'been reviewed by the ServerKit maintainers.'} + : riskTarget.reason === 'unsigned' + ? 'This release is not signed with a publisher key pinned ' + + 'by this panel, so its origin cannot be verified. It ' + + 'may be a legitimate older release — or a downgrade attempt.' + : 'This is a community extension whose exact code has not ' + + 'been reviewed by the ServerKit maintainers.'}

It runs with full panel privileges. Only install it if you trust diff --git a/frontend/src/services/api/plugins.js b/frontend/src/services/api/plugins.js index feae3afa..6a9320bb 100644 --- a/frontend/src/services/api/plugins.js +++ b/frontend/src/services/api/plugins.js @@ -150,8 +150,11 @@ export async function getPluginUpdates() { } // Updates an installed plugin in place; returns the updated plugin dict. -export async function updatePlugin(pluginId) { +// Pass { acknowledge_risk: true } to proceed past the 409 consent gate +// (unsigned/untrusted-key/unreviewed registry versions — audit M2). +export async function updatePlugin(pluginId, body) { return this.request(`/plugins/${pluginId}/update`, { method: 'POST', + body: body ? JSON.stringify(body) : undefined, }); }