diff --git a/CHANGELOG.md b/CHANGELOG.md index 24f38588d..7f6f1476a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,6 @@ Docs: https://docs.clawd.bot ### Changes - Agents: keep system prompt time zone-only and move current time to `session_status` for better cache hits. -- Cron: append current time to isolated agent prompt context for scheduled runs. - Browser: add node-host proxy auto-routing for remote gateways (configurable per gateway/node). - Plugins: add optional llm-task JSON-only tool for workflows. (#1498) Thanks @vignesh07. - CLI: restart the gateway by default after `clawdbot update`; add `--no-restart` to skip it. @@ -20,7 +19,6 @@ Docs: https://docs.clawd.bot - Channels: allow per-group tool allow/deny policies across built-in + plugin channels. (#1546) Thanks @adam91holt. ### Fixes -- Slack: honor open groupPolicy for unlisted channels in message + slash gating. (#1563) Thanks @itsjaydesu. - Agents: ignore IDENTITY.md template placeholders when parsing identity to avoid placeholder replies. (#1556) - Docker: update gateway command in docker-compose and Hetzner guide. (#1514) - Sessions: reject array-backed session stores to prevent silent wipes. (#1469) @@ -57,8 +55,7 @@ Docs: https://docs.clawd.bot - Exec approvals: persist allowlist entry ids to keep macOS allowlist rows stable. (#1521) Thanks @ngutman. - MS Teams (plugin): remove `.default` suffix from Graph scopes to avoid double-appending. (#1507) Thanks @Evizero. - Browser: keep extension relay tabs controllable when the extension reuses a session id after switching tabs. (#1160) -- TUI: track active run ids from chat events so tool/lifecycle updates show for non-TUI runs. (#1567) Thanks @vignesh07. -- TUI: ignore lifecycle updates from non-active runs to keep status accurate. (#1567) Thanks @vignesh07. +- Agents: warn and ignore tool allowlists that only reference unknown or unloaded plugin tools. (#1566) ## 2026.1.22 diff --git a/docs/tools/index.md b/docs/tools/index.md index 42e216a6d..88e552afa 100644 --- a/docs/tools/index.md +++ b/docs/tools/index.md @@ -25,6 +25,7 @@ You can globally allow/deny tools via `tools.allow` / `tools.deny` in `clawdbot. Notes: - Matching is case-insensitive. - `*` wildcards are supported (`"*"` means all tools). +- If `tools.allow` only references unknown or unloaded plugin tool names, Clawdbot logs a warning and ignores the allowlist so core tools stay available. ## Tool profiles (base allowlist) diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 292b706b7..b8c328e40 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -44,10 +44,12 @@ import { buildPluginToolGroups, collectExplicitAllowlist, expandPolicyWithPluginGroups, + normalizeToolName, resolveToolProfilePolicy, stripPluginOnlyAllowlist, } from "./tool-policy.js"; import { getPluginToolMeta } from "../plugins/tools.js"; +import { logWarn } from "../logger.js"; function isOpenAIProvider(provider?: string) { const normalized = provider?.trim().toLowerCase(); @@ -319,38 +321,46 @@ export function createClawdbotCodingTools(options?: { modelHasVision: options?.modelHasVision, }), ]; + const coreToolNames = new Set( + tools + .filter((tool) => !getPluginToolMeta(tool as AnyAgentTool)) + .map((tool) => normalizeToolName(tool.name)) + .filter(Boolean), + ); const pluginGroups = buildPluginToolGroups({ tools, toolMeta: (tool) => getPluginToolMeta(tool as AnyAgentTool), }); - const profilePolicyExpanded = expandPolicyWithPluginGroups( - stripPluginOnlyAllowlist(profilePolicy, pluginGroups), - pluginGroups, + const resolvePolicy = (policy: typeof profilePolicy, label: string) => { + const resolved = stripPluginOnlyAllowlist(policy, pluginGroups, coreToolNames); + if (resolved.unknownAllowlist.length > 0) { + const entries = resolved.unknownAllowlist.join(", "); + const suffix = resolved.strippedAllowlist + ? "Ignoring allowlist so core tools remain available." + : "These entries won't match any tool unless the plugin is enabled."; + logWarn(`tools: ${label} allowlist contains unknown entries (${entries}). ${suffix}`); + } + return expandPolicyWithPluginGroups(resolved.policy, pluginGroups); + }; + const profilePolicyExpanded = resolvePolicy( + profilePolicy, + profile ? `tools.profile (${profile})` : "tools.profile", ); - const providerProfileExpanded = expandPolicyWithPluginGroups( - stripPluginOnlyAllowlist(providerProfilePolicy, pluginGroups), - pluginGroups, + const providerProfileExpanded = resolvePolicy( + providerProfilePolicy, + providerProfile ? `tools.byProvider.profile (${providerProfile})` : "tools.byProvider.profile", ); - const globalPolicyExpanded = expandPolicyWithPluginGroups( - stripPluginOnlyAllowlist(globalPolicy, pluginGroups), - pluginGroups, + const globalPolicyExpanded = resolvePolicy(globalPolicy, "tools.allow"); + const globalProviderExpanded = resolvePolicy(globalProviderPolicy, "tools.byProvider.allow"); + const agentPolicyExpanded = resolvePolicy( + agentPolicy, + agentId ? `agents.${agentId}.tools.allow` : "agent tools.allow", ); - const globalProviderExpanded = expandPolicyWithPluginGroups( - stripPluginOnlyAllowlist(globalProviderPolicy, pluginGroups), - pluginGroups, - ); - const agentPolicyExpanded = expandPolicyWithPluginGroups( - stripPluginOnlyAllowlist(agentPolicy, pluginGroups), - pluginGroups, - ); - const agentProviderExpanded = expandPolicyWithPluginGroups( - stripPluginOnlyAllowlist(agentProviderPolicy, pluginGroups), - pluginGroups, - ); - const groupPolicyExpanded = expandPolicyWithPluginGroups( - stripPluginOnlyAllowlist(groupPolicy, pluginGroups), - pluginGroups, + const agentProviderExpanded = resolvePolicy( + agentProviderPolicy, + agentId ? `agents.${agentId}.tools.byProvider.allow` : "agent tools.byProvider.allow", ); + const groupPolicyExpanded = resolvePolicy(groupPolicy, "group tools.allow"); const sandboxPolicyExpanded = expandPolicyWithPluginGroups(sandbox?.tools, pluginGroups); const subagentPolicyExpanded = expandPolicyWithPluginGroups(subagentPolicy, pluginGroups); diff --git a/src/agents/tool-policy.plugin-only-allowlist.test.ts b/src/agents/tool-policy.plugin-only-allowlist.test.ts index b5f6c9d42..7964519aa 100644 --- a/src/agents/tool-policy.plugin-only-allowlist.test.ts +++ b/src/agents/tool-policy.plugin-only-allowlist.test.ts @@ -6,20 +6,46 @@ const pluginGroups: PluginToolGroups = { all: ["lobster", "workflow_tool"], byPlugin: new Map([["lobster", ["lobster", "workflow_tool"]]]), }; +const coreTools = new Set(["read", "write", "exec", "session_status"]); describe("stripPluginOnlyAllowlist", () => { it("strips allowlist when it only targets plugin tools", () => { - const policy = stripPluginOnlyAllowlist({ allow: ["lobster"] }, pluginGroups); - expect(policy?.allow).toBeUndefined(); + const policy = stripPluginOnlyAllowlist({ allow: ["lobster"] }, pluginGroups, coreTools); + expect(policy.policy?.allow).toBeUndefined(); + expect(policy.unknownAllowlist).toEqual([]); }); it("strips allowlist when it only targets plugin groups", () => { - const policy = stripPluginOnlyAllowlist({ allow: ["group:plugins"] }, pluginGroups); - expect(policy?.allow).toBeUndefined(); + const policy = stripPluginOnlyAllowlist({ allow: ["group:plugins"] }, pluginGroups, coreTools); + expect(policy.policy?.allow).toBeUndefined(); + expect(policy.unknownAllowlist).toEqual([]); }); it("keeps allowlist when it mixes plugin and core entries", () => { - const policy = stripPluginOnlyAllowlist({ allow: ["lobster", "read"] }, pluginGroups); - expect(policy?.allow).toEqual(["lobster", "read"]); + const policy = stripPluginOnlyAllowlist( + { allow: ["lobster", "read"] }, + pluginGroups, + coreTools, + ); + expect(policy.policy?.allow).toEqual(["lobster", "read"]); + expect(policy.unknownAllowlist).toEqual([]); + }); + + it("strips allowlist with unknown entries when no core tools match", () => { + const emptyPlugins: PluginToolGroups = { all: [], byPlugin: new Map() }; + const policy = stripPluginOnlyAllowlist({ allow: ["lobster"] }, emptyPlugins, coreTools); + expect(policy.policy?.allow).toBeUndefined(); + expect(policy.unknownAllowlist).toEqual(["lobster"]); + }); + + it("keeps allowlist with core tools and reports unknown entries", () => { + const emptyPlugins: PluginToolGroups = { all: [], byPlugin: new Map() }; + const policy = stripPluginOnlyAllowlist( + { allow: ["read", "lobster"] }, + emptyPlugins, + coreTools, + ); + expect(policy.policy?.allow).toEqual(["read", "lobster"]); + expect(policy.unknownAllowlist).toEqual(["lobster"]); }); }); diff --git a/src/agents/tool-policy.ts b/src/agents/tool-policy.ts index d5e7e887c..c2ead21d4 100644 --- a/src/agents/tool-policy.ts +++ b/src/agents/tool-policy.ts @@ -95,6 +95,12 @@ export type PluginToolGroups = { byPlugin: Map; }; +export type AllowlistResolution = { + policy: ToolPolicyLike | undefined; + unknownAllowlist: string[]; + strippedAllowlist: boolean; +}; + export function expandToolGroups(list?: string[]) { const normalized = normalizeToolList(list); const expanded: string[] = []; @@ -181,17 +187,33 @@ export function expandPolicyWithPluginGroups( export function stripPluginOnlyAllowlist( policy: ToolPolicyLike | undefined, groups: PluginToolGroups, -): ToolPolicyLike | undefined { - if (!policy?.allow || policy.allow.length === 0) return policy; + coreTools: Set, +): AllowlistResolution { + if (!policy?.allow || policy.allow.length === 0) { + return { policy, unknownAllowlist: [], strippedAllowlist: false }; + } const normalized = normalizeToolList(policy.allow); - if (normalized.length === 0) return policy; + if (normalized.length === 0) { + return { policy, unknownAllowlist: [], strippedAllowlist: false }; + } const pluginIds = new Set(groups.byPlugin.keys()); const pluginTools = new Set(groups.all); - const isPluginEntry = (entry: string) => - entry === "group:plugins" || pluginIds.has(entry) || pluginTools.has(entry); - const isPluginOnly = normalized.every((entry) => isPluginEntry(entry)); - if (!isPluginOnly) return policy; - return { ...policy, allow: undefined }; + const unknownAllowlist: string[] = []; + let hasCoreEntry = false; + for (const entry of normalized) { + const isPluginEntry = + entry === "group:plugins" || pluginIds.has(entry) || pluginTools.has(entry); + const expanded = expandToolGroups([entry]); + const isCoreEntry = expanded.some((tool) => coreTools.has(tool)); + if (isCoreEntry) hasCoreEntry = true; + if (!isCoreEntry && !isPluginEntry) unknownAllowlist.push(entry); + } + const strippedAllowlist = !hasCoreEntry; + return { + policy: strippedAllowlist ? { ...policy, allow: undefined } : policy, + unknownAllowlist: Array.from(new Set(unknownAllowlist)), + strippedAllowlist, + }; } export function resolveToolProfilePolicy(profile?: string): ToolProfilePolicy | undefined {