feat(plugins): custom hooks + registry unification + SDK + marketplace + dashboard [deferred to v4.0.0-rc1]

[oyi77]

Summary

Plugin system improvements for v4.0.0-rc1. Extends PR #2385 with:

Custom Hooks (Priority 1+2)

• emitHookBlocking() with body/metadata chaining for blocking events
• runOnRequest, runOnResponse, runOnError lifecycle wrappers
• Unified hook registry — hooks.ts is canonical, index.ts is re-export shim
• runOnResponse wired in chatCore.ts (was missing)

SDK

• sdk.ts: definePlugin() factory + blockRequest, modifyBody, addMetadata helpers
• Dashboard plugin list page with scan/activate/deactivate/uninstall
• Config page with auto-generated form from configSchema

Marketplace

• marketplace.ts: local plugin registry with 3 seed plugins
• Phase 1: local data. Phase 2: remote registry.

Tests

• 19 hooks.ts unit tests
• 17 integration tests (full lifecycle)

Test plan

• npm run typecheck:core — 0 errors
• node --import tsx/esm --test tests/unit/plugins-hooks.test.ts — 19/19 pass
• node --import tsx/esm --test tests/integration/plugins-lifecycle.test.ts — 17/17 pass

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive plugin management system, including lifecycle tracking, a custom hook registry, sandboxed loading, and dashboard UI pages, while removing legacy gamification and token limit features. The review feedback highlights critical security vulnerabilities, specifically sandbox escapes through the Node.js vm module and path traversal risks in the plugin scanner. Additionally, the feedback addresses functional issues such as broken hook chaining in emitHookBlocking, performance overhead from dynamic imports in chatCore.ts, and silent UI failures during plugin activation.

[gemini-code-assist]

The Node.js vm module is not a secure sandbox. Untrusted plugins running in vm can easily escape the sandbox and execute arbitrary code on the host system (e.g., by accessing the constructor of standard objects to run arbitrary shell commands). If this plugin system is intended to support third-party plugins or a marketplace, using vm poses a critical security risk. Consider using a more secure isolation library like isolated-vm or running plugins in separate worker threads/processes with restricted permissions.

[gemini-code-assist]

In emitHookBlocking, subsequent hook handlers are called with the original payload object. If a previous handler modified the request body or metadata, those modifications are accumulated in mergedBody and mergedMetadata but are not passed to the next handler in the chain. This breaks hook chaining. We should pass an updated copy of the payload to each handler.

  for (const reg of list.sort((a, b) => a.priority - b.priority)) {
    try {
      const currentPayload = { ...ctx, body: mergedBody, metadata: mergedMetadata };
      const result = await reg.handler(currentPayload);

[gemini-code-assist]

There is a potential path traversal vulnerability when resolving the plugin's entry point. If a malicious or malformed plugin.json specifies a main path containing directory traversal sequences (e.g., ../../malicious.js), join(pluginDir, manifest.main) will resolve to a path outside the plugin's directory. We should validate that the resolved entryPoint is strictly within pluginDir.

      const manifest = result.data;
      const entryPoint = join(pluginDir, manifest.main);

      if (!entryPoint.startsWith(pluginDir)) {
        errors.push({
          name: entry,
          error: `entry point must be within the plugin directory`,
        });
        continue;
      }

[gemini-code-assist]

Importing @/lib/plugins/hooks dynamically on every request (and doing so multiple times for different hooks like runOnRequest, runOnResponse, runOnError, and emitHook) introduces unnecessary overhead. Since there are no circular dependencies, these should be statically imported at the top of the file for better performance.

[gemini-code-assist]

In handleToggle, if the fetch request returns a non-OK status (e.g., 400 Bad Request because activation failed), the promise still resolves successfully, so the catch block is not triggered. As a result, no error notification is shown, and the failure is silently ignored in the UI. We should handle res.ok explicitly to show an error notification.

      if (res.ok) {
        addNotification({ type: "success", message: `${name} ${enable ? "activated" : "deactivated"}` });
        await fetchPlugins();
      } else {
        addNotification({ type: "error", message: `Failed to ${enable ? "activate" : "deactivate"} ${name}` });
      }

[kilo-code-bot]

CRITICAL: DEFAULT value 0 for feasibility conflicts with CHECK constraint requiring values BETWEEN 1 AND 5. Rows using the default will violate the constraint and fail to insert.

Either change DEFAULT to 1, or update the CHECK to allow 0: CHECK (feasibility >= 0 AND feasibility <= 5).

[kilo-code-bot]

WARNING: Potential directory traversal via absolute path in file operations. The plugin is granted file-read permission but can use absolute paths to read/write outside the plugin directory. The sandbox should restrict paths to be within pluginDir.

Explanation: The resolve(pluginDir, p) function will return p if it is an absolute path, allowing plugins to bypass the intended directory restriction.

[diegosouzapw]

Hi @oyi77 — thank you for the ambitious plugin system work here! 🙏

We're keeping this deferred to v4.0.0-rc1 as you flagged. While reviewing it for integration I want to flag one important security item and a concrete path forward.

🔒 Loader sandbox regression. On this branch src/lib/plugins/loader.ts and src/lib/plugins/pluginWorker.ts execute plugin code via vm.createContext / vm.runInContext, and pluginWorker.ts also exposes child_process.exec/execSync into the sandbox. That reverts the process-level isolation that the already-merged PR #2912 established (each plugin runs in a separate child_process over IPC with a permission-filtered env). vm + exposed exec together would let a plugin run arbitrary host commands, so we can't ship the loader from this branch as-is. It looks like it came from the merge history — the branch carries both the old vm loader and #2912's child_process fix, and the vm one won the merge.

Path forward for v4: could you rebase this onto the merged #2912 backend so the loader stays child_process-based, and build the new pieces on top of it? The genuinely new value here is excellent and we definitely want it:

• the unified hook registry (index.ts → re-export shim over hooks.ts) — this is the key fix that makes activated plugins actually fire on requests (today the two registries are disconnected)
• runOnResponse wiring in chatCore.ts
• the SDK (definePlugin), per-hook metrics, and the dashboard pages

One small robustness note: recordPluginMetric should be best-effort (wrap in try/catch) so a DB hiccup never breaks hook execution.

Happy to help coordinate the v4 integration — thanks again for pushing this forward! 🚀

[diegosouzapw]

Hi @oyi77 — thank you for this substantial contribution! 🙏

Your plugins framework has been integrated into release/v3.8.8 via #3041 (merged as 20c31493a). During integration I resolved the migration-numbering collisions (your 077 clashed with the release → renumbered to 089/090/091), added the missing plugin_analytics migration, and hardened the sandbox for production:

• /api/plugins/* is now classified loopback-only (isLocalOnlyPath) in the route guard, closing a remote process-spawn vector
• child_process exec is opt-in behind OMNIROUTE_PLUGINS_ALLOW_EXEC (default off); the worker sandbox keeps its vm timeout
• atomic install/upgrade with path containment (assertWithinPluginDir), O_EXCL tmp-file creation (TOCTOU fix), rate-limit-map eviction, validatePluginConfig on configure, and buildErrorBody on every error path

Because the work shipped through the combined integration branch rather than a direct merge of this PR, I'm closing this one — but you're credited in the CHANGELOG (#2913 — thanks @oyi77) and as a co-author on the squash commit. It'll be part of the upcoming v3.8.8 release. Thanks again for the excellent work! 🚀