diff --git a/CHANGELOG.md b/CHANGELOG.md index 1063db07596..f0cf4a42a2c 100644 --- 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 index b09c25fbc08..a1331159f22 100644 --- 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 { 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 { +async function collectPackageManifestPaths(params: { + allowManagedNpmRootPackagePeerSymlinks?: boolean; + rootDir: string; +}): Promise { 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 { - 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,6 +894,7 @@ export async function scanPackageInstallSourceRuntime( } export async function scanInstalledPackageDependencyTreeRuntime(params: { + allowManagedNpmRootPackagePeerSymlinks?: boolean; logger: InstallScanLogger; packageDir: string; pluginId: string; @@ -864,6 +902,7 @@ export async function scanInstalledPackageDependencyTreeRuntime(params: { 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 index 5e15c3894cb..0c8e4689cd2 100644 --- 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 index b312d6a40ea..7347d4424c8 100644 --- 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; }) { 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; }) { + 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; + }>, +) { + 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 index c50ac57a801..a8fb30d1144 100644 --- 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,