diff --git a/CHANGELOG.md b/CHANGELOG.md index e0b1c786b76..8f7fe0ab235 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/docs/gateway/doctor.md b/docs/gateway/doctor.md index d3c0a6a1a35..71659e9eeca 100644 --- a/docs/gateway/doctor.md +++ b/docs/gateway/doctor.md @@ -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.`. 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. + When the config contains deprecated keys, other commands refuse to run and ask you to run `openclaw doctor`. diff --git a/docs/tools/plugin.md b/docs/tools/plugin.md index edece4b8363..c0dfbbe437b 100644 --- a/docs/tools/plugin.md +++ b/docs/tools/plugin.md @@ -218,6 +218,12 @@ Looking for third-party plugins? See [Community Plugins](/plugins/community). | `slots` | Exclusive slot selectors (e.g. `memory`, `contextEngine`) | | `entries.\` | 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. diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 4d3d83378c8..ec50f7a7db4 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -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"); diff --git a/src/commands/doctor/shared/plugin-tool-allowlist-warnings.test.ts b/src/commands/doctor/shared/plugin-tool-allowlist-warnings.test.ts new file mode 100644 index 00000000000..6f6375b870e --- /dev/null +++ b/src/commands/doctor/shared/plugin-tool-allowlist-warnings.test.ts @@ -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([]); + }); +}); diff --git a/src/commands/doctor/shared/plugin-tool-allowlist-warnings.ts b/src/commands/doctor/shared/plugin-tool-allowlist-warnings.ts new file mode 100644 index 00000000000..9a250c92119 --- /dev/null +++ b/src/commands/doctor/shared/plugin-tool-allowlist-warnings.ts @@ -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 { + 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 { + 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 { + const owners = new Map(); + 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 { + 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>, key: string, sourceLabel: string) { + const sources = issues.get(key) ?? new Set(); + 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>(); + const missingToolOwnerIssues = new Map>(); + + 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; +}