From f97ad8f288d2d2dc63f51a488593e5cdc9fca2d9 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 14 Feb 2026 14:11:40 +0000 Subject: [PATCH] refactor(tools): share tool policy pipeline --- src/agents/pi-tools.ts | 110 ++++++++--------------- src/agents/tool-policy-pipeline.test.ts | 66 ++++++++++++++ src/agents/tool-policy-pipeline.ts | 60 +++++++++++++ src/gateway/tools-invoke-http.ts | 114 +++++++++--------------- 4 files changed, 204 insertions(+), 146 deletions(-) create mode 100644 src/agents/tool-policy-pipeline.test.ts create mode 100644 src/agents/tool-policy-pipeline.ts diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 26e16008c0..9f69d16be5 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -26,7 +26,6 @@ import { createOpenClawTools } from "./openclaw-tools.js"; import { wrapToolWithAbortSignal } from "./pi-tools.abort.js"; import { wrapToolWithBeforeToolCallHook } from "./pi-tools.before-tool-call.js"; import { - filterToolsByPolicy, isToolAllowedByPolicies, resolveEffectiveToolPolicy, resolveGroupToolPolicy, @@ -44,14 +43,11 @@ import { wrapToolParamNormalization, } from "./pi-tools.read.js"; import { cleanToolSchemaForGemini, normalizeToolParameters } from "./pi-tools.schema.js"; +import { applyToolPolicyPipeline } from "./tool-policy-pipeline.js"; import { applyOwnerOnlyToolPolicy, - buildPluginToolGroups, collectExplicitAllowlist, - expandPolicyWithPluginGroups, - normalizeToolName, resolveToolProfilePolicy, - stripPluginOnlyAllowlist, } from "./tool-policy.js"; function isOpenAIProvider(provider?: string) { @@ -388,76 +384,46 @@ export function createOpenClawCodingTools(options?: { // Security: treat unknown/undefined as unauthorized (opt-in, not opt-out) const senderIsOwner = options?.senderIsOwner === true; const toolsByAuthorization = applyOwnerOnlyToolPolicy(tools, senderIsOwner); - const coreToolNames = new Set( - toolsByAuthorization - .filter((tool) => !getPluginToolMeta(tool)) - .map((tool) => normalizeToolName(tool.name)) - .filter(Boolean), - ); - const pluginGroups = buildPluginToolGroups({ + const subagentFiltered = applyToolPolicyPipeline({ tools: toolsByAuthorization, toolMeta: (tool) => getPluginToolMeta(tool), + warn: logWarn, + steps: [ + { + policy: profilePolicyWithAlsoAllow, + label: profile ? `tools.profile (${profile})` : "tools.profile", + stripPluginOnlyAllowlist: true, + }, + { + policy: providerProfilePolicyWithAlsoAllow, + label: providerProfile + ? `tools.byProvider.profile (${providerProfile})` + : "tools.byProvider.profile", + stripPluginOnlyAllowlist: true, + }, + { policy: globalPolicy, label: "tools.allow", stripPluginOnlyAllowlist: true }, + { + policy: globalProviderPolicy, + label: "tools.byProvider.allow", + stripPluginOnlyAllowlist: true, + }, + { + policy: agentPolicy, + label: agentId ? `agents.${agentId}.tools.allow` : "agent tools.allow", + stripPluginOnlyAllowlist: true, + }, + { + policy: agentProviderPolicy, + label: agentId + ? `agents.${agentId}.tools.byProvider.allow` + : "agent tools.byProvider.allow", + stripPluginOnlyAllowlist: true, + }, + { policy: groupPolicy, label: "group tools.allow", stripPluginOnlyAllowlist: true }, + { policy: sandbox?.tools, label: "sandbox tools.allow" }, + { policy: subagentPolicy, label: "subagent tools.allow" }, + ], }); - 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. Use tools.alsoAllow for additive plugin tool enablement." - : "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( - profilePolicyWithAlsoAllow, - profile ? `tools.profile (${profile})` : "tools.profile", - ); - const providerProfileExpanded = resolvePolicy( - providerProfilePolicyWithAlsoAllow, - providerProfile ? `tools.byProvider.profile (${providerProfile})` : "tools.byProvider.profile", - ); - 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 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); - - const toolsFiltered = profilePolicyExpanded - ? filterToolsByPolicy(toolsByAuthorization, profilePolicyExpanded) - : toolsByAuthorization; - const providerProfileFiltered = providerProfileExpanded - ? filterToolsByPolicy(toolsFiltered, providerProfileExpanded) - : toolsFiltered; - const globalFiltered = globalPolicyExpanded - ? filterToolsByPolicy(providerProfileFiltered, globalPolicyExpanded) - : providerProfileFiltered; - const globalProviderFiltered = globalProviderExpanded - ? filterToolsByPolicy(globalFiltered, globalProviderExpanded) - : globalFiltered; - const agentFiltered = agentPolicyExpanded - ? filterToolsByPolicy(globalProviderFiltered, agentPolicyExpanded) - : globalProviderFiltered; - const agentProviderFiltered = agentProviderExpanded - ? filterToolsByPolicy(agentFiltered, agentProviderExpanded) - : agentFiltered; - const groupFiltered = groupPolicyExpanded - ? filterToolsByPolicy(agentProviderFiltered, groupPolicyExpanded) - : agentProviderFiltered; - const sandboxed = sandboxPolicyExpanded - ? filterToolsByPolicy(groupFiltered, sandboxPolicyExpanded) - : groupFiltered; - const subagentFiltered = subagentPolicyExpanded - ? filterToolsByPolicy(sandboxed, subagentPolicyExpanded) - : sandboxed; // Always normalize tool JSON Schemas before handing them to pi-agent/pi-ai. // Without this, some providers (notably OpenAI) will reject root-level union schemas. const normalized = subagentFiltered.map(normalizeToolParameters); diff --git a/src/agents/tool-policy-pipeline.test.ts b/src/agents/tool-policy-pipeline.test.ts new file mode 100644 index 0000000000..9d0a9d5846 --- /dev/null +++ b/src/agents/tool-policy-pipeline.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, test } from "vitest"; +import { applyToolPolicyPipeline } from "./tool-policy-pipeline.js"; + +type DummyTool = { name: string }; + +describe("tool-policy-pipeline", () => { + test("strips allowlists that would otherwise disable core tools", () => { + const tools = [{ name: "exec" }, { name: "plugin_tool" }] as unknown as DummyTool[]; + const filtered = applyToolPolicyPipeline({ + // oxlint-disable-next-line typescript/no-explicit-any + tools: tools as any, + // oxlint-disable-next-line typescript/no-explicit-any + toolMeta: (t: any) => (t.name === "plugin_tool" ? { pluginId: "foo" } : undefined), + warn: () => {}, + steps: [ + { + policy: { allow: ["plugin_tool"] }, + label: "tools.allow", + stripPluginOnlyAllowlist: true, + }, + ], + }); + const names = filtered.map((t) => (t as unknown as DummyTool).name).toSorted(); + expect(names).toEqual(["exec", "plugin_tool"]); + }); + + test("warns about unknown allowlist entries", () => { + const warnings: string[] = []; + const tools = [{ name: "exec" }] as unknown as DummyTool[]; + applyToolPolicyPipeline({ + // oxlint-disable-next-line typescript/no-explicit-any + tools: tools as any, + // oxlint-disable-next-line typescript/no-explicit-any + toolMeta: () => undefined, + warn: (msg) => warnings.push(msg), + steps: [ + { + policy: { allow: ["wat"] }, + label: "tools.allow", + stripPluginOnlyAllowlist: true, + }, + ], + }); + expect(warnings.length).toBe(1); + expect(warnings[0]).toContain("unknown entries (wat)"); + }); + + test("applies allowlist filtering when core tools are explicitly listed", () => { + const tools = [{ name: "exec" }, { name: "process" }] as unknown as DummyTool[]; + const filtered = applyToolPolicyPipeline({ + // oxlint-disable-next-line typescript/no-explicit-any + tools: tools as any, + // oxlint-disable-next-line typescript/no-explicit-any + toolMeta: () => undefined, + warn: () => {}, + steps: [ + { + policy: { allow: ["exec"] }, + label: "tools.allow", + stripPluginOnlyAllowlist: true, + }, + ], + }); + expect(filtered.map((t) => (t as unknown as DummyTool).name)).toEqual(["exec"]); + }); +}); diff --git a/src/agents/tool-policy-pipeline.ts b/src/agents/tool-policy-pipeline.ts new file mode 100644 index 0000000000..b4a4dfda93 --- /dev/null +++ b/src/agents/tool-policy-pipeline.ts @@ -0,0 +1,60 @@ +import type { AnyAgentTool } from "./pi-tools.types.js"; +import { filterToolsByPolicy } from "./pi-tools.policy.js"; +import { + buildPluginToolGroups, + expandPolicyWithPluginGroups, + normalizeToolName, + stripPluginOnlyAllowlist, + type ToolPolicyLike, +} from "./tool-policy.js"; + +export type ToolPolicyPipelineStep = { + policy: ToolPolicyLike | undefined; + label: string; + stripPluginOnlyAllowlist?: boolean; +}; + +export function applyToolPolicyPipeline(params: { + tools: AnyAgentTool[]; + toolMeta: (tool: AnyAgentTool) => { pluginId: string } | undefined; + warn: (message: string) => void; + steps: ToolPolicyPipelineStep[]; +}): AnyAgentTool[] { + const coreToolNames = new Set( + params.tools + .filter((tool) => !params.toolMeta(tool)) + .map((tool) => normalizeToolName(tool.name)) + .filter(Boolean), + ); + + const pluginGroups = buildPluginToolGroups({ + tools: params.tools, + toolMeta: params.toolMeta, + }); + + let filtered = params.tools; + for (const step of params.steps) { + if (!step.policy) { + continue; + } + + let policy: ToolPolicyLike | undefined = step.policy; + if (step.stripPluginOnlyAllowlist) { + 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. Use tools.alsoAllow for additive plugin tool enablement." + : "These entries won't match any tool unless the plugin is enabled."; + params.warn( + `tools: ${step.label} allowlist contains unknown entries (${entries}). ${suffix}`, + ); + } + policy = resolved.policy; + } + + const expanded = expandPolicyWithPluginGroups(policy, pluginGroups); + filtered = expanded ? filterToolsByPolicy(filtered, expanded) : filtered; + } + return filtered; +} diff --git a/src/gateway/tools-invoke-http.ts b/src/gateway/tools-invoke-http.ts index f85c109de3..6413a0d6b5 100644 --- a/src/gateway/tools-invoke-http.ts +++ b/src/gateway/tools-invoke-http.ts @@ -2,19 +2,12 @@ import type { IncomingMessage, ServerResponse } from "node:http"; import type { AuthRateLimiter } from "./auth-rate-limit.js"; import { createOpenClawTools } from "../agents/openclaw-tools.js"; import { - filterToolsByPolicy, resolveEffectiveToolPolicy, resolveGroupToolPolicy, resolveSubagentToolPolicy, } from "../agents/pi-tools.policy.js"; -import { - buildPluginToolGroups, - collectExplicitAllowlist, - expandPolicyWithPluginGroups, - normalizeToolName, - resolveToolProfilePolicy, - stripPluginOnlyAllowlist, -} from "../agents/tool-policy.js"; +import { applyToolPolicyPipeline } from "../agents/tool-policy-pipeline.js"; +import { collectExplicitAllowlist, resolveToolProfilePolicy } from "../agents/tool-policy.js"; import { ToolInputError } from "../agents/tools/common.js"; import { loadConfig } from "../config/config.js"; import { resolveMainSessionKey } from "../config/sessions.js"; @@ -259,74 +252,47 @@ export async function handleToolsInvokeHttpRequest( ]), }); - const coreToolNames = new Set( - allTools - // oxlint-disable-next-line typescript/no-explicit-any - .filter((tool) => !getPluginToolMeta(tool as any)) - .map((tool) => normalizeToolName(tool.name)) - .filter(Boolean), - ); - const pluginGroups = buildPluginToolGroups({ - tools: allTools, + const subagentFiltered = applyToolPolicyPipeline({ + // oxlint-disable-next-line typescript/no-explicit-any + tools: allTools as any, // oxlint-disable-next-line typescript/no-explicit-any toolMeta: (tool) => getPluginToolMeta(tool as any), + warn: logWarn, + steps: [ + { + policy: profilePolicyWithAlsoAllow, + label: profile ? `tools.profile (${profile})` : "tools.profile", + stripPluginOnlyAllowlist: true, + }, + { + policy: providerProfilePolicyWithAlsoAllow, + label: providerProfile + ? `tools.byProvider.profile (${providerProfile})` + : "tools.byProvider.profile", + stripPluginOnlyAllowlist: true, + }, + { policy: globalPolicy, label: "tools.allow", stripPluginOnlyAllowlist: true }, + { + policy: globalProviderPolicy, + label: "tools.byProvider.allow", + stripPluginOnlyAllowlist: true, + }, + { + policy: agentPolicy, + label: agentId ? `agents.${agentId}.tools.allow` : "agent tools.allow", + stripPluginOnlyAllowlist: true, + }, + { + policy: agentProviderPolicy, + label: agentId + ? `agents.${agentId}.tools.byProvider.allow` + : "agent tools.byProvider.allow", + stripPluginOnlyAllowlist: true, + }, + { policy: groupPolicy, label: "group tools.allow", stripPluginOnlyAllowlist: true }, + { policy: subagentPolicy, label: "subagent tools.allow" }, + ], }); - 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. Use tools.alsoAllow for additive plugin tool enablement." - : "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( - profilePolicyWithAlsoAllow, - profile ? `tools.profile (${profile})` : "tools.profile", - ); - const providerProfileExpanded = resolvePolicy( - providerProfilePolicyWithAlsoAllow, - providerProfile ? `tools.byProvider.profile (${providerProfile})` : "tools.byProvider.profile", - ); - 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 agentProviderExpanded = resolvePolicy( - agentProviderPolicy, - agentId ? `agents.${agentId}.tools.byProvider.allow` : "agent tools.byProvider.allow", - ); - const groupPolicyExpanded = resolvePolicy(groupPolicy, "group tools.allow"); - const subagentPolicyExpanded = expandPolicyWithPluginGroups(subagentPolicy, pluginGroups); - - const toolsFiltered = profilePolicyExpanded - ? filterToolsByPolicy(allTools, profilePolicyExpanded) - : allTools; - const providerProfileFiltered = providerProfileExpanded - ? filterToolsByPolicy(toolsFiltered, providerProfileExpanded) - : toolsFiltered; - const globalFiltered = globalPolicyExpanded - ? filterToolsByPolicy(providerProfileFiltered, globalPolicyExpanded) - : providerProfileFiltered; - const globalProviderFiltered = globalProviderExpanded - ? filterToolsByPolicy(globalFiltered, globalProviderExpanded) - : globalFiltered; - const agentFiltered = agentPolicyExpanded - ? filterToolsByPolicy(globalProviderFiltered, agentPolicyExpanded) - : globalProviderFiltered; - const agentProviderFiltered = agentProviderExpanded - ? filterToolsByPolicy(agentFiltered, agentProviderExpanded) - : agentFiltered; - const groupFiltered = groupPolicyExpanded - ? filterToolsByPolicy(agentProviderFiltered, groupPolicyExpanded) - : agentProviderFiltered; - const subagentFiltered = subagentPolicyExpanded - ? filterToolsByPolicy(groupFiltered, subagentPolicyExpanded) - : groupFiltered; // Gateway HTTP-specific deny list — applies to ALL sessions via HTTP. const gatewayToolsCfg = cfg.gateway?.tools;