fix(plugins): trust managed npm peer links

This commit is contained in:
Vincent Koc
2026-05-03 01:45:21 -07:00
parent 9ef35ea5c7
commit 5ecd01ff94
5 changed files with 143 additions and 14 deletions

View File

@@ -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.

View File

@@ -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,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`,
});
}

View File

@@ -73,6 +73,7 @@ export async function scanPackageInstallSource(
}
export async function scanInstalledPackageDependencyTree(params: {
allowManagedNpmRootPackagePeerSymlinks?: boolean;
logger: InstallScanLogger;
packageDir: string;
pluginId: string;

View File

@@ -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[] = [];

View File

@@ -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,