From 571d75aab351e8938f6deebbccf94367a44c0e62 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 3 May 2026 19:33:00 -0700 Subject: [PATCH] fix(plugins): honor plugin tool denylists --- CHANGELOG.md | 1 + src/agents/openclaw-plugin-tools.ts | 2 + ...w-tools.browser-plugin.integration.test.ts | 25 ++++ src/agents/pi-tools.ts | 1 + src/gateway/tool-resolution.ts | 11 ++ src/plugins/tools.optional.test.ts | 26 ++++ src/plugins/tools.ts | 136 ++++++++++++++---- 7 files changed, 176 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45fc2566206..ed3ed3cbe00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ Docs: https://docs.openclaw.ai - Gateway/sessions: memoize repeated thinking-option enrichment and skip unused cost fallback checks while listing sessions, reducing per-row work on large multi-agent stores. Fixes #76931. - Agents/tools: use config-only runtime snapshots for plugin tool registration and live runtime config getters, avoiding expensive full secrets snapshot clones on the core-plugin-tools prep path. Fixes #76295. - Agents/tools: honor the effective tool denylist before constructing optional PDF/media tool factories, so `tools.deny: ["pdf"]` skips PDF setup before later policy filtering. Fixes #76997. +- Plugin tools: honor explicit tool denylists while selecting plugin tool runtimes, so denied plugin tools are not materialized for direct command or gateway surfaces before later policy filtering. Thanks @vincentkoc. - Agents/bootstrap: keep pending `BOOTSTRAP.md` and bootstrap truncation notices in system-prompt Project Context instead of copying setup text or raw warning diagnostics into WebChat user/runtime context. Fixes #76946. - Channels/WhatsApp: allow `@whiskeysockets/libsignal-node` in `onlyBuiltDependencies` so pnpm v9+ `blockExoticSubdeps` no longer rejects the baileys git-tarball subdep and silences all inbound agent replies. Fixes #76539. Thanks @ottodeng and @vincentkoc. - Gateway/install: keep `.env`-managed values in the macOS LaunchAgent env file while still tracking `OPENCLAW_SERVICE_MANAGED_ENV_KEYS`, so regenerated services do not boot without managed auth/provider keys. Fixes #75374. diff --git a/src/agents/openclaw-plugin-tools.ts b/src/agents/openclaw-plugin-tools.ts index e9f555454c6..4599d98ff86 100644 --- a/src/agents/openclaw-plugin-tools.ts +++ b/src/agents/openclaw-plugin-tools.ts @@ -17,6 +17,7 @@ import type { AnyAgentTool } from "./tools/common.js"; type ResolveOpenClawPluginToolsOptions = OpenClawPluginToolOptions & { pluginToolAllowlist?: string[]; + pluginToolDenylist?: string[]; currentChannelId?: string; currentThreadTs?: string; currentMessageId?: string | number; @@ -81,6 +82,7 @@ export function resolveOpenClawPluginToolsForOptions(params: { }), existingToolNames: params.existingToolNames ?? new Set(), toolAllowlist: params.options?.pluginToolAllowlist, + toolDenylist: params.options?.pluginToolDenylist, allowGatewaySubagentBinding: params.options?.allowGatewaySubagentBinding, ...(authProfileStore ? { diff --git a/src/agents/openclaw-tools.browser-plugin.integration.test.ts b/src/agents/openclaw-tools.browser-plugin.integration.test.ts index 2ab38080f25..b59d241ca6b 100644 --- a/src/agents/openclaw-tools.browser-plugin.integration.test.ts +++ b/src/agents/openclaw-tools.browser-plugin.integration.test.ts @@ -142,6 +142,31 @@ describe("createOpenClawTools browser plugin integration", () => { ); }); + it("forwards plugin tool deny policy to plugin resolution", () => { + hoisted.resolvePluginTools.mockReturnValue([]); + const config = { + plugins: { + allow: ["browser"], + }, + } as OpenClawConfig; + + resolveOpenClawPluginToolsForOptions({ + options: { + config, + pluginToolAllowlist: ["*"], + pluginToolDenylist: ["browser"], + }, + resolvedConfig: config, + }); + + expect(hoisted.resolvePluginTools).toHaveBeenCalledWith( + expect.objectContaining({ + toolAllowlist: ["*"], + toolDenylist: ["browser"], + }), + ); + }); + it("does not pass a stale active snapshot as plugin runtime config for a resolved run config", () => { const staleSourceConfig = { plugins: { diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 3c760e7f6cc..db233bc9165 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -649,6 +649,7 @@ export function createOpenClawCodingTools(options?: { allowHostBrowserControl: sandbox ? sandbox.browserAllowHostControl : true, sandboxed: !!sandbox, pluginToolAllowlist, + pluginToolDenylist, currentChannelId: options?.currentChannelId, currentThreadTs: options?.currentThreadTs, currentMessageId: options?.currentMessageId, diff --git a/src/gateway/tool-resolution.ts b/src/gateway/tool-resolution.ts index 7dafd079701..37c2e690c6d 100644 --- a/src/gateway/tool-resolution.ts +++ b/src/gateway/tool-resolution.ts @@ -15,6 +15,7 @@ import { } from "../agents/tool-policy-pipeline.js"; import { collectExplicitAllowlist, + collectExplicitDenylist, mergeAlsoAllowPolicy, resolveToolProfilePolicy, } from "../agents/tool-policy.js"; @@ -108,6 +109,16 @@ export function resolveGatewayScopedTools(params: { subagentPolicy, gatewayRequestedTools.length > 0 ? { allow: gatewayRequestedTools } : undefined, ]), + pluginToolDenylist: collectExplicitDenylist([ + profilePolicy, + providerProfilePolicy, + globalPolicy, + globalProviderPolicy, + agentPolicy, + agentProviderPolicy, + groupPolicy, + subagentPolicy, + ]), }); const policyFiltered = applyToolPolicyPipeline({ diff --git a/src/plugins/tools.optional.test.ts b/src/plugins/tools.optional.test.ts index 55c2306b753..2864ccf0922 100644 --- a/src/plugins/tools.optional.test.ts +++ b/src/plugins/tools.optional.test.ts @@ -68,6 +68,7 @@ function createContext() { function createResolveToolsParams(params?: { context?: ReturnType & Record; toolAllowlist?: readonly string[]; + toolDenylist?: readonly string[]; existingToolNames?: Set; env?: NodeJS.ProcessEnv; suppressNameConflicts?: boolean; @@ -76,6 +77,7 @@ function createResolveToolsParams(params?: { return { context: (params?.context ?? createContext()) as never, ...(params?.toolAllowlist ? { toolAllowlist: [...params.toolAllowlist] } : {}), + ...(params?.toolDenylist ? { toolDenylist: [...params.toolDenylist] } : {}), ...(params?.existingToolNames ? { existingToolNames: params.existingToolNames } : {}), ...(params?.env ? { env: params.env } : {}), ...(params?.suppressNameConflicts ? { suppressNameConflicts: true } : {}), @@ -2177,6 +2179,30 @@ describe("resolvePluginTools optional tools", () => { expectResolvedToolNames(tools, ["browser"]); }); + it("does not materialize plugin tools blocked by explicit deny policy", () => { + const browserFactory = vi.fn(() => makeTool("browser")); + const browserEntry: MockRegistryToolEntry = { + pluginId: "browser", + optional: false, + source: "/tmp/browser.js", + names: ["browser"], + declaredNames: ["browser"], + factory: browserFactory, + }; + setRegistry([browserEntry]); + + const tools = resolvePluginTools( + createResolveToolsParams({ + toolAllowlist: ["*"], + toolDenylist: ["browser"], + }), + ); + + expectResolvedToolNames(tools, []); + expect(browserFactory).not.toHaveBeenCalled(); + expect(loadOpenClawPluginsMock).not.toHaveBeenCalled(); + }); + it("includes optional tools when wildcard allowlist is active (#76507)", () => { setOptionalDemoRegistry(); diff --git a/src/plugins/tools.ts b/src/plugins/tools.ts index a14b42e025a..8aaf7511136 100644 --- a/src/plugins/tools.ts +++ b/src/plugins/tools.ts @@ -1,3 +1,4 @@ +import { compileGlobPatterns, matchesAnyGlobPattern } from "../agents/glob-pattern.js"; import { DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY, normalizeToolName } from "../agents/tool-policy.js"; import type { AnyAgentTool } from "../agents/tools/common.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; @@ -86,6 +87,39 @@ function normalizeAllowlist(list?: string[]) { return new Set((list ?? []).map(normalizeToolName).filter(Boolean)); } +function normalizeDenylist(list?: string[]) { + return compileGlobPatterns({ + raw: list, + normalize: normalizeToolName, + }); +} + +function denylistBlocksName(name: string, denylist: ReturnType): boolean { + const normalized = normalizeToolName(name); + return normalized ? matchesAnyGlobPattern(normalized, denylist) : false; +} + +function denylistBlocksPlugin(params: { + pluginId: string; + denylist: ReturnType; +}): boolean { + return ( + denylistBlocksName(params.pluginId, params.denylist) || + matchesAnyGlobPattern("group:plugins", params.denylist) + ); +} + +function denylistBlocksPluginTool(params: { + pluginId: string; + toolName: string; + denylist: ReturnType; +}): boolean { + return ( + denylistBlocksPlugin({ pluginId: params.pluginId, denylist: params.denylist }) || + denylistBlocksName(params.toolName, params.denylist) + ); +} + function allowlistIncludesDefaultPluginTools(allowlist: Set): boolean { return allowlist.size === 0 || allowlist.has(DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY); } @@ -331,15 +365,6 @@ function listManifestToolNamesForAllowlist(params: { return [...new Set([...defaultToolNames, ...matchedToolNames])]; } -function manifestToolContractMatchesAllowlist(params: { - plugin: PluginManifestRecord; - toolNames: readonly string[]; - pluginId: string; - allowlist: Set; -}): boolean { - return listManifestToolNamesForAllowlist(params).length > 0; -} - function listManifestToolNamesForAvailability(params: { plugin: PluginManifestRecord; toolNames: readonly string[]; @@ -389,11 +414,13 @@ function resolvePluginToolRuntimePluginIds(params: { workspaceDir?: string; env: NodeJS.ProcessEnv; toolAllowlist?: string[]; + toolDenylist?: string[]; hasAuthForProvider?: (providerId: string) => boolean; snapshot?: PluginMetadataManifestView; }): string[] { const pluginIds = new Set(); const allowlist = normalizeAllowlist(params.toolAllowlist); + const denylist = normalizeDenylist(params.toolDenylist); const normalizedPlugins = normalizePluginsConfig(params.config?.plugins); const snapshot = params.snapshot ?? @@ -418,22 +445,28 @@ function resolvePluginToolRuntimePluginIds(params: { ) { continue; } + if (denylistBlocksPlugin({ pluginId: plugin.id, denylist })) { + continue; + } const toolNames = plugin.contracts?.tools ?? []; + const selectedToolNames = listManifestToolNamesForAvailability({ + toolNames, + plugin, + pluginId: plugin.id, + allowlist, + }).filter( + (toolName) => + !denylistBlocksPluginTool({ + pluginId: plugin.id, + toolName, + denylist, + }), + ); if ( - manifestToolContractMatchesAllowlist({ - plugin, - toolNames, - pluginId: plugin.id, - allowlist, - }) && + selectedToolNames.length > 0 && hasManifestToolAvailability({ plugin, - toolNames: listManifestToolNamesForAvailability({ - toolNames, - plugin, - pluginId: plugin.id, - allowlist, - }), + toolNames: selectedToolNames, config: params.availabilityConfig ?? params.config, env: params.env, hasAuthForProvider: params.hasAuthForProvider, @@ -553,6 +586,7 @@ function resolveCachedPluginTools(params: { availabilityConfig: PluginLoadOptions["config"]; env: NodeJS.ProcessEnv; allowlist: Set; + denylist: ReturnType; hasAuthForProvider?: (providerId: string) => boolean; onlyPluginIds: readonly string[]; existing: Set; @@ -570,6 +604,9 @@ function resolveCachedPluginTools(params: { if (!onlyPluginIdSet.has(plugin.id)) { continue; } + if (denylistBlocksPlugin({ pluginId: plugin.id, denylist: params.denylist })) { + continue; + } if ( !isManifestPluginAvailableForControlPlane({ snapshot: params.snapshot, @@ -585,7 +622,14 @@ function resolveCachedPluginTools(params: { toolNames: contractToolNames, pluginId: plugin.id, allowlist: params.allowlist, - }); + }).filter( + (toolName) => + !denylistBlocksPluginTool({ + pluginId: plugin.id, + toolName, + denylist: params.denylist, + }), + ); const availableToolNames = filterManifestToolNamesForAvailability({ plugin, toolNames: allowedToolNames, @@ -639,6 +683,15 @@ function resolveCachedPluginTools(params: { continue; } const normalizedDescriptorName = normalizeToolName(cachedDescriptor.descriptor.name); + if ( + denylistBlocksPluginTool({ + pluginId: plugin.id, + toolName: cachedDescriptor.descriptor.name, + denylist: params.denylist, + }) + ) { + continue; + } if ( localNormalizedNames.has(normalizedDescriptorName) || params.existingNormalized.has(normalizedDescriptorName) @@ -732,6 +785,7 @@ function registryHasScopedPluginTools( function resolvePluginToolLoadState(params: { context: OpenClawPluginToolContext; toolAllowlist?: string[]; + toolDenylist?: string[]; allowGatewaySubagentBinding?: boolean; hasAuthForProvider?: (providerId: string) => boolean; env?: NodeJS.ProcessEnv; @@ -771,6 +825,7 @@ function resolvePluginToolLoadState(params: { workspaceDir: context.workspaceDir, env, toolAllowlist: params.toolAllowlist, + toolDenylist: params.toolDenylist, hasAuthForProvider: params.hasAuthForProvider, snapshot, }); @@ -786,6 +841,7 @@ function resolvePluginToolLoadState(params: { export function ensureStandalonePluginToolRegistryLoaded(params: { context: OpenClawPluginToolContext; toolAllowlist?: string[]; + toolDenylist?: string[]; allowGatewaySubagentBinding?: boolean; hasAuthForProvider?: (providerId: string) => boolean; env?: NodeJS.ProcessEnv; @@ -805,6 +861,7 @@ export function resolvePluginTools(params: { context: OpenClawPluginToolContext; existingToolNames?: Set; toolAllowlist?: string[]; + toolDenylist?: string[]; suppressNameConflicts?: boolean; allowGatewaySubagentBinding?: boolean; hasAuthForProvider?: (providerId: string) => boolean; @@ -821,6 +878,7 @@ export function resolvePluginTools(params: { const existing = params.existingToolNames ?? new Set(); const existingNormalized = new Set(Array.from(existing, (tool) => normalizeToolName(tool))); const allowlist = normalizeAllowlist(params.toolAllowlist); + const denylist = normalizeDenylist(params.toolDenylist); const configCacheKeyMemo = createPluginToolDescriptorConfigCacheKeyMemo(); let currentRuntimeConfigForDescriptorCache: PluginLoadOptions["config"] | null | undefined = params.context.runtimeConfig; @@ -837,6 +895,7 @@ export function resolvePluginTools(params: { availabilityConfig: params.context.runtimeConfig ?? context.config, env, allowlist, + denylist, hasAuthForProvider: params.hasAuthForProvider, onlyPluginIds, existing, @@ -919,6 +978,9 @@ export function resolvePluginTools(params: { if (!scopedPluginIds.has(entry.pluginId)) { continue; } + if (denylistBlocksPlugin({ pluginId: entry.pluginId, denylist })) { + continue; + } if (blockedPlugins.has(entry.pluginId)) { continue; } @@ -948,7 +1010,14 @@ export function resolvePluginTools(params: { config: params.context.runtimeConfig ?? context.config, env, hasAuthForProvider: params.hasAuthForProvider, - }) + }).filter( + (toolName) => + !denylistBlocksPluginTool({ + pluginId: entry.pluginId, + toolName, + denylist, + }), + ) : declaredNames; if (manifestPlugin && availabilityNames.length > 0 && allowlistNames.length === 0) { continue; @@ -995,15 +1064,23 @@ export function resolvePluginTools(params: { }), ) : listRaw; + const policyAvailableList = availableList.filter( + (tool) => + !denylistBlocksPluginTool({ + pluginId: entry.pluginId, + toolName: readPluginToolName(tool), + denylist, + }), + ); const list = entry.optional - ? availableList.filter((tool) => + ? policyAvailableList.filter((tool) => isOptionalToolAllowed({ toolName: readPluginToolName(tool), pluginId: entry.pluginId, allowlist, }), ) - : availableList; + : policyAvailableList; if (list.length === 0) { continue; } @@ -1087,7 +1164,14 @@ export function resolvePluginTools(params: { toolNames: manifestPlugin.contracts?.tools ?? [], pluginId, allowlist, - }); + }).filter( + (toolName) => + !denylistBlocksPluginTool({ + pluginId, + toolName, + denylist, + }), + ); if ( cachedDescriptorsCoverToolNames({ descriptors,