fix: isolate plugin discovery env from global state

This commit is contained in:
Peter Steinberger
2026-03-12 02:46:29 +00:00
parent 17fd46ab66
commit 43a10677ed
6 changed files with 70 additions and 87 deletions

View File

@@ -38,12 +38,15 @@ describe("config plugin validation", () => {
let enumPluginDir = ""; let enumPluginDir = "";
let bluebubblesPluginDir = ""; let bluebubblesPluginDir = "";
let voiceCallSchemaPluginDir = ""; let voiceCallSchemaPluginDir = "";
const envSnapshot = { const suiteEnv = () =>
OPENCLAW_STATE_DIR: process.env.OPENCLAW_STATE_DIR, ({
OPENCLAW_PLUGIN_MANIFEST_CACHE_MS: process.env.OPENCLAW_PLUGIN_MANIFEST_CACHE_MS, ...process.env,
}; OPENCLAW_STATE_DIR: path.join(suiteHome, ".openclaw"),
OPENCLAW_PLUGIN_MANIFEST_CACHE_MS: "10000",
}) satisfies NodeJS.ProcessEnv;
const validateInSuite = (raw: unknown) => validateConfigObjectWithPlugins(raw); const validateInSuite = (raw: unknown) =>
validateConfigObjectWithPlugins(raw, { env: suiteEnv() });
beforeAll(async () => { beforeAll(async () => {
fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-plugin-validation-")); fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-plugin-validation-"));
@@ -102,8 +105,6 @@ describe("config plugin validation", () => {
id: "voice-call-schema-fixture", id: "voice-call-schema-fixture",
schema: voiceCallManifest.configSchema, schema: voiceCallManifest.configSchema,
}); });
process.env.OPENCLAW_STATE_DIR = path.join(suiteHome, ".openclaw");
process.env.OPENCLAW_PLUGIN_MANIFEST_CACHE_MS = "10000";
clearPluginManifestRegistryCache(); clearPluginManifestRegistryCache();
// Warm the plugin manifest cache once so path-based validations can reuse // Warm the plugin manifest cache once so path-based validations can reuse
// parsed manifests across test cases. // parsed manifests across test cases.
@@ -118,16 +119,6 @@ describe("config plugin validation", () => {
afterAll(async () => { afterAll(async () => {
await fs.rm(fixtureRoot, { recursive: true, force: true }); await fs.rm(fixtureRoot, { recursive: true, force: true });
clearPluginManifestRegistryCache(); clearPluginManifestRegistryCache();
if (envSnapshot.OPENCLAW_STATE_DIR === undefined) {
delete process.env.OPENCLAW_STATE_DIR;
} else {
process.env.OPENCLAW_STATE_DIR = envSnapshot.OPENCLAW_STATE_DIR;
}
if (envSnapshot.OPENCLAW_PLUGIN_MANIFEST_CACHE_MS === undefined) {
delete process.env.OPENCLAW_PLUGIN_MANIFEST_CACHE_MS;
} else {
process.env.OPENCLAW_PLUGIN_MANIFEST_CACHE_MS = envSnapshot.OPENCLAW_PLUGIN_MANIFEST_CACHE_MS;
}
}); });
it("reports missing plugin refs across load paths, entries, and allowlist surfaces", async () => { it("reports missing plugin refs across load paths, entries, and allowlist surfaces", async () => {

View File

@@ -297,17 +297,23 @@ type ValidateConfigWithPluginsResult =
warnings: ConfigValidationIssue[]; warnings: ConfigValidationIssue[];
}; };
export function validateConfigObjectWithPlugins(raw: unknown): ValidateConfigWithPluginsResult { export function validateConfigObjectWithPlugins(
return validateConfigObjectWithPluginsBase(raw, { applyDefaults: true }); raw: unknown,
params?: { env?: NodeJS.ProcessEnv },
): ValidateConfigWithPluginsResult {
return validateConfigObjectWithPluginsBase(raw, { applyDefaults: true, env: params?.env });
} }
export function validateConfigObjectRawWithPlugins(raw: unknown): ValidateConfigWithPluginsResult { export function validateConfigObjectRawWithPlugins(
return validateConfigObjectWithPluginsBase(raw, { applyDefaults: false }); raw: unknown,
params?: { env?: NodeJS.ProcessEnv },
): ValidateConfigWithPluginsResult {
return validateConfigObjectWithPluginsBase(raw, { applyDefaults: false, env: params?.env });
} }
function validateConfigObjectWithPluginsBase( function validateConfigObjectWithPluginsBase(
raw: unknown, raw: unknown,
opts: { applyDefaults: boolean }, opts: { applyDefaults: boolean; env?: NodeJS.ProcessEnv },
): ValidateConfigWithPluginsResult { ): ValidateConfigWithPluginsResult {
const base = opts.applyDefaults ? validateConfigObject(raw) : validateConfigObjectRaw(raw); const base = opts.applyDefaults ? validateConfigObject(raw) : validateConfigObjectRaw(raw);
if (!base.ok) { if (!base.ok) {
@@ -345,6 +351,7 @@ function validateConfigObjectWithPluginsBase(
const registry = loadPluginManifestRegistry({ const registry = loadPluginManifestRegistry({
config, config,
workspaceDir: workspaceDir ?? undefined, workspaceDir: workspaceDir ?? undefined,
env: opts.env,
}); });
for (const diag of registry.diagnostics) { for (const diag of registry.diagnostics) {

View File

@@ -2,8 +2,8 @@ import fs from "node:fs";
import path from "node:path"; import path from "node:path";
import { fileURLToPath } from "node:url"; import { fileURLToPath } from "node:url";
export function resolveBundledPluginsDir(): string | undefined { export function resolveBundledPluginsDir(env: NodeJS.ProcessEnv = process.env): string | undefined {
const override = process.env.OPENCLAW_BUNDLED_PLUGINS_DIR?.trim(); const override = env.OPENCLAW_BUNDLED_PLUGINS_DIR?.trim();
if (override) { if (override) {
return override; return override;
} }

View File

@@ -3,7 +3,6 @@ import fs from "node:fs";
import os from "node:os"; import os from "node:os";
import path from "node:path"; import path from "node:path";
import { afterEach, describe, expect, it } from "vitest"; import { afterEach, describe, expect, it } from "vitest";
import { withEnvAsync } from "../test-utils/env.js";
import { clearPluginDiscoveryCache, discoverOpenClawPlugins } from "./discovery.js"; import { clearPluginDiscoveryCache, discoverOpenClawPlugins } from "./discovery.js";
const tempDirs: string[] = []; const tempDirs: string[] = [];
@@ -15,24 +14,20 @@ function makeTempDir() {
return dir; return dir;
} }
async function withStateDir<T>(stateDir: string, fn: () => Promise<T>) { function buildDiscoveryEnv(stateDir: string): NodeJS.ProcessEnv {
return await withEnvAsync( return {
{ ...process.env,
OPENCLAW_STATE_DIR: stateDir, OPENCLAW_STATE_DIR: stateDir,
CLAWDBOT_STATE_DIR: undefined, CLAWDBOT_STATE_DIR: undefined,
OPENCLAW_BUNDLED_PLUGINS_DIR: "/nonexistent/bundled/plugins", OPENCLAW_BUNDLED_PLUGINS_DIR: "/nonexistent/bundled/plugins",
}, };
fn,
);
} }
async function discoverWithStateDir( async function discoverWithStateDir(
stateDir: string, stateDir: string,
params: Parameters<typeof discoverOpenClawPlugins>[0], params: Parameters<typeof discoverOpenClawPlugins>[0],
) { ) {
return await withStateDir(stateDir, async () => { return discoverOpenClawPlugins({ ...params, env: buildDiscoveryEnv(stateDir) });
return discoverOpenClawPlugins(params);
});
} }
function writePluginPackageManifest(params: { function writePluginPackageManifest(params: {
@@ -80,9 +75,7 @@ describe("discoverOpenClawPlugins", () => {
fs.mkdirSync(workspaceExt, { recursive: true }); fs.mkdirSync(workspaceExt, { recursive: true });
fs.writeFileSync(path.join(workspaceExt, "beta.ts"), "export default function () {}", "utf-8"); fs.writeFileSync(path.join(workspaceExt, "beta.ts"), "export default function () {}", "utf-8");
const { candidates } = await withStateDir(stateDir, async () => { const { candidates } = await discoverWithStateDir(stateDir, { workspaceDir });
return discoverOpenClawPlugins({ workspaceDir });
});
const ids = candidates.map((c) => c.idHint); const ids = candidates.map((c) => c.idHint);
expect(ids).toContain("alpha"); expect(ids).toContain("alpha");
@@ -110,9 +103,7 @@ describe("discoverOpenClawPlugins", () => {
fs.mkdirSync(liveDir, { recursive: true }); fs.mkdirSync(liveDir, { recursive: true });
fs.writeFileSync(path.join(liveDir, "index.ts"), "export default function () {}", "utf-8"); fs.writeFileSync(path.join(liveDir, "index.ts"), "export default function () {}", "utf-8");
const { candidates } = await withStateDir(stateDir, async () => { const { candidates } = await discoverWithStateDir(stateDir, {});
return discoverOpenClawPlugins({});
});
const ids = candidates.map((candidate) => candidate.idHint); const ids = candidates.map((candidate) => candidate.idHint);
expect(ids).toContain("live"); expect(ids).toContain("live");
@@ -142,9 +133,7 @@ describe("discoverOpenClawPlugins", () => {
"utf-8", "utf-8",
); );
const { candidates } = await withStateDir(stateDir, async () => { const { candidates } = await discoverWithStateDir(stateDir, {});
return discoverOpenClawPlugins({});
});
const ids = candidates.map((c) => c.idHint); const ids = candidates.map((c) => c.idHint);
expect(ids).toContain("pack/one"); expect(ids).toContain("pack/one");
@@ -167,9 +156,7 @@ describe("discoverOpenClawPlugins", () => {
"utf-8", "utf-8",
); );
const { candidates } = await withStateDir(stateDir, async () => { const { candidates } = await discoverWithStateDir(stateDir, {});
return discoverOpenClawPlugins({});
});
const ids = candidates.map((c) => c.idHint); const ids = candidates.map((c) => c.idHint);
expect(ids).toContain("voice-call"); expect(ids).toContain("voice-call");
@@ -187,9 +174,7 @@ describe("discoverOpenClawPlugins", () => {
}); });
fs.writeFileSync(path.join(packDir, "index.js"), "module.exports = {}", "utf-8"); fs.writeFileSync(path.join(packDir, "index.js"), "module.exports = {}", "utf-8");
const { candidates } = await withStateDir(stateDir, async () => { const { candidates } = await discoverWithStateDir(stateDir, { extraPaths: [packDir] });
return discoverOpenClawPlugins({ extraPaths: [packDir] });
});
const ids = candidates.map((c) => c.idHint); const ids = candidates.map((c) => c.idHint);
expect(ids).toContain("demo-plugin-dir"); expect(ids).toContain("demo-plugin-dir");
@@ -266,9 +251,7 @@ describe("discoverOpenClawPlugins", () => {
extensions: ["./escape.ts"], extensions: ["./escape.ts"],
}); });
const { candidates, diagnostics } = await withStateDir(stateDir, async () => { const { candidates, diagnostics } = await discoverWithStateDir(stateDir, {});
return discoverOpenClawPlugins({});
});
expect(candidates.some((candidate) => candidate.idHint === "pack")).toBe(false); expect(candidates.some((candidate) => candidate.idHint === "pack")).toBe(false);
expectEscapesPackageDiagnostic(diagnostics); expectEscapesPackageDiagnostic(diagnostics);
@@ -303,9 +286,7 @@ describe("discoverOpenClawPlugins", () => {
throw err; throw err;
} }
const { candidates } = await withStateDir(stateDir, async () => { const { candidates } = await discoverWithStateDir(stateDir, {});
return discoverOpenClawPlugins({});
});
expect(candidates.some((candidate) => candidate.idHint === "pack")).toBe(false); expect(candidates.some((candidate) => candidate.idHint === "pack")).toBe(false);
}); });
@@ -318,9 +299,7 @@ describe("discoverOpenClawPlugins", () => {
fs.writeFileSync(pluginPath, "export default function () {}", "utf-8"); fs.writeFileSync(pluginPath, "export default function () {}", "utf-8");
fs.chmodSync(pluginPath, 0o777); fs.chmodSync(pluginPath, 0o777);
const result = await withStateDir(stateDir, async () => { const result = await discoverWithStateDir(stateDir, {});
return discoverOpenClawPlugins({});
});
expect(result.candidates).toHaveLength(0); expect(result.candidates).toHaveLength(0);
expect(result.diagnostics.some((diag) => diag.message.includes("world-writable path"))).toBe( expect(result.diagnostics.some((diag) => diag.message.includes("world-writable path"))).toBe(
@@ -338,14 +317,14 @@ describe("discoverOpenClawPlugins", () => {
fs.writeFileSync(path.join(packDir, "index.ts"), "export default function () {}", "utf-8"); fs.writeFileSync(path.join(packDir, "index.ts"), "export default function () {}", "utf-8");
fs.chmodSync(packDir, 0o777); fs.chmodSync(packDir, 0o777);
const result = await withEnvAsync( const result = discoverOpenClawPlugins({
{ env: {
...process.env,
OPENCLAW_STATE_DIR: stateDir, OPENCLAW_STATE_DIR: stateDir,
CLAWDBOT_STATE_DIR: undefined, CLAWDBOT_STATE_DIR: undefined,
OPENCLAW_BUNDLED_PLUGINS_DIR: bundledDir, OPENCLAW_BUNDLED_PLUGINS_DIR: bundledDir,
}, },
async () => discoverOpenClawPlugins({}), });
);
expect(result.candidates.some((candidate) => candidate.idHint === "demo-pack")).toBe(true); expect(result.candidates.some((candidate) => candidate.idHint === "demo-pack")).toBe(true);
expect( expect(
@@ -370,9 +349,7 @@ describe("discoverOpenClawPlugins", () => {
); );
const actualUid = (process as NodeJS.Process & { getuid: () => number }).getuid(); const actualUid = (process as NodeJS.Process & { getuid: () => number }).getuid();
const result = await withStateDir(stateDir, async () => { const result = await discoverWithStateDir(stateDir, { ownershipUid: actualUid + 1 });
return discoverOpenClawPlugins({ ownershipUid: actualUid + 1 });
});
const shouldBlockForMismatch = actualUid !== 0; const shouldBlockForMismatch = actualUid !== 0;
expect(result.candidates).toHaveLength(shouldBlockForMismatch ? 0 : 1); expect(result.candidates).toHaveLength(shouldBlockForMismatch ? 0 : 1);
expect(result.diagnostics.some((diag) => diag.message.includes("suspicious ownership"))).toBe( expect(result.diagnostics.some((diag) => diag.message.includes("suspicious ownership"))).toBe(
@@ -388,32 +365,32 @@ describe("discoverOpenClawPlugins", () => {
const pluginPath = path.join(globalExt, "cached.ts"); const pluginPath = path.join(globalExt, "cached.ts");
fs.writeFileSync(pluginPath, "export default function () {}", "utf-8"); fs.writeFileSync(pluginPath, "export default function () {}", "utf-8");
const first = await withEnvAsync( const first = discoverOpenClawPlugins({
{ env: {
...buildDiscoveryEnv(stateDir),
OPENCLAW_PLUGIN_DISCOVERY_CACHE_MS: "5000", OPENCLAW_PLUGIN_DISCOVERY_CACHE_MS: "5000",
}, },
async () => withStateDir(stateDir, async () => discoverOpenClawPlugins({})), });
);
expect(first.candidates.some((candidate) => candidate.idHint === "cached")).toBe(true); expect(first.candidates.some((candidate) => candidate.idHint === "cached")).toBe(true);
fs.rmSync(pluginPath, { force: true }); fs.rmSync(pluginPath, { force: true });
const second = await withEnvAsync( const second = discoverOpenClawPlugins({
{ env: {
...buildDiscoveryEnv(stateDir),
OPENCLAW_PLUGIN_DISCOVERY_CACHE_MS: "5000", OPENCLAW_PLUGIN_DISCOVERY_CACHE_MS: "5000",
}, },
async () => withStateDir(stateDir, async () => discoverOpenClawPlugins({})), });
);
expect(second.candidates.some((candidate) => candidate.idHint === "cached")).toBe(true); expect(second.candidates.some((candidate) => candidate.idHint === "cached")).toBe(true);
clearPluginDiscoveryCache(); clearPluginDiscoveryCache();
const third = await withEnvAsync( const third = discoverOpenClawPlugins({
{ env: {
...buildDiscoveryEnv(stateDir),
OPENCLAW_PLUGIN_DISCOVERY_CACHE_MS: "5000", OPENCLAW_PLUGIN_DISCOVERY_CACHE_MS: "5000",
}, },
async () => withStateDir(stateDir, async () => discoverOpenClawPlugins({})), });
);
expect(third.candidates.some((candidate) => candidate.idHint === "cached")).toBe(false); expect(third.candidates.some((candidate) => candidate.idHint === "cached")).toBe(false);
}); });
}); });

View File

@@ -69,10 +69,11 @@ function buildDiscoveryCacheKey(params: {
workspaceDir?: string; workspaceDir?: string;
extraPaths?: string[]; extraPaths?: string[];
ownershipUid?: number | null; ownershipUid?: number | null;
env: NodeJS.ProcessEnv;
}): string { }): string {
const workspaceKey = params.workspaceDir ? resolveUserPath(params.workspaceDir) : ""; const workspaceKey = params.workspaceDir ? resolveUserPath(params.workspaceDir) : "";
const configExtensionsRoot = path.join(resolveConfigDir(), "extensions"); const configExtensionsRoot = path.join(resolveConfigDir(params.env), "extensions");
const bundledRoot = resolveBundledPluginsDir() ?? ""; const bundledRoot = resolveBundledPluginsDir(params.env) ?? "";
const normalizedExtraPaths = (params.extraPaths ?? []) const normalizedExtraPaths = (params.extraPaths ?? [])
.filter((entry): entry is string => typeof entry === "string") .filter((entry): entry is string => typeof entry === "string")
.map((entry) => entry.trim()) .map((entry) => entry.trim())
@@ -649,6 +650,7 @@ export function discoverOpenClawPlugins(params: {
workspaceDir: params.workspaceDir, workspaceDir: params.workspaceDir,
extraPaths: params.extraPaths, extraPaths: params.extraPaths,
ownershipUid: params.ownershipUid, ownershipUid: params.ownershipUid,
env,
}); });
if (cacheEnabled) { if (cacheEnabled) {
const cached = discoveryCache.get(cacheKey); const cached = discoveryCache.get(cacheKey);
@@ -697,7 +699,7 @@ export function discoverOpenClawPlugins(params: {
} }
} }
const bundledDir = resolveBundledPluginsDir(); const bundledDir = resolveBundledPluginsDir(env);
if (bundledDir) { if (bundledDir) {
discoverInDirectory({ discoverInDirectory({
dir: bundledDir, dir: bundledDir,
@@ -711,7 +713,7 @@ export function discoverOpenClawPlugins(params: {
// Keep auto-discovered global extensions behind bundled plugins. // Keep auto-discovered global extensions behind bundled plugins.
// Users can still intentionally override via plugins.load.paths (origin=config). // Users can still intentionally override via plugins.load.paths (origin=config).
const globalDir = path.join(resolveConfigDir(), "extensions"); const globalDir = path.join(resolveConfigDir(env), "extensions");
discoverInDirectory({ discoverInDirectory({
dir: globalDir, dir: globalDir,
origin: "global", origin: "global",

View File

@@ -1,6 +1,8 @@
import fs from "node:fs"; import fs from "node:fs";
import path from "node:path";
import type { OpenClawConfig } from "../config/config.js"; import type { OpenClawConfig } from "../config/config.js";
import { resolveUserPath } from "../utils.js"; import { resolveConfigDir, resolveUserPath } from "../utils.js";
import { resolveBundledPluginsDir } from "./bundled-dir.js";
import { normalizePluginsConfig, type NormalizedPluginsConfig } from "./config-state.js"; import { normalizePluginsConfig, type NormalizedPluginsConfig } from "./config-state.js";
import { discoverOpenClawPlugins, type PluginCandidate } from "./discovery.js"; import { discoverOpenClawPlugins, type PluginCandidate } from "./discovery.js";
import { loadPluginManifest, type PluginManifest } from "./manifest.js"; import { loadPluginManifest, type PluginManifest } from "./manifest.js";
@@ -79,8 +81,11 @@ function shouldUseManifestCache(env: NodeJS.ProcessEnv): boolean {
function buildCacheKey(params: { function buildCacheKey(params: {
workspaceDir?: string; workspaceDir?: string;
plugins: NormalizedPluginsConfig; plugins: NormalizedPluginsConfig;
env: NodeJS.ProcessEnv;
}): string { }): string {
const workspaceKey = params.workspaceDir ? resolveUserPath(params.workspaceDir) : ""; const workspaceKey = params.workspaceDir ? resolveUserPath(params.workspaceDir) : "";
const configExtensionsRoot = path.join(resolveConfigDir(params.env), "extensions");
const bundledRoot = resolveBundledPluginsDir(params.env) ?? "";
// The manifest registry only depends on where plugins are discovered from (workspace + load paths). // The manifest registry only depends on where plugins are discovered from (workspace + load paths).
// It does not depend on allow/deny/entries enable-state, so exclude those for higher cache hit rates. // It does not depend on allow/deny/entries enable-state, so exclude those for higher cache hit rates.
const loadPaths = params.plugins.loadPaths const loadPaths = params.plugins.loadPaths
@@ -88,7 +93,7 @@ function buildCacheKey(params: {
.map((p) => p.trim()) .map((p) => p.trim())
.filter(Boolean) .filter(Boolean)
.toSorted(); .toSorted();
return `${workspaceKey}::${JSON.stringify(loadPaths)}`; return `${workspaceKey}::${configExtensionsRoot}::${bundledRoot}::${JSON.stringify(loadPaths)}`;
} }
function safeStatMtimeMs(filePath: string): number | null { function safeStatMtimeMs(filePath: string): number | null {
@@ -142,8 +147,8 @@ export function loadPluginManifestRegistry(params: {
}): PluginManifestRegistry { }): PluginManifestRegistry {
const config = params.config ?? {}; const config = params.config ?? {};
const normalized = normalizePluginsConfig(config.plugins); const normalized = normalizePluginsConfig(config.plugins);
const cacheKey = buildCacheKey({ workspaceDir: params.workspaceDir, plugins: normalized });
const env = params.env ?? process.env; const env = params.env ?? process.env;
const cacheKey = buildCacheKey({ workspaceDir: params.workspaceDir, plugins: normalized, env });
const cacheEnabled = params.cache !== false && shouldUseManifestCache(env); const cacheEnabled = params.cache !== false && shouldUseManifestCache(env);
if (cacheEnabled) { if (cacheEnabled) {
const cached = registryCache.get(cacheKey); const cached = registryCache.get(cacheKey);
@@ -160,6 +165,7 @@ export function loadPluginManifestRegistry(params: {
: discoverOpenClawPlugins({ : discoverOpenClawPlugins({
workspaceDir: params.workspaceDir, workspaceDir: params.workspaceDir,
extraPaths: normalized.loadPaths, extraPaths: normalized.loadPaths,
env,
}); });
const diagnostics: PluginDiagnostic[] = [...discovery.diagnostics]; const diagnostics: PluginDiagnostic[] = [...discovery.diagnostics];
const candidates: PluginCandidate[] = discovery.candidates; const candidates: PluginCandidate[] = discovery.candidates;