From 31a615c970080ab2e053f6f4397c4fa7962cb063 Mon Sep 17 00:00:00 2001 From: oyi77 Date: Fri, 29 May 2026 22:34:32 +0700 Subject: [PATCH] fix(plugins): security + ESM fixes for loader and manager loader.ts: - Fix IPC: use process.send()/process.on("message") instead of worker_threads.parentPort - Fix ESM: write host script as .mjs (not .js) to force ESM execution - Add timeout: 10s default on callHook() with Promise.race - Add SIGKILL escalation: SIGTERM first, then SIGKILL after 3s grace - Fix env filtering: use allowlist (safeKeys) instead of passing all env vars - Clear timeout on successful IPC response (no timer leak) manager.ts: - Fix path traversal: use fs.realpath() instead of startsWith() - Fix imports: use registerHook/unregisterHooks from hooks.ts - Register hooks individually via registerHook(event, name, handler) hooks.ts: - Copied from feat/plugin-custom-hooks (canonical registry) --- src/lib/plugins/hooks.ts | 265 +++++++++++++++++++++++++++++++++++++ src/lib/plugins/loader.ts | 156 +++++++++++++--------- src/lib/plugins/manager.ts | 30 ++++- 3 files changed, 383 insertions(+), 68 deletions(-) create mode 100644 src/lib/plugins/hooks.ts diff --git a/src/lib/plugins/hooks.ts b/src/lib/plugins/hooks.ts new file mode 100644 index 0000000000..c82c7becb6 --- /dev/null +++ b/src/lib/plugins/hooks.ts @@ -0,0 +1,265 @@ +/** + * Custom hook registry — event-driven plugin hook system. + * + * Plugins can register handlers for any OmniRoute event. Built-in events + * cover the full request lifecycle plus routing, rate limiting, and errors. + * + * @module plugins/hooks + */ + +import { logger } from "../../../open-sse/utils/logger.ts"; + +const log = logger("PLUGIN_HOOKS"); + +// ── Types ── + +export type BlockingHookResult = { + blocked?: boolean; + response?: unknown; + body?: unknown; + metadata?: Record; +}; + +export type HookHandler = ( + payload: unknown +) => void | Promise | BlockingHookResult | Promise; + +export interface HookRegistration { + pluginName: string; + handler: HookHandler; + priority: number; +} + +// ── Built-in events ── + +export const BUILTIN_EVENTS = [ + "onRequest", + "onResponse", + "onError", + "onModelSelect", + "onComboResolve", + "onRateLimit", + "onQuotaExhaust", + "onProviderError", + "onStreamStart", + "onStreamEnd", +] as const; + +export type BuiltinEvent = (typeof BUILTIN_EVENTS)[number]; + +// ── Registry ── + +const hooks: Map = new Map(); + +/** + * Register a handler for an event. + */ +export function registerHook( + event: string, + pluginName: string, + handler: HookHandler, + priority: number = 100 +): void { + if (!hooks.has(event)) { + hooks.set(event, []); + } + const list = hooks.get(event)!; + + // Prevent duplicate registration + if (list.some((r) => r.pluginName === pluginName && r.handler === handler)) { + return; + } + + list.push({ pluginName, handler, priority }); + list.sort((a, b) => a.priority - b.priority); + + log.info("hook.registered", { event, pluginName, priority }); +} + +/** + * Unregister all handlers for a plugin. + */ +export function unregisterHooks(pluginName: string): void { + for (const [event, list] of hooks.entries()) { + const before = list.length; + const filtered = list.filter((r) => r.pluginName !== pluginName); + if (filtered.length !== before) { + hooks.set(event, filtered); + log.info("hook.unregistered", { event, pluginName, removed: before - filtered.length }); + } + } +} + +/** + * Unregister a specific handler. + */ +export function unregisterHook(event: string, pluginName: string): void { + const list = hooks.get(event); + if (!list) return; + const before = list.length; + const filtered = list.filter((r) => r.pluginName !== pluginName); + hooks.set(event, filtered); + if (before !== filtered.length) { + log.info("hook.unregistered", { event, pluginName }); + } +} + +/** + * Emit an event — fire all registered handlers. + * Handler errors are logged but don't block other handlers. + */ +export async function emitHook(event: string, payload: unknown): Promise { + const list = hooks.get(event); + if (!list || list.length === 0) return; + + for (const reg of list) { + try { + await reg.handler(payload); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + log.error("hook.handler_error", { + event, + pluginName: reg.pluginName, + error: message, + }); + } + } +} + +/** + * Emit a blocking event — fire handlers with body/metadata chaining. + * Returns blocking result from the first handler that blocks, or merged body/metadata. + * Used for onRequest and onResponse where plugins can modify or block the request. + */ +export async function emitHookBlocking( + event: string, + payload: unknown +): Promise<{ + blocked?: boolean; + response?: unknown; + body?: unknown; + metadata?: Record; +}> { + const list = hooks.get(event) || []; + const ctx = (payload || {}) as Record; + let mergedBody: unknown = ctx.body; + let mergedMetadata: Record = (ctx.metadata as Record) || {}; + + for (const reg of list.sort((a, b) => a.priority - b.priority)) { + try { + const result = await reg.handler(payload); + if (result && typeof result === "object") { + if ("body" in result) mergedBody = (result as Record).body; + if ("metadata" in result) + mergedMetadata = { + ...mergedMetadata, + ...(((result as Record).metadata as Record) || {}), + }; + if ("blocked" in result && (result as BlockingHookResult).blocked) { + return { + ...result, + body: (result as BlockingHookResult).body ?? mergedBody, + metadata: { ...mergedMetadata, ...((result as BlockingHookResult).metadata || {}) }, + }; + } + } + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + log.error("hook.blocking_handler_error", { + event, + pluginName: reg.pluginName, + error: message, + }); + } + } + return { body: mergedBody, metadata: mergedMetadata }; +} + +// ── Lifecycle wrappers (for chatCore.ts convenience) ── + +export interface PluginContext { + requestId: string; + body: unknown; + model: string; + provider: string; + apiKeyInfo?: unknown; + metadata: Record; +} + +export interface PluginResult { + blocked?: boolean; + response?: unknown; + body?: unknown; + metadata?: Record; +} + +// ── Plugin interface (for loader/manager compatibility) ── + +export interface Plugin { + name: string; + priority?: number; + enabled?: boolean; + onRequest?: (ctx: PluginContext) => Promise | PluginResult | void; + onResponse?: (ctx: PluginContext, response: unknown) => Promise | unknown | void; + onError?: (ctx: PluginContext, error: Error) => Promise | unknown | void; +} + +/** + * Run onRequest hooks — blocking. Plugins can modify body/metadata or block with 403. + */ +export async function runOnRequest(ctx: PluginContext): Promise { + return emitHookBlocking("onRequest", ctx); +} + +/** + * Run onResponse hooks — chains response through plugins. Each plugin can modify the response. + */ +export async function runOnResponse(ctx: PluginContext, response: unknown): Promise { + let currentResponse = response; + const list = hooks.get("onResponse") || []; + for (const reg of list.sort((a, b) => a.priority - b.priority)) { + try { + const result = await reg.handler({ ...ctx, response: currentResponse }); + if ( + result !== undefined && + result !== null && + typeof result === "object" && + "response" in result + ) { + currentResponse = (result as { response: unknown }).response; + } + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + log.error("hook.response_handler_error", { pluginName: reg.pluginName, error: message }); + } + } + return currentResponse; +} + +/** + * Run onError hooks — fire-and-forget notification. + */ +export async function runOnError(ctx: PluginContext, error: Error): Promise { + await emitHook("onError", { ...ctx, error }); +} + +/** + * Get all registered hooks for an event. + */ +export function getHooks(event: string): HookRegistration[] { + return hooks.get(event) ?? []; +} + +/** + * Get all events that have registered handlers. + */ +export function getActiveEvents(): string[] { + return [...hooks.entries()].filter(([, list]) => list.length > 0).map(([event]) => event); +} + +/** + * Reset all hooks (for testing). + */ +export function resetHooks(): void { + hooks.clear(); +} diff --git a/src/lib/plugins/loader.ts b/src/lib/plugins/loader.ts index 7209251184..b6a60181da 100644 --- a/src/lib/plugins/loader.ts +++ b/src/lib/plugins/loader.ts @@ -19,6 +19,9 @@ import type { Plugin, PluginContext, PluginResult } from "./index"; const log = logger("PLUGIN_LOADER"); +const DEFAULT_HOOK_TIMEOUT = 10_000; +const SIGKILL_GRACE_MS = 3_000; + export interface LoadedPlugin { name: string; manifest: PluginManifestWithDefaults; @@ -26,33 +29,34 @@ export interface LoadedPlugin { cleanup: () => void; } -// ── Plugin host script (runs in child process) ── +// ── Plugin host script (runs in child process via fork) ── +// Uses process.send()/process.on("message") — NOT worker_threads. +// Written as .mjs to force ESM execution regardless of package.json. const PLUGIN_HOST_SCRIPT = ` -const { parentPort } = require("worker_threads"); -const path = require("path"); +import { createRequire } from "node:module"; +const require = createRequire(import.meta.url); -// Load the plugin module const pluginPath = process.argv[2]; -const plugin = require(pluginPath); +const plugin = await import(pluginPath); const exports = plugin.default || plugin; // Send ready signal -parentPort.postMessage({ type: "ready", hooks: Object.keys(exports).filter(k => typeof exports[k] === "function") }); +process.send({ type: "ready", hooks: Object.keys(exports).filter(k => typeof exports[k] === "function") }); // Handle messages from parent -parentPort.on("message", async (msg) => { +process.on("message", async (msg) => { if (msg.type === "call") { try { const handler = exports[msg.hook]; if (typeof handler !== "function") { - parentPort.postMessage({ type: "result", id: msg.id, error: "Hook not found" }); + process.send({ type: "result", id: msg.id, error: "Hook not found" }); return; } const result = await handler(msg.payload); - parentPort.postMessage({ type: "result", id: msg.id, result }); + process.send({ type: "result", id: msg.id, result }); } catch (err) { - parentPort.postMessage({ type: "result", id: msg.id, error: err.message }); + process.send({ type: "result", id: msg.id, error: err.message }); } } }); @@ -68,45 +72,53 @@ export async function loadPlugin( ): Promise { const permissions = manifest.requires.permissions; const hostId = randomUUID(); - const hostScriptPath = join(tmpdir(), `omniroute-plugin-host-${hostId}.js`); + // .mjs extension forces ESM execution + const hostScriptPath = join(tmpdir(), `omniroute-plugin-host-${hostId}.mjs`); - // Write host script to temp file await writeFile(hostScriptPath, PLUGIN_HOST_SCRIPT, "utf-8"); - // Build restricted environment for child process const env: Record = { ...getFilteredEnv(permissions), PLUGIN_ENTRY: entryPoint, PLUGIN_NAME: manifest.name, }; - // Fork child process with restricted args const child = fork(hostScriptPath, [entryPoint], { env, stdio: ["pipe", "pipe", "pipe", "ipc"], execArgv: ["--no-warnings"], }); - // Track pending calls - const pendingCalls: Map = new Map(); + // Track pending calls with timeout support + const pendingCalls: Map< + string, + { + resolve: (value: unknown) => void; + reject: (reason: Error) => void; + timer: ReturnType; + } + > = new Map(); let callCounter = 0; - // Handle IPC messages - child.on("message", (msg: any) => { - if (msg.type === "ready") { - log.info("loader.process_ready", { name: manifest.name, hooks: msg.hooks }); - } else if (msg.type === "result") { - const pending = pendingCalls.get(msg.id); - if (pending) { - pendingCalls.delete(msg.id); - if (msg.error) { - pending.reject(new Error(msg.error)); - } else { - pending.resolve(msg.result); + child.on( + "message", + (msg: { type: string; id?: string; hooks?: string[]; result?: unknown; error?: string }) => { + if (msg.type === "ready") { + log.info("loader.process_ready", { name: manifest.name, hooks: msg.hooks }); + } else if (msg.type === "result" && msg.id) { + const pending = pendingCalls.get(msg.id); + if (pending) { + clearTimeout(pending.timer); + pendingCalls.delete(msg.id); + if (msg.error) { + pending.reject(new Error(msg.error)); + } else { + pending.resolve(msg.result); + } } } } - }); + ); child.on("error", (err) => { log.error("loader.process_error", { name: manifest.name, error: err.message }); @@ -114,65 +126,96 @@ export async function loadPlugin( child.on("exit", (code) => { log.info("loader.process_exit", { name: manifest.name, code }); - // Reject all pending calls for (const [, pending] of pendingCalls) { + clearTimeout(pending.timer); pending.reject(new Error(`Plugin process exited with code ${code}`)); } pendingCalls.clear(); - // Cleanup temp file rm(hostScriptPath, { force: true }).catch(() => {}); }); - // Helper to call a hook in the child process - const callHook = (hook: string, payload: unknown): Promise => { + // Call a hook in the child process with timeout + SIGTERM + SIGKILL escalation + const callHook = ( + hook: string, + payload: unknown, + timeout = DEFAULT_HOOK_TIMEOUT + ): Promise => { return new Promise((resolve, reject) => { const id = String(++callCounter); - pendingCalls.set(id, { resolve, reject }); + const timer = setTimeout(() => { + pendingCalls.delete(id); + child.kill("SIGTERM"); + // Escalate to SIGKILL if plugin ignores SIGTERM + const killTimer = setTimeout(() => { + try { + child.kill("SIGKILL"); + } catch {} + }, SIGKILL_GRACE_MS); + child.once("exit", () => clearTimeout(killTimer)); + reject(new Error(`Plugin hook '${hook}' timed out after ${timeout}ms`)); + }, timeout); + + pendingCalls.set(id, { resolve, reject, timer }); child.send({ type: "call", id, hook, payload }); }); }; // Build Plugin interface - const hooks: string[] = []; const plugin: Plugin = { name: manifest.name, priority: 100, enabled: true, }; - // Create hook wrappers plugin.onRequest = async (ctx: PluginContext): Promise => { try { const result = await callHook("onRequest", ctx); return result as PluginResult | void; - } catch (err: any) { - log.error("plugin.onRequest_error", { name: manifest.name, error: err.message }); + } catch (err: unknown) { + log.error("plugin.onRequest_error", { + name: manifest.name, + error: err instanceof Error ? err.message : String(err), + }); } }; - hooks.push("onRequest"); plugin.onResponse = async (ctx: PluginContext, response: unknown): Promise => { try { return await callHook("onResponse", { ctx, response }); - } catch (err: any) { - log.error("plugin.onResponse_error", { name: manifest.name, error: err.message }); + } catch (err: unknown) { + log.error("plugin.onResponse_error", { + name: manifest.name, + error: err instanceof Error ? err.message : String(err), + }); } }; - hooks.push("onResponse"); plugin.onError = async (ctx: PluginContext, error: Error): Promise => { try { return await callHook("onError", { ctx, error: error.message }); - } catch (err: any) { - log.error("plugin.onError_error", { name: manifest.name, error: err.message }); + } catch (err: unknown) { + log.error("plugin.onError_error", { + name: manifest.name, + error: err instanceof Error ? err.message : String(err), + }); } }; - hooks.push("onError"); - log.info("loader.loaded", { name: manifest.name, hooks, pid: child.pid }); + log.info("loader.loaded", { + name: manifest.name, + hooks: ["onRequest", "onResponse", "onError"], + pid: child.pid, + }); const cleanup = () => { - child.kill(); + child.kill("SIGTERM"); + // Escalate to SIGKILL after grace period + const killTimer = setTimeout(() => { + try { + child.kill("SIGKILL"); + } catch {} + }, SIGKILL_GRACE_MS); + child.once("exit", () => clearTimeout(killTimer)); rm(hostScriptPath, { force: true }).catch(() => {}); log.info("loader.cleanup", { name: manifest.name }); }; @@ -182,25 +225,16 @@ export async function loadPlugin( /** * Filter environment variables based on permissions. - * Only pass safe env vars unless "env" permission is granted. + * Uses allowlist approach — only pass explicitly safe vars. */ function getFilteredEnv(permissions: Permission[]): Record { const safeKeys = ["PATH", "HOME", "USER", "LANG", "LC_ALL", "NODE_ENV"]; + const extendedSafeKeys = [...safeKeys, "PORT", "HOSTNAME", "TZ", "TMPDIR"]; + const allowedKeys = permissions.includes("env") ? extendedSafeKeys : safeKeys; const env: Record = {}; - for (const key of safeKeys) { - if (process.env[key]) { - env[key] = process.env[key]!; - } - } - - if (permissions.includes("env")) { - // Pass all env vars - for (const [key, value] of Object.entries(process.env)) { - if (value !== undefined && !safeKeys.includes(key)) { - env[key] = value; - } - } + for (const key of allowedKeys) { + if (process.env[key] !== undefined) env[key] = process.env[key]!; } return env; diff --git a/src/lib/plugins/manager.ts b/src/lib/plugins/manager.ts index 94d74a7487..e46b60a124 100644 --- a/src/lib/plugins/manager.ts +++ b/src/lib/plugins/manager.ts @@ -7,13 +7,13 @@ * @module plugins/manager */ -import { mkdir, cp, rm } from "fs/promises"; +import { mkdir, cp, rm, realpath } from "fs/promises"; import { join, dirname } from "path"; import { randomUUID } from "crypto"; import { logger } from "../../../open-sse/utils/logger.ts"; import { getDefaultPluginDir, scanPluginDir } from "./scanner"; import { loadPlugin, type LoadedPlugin } from "./loader"; -import { registerPlugin, unregisterPlugin } from "./index"; +import { registerHook, unregisterHooks } from "./hooks"; import { insertPlugin, getPluginByName, @@ -140,17 +140,33 @@ class PluginManager { const manifest = JSON.parse(row.manifest) as PluginManifestWithDefaults; - // Path traversal guard: entry point must stay within plugin directory + // Path traversal guard: use realpath to resolve symlinks const entryPoint = join(row.pluginDir, manifest.main); - if (!entryPoint.startsWith(row.pluginDir)) { + let resolvedPluginDir: string; + try { + resolvedPluginDir = await realpath(row.pluginDir); + } catch { + throw new Error(`Plugin directory '${row.pluginDir}' does not exist`); + } + const resolvedEntry = await realpath(entryPoint).catch(() => null); + if ( + !resolvedEntry || + (!resolvedEntry.startsWith(resolvedPluginDir + "/") && resolvedEntry !== resolvedPluginDir) + ) { throw new Error(`Plugin '${name}' entry point escapes plugin directory`); } try { const loaded = await loadPlugin(entryPoint, manifest); - // Register hooks with the existing plugin system - registerPlugin(loaded.plugin); + // Register hooks individually via registerHook + const hookNames = ["onRequest", "onResponse", "onError"] as const; + for (const hookName of hookNames) { + const handler = loaded.plugin[hookName]; + if (typeof handler === "function") { + registerHook(hookName, name, handler as (payload: unknown) => void | Promise); + } + } this.loadedPlugins.set(name, loaded); updatePluginStatus(name, "active"); @@ -169,7 +185,7 @@ class PluginManager { async deactivate(name: string): Promise { const loaded = this.loadedPlugins.get(name); if (loaded) { - unregisterPlugin(name); + unregisterHooks(name); loaded.cleanup(); this.loadedPlugins.delete(name); }