mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-03 03:03:24 -04:00
fix(security): harden npm plugin and hook install integrity flow
This commit is contained in:
@@ -1,8 +1,8 @@
|
||||
import JSZip from "jszip";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import JSZip from "jszip";
|
||||
import * as tar from "tar";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import * as skillScanner from "../security/skill-scanner.js";
|
||||
@@ -491,7 +491,16 @@ describe("installPluginFromNpmSpec", () => {
|
||||
await packToArchive({ pkgDir, outDir: packTmpDir, outName: packedName });
|
||||
return {
|
||||
code: 0,
|
||||
stdout: `${packedName}\n`,
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
id: "@openclaw/voice-call@0.0.1",
|
||||
name: "@openclaw/voice-call",
|
||||
version: "0.0.1",
|
||||
filename: packedName,
|
||||
integrity: "sha512-plugin-test",
|
||||
shasum: "pluginshasum",
|
||||
},
|
||||
]),
|
||||
stderr: "",
|
||||
signal: null,
|
||||
killed: false,
|
||||
@@ -508,6 +517,11 @@ describe("installPluginFromNpmSpec", () => {
|
||||
logger: { info: () => {}, warn: () => {} },
|
||||
});
|
||||
expect(result.ok).toBe(true);
|
||||
if (!result.ok) {
|
||||
return;
|
||||
}
|
||||
expect(result.npmResolution?.resolvedSpec).toBe("@openclaw/voice-call@0.0.1");
|
||||
expect(result.npmResolution?.integrity).toBe("sha512-plugin-test");
|
||||
|
||||
expectSingleNpmPackIgnoreScriptsCall({
|
||||
calls: run.mock.calls,
|
||||
@@ -527,4 +541,46 @@ describe("installPluginFromNpmSpec", () => {
|
||||
}
|
||||
expect(result.error).toContain("unsupported npm spec");
|
||||
});
|
||||
|
||||
it("aborts when integrity drift callback rejects the fetched artifact", async () => {
|
||||
const { runCommandWithTimeout } = await import("../process/exec.js");
|
||||
const run = vi.mocked(runCommandWithTimeout);
|
||||
run.mockResolvedValue({
|
||||
code: 0,
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
id: "@openclaw/voice-call@0.0.1",
|
||||
name: "@openclaw/voice-call",
|
||||
version: "0.0.1",
|
||||
filename: "voice-call-0.0.1.tgz",
|
||||
integrity: "sha512-new",
|
||||
shasum: "newshasum",
|
||||
},
|
||||
]),
|
||||
stderr: "",
|
||||
signal: null,
|
||||
killed: false,
|
||||
termination: "exit",
|
||||
});
|
||||
|
||||
const onIntegrityDrift = vi.fn(async () => false);
|
||||
const { installPluginFromNpmSpec } = await import("./install.js");
|
||||
const result = await installPluginFromNpmSpec({
|
||||
spec: "@openclaw/voice-call@0.0.1",
|
||||
expectedIntegrity: "sha512-old",
|
||||
onIntegrityDrift,
|
||||
});
|
||||
|
||||
expect(onIntegrityDrift).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
expectedIntegrity: "sha512-old",
|
||||
actualIntegrity: "sha512-new",
|
||||
}),
|
||||
);
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) {
|
||||
return;
|
||||
}
|
||||
expect(result.error).toContain("integrity drift");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -15,6 +15,8 @@ import {
|
||||
unscopedPackageName,
|
||||
} from "../infra/install-safe-path.js";
|
||||
import {
|
||||
type NpmIntegrityDrift,
|
||||
type NpmSpecResolution,
|
||||
packNpmSpecToArchive,
|
||||
resolveArchiveSourcePath,
|
||||
withTempDir,
|
||||
@@ -43,9 +45,18 @@ export type InstallPluginResult =
|
||||
manifestName?: string;
|
||||
version?: string;
|
||||
extensions: string[];
|
||||
npmResolution?: NpmSpecResolution;
|
||||
integrityDrift?: NpmIntegrityDrift;
|
||||
}
|
||||
| { ok: false; error: string };
|
||||
|
||||
export type PluginNpmIntegrityDriftParams = {
|
||||
spec: string;
|
||||
expectedIntegrity: string;
|
||||
actualIntegrity: string;
|
||||
resolution: NpmSpecResolution;
|
||||
};
|
||||
|
||||
const defaultLogger: PluginInstallLogger = {};
|
||||
function safeFileName(input: string): string {
|
||||
return safeDirName(input);
|
||||
@@ -420,6 +431,8 @@ export async function installPluginFromNpmSpec(params: {
|
||||
mode?: "install" | "update";
|
||||
dryRun?: boolean;
|
||||
expectedPluginId?: string;
|
||||
expectedIntegrity?: string;
|
||||
onIntegrityDrift?: (params: PluginNpmIntegrityDriftParams) => boolean | Promise<boolean>;
|
||||
}): Promise<InstallPluginResult> {
|
||||
const { logger, timeoutMs, mode, dryRun } = resolveTimedPluginInstallModeOptions(params);
|
||||
const expectedPluginId = params.expectedPluginId;
|
||||
@@ -440,7 +453,44 @@ export async function installPluginFromNpmSpec(params: {
|
||||
return packedResult;
|
||||
}
|
||||
|
||||
return await installPluginFromArchive({
|
||||
const npmResolution: NpmSpecResolution = {
|
||||
...packedResult.metadata,
|
||||
resolvedAt: new Date().toISOString(),
|
||||
};
|
||||
|
||||
let integrityDrift: NpmIntegrityDrift | undefined;
|
||||
if (
|
||||
params.expectedIntegrity &&
|
||||
npmResolution.integrity &&
|
||||
params.expectedIntegrity !== npmResolution.integrity
|
||||
) {
|
||||
integrityDrift = {
|
||||
expectedIntegrity: params.expectedIntegrity,
|
||||
actualIntegrity: npmResolution.integrity,
|
||||
};
|
||||
const driftPayload: PluginNpmIntegrityDriftParams = {
|
||||
spec,
|
||||
expectedIntegrity: integrityDrift.expectedIntegrity,
|
||||
actualIntegrity: integrityDrift.actualIntegrity,
|
||||
resolution: npmResolution,
|
||||
};
|
||||
let proceed = true;
|
||||
if (params.onIntegrityDrift) {
|
||||
proceed = await params.onIntegrityDrift(driftPayload);
|
||||
} else {
|
||||
logger.warn?.(
|
||||
`Integrity drift detected for ${driftPayload.resolution.resolvedSpec ?? driftPayload.spec}: expected ${driftPayload.expectedIntegrity}, got ${driftPayload.actualIntegrity}`,
|
||||
);
|
||||
}
|
||||
if (!proceed) {
|
||||
return {
|
||||
ok: false,
|
||||
error: `aborted: npm package integrity drift detected for ${driftPayload.resolution.resolvedSpec ?? driftPayload.spec}`,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
const installResult = await installPluginFromArchive({
|
||||
archivePath: packedResult.archivePath,
|
||||
extensionsDir: params.extensionsDir,
|
||||
timeoutMs,
|
||||
@@ -449,6 +499,15 @@ export async function installPluginFromNpmSpec(params: {
|
||||
dryRun,
|
||||
expectedPluginId,
|
||||
});
|
||||
if (!installResult.ok) {
|
||||
return installResult;
|
||||
}
|
||||
|
||||
return {
|
||||
...installResult,
|
||||
npmResolution,
|
||||
integrityDrift,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -29,6 +29,16 @@ export type PluginUpdateSummary = {
|
||||
outcomes: PluginUpdateOutcome[];
|
||||
};
|
||||
|
||||
export type PluginUpdateIntegrityDriftParams = {
|
||||
pluginId: string;
|
||||
spec: string;
|
||||
expectedIntegrity: string;
|
||||
actualIntegrity: string;
|
||||
resolvedSpec?: string;
|
||||
resolvedVersion?: string;
|
||||
dryRun: boolean;
|
||||
};
|
||||
|
||||
export type PluginChannelSyncSummary = {
|
||||
switchedToBundled: string[];
|
||||
switchedToNpm: string[];
|
||||
@@ -143,6 +153,7 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
pluginIds?: string[];
|
||||
skipIds?: Set<string>;
|
||||
dryRun?: boolean;
|
||||
onIntegrityDrift?: (params: PluginUpdateIntegrityDriftParams) => boolean | Promise<boolean>;
|
||||
}): Promise<PluginUpdateSummary> {
|
||||
const logger = params.logger ?? {};
|
||||
const installs = params.config.plugins?.installs ?? {};
|
||||
@@ -210,6 +221,25 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
mode: "update",
|
||||
dryRun: true,
|
||||
expectedPluginId: pluginId,
|
||||
expectedIntegrity: record.integrity,
|
||||
onIntegrityDrift: async (drift) => {
|
||||
const payload: PluginUpdateIntegrityDriftParams = {
|
||||
pluginId,
|
||||
spec: drift.spec,
|
||||
expectedIntegrity: drift.expectedIntegrity,
|
||||
actualIntegrity: drift.actualIntegrity,
|
||||
resolvedSpec: drift.resolution.resolvedSpec,
|
||||
resolvedVersion: drift.resolution.version,
|
||||
dryRun: true,
|
||||
};
|
||||
if (params.onIntegrityDrift) {
|
||||
return await params.onIntegrityDrift(payload);
|
||||
}
|
||||
logger.warn?.(
|
||||
`Integrity drift for "${pluginId}" (${payload.resolvedSpec ?? payload.spec}): expected ${payload.expectedIntegrity}, got ${payload.actualIntegrity}`,
|
||||
);
|
||||
return true;
|
||||
},
|
||||
logger,
|
||||
});
|
||||
} catch (err) {
|
||||
@@ -257,6 +287,25 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
spec: record.spec,
|
||||
mode: "update",
|
||||
expectedPluginId: pluginId,
|
||||
expectedIntegrity: record.integrity,
|
||||
onIntegrityDrift: async (drift) => {
|
||||
const payload: PluginUpdateIntegrityDriftParams = {
|
||||
pluginId,
|
||||
spec: drift.spec,
|
||||
expectedIntegrity: drift.expectedIntegrity,
|
||||
actualIntegrity: drift.actualIntegrity,
|
||||
resolvedSpec: drift.resolution.resolvedSpec,
|
||||
resolvedVersion: drift.resolution.version,
|
||||
dryRun: false,
|
||||
};
|
||||
if (params.onIntegrityDrift) {
|
||||
return await params.onIntegrityDrift(payload);
|
||||
}
|
||||
logger.warn?.(
|
||||
`Integrity drift for "${pluginId}" (${payload.resolvedSpec ?? payload.spec}): expected ${payload.expectedIntegrity}, got ${payload.actualIntegrity}`,
|
||||
);
|
||||
return true;
|
||||
},
|
||||
logger,
|
||||
});
|
||||
} catch (err) {
|
||||
@@ -283,6 +332,12 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
spec: record.spec,
|
||||
installPath: result.targetDir,
|
||||
version: nextVersion,
|
||||
resolvedName: result.npmResolution?.name,
|
||||
resolvedVersion: result.npmResolution?.version,
|
||||
resolvedSpec: result.npmResolution?.resolvedSpec,
|
||||
integrity: result.npmResolution?.integrity,
|
||||
shasum: result.npmResolution?.shasum,
|
||||
resolvedAt: result.npmResolution?.resolvedAt,
|
||||
});
|
||||
changed = true;
|
||||
|
||||
@@ -406,6 +461,12 @@ export async function syncPluginsForUpdateChannel(params: {
|
||||
spec,
|
||||
installPath: result.targetDir,
|
||||
version: result.version,
|
||||
resolvedName: result.npmResolution?.name,
|
||||
resolvedVersion: result.npmResolution?.version,
|
||||
resolvedSpec: result.npmResolution?.resolvedSpec,
|
||||
integrity: result.npmResolution?.integrity,
|
||||
shasum: result.npmResolution?.shasum,
|
||||
resolvedAt: result.npmResolution?.resolvedAt,
|
||||
sourcePath: undefined,
|
||||
});
|
||||
summary.switchedToNpm.push(pluginId);
|
||||
|
||||
Reference in New Issue
Block a user