fix(server): harden internal plugin startup and boundary checks

2026-11-06 08:24:46 +00:00 · 2026-09-26 20:52:37 +02:00
parent c03853f4e1
commit 3c65670d76
7 changed files with 127 additions and 91 deletions
@@ -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",
@@ -67,6 +67,7 @@ function resolvePluginStatus(input: {
 export class PluginService {
  private readonly runtime: PluginRuntimePort;
  private readonly managedSources: ManagedPluginSources | null;
+  private readonly internalPluginIds: ReadonlySet<string>;
  private readonly logger: pino.Logger;
  private readonly errors = new Map<string, string>();
  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<PluginRuntime["catalog"]> {
-    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<PluginListItem> {
@@ -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`,
        );
@@ -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<string> = 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<string>();
-    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<void> {
+  const server = path.join(directory, "index.server.ts");
+  const reader = createPluginImportReader(directory);
+  const pending = [server];
+  const visited = new Set<string>();
+  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 });
+  }
 });
@@ -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<InternalPluginChild>();
-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<keyof typeof originalConsole>) {
-    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<keyof typeof originalConsole>) {
-    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);
@@ -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);
@@ -341,7 +341,16 @@ export class PluginRuntime {
  }): Promise<void> {
    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<LoadedPlugin> {
    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<LoadedPlugin> {
+    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 ?? [],
@@ -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 () => {};
 }