fix(plugins): trust managed npm peer links

vincentkoc · vincentkoc · commit 5ecd01ff94d6 · 2026-05-03T01:49:24.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai
 - Matrix: persist pending approval reaction targets across Gateway restarts so room approvers can still approve or deny outstanding prompts after OpenClaw comes back online. (#75586) Thanks @amknight.
 - Channels/onboarding: map third-party official WeCom and Yuanbao catalog entries to their published plugin ids so npm installs pass expected-plugin validation. Thanks @vincentkoc.
 - Plugin SDK: restore the Mattermost and Matrix compatibility subpaths used by the pinned Yuanbao channel package so external installs can module-load after npm install. Thanks @vincentkoc.
+- Plugins/install: keep managed npm-root security scans from treating earlier plugin `openclaw` peer links as failures, so one external plugin install cannot poison later official npm installs. Thanks @vincentkoc.
 - CLI/plugins: keep `plugins enable` and `plugins disable` from creating unconfigured channel config sections, so channel plugins with required setup fields no longer fail validation during lifecycle probes. Thanks @vincentkoc.
 - Agents/sessions: keep delayed `sessions_send` A2A replies alive after soft wait-window timeouts, while preserving terminal run timeouts and avoiding stale target replies in requester sessions. Fixes #76443. Thanks @ryswork1993 and @vincentkoc.
 - CLI/sessions: keep intentional empty agent replies silent after tool-delivered channel output, instead of surfacing a misleading "No reply from agent." fallback. Thanks @vincentkoc.
diff --git a/src/plugins/install-security-scan.runtime.ts b/src/plugins/install-security-scan.runtime.ts
@@ -177,8 +177,7 @@ function pathContainsNodeModulesSegment(relativePath: string): boolean {
     .includes("node_modules");
 }
 
-function isTrustedOpenClawPeerSymlink(relativePath: string): boolean {
-  const segments = relativePath.split(/[\\/]+/);
+function isPackageRootOpenClawPeerSymlink(segments: string[]): boolean {
   return (
     (segments.length === 2 && segments[0] === "node_modules" && segments[1] === "openclaw") ||
     (segments.length === 3 &&
@@ -188,6 +187,33 @@ function isTrustedOpenClawPeerSymlink(relativePath: string): boolean {
   );
 }
 
+function isManagedNpmRootPackagePeerSymlink(segments: string[]): boolean {
+  if (segments[0] !== "node_modules") {
+    return false;
+  }
+  const packageEndIndex = segments[1]?.startsWith("@") ? 3 : 2;
+  const packageNameSegments = segments.slice(1, packageEndIndex);
+  if (
+    packageNameSegments.length === 0 ||
+    packageNameSegments.some((segment) => !segment || segment === "." || segment === "..")
+  ) {
+    return false;
+  }
+  return isPackageRootOpenClawPeerSymlink(segments.slice(packageEndIndex));
+}
+
+function isTrustedOpenClawPeerSymlink(params: {
+  allowManagedNpmRootPackagePeerSymlinks?: boolean;
+  relativePath: string;
+}): boolean {
+  const segments = params.relativePath.split(/[\\/]+/);
+  return (
+    isPackageRootOpenClawPeerSymlink(segments) ||
+    (params.allowManagedNpmRootPackagePeerSymlinks === true &&
+      isManagedNpmRootPackagePeerSymlink(segments))
+  );
+}
+
 async function resolveTrustedHostOpenClawRootRealPath(): Promise<string | null> {
   const hostRoot = resolveOpenClawPackageRootSync({
     argv1: process.argv[1],
@@ -211,6 +237,7 @@ function isTrustedHostOpenClawPath(params: {
 }
 
 async function inspectNodeModulesSymlinkTarget(params: {
+  allowManagedNpmRootPackagePeerSymlinks?: boolean;
   rootRealPath: string;
   symlinkPath: string;
   symlinkRelativePath: string;
@@ -235,7 +262,10 @@ async function inspectNodeModulesSymlinkTarget(params: {
     // package. Trust only the exact peer-link shapes and only when the resolved
     // target stays inside the host package root.
     if (
-      isTrustedOpenClawPeerSymlink(params.symlinkRelativePath) &&
+      isTrustedOpenClawPeerSymlink({
+        allowManagedNpmRootPackagePeerSymlinks: params.allowManagedNpmRootPackagePeerSymlinks,
+        relativePath: params.symlinkRelativePath,
+      }) &&
       isTrustedHostOpenClawPath({
         resolvedTargetPath,
         trustedHostOpenClawRootRealPath: params.trustedHostOpenClawRootRealPath,
@@ -329,10 +359,12 @@ function resolvePackageManifestTraversalLimits(): PackageManifestTraversalLimits
   };
 }
 
-async function collectPackageManifestPaths(
-  rootDir: string,
-): Promise<PackageManifestTraversalResult> {
+async function collectPackageManifestPaths(params: {
+  allowManagedNpmRootPackagePeerSymlinks?: boolean;
+  rootDir: string;
+}): Promise<PackageManifestTraversalResult> {
   const limits = resolvePackageManifestTraversalLimits();
+  const rootDir = params.rootDir;
   const rootRealPath = await fs.realpath(rootDir).catch(() => rootDir);
   const trustedHostOpenClawRootRealPath = await resolveTrustedHostOpenClawRootRealPath();
   const queue: Array<{ depth: number; dir: string }> = [{ depth: 0, dir: rootDir }];
@@ -401,6 +433,7 @@ async function collectPackageManifestPaths(
         }
         if (pathContainsNodeModulesSegment(relativeNextPath)) {
           const symlinkTargetInspection = await inspectNodeModulesSymlinkTarget({
+            allowManagedNpmRootPackagePeerSymlinks: params.allowManagedNpmRootPackagePeerSymlinks,
             rootRealPath,
             symlinkPath: nextPath,
             symlinkRelativePath: relativeNextPath,
@@ -452,11 +485,15 @@ async function collectPackageManifestPaths(
 }
 
 async function scanManifestDependencyDenylist(params: {
+  allowManagedNpmRootPackagePeerSymlinks?: boolean;
   logger: InstallScanLogger;
   packageDir: string;
   targetLabel: string;
 }): Promise<InstallSecurityScanResult | undefined> {
-  const traversalResult = await collectPackageManifestPaths(params.packageDir);
+  const traversalResult = await collectPackageManifestPaths({
+    allowManagedNpmRootPackagePeerSymlinks: params.allowManagedNpmRootPackagePeerSymlinks,
+    rootDir: params.packageDir,
+  });
   const packageManifestPaths = traversalResult.packageManifestPaths;
   for (const manifestPath of packageManifestPaths) {
     let manifest: PackageManifest;
@@ -857,13 +894,15 @@ export async function scanPackageInstallSourceRuntime(
 }
 
 export async function scanInstalledPackageDependencyTreeRuntime(params: {
+  allowManagedNpmRootPackagePeerSymlinks?: boolean;
   logger: InstallScanLogger;
   packageDir: string;
   pluginId: string;
 }): Promise<InstallSecurityScanResult | undefined> {
   return await scanManifestDependencyDenylist({
     logger: params.logger,
     packageDir: params.packageDir,
+    allowManagedNpmRootPackagePeerSymlinks: params.allowManagedNpmRootPackagePeerSymlinks,
     targetLabel: `Plugin "${params.pluginId}" installation`,
   });
 }
diff --git a/src/plugins/install-security-scan.ts b/src/plugins/install-security-scan.ts
@@ -73,6 +73,7 @@ export async function scanPackageInstallSource(
 }
 
 export async function scanInstalledPackageDependencyTree(params: {
+  allowManagedNpmRootPackagePeerSymlinks?: boolean;
   logger: InstallScanLogger;
   packageDir: string;
   pluginId: string;
diff --git a/src/plugins/install.npm-spec.test.ts b/src/plugins/install.npm-spec.test.ts
@@ -61,6 +61,7 @@ function writeInstalledNpmPlugin(params: {
   indexJs?: string;
   dependency?: { name: string; version: string };
   hoistedDependency?: { name: string; version: string };
+  peerDependencies?: Record<string, string>;
 }) {
   const pluginDir = path.join(params.npmRoot, "node_modules", params.packageName);
   fs.mkdirSync(path.join(pluginDir, "dist"), { recursive: true });
@@ -73,6 +74,7 @@ function writeInstalledNpmPlugin(params: {
       ...(params.dependency
         ? { dependencies: { [params.dependency.name]: params.dependency.version } }
         : {}),
+      ...(params.peerDependencies ? { peerDependencies: params.peerDependencies } : {}),
     }),
     "utf-8",
   );
@@ -128,26 +130,60 @@ function mockNpmViewAndInstall(params: {
   indexJs?: string;
   dependency?: { name: string; version: string };
   hoistedDependency?: { name: string; version: string };
+  peerDependencies?: Record<string, string>;
 }) {
+  mockNpmViewAndInstallMany([params]);
+}
+
+function mockNpmViewAndInstallMany(
+  packages: Array<{
+    spec: string;
+    packageName: string;
+    version: string;
+    npmRoot: string;
+    pluginId?: string;
+    integrity?: string;
+    shasum?: string;
+    indexJs?: string;
+    dependency?: { name: string; version: string };
+    hoistedDependency?: { name: string; version: string };
+    peerDependencies?: Record<string, string>;
+  }>,
+) {
+  const packagesBySpec = new Map(packages.map((pkg) => [pkg.spec, pkg]));
+  const packagesByName = new Map(packages.map((pkg) => [pkg.packageName, pkg]));
   runCommandWithTimeoutMock.mockImplementation(async (argv: string[]) => {
-    if (JSON.stringify(argv) === JSON.stringify(npmViewArgv(params.spec))) {
+    const viewPackage = packages.find(
+      (pkg) => JSON.stringify(argv) === JSON.stringify(npmViewArgv(pkg.spec)),
+    );
+    if (viewPackage) {
       return successfulSpawn(
         JSON.stringify({
-          name: params.packageName,
-          version: params.version,
+          name: viewPackage.packageName,
+          version: viewPackage.version,
           dist: {
-            integrity: params.integrity ?? "sha512-plugin-test",
-            shasum: params.shasum ?? "pluginshasum",
+            integrity: viewPackage.integrity ?? "sha512-plugin-test",
+            shasum: viewPackage.shasum ?? "pluginshasum",
           },
         }),
       );
     }
     if (argv[0] === "npm" && argv[1] === "install") {
-      writeInstalledNpmPlugin(params);
+      const spec = argv.at(-1);
+      const pkg = spec ? packagesBySpec.get(spec) : undefined;
+      if (!pkg) {
+        throw new Error(`unexpected npm install spec: ${spec ?? ""}`);
+      }
+      writeInstalledNpmPlugin(pkg);
       return successfulSpawn();
     }
     if (argv[0] === "npm" && argv[1] === "uninstall") {
-      fs.rmSync(path.join(params.npmRoot, "node_modules", params.packageName), {
+      const packageName = argv.at(-1);
+      const pkg = packageName ? packagesByName.get(packageName) : undefined;
+      if (!pkg) {
+        throw new Error(`unexpected npm uninstall package: ${packageName ?? ""}`);
+      }
+      fs.rmSync(path.join(pkg.npmRoot, "node_modules", pkg.packageName), {
         recursive: true,
         force: true,
       });
@@ -230,6 +266,55 @@ describe("installPluginFromNpmSpec", () => {
     }
   });
 
+  it.runIf(process.platform !== "win32")(
+    "does not let managed openclaw peer links poison later npm installs",
+    async () => {
+      const stateDir = suiteTempRootTracker.makeTempDir();
+      const npmRoot = path.join(stateDir, "npm");
+
+      mockNpmViewAndInstallMany([
+        {
+          spec: "peer-plugin@1.0.0",
+          packageName: "peer-plugin",
+          version: "1.0.0",
+          pluginId: "peer-plugin",
+          npmRoot,
+          peerDependencies: { openclaw: "^2026.0.0" },
+        },
+        {
+          spec: "next-plugin@1.0.0",
+          packageName: "next-plugin",
+          version: "1.0.0",
+          pluginId: "next-plugin",
+          npmRoot,
+        },
+      ]);
+
+      const first = await installPluginFromNpmSpec({
+        spec: "peer-plugin@1.0.0",
+        npmDir: npmRoot,
+        logger: { info: () => {}, warn: () => {} },
+      });
+      expect(first.ok).toBe(true);
+      expect(
+        fs
+          .lstatSync(path.join(npmRoot, "node_modules", "peer-plugin", "node_modules", "openclaw"))
+          .isSymbolicLink(),
+      ).toBe(true);
+
+      const second = await installPluginFromNpmSpec({
+        spec: "next-plugin@1.0.0",
+        npmDir: npmRoot,
+        logger: { info: () => {}, warn: () => {} },
+      });
+
+      expect(second.ok).toBe(true);
+      if (!second.ok) {
+        expect(second.error).not.toContain("peer-plugin/node_modules/openclaw");
+      }
+    },
+  );
+
   it("allows npm-spec installs with dangerous code patterns when forced unsafe install is set", async () => {
     const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm");
     const warnings: string[] = [];
diff --git a/src/plugins/install.ts b/src/plugins/install.ts
@@ -799,6 +799,9 @@ async function scanAndLinkInstalledPackage(params: {
     subject: `Plugin "${params.pluginId}"`,
     scan: async () =>
       await params.runtime.scanInstalledPackageDependencyTree({
+        allowManagedNpmRootPackagePeerSymlinks:
+          params.dependencyScanRootDir !== undefined &&
+          path.resolve(params.dependencyScanRootDir) !== path.resolve(params.installedDir),
         logger: params.logger,
         packageDir: params.dependencyScanRootDir ?? params.installedDir,
         pluginId: params.pluginId,

Original file line number	Diff line number	Diff line change
`@@ -73,6 +73,7 @@ export async function scanPackageInstallSource(`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`export async function scanInstalledPackageDependencyTree(params: {`
	`76`	`+ allowManagedNpmRootPackagePeerSymlinks?: boolean;`
`76`	`77`	`logger: InstallScanLogger;`
`77`	`78`	`packageDir: string;`
`78`	`79`	`pluginId: string;`