fix(plugins): surface invalid metadata manifests (#125117)

src/plugins/manifest-metadata-scan.test.ts

@@ -2,12 +2,30 @@
 import fs from "node:fs";
 import os from "node:os";
 import path from "node:path";
-import { afterEach, describe, expect, it, vi } from "vitest";
+import { afterEach, assert, beforeEach, describe, expect, it, vi } from "vitest";
 import { writePersistedInstalledPluginIndexSync } from "./installed-plugin-index-store.js";
 import { listOpenClawPluginManifestMetadata } from "./manifest-metadata-scan.js";
 import { loadPluginManifest } from "./manifest.js";
 import { clearPluginMetadataLifecycleCaches } from "./plugin-metadata-lifecycle.js";
 
+const { manifestScanWarn } = vi.hoisted(() => ({
+  manifestScanWarn: vi.fn(),
+}));
+
+vi.mock("../logging/subsystem.js", async () => {
+  const actual =
+    await vi.importActual<typeof import("../logging/subsystem.js")>("../logging/subsystem.js");
+  return {
+    ...actual,
+    createSubsystemLogger: (subsystem: string) => {
+      const logger = actual.createSubsystemLogger(subsystem);
+      return subsystem === "plugins/manifest-metadata-scan"
+        ? { ...logger, warn: manifestScanWarn }
+        : logger;
+    },
+  };
+});
+
 const tempRoots: string[] = [];
 
 function createTempRoot(): string {
@@ -21,7 +39,42 @@ function writeJson(filePath: string, value: unknown): void {
   fs.writeFileSync(filePath, JSON.stringify(value, null, 2), "utf8");
 }
 
+function createGlobalPluginFixture(pluginName: string) {
+  const root = createTempRoot();
+  const home = path.join(root, "home");
+  const pluginDir = path.join(home, ".openclaw", "extensions", pluginName);
+  const manifestPath = path.join(pluginDir, "openclaw.plugin.json");
+  fs.mkdirSync(pluginDir, { recursive: true });
+  return {
+    pluginDir,
+    manifestPath,
+    env: {
+      OPENCLAW_HOME: home,
+      OPENCLAW_BUNDLED_PLUGINS_DIR: path.join(root, "empty-bundled"),
+    },
+  };
+}
+
+function warningMessagesForPath(manifestPath: string): string[] {
+  return manifestScanWarn.mock.calls
+    .map(([message]) => String(message))
+    .filter((message) => message.includes(manifestPath));
+}
+
+function expectPluginAbsentAcrossTwoScans(pluginDir: string, env: NodeJS.ProcessEnv): void {
+  for (const records of [
+    listOpenClawPluginManifestMetadata(env),
+    listOpenClawPluginManifestMetadata(env),
+  ]) {
+    expect(records.find((record) => record.pluginDir === pluginDir)).toBeUndefined();
+  }
+}
+
 describe("listOpenClawPluginManifestMetadata", () => {
+  beforeEach(() => {
+    manifestScanWarn.mockClear();
+  });
+
   afterEach(() => {
     vi.restoreAllMocks();
     clearPluginMetadataLifecycleCaches();
@@ -248,14 +301,75 @@ describe("listOpenClawPluginManifestMetadata", () => {
     );
     expect(fs.statSync(oversizedPath).size).toBeGreaterThan(256 * 1024);
 
-    const records = listOpenClawPluginManifestMetadata({
+    const env = {
       OPENCLAW_HOME: home,
       OPENCLAW_BUNDLED_PLUGINS_DIR: path.join(root, "empty-bundled"),
-    });
+    };
+    const records = listOpenClawPluginManifestMetadata(env);
+    const cachedRecords = listOpenClawPluginManifestMetadata(env);
 
     // "good-plugin" is present; "big-plugin" is skipped due to oversized manifest.
     expect(records.find((record) => record.manifest.id === "good-plugin")).toBeTruthy();
     expect(records.find((record) => record.manifest.id === "big-plugin")).toBeUndefined();
+    expect(cachedRecords).toEqual(records);
+    expect(warningMessagesForPath(oversizedPath)).toEqual([
+      `Ignoring oversized plugin manifest at ${oversizedPath}: file exceeds the 262144-byte limit`,
+    ]);
+  });
+
+  it.each([
+    {
+      name: "malformed JSON and JSON5",
+      contents: "{invalid",
+    },
+    {
+      name: "valid non-object JSON",
+      contents: "[]",
+    },
+  ])("skips $name and warns once across cache hits", ({ contents }) => {
+    const { pluginDir, manifestPath, env } = createGlobalPluginFixture("invalid-plugin");
+    fs.writeFileSync(manifestPath, contents, "utf8");
+
+    const canonicalResult = loadPluginManifest(pluginDir, false);
+    assert(!canonicalResult.ok);
+
+    expectPluginAbsentAcrossTwoScans(pluginDir, env);
+    expect(warningMessagesForPath(manifestPath)).toEqual([
+      `Ignoring invalid plugin manifest at ${manifestPath}: ${canonicalResult.error}`,
+    ]);
+  });
+
+  it("warns once for a present non-regular manifest across cache hits", () => {
+    const { pluginDir, manifestPath, env } = createGlobalPluginFixture("non-regular-plugin");
+    fs.mkdirSync(manifestPath);
+
+    expectPluginAbsentAcrossTwoScans(pluginDir, env);
+    expect(warningMessagesForPath(manifestPath)).toEqual([
+      `Ignoring unreadable plugin manifest at ${manifestPath}: Error: path must be a regular file`,
+    ]);
+  });
+
+  it("silently skips a child plugin directory with no manifest across cache hits", () => {
+    const { pluginDir, manifestPath, env } = createGlobalPluginFixture("missing-manifest-plugin");
+
+    expectPluginAbsentAcrossTwoScans(pluginDir, env);
+    expect(warningMessagesForPath(manifestPath)).toEqual([]);
+  });
+
+  it("accepts JSON5 manifests without warning when strict JSON parsing fails", () => {
+    const { pluginDir, manifestPath, env } = createGlobalPluginFixture("json5-plugin");
+    const contents = "{ id: 'json5-plugin', }";
+    fs.writeFileSync(manifestPath, contents, "utf8");
+    expect(() => JSON.parse(contents)).toThrow();
+
+    const records = listOpenClawPluginManifestMetadata(env);
+
+    expect(records).toContainEqual({
+      pluginDir,
+      manifest: { id: "json5-plugin" },
+      origin: "global",
+    });
+    expect(warningMessagesForPath(manifestPath)).toEqual([]);
   });
 
   it("accepts plugin manifests at the exact byte limit", () => {

src/plugins/manifest-metadata-scan.ts

@@ -6,6 +6,7 @@ import { normalizeOptionalString as normalizeTrimmedString } from "@openclaw/nor
 import { resolveRealpathOrAbsolute } from "../infra/boundary-path.js";
 import { resolveHomeRelativePath } from "../infra/home-dir.js";
 import { resolveOpenClawPackageRootSync } from "../infra/openclaw-root.js";
+import { hasNodeErrorCode, isNotFoundPathError } from "../infra/path-guards.js";
 import { readRegularFileSync } from "../infra/regular-file.js";
 import { createSubsystemLogger } from "../logging/subsystem.js";
 import { parseJsonWithJson5Fallback } from "../utils/parse-json-compat.js";
@@ -71,21 +72,42 @@ function listChildPluginDirs(
 }
 
 function readJsonObject(filePath: string): Record<string, unknown> | undefined {
+  let raw: string;
   try {
     const { buffer } = readRegularFileSync({
       filePath,
       maxBytes: PLUGIN_MANIFEST_METADATA_MAX_BYTES,
     });
-    const parsed = parseJsonWithJson5Fallback(buffer.toString("utf-8"));
-    return isRecord(parsed) ? parsed : undefined;
-  } catch (err) {
-    if (err instanceof Error && err.message.includes("exceeds")) {
+    raw = buffer.toString("utf-8");
+  } catch (error) {
+    if (isNotFoundPathError(error)) {
+      return undefined;
+    }
+    if (hasNodeErrorCode(error, "too-large")) {
       log.warn(
         `Ignoring oversized plugin manifest at ${filePath}: file exceeds the ${PLUGIN_MANIFEST_METADATA_MAX_BYTES}-byte limit`,
       );
+    } else {
+      log.warn(`Ignoring unreadable plugin manifest at ${filePath}: ${String(error)}`);
     }
     return undefined;
   }
+
+  let parsed: unknown;
+  try {
+    parsed = parseJsonWithJson5Fallback(raw);
+  } catch (error) {
+    log.warn(
+      `Ignoring invalid plugin manifest at ${filePath}: failed to parse plugin manifest: ${String(error)}`,
+    );
+    return undefined;
+  }
+
+  if (!isRecord(parsed)) {
+    log.warn(`Ignoring invalid plugin manifest at ${filePath}: plugin manifest must be an object`);
+    return undefined;
+  }
+  return parsed;
 }
 
 function readManifestObject(pluginDir: string): Record<string, unknown> | undefined {