From 3be24fe7149a1c4eec1619e45fbe160b9ab6da40 Mon Sep 17 00:00:00 2001 From: Voidra <151359367+NirvanJain@users.noreply.github.com> Date: Wed, 26 Aug 2026 07:23:27 +0530 Subject: [PATCH] fix(plugins): report missing manifest instead of misleading id mismatch on install (#128527) * fix(plugins): report missing manifest instead of misleading id mismatch on install Added comments to clarify error handling for missing or invalid plugin manifests. * fix/comfy-manifest-mismatch-error * fix(plugins): require manifests for trusted npm installs Co-authored-by: Voidra <151359367+NirvanJain@users.noreply.github.com> --------- Co-authored-by: Peter Steinberger --- src/plugins/install-managed-npm.ts | 1 + src/plugins/install.npm-spec.test.ts | 108 ++++++++++++++++++++++++--- 2 files changed, 99 insertions(+), 10 deletions(-) diff --git a/src/plugins/install-managed-npm.ts b/src/plugins/install-managed-npm.ts index be1bf7f572ec..a47b19de1887 100644 --- a/src/plugins/install-managed-npm.ts +++ b/src/plugins/install-managed-npm.ts @@ -602,6 +602,7 @@ export async function installPluginFromManagedNpmRoot( dependencyScanRootDir: npmRoot, logger, expectedPluginId: installedExpectedPluginId, + requirePluginManifest: params.trustedSourceLinkedOfficialInstall, trustedSourceLinkedOfficialInstall: params.trustedSourceLinkedOfficialInstall, mode: policyMode, installPolicyRequest: params.installPolicyRequest, diff --git a/src/plugins/install.npm-spec.test.ts b/src/plugins/install.npm-spec.test.ts index 634e04c965d0..85bc17f9e8ff 100644 --- a/src/plugins/install.npm-spec.test.ts +++ b/src/plugins/install.npm-spec.test.ts @@ -222,6 +222,7 @@ function writeInstalledNpmPlugin(params: { packageName: string; version: string; pluginId?: string; + nativeManifest?: "missing" | "malformed"; legacyPluginIds?: string[]; indexJs?: string; extraDistFiles?: Record; @@ -249,16 +250,20 @@ function writeInstalledNpmPlugin(params: { }), "utf-8", ); - fs.writeFileSync( - path.join(pluginDir, "openclaw.plugin.json"), - JSON.stringify({ - id: params.pluginId ?? params.packageName, - name: params.pluginId ?? params.packageName, - ...(params.legacyPluginIds ? { legacyPluginIds: params.legacyPluginIds } : {}), - configSchema: { type: "object" }, - }), - "utf-8", - ); + if (params.nativeManifest !== "missing") { + fs.writeFileSync( + path.join(pluginDir, "openclaw.plugin.json"), + params.nativeManifest === "malformed" + ? "{invalid plugin manifest" + : JSON.stringify({ + id: params.pluginId ?? params.packageName, + name: params.pluginId ?? params.packageName, + ...(params.legacyPluginIds ? { legacyPluginIds: params.legacyPluginIds } : {}), + configSchema: { type: "object" }, + }), + "utf-8", + ); + } fs.writeFileSync( path.join(pluginDir, "dist", "index.js"), params.indexJs ?? "export {};", @@ -302,6 +307,7 @@ type MockNpmPackage = { version: string; npmRoot: string; pluginId?: string; + nativeManifest?: "missing" | "malformed"; legacyPluginIds?: string[]; integrity?: string; shasum?: string; @@ -3426,6 +3432,88 @@ describe("installPluginFromNpmSpec", () => { }, ); + it.each([ + { + name: "missing manifest with a different expected id", + packageName: "@openclaw/comfy-provider", + expectedPluginId: "comfy", + nativeManifest: "missing", + }, + { + name: "missing manifest even when the package name matches the expected id", + packageName: "@openclaw/official-placeholder", + expectedPluginId: "@openclaw/official-placeholder", + nativeManifest: "missing", + }, + { + name: "malformed manifest", + packageName: "@openclaw/official-malformed", + expectedPluginId: "official-malformed", + nativeManifest: "malformed", + }, + ] as const)("rejects a trusted official npm plugin with a $name", async (testCase) => { + const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm"); + const npmProjectRoot = resolvePluginNpmProjectDir({ + npmDir: npmRoot, + packageName: testCase.packageName, + }); + mockNpmViewAndInstall({ + spec: testCase.packageName, + packageName: testCase.packageName, + version: "0.0.0", + pluginId: testCase.expectedPluginId, + nativeManifest: testCase.nativeManifest, + npmRoot, + }); + + const result = await installPluginFromNpmSpec({ + spec: testCase.packageName, + npmDir: npmRoot, + expectedPluginId: testCase.expectedPluginId, + trustedSourceLinkedOfficialInstall: true, + logger: { info: () => {}, warn: () => {} }, + }); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + expect(result.code).toBe(PLUGIN_INSTALL_ERROR_CODE.MISSING_PLUGIN_MANIFEST); + expect(result.error).toContain("package missing valid openclaw.plugin.json"); + expect(result.error).not.toContain("plugin id mismatch"); + expect(fs.existsSync(resolveTestPluginPackageDir(npmRoot, testCase.packageName))).toBe(false); + expect(fs.existsSync(path.join(npmProjectRoot, "package.json"))).toBe(false); + expect(fs.existsSync(path.join(npmProjectRoot, "package-lock.json"))).toBe(false); + expect(fs.existsSync(path.join(npmProjectRoot, "node_modules"))).toBe(false); + }); + + it("preserves untrusted manifestless npm updates keyed by the legacy package name", async () => { + const packageName = "@third-party/legacy-plugin"; + const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm"); + mockNpmViewAndInstall({ + spec: packageName, + packageName, + version: "1.0.0", + nativeManifest: "missing", + npmRoot, + }); + + const result = await installPluginFromNpmSpec({ + spec: packageName, + npmDir: npmRoot, + expectedPluginId: "legacy-plugin", + trustedSourceLinkedOfficialInstall: false, + mode: "update", + logger: { info: () => {}, warn: () => {} }, + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.pluginId).toBe(packageName); + expect(fs.existsSync(resolveTestPluginPackageDir(npmRoot, packageName))).toBe(true); + } + }); + it("accepts a trusted manifest-declared plugin id replacement during update", async () => { const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm"); mockNpmViewAndInstall({