From 3c65670d763ae176ba0d1647d975b708d6fbe423 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sat, 26 Sep 2026 18:12:39 +0200 Subject: [PATCH] fix(server): harden internal plugin startup and boundary checks --- .../server/src/server/plugins/compiler.ts | 3 +- packages/server/src/server/plugins/index.ts | 23 +++--- .../server/plugins/internal-boundary.test.ts | 75 ++++++++++++------- .../src/server/plugins/internal-child.ts | 37 +-------- .../server/plugins/internal-seam.e2e.test.ts | 33 ++++++-- packages/server/src/server/plugins/runtime.ts | 46 +++++++++--- .../internal-seam/index.server.ts | 1 - 7 files changed, 127 insertions(+), 91 deletions(-) diff --git a/packages/server/src/server/plugins/compiler.ts b/packages/server/src/server/plugins/compiler.ts index ca5ae9f30c..80dbb1b7b0 100644 --- a/packages/server/src/server/plugins/compiler.ts +++ b/packages/server/src/server/plugins/compiler.ts @@ -10,6 +10,7 @@ import { } from "./plugin-sdk-specifiers.js"; const nodeRequire = createRequire(import.meta.url); +export const SERVER_HOST_MODULES = [...PLUGIN_SDK_SPECIFIERS, "zod"]; const ESBUILD_BINARY_PATH = "ESBUILD_BINARY_PATH"; // esbuild resolves its own platform binary via require.resolve() the first time its @@ -403,7 +404,7 @@ async function compileTarget(entryPath: string, target: PluginBuildTarget): Prom "react-native", "zod", ] - : [...PLUGIN_SDK_SPECIFIERS, "zod"], + : SERVER_HOST_MODULES, plugins: [createRuntimeBoundaryPlugin(target, pluginDirectory)], metafile: true, logLevel: "silent", diff --git a/packages/server/src/server/plugins/index.ts b/packages/server/src/server/plugins/index.ts index 1ce3a0c2fd..287b33224c 100644 --- a/packages/server/src/server/plugins/index.ts +++ b/packages/server/src/server/plugins/index.ts @@ -67,6 +67,7 @@ function resolvePluginStatus(input: { export class PluginService { private readonly runtime: PluginRuntimePort; private readonly managedSources: ManagedPluginSources | null; + private readonly internalPluginIds: ReadonlySet; private readonly logger: pino.Logger; private readonly errors = new Map(); private readonly listeners = new Set<(pluginId: string) => void>(); @@ -94,6 +95,9 @@ export class PluginService { }, }); this.managedSources = dependencies.managedSources ?? null; + this.internalPluginIds = new Set( + dependencies.internalPlugins?.map((plugin) => plugin.id) ?? [], + ); this.runtime.subscribe((pluginId, error) => { this.removeProviderRegistrations(pluginId); if (error) this.errors.set(pluginId, error); @@ -141,8 +145,12 @@ export class PluginService { const config = this.configStore.get(); this.globalStartsBlocked = config.pluginsEnabled !== true; for (const plugin of this.dependencies.internalPlugins ?? []) { - await this.runtime.startInternalPlugin?.(plugin); - await this.publishProviderRegistrations(plugin.id, plugin.directory); + try { + await this.runtime.startInternalPlugin?.(plugin); + await this.publishProviderRegistrations(plugin.id, plugin.directory); + } catch (error) { + this.logger.error({ err: error, pluginId: plugin.id }, "Failed to start internal plugin"); + } } if (config.pluginsEnabled === true) { for (const [pluginId, source] of Object.entries(config.plugins ?? {})) { @@ -193,16 +201,14 @@ export class PluginService { } getLogs(pluginId: string): PluginLogEntry[] { - if (!this.dependencies.internalPlugins?.some((plugin) => plugin.id === pluginId)) { + if (!this.internalPluginIds.has(pluginId)) { this.requireSource(pluginId); } return this.runtime.getLogs(pluginId); } catalog(): ReturnType { - return this.runtime - .catalog() - .filter(({ id }) => !this.dependencies.internalPlugins?.some((plugin) => plugin.id === id)); + return this.runtime.catalog().filter(({ id }) => !this.internalPluginIds.has(id)); } async installDirectory(input: { path: string; id?: string }): Promise { @@ -211,10 +217,7 @@ export class PluginService { const manifest = await readPluginManifest(directory); assertPluginCompatibility({ ...manifest, version: this.daemonVersion, runtime: "daemon" }); const pluginId = PluginIdSchema.parse(input.id ?? manifest.id); - if ( - this.configStore.get().plugins?.[pluginId] || - this.dependencies.internalPlugins?.some((plugin) => plugin.id === pluginId) - ) { + if (this.configStore.get().plugins?.[pluginId] || this.internalPluginIds.has(pluginId)) { throw new Error( `Plugin ID "${pluginId}" is already configured; choose another ID with --id`, ); diff --git a/packages/server/src/server/plugins/internal-boundary.test.ts b/packages/server/src/server/plugins/internal-boundary.test.ts index 73b2a96818..cfd0eaea48 100644 --- a/packages/server/src/server/plugins/internal-boundary.test.ts +++ b/packages/server/src/server/plugins/internal-boundary.test.ts @@ -1,37 +1,62 @@ -import { readdir } from "node:fs/promises"; +import { mkdtemp, readdir, rm, writeFile } from "node:fs/promises"; import { isBuiltin } from "node:module"; +import { tmpdir } from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { expect, test } from "vitest"; import { createPluginImportReader } from "./compiler-imports.js"; -import { compilePlugin } from "./compiler.js"; +import { compilePlugin, SERVER_HOST_MODULES } from "./compiler.js"; const root = fileURLToPath(new URL("../../plugins/", import.meta.url)); +const fixture = fileURLToPath(new URL("./test-fixtures/internal-seam/", import.meta.url)); +const hostModules: ReadonlySet = new Set(SERVER_HOST_MODULES); -test("internal plugins keep every import inside the public plugin boundary", async () => { - const entries = await readdir(root, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isDirectory()) continue; - const directory = path.join(root, entry.name); - const server = path.join(directory, "index.server.ts"); - await compilePlugin({ server, client: null }); - const reader = createPluginImportReader(directory); - const pending = [server]; - const visited = new Set(); - while (pending.length > 0) { - const file = pending.pop()!; - if (visited.has(file)) continue; - visited.add(file); - for (const dependency of reader.read(file)) { - const specifier = dependency.specifier; - if (specifier === "@getpaseo/plugin" || specifier.startsWith("@getpaseo/plugin/")) continue; - if (isBuiltin(specifier)) continue; - const resolved = reader.resolve(specifier, file, dependency.kind); - expect(resolved, `${file}: ${specifier}`).toBeDefined(); - const relative = path.relative(directory, resolved!); - expect(relative, `${file}: ${specifier}`).toMatch(/^(server|shared)\//); - pending.push(resolved!); +async function checkInternalPluginBoundary(directory: string): Promise { + const server = path.join(directory, "index.server.ts"); + const reader = createPluginImportReader(directory); + const pending = [server]; + const visited = new Set(); + while (pending.length > 0) { + const file = pending.pop()!; + if (visited.has(file)) continue; + visited.add(file); + for (const dependency of reader.read(file)) { + const specifier = dependency.specifier; + if (hostModules.has(specifier) || isBuiltin(specifier)) continue; + const resolved = reader.resolve(specifier, file, dependency.kind); + const relative = resolved && path.relative(directory, resolved); + const owned = relative && /^(server|shared)\//.test(relative); + if (!specifier.startsWith(".") || !owned) { + throw new Error(`Internal plugin import boundary: ${file} imports ${specifier}`); } + pending.push(resolved); } } + await compilePlugin({ server, client: null }); +} + +test("internal plugin fixture and production directories respect the compiler host-module boundary", async () => { + await checkInternalPluginBoundary(fixture); + const entries = await readdir(root, { withFileTypes: true }); + for (const entry of entries) { + if (entry.isDirectory()) await checkInternalPluginBoundary(path.join(root, entry.name)); + } +}); + +test("internal plugin boundary rejects daemon imports with the file and specifier", async () => { + const directory = await mkdtemp(path.join(tmpdir(), "paseo-internal-boundary-")); + const server = path.join(directory, "index.server.ts"); + const daemonFile = fileURLToPath(new URL("../bootstrap.ts", import.meta.url)); + const specifier = path.relative(directory, daemonFile); + try { + await writeFile( + server, + `import ${JSON.stringify(specifier)};\nexport default () => () => {};\n`, + ); + await expect(checkInternalPluginBoundary(directory)).rejects.toThrow( + `Internal plugin import boundary: ${server} imports ${specifier}`, + ); + } finally { + await rm(directory, { recursive: true, force: true }); + } }); diff --git a/packages/server/src/server/plugins/internal-child.ts b/packages/server/src/server/plugins/internal-child.ts index 15ad4c5362..a80a8c5ec5 100644 --- a/packages/server/src/server/plugins/internal-child.ts +++ b/packages/server/src/server/plugins/internal-child.ts @@ -1,40 +1,9 @@ import { EventEmitter } from "node:events"; -import { AsyncLocalStorage } from "node:async_hooks"; -import { format } from "node:util"; import { PassThrough } from "node:stream"; import type { PluginServerContribution } from "@getpaseo/plugin/server"; import { createPluginWorker, type PluginWorkerChannel } from "./plugin-process.js"; import type { PluginProcessMessage, PluginProcessRequest } from "./plugin-process-protocol.js"; -const outputContext = new AsyncLocalStorage(); -const originalConsole = { - log: console.log, - info: console.info, - debug: console.debug, - warn: console.warn, - error: console.error, -}; -let activeChildren = 0; - -function captureOutput(): void { - if (activeChildren++ > 0) return; - for (const method of Object.keys(originalConsole) as Array) { - console[method] = (...args: unknown[]) => { - const child = outputContext.getStore(); - if (!child) return originalConsole[method](...args); - const stream = method === "warn" || method === "error" ? child.stderr : child.stdout; - stream.write(`${format(...args)}\n`); - }; - } -} - -function releaseOutput(): void { - if (--activeChildren > 0) return; - for (const method of Object.keys(originalConsole) as Array) { - console[method] = originalConsole[method]; - } -} - export class InternalPluginChild extends EventEmitter { connected = true; killed = false; @@ -46,7 +15,6 @@ export class InternalPluginChild extends EventEmitter { constructor(contribute: PluginServerContribution) { super(); - captureOutput(); const channel: PluginWorkerChannel = { send: (message: PluginProcessMessage, callback?: () => void) => { const copy = structuredClone(message); @@ -71,9 +39,7 @@ export class InternalPluginChild extends EventEmitter { } const copy = structuredClone(message); queueMicrotask(() => { - outputContext.run(this, () => { - for (const handler of this.workerMessages) handler(copy); - }); + for (const handler of this.workerMessages) handler(copy); callback?.(null); }); return true; @@ -92,7 +58,6 @@ export class InternalPluginChild extends EventEmitter { .then(() => this.worker.shutdown()) .finally(() => { this.connected = false; - releaseOutput(); this.stdout.end(); this.stderr.end(); this.emit("close", 0, null); diff --git a/packages/server/src/server/plugins/internal-seam.e2e.test.ts b/packages/server/src/server/plugins/internal-seam.e2e.test.ts index fe36cd5e86..717dd1e5b9 100644 --- a/packages/server/src/server/plugins/internal-seam.e2e.test.ts +++ b/packages/server/src/server/plugins/internal-seam.e2e.test.ts @@ -38,13 +38,6 @@ test("internal and directory plugins share RPC and lifecycle behavior while inte await expect .poll(() => client.invokePluginRpc("directory-seam", "state", {})) .toEqual({ workspaces: 1 }); - await expect - .poll(async () => - (await client.getPluginLogs("internal-seam")).some( - ({ message }) => message === "workspace created by seam fixture", - ), - ) - .toBe(true); await client.patchDaemonConfig({ pluginsEnabled: false }); expect(await client.invokePluginRpc("internal-seam", "state", {})).toEqual({ workspaces: 1 }); expect(await client.listPlugins()).toMatchObject([ @@ -56,3 +49,29 @@ test("internal and directory plugins share RPC and lifecycle behavior while inte await rm(workspaceDirectory, { recursive: true, force: true }); } }, 60_000); + +test("a failing internal plugin does not stop the daemon or the next internal plugin", async () => { + const daemon = await createTestPaseoDaemon({ + daemonVersion: "0.8.0", + pluginsEnabled: false, + internalPlugins: [ + { + id: "failing-internal", + directory, + contribute: () => { + throw new Error("fixture startup failure"); + }, + }, + { id: "internal-seam", directory, contribute }, + ], + }); + const client = new DaemonClient({ url: `ws://127.0.0.1:${daemon.port}/ws`, appVersion: "0.8.0" }); + try { + await client.connect(); + expect(await client.invokePluginRpc("internal-seam", "state", {})).toEqual({ workspaces: 0 }); + expect(await client.listPlugins()).toEqual([]); + } finally { + await client.close(); + await daemon.close(); + } +}, 60_000); diff --git a/packages/server/src/server/plugins/runtime.ts b/packages/server/src/server/plugins/runtime.ts index f5a917ebcb..af04af06b6 100644 --- a/packages/server/src/server/plugins/runtime.ts +++ b/packages/server/src/server/plugins/runtime.ts @@ -341,7 +341,16 @@ export class PluginRuntime { }): Promise { if (this.plugins.has(input.id)) throw new Error(`Plugin is already running: ${input.id}`); this.appendLog(input.id, "stdout", "[paseo] Loading plugin"); - const loaded = await this.loadDirectoryPlugin(input.id, input.directory, input.contribute); + const directory = path.resolve(input.directory); + const manifest = await readPluginManifest(directory); + assertPluginCompatibility({ ...manifest, version: this.daemonVersion, runtime: "daemon" }); + const loaded = await this.launchPlugin({ + pluginId: input.id, + requirements: manifest.requirements, + child: new InternalPluginChild(input.contribute), + bundle: "", + clientBundle: "", + }); this.plugins.set(input.id, loaded); this.appendLog(input.id, "stdout", "[paseo] Plugin ready"); } @@ -565,18 +574,17 @@ export class PluginRuntime { private async loadDirectoryPlugin( pluginId: string, configuredPath: string, - contribute?: PluginServerContribution, ): Promise { const directory = path.resolve(configuredPath); const manifest = await readPluginManifest(directory); assertPluginCompatibility({ ...manifest, version: this.daemonVersion, runtime: "daemon" }); - const entryPaths = contribute ? null : await resolveEntryPaths(directory); - const bundles = entryPaths ? await compilePlugin(entryPaths) : null; - const serverBundle = contribute ? "" : bundles?.serverBundle; - if (!contribute && !serverBundle) { + const entryPaths = await resolveEntryPaths(directory); + const bundles = await compilePlugin(entryPaths); + const serverBundle = bundles.serverBundle; + if (!serverBundle) { return { id: pluginId, - clientBundle: bundles?.clientBundle ?? "", + clientBundle: bundles.clientBundle ?? "", requirements: manifest.requirements, methods: new Set(), hooks: { events: [], before: [] }, @@ -590,9 +598,25 @@ export class PluginRuntime { sessionClosed: null, }; } + return this.launchPlugin({ + pluginId, + requirements: manifest.requirements, + child: this.spawnChild(), + bundle: serverBundle, + clientBundle: bundles.clientBundle ?? "", + }); + } + + private async launchPlugin(input: { + pluginId: string; + requirements: PluginRequirements | undefined; + child: PluginChild; + bundle: string; + clientBundle: string; + }): Promise { + const { pluginId, requirements, child, bundle, clientBundle } = input; const sessionHost = this.sessionHost; if (!sessionHost) throw new Error("Plugin Paseo session host is not attached"); - const child = contribute ? new InternalPluginChild(contribute) : this.spawnChild(); const outputCapture = new PluginOutputCapture(child, (stream, message) => { this.appendLog(pluginId, stream, message); }); @@ -669,7 +693,7 @@ export class PluginRuntime { type: "initialize", pluginId, appVersion: this.daemonVersion, - bundle: serverBundle ?? "", + bundle, settingsDirectory: this.dependencies.settingsDirectory ? path.join(this.dependencies.settingsDirectory, pluginId) : undefined, @@ -684,8 +708,8 @@ export class PluginRuntime { } loaded = { id: pluginId, - clientBundle: bundles?.clientBundle ?? "", - requirements: manifest.requirements, + clientBundle, + requirements, methods: new Set(ready.methods), hooks: ready.hooks ?? { events: [], before: [] }, providers: ready.providers ?? [], diff --git a/packages/server/src/server/plugins/test-fixtures/internal-seam/index.server.ts b/packages/server/src/server/plugins/test-fixtures/internal-seam/index.server.ts index 291cbd705a..8f0914a4c8 100644 --- a/packages/server/src/server/plugins/test-fixtures/internal-seam/index.server.ts +++ b/packages/server/src/server/plugins/test-fixtures/internal-seam/index.server.ts @@ -13,7 +13,6 @@ export default function contribute(server: PluginServerContext) { server.handle(state, () => ({ workspaces })); server.on("workspace.created", () => { workspaces += 1; - console.log("workspace created by seam fixture"); }); return () => {}; }