fix: harden managed plugin peer recovery

shakkernerd · shakkernerd · commit f4cb20300f6b · 2026-05-13T15:26:40.000+01:00
diff --git a/src/infra/npm-managed-root.ts b/src/infra/npm-managed-root.ts
@@ -29,6 +29,11 @@ type ManagedNpmRootOpenClawMetadata = {
   [key: string]: unknown;
 };
 
+export type ManagedNpmRootPeerDependencySnapshot = {
+  dependencies: Record<string, string>;
+  managedPeerDependencies: string[];
+};
+
 export type ManagedNpmRootInstalledDependency = {
   version?: string;
   integrity?: string;
@@ -47,6 +52,16 @@ type ManagedNpmRootLogger = {
 
 type ManagedNpmRootRunCommand = typeof runCommandWithTimeout;
 
+type ManagedNpmPeerTraversalLimits = {
+  maxDepth: number;
+  maxDirectories: number;
+};
+
+const DEFAULT_MANAGED_NPM_PEER_TRAVERSAL_LIMITS: ManagedNpmPeerTraversalLimits = {
+  maxDepth: 64,
+  maxDirectories: 10_000,
+};
+
 function isRecord(value: unknown): value is Record<string, unknown> {
   return typeof value === "object" && value !== null && !Array.isArray(value);
 }
@@ -68,6 +83,47 @@ function readDependencyRecord(value: unknown): Record<string, string> {
   return dependencies;
 }
 
+function readPositiveIntegerEnv(name: string, fallback: number): number {
+  const rawValue = process.env[name];
+  if (!rawValue) {
+    return fallback;
+  }
+  const parsedValue = Number.parseInt(rawValue, 10);
+  return Number.isFinite(parsedValue) && parsedValue >= 1 ? parsedValue : fallback;
+}
+
+function resolveManagedNpmPeerTraversalLimits(): ManagedNpmPeerTraversalLimits {
+  return {
+    maxDepth: readPositiveIntegerEnv(
+      "OPENCLAW_INSTALL_SCAN_MAX_DEPTH",
+      DEFAULT_MANAGED_NPM_PEER_TRAVERSAL_LIMITS.maxDepth,
+    ),
+    maxDirectories: readPositiveIntegerEnv(
+      "OPENCLAW_INSTALL_SCAN_MAX_DIRECTORIES",
+      DEFAULT_MANAGED_NPM_PEER_TRAVERSAL_LIMITS.maxDirectories,
+    ),
+  };
+}
+
+function isSamePathOrInside(parentPath: string, candidatePath: string): boolean {
+  const relative = path.relative(parentPath, candidatePath);
+  return (
+    relative === "" || (!!relative && !relative.startsWith("..") && !path.isAbsolute(relative))
+  );
+}
+
+function isSafePackageName(name: string): boolean {
+  if (name.startsWith("@")) {
+    const parts = name.split("/");
+    return (
+      parts.length === 2 && parts.every((part) => part.length > 0 && part !== "." && part !== "..")
+    );
+  }
+  return (
+    name.length > 0 && !name.includes("/") && !name.includes("\\") && name !== "." && name !== ".."
+  );
+}
+
 function readOverrideRecord(value: unknown): Record<string, unknown> {
   if (!isRecord(value)) {
     return {};
@@ -301,7 +357,7 @@ async function listNodeModulesPackageDirs(nodeModulesDir: string): Promise<strin
   }
   const packageDirs: string[] = [];
   for (const entry of entries.toSorted((left, right) => left.name.localeCompare(right.name))) {
-    if (entry.name === ".bin" || entry.name.startsWith(".")) {
+    if (entry.name === ".bin" || entry.name === "openclaw" || entry.name.startsWith(".")) {
       continue;
     }
     const entryPath = path.join(nodeModulesDir, entry.name);
@@ -338,20 +394,50 @@ async function collectManagedNpmRootPeerDependencyPins(params: {
   npmRoot: string;
 }): Promise<Record<string, string>> {
   const pins = new Map<string, string>();
-  const queue = await listNodeModulesPackageDirs(path.join(params.npmRoot, "node_modules"));
+  const limits = resolveManagedNpmPeerTraversalLimits();
+  const boundaryRealPath = await fs
+    .realpath(params.npmRoot)
+    .catch(() => path.resolve(params.npmRoot));
+  const queue = (await listNodeModulesPackageDirs(path.join(params.npmRoot, "node_modules"))).map(
+    (packageDir) => ({ depth: 0, packageDir }),
+  );
+  const visitedRealPaths = new Set<string>();
   for (let index = 0; index < queue.length; index += 1) {
-    const packageDir = queue[index];
-    if (!packageDir) {
+    const current = queue[index];
+    if (!current) {
+      continue;
+    }
+    if (current.depth > limits.maxDepth) {
+      throw new Error(
+        `managed npm peer dependency scan exceeded max depth (${limits.maxDepth}) at ${current.packageDir}`,
+      );
+    }
+    const packageDirRealPath = await fs
+      .realpath(current.packageDir)
+      .catch(() => path.resolve(current.packageDir));
+    if (!isSamePathOrInside(boundaryRealPath, packageDirRealPath)) {
+      throw new Error(
+        `managed npm peer dependency scan found package outside managed npm root at ${current.packageDir}`,
+      );
+    }
+    if (visitedRealPaths.has(packageDirRealPath)) {
       continue;
     }
+    visitedRealPaths.add(packageDirRealPath);
+    if (visitedRealPaths.size > limits.maxDirectories) {
+      throw new Error(
+        `managed npm peer dependency scan exceeded max packages (${limits.maxDirectories}) under ${params.npmRoot}`,
+      );
+    }
+    const packageDir = current.packageDir;
     const manifest = await readPackageJsonIfExists(packageDir);
     if (manifest) {
       if (readOptionalString(manifest.name) === "openclaw") {
         continue;
       }
       const peerDependencies = readDependencyRecord(manifest.peerDependencies);
       for (const [peerName, peerRange] of Object.entries(peerDependencies)) {
-        if (peerName === "openclaw" || pins.has(peerName)) {
+        if (peerName === "openclaw" || pins.has(peerName) || !isSafePackageName(peerName)) {
           continue;
         }
         const installedVersion = await readPackageVersion(
@@ -363,13 +449,69 @@ async function collectManagedNpmRootPeerDependencyPins(params: {
         pins.set(peerName, installedVersion ?? peerRange);
       }
     }
-    queue.push(...(await listNodeModulesPackageDirs(path.join(packageDir, "node_modules"))));
+    queue.push(
+      ...(await listNodeModulesPackageDirs(path.join(packageDir, "node_modules"))).map(
+        (nestedPackageDir) => ({
+          depth: current.depth + 1,
+          packageDir: nestedPackageDir,
+        }),
+      ),
+    );
   }
   return Object.fromEntries(
     [...pins.entries()].toSorted(([left], [right]) => left.localeCompare(right)),
   );
 }
 
+export async function readManagedNpmRootPeerDependencySnapshot(params: {
+  npmRoot: string;
+}): Promise<ManagedNpmRootPeerDependencySnapshot> {
+  const manifest = await readManagedNpmRootManifest(path.join(params.npmRoot, "package.json"));
+  const dependencies = readDependencyRecord(manifest.dependencies);
+  const managedPeerDependencies = readManagedPeerDependencyKeys(manifest.openclaw).toSorted();
+  const dependencySnapshot: Record<string, string> = {};
+  for (const packageName of managedPeerDependencies) {
+    const dependencySpec = dependencies[packageName];
+    if (dependencySpec) {
+      dependencySnapshot[packageName] = dependencySpec;
+    }
+  }
+  return {
+    dependencies: dependencySnapshot,
+    managedPeerDependencies,
+  };
+}
+
+export async function restoreManagedNpmRootPeerDependencySnapshot(params: {
+  npmRoot: string;
+  snapshot: ManagedNpmRootPeerDependencySnapshot;
+}): Promise<void> {
+  const manifestPath = path.join(params.npmRoot, "package.json");
+  const manifest = await readManagedNpmRootManifest(manifestPath);
+  const dependencies = readDependencyRecord(manifest.dependencies);
+  for (const packageName of readManagedPeerDependencyKeys(manifest.openclaw)) {
+    delete dependencies[packageName];
+  }
+  Object.assign(dependencies, params.snapshot.dependencies);
+  const managedOverrideKeys = readManagedOverrideKeys(manifest.openclaw).toSorted();
+  const openclawMetadata = buildManagedOpenClawMetadata({
+    current: manifest.openclaw,
+    managedOverrideKeys,
+    managedPeerDependencyKeys: params.snapshot.managedPeerDependencies.toSorted(),
+  });
+  const next: ManagedNpmRootManifest = {
+    ...manifest,
+    private: true,
+    dependencies,
+  };
+  if (openclawMetadata) {
+    next.openclaw = openclawMetadata;
+  } else {
+    delete next.openclaw;
+  }
+  await writeJson(manifestPath, next, { trailingNewline: true });
+}
+
 export async function syncManagedNpmRootPeerDependencies(params: {
   npmRoot: string;
   managedOverrides?: Record<string, unknown>;
diff --git a/src/plugins/install.npm-spec.e2e.test.ts b/src/plugins/install.npm-spec.e2e.test.ts
@@ -540,6 +540,146 @@ describe("installPluginFromNpmSpec e2e", () => {
     ).resolves.toBeTruthy();
   });
 
+  it("bounds peer dependency discovery across repeated nested package realpaths", async () => {
+    const rootDir = await makeTempDir("npm-plugin-peer-cycle-e2e");
+    const npmRoot = path.join(rootDir, "managed-npm");
+    const existingPlugin = `existing-plugin-${crypto.randomUUID().replace(/-/g, "").slice(0, 12)}`;
+    const laterPlugin = `later-plugin-${crypto.randomUUID().replace(/-/g, "").slice(0, 12)}`;
+    const registry = await startStaticRegistry([
+      {
+        packageName: existingPlugin,
+        latest: "1.0.0",
+        versions: [
+          await packPlugin({
+            packageName: existingPlugin,
+            pluginId: existingPlugin,
+            version: "1.0.0",
+            rootDir,
+          }),
+        ],
+      },
+      {
+        packageName: laterPlugin,
+        latest: "1.0.0",
+        versions: [
+          await packPlugin({
+            packageName: laterPlugin,
+            pluginId: laterPlugin,
+            version: "1.0.0",
+            rootDir,
+          }),
+        ],
+      },
+    ]);
+    process.env.NPM_CONFIG_REGISTRY = registry;
+    process.env.npm_config_registry = registry;
+
+    await fs.mkdir(npmRoot, { recursive: true });
+    await fs.writeFile(
+      path.join(npmRoot, "package.json"),
+      `${JSON.stringify(
+        {
+          private: true,
+          dependencies: { [existingPlugin]: "1.0.0" },
+        },
+        null,
+        2,
+      )}\n`,
+      "utf8",
+    );
+    await execFileAsync(
+      "npm",
+      [
+        "install",
+        "--omit=dev",
+        "--omit=peer",
+        "--legacy-peer-deps",
+        "--loglevel=error",
+        "--ignore-scripts",
+        "--no-audit",
+        "--no-fund",
+      ],
+      { cwd: npmRoot },
+    );
+    const existingPluginDir = path.join(npmRoot, "node_modules", existingPlugin);
+    await fs.mkdir(path.join(existingPluginDir, "node_modules"), { recursive: true });
+    await fs.symlink(existingPluginDir, path.join(existingPluginDir, "node_modules", "self"));
+
+    const later = await installPluginFromNpmSpec({
+      spec: `${laterPlugin}@1.0.0`,
+      npmDir: npmRoot,
+      logger: { info: () => {}, warn: () => {} },
+      timeoutMs: 120_000,
+    });
+
+    expect(later.ok).toBe(true);
+    await expect(
+      fs.lstat(path.join(npmRoot, "node_modules", laterPlugin, "package.json")),
+    ).resolves.toBeTruthy();
+  });
+
+  it("rolls back managed peer dependencies added before a failed install scan", async () => {
+    const rootDir = await makeTempDir("npm-plugin-peer-rollback-e2e");
+    const npmRoot = path.join(rootDir, "managed-npm");
+    const blockedPlugin = `blocked-plugin-${crypto.randomUUID().replace(/-/g, "").slice(0, 12)}`;
+    const runtimePeer = `runtime-peer-${crypto.randomUUID().replace(/-/g, "").slice(0, 12)}`;
+    const registry = await startStaticRegistry([
+      {
+        packageName: blockedPlugin,
+        latest: "1.0.0",
+        versions: [
+          await packPlugin({
+            packageName: blockedPlugin,
+            peerDependencies: { [runtimePeer]: "^1.0.0" },
+            peerDependenciesMeta: {},
+            pluginId: blockedPlugin,
+            version: "1.0.0",
+            rootDir,
+          }),
+        ],
+      },
+      {
+        packageName: runtimePeer,
+        latest: "1.0.0",
+        versions: [
+          await packPlugin({
+            indexJs: "eval('1');\n",
+            packageName: runtimePeer,
+            pluginId: runtimePeer,
+            version: "1.0.0",
+            rootDir,
+          }),
+        ],
+      },
+    ]);
+    process.env.NPM_CONFIG_REGISTRY = registry;
+    process.env.npm_config_registry = registry;
+
+    const result = await installPluginFromNpmSpec({
+      spec: `${blockedPlugin}@1.0.0`,
+      npmDir: npmRoot,
+      logger: { info: () => {}, warn: () => {} },
+      timeoutMs: 120_000,
+    });
+
+    expect(result.ok).toBe(false);
+    const rootManifest = JSON.parse(
+      await fs.readFile(path.join(npmRoot, "package.json"), "utf8"),
+    ) as {
+      dependencies?: Record<string, string>;
+      openclaw?: { managedPeerDependencies?: string[] };
+    };
+    expect(rootManifest.dependencies?.[blockedPlugin]).toBeUndefined();
+    expect(rootManifest.dependencies?.[runtimePeer]).toBeUndefined();
+    expect(rootManifest.openclaw?.managedPeerDependencies ?? []).not.toContain(runtimePeer);
+    await expect(
+      fs.lstat(path.join(npmRoot, "node_modules", blockedPlugin, "package.json")),
+    ).rejects.toHaveProperty("code", "ENOENT");
+    await expect(
+      fs.lstat(path.join(npmRoot, "node_modules", runtimePeer, "package.json")),
+    ).rejects.toHaveProperty("code", "ENOENT");
+  });
+
   it("scrubs host peers when installing beside an existing host-peer plugin", async () => {
     const rootDir = await makeTempDir("npm-plugin-sibling-peer-e2e");
     const npmRoot = path.join(rootDir, "managed-npm");
diff --git a/src/plugins/install.ts b/src/plugins/install.ts
