mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:40:44 +00:00
fix(doctor): warn on plugin tool allowlist mismatch
This commit is contained in:
@@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Agents/pi-embedded-runner: extract the `abortable` provider-call wrapper from `runEmbeddedAttempt` to module scope so its promise handlers no longer close over the run lexical context, releasing transcripts, tool buffers, and subscription callbacks when a provider call hangs past abort. (#74182) Thanks @cjboy007.
|
||||
- Docker: restore `python3` in the gateway runtime image after the slim-runtime switch. Fixes #75041.
|
||||
- CLI/Voice Call: scope `voicecall` command activation to the Voice Call plugin so setup and smoke checks no longer broad-load unrelated plugin runtimes or hang after printing JSON. Thanks @vincentkoc.
|
||||
- Doctor/plugins: warn when restrictive `plugins.allow` is paired with wildcard or plugin-owned tool allowlists, making the exclusive plugin allowlist behavior visible before users hit empty callable-tool runs. Refs #58009 and #64982. Thanks @KR-Python and @BKF-Gitty.
|
||||
- Agents/commitments: keep inferred follow-ups internal when heartbeat target is none, strip raw source text from stored commitments, disable tools during due-commitment heartbeat turns, bound hidden extraction queue growth, expire stale commitments, and add QA/Docker safety coverage. Thanks @vignesh07.
|
||||
- Telegram/agents: keep typing indicators and optional generation tools off the reply critical path, so fresh Telegram replies no longer stall while provider catalogs and media models load. (#75360) Thanks @obviyus.
|
||||
- Agents/commitments: run hidden follow-up extraction on the configured agent/default model instead of falling back to direct OpenAI, so OpenAI Codex OAuth-only gateways no longer spam background API-key failures. Fixes #75334. Thanks @sene1337.
|
||||
|
||||
@@ -83,6 +83,7 @@ cat ~/.openclaw/openclaw.json
|
||||
- OpenCode provider override warnings (`models.providers.opencode` / `models.providers.opencode-go`).
|
||||
- Codex OAuth shadowing warnings (`models.providers.openai-codex`).
|
||||
- OAuth TLS prerequisites check for OpenAI Codex OAuth profiles.
|
||||
- Plugin/tool allowlist warnings when `plugins.allow` is restrictive but tool policy still asks for wildcard or plugin-owned tools.
|
||||
- Legacy on-disk state migration (sessions/agent dir/WhatsApp auth).
|
||||
- Legacy plugin manifest contract key migration (`speechProviders`, `realtimeTranscriptionProviders`, `realtimeVoiceProviders`, `mediaUnderstandingProviders`, `imageGenerationProviders`, `videoGenerationProviders`, `webFetchProviders`, `webSearchProviders` → `contracts`).
|
||||
- Legacy cron store migration (`jobId`, `schedule.cron`, top-level delivery/payload fields, payload `provider`, simple `notify: true` webhook fallback jobs).
|
||||
@@ -164,6 +165,11 @@ That stages grounded durable candidates into the short-term dreaming store while
|
||||
|
||||
That includes legacy Talk flat fields. Current public Talk config is `talk.provider` + `talk.providers.<provider>`. Doctor rewrites old `talk.voiceId` / `talk.voiceAliases` / `talk.modelId` / `talk.outputFormat` / `talk.apiKey` shapes into the provider map.
|
||||
|
||||
Doctor also warns when `plugins.allow` is non-empty and tool policy uses
|
||||
wildcard or plugin-owned tool entries. `tools.allow: ["*"]` only matches tools
|
||||
from plugins that actually load; it does not bypass the exclusive plugin
|
||||
allowlist.
|
||||
|
||||
</Accordion>
|
||||
<Accordion title="2. Legacy config key migrations">
|
||||
When the config contains deprecated keys, other commands refuse to run and ask you to run `openclaw doctor`.
|
||||
|
||||
@@ -218,6 +218,12 @@ Looking for third-party plugins? See [Community Plugins](/plugins/community).
|
||||
| `slots` | Exclusive slot selectors (e.g. `memory`, `contextEngine`) |
|
||||
| `entries.\<id\>` | Per-plugin toggles + config |
|
||||
|
||||
`plugins.allow` is exclusive. When it is non-empty, only listed plugins can load
|
||||
or expose tools, even if `tools.allow` contains `"*"` or a specific plugin-owned
|
||||
tool name. If a tool allowlist references plugin tools, add the owning plugin ids
|
||||
to `plugins.allow` or remove `plugins.allow`; `openclaw doctor` warns about this
|
||||
shape.
|
||||
|
||||
Config changes **require a gateway restart**. If the Gateway is running with config
|
||||
watch + in-process restart enabled (the default `openclaw gateway` path), that
|
||||
restart is usually performed automatically a moment after the config write lands.
|
||||
|
||||
@@ -135,6 +135,16 @@ export async function loadAndMaybeMigrateDoctorConfig(params: {
|
||||
}));
|
||||
}
|
||||
|
||||
const { collectPluginToolAllowlistWarnings } =
|
||||
await import("./doctor/shared/plugin-tool-allowlist-warnings.js");
|
||||
const pluginToolAllowlistWarnings = collectPluginToolAllowlistWarnings({
|
||||
cfg: candidate,
|
||||
env: process.env,
|
||||
});
|
||||
if (pluginToolAllowlistWarnings.length > 0) {
|
||||
note(sanitizeDoctorNote(pluginToolAllowlistWarnings.join("\n")), "Doctor warnings");
|
||||
}
|
||||
|
||||
if (params.runtime && params.prompter) {
|
||||
const { maybeRepairBundledPluginRuntimeDeps } =
|
||||
await import("./doctor-bundled-plugin-runtime-deps.js");
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { PluginManifestRegistry } from "../../../plugins/manifest-registry.js";
|
||||
import { collectPluginToolAllowlistWarnings } from "./plugin-tool-allowlist-warnings.js";
|
||||
|
||||
const manifestRegistry: PluginManifestRegistry = {
|
||||
diagnostics: [],
|
||||
plugins: [
|
||||
{
|
||||
id: "firecrawl",
|
||||
channels: [],
|
||||
cliBackends: [],
|
||||
hooks: [],
|
||||
manifestPath: "/virtual/firecrawl/openclaw.plugin.json",
|
||||
origin: "bundled",
|
||||
providers: [],
|
||||
rootDir: "/virtual/firecrawl",
|
||||
skills: [],
|
||||
source: "/virtual/firecrawl/index.ts",
|
||||
contracts: {
|
||||
tools: ["firecrawl_search", "firecrawl_scrape"],
|
||||
},
|
||||
},
|
||||
{
|
||||
id: "lobster",
|
||||
channels: [],
|
||||
cliBackends: [],
|
||||
hooks: [],
|
||||
manifestPath: "/virtual/lobster/openclaw.plugin.json",
|
||||
origin: "bundled",
|
||||
providers: [],
|
||||
rootDir: "/virtual/lobster",
|
||||
skills: [],
|
||||
source: "/virtual/lobster/index.ts",
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
describe("collectPluginToolAllowlistWarnings", () => {
|
||||
it("warns when tools.allow wildcard is paired with restrictive plugins.allow", () => {
|
||||
const warnings = collectPluginToolAllowlistWarnings({
|
||||
cfg: {
|
||||
plugins: { allow: ["telegram"] },
|
||||
tools: { allow: ["*"] },
|
||||
},
|
||||
manifestRegistry,
|
||||
});
|
||||
|
||||
expect(warnings).toEqual([
|
||||
expect.stringContaining(
|
||||
'plugins.allow is an exclusive plugin allowlist. tools.allow contains "*"',
|
||||
),
|
||||
]);
|
||||
});
|
||||
|
||||
it("warns when an allowlisted tool is owned by a plugin outside plugins.allow", () => {
|
||||
const warnings = collectPluginToolAllowlistWarnings({
|
||||
cfg: {
|
||||
plugins: { allow: ["telegram"] },
|
||||
tools: { allow: ["firecrawl_search"] },
|
||||
},
|
||||
manifestRegistry,
|
||||
});
|
||||
|
||||
expect(warnings).toEqual([
|
||||
'- tools.allow references tool "firecrawl_search", owned by plugin "firecrawl", but plugins.allow does not include the owning plugin. Add "firecrawl" to plugins.allow or remove plugins.allow.',
|
||||
]);
|
||||
});
|
||||
|
||||
it("warns when a tool policy references a known plugin outside plugins.allow", () => {
|
||||
const warnings = collectPluginToolAllowlistWarnings({
|
||||
cfg: {
|
||||
plugins: { allow: ["telegram"] },
|
||||
agents: {
|
||||
list: [
|
||||
{
|
||||
id: "agent-a",
|
||||
tools: { alsoAllow: ["lobster"] },
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
manifestRegistry,
|
||||
});
|
||||
|
||||
expect(warnings).toEqual([
|
||||
'- agents.list[0].tools.alsoAllow references plugin "lobster", but plugins.allow does not include it. Add "lobster" to plugins.allow or remove plugins.allow.',
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not warn when the owning plugin is allowed", () => {
|
||||
const warnings = collectPluginToolAllowlistWarnings({
|
||||
cfg: {
|
||||
plugins: { allow: ["firecrawl"] },
|
||||
tools: { allow: ["firecrawl_search"] },
|
||||
},
|
||||
manifestRegistry,
|
||||
});
|
||||
|
||||
expect(warnings).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not warn when plugins.allow is not restrictive", () => {
|
||||
const warnings = collectPluginToolAllowlistWarnings({
|
||||
cfg: {
|
||||
tools: { allow: ["*"] },
|
||||
},
|
||||
manifestRegistry,
|
||||
});
|
||||
|
||||
expect(warnings).toEqual([]);
|
||||
});
|
||||
});
|
||||
196
src/commands/doctor/shared/plugin-tool-allowlist-warnings.ts
Normal file
196
src/commands/doctor/shared/plugin-tool-allowlist-warnings.ts
Normal file
@@ -0,0 +1,196 @@
|
||||
import { normalizeToolName } from "../../../agents/tool-policy-shared.js";
|
||||
import type { OpenClawConfig } from "../../../config/types.openclaw.js";
|
||||
import { normalizePluginId } from "../../../plugins/config-state.js";
|
||||
import type { PluginManifestRegistry } from "../../../plugins/manifest-registry.js";
|
||||
import { loadPluginManifestRegistryForPluginRegistry } from "../../../plugins/plugin-registry.js";
|
||||
|
||||
type ToolAllowlistSource = {
|
||||
label: string;
|
||||
entries: string[];
|
||||
};
|
||||
|
||||
function hasRecord(value: unknown): value is Record<string, unknown> {
|
||||
return Boolean(value && typeof value === "object" && !Array.isArray(value));
|
||||
}
|
||||
|
||||
function normalizePluginIdMaybe(value: unknown): string | undefined {
|
||||
return typeof value === "string" && value.trim() ? normalizePluginId(value) : undefined;
|
||||
}
|
||||
|
||||
function collectListSource(params: { out: ToolAllowlistSource[]; value: unknown; label: string }) {
|
||||
if (!Array.isArray(params.value)) {
|
||||
return;
|
||||
}
|
||||
const entries = params.value
|
||||
.filter((entry): entry is string => typeof entry === "string")
|
||||
.map((entry) => entry.trim())
|
||||
.filter(Boolean);
|
||||
if (entries.length > 0) {
|
||||
params.out.push({ label: params.label, entries });
|
||||
}
|
||||
}
|
||||
|
||||
function collectToolPolicySources(policy: unknown, label: string, out: ToolAllowlistSource[]) {
|
||||
if (!hasRecord(policy)) {
|
||||
return;
|
||||
}
|
||||
collectListSource({ out, value: policy.allow, label: `${label}.allow` });
|
||||
collectListSource({ out, value: policy.alsoAllow, label: `${label}.alsoAllow` });
|
||||
|
||||
if (hasRecord(policy.byProvider)) {
|
||||
for (const [providerId, providerPolicy] of Object.entries(policy.byProvider)) {
|
||||
collectToolPolicySources(providerPolicy, `${label}.byProvider.${providerId}`, out);
|
||||
}
|
||||
}
|
||||
|
||||
const sandboxTools = hasRecord(policy.sandbox) ? policy.sandbox.tools : undefined;
|
||||
collectToolPolicySources(sandboxTools, `${label}.sandbox.tools`, out);
|
||||
|
||||
const subagentTools = hasRecord(policy.subagents) ? policy.subagents.tools : undefined;
|
||||
collectToolPolicySources(subagentTools, `${label}.subagents.tools`, out);
|
||||
}
|
||||
|
||||
function collectToolAllowlistSources(cfg: OpenClawConfig): ToolAllowlistSource[] {
|
||||
const sources: ToolAllowlistSource[] = [];
|
||||
collectToolPolicySources(cfg.tools, "tools", sources);
|
||||
const agentList = cfg.agents?.list;
|
||||
if (Array.isArray(agentList)) {
|
||||
agentList.forEach((agent, index) => {
|
||||
if (!hasRecord(agent)) {
|
||||
return;
|
||||
}
|
||||
collectToolPolicySources(agent.tools, `agents.list[${index}].tools`, sources);
|
||||
});
|
||||
}
|
||||
return sources;
|
||||
}
|
||||
|
||||
function formatSourceLabels(labels: Iterable<string>): string {
|
||||
const sorted = [...new Set(labels)].toSorted((left, right) => left.localeCompare(right));
|
||||
if (sorted.length <= 3) {
|
||||
return sorted.join(", ");
|
||||
}
|
||||
return `${sorted.slice(0, 3).join(", ")} (+${sorted.length - 3} more)`;
|
||||
}
|
||||
|
||||
function collectToolOwners(registry: PluginManifestRegistry): Map<string, string[]> {
|
||||
const owners = new Map<string, string[]>();
|
||||
for (const plugin of registry.plugins) {
|
||||
const pluginId = normalizePluginId(plugin.id);
|
||||
for (const toolNameRaw of plugin.contracts?.tools ?? []) {
|
||||
const toolName = normalizeToolName(toolNameRaw);
|
||||
if (!toolName) {
|
||||
continue;
|
||||
}
|
||||
owners.set(toolName, [...(owners.get(toolName) ?? []), pluginId]);
|
||||
}
|
||||
}
|
||||
return owners;
|
||||
}
|
||||
|
||||
function collectKnownPluginIds(registry: PluginManifestRegistry): Set<string> {
|
||||
return new Set(registry.plugins.map((plugin) => normalizePluginId(plugin.id)));
|
||||
}
|
||||
|
||||
function formatPluginList(pluginIds: readonly string[]): string {
|
||||
if (pluginIds.length === 1) {
|
||||
return `"${pluginIds[0]}"`;
|
||||
}
|
||||
return pluginIds.map((pluginId) => `"${pluginId}"`).join(", ");
|
||||
}
|
||||
|
||||
function addIssue(issues: Map<string, Set<string>>, key: string, sourceLabel: string) {
|
||||
const sources = issues.get(key) ?? new Set<string>();
|
||||
sources.add(sourceLabel);
|
||||
issues.set(key, sources);
|
||||
}
|
||||
|
||||
export function collectPluginToolAllowlistWarnings(params: {
|
||||
cfg: OpenClawConfig;
|
||||
env?: NodeJS.ProcessEnv;
|
||||
manifestRegistry?: PluginManifestRegistry;
|
||||
}): string[] {
|
||||
if (params.cfg.plugins?.enabled === false) {
|
||||
return [];
|
||||
}
|
||||
const allowedPluginIds = (params.cfg.plugins?.allow ?? [])
|
||||
.map(normalizePluginIdMaybe)
|
||||
.filter((pluginId): pluginId is string => Boolean(pluginId));
|
||||
const allowedPlugins = new Set(allowedPluginIds);
|
||||
if (allowedPlugins.size === 0) {
|
||||
return [];
|
||||
}
|
||||
|
||||
const sources = collectToolAllowlistSources(params.cfg);
|
||||
if (sources.length === 0) {
|
||||
return [];
|
||||
}
|
||||
|
||||
const wildcardSources = sources
|
||||
.filter((source) => source.entries.some((entry) => normalizeToolName(entry) === "*"))
|
||||
.map((source) => source.label);
|
||||
const warnings: string[] = [];
|
||||
if (wildcardSources.length > 0) {
|
||||
warnings.push(
|
||||
`- plugins.allow is an exclusive plugin allowlist. ${formatSourceLabels(wildcardSources)} contains "*", but that wildcard only matches tools from plugins that are loaded; plugin tools outside plugins.allow stay unavailable. Add the required plugin ids to plugins.allow or remove plugins.allow.`,
|
||||
);
|
||||
}
|
||||
|
||||
const exactEntries = sources.flatMap((source) =>
|
||||
source.entries
|
||||
.map((entry) => ({ source: source.label, entry: normalizeToolName(entry) }))
|
||||
.filter(({ entry }) => entry && entry !== "*" && entry !== "group:plugins"),
|
||||
);
|
||||
if (exactEntries.length === 0) {
|
||||
return warnings;
|
||||
}
|
||||
|
||||
const registry =
|
||||
params.manifestRegistry ??
|
||||
loadPluginManifestRegistryForPluginRegistry({
|
||||
config: params.cfg,
|
||||
env: params.env,
|
||||
includeDisabled: true,
|
||||
});
|
||||
const knownPluginIds = collectKnownPluginIds(registry);
|
||||
const toolOwners = collectToolOwners(registry);
|
||||
const missingPluginIssues = new Map<string, Set<string>>();
|
||||
const missingToolOwnerIssues = new Map<string, Set<string>>();
|
||||
|
||||
for (const { source, entry } of exactEntries) {
|
||||
const pluginId = normalizePluginId(entry);
|
||||
if (knownPluginIds.has(pluginId) && !allowedPlugins.has(pluginId)) {
|
||||
addIssue(missingPluginIssues, pluginId, source);
|
||||
continue;
|
||||
}
|
||||
|
||||
const owners = (toolOwners.get(entry) ?? []).filter(
|
||||
(ownerPluginId) => !allowedPlugins.has(ownerPluginId),
|
||||
);
|
||||
if (owners.length > 0 && owners.length === (toolOwners.get(entry) ?? []).length) {
|
||||
addIssue(missingToolOwnerIssues, `${entry}\u0000${owners.join("\u0000")}`, source);
|
||||
}
|
||||
}
|
||||
|
||||
for (const [pluginId, issueSources] of [...missingPluginIssues.entries()].toSorted(
|
||||
(left, right) => left[0].localeCompare(right[0]),
|
||||
)) {
|
||||
warnings.push(
|
||||
`- ${formatSourceLabels(issueSources)} references plugin "${pluginId}", but plugins.allow does not include it. Add "${pluginId}" to plugins.allow or remove plugins.allow.`,
|
||||
);
|
||||
}
|
||||
|
||||
for (const [issueKey, issueSources] of [...missingToolOwnerIssues.entries()].toSorted(
|
||||
(left, right) => left[0].localeCompare(right[0]),
|
||||
)) {
|
||||
const [toolName, ...ownerPluginIds] = issueKey.split("\u0000");
|
||||
if (!toolName) {
|
||||
continue;
|
||||
}
|
||||
warnings.push(
|
||||
`- ${formatSourceLabels(issueSources)} references tool "${toolName}", owned by plugin ${formatPluginList(ownerPluginIds)}, but plugins.allow does not include the owning plugin. Add ${formatPluginList(ownerPluginIds)} to plugins.allow or remove plugins.allow.`,
|
||||
);
|
||||
}
|
||||
|
||||
return warnings;
|
||||
}
|
||||
Reference in New Issue
Block a user