diff --git a/src/plugins/manifest-metadata-scan.test.ts b/src/plugins/manifest-metadata-scan.test.ts index de2ab9f413dc..0cc86eefc33d 100644 --- a/src/plugins/manifest-metadata-scan.test.ts +++ b/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("../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", () => { diff --git a/src/plugins/manifest-metadata-scan.ts b/src/plugins/manifest-metadata-scan.ts index 85667b268ac4..ba7fc7802c34 100644 --- a/src/plugins/manifest-metadata-scan.ts +++ b/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 | 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 | undefined {