mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 10:50:44 +00:00
fix: clarify blocked plugin validation
This commit is contained in:
@@ -44,6 +44,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Agents/tools: stop treating `tools.deny: ["write"]` as an implicit `apply_patch` deny; operators who want to block patch writes should deny `apply_patch` or `group:fs` explicitly. Fixes #76749. (#76795) Thanks @Nek-12 and @hclsys.
|
||||
- Plugins/release: verify published plugin npm tarballs expose compiled runtime entries after publish, catching TS-only package artifacts before release closeout. Thanks @vincentkoc.
|
||||
- CLI/message: exit cleanly with a nonzero status when message-command plugin registry loading fails before dispatch, preventing `openclaw-message` children from staying alive after plugin load errors. Fixes #76168.
|
||||
- Plugins/config: report configured plugins that are present but blocked by path-safety checks as blocked instead of stale `plugin not found` entries, and deduplicate repeated blocked-candidate warnings during discovery. Fixes #76144. Thanks @mayank6136.
|
||||
- Gateway/update: recover an installed-but-unloaded macOS LaunchAgent after package updates, rerun Gateway health/version/channel readiness checks, and print restart, reinstall, and rollback guidance before reporting update failure. (#76790) Thanks @jonathanlindsay.
|
||||
- CLI/plugins: explain when a missing plugin command alias belongs to a bundled plugin that is disabled by default, including the `openclaw plugins enable <plugin>` repair command. (#76835)
|
||||
- Gateway/Bonjour: auto-start LAN multicast discovery only on macOS hosts while preserving explicit `openclaw plugins enable bonjour` startup elsewhere, so Linux servers and containers that do not need LAN discovery avoid default mDNS probing and watchdog churn. Refs #74209.
|
||||
|
||||
@@ -371,6 +371,8 @@ openclaw plugins doctor
|
||||
|
||||
`doctor` reports plugin load errors, manifest/discovery diagnostics, and compatibility notices. When everything is clean it prints `No plugin issues detected.`
|
||||
|
||||
If a configured plugin is present on disk but blocked by the loader's path-safety checks, config validation keeps the plugin entry and reports it as `present but blocked`. Fix the preceding blocked-plugin diagnostic, such as path ownership or world-writable permissions, instead of removing the `plugins.entries.<id>` or `plugins.allow` config.
|
||||
|
||||
For module-shape failures such as missing `register`/`activate` exports, rerun with `OPENCLAW_PLUGIN_LOAD_DEBUG=1` to include a compact export-shape summary in the diagnostic output.
|
||||
|
||||
### Registry
|
||||
|
||||
@@ -35,6 +35,11 @@ The safety gates happen **before** runtime execution. Candidates are blocked
|
||||
when the entry escapes the plugin root, the path is world-writable, or path
|
||||
ownership looks suspicious for non-bundled plugins.
|
||||
|
||||
Blocked candidates remain tied to their plugin id for diagnostics. If config
|
||||
still references that id, validation reports the plugin as present but blocked
|
||||
and points back to the path-safety warning instead of treating the config entry
|
||||
as stale.
|
||||
|
||||
### Manifest-first behavior
|
||||
|
||||
The manifest is the control-plane source of truth. OpenClaw uses it to:
|
||||
|
||||
@@ -102,6 +102,7 @@ describe("config plugin validation", () => {
|
||||
let voiceCallSchemaPluginDir = "";
|
||||
let bundlePluginDir = "";
|
||||
let manifestlessClaudeBundleDir = "";
|
||||
let blockedPluginDir = "";
|
||||
const suiteEnv = () =>
|
||||
({
|
||||
HOME: suiteHome,
|
||||
@@ -188,6 +189,12 @@ describe("config plugin validation", () => {
|
||||
await writeManifestlessClaudeBundleFixture({
|
||||
dir: manifestlessClaudeBundleDir,
|
||||
});
|
||||
blockedPluginDir = path.join(suiteHome, "blocked-plugin");
|
||||
await writePluginFixture({
|
||||
dir: blockedPluginDir,
|
||||
id: "blocked-plugin",
|
||||
schema: { type: "object" },
|
||||
});
|
||||
voiceCallSchemaPluginDir = path.join(suiteHome, "voice-call-schema-plugin");
|
||||
const voiceCallManifestPath = path.join(
|
||||
process.cwd(),
|
||||
@@ -246,6 +253,165 @@ describe("config plugin validation", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"reports configured blocked plugins without stale not-found wording",
|
||||
async () => {
|
||||
await fs.chmod(blockedPluginDir, 0o777);
|
||||
try {
|
||||
const res = validateInSuite({
|
||||
agents: { list: [{ id: "pi" }] },
|
||||
plugins: {
|
||||
enabled: true,
|
||||
load: { paths: [blockedPluginDir] },
|
||||
entries: { "blocked-plugin": { enabled: true } },
|
||||
allow: ["blocked-plugin"],
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.ok).toBe(true);
|
||||
if (!res.ok) {
|
||||
return;
|
||||
}
|
||||
expect(res.warnings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
path: "plugins.entries.blocked-plugin",
|
||||
message: expect.stringContaining("plugin present but blocked: blocked-plugin"),
|
||||
}),
|
||||
expect.objectContaining({
|
||||
path: "plugins.allow",
|
||||
message: expect.stringContaining("plugin present but blocked: blocked-plugin"),
|
||||
}),
|
||||
]),
|
||||
);
|
||||
expect(
|
||||
res.warnings.some(
|
||||
(warning) =>
|
||||
warning.message.includes("plugin not found: blocked-plugin") ||
|
||||
warning.message.includes("remove it from plugins config"),
|
||||
),
|
||||
).toBe(false);
|
||||
} finally {
|
||||
await chmodSafeDir(blockedPluginDir);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("maps legacy blocked diagnostics without plugin ids to configured load paths", () => {
|
||||
const res = validateConfigObjectWithPlugins(
|
||||
{
|
||||
agents: { list: [{ id: "pi" }] },
|
||||
plugins: {
|
||||
enabled: true,
|
||||
load: { paths: [blockedPluginDir] },
|
||||
entries: { "blocked-plugin": { enabled: true } },
|
||||
allow: ["blocked-plugin"],
|
||||
},
|
||||
},
|
||||
{
|
||||
env: suiteEnv(),
|
||||
pluginMetadataSnapshot: {
|
||||
manifestRegistry: {
|
||||
plugins: [],
|
||||
diagnostics: [
|
||||
{
|
||||
level: "warn",
|
||||
source: path.join(blockedPluginDir, "index.js"),
|
||||
message: `blocked plugin candidate: world-writable path (${blockedPluginDir}, mode=0777)`,
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
expect(res.ok).toBe(true);
|
||||
if (!res.ok) {
|
||||
return;
|
||||
}
|
||||
expect(res.warnings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
path: "plugins.entries.blocked-plugin",
|
||||
message: expect.stringContaining("plugin present but blocked: blocked-plugin"),
|
||||
}),
|
||||
expect.objectContaining({
|
||||
path: "plugins.allow",
|
||||
message: expect.stringContaining("plugin present but blocked: blocked-plugin"),
|
||||
}),
|
||||
]),
|
||||
);
|
||||
expect(
|
||||
res.warnings.some((warning) => warning.message.includes("plugin not found: blocked-plugin")),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("does not source-match blocked diagnostics that already name a different plugin id", () => {
|
||||
const aliasDir = path.join(suiteHome, "alias-dir");
|
||||
const res = validateConfigObjectWithPlugins(
|
||||
{
|
||||
agents: { list: [{ id: "pi" }] },
|
||||
plugins: {
|
||||
enabled: true,
|
||||
load: { paths: [aliasDir] },
|
||||
entries: {
|
||||
"actual-id": { enabled: true },
|
||||
"alias-dir": { enabled: true },
|
||||
},
|
||||
allow: ["actual-id", "alias-dir"],
|
||||
},
|
||||
},
|
||||
{
|
||||
env: suiteEnv(),
|
||||
pluginMetadataSnapshot: {
|
||||
manifestRegistry: {
|
||||
plugins: [],
|
||||
diagnostics: [
|
||||
{
|
||||
level: "warn",
|
||||
pluginId: "actual-id",
|
||||
source: path.join(aliasDir, "index.js"),
|
||||
message: `blocked plugin candidate: world-writable path (${aliasDir}, mode=0777)`,
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
expect(res.ok).toBe(true);
|
||||
if (!res.ok) {
|
||||
return;
|
||||
}
|
||||
expect(res.warnings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
path: "plugins.entries.actual-id",
|
||||
message: expect.stringContaining("plugin present but blocked: actual-id"),
|
||||
}),
|
||||
expect.objectContaining({
|
||||
path: "plugins.allow",
|
||||
message: expect.stringContaining("plugin present but blocked: actual-id"),
|
||||
}),
|
||||
expect.objectContaining({
|
||||
path: "plugins.entries.alias-dir",
|
||||
message:
|
||||
"plugin not found: alias-dir (stale config entry ignored; remove it from plugins config)",
|
||||
}),
|
||||
expect.objectContaining({
|
||||
path: "plugins.allow",
|
||||
message:
|
||||
"plugin not found: alias-dir (stale config entry ignored; remove it from plugins config)",
|
||||
}),
|
||||
]),
|
||||
);
|
||||
expect(
|
||||
res.warnings.some((warning) =>
|
||||
warning.message.includes("plugin present but blocked: alias-dir"),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("warns instead of failing for stale channel config backed by missing plugin refs", async () => {
|
||||
const res = validateInSuite({
|
||||
agents: { list: [{ id: "pi" }] },
|
||||
|
||||
@@ -29,7 +29,7 @@ import {
|
||||
} from "../shared/avatar-policy.js";
|
||||
import { isCanonicalDottedDecimalIPv4, isLoopbackIpAddress } from "../shared/net/ip.js";
|
||||
import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
|
||||
import { isRecord } from "../utils.js";
|
||||
import { isRecord, resolveUserPath } from "../utils.js";
|
||||
import { findDuplicateAgentDirs, formatDuplicateAgentDirError } from "./agent-dirs.js";
|
||||
import { appendAllowedValuesHint, summarizeAllowedValues } from "./allowed-values.js";
|
||||
import { GENERATED_BUNDLED_CHANNEL_CONFIG_METADATA } from "./bundled-channel-config-metadata.generated.js";
|
||||
@@ -40,6 +40,7 @@ import { coerceSecretRef } from "./types.secrets.js";
|
||||
import { OpenClawSchema } from "./zod-schema.js";
|
||||
|
||||
const LEGACY_REMOVED_PLUGIN_IDS = new Set(["google-antigravity-auth", "google-gemini-cli-auth"]);
|
||||
const BLOCKED_PLUGIN_CANDIDATE_PREFIX = "blocked plugin candidate:";
|
||||
|
||||
type UnknownIssueRecord = Record<string, unknown>;
|
||||
type ConfigPathSegment = string | number;
|
||||
@@ -1382,6 +1383,96 @@ function validateConfigObjectWithPluginsBase(
|
||||
const knownIds = ensureKnownIds();
|
||||
const normalizedPlugins = ensureNormalizedPlugins();
|
||||
const effectiveConfig = ensureCompatConfig();
|
||||
const blockedPluginDiagnostics = new Map<string, { message: string; source?: string }>();
|
||||
const blockedPluginDiagnosticsWithSource: Array<{ message: string; source: string }> = [];
|
||||
const normalizeBlockedDiagnosticPath = (value: string | undefined): string => {
|
||||
const trimmed = value?.trim();
|
||||
if (!trimmed) {
|
||||
return "";
|
||||
}
|
||||
try {
|
||||
return path.resolve(resolveUserPath(trimmed, opts.env ?? process.env));
|
||||
} catch {
|
||||
return path.resolve(trimmed);
|
||||
}
|
||||
};
|
||||
for (const diag of registry.diagnostics) {
|
||||
if (!diag.message.startsWith(BLOCKED_PLUGIN_CANDIDATE_PREFIX)) {
|
||||
continue;
|
||||
}
|
||||
if (!diag.pluginId && diag.source) {
|
||||
blockedPluginDiagnosticsWithSource.push({
|
||||
message: diag.message,
|
||||
source: diag.source,
|
||||
});
|
||||
}
|
||||
if (diag.pluginId) {
|
||||
const normalizedPluginId = normalizePluginId(diag.pluginId);
|
||||
for (const key of [diag.pluginId, normalizedPluginId]) {
|
||||
if (!key || blockedPluginDiagnostics.has(key)) {
|
||||
continue;
|
||||
}
|
||||
blockedPluginDiagnostics.set(key, {
|
||||
message: diag.message,
|
||||
...(diag.source ? { source: diag.source } : {}),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
const blockedDiagnosticSourceMatchesPluginId = (
|
||||
diagnostic: { message: string; source: string },
|
||||
pluginId: string,
|
||||
): boolean => {
|
||||
const normalizedPluginId = normalizePluginId(pluginId);
|
||||
if (!normalizedPluginId) {
|
||||
return false;
|
||||
}
|
||||
const sourcePath = normalizeBlockedDiagnosticPath(diagnostic.source);
|
||||
if (!sourcePath) {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
normalizePluginId(path.basename(sourcePath)) === normalizedPluginId ||
|
||||
normalizePluginId(path.basename(path.dirname(sourcePath))) === normalizedPluginId
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
const loadPaths = config.plugins?.load?.paths;
|
||||
if (!Array.isArray(loadPaths)) {
|
||||
return false;
|
||||
}
|
||||
for (const loadPath of loadPaths) {
|
||||
if (typeof loadPath !== "string") {
|
||||
continue;
|
||||
}
|
||||
const resolvedLoadPath = normalizeBlockedDiagnosticPath(loadPath);
|
||||
if (!resolvedLoadPath) {
|
||||
continue;
|
||||
}
|
||||
if (normalizePluginId(path.basename(resolvedLoadPath)) !== normalizedPluginId) {
|
||||
continue;
|
||||
}
|
||||
if (
|
||||
sourcePath === resolvedLoadPath ||
|
||||
sourcePath.startsWith(`${resolvedLoadPath}${path.sep}`) ||
|
||||
resolvedLoadPath.startsWith(`${sourcePath}${path.sep}`)
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
};
|
||||
const findBlockedPluginDiagnostic = (pluginId: string) => {
|
||||
const direct =
|
||||
blockedPluginDiagnostics.get(pluginId) ??
|
||||
blockedPluginDiagnostics.get(normalizePluginId(pluginId));
|
||||
if (direct) {
|
||||
return direct;
|
||||
}
|
||||
return blockedPluginDiagnosticsWithSource.find((diagnostic) =>
|
||||
blockedDiagnosticSourceMatchesPluginId(diagnostic, pluginId),
|
||||
);
|
||||
};
|
||||
const pushMissingPluginIssue = (
|
||||
path: string,
|
||||
pluginId: string,
|
||||
@@ -1394,6 +1485,17 @@ function validateConfigObjectWithPluginsBase(
|
||||
});
|
||||
return;
|
||||
}
|
||||
const blockedDiagnostic = findBlockedPluginDiagnostic(pluginId);
|
||||
if (blockedDiagnostic) {
|
||||
const source = blockedDiagnostic.source ? `; source: ${blockedDiagnostic.source}` : "";
|
||||
const message = `plugin present but blocked: ${pluginId} (see preceding plugin warning${source}; fix the blocked plugin path instead of removing config)`;
|
||||
if (opts?.warnOnly) {
|
||||
warnings.push({ path, message });
|
||||
} else {
|
||||
issues.push({ path, message });
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (opts?.warnOnly) {
|
||||
warnings.push({
|
||||
path,
|
||||
|
||||
@@ -1605,6 +1605,71 @@ describe("discoverOpenClawPlugins", () => {
|
||||
expect(result.diagnostics.some((diag) => diag.message.includes("suspicious ownership"))).toBe(
|
||||
shouldBlockForMismatch,
|
||||
);
|
||||
if (shouldBlockForMismatch) {
|
||||
expect(result.diagnostics).toContainEqual(
|
||||
expect.objectContaining({
|
||||
pluginId: "owner-mismatch",
|
||||
}),
|
||||
);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(process.platform !== "win32")("deduplicates blocked candidate diagnostics", () => {
|
||||
const stateDir = makeTempDir();
|
||||
const globalExt = path.join(stateDir, "extensions");
|
||||
mkdirSafe(globalExt);
|
||||
const blockedDir = path.join(globalExt, "blocked-plugin");
|
||||
mkdirSafe(blockedDir);
|
||||
fs.writeFileSync(path.join(blockedDir, "index.ts"), "export default function () {}", "utf-8");
|
||||
fs.chmodSync(blockedDir, 0o777);
|
||||
|
||||
try {
|
||||
const result = discoverOpenClawPlugins({
|
||||
env: {
|
||||
...buildDiscoveryEnv(stateDir),
|
||||
OPENCLAW_PLUGINS_PATHS: blockedDir,
|
||||
},
|
||||
});
|
||||
const blockedDiagnostics = result.diagnostics.filter(
|
||||
(diag) =>
|
||||
diag.pluginId === "blocked-plugin" &&
|
||||
diag.message.includes("blocked plugin candidate: world-writable path"),
|
||||
);
|
||||
expect(blockedDiagnostics).toHaveLength(1);
|
||||
} finally {
|
||||
fs.chmodSync(blockedDir, 0o755);
|
||||
}
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"uses native manifest ids for blocked index-file directory diagnostics",
|
||||
() => {
|
||||
const stateDir = makeTempDir();
|
||||
const pluginDir = path.join(stateDir, "alias-dir");
|
||||
mkdirSafe(pluginDir);
|
||||
writePluginManifest({ pluginDir, id: "actual-id" });
|
||||
writePluginEntry(path.join(pluginDir, "index.ts"));
|
||||
fs.chmodSync(pluginDir, 0o777);
|
||||
|
||||
try {
|
||||
const result = discoverOpenClawPlugins({
|
||||
extraPaths: [pluginDir],
|
||||
env: {
|
||||
...buildDiscoveryEnv(stateDir),
|
||||
},
|
||||
});
|
||||
expect(result.candidates).toHaveLength(0);
|
||||
expect(result.diagnostics).toContainEqual(
|
||||
expect.objectContaining({
|
||||
pluginId: "actual-id",
|
||||
source: expect.stringMatching(/alias-dir$/),
|
||||
message: expect.stringContaining("blocked plugin candidate: world-writable path"),
|
||||
}),
|
||||
);
|
||||
} finally {
|
||||
fs.chmodSync(pluginDir, 0o755);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -247,6 +247,7 @@ function isUnsafePluginCandidate(params: {
|
||||
source: string;
|
||||
rootDir: string;
|
||||
origin: PluginOrigin;
|
||||
pluginId?: string;
|
||||
diagnostics: PluginDiagnostic[];
|
||||
ownershipUid?: number | null;
|
||||
realpathCache: Map<string, string>;
|
||||
@@ -263,6 +264,7 @@ function isUnsafePluginCandidate(params: {
|
||||
}
|
||||
params.diagnostics.push({
|
||||
level: "warn",
|
||||
...(params.pluginId ? { pluginId: params.pluginId } : {}),
|
||||
source: issue.targetPath,
|
||||
message: formatCandidateBlockMessage(issue),
|
||||
});
|
||||
@@ -356,6 +358,7 @@ function mergeDiscoveryResult(
|
||||
target: PluginDiscoveryResult,
|
||||
source: PluginDiscoveryResult,
|
||||
seenSources: Set<string>,
|
||||
seenDiagnostics: Set<string>,
|
||||
): void {
|
||||
for (const candidate of source.candidates) {
|
||||
const key = candidate.source;
|
||||
@@ -365,7 +368,19 @@ function mergeDiscoveryResult(
|
||||
seenSources.add(key);
|
||||
target.candidates.push(candidate);
|
||||
}
|
||||
target.diagnostics.push(...source.diagnostics);
|
||||
for (const diagnostic of source.diagnostics) {
|
||||
const key = [
|
||||
diagnostic.level,
|
||||
diagnostic.pluginId ?? "",
|
||||
diagnostic.source ?? "",
|
||||
diagnostic.message,
|
||||
].join("\0");
|
||||
if (seenDiagnostics.has(key)) {
|
||||
continue;
|
||||
}
|
||||
seenDiagnostics.add(key);
|
||||
target.diagnostics.push(diagnostic);
|
||||
}
|
||||
}
|
||||
|
||||
function collectInstalledPluginRecordPaths(
|
||||
@@ -491,11 +506,13 @@ function addCandidate(params: {
|
||||
source: resolved,
|
||||
rootDir: resolvedRoot,
|
||||
origin: params.origin,
|
||||
pluginId: params.idHint,
|
||||
diagnostics: params.diagnostics,
|
||||
ownershipUid: params.ownershipUid,
|
||||
realpathCache: params.realpathCache,
|
||||
})
|
||||
) {
|
||||
params.seen.add(resolved);
|
||||
return;
|
||||
}
|
||||
params.seen.add(resolved);
|
||||
@@ -716,7 +733,7 @@ function discoverInDirectory(params: {
|
||||
candidates: params.candidates,
|
||||
diagnostics: params.diagnostics,
|
||||
seen: params.seen,
|
||||
idHint: entry.name,
|
||||
idHint: manifestId ?? entry.name,
|
||||
source: indexFile,
|
||||
...(setupSource ? { setupSource } : {}),
|
||||
rootDir: fullPath,
|
||||
@@ -917,7 +934,7 @@ function discoverFromPath(params: {
|
||||
candidates: params.candidates,
|
||||
diagnostics: params.diagnostics,
|
||||
seen: params.seen,
|
||||
idHint: path.basename(resolved),
|
||||
idHint: manifestId ?? path.basename(resolved),
|
||||
source: indexFile,
|
||||
...(setupSource ? { setupSource } : {}),
|
||||
rootDir: resolved,
|
||||
@@ -1114,7 +1131,8 @@ export function discoverOpenClawPlugins(params: {
|
||||
);
|
||||
const result = createDiscoveryResult();
|
||||
const seenSources = new Set<string>();
|
||||
mergeDiscoveryResult(result, scopedResult, seenSources);
|
||||
mergeDiscoveryResult(result, sharedResult, seenSources);
|
||||
const seenDiagnostics = new Set<string>();
|
||||
mergeDiscoveryResult(result, scopedResult, seenSources, seenDiagnostics);
|
||||
mergeDiscoveryResult(result, sharedResult, seenSources, seenDiagnostics);
|
||||
return result;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user