refactor(tools): share tool policy pipeline

2026-02-19 18:39:20 -05:00 · 2026-02-14 14:11:40 +00:00
parent 4c74a2f06e
commit f97ad8f288
4 changed files with 204 additions and 146 deletions
--- 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);
--- a/src/agents/tool-policy-pipeline.test.ts
+++ 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"]);
+  });
+});
--- a/src/agents/tool-policy-pipeline.ts
+++ 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;
+}
--- 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;