refactor: simplify plugin dependency loading

This commit is contained in:
Peter Steinberger
2026-05-01 21:56:30 +01:00
parent 112dedd093
commit 257a3c068d
5 changed files with 31 additions and 47 deletions

View File

@@ -511,7 +511,7 @@ Plugins run **in-process** with the Gateway. Treat them as trusted code:
- If you install or update plugins (`openclaw plugins install <package>`, `openclaw plugins update <id>`), treat it like running untrusted code:
- The install path is the per-plugin directory under the active plugin install root.
- OpenClaw runs a built-in dangerous-code scan before install/update. `critical` findings block by default.
- OpenClaw uses `npm pack`, then runs a project-local `npm install --omit=dev --ignore-scripts` in that directory. Inherited global npm install settings are ignored so dependencies stay under the plugin install path.
- npm and git plugin installs run package-manager dependency convergence only during the explicit install/update flow. Local paths and archives are treated as self-contained plugin packages; OpenClaw copies/references them without running `npm install`.
- Prefer pinned, exact versions (`@scope/pkg@1.2.3`), and inspect the unpacked code on disk before enabling.
- `--dangerously-force-unsafe-install` is break-glass only for built-in scan false positives on plugin install/update flows. It does not bypass plugin `before_install` hook policy blocks and does not bypass scan failures.
- Gateway-backed skill dependency installs follow the same dangerous/suspicious split: built-in `critical` findings block unless the caller explicitly sets `dangerouslyForceUnsafeInstall`, while suspicious findings still warn only. `openclaw skills install` remains the separate ClawHub skill download/install flow.

View File

@@ -618,7 +618,7 @@ beforeEach(() => {
});
describe("installPluginFromArchive", () => {
it("runs npm for package archive runtime dependencies", async () => {
it("does not run npm for package archive runtime dependencies", async () => {
const result = await installArchivePackageAndReturnResult({
packageJson: {
name: "archive-with-deps",
@@ -631,14 +631,7 @@ describe("installPluginFromArchive", () => {
});
expect(result.ok).toBe(true);
expect(vi.mocked(runCommandWithTimeout)).toHaveBeenCalledTimes(1);
expect(vi.mocked(runCommandWithTimeout).mock.calls[0]?.[0]).toEqual([
"npm",
"install",
"--omit=dev",
"--loglevel=error",
"--ignore-scripts",
]);
expect(vi.mocked(runCommandWithTimeout)).not.toHaveBeenCalled();
});
it("installs scoped archives, rejects duplicate installs, and allows updates", async () => {

View File

@@ -201,7 +201,6 @@ type PackageInstallCommonParams = InstallSafetyOverrides & {
dryRun?: boolean;
expectedPluginId?: string;
requirePluginManifest?: boolean;
installDependencies?: boolean;
installPolicyRequest?: PluginInstallPolicyRequest;
};
@@ -228,7 +227,6 @@ function pickPackageInstallCommonParams(
dryRun: params.dryRun,
expectedPluginId: params.expectedPluginId,
requirePluginManifest: params.requirePluginManifest,
installDependencies: params.installDependencies,
installPolicyRequest: params.installPolicyRequest,
};
}
@@ -607,13 +605,6 @@ type ValidatedPackagePlugin = {
peerDependencies: Record<string, string>;
};
function hasInstallablePackageDependencies(manifest: PackageManifest): boolean {
return (
Object.keys(manifest.dependencies ?? {}).length > 0 ||
Object.keys(manifest.optionalDependencies ?? {}).length > 0
);
}
async function validatePackagePluginInstallSource(params: {
runtime: Awaited<ReturnType<typeof loadPluginInstallRuntime>>;
packageDir: string;
@@ -902,9 +893,8 @@ async function installPluginFromPackageDir(
mode: preparedTarget.effectiveMode,
dryRun,
copyErrorPrefix: "failed to copy plugin",
hasDeps:
params.installDependencies === true && hasInstallablePackageDependencies(plugin.manifest),
depsLogMessage: "Installing plugin dependencies with npm…",
hasDeps: false,
depsLogMessage: "",
nameEncoder: encodePluginInstallDirName,
afterInstall: async (installedDir) => {
return await scanAndLinkInstalledPackage({
@@ -955,7 +945,6 @@ export async function installPluginFromArchive(
dryRun: params.dryRun,
expectedPluginId: params.expectedPluginId,
requirePluginManifest: true,
installDependencies: true,
installPolicyRequest,
}),
}),

View File

@@ -47,8 +47,8 @@ afterEach(() => {
}
});
describe("createPluginJitiLoader", () => {
it("uses the bundled plugin module path as the jiti filename", async () => {
describe("createPluginModuleLoader", () => {
it("loads bundled JavaScript without creating a jiti loader", async () => {
const jitiLoaderCalls: Array<{ modulePath: string; jitiFilename?: string }> = [];
vi.doMock("./jiti-loader-cache.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("./jiti-loader-cache.js")>();
@@ -92,8 +92,6 @@ describe("createPluginJitiLoader", () => {
},
});
const bundledPluginLoad = jitiLoaderCalls.find((call) => call.modulePath.endsWith("index.cjs"));
expect(bundledPluginLoad).toBeDefined();
expect(bundledPluginLoad?.jitiFilename).toBe(bundledPluginLoad?.modulePath);
expect(jitiLoaderCalls).toEqual([]);
});
});

View File

@@ -102,6 +102,7 @@ import {
restoreMemoryPluginState,
} from "./memory-state.js";
import { unwrapDefaultModuleExport } from "./module-export.js";
import { tryNativeRequireJavaScriptModule } from "./native-module-require.js";
import { withProfile } from "./plugin-load-profile.js";
import {
createPluginIdScopeSet,
@@ -455,10 +456,9 @@ function runPluginRegisterSync(
}
}
function createPluginJitiLoader(options: Pick<PluginLoadOptions, "pluginSdkResolution">) {
function createPluginModuleLoader(options: Pick<PluginLoadOptions, "pluginSdkResolution">) {
const jitiLoaders: PluginJitiLoaderCache = new Map();
return (modulePath: string) => {
const tryNative = shouldPreferNativeJiti(modulePath);
const loadWithJiti = (modulePath: string) => {
return getCachedPluginJitiLoader({
cache: jitiLoaders,
modulePath,
@@ -471,13 +471,21 @@ function createPluginJitiLoader(options: Pick<PluginLoadOptions, "pluginSdkResol
options.pluginSdkResolution,
),
pluginSdkResolution: options.pluginSdkResolution,
// Source .ts runtime shims import sibling ".js" specifiers that only exist
// after build. Disable native loading for source entries so Jiti rewrites
// those imports against the source graph, while keeping native dist/*.js
// loading for the canonical built module graph.
tryNative,
tryNative: false,
});
};
return (modulePath: string): unknown => {
if (shouldPreferNativeJiti(modulePath)) {
const native = tryNativeRequireJavaScriptModule(modulePath, { allowWindows: true });
if (native.ok) {
return native.moduleExport;
}
}
// Source .ts runtime shims import sibling ".js" specifiers that only exist
// after build. Jiti remains the dev/source fallback because it rewrites those
// imports against the source graph and applies SDK aliases.
return loadWithJiti(modulePath)(toSafeImportPath(modulePath));
};
}
function resolveCanonicalDistRuntimeSource(source: string): string {
@@ -1241,8 +1249,8 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
clearMemoryPluginState();
}
// Lazy: avoid creating the Jiti loader when all plugins are disabled (common in unit tests).
const getJiti = createPluginJitiLoader(options);
// Lazy: avoid creating module loaders when all plugins are disabled (common in unit tests).
const loadPluginModule = createPluginModuleLoader(options);
let createPluginRuntimeFactory:
| ((options?: CreatePluginRuntimeOptions) => PluginRuntime)
@@ -1259,12 +1267,11 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
if (!runtimeModulePath) {
throw new Error("Unable to resolve plugin runtime module");
}
const safeRuntimePath = toSafeImportPath(runtimeModulePath);
const runtimeModule = withProfile(
{ source: runtimeModulePath },
"runtime-module",
() =>
getJiti(runtimeModulePath)(safeRuntimePath) as {
loadPluginModule(runtimeModulePath) as {
createPluginRuntime?: (options?: CreatePluginRuntimeOptions) => PluginRuntime;
},
);
@@ -1715,7 +1722,6 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
}
const safeSource = opened.path;
fs.closeSync(opened.fd);
const safeImportSource = toSafeImportPath(safeSource);
let mod: OpenClawPluginModule | null = null;
try {
@@ -1727,7 +1733,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
mod = withProfile(
{ pluginId: record.id, source: safeSource },
registrationMode,
() => getJiti(safeSource)(safeImportSource) as OpenClawPluginModule,
() => loadPluginModule(safeSource) as OpenClawPluginModule,
);
} catch (err) {
recordPluginError({
@@ -1801,13 +1807,12 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
}
const safeRuntimeSource = runtimeOpened.path;
fs.closeSync(runtimeOpened.fd);
const safeRuntimeImportSource = toSafeImportPath(safeRuntimeSource);
let runtimeMod: OpenClawPluginModule | null = null;
try {
runtimeMod = withProfile(
{ pluginId: record.id, source: safeRuntimeSource },
"load-setup-runtime-entry",
() => getJiti(safeRuntimeSource)(safeRuntimeImportSource) as OpenClawPluginModule,
() => loadPluginModule(safeRuntimeSource) as OpenClawPluginModule,
);
} catch (err) {
recordPluginError({
@@ -2165,7 +2170,7 @@ export async function loadOpenClawPluginCliRegistry(
});
const logger = options.logger ?? defaultLogger();
const onlyPluginIdSet = createPluginIdScopeSet(onlyPluginIds);
const getJiti = createPluginJitiLoader(options);
const loadPluginModule = createPluginModuleLoader(options);
const { registry, registerCli } = createPluginRegistry({
logger,
runtime: {} as PluginRuntime,
@@ -2387,14 +2392,13 @@ export async function loadOpenClawPluginCliRegistry(
}
const safeSource = opened.path;
fs.closeSync(opened.fd);
const safeImportSource = toSafeImportPath(safeSource);
let mod: OpenClawPluginModule | null = null;
try {
mod = withProfile(
{ pluginId: record.id, source: safeSource },
"cli-metadata",
() => getJiti(safeSource)(safeImportSource) as OpenClawPluginModule,
() => loadPluginModule(safeSource) as OpenClawPluginModule,
);
} catch (err) {
recordPluginError({