mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:30:42 +00:00
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
This commit is contained in:
committed by
GitHub
parent
0bf19f540d
commit
e5ec14a06a
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -191,7 +191,7 @@ Or per-agent:
|
||||
Avoid using `tools.allow: ["lobster"]` unless you intend to run in restrictive allowlist mode.
|
||||
|
||||
<Note>
|
||||
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.
|
||||
</Note>
|
||||
|
||||
## Example: Email triage
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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 = [
|
||||
{
|
||||
|
||||
@@ -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<ToolPolicyLike | undefined>): string[] {
|
||||
const entries: string[] = [];
|
||||
for (const policy of policies) {
|
||||
@@ -104,12 +108,18 @@ export function collectExplicitAllowlist(policies: Array<ToolPolicyLike | undefi
|
||||
continue;
|
||||
}
|
||||
const trimmed = value.trim();
|
||||
if (trimmed === "*" && policy[IMPLICIT_ALLOW_ALL_FROM_ALSO_ALLOW] === true) {
|
||||
continue;
|
||||
}
|
||||
if (trimmed) {
|
||||
entries.push(trimmed);
|
||||
}
|
||||
}
|
||||
if (policy[IMPLICIT_ALLOW_ALL_FROM_ALSO_ALLOW] === true) {
|
||||
entries.push(DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY);
|
||||
}
|
||||
}
|
||||
return entries;
|
||||
return Array.from(new Set(entries));
|
||||
}
|
||||
|
||||
export function collectExplicitDenylist(policies: Array<ToolPolicyLike | undefined>): string[] {
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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([
|
||||
{
|
||||
|
||||
@@ -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<string>): 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<string>;
|
||||
}): 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>;
|
||||
}): string[] {
|
||||
if (
|
||||
params.allowlist.size === 0 ||
|
||||
allowlistIncludesDefaultPluginTools(params.allowlist) ||
|
||||
params.allowlist.has("*") ||
|
||||
params.allowlist.has("group:plugins")
|
||||
) {
|
||||
|
||||
Reference in New Issue
Block a user