fix: enforce plugin config schemas (#1272) (thanks @thewilloftheshadow)

Co-authored-by: thewilloftheshadow <thewilloftheshadow@users.noreply.github.com>
2026-01-19 21:13:51 -06:00
parent 48f733e4b3
commit 2f6d5805de
49 changed files with 1817 additions and 377 deletions
--- a/src/plugins/loader.ts
+++ b/src/plugins/loader.ts
@@ -8,16 +8,21 @@ import type { GatewayRequestHandler } from "../gateway/server-methods/types.js";
 import { createSubsystemLogger } from "../logging/subsystem.js";
 import { resolveUserPath } from "../utils.js";
 import { discoverClawdbotPlugins } from "./discovery.js";
+import { loadPluginManifestRegistry } from "./manifest-registry.js";
+import {
+  normalizePluginsConfig,
+  resolveEnableState,
+  resolveMemorySlotDecision,
+  type NormalizedPluginsConfig,
+} from "./config-state.js";
 import { initializeGlobalHookRunner } from "./hook-runner-global.js";
 import { createPluginRegistry, type PluginRecord, type PluginRegistry } from "./registry.js";
 import { createPluginRuntime } from "./runtime/index.js";
 import { setActivePluginRegistry } from "./runtime.js";
-import { defaultSlotIdForKey } from "./slots.js";
+import { validateJsonSchemaValue } from "./schema-validator.js";
 import type {
-  ClawdbotPluginConfigSchema,
  ClawdbotPluginDefinition,
  ClawdbotPluginModule,
-  PluginConfigUiHint,
  PluginDiagnostic,
  PluginLogger,
 } from "./types.js";
@@ -33,73 +38,10 @@ export type PluginLoadOptions = {
  mode?: "full" | "validate";
 };

-type NormalizedPluginsConfig = {
-  enabled: boolean;
-  allow: string[];
-  deny: string[];
-  loadPaths: string[];
-  slots: {
-    memory?: string | null;
-  };
-  entries: Record<string, { enabled?: boolean; config?: Record<string, unknown> }>;
-};
-
 const registryCache = new Map<string, PluginRegistry>();

 const defaultLogger = () => createSubsystemLogger("plugins");

-const BUNDLED_ENABLED_BY_DEFAULT = new Set<string>();
-
-const normalizeList = (value: unknown): string[] => {
-  if (!Array.isArray(value)) return [];
-  return value.map((entry) => (typeof entry === "string" ? entry.trim() : "")).filter(Boolean);
-};
-
-const normalizeSlotValue = (value: unknown): string | null | undefined => {
-  if (typeof value !== "string") return undefined;
-  const trimmed = value.trim();
-  if (!trimmed) return undefined;
-  if (trimmed.toLowerCase() === "none") return null;
-  return trimmed;
-};
-
-const normalizePluginEntries = (entries: unknown): NormalizedPluginsConfig["entries"] => {
-  if (!entries || typeof entries !== "object" || Array.isArray(entries)) {
-    return {};
-  }
-  const normalized: NormalizedPluginsConfig["entries"] = {};
-  for (const [key, value] of Object.entries(entries)) {
-    if (!key.trim()) continue;
-    if (!value || typeof value !== "object" || Array.isArray(value)) {
-      normalized[key] = {};
-      continue;
-    }
-    const entry = value as Record<string, unknown>;
-    normalized[key] = {
-      enabled: typeof entry.enabled === "boolean" ? entry.enabled : undefined,
-      config:
-        entry.config && typeof entry.config === "object" && !Array.isArray(entry.config)
-          ? (entry.config as Record<string, unknown>)
-          : undefined,
-    };
-  }
-  return normalized;
-};
-
-const normalizePluginsConfig = (config?: ClawdbotConfig["plugins"]): NormalizedPluginsConfig => {
-  const memorySlot = normalizeSlotValue(config?.slots?.memory);
-  return {
-    enabled: config?.enabled !== false,
-    allow: normalizeList(config?.allow),
-    deny: normalizeList(config?.deny),
-    loadPaths: normalizeList(config?.load?.paths),
-    slots: {
-      memory: memorySlot ?? defaultSlotIdForKey("memory"),
-    },
-    entries: normalizePluginEntries(config?.entries),
-  };
-};
-
 const resolvePluginSdkAlias = (): string | null => {
  try {
    const modulePath = fileURLToPath(import.meta.url);
@@ -133,105 +75,25 @@ function buildCacheKey(params: {
  return `${workspaceKey}::${JSON.stringify(params.plugins)}`;
 }

-function resolveMemorySlotDecision(params: {
-  id: string;
-  kind?: string;
-  slot: string | null | undefined;
-  selectedId: string | null;
-}): { enabled: boolean; reason?: string; selected?: boolean } {
-  if (params.kind !== "memory") return { enabled: true };
-  if (params.slot === null) {
-    return { enabled: false, reason: "memory slot disabled" };
-  }
-  if (typeof params.slot === "string") {
-    if (params.slot === params.id) {
-      return { enabled: true, selected: true };
-    }
-    return {
-      enabled: false,
-      reason: `memory slot set to "${params.slot}"`,
-    };
-  }
-  if (params.selectedId && params.selectedId !== params.id) {
-    return {
-      enabled: false,
-      reason: `memory slot already filled by "${params.selectedId}"`,
-    };
-  }
-  return { enabled: true, selected: true };
-}
-
-function resolveEnableState(
-  id: string,
-  origin: PluginRecord["origin"],
-  config: NormalizedPluginsConfig,
-): { enabled: boolean; reason?: string } {
-  if (!config.enabled) {
-    return { enabled: false, reason: "plugins disabled" };
-  }
-  if (config.deny.includes(id)) {
-    return { enabled: false, reason: "blocked by denylist" };
-  }
-  if (config.allow.length > 0 && !config.allow.includes(id)) {
-    return { enabled: false, reason: "not in allowlist" };
-  }
-  if (config.slots.memory === id) {
-    return { enabled: true };
-  }
-  const entry = config.entries[id];
-  if (entry?.enabled === true) {
-    return { enabled: true };
-  }
-  if (entry?.enabled === false) {
-    return { enabled: false, reason: "disabled in config" };
-  }
-  if (origin === "bundled" && BUNDLED_ENABLED_BY_DEFAULT.has(id)) {
-    return { enabled: true };
-  }
-  if (origin === "bundled") {
-    return { enabled: false, reason: "bundled (disabled by default)" };
-  }
-  return { enabled: true };
-}
-
 function validatePluginConfig(params: {
-  schema?: ClawdbotPluginConfigSchema;
-  value?: Record<string, unknown>;
+  schema?: Record<string, unknown>;
+  cacheKey?: string;
+  value?: unknown;
 }): { ok: boolean; value?: Record<string, unknown>; errors?: string[] } {
  const schema = params.schema;
-  if (!schema) return { ok: true, value: params.value };
-
-  if (typeof schema.validate === "function") {
-    const result = schema.validate(params.value);
-    if (result.ok) {
-      return { ok: true, value: result.value as Record<string, unknown> };
-    }
-    return { ok: false, errors: result.errors };
+  if (!schema) {
+    return { ok: true, value: params.value as Record<string, unknown> | undefined };
  }
-
-  if (typeof schema.safeParse === "function") {
-    const result = schema.safeParse(params.value);
-    if (result.success) {
-      return { ok: true, value: result.data as Record<string, unknown> };
-    }
-    const issues = result.error?.issues ?? [];
-    const errors = issues.map((issue) => {
-      const path = issue.path.length > 0 ? issue.path.join(".") : "<root>";
-      return `${path}: ${issue.message}`;
-    });
-    return { ok: false, errors };
+  const cacheKey = params.cacheKey ?? JSON.stringify(schema);
+  const result = validateJsonSchemaValue({
+    schema,
+    cacheKey,
+    value: params.value ?? {},
+  });
+  if (result.ok) {
+    return { ok: true, value: params.value as Record<string, unknown> | undefined };
  }
-
-  if (typeof schema.parse === "function") {
-    try {
-      const parsed = schema.parse(params.value);
-      return { ok: true, value: parsed as Record<string, unknown> };
-    } catch (err) {
-      return { ok: false, errors: [String(err)] };
-    }
-  }
-
-  return { ok: true, value: params.value };
+  return { ok: false, errors: result.errors };
 }

 function resolvePluginModuleExport(moduleExport: unknown): {
@@ -326,7 +188,14 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
    workspaceDir: options.workspaceDir,
    extraPaths: normalized.loadPaths,
  });
-  pushDiagnostics(registry.diagnostics, discovery.diagnostics);
+  const manifestRegistry = loadPluginManifestRegistry({
+    config: cfg,
+    workspaceDir: options.workspaceDir,
+    cache: options.cache,
+    candidates: discovery.candidates,
+    diagnostics: discovery.diagnostics,
+  });
+  pushDiagnostics(registry.diagnostics, manifestRegistry.diagnostics);

  const pluginSdkAlias = resolvePluginSdkAlias();
  const jiti = createJiti(import.meta.url, {
@@ -335,10 +204,8 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
    ...(pluginSdkAlias ? { alias: { "clawdbot/plugin-sdk": pluginSdkAlias } } : {}),
  });

-  const bundledIds = new Set(
-    discovery.candidates
-      .filter((candidate) => candidate.origin === "bundled")
-      .map((candidate) => candidate.idHint),
+  const manifestByRoot = new Map(
+    manifestRegistry.plugins.map((record) => [record.rootDir, record]),
  );

  const seenIds = new Map<string, PluginRecord["origin"]>();
@@ -347,18 +214,23 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
  let memorySlotMatched = false;

  for (const candidate of discovery.candidates) {
-    const existingOrigin = seenIds.get(candidate.idHint);
+    const manifestRecord = manifestByRoot.get(candidate.rootDir);
+    if (!manifestRecord) {
+      continue;
+    }
+    const pluginId = manifestRecord.id;
+    const existingOrigin = seenIds.get(pluginId);
    if (existingOrigin) {
      const record = createPluginRecord({
-        id: candidate.idHint,
-        name: candidate.packageName ?? candidate.idHint,
-        description: candidate.packageDescription,
-        version: candidate.packageVersion,
+        id: pluginId,
+        name: manifestRecord.name ?? pluginId,
+        description: manifestRecord.description,
+        version: manifestRecord.version,
        source: candidate.source,
        origin: candidate.origin,
        workspaceDir: candidate.workspaceDir,
        enabled: false,
-        configSchema: false,
+        configSchema: Boolean(manifestRecord.configSchema),
      });
      record.status = "disabled";
      record.error = `overridden by ${existingOrigin} plugin`;
@@ -366,25 +238,42 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
      continue;
    }

-    const enableState = resolveEnableState(candidate.idHint, candidate.origin, normalized);
-    const entry = normalized.entries[candidate.idHint];
+    const enableState = resolveEnableState(pluginId, candidate.origin, normalized);
+    const entry = normalized.entries[pluginId];
    const record = createPluginRecord({
-      id: candidate.idHint,
-      name: candidate.packageName ?? candidate.idHint,
-      description: candidate.packageDescription,
-      version: candidate.packageVersion,
+      id: pluginId,
+      name: manifestRecord.name ?? pluginId,
+      description: manifestRecord.description,
+      version: manifestRecord.version,
      source: candidate.source,
      origin: candidate.origin,
      workspaceDir: candidate.workspaceDir,
      enabled: enableState.enabled,
-      configSchema: false,
+      configSchema: Boolean(manifestRecord.configSchema),
    });
+    record.kind = manifestRecord.kind;
+    record.configUiHints = manifestRecord.configUiHints;
+    record.configJsonSchema = manifestRecord.configSchema;

    if (!enableState.enabled) {
      record.status = "disabled";
      record.error = enableState.reason;
      registry.plugins.push(record);
-      seenIds.set(candidate.idHint, candidate.origin);
+      seenIds.set(pluginId, candidate.origin);
+      continue;
+    }
+
+    if (!manifestRecord.configSchema) {
+      record.status = "error";
+      record.error = "missing config schema";
+      registry.plugins.push(record);
+      seenIds.set(pluginId, candidate.origin);
+      registry.diagnostics.push({
+        level: "error",
+        pluginId: record.id,
+        source: record.source,
+        message: record.error,
+      });
      continue;
    }

@@ -396,7 +285,7 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
      record.status = "error";
      record.error = String(err);
      registry.plugins.push(record);
-      seenIds.set(candidate.idHint, candidate.origin);
+      seenIds.set(pluginId, candidate.origin);
      registry.diagnostics.push({
        level: "error",
        pluginId: record.id,
@@ -422,61 +311,17 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
    record.name = definition?.name ?? record.name;
    record.description = definition?.description ?? record.description;
    record.version = definition?.version ?? record.version;
-    record.kind = definition?.kind;
-    record.configSchema = Boolean(definition?.configSchema);
-    record.configUiHints =
-      definition?.configSchema &&
-      typeof definition.configSchema === "object" &&
-      (definition.configSchema as { uiHints?: unknown }).uiHints &&
-      typeof (definition.configSchema as { uiHints?: unknown }).uiHints === "object" &&
-      !Array.isArray((definition.configSchema as { uiHints?: unknown }).uiHints)
-        ? ((definition.configSchema as { uiHints?: unknown }).uiHints as Record<
-            string,
-            PluginConfigUiHint
-          >)
-        : undefined;
-    record.configJsonSchema =
-      definition?.configSchema &&
-      typeof definition.configSchema === "object" &&
-      (definition.configSchema as { jsonSchema?: unknown }).jsonSchema &&
-      typeof (definition.configSchema as { jsonSchema?: unknown }).jsonSchema === "object" &&
-      !Array.isArray((definition.configSchema as { jsonSchema?: unknown }).jsonSchema)
-        ? ((definition.configSchema as { jsonSchema?: unknown }).jsonSchema as Record<
-            string,
-            unknown
-          >)
-        : undefined;
-
-    if (!definition?.configSchema) {
-      const hasBundledFallback =
-        candidate.origin !== "bundled" && bundledIds.has(candidate.idHint);
-      if (hasBundledFallback) {
-        record.enabled = false;
-        record.status = "disabled";
-        record.error = "missing config schema (using bundled plugin)";
-        registry.plugins.push(record);
-        registry.diagnostics.push({
-          level: "warn",
-          pluginId: record.id,
-          source: record.source,
-          message: record.error,
-        });
-        continue;
-      }
-
-      logger.error(`[plugins] ${record.id} missing config schema`);
-      record.status = "error";
-      record.error = "missing config schema";
-      registry.plugins.push(record);
-      seenIds.set(candidate.idHint, candidate.origin);
+    const manifestKind = record.kind as string | undefined;
+    const exportKind = definition?.kind as string | undefined;
+    if (manifestKind && exportKind && exportKind !== manifestKind) {
      registry.diagnostics.push({
-        level: "error",
+        level: "warn",
        pluginId: record.id,
        source: record.source,
-        message: record.error,
+        message: `plugin kind mismatch (manifest uses "${manifestKind}", export uses "${exportKind}")`,
      });
-      continue;
    }
+    record.kind = definition?.kind ?? record.kind;

    if (record.kind === "memory" && memorySlot === record.id) {
      memorySlotMatched = true;
@@ -494,7 +339,7 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
      record.status = "disabled";
      record.error = memoryDecision.reason;
      registry.plugins.push(record);
-      seenIds.set(candidate.idHint, candidate.origin);
+      seenIds.set(pluginId, candidate.origin);
      continue;
    }

@@ -503,7 +348,8 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
    }

    const validatedConfig = validatePluginConfig({
-      schema: definition?.configSchema,
+      schema: manifestRecord.configSchema,
+      cacheKey: manifestRecord.schemaCacheKey,
      value: entry?.config,
    });

@@ -512,7 +358,7 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
      record.status = "error";
      record.error = `invalid config: ${validatedConfig.errors?.join(", ")}`;
      registry.plugins.push(record);
-      seenIds.set(candidate.idHint, candidate.origin);
+      seenIds.set(pluginId, candidate.origin);
      registry.diagnostics.push({
        level: "error",
        pluginId: record.id,
@@ -524,7 +370,7 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi

    if (validateOnly) {
      registry.plugins.push(record);
-      seenIds.set(candidate.idHint, candidate.origin);
+      seenIds.set(pluginId, candidate.origin);
      continue;
    }

@@ -533,7 +379,7 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
      record.status = "error";
      record.error = "plugin export missing register/activate";
      registry.plugins.push(record);
-      seenIds.set(candidate.idHint, candidate.origin);
+      seenIds.set(pluginId, candidate.origin);
      registry.diagnostics.push({
        level: "error",
        pluginId: record.id,
@@ -559,7 +405,7 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
        });
      }
      registry.plugins.push(record);
-      seenIds.set(candidate.idHint, candidate.origin);
+      seenIds.set(pluginId, candidate.origin);
    } catch (err) {
      logger.error(
        `[plugins] ${record.id} failed during register from ${record.source}: ${String(err)}`,
@@ -567,7 +413,7 @@ export function loadClawdbotPlugins(options: PluginLoadOptions = {}): PluginRegi
      record.status = "error";
      record.error = String(err);
      registry.plugins.push(record);
-      seenIds.set(candidate.idHint, candidate.origin);
+      seenIds.set(pluginId, candidate.origin);
      registry.diagnostics.push({
        level: "error",
        pluginId: record.id,