Conversation
Introduce a plugin installation and management system. Backend: add InstalledPlugin model, plugin_service with download/install/enable/disable/uninstall, dynamic blueprint registration and frontend manifest generation, and plugins API endpoints (list/get/install/enable/disable/uninstall). Register plugins blueprint and attempt to hot-load installed plugin blueprints at app startup. Frontend: add PluginLoader component (Vite import.meta.glob) to render plugin widgets, extend Marketplace UI to install/manage plugins, add API client methods for plugins, and update styles for the marketplace plugin UI. This enables installing plugins from GitHub/repos/zips and managing them from the app.
There was a problem hiding this comment.
Pull request overview
Adds an install/manage plugin system spanning backend (install, persist, dynamically register blueprints) and frontend (plugin UI + loader + API client), integrating plugin management into the existing Marketplace flow.
Changes:
- Backend: introduce
InstalledPluginmodel, plugin download/install/enable/disable/uninstall service, and/api/v1/pluginsendpoints; attempt to load active plugin blueprints at startup. - Frontend: add plugins API client methods, Marketplace “Plugins” tab with install/manage UI, and a
PluginLoaderto render plugin UI components. - Styling/version: extend Marketplace styles for the plugins UI and bump VERSION to 1.4.14.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/styles/pages/_marketplace.scss | Adds styles for the new Marketplace “Plugins” install/manage section. |
| frontend/src/services/api/plugins.js | Introduces frontend API methods for plugin management endpoints. |
| frontend/src/services/api/index.js | Registers the new plugin API methods into the ApiService. |
| frontend/src/plugins/PluginLoader.jsx | Adds Vite glob-based discovery and rendering of plugin UI components. |
| frontend/src/pages/Marketplace.jsx | Adds “Plugins” tab UI and actions to install/enable/disable/uninstall plugins. |
| frontend/src/layouts/DashboardLayout.jsx | Renders PluginLoader in the main dashboard layout. |
| backend/app/services/plugin_service.py | Implements plugin download, zip extraction, (optional) pip deps install, and blueprint registration + frontend manifest generation. |
| backend/app/models/plugin.py | Adds InstalledPlugin model and serialization. |
| backend/app/api/plugins.py | Adds plugin management API endpoints. |
| backend/app/init.py | Registers the plugins blueprint and loads plugin blueprints at startup. |
| VERSION | Bumps app version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const toast = useToast(); | ||
| const { user } = useAuth(); | ||
| const [extensions, setExtensions] = useState([]); | ||
| const [myExtensions, setMyExtensions] = useState([]); | ||
| const [plugins, setPlugins] = useState([]); |
There was a problem hiding this comment.
user is destructured from useAuth() but never used, which can trigger lint/build failures. Either remove it or use it to gate plugin install/enable/disable UI to admins (since the backend endpoints return 403 for non-admins).
| if rel_path.startswith('backend/'): | ||
| has_backend = True | ||
| out_path = os.path.join(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()) | ||
|
|
||
| elif rel_path.startswith('frontend/'): | ||
| has_frontend = True | ||
| out_path = os.path.join(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()) | ||
|
|
||
| elif rel_path == 'requirements.txt': |
There was a problem hiding this comment.
Zip extraction is vulnerable to path traversal (Zip Slip): rel_path comes from the archive and can contain ../ or absolute paths, allowing writes outside backend_dest/frontend_dest. Validate/sanitize each member path (e.g., reject absolute paths and .. segments, and ensure the resolved output path stays within the destination root) before writing.
| if rel_path.startswith('backend/'): | |
| has_backend = True | |
| out_path = os.path.join(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()) | |
| elif rel_path.startswith('frontend/'): | |
| has_frontend = True | |
| out_path = os.path.join(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()) | |
| elif rel_path == 'requirements.txt': | |
| # Basic Zip Slip protection: reject absolute paths and traversal segments | |
| normalized_rel = rel_path.replace('\\', '/') | |
| if os.path.isabs(normalized_rel) or '..' in normalized_rel.split('/'): | |
| logger.warning(f'Skipping suspicious zip member path in plugin {slug}: {rel_path}') | |
| continue | |
| if normalized_rel.startswith('backend/'): | |
| has_backend = True | |
| rel_backend = normalized_rel[len('backend/'):] | |
| out_path = os.path.normpath(os.path.join(backend_dest, rel_backend)) | |
| # Ensure the final path is still within backend_dest | |
| backend_root = os.path.join(backend_dest, '') | |
| if not out_path.startswith(backend_root): | |
| logger.warning(f'Skipping backend file escaping plugin directory in {slug}: {rel_path}') | |
| continue | |
| 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()) | |
| elif normalized_rel.startswith('frontend/'): | |
| has_frontend = True | |
| rel_frontend = normalized_rel[len('frontend/'):] | |
| out_path = os.path.normpath(os.path.join(frontend_dest, rel_frontend)) | |
| # Ensure the final path is still within frontend_dest | |
| frontend_root = os.path.join(frontend_dest, '') | |
| if not out_path.startswith(frontend_root): | |
| logger.warning(f'Skipping frontend file escaping plugin directory in {slug}: {rel_path}') | |
| continue | |
| 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()) | |
| elif normalized_rel == 'requirements.txt': |
| # Install Python dependencies | ||
| req_content = zf.read(member).decode('utf-8') | ||
| _install_requirements(req_content, slug) | ||
|
|
There was a problem hiding this comment.
Installing requirements.txt from an untrusted plugin archive runs arbitrary code with the ServerKit backend’s privileges (pip can execute setup hooks / build steps). Consider disabling this by default and/or installing into an isolated per-plugin virtualenv/container with allowlisted packages, plus clear admin warnings and logging.
| # Install Python dependencies | |
| req_content = zf.read(member).decode('utf-8') | |
| _install_requirements(req_content, slug) | |
| # Handle Python dependencies from plugin requirements.txt | |
| req_content = zf.read(member).decode('utf-8') | |
| # By default, do NOT install requirements from untrusted plugins, | |
| # as this may execute arbitrary code with backend privileges. | |
| allow_untrusted = os.getenv( | |
| "SERVERKIT_ENABLE_UNTRUSTED_PLUGIN_REQUIREMENTS", "" | |
| ).lower() in ("1", "true", "yes") | |
| if not allow_untrusted: | |
| logger.warning( | |
| "Skipping installation of requirements.txt for plugin '%s' " | |
| "because SERVERKIT_ENABLE_UNTRUSTED_PLUGIN_REQUIREMENTS is not enabled. " | |
| "The requirements file will be extracted for manual review/installation.", | |
| slug, | |
| ) | |
| # Optionally persist requirements.txt into the backend directory | |
| # so administrators can inspect and install dependencies manually. | |
| os.makedirs(backend_dest, exist_ok=True) | |
| req_out_path = os.path.join(backend_dest, "requirements.txt") | |
| with open(req_out_path, "w", encoding="utf-8") as req_file: | |
| req_file.write(req_content) | |
| else: | |
| logger.warning( | |
| "Installing requirements.txt for plugin '%s' as " | |
| "SERVERKIT_ENABLE_UNTRUSTED_PLUGIN_REQUIREMENTS is enabled. " | |
| "This may execute arbitrary code during package installation.", | |
| slug, | |
| ) | |
| _install_requirements(req_content, slug) |
| def enable_plugin(plugin_id): | ||
| """Enable a disabled plugin.""" | ||
| plugin = InstalledPlugin.query.get(plugin_id) | ||
| if not plugin: | ||
| return None | ||
| plugin.status = InstalledPlugin.STATUS_ACTIVE | ||
| plugin.error_message = None | ||
| db.session.commit() | ||
| _regenerate_frontend_manifest() | ||
| return plugin | ||
|
|
||
|
|
||
| def disable_plugin(plugin_id): | ||
| """Disable a plugin without removing files.""" | ||
| plugin = InstalledPlugin.query.get(plugin_id) | ||
| if not plugin: | ||
| return None | ||
| plugin.status = InstalledPlugin.STATUS_DISABLED | ||
| db.session.commit() | ||
| _regenerate_frontend_manifest() | ||
| return plugin |
There was a problem hiding this comment.
disable_plugin()/enable_plugin() only flip DB status + regenerate the frontend manifest; they do not actually disable/enable already-registered Flask blueprints. Once a plugin blueprint is registered, its backend routes will remain reachable until process restart, which makes the “disabled” state misleading (and potentially unsafe). Consider enforcing status at request time (middleware) or requiring/recommending a restart for backend changes.
| class InstalledPlugin(db.Model): | ||
| """Tracks plugins installed from external sources (zips/URLs).""" | ||
| __tablename__ = 'installed_plugins' | ||
|
|
||
| id = db.Column(db.Integer, primary_key=True) | ||
| name = db.Column(db.String(128), nullable=False, unique=True) | ||
| display_name = db.Column(db.String(256), nullable=False) | ||
| slug = db.Column(db.String(128), nullable=False, unique=True) | ||
| version = db.Column(db.String(32), nullable=False) |
There was a problem hiding this comment.
A new SQLAlchemy model/table is introduced, but there is no Alembic migration for installed_plugins, and the model isn’t imported in app/models/__init__.py (used for model discovery). Add a migration revision and include InstalledPlugin in the models package exports so migrations and metadata include it.
| plugin.status = InstalledPlugin.STATUS_INSTALLING | ||
| plugin.error_message = None | ||
| plugin.version = manifest['version'] | ||
| plugin.source_url = url |
There was a problem hiding this comment.
When reinstalling an existing plugin record, only status, version, source_url, and manifest are updated. Fields like display_name, description, author, etc. can become stale if the new manifest changes them; update these fields from the manifest during reinstall to keep the DB consistent.
| plugin.source_url = url | |
| plugin.source_url = url | |
| plugin.name = manifest['name'] | |
| plugin.display_name = manifest['display_name'] | |
| plugin.description = manifest.get('description', '') | |
| plugin.author = manifest.get('author', '') | |
| plugin.homepage = manifest.get('homepage', '') | |
| plugin.repository = manifest.get('repository', '') | |
| plugin.license = manifest.get('license', '') | |
| plugin.category = manifest.get('category', 'utility') |
| except ValueError as e: | ||
| return jsonify({'error': str(e)}), 400 | ||
| except Exception as e: | ||
| return jsonify({'error': f'Installation failed: {e}'}), 500 |
There was a problem hiding this comment.
The 500-path returns the raw exception string in the response (Installation failed: {e}), which can leak internal details. Log the exception server-side and return a generic message (optionally with a correlation id) while storing the detailed error in InstalledPlugin.error_message.
| * - A default component (the widget/UI to render) | ||
| * - Optionally a Provider component for context wrapping | ||
| */ | ||
| import React, { Suspense, useMemo } from 'react'; |
There was a problem hiding this comment.
Suspense is imported but never used, which will fail linting/build in many setups. Remove the unused import or actually wrap plugin rendering in a <Suspense> boundary if you plan to lazy-load plugin components.
| import React, { Suspense, useMemo } from 'react'; | |
| import React, { useMemo } from 'react'; |
| // Plugin management API methods | ||
|
|
||
| export async function getInstalledPlugins(status) { | ||
| const params = status ? `?status=${status}` : ''; |
There was a problem hiding this comment.
Query param values should be URL-encoded. status is interpolated directly into the query string here; use encodeURIComponent(status) to avoid malformed URLs (and to match existing patterns like encodeURIComponent used elsewhere).
| const params = status ? `?status=${status}` : ''; | |
| const params = status ? `?status=${encodeURIComponent(status)}` : ''; |
| # Load installed plugins (dynamic blueprints) | ||
| try: | ||
| from app.services.plugin_service import load_all_plugins | ||
| load_all_plugins(app) | ||
| except Exception as e: | ||
| import logging | ||
| logging.getLogger(__name__).warning(f'Plugin loader: {e}') |
There was a problem hiding this comment.
load_all_plugins(app) is invoked before Alembic migrations run. On upgrade (when installed_plugins table doesn’t exist yet), this will fail and plugins won’t be loaded even after migrations are applied later in the same startup. Move plugin loading to after MigrationService.check_and_prepare(app) (or rerun after migrations) so plugin blueprints reliably register.
Introduce a comprehensive Style Guide (frontend/src/pages/StyleGuide.jsx) as a developer-only design system reference, plus new SCSS modules (_style-guide.scss, _alerts.scss, _tables.scss) and updates to existing component styles. Register the page route and title in App.jsx and add a DEV-only sidebar link (import.meta.env.DEV) in Sidebar.jsx; also import SIDEBAR_ITEMS from sidebarItems. These changes centralize UI patterns, components and utilities for easier visual QA and consistent styling without exposing the guide in production builds.
Add loading and size props to EmptyState (imports Spinner, shows spinner when loading, adjusts icon sizes) and add a large variant styling. Update StyleGuide to demonstrate loading, large/not-installed/unavailable states, and card contexts. Refactor SCSS across pages/components to use Sass theme variables ($bg-card, $border-subtle, $text-secondary, $accent-primary, etc.) replacing many var(...) usages and tweak related spacing/typography styles.
Introduce a plugin installation and management system.
Backend: add InstalledPlugin model, plugin_service with download/install/enable/disable/uninstall, dynamic blueprint registration and frontend manifest generation, and plugins API endpoints (list/get/install/enable/disable/uninstall). Register plugins blueprint and attempt to hot-load installed plugin blueprints at app startup.
Frontend: add PluginLoader component (Vite import.meta.glob) to render plugin widgets, extend Marketplace UI to install/manage plugins, add API client methods for plugins, and update styles for the marketplace plugin UI. This enables installing plugins from GitHub/repos/zips and managing them from the app.