fix: guard tool allowlists with warnings

This commit is contained in:
Peter Steinberger
2026-01-24 07:38:15 +00:00
parent ad7fc4964a
commit 15620b1092
5 changed files with 98 additions and 42 deletions

View File

@@ -6,7 +6,6 @@ Docs: https://docs.clawd.bot
### Changes ### Changes
- Agents: keep system prompt time zone-only and move current time to `session_status` for better cache hits. - 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). - 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. - 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. - 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. - Channels: allow per-group tool allow/deny policies across built-in + plugin channels. (#1546) Thanks @adam91holt.
### Fixes ### 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) - 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) - Docker: update gateway command in docker-compose and Hetzner guide. (#1514)
- Sessions: reject array-backed session stores to prevent silent wipes. (#1469) - 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. - 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. - 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) - 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. - Agents: warn and ignore tool allowlists that only reference unknown or unloaded plugin tools. (#1566)
- TUI: ignore lifecycle updates from non-active runs to keep status accurate. (#1567) Thanks @vignesh07.
## 2026.1.22 ## 2026.1.22

View File

@@ -25,6 +25,7 @@ You can globally allow/deny tools via `tools.allow` / `tools.deny` in `clawdbot.
Notes: Notes:
- Matching is case-insensitive. - Matching is case-insensitive.
- `*` wildcards are supported (`"*"` means all tools). - `*` 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) ## Tool profiles (base allowlist)

View File

@@ -44,10 +44,12 @@ import {
buildPluginToolGroups, buildPluginToolGroups,
collectExplicitAllowlist, collectExplicitAllowlist,
expandPolicyWithPluginGroups, expandPolicyWithPluginGroups,
normalizeToolName,
resolveToolProfilePolicy, resolveToolProfilePolicy,
stripPluginOnlyAllowlist, stripPluginOnlyAllowlist,
} from "./tool-policy.js"; } from "./tool-policy.js";
import { getPluginToolMeta } from "../plugins/tools.js"; import { getPluginToolMeta } from "../plugins/tools.js";
import { logWarn } from "../logger.js";
function isOpenAIProvider(provider?: string) { function isOpenAIProvider(provider?: string) {
const normalized = provider?.trim().toLowerCase(); const normalized = provider?.trim().toLowerCase();
@@ -319,38 +321,46 @@ export function createClawdbotCodingTools(options?: {
modelHasVision: options?.modelHasVision, modelHasVision: options?.modelHasVision,
}), }),
]; ];
const coreToolNames = new Set(
tools
.filter((tool) => !getPluginToolMeta(tool as AnyAgentTool))
.map((tool) => normalizeToolName(tool.name))
.filter(Boolean),
);
const pluginGroups = buildPluginToolGroups({ const pluginGroups = buildPluginToolGroups({
tools, tools,
toolMeta: (tool) => getPluginToolMeta(tool as AnyAgentTool), toolMeta: (tool) => getPluginToolMeta(tool as AnyAgentTool),
}); });
const profilePolicyExpanded = expandPolicyWithPluginGroups( const resolvePolicy = (policy: typeof profilePolicy, label: string) => {
stripPluginOnlyAllowlist(profilePolicy, pluginGroups), const resolved = stripPluginOnlyAllowlist(policy, pluginGroups, coreToolNames);
pluginGroups, 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( const providerProfileExpanded = resolvePolicy(
stripPluginOnlyAllowlist(providerProfilePolicy, pluginGroups), providerProfilePolicy,
pluginGroups, providerProfile ? `tools.byProvider.profile (${providerProfile})` : "tools.byProvider.profile",
); );
const globalPolicyExpanded = expandPolicyWithPluginGroups( const globalPolicyExpanded = resolvePolicy(globalPolicy, "tools.allow");
stripPluginOnlyAllowlist(globalPolicy, pluginGroups), const globalProviderExpanded = resolvePolicy(globalProviderPolicy, "tools.byProvider.allow");
pluginGroups, const agentPolicyExpanded = resolvePolicy(
agentPolicy,
agentId ? `agents.${agentId}.tools.allow` : "agent tools.allow",
); );
const globalProviderExpanded = expandPolicyWithPluginGroups( const agentProviderExpanded = resolvePolicy(
stripPluginOnlyAllowlist(globalProviderPolicy, pluginGroups), agentProviderPolicy,
pluginGroups, agentId ? `agents.${agentId}.tools.byProvider.allow` : "agent tools.byProvider.allow",
);
const agentPolicyExpanded = expandPolicyWithPluginGroups(
stripPluginOnlyAllowlist(agentPolicy, pluginGroups),
pluginGroups,
);
const agentProviderExpanded = expandPolicyWithPluginGroups(
stripPluginOnlyAllowlist(agentProviderPolicy, pluginGroups),
pluginGroups,
);
const groupPolicyExpanded = expandPolicyWithPluginGroups(
stripPluginOnlyAllowlist(groupPolicy, pluginGroups),
pluginGroups,
); );
const groupPolicyExpanded = resolvePolicy(groupPolicy, "group tools.allow");
const sandboxPolicyExpanded = expandPolicyWithPluginGroups(sandbox?.tools, pluginGroups); const sandboxPolicyExpanded = expandPolicyWithPluginGroups(sandbox?.tools, pluginGroups);
const subagentPolicyExpanded = expandPolicyWithPluginGroups(subagentPolicy, pluginGroups); const subagentPolicyExpanded = expandPolicyWithPluginGroups(subagentPolicy, pluginGroups);

View File

@@ -6,20 +6,46 @@ const pluginGroups: PluginToolGroups = {
all: ["lobster", "workflow_tool"], all: ["lobster", "workflow_tool"],
byPlugin: new Map([["lobster", ["lobster", "workflow_tool"]]]), byPlugin: new Map([["lobster", ["lobster", "workflow_tool"]]]),
}; };
const coreTools = new Set(["read", "write", "exec", "session_status"]);
describe("stripPluginOnlyAllowlist", () => { describe("stripPluginOnlyAllowlist", () => {
it("strips allowlist when it only targets plugin tools", () => { it("strips allowlist when it only targets plugin tools", () => {
const policy = stripPluginOnlyAllowlist({ allow: ["lobster"] }, pluginGroups); const policy = stripPluginOnlyAllowlist({ allow: ["lobster"] }, pluginGroups, coreTools);
expect(policy?.allow).toBeUndefined(); expect(policy.policy?.allow).toBeUndefined();
expect(policy.unknownAllowlist).toEqual([]);
}); });
it("strips allowlist when it only targets plugin groups", () => { it("strips allowlist when it only targets plugin groups", () => {
const policy = stripPluginOnlyAllowlist({ allow: ["group:plugins"] }, pluginGroups); const policy = stripPluginOnlyAllowlist({ allow: ["group:plugins"] }, pluginGroups, coreTools);
expect(policy?.allow).toBeUndefined(); expect(policy.policy?.allow).toBeUndefined();
expect(policy.unknownAllowlist).toEqual([]);
}); });
it("keeps allowlist when it mixes plugin and core entries", () => { it("keeps allowlist when it mixes plugin and core entries", () => {
const policy = stripPluginOnlyAllowlist({ allow: ["lobster", "read"] }, pluginGroups); const policy = stripPluginOnlyAllowlist(
expect(policy?.allow).toEqual(["lobster", "read"]); { 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"]);
}); });
}); });

View File

@@ -95,6 +95,12 @@ export type PluginToolGroups = {
byPlugin: Map<string, string[]>; byPlugin: Map<string, string[]>;
}; };
export type AllowlistResolution = {
policy: ToolPolicyLike | undefined;
unknownAllowlist: string[];
strippedAllowlist: boolean;
};
export function expandToolGroups(list?: string[]) { export function expandToolGroups(list?: string[]) {
const normalized = normalizeToolList(list); const normalized = normalizeToolList(list);
const expanded: string[] = []; const expanded: string[] = [];
@@ -181,17 +187,33 @@ export function expandPolicyWithPluginGroups(
export function stripPluginOnlyAllowlist( export function stripPluginOnlyAllowlist(
policy: ToolPolicyLike | undefined, policy: ToolPolicyLike | undefined,
groups: PluginToolGroups, groups: PluginToolGroups,
): ToolPolicyLike | undefined { coreTools: Set<string>,
if (!policy?.allow || policy.allow.length === 0) return policy; ): AllowlistResolution {
if (!policy?.allow || policy.allow.length === 0) {
return { policy, unknownAllowlist: [], strippedAllowlist: false };
}
const normalized = normalizeToolList(policy.allow); 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 pluginIds = new Set(groups.byPlugin.keys());
const pluginTools = new Set(groups.all); const pluginTools = new Set(groups.all);
const isPluginEntry = (entry: string) => const unknownAllowlist: string[] = [];
entry === "group:plugins" || pluginIds.has(entry) || pluginTools.has(entry); let hasCoreEntry = false;
const isPluginOnly = normalized.every((entry) => isPluginEntry(entry)); for (const entry of normalized) {
if (!isPluginOnly) return policy; const isPluginEntry =
return { ...policy, allow: undefined }; 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 { export function resolveToolProfilePolicy(profile?: string): ToolProfilePolicy | undefined {