mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:50:43 +00:00
fix(plugins): pin npm plugin installs
This commit is contained in:
committed by
Peter Steinberger
parent
97cdd73aa6
commit
fb3ee066d8
@@ -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(
|
||||
|
||||
@@ -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<string, unknown> {
|
||||
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<string, string> {
|
||||
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<ManagedNpmRootInstalledDependency | null> {
|
||||
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;
|
||||
|
||||
@@ -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<string, string>;
|
||||
expectedDependencySpec?: string;
|
||||
installedVersion?: string;
|
||||
installedIntegrity?: string;
|
||||
};
|
||||
|
||||
function writeNpmRootPackageLock(params: {
|
||||
npmRoot: string;
|
||||
dependencies: Record<string, string>;
|
||||
packages: MockNpmPackage[];
|
||||
}) {
|
||||
const lockPackages: Record<string, unknown> = {
|
||||
"": {
|
||||
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<string, string>;
|
||||
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<string, string>;
|
||||
}>,
|
||||
) {
|
||||
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<string, string>;
|
||||
};
|
||||
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");
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user