From fb3ee066d8cb335718cd0747568bf54773390642 Mon Sep 17 00:00:00 2001 From: Lucenx9 <185146821+Lucenx9@users.noreply.github.com> Date: Sun, 3 May 2026 14:29:56 +0200 Subject: [PATCH] fix(plugins): pin npm plugin installs --- src/infra/npm-managed-root.test.ts | 37 +++++++- src/infra/npm-managed-root.ts | 32 ++++++- src/plugins/install.npm-spec.test.ts | 136 +++++++++++++++++++++++---- src/plugins/install.ts | 57 +++++++++++ 4 files changed, 243 insertions(+), 19 deletions(-) diff --git a/src/infra/npm-managed-root.test.ts b/src/infra/npm-managed-root.test.ts index e49f1dece62..f7a8278bdc6 100644 --- a/src/infra/npm-managed-root.test.ts +++ b/src/infra/npm-managed-root.test.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { removeManagedNpmRootDependency, + readManagedNpmRootInstalledDependency, resolveManagedNpmRootDependencySpec, upsertManagedNpmRootDependency, } from "./npm-managed-root.js"; @@ -60,7 +61,7 @@ describe("managed npm root", () => { }); }); - it("uses the requested selector before falling back to resolved version", () => { + it("pins managed dependencies to the resolved version", () => { expect( resolveManagedNpmRootDependencySpec({ parsedSpec: { @@ -77,7 +78,7 @@ describe("managed npm root", () => { resolvedAt: "2026-05-03T00:00:00.000Z", }, }), - ).toBe("stable"); + ).toBe("2026.5.2"); expect( resolveManagedNpmRootDependencySpec({ @@ -97,6 +98,38 @@ describe("managed npm root", () => { ).toBe("2026.5.2"); }); + it("reads installed dependency metadata from package-lock", async () => { + const npmRoot = await makeTempRoot(); + await fs.writeFile( + path.join(npmRoot, "package-lock.json"), + `${JSON.stringify( + { + lockfileVersion: 3, + packages: { + "node_modules/@openclaw/discord": { + version: "2026.5.2", + resolved: "https://registry.npmjs.org/@openclaw/discord/-/discord-2026.5.2.tgz", + integrity: "sha512-discord", + }, + }, + }, + null, + 2, + )}\n`, + ); + + await expect( + readManagedNpmRootInstalledDependency({ + npmRoot, + packageName: "@openclaw/discord", + }), + ).resolves.toEqual({ + version: "2026.5.2", + resolved: "https://registry.npmjs.org/@openclaw/discord/-/discord-2026.5.2.tgz", + integrity: "sha512-discord", + }); + }); + it("removes one managed dependency without dropping unrelated metadata", async () => { const npmRoot = await makeTempRoot(); await fs.writeFile( diff --git a/src/infra/npm-managed-root.ts b/src/infra/npm-managed-root.ts index a9181e91cce..74adc2943e0 100644 --- a/src/infra/npm-managed-root.ts +++ b/src/infra/npm-managed-root.ts @@ -9,10 +9,20 @@ type ManagedNpmRootManifest = { [key: string]: unknown; }; +export type ManagedNpmRootInstalledDependency = { + version?: string; + integrity?: string; + resolved?: string; +}; + function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } +function readOptionalString(value: unknown): string | undefined { + return typeof value === "string" && value.trim() ? value.trim() : undefined; +} + function readDependencyRecord(value: unknown): Record { if (!isRecord(value)) { return {}; @@ -42,7 +52,7 @@ export function resolveManagedNpmRootDependencySpec(params: { parsedSpec: ParsedRegistryNpmSpec; resolution: NpmSpecResolution; }): string { - return params.parsedSpec.selector ?? params.resolution.version ?? "latest"; + return params.resolution.version ?? params.parsedSpec.selector ?? "latest"; } export async function upsertManagedNpmRootDependency(params: { @@ -65,6 +75,26 @@ export async function upsertManagedNpmRootDependency(params: { await fs.writeFile(manifestPath, `${JSON.stringify(next, null, 2)}\n`, "utf8"); } +export async function readManagedNpmRootInstalledDependency(params: { + npmRoot: string; + packageName: string; +}): Promise { + const lockPath = path.join(params.npmRoot, "package-lock.json"); + const parsed = JSON.parse(await fs.readFile(lockPath, "utf8")) as unknown; + if (!isRecord(parsed) || !isRecord(parsed.packages)) { + return null; + } + const entry = parsed.packages[`node_modules/${params.packageName}`]; + if (!isRecord(entry)) { + return null; + } + return { + version: readOptionalString(entry.version), + integrity: readOptionalString(entry.integrity), + resolved: readOptionalString(entry.resolved), + }; +} + export async function removeManagedNpmRootDependency(params: { npmRoot: string; packageName: string; diff --git a/src/plugins/install.npm-spec.test.ts b/src/plugins/install.npm-spec.test.ts index 9151c5ed80d..2cd5f2ef952 100644 --- a/src/plugins/install.npm-spec.test.ts +++ b/src/plugins/install.npm-spec.test.ts @@ -118,6 +118,46 @@ function writeInstalledNpmPlugin(params: { return pluginDir; } +type MockNpmPackage = { + 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; + expectedDependencySpec?: string; + installedVersion?: string; + installedIntegrity?: string; +}; + +function writeNpmRootPackageLock(params: { + npmRoot: string; + dependencies: Record; + packages: MockNpmPackage[]; +}) { + const lockPackages: Record = { + "": { + dependencies: params.dependencies, + }, + }; + for (const pkg of params.packages) { + lockPackages[`node_modules/${pkg.packageName}`] = { + version: pkg.installedVersion ?? pkg.version, + integrity: pkg.installedIntegrity ?? pkg.integrity ?? "sha512-plugin-test", + }; + } + fs.writeFileSync( + path.join(params.npmRoot, "package-lock.json"), + `${JSON.stringify({ lockfileVersion: 3, packages: lockPackages }, null, 2)}\n`, + "utf-8", + ); +} + function mockNpmViewAndInstall(params: { spec: string; packageName: string; @@ -130,25 +170,14 @@ function mockNpmViewAndInstall(params: { dependency?: { name: string; version: string }; hoistedDependency?: { name: string; version: string }; peerDependencies?: Record; + expectedDependencySpec?: string; + installedVersion?: string; + installedIntegrity?: 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; - }>, -) { +function mockNpmViewAndInstallMany(packages: MockNpmPackage[]) { const packagesByName = new Map(packages.map((pkg) => [pkg.packageName, pkg])); runCommandWithTimeoutMock.mockImplementation(async (argv: string[]) => { const viewPackage = packages.find( @@ -175,13 +204,29 @@ function mockNpmViewAndInstallMany( const manifest = JSON.parse(fs.readFileSync(path.join(npmRoot, "package.json"), "utf8")) as { dependencies?: Record; }; + const installedPackages: MockNpmPackage[] = []; for (const packageName of Object.keys(manifest.dependencies ?? {})) { const pkg = packagesByName.get(packageName); if (!pkg) { throw new Error(`unexpected managed npm dependency: ${packageName}`); } - writeInstalledNpmPlugin(pkg); + const dependencySpec = manifest.dependencies?.[packageName]; + if (pkg.expectedDependencySpec && dependencySpec !== pkg.expectedDependencySpec) { + throw new Error( + `expected managed npm dependency ${packageName}@${pkg.expectedDependencySpec}, got ${dependencySpec ?? ""}`, + ); + } + writeInstalledNpmPlugin({ + ...pkg, + version: pkg.installedVersion ?? pkg.version, + }); + installedPackages.push(pkg); } + writeNpmRootPackageLock({ + npmRoot, + dependencies: manifest.dependencies ?? {}, + packages: installedPackages, + }); return successfulSpawn(); } if (argv[0] === "npm" && argv[1] === "uninstall") { @@ -246,6 +291,65 @@ describe("installPluginFromNpmSpec", () => { }); }); + it("pins mutable npm specs to the verified resolved version", async () => { + const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm"); + mockNpmViewAndInstall({ + spec: "mutable-plugin@latest", + packageName: "mutable-plugin", + version: "1.2.3", + pluginId: "mutable-plugin", + npmRoot, + expectedDependencySpec: "1.2.3", + }); + + const result = await installPluginFromNpmSpec({ + spec: "mutable-plugin@latest", + npmDir: npmRoot, + logger: { info: () => {}, warn: () => {} }, + }); + + expect(result.ok).toBe(true); + await expect( + fs.promises + .readFile(path.join(npmRoot, "package.json"), "utf8") + .then((raw) => JSON.parse(raw)), + ).resolves.toMatchObject({ + dependencies: { + "mutable-plugin": "1.2.3", + }, + }); + }); + + it("rejects npm installs when the installed artifact drifts from verified metadata", async () => { + const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm"); + mockNpmViewAndInstall({ + spec: "drift-plugin@latest", + packageName: "drift-plugin", + version: "1.0.0", + pluginId: "drift-plugin", + integrity: "sha512-safe", + installedVersion: "1.0.0", + installedIntegrity: "sha512-evil", + npmRoot, + expectedDependencySpec: "1.0.0", + }); + + const result = await installPluginFromNpmSpec({ + spec: "drift-plugin@latest", + expectedIntegrity: "sha512-safe", + npmDir: npmRoot, + logger: { info: () => {}, warn: () => {} }, + }); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + expect(result.error).toContain("integrity sha512-evil"); + expect(result.error).toContain("expected sha512-safe"); + expect(fs.existsSync(path.join(npmRoot, "node_modules", "drift-plugin"))).toBe(false); + }); + it("rejects npm installs with blocked hoisted transitive dependencies", async () => { const stateDir = suiteTempRootTracker.makeTempDir(); const npmRoot = path.join(stateDir, "npm"); diff --git a/src/plugins/install.ts b/src/plugins/install.ts index 2b2bf71d6a6..2d28827578d 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -8,9 +8,11 @@ import { } from "../infra/install-source-utils.js"; import { resolveNpmIntegrityDriftWithDefaultMessage } from "../infra/npm-integrity.js"; import { + readManagedNpmRootInstalledDependency, removeManagedNpmRootDependency, resolveManagedNpmRootDependencySpec, upsertManagedNpmRootDependency, + type ManagedNpmRootInstalledDependency, } from "../infra/npm-managed-root.js"; import { formatPrereleaseResolutionError, @@ -253,6 +255,23 @@ async function rollbackManagedNpmPluginInstall(params: { } } +function resolveInstalledNpmResolutionMismatch(params: { + packageName: string; + expected: NpmSpecResolution; + installed: ManagedNpmRootInstalledDependency | null; +}): string | null { + if (!params.installed) { + return `npm install did not record package-lock metadata for ${params.packageName}`; + } + if (params.expected.version && params.installed.version !== params.expected.version) { + return `npm install resolved ${params.packageName} to version ${params.installed.version ?? "unknown"}, expected ${params.expected.version}`; + } + if (params.expected.integrity && params.installed.integrity !== params.expected.integrity) { + return `npm install resolved ${params.packageName} with integrity ${params.installed.integrity ?? "unknown"}, expected ${params.expected.integrity}`; + } + return null; +} + type PackageInstallCommonParams = InstallSafetyOverrides & { extensionsDir?: string; npmDir?: string; @@ -1242,6 +1261,44 @@ export async function installPluginFromNpmSpec( }; } + let installedDependency: ManagedNpmRootInstalledDependency | null; + try { + installedDependency = await readManagedNpmRootInstalledDependency({ + npmRoot, + packageName: parsedSpec.name, + }); + } catch (error) { + await rollbackManagedNpmPluginInstall({ + npmRoot, + packageName: parsedSpec.name, + targetDir: installRoot, + timeoutMs, + logger, + }); + return { + ok: false, + error: `Failed to verify npm install metadata for ${parsedSpec.name}: ${String(error)}`, + }; + } + const resolutionMismatch = resolveInstalledNpmResolutionMismatch({ + packageName: parsedSpec.name, + expected: npmResolution, + installed: installedDependency, + }); + if (resolutionMismatch) { + await rollbackManagedNpmPluginInstall({ + npmRoot, + packageName: parsedSpec.name, + targetDir: installRoot, + timeoutMs, + logger, + }); + return { + ok: false, + error: resolutionMismatch, + }; + } + const result = await installPluginFromInstalledPackageDir({ dangerouslyForceUnsafeInstall: params.dangerouslyForceUnsafeInstall, packageDir: installRoot,