From e536a8648acb3ef1c28c27e68dc01a98fb37fa01 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 5 May 2026 23:04:06 -0700 Subject: [PATCH] fix(plugins): prevent managed npm peer materialization --- src/infra/safe-package-install.test.ts | 2 + src/infra/safe-package-install.ts | 2 + src/plugins/install.npm-spec.test.ts | 50 +++++++++++++ src/plugins/install.ts | 100 ++++++++++++++++++------- 4 files changed, 128 insertions(+), 26 deletions(-) diff --git a/src/infra/safe-package-install.test.ts b/src/infra/safe-package-install.test.ts index 98caedccf43..03a5bdf4d0c 100644 --- a/src/infra/safe-package-install.test.ts +++ b/src/infra/safe-package-install.test.ts @@ -6,6 +6,7 @@ describe("safe npm install helpers", () => { expect( createSafeNpmInstallArgs({ omitDev: true, + omitPeer: true, ignoreWorkspaces: true, loglevel: "error", noAudit: true, @@ -14,6 +15,7 @@ describe("safe npm install helpers", () => { ).toEqual([ "install", "--omit=dev", + "--omit=peer", "--loglevel=error", "--ignore-scripts", "--workspaces=false", diff --git a/src/infra/safe-package-install.ts b/src/infra/safe-package-install.ts index 72f318981d3..d4b83399936 100644 --- a/src/infra/safe-package-install.ts +++ b/src/infra/safe-package-install.ts @@ -14,6 +14,7 @@ type SafeNpmInstallArgsOptions = { noAudit?: boolean; noFund?: boolean; omitDev?: boolean; + omitPeer?: boolean; }; export function createSafeNpmInstallEnv( @@ -46,6 +47,7 @@ export function createSafeNpmInstallArgs(options: SafeNpmInstallArgsOptions = {} return [ "install", ...(options.omitDev ? ["--omit=dev"] : []), + ...(options.omitPeer ? ["--omit=peer"] : []), ...(options.loglevel ? [`--loglevel=${options.loglevel}`] : []), "--ignore-scripts", ...(options.ignoreWorkspaces ? ["--workspaces=false"] : []), diff --git a/src/plugins/install.npm-spec.test.ts b/src/plugins/install.npm-spec.test.ts index 7aea207823e..616fa80bc37 100644 --- a/src/plugins/install.npm-spec.test.ts +++ b/src/plugins/install.npm-spec.test.ts @@ -54,6 +54,7 @@ function expectNpmInstallIntoRoot(params: { calls: unknown[][]; npmRoot: string "npm", "install", "--omit=dev", + "--omit=peer", "--loglevel=error", "--ignore-scripts", "--no-audit", @@ -145,6 +146,7 @@ type MockNpmPackage = { versions?: string[]; installedVersion?: string; installedIntegrity?: string; + materializesRootOpenClaw?: boolean; skipLockfileEntry?: boolean; }; @@ -166,6 +168,11 @@ function writeNpmRootPackageLock(params: { version: pkg.installedVersion ?? pkg.version, integrity: pkg.installedIntegrity ?? pkg.integrity ?? "sha512-plugin-test", }; + if (pkg.materializesRootOpenClaw) { + lockPackages["node_modules/openclaw"] = { + version: "2026.5.3", + }; + } } fs.writeFileSync( path.join(params.npmRoot, "package-lock.json"), @@ -190,6 +197,7 @@ function mockNpmViewAndInstall(params: { versions?: string[]; installedVersion?: string; installedIntegrity?: string; + materializesRootOpenClaw?: boolean; skipLockfileEntry?: boolean; }) { mockNpmViewAndInstallMany([params]); @@ -255,6 +263,15 @@ function mockNpmViewAndInstallMany(packages: MockNpmPackage[]) { ...pkg, version: pkg.installedVersion ?? pkg.version, }); + if (pkg.materializesRootOpenClaw) { + const openclawRoot = path.join(npmRoot, "node_modules", "openclaw"); + fs.mkdirSync(openclawRoot, { recursive: true }); + fs.writeFileSync( + path.join(openclawRoot, "package.json"), + JSON.stringify({ name: "openclaw", version: "2026.5.3" }), + "utf-8", + ); + } installedPackages.push(pkg); } writeNpmRootPackageLock({ @@ -553,6 +570,39 @@ describe("installPluginFromNpmSpec", () => { }, ); + it.runIf(process.platform !== "win32")( + "repairs root openclaw materialized by npm peer handling and links the host peer", + async () => { + const stateDir = suiteTempRootTracker.makeTempDir(); + const npmRoot = path.join(stateDir, "npm"); + + mockNpmViewAndInstall({ + spec: "peer-plugin@1.0.0", + packageName: "peer-plugin", + version: "1.0.0", + pluginId: "peer-plugin", + npmRoot, + peerDependencies: { openclaw: "^2026.0.0" }, + materializesRootOpenClaw: true, + }); + + const result = await installPluginFromNpmSpec({ + spec: "peer-plugin@1.0.0", + npmDir: npmRoot, + trustedManagedNpmRoot: true, + logger: { info: () => {}, warn: () => {} }, + }); + + expect(result.ok).toBe(true); + expect(fs.existsSync(path.join(npmRoot, "node_modules", "openclaw"))).toBe(false); + expect( + fs + .lstatSync(path.join(npmRoot, "node_modules", "peer-plugin", "node_modules", "openclaw")) + .isSymbolicLink(), + ).toBe(true); + }, + ); + it("repairs stale managed openclaw root packages before npm plugin installs", async () => { const stateDir = suiteTempRootTracker.makeTempDir(); const npmRoot = path.join(stateDir, "npm"); diff --git a/src/plugins/install.ts b/src/plugins/install.ts index 7ba9a7cf1d3..bb59c2ab313 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -744,6 +744,14 @@ type ValidatedPackagePlugin = { peerDependencies: Record; }; +function resolveOpenClawHostLinkDependencies(manifest: PackageManifest): Record { + const spec = + manifest.peerDependencies?.openclaw ?? + manifest.dependencies?.openclaw ?? + manifest.optionalDependencies?.openclaw; + return spec ? { openclaw: spec } : {}; +} + async function validatePackagePluginInstallSource(params: { runtime: Awaited>; packageDir: string; @@ -901,7 +909,7 @@ async function validatePackagePluginInstallSource(params: { version: typeof manifest.version === "string" ? manifest.version : undefined, extensions, hasRuntimeDependencies: hasPackageRuntimeDependencies(manifest), - peerDependencies: manifest.peerDependencies ?? {}, + peerDependencies: resolveOpenClawHostLinkDependencies(manifest), }, }; } @@ -913,7 +921,15 @@ async function scanAndLinkInstalledPackage(params: { pluginId: string; peerDependencies: Record; logger: PluginInstallLogger; + linkOpenClawBeforeScan?: boolean; }): Promise | null> { + if (params.linkOpenClawBeforeScan) { + await linkOpenClawPeerDependencies({ + installedDir: params.installedDir, + peerDependencies: params.peerDependencies, + logger: params.logger, + }); + } const scanResult = await runInstallSourceScan({ subject: `Plugin "${params.pluginId}"`, scan: async () => @@ -929,11 +945,13 @@ async function scanAndLinkInstalledPackage(params: { if (scanResult) { return scanResult; } - await linkOpenClawPeerDependencies({ - installedDir: params.installedDir, - peerDependencies: params.peerDependencies, - logger: params.logger, - }); + if (!params.linkOpenClawBeforeScan) { + await linkOpenClawPeerDependencies({ + installedDir: params.installedDir, + peerDependencies: params.peerDependencies, + logger: params.logger, + }); + } return null; } @@ -966,6 +984,7 @@ export async function installPluginFromInstalledPackageDir( pluginId: validated.plugin.pluginId, peerDependencies: validated.plugin.peerDependencies, logger, + linkOpenClawBeforeScan: params.dependencyScanRootDir !== undefined, }); if (postInstallError) { return postInstallError; @@ -1216,6 +1235,40 @@ export async function installPluginFromFile(params: { return buildFileInstallResult(pluginId, preparedTarget.targetPath); } +async function repairManagedNpmRootOpenClawPeerForInstall(params: { + logger: PluginInstallLogger; + npmRoot: string; + phase: "before npm install" | "after npm install"; + timeoutMs: number; + trustedManagedNpmRoot?: boolean; +}): Promise { + const repairedOpenClawPeer = await repairManagedNpmRootOpenClawPeer({ + defaultNpmRoot: resolveDefaultPluginNpmDir(), + env: createSafeNpmInstallEnv(process.env, { packageLock: true, quiet: true }), + hostPackageRoot: resolveOpenClawPackageRootSync({ + argv1: process.argv[1], + moduleUrl: import.meta.url, + cwd: process.cwd(), + }), + npmRoot: params.npmRoot, + runCommand: runCommandWithTimeout, + timeoutMs: params.timeoutMs, + trustedByInstallRecord: params.trustedManagedNpmRoot, + }); + for (const warning of repairedOpenClawPeer.warnings) { + params.logger.warn?.(warning); + } + if (repairedOpenClawPeer.status === "repaired") { + params.logger.info?.( + `Repaired stale openclaw peer dependency in ${params.npmRoot} ${params.phase}`, + ); + } else if (repairedOpenClawPeer.status === "skipped") { + params.logger.warn?.( + `Skipped stale openclaw peer repair in ${params.npmRoot} ${params.phase}: ${repairedOpenClawPeer.reason ?? "unproven managed npm root"}`, + ); + } +} + export async function installPluginFromNpmSpec( params: InstallSafetyOverrides & { spec: string; @@ -1340,29 +1393,13 @@ export async function installPluginFromNpmSpec( logger.info?.(`Installing ${spec} into ${npmRoot}…`); if (parsedSpec.name !== "openclaw") { - const repairedOpenClawPeer = await repairManagedNpmRootOpenClawPeer({ - defaultNpmRoot: resolveDefaultPluginNpmDir(), - env: createSafeNpmInstallEnv(process.env, { packageLock: true, quiet: true }), - hostPackageRoot: resolveOpenClawPackageRootSync({ - argv1: process.argv[1], - moduleUrl: import.meta.url, - cwd: process.cwd(), - }), + await repairManagedNpmRootOpenClawPeerForInstall({ + logger, npmRoot, - runCommand: runCommandWithTimeout, + phase: "before npm install", timeoutMs, - trustedByInstallRecord: params.trustedManagedNpmRoot, + trustedManagedNpmRoot: params.trustedManagedNpmRoot, }); - for (const warning of repairedOpenClawPeer.warnings) { - logger.warn?.(warning); - } - if (repairedOpenClawPeer.status === "repaired") { - logger.info?.(`Repaired stale openclaw peer dependency in ${npmRoot}`); - } else if (repairedOpenClawPeer.status === "skipped") { - logger.warn?.( - `Skipped stale openclaw peer repair in ${npmRoot}: ${repairedOpenClawPeer.reason ?? "unproven managed npm root"}`, - ); - } } await upsertManagedNpmRootDependency({ npmRoot, @@ -1377,6 +1414,7 @@ export async function installPluginFromNpmSpec( "npm", ...createSafeNpmInstallArgs({ omitDev: true, + omitPeer: true, loglevel: "error", noAudit: true, noFund: true, @@ -1401,6 +1439,16 @@ export async function installPluginFromNpmSpec( }; } + if (parsedSpec.name !== "openclaw") { + await repairManagedNpmRootOpenClawPeerForInstall({ + logger, + npmRoot, + phase: "after npm install", + timeoutMs, + trustedManagedNpmRoot: params.trustedManagedNpmRoot, + }); + } + let installedDependency: ManagedNpmRootInstalledDependency | null; try { installedDependency = await readManagedNpmRootInstalledDependency({