From e5ec14a06a670c9ac276f746be6e794748c2cb22 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 3 May 2026 23:46:14 +0100 Subject: [PATCH] fix(plugins): discover alsoAllow plugin tools Summary: - Discover optional plugin tools named in tools.alsoAllow without treating additive alsoAllow as a restrictive plugin-tool allowlist. - Preserve explicit alsoAllow wildcards and keep default non-optional plugin tools visible. - Document llm-task and lobster enablement and add changelog coverage. Verification: - pnpm test src/agents/tool-policy.test.ts src/gateway/tools-invoke-http.test.ts src/agents/pi-tools.create-openclaw-coding-tools.test.ts src/plugins/tools.optional.test.ts - pnpm exec oxfmt --check --threads=1 src/agents/sandbox-tool-policy.ts src/agents/tool-policy.ts src/agents/tool-policy.test.ts src/agents/pi-tools.create-openclaw-coding-tools.test.ts src/gateway/tools-invoke-http.test.ts src/plugins/tools.ts src/plugins/tools.optional.test.ts - git diff --check - Blacksmith Testbox tbx_01kqr05924hz9kw50myxrqmsf9: pnpm check:changed Fixes #76616 --- CHANGELOG.md | 1 + docs/tools/llm-task.md | 13 ++++------- docs/tools/lobster.md | 2 +- ...tools.create-openclaw-coding-tools.test.ts | 17 +++++++++++++- src/agents/sandbox-tool-policy.ts | 23 ++++++++++++++++++- src/agents/tool-policy.test.ts | 21 ++++++++++++++++- src/agents/tool-policy.ts | 12 +++++++++- src/gateway/tools-invoke-http.test.ts | 20 ++++++++++++++++ src/plugins/tools.optional.test.ts | 21 +++++++++++++++++ src/plugins/tools.ts | 14 +++++++---- 10 files changed, 126 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6774cb35cf..ace7e27c490 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ Docs: https://docs.openclaw.ai - Doctor/plugins: reset stale `plugins.slots.memory` and `plugins.slots.contextEngine` references during `doctor --fix`, so cleanup of missing plugin config does not leave unrecoverable slot owners behind. Fixes #76550 and #76551. Thanks @vincentkoc. - Docs/WhatsApp: merge the duplicate top-level `web` objects in the gateway channel config example so copy-pasted WhatsApp config keeps both `web.whatsapp` and reconnect settings. Fixes #76619. Thanks @WadydX. - Plugins/Anthropic: expose Claude thinking profiles from the bundled provider-policy artifact so non-runtime callers keep Opus 4.7 `adaptive`, `xhigh`, and `max` instead of downgrading to `high`. Fixes #76779. Thanks @tomascupr and @iAbhi001. +- Plugins/tools: honor `tools.alsoAllow` as an optional plugin tool discovery hint without treating its internal allow-all default as permission to load every optional plugin tool. Fixes #76616. - Discord/native commands: skip slash-command registration and cleanup REST calls when `channels.discord.commands.native=false`, letting low-power gateways start without waiting on disabled native-command lifecycle requests. Fixes #76202. Thanks @vincentkoc. - CLI/plugins: reject unowned command roots such as `openclaw foo` before managed proxy startup and full plugin CLI runtime loading while preserving manifest-owned and CLI-metadata-owned plugin commands. Fixes #75287. Thanks @neilofneils404. - CLI/message: skip local configured-channel plugin preload for explicit gateway-owned message actions, letting normalized CLI delivery delegate to the gateway without initializing channel runtime in the short-lived CLI process. Fixes #75477. diff --git a/docs/tools/llm-task.md b/docs/tools/llm-task.md index 727bd08fd2a..02397dd75f1 100644 --- a/docs/tools/llm-task.md +++ b/docs/tools/llm-task.md @@ -26,21 +26,18 @@ without writing custom OpenClaw code for each workflow. } ``` -2. Allowlist the tool (it is registered with `optional: true`): +2. Allow the optional tool: ```json { - "agents": { - "list": [ - { - "id": "main", - "tools": { "allow": ["llm-task"] } - } - ] + "tools": { + "alsoAllow": ["llm-task"] } } ``` +Use `tools.allow` only when you want restrictive allowlist mode. + ## Config (optional) ```json diff --git a/docs/tools/lobster.md b/docs/tools/lobster.md index 0b94c2cd4fd..9e6575c87d5 100644 --- a/docs/tools/lobster.md +++ b/docs/tools/lobster.md @@ -191,7 +191,7 @@ Or per-agent: Avoid using `tools.allow: ["lobster"]` unless you intend to run in restrictive allowlist mode. -Allowlists are opt-in for optional plugins. If your allowlist only names plugin tools (like `lobster`), OpenClaw keeps core tools enabled. To restrict core tools, include the core tools or groups you want in the allowlist too. +Allowlists are opt-in for optional plugins. `alsoAllow` enables only the named optional plugin tools while preserving the normal core tool set. To restrict core tools, use `tools.allow` with the core tools or groups you want. ## Example: Email triage diff --git a/src/agents/pi-tools.create-openclaw-coding-tools.test.ts b/src/agents/pi-tools.create-openclaw-coding-tools.test.ts index 3fccb194352..83c7cb2481c 100644 --- a/src/agents/pi-tools.create-openclaw-coding-tools.test.ts +++ b/src/agents/pi-tools.create-openclaw-coding-tools.test.ts @@ -19,7 +19,7 @@ import { expectReadWriteEditTools } from "./test-helpers/pi-tools-fs-helpers.js" import { createPiToolsSandboxContext } from "./test-helpers/pi-tools-sandbox-context.js"; import { providerAliasCases } from "./test-helpers/provider-alias-cases.js"; import { buildEmptyExplicitToolAllowlistError } from "./tool-allowlist-guard.js"; -import { normalizeToolName } from "./tool-policy.js"; +import { DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY, normalizeToolName } from "./tool-policy.js"; const tinyPngBuffer = Buffer.from( "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO2f7z8AAAAASUVORK5CYII=", @@ -180,6 +180,21 @@ describe("createOpenClawCodingTools", () => { ); }); + it("uses tools.alsoAllow for optional plugin discovery without widening to all plugins", () => { + const createOpenClawToolsMock = vi.mocked(createOpenClawTools); + createOpenClawToolsMock.mockClear(); + + createOpenClawCodingTools({ + config: { tools: { alsoAllow: ["lobster"] } }, + }); + + expect(createOpenClawToolsMock).toHaveBeenCalledWith( + expect.objectContaining({ + pluginToolAllowlist: ["lobster", DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY], + }), + ); + }); + it("passes explicit denylist entries to OpenClaw tool factory planning", () => { const createOpenClawToolsMock = vi.mocked(createOpenClawTools); createOpenClawToolsMock.mockClear(); diff --git a/src/agents/sandbox-tool-policy.ts b/src/agents/sandbox-tool-policy.ts index af1e26d17a7..7a610d622e7 100644 --- a/src/agents/sandbox-tool-policy.ts +++ b/src/agents/sandbox-tool-policy.ts @@ -1,5 +1,9 @@ import type { SandboxToolPolicy } from "./sandbox/types.js"; +export const IMPLICIT_ALLOW_ALL_FROM_ALSO_ALLOW = Symbol.for( + "openclaw.toolPolicy.implicitAllowAllFromAlsoAllow", +); + type SandboxToolPolicyConfig = { allow?: string[]; alsoAllow?: string[]; @@ -19,12 +23,21 @@ function unionAllow(base?: string[], extra?: string[]): string[] | undefined { return Array.from(new Set([...base, ...extra])); } +function hasExplicitAllowAll(list?: string[]): boolean { + return Array.isArray(list) && list.some((entry) => entry.trim() === "*"); +} + export function pickSandboxToolPolicy( config?: SandboxToolPolicyConfig, ): SandboxToolPolicy | undefined { if (!config) { return undefined; } + const allowFromAlsoAllowOnly = + !Array.isArray(config.allow) && + Array.isArray(config.alsoAllow) && + config.alsoAllow.length > 0 && + !hasExplicitAllowAll(config.alsoAllow); const allow = Array.isArray(config.allow) ? unionAllow(config.allow, config.alsoAllow) : Array.isArray(config.alsoAllow) && config.alsoAllow.length > 0 @@ -34,5 +47,13 @@ export function pickSandboxToolPolicy( if (!allow && !deny) { return undefined; } - return { allow, deny }; + const policy = { allow, deny } as SandboxToolPolicy & { + [IMPLICIT_ALLOW_ALL_FROM_ALSO_ALLOW]?: true; + }; + if (allowFromAlsoAllowOnly) { + Object.defineProperty(policy, IMPLICIT_ALLOW_ALL_FROM_ALSO_ALLOW, { + value: true, + }); + } + return policy; } diff --git a/src/agents/tool-policy.test.ts b/src/agents/tool-policy.test.ts index 912bad8829b..a535ad970f1 100644 --- a/src/agents/tool-policy.test.ts +++ b/src/agents/tool-policy.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; import { DEFAULT_GATEWAY_HTTP_TOOL_DENY } from "../security/dangerous-tools.js"; +import { pickSandboxToolPolicy } from "./sandbox-tool-policy.js"; import { isToolAllowed, resolveSandboxToolPolicyForAgent } from "./sandbox/tool-policy.js"; import type { SandboxToolPolicy } from "./sandbox/types.js"; import { isToolAllowedByPolicyName } from "./tool-policy-match.js"; @@ -8,6 +9,7 @@ import { TOOL_POLICY_CONFORMANCE } from "./tool-policy.conformance.js"; import { applyOwnerOnlyToolPolicy, collectExplicitAllowlist, + DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY, expandToolGroups, isOwnerOnlyToolName, normalizeToolName, @@ -141,7 +143,7 @@ describe("tool-policy", () => { expect(applyOwnerOnlyToolPolicy(tools, true)).toHaveLength(1); }); - it("preserves explicit alsoAllow hints when allow is empty", () => { + it("collects explicit allowlist entries", () => { expect( collectExplicitAllowlist([ { @@ -151,6 +153,23 @@ describe("tool-policy", () => { ).toContain("optional-demo"); }); + it("uses alsoAllow entries for plugin discovery without the synthetic allow-all", () => { + expect(collectExplicitAllowlist([pickSandboxToolPolicy({ alsoAllow: ["lobster"] })])).toEqual([ + "lobster", + DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY, + ]); + expect( + collectExplicitAllowlist([pickSandboxToolPolicy({ allow: [], alsoAllow: ["lobster"] })]), + ).toEqual(["*", "lobster"]); + }); + + it("preserves explicit alsoAllow wildcards for plugin discovery", () => { + expect(collectExplicitAllowlist([pickSandboxToolPolicy({ alsoAllow: ["*"] })])).toEqual(["*"]); + expect(collectExplicitAllowlist([pickSandboxToolPolicy({ alsoAllow: [" * "] })])).toEqual([ + "*", + ]); + }); + it("strips nodes for non-owner senders via fallback policy", () => { const tools = [ { diff --git a/src/agents/tool-policy.ts b/src/agents/tool-policy.ts index 4a06b62334d..65767703021 100644 --- a/src/agents/tool-policy.ts +++ b/src/agents/tool-policy.ts @@ -1,4 +1,5 @@ import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js"; +import { IMPLICIT_ALLOW_ALL_FROM_ALSO_ALLOW } from "./sandbox-tool-policy.js"; import { expandToolGroups, normalizeToolList, @@ -80,6 +81,7 @@ export function applyOwnerOnlyToolPolicy( export type ToolPolicyLike = { allow?: string[]; deny?: string[]; + [IMPLICIT_ALLOW_ALL_FROM_ALSO_ALLOW]?: true; }; export type PluginToolGroups = { @@ -93,6 +95,8 @@ export type AllowlistResolution = { pluginOnlyAllowlist: boolean; }; +export const DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY = "__openclaw_default_plugin_tools__"; + export function collectExplicitAllowlist(policies: Array): string[] { const entries: string[] = []; for (const policy of policies) { @@ -104,12 +108,18 @@ export function collectExplicitAllowlist(policies: Array): string[] { diff --git a/src/gateway/tools-invoke-http.test.ts b/src/gateway/tools-invoke-http.test.ts index 6764b5b2f3f..5db5d023170 100644 --- a/src/gateway/tools-invoke-http.test.ts +++ b/src/gateway/tools-invoke-http.test.ts @@ -494,6 +494,26 @@ describe("POST /tools/invoke", () => { ); }); + it("uses tools.alsoAllow for optional plugin discovery without loading every plugin tool", async () => { + cfg = { + ...cfg, + agents: { list: [{ id: "main", default: true }] }, + tools: { alsoAllow: ["plugin_doctor"] }, + }; + + const res = await invokeToolAuthed({ + tool: "plugin_doctor", + sessionKey: "main", + }); + + const body = await expectOkInvokeResponse(res); + expect(body.result).toMatchObject({ ok: true, permissionFlow: true }); + expect(lastCreateOpenClawToolsContext?.pluginToolAllowlist).toEqual( + expect.arrayContaining(["plugin_doctor"]), + ); + expect(lastCreateOpenClawToolsContext?.pluginToolAllowlist).not.toContain("*"); + }); + it("blocks tool execution when before_tool_call rejects the invoke", async () => { setMainAllowedTools({ allow: ["tools_invoke_test"] }); hookMocks.runBeforeToolCallHook.mockResolvedValueOnce({ diff --git a/src/plugins/tools.optional.test.ts b/src/plugins/tools.optional.test.ts index fd7de8e139e..a8ebf3182cd 100644 --- a/src/plugins/tools.optional.test.ts +++ b/src/plugins/tools.optional.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY } from "../agents/tool-policy.js"; import { resetLogger, setLoggerOverride } from "../logging/logger.js"; import { loggingState } from "../logging/state.js"; import { resolveInstalledPluginIndexPolicyHash } from "./installed-plugin-index-policy.js"; @@ -945,6 +946,26 @@ describe("resolvePluginTools optional tools", () => { expectResolvedToolNames(tools, ["optional_tool"]); }); + it("keeps default non-optional plugin tools when alsoAllow opts into optional tools", () => { + const defaultEntry: MockRegistryToolEntry = { + pluginId: "multi", + optional: false, + source: "/tmp/multi.js", + names: ["other_tool"], + declaredNames: ["other_tool"], + factory: () => makeTool("other_tool"), + }; + setRegistry([defaultEntry, createOptionalDemoEntry()]); + + const tools = resolvePluginTools( + createResolveToolsParams({ + toolAllowlist: [DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY, "optional_tool"], + }), + ); + + expectResolvedToolNames(tools, ["other_tool", "optional_tool"]); + }); + it("rejects plugin id collisions with core tool names", () => { const registry = setRegistry([ { diff --git a/src/plugins/tools.ts b/src/plugins/tools.ts index e6949f214e5..bd634b6d7c8 100644 --- a/src/plugins/tools.ts +++ b/src/plugins/tools.ts @@ -1,4 +1,4 @@ -import { normalizeToolName } from "../agents/tool-policy.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"; import { getLoadedRuntimePluginRegistry } from "./active-runtime-registry.js"; @@ -86,6 +86,10 @@ function normalizeAllowlist(list?: string[]) { return new Set((list ?? []).map(normalizeToolName).filter(Boolean)); } +function allowlistIncludesDefaultPluginTools(allowlist: Set): boolean { + return allowlist.size === 0 || allowlist.has(DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY); +} + function isOptionalToolAllowed(params: { toolName: string; pluginId: string; @@ -289,8 +293,8 @@ function pluginToolNamesMatchAllowlist(params: { optional: boolean; allowlist: Set; }): boolean { - if (params.allowlist.size === 0) { - return !params.optional; + if (!params.optional && allowlistIncludesDefaultPluginTools(params.allowlist)) { + return true; } return isOptionalToolEntryPotentiallyAllowed(params); } @@ -303,7 +307,7 @@ function manifestToolContractMatchesAllowlist(params: { if (params.toolNames.length === 0) { return false; } - if (params.allowlist.size === 0) { + if (allowlistIncludesDefaultPluginTools(params.allowlist)) { return true; } if (params.allowlist.has("*") || params.allowlist.has("group:plugins")) { @@ -322,7 +326,7 @@ function listManifestToolNamesForAvailability(params: { allowlist: Set; }): string[] { if ( - params.allowlist.size === 0 || + allowlistIncludesDefaultPluginTools(params.allowlist) || params.allowlist.has("*") || params.allowlist.has("group:plugins") ) {