fix(policy): preserve restrictive tool allowlists (#58476)

* fix(policy): preserve restrictive tool allowlists

Co-authored-by: David Silva <david.silva@gendigital.com>

* fix(policy): address review follow-ups

* fix(policy): restore additive alsoAllow semantics

* fix(policy): preserve optional tool opt-ins for allow-all configs

* fix(policy): narrow plugin-only allowlist warnings

* fix(policy): add changelog entry

* Revert "fix(policy): add changelog entry"

This reverts commit 4a996bf4caedfe8c9ff3a7f190816e657ead5d10.

* chore: add changelog for restrictive tool allowlists

---------

Co-authored-by: David Silva <david.silva@gendigital.com>
Co-authored-by: Devin Robison <drobison@nvidia.com>

CHANGELOG.md

@@ -26,6 +26,7 @@ Docs: https://docs.openclaw.ai
 - Exec/Windows: prefer strict-inline-eval denial over generic allowlist prompts for interpreter carriers, while keeping persisted Windows allow-always approvals argv-bound. (#59780) Thanks @luoyanglang.
 - Gateway/connect: omit admin-scoped config and auth metadata from lower-privilege `hello-ok` snapshots while preserving those fields for admin reconnects. (#58469) Thanks @eleqtrizit.
 - iOS/canvas: restrict A2UI bridge trust to the bundled scaffold and exact capability-backed remote canvas URLs, so generic `canvas.navigate` and `canvas.present` loads no longer gain action-dispatch authority. (#58471) Thanks @eleqtrizit.
+- Agents/tool policy: preserve restrictive plugin-only allowlists instead of silently widening access to core tools, and keep allowlist warnings aligned with the enforced policy. (#58476) Thanks @eleqtrizit.
 
 ## 2026.4.2
 

src/agents/sandbox-tool-policy.test.ts

@@ -0,0 +1,80 @@
+import { describe, expect, it } from "vitest";
+import type { OpenClawConfig } from "../config/config.js";
+import { resolveEffectiveToolPolicy } from "./pi-tools.policy.js";
+import { pickSandboxToolPolicy } from "./sandbox-tool-policy.js";
+import { resolveEffectiveToolFsRootExpansionAllowed } from "./tool-fs-policy.js";
+
+describe("pickSandboxToolPolicy", () => {
+  it("returns undefined when neither allow nor deny is configured", () => {
+    expect(pickSandboxToolPolicy({})).toBeUndefined();
+  });
+
+  it("keeps alsoAllow without allow additive", () => {
+    expect(
+      pickSandboxToolPolicy({
+        alsoAllow: ["web_search"],
+      }),
+    ).toEqual({
+      allow: ["*", "web_search"],
+      deny: undefined,
+    });
+  });
+
+  it("merges allow and alsoAllow when both are present", () => {
+    expect(
+      pickSandboxToolPolicy({
+        allow: ["read"],
+        alsoAllow: ["write"],
+      }),
+    ).toEqual({
+      allow: ["read", "write"],
+      deny: undefined,
+    });
+  });
+
+  it("preserves allow-all semantics for allow: [] plus alsoAllow", () => {
+    expect(
+      pickSandboxToolPolicy({
+        allow: [],
+        alsoAllow: ["web_search"],
+      }),
+    ).toEqual({
+      allow: ["*", "web_search"],
+      deny: undefined,
+    });
+  });
+
+  it("passes deny through unchanged", () => {
+    expect(
+      pickSandboxToolPolicy({
+        deny: ["exec"],
+      }),
+    ).toEqual({
+      allow: undefined,
+      deny: ["exec"],
+    });
+  });
+
+  it("keeps global alsoAllow additive in effective tool policy resolution", () => {
+    const cfg: OpenClawConfig = {
+      tools: {
+        profile: "coding",
+        alsoAllow: ["lobster"],
+      },
+    };
+
+    const resolved = resolveEffectiveToolPolicy({ config: cfg, agentId: "main" });
+    expect(resolved.globalPolicy).toEqual({ allow: ["*", "lobster"], deny: undefined });
+    expect(resolved.profileAlsoAllow).toEqual(["lobster"]);
+  });
+
+  it("does not block fs root expansion when only global alsoAllow is configured", () => {
+    const cfg: OpenClawConfig = {
+      tools: {
+        alsoAllow: ["lobster"],
+      },
+    };
+
+    expect(resolveEffectiveToolFsRootExpansionAllowed({ cfg, agentId: "main" })).toBe(true);
+  });
+});

src/agents/sandbox-tool-policy.ts

@@ -10,9 +10,10 @@ function unionAllow(base?: string[], extra?: string[]): string[] | undefined {
   if (!Array.isArray(extra) || extra.length === 0) {
     return base;
   }
-  // If the user is using alsoAllow without an allowlist, treat it as additive on top of
-  // an implicit allow-all policy.
-  if (!Array.isArray(base) || base.length === 0) {
+  if (!Array.isArray(base)) {
+    return Array.from(new Set(["*", ...extra]));
+  }
+  if (base.length === 0) {
     return Array.from(new Set(["*", ...extra]));
   }
   return Array.from(new Set([...base, ...extra]));

src/agents/tool-policy-pipeline.test.ts

@@ -36,7 +36,7 @@ describe("tool-policy-pipeline", () => {
     resetToolPolicyWarningCacheForTest();
   });
 
-  test("strips allowlists that would otherwise disable core tools", () => {
+  test("preserves plugin-only allowlists instead of silently stripping them", () => {
     const tools = [{ name: "exec" }, { name: "plugin_tool" }] as unknown as DummyTool[];
     const filtered = applyToolPolicyPipeline({
       // oxlint-disable-next-line typescript/no-explicit-any
@@ -53,7 +53,7 @@ describe("tool-policy-pipeline", () => {
       ],
     });
     const names = filtered.map((t) => (t as unknown as DummyTool).name).toSorted();
-    expect(names).toEqual(["exec", "plugin_tool"]);
+    expect(names).toEqual(["plugin_tool"]);
   });
 
   test("warns about unknown allowlist entries", () => {
@@ -109,6 +109,7 @@ describe("tool-policy-pipeline", () => {
     expect(warnings[0]).toContain(
       "shipped core tools but unavailable in the current runtime/provider/model/config",
     );
+    expect(warnings[0]).not.toContain("Allowlist contains only plugin entries");
     expect(warnings[0]).not.toContain("unless the plugin is enabled");
   });
 
@@ -175,6 +176,58 @@ describe("tool-policy-pipeline", () => {
     expect(warnings).toHaveLength(258);
   });
 
+  test("evicts the oldest warning when the dedupe cache is full", () => {
+    const warnings: string[] = [];
+    const tools = [{ name: "exec" }] as unknown as DummyTool[];
+
+    for (let i = 0; i < 256; i += 1) {
+      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: string) => warnings.push(msg),
+        steps: [
+          {
+            policy: { allow: [`unknown_${i}`] },
+            label: "tools.allow",
+            stripPluginOnlyAllowlist: true,
+          },
+        ],
+      });
+    }
+
+    warnings.length = 0;
+
+    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: string) => warnings.push(msg),
+      steps: [
+        {
+          policy: { allow: ["unknown_256"] },
+          label: "tools.allow",
+          stripPluginOnlyAllowlist: true,
+        },
+      ],
+    });
+    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: string) => warnings.push(msg),
+      steps: [
+        { policy: { allow: ["unknown_0"] }, label: "tools.allow", stripPluginOnlyAllowlist: true },
+      ],
+    });
+
+    expect(warnings).toHaveLength(2);
+    expect(warnings[1]).toContain("unknown_0");
+  });
+
   test("applies allowlist filtering when core tools are explicitly listed", () => {
     const tools = [{ name: "exec" }, { name: "process" }] as unknown as DummyTool[];
     const filtered = applyToolPolicyPipeline({
@@ -193,4 +246,23 @@ describe("tool-policy-pipeline", () => {
     });
     expect(filtered.map((t) => (t as unknown as DummyTool).name)).toEqual(["exec"]);
   });
+
+  test("applies deny filtering after allow filtering", () => {
+    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", "process"], deny: ["process"] },
+          label: "tools.allow",
+          stripPluginOnlyAllowlist: true,
+        },
+      ],
+    });
+    expect(filtered.map((t) => (t as unknown as DummyTool).name)).toEqual(["exec"]);
+  });
 });

src/agents/tool-policy-pipeline.ts

@@ -2,27 +2,29 @@ import { filterToolsByPolicy } from "./pi-tools.policy.js";
 import type { AnyAgentTool } from "./pi-tools.types.js";
 import { isKnownCoreToolId } from "./tool-catalog.js";
 import {
+  analyzeAllowlistByToolType,
   buildPluginToolGroups,
   expandPolicyWithPluginGroups,
   normalizeToolName,
-  stripPluginOnlyAllowlist,
   type ToolPolicyLike,
 } from "./tool-policy.js";
 
 const MAX_TOOL_POLICY_WARNING_CACHE = 256;
 const seenToolPolicyWarnings = new Set<string>();
+const toolPolicyWarningOrder: string[] = [];
 
 function rememberToolPolicyWarning(warning: string): boolean {
   if (seenToolPolicyWarnings.has(warning)) {
     return false;
   }
   if (seenToolPolicyWarnings.size >= MAX_TOOL_POLICY_WARNING_CACHE) {
-    const oldest = seenToolPolicyWarnings.values().next().value;
+    const oldest = toolPolicyWarningOrder.shift();
     if (oldest) {
       seenToolPolicyWarnings.delete(oldest);
     }
   }
   seenToolPolicyWarnings.add(warning);
+  toolPolicyWarningOrder.push(warning);
   return true;
 }
 
@@ -114,7 +116,7 @@ export function applyToolPolicyPipeline(params: {
 
     let policy: ToolPolicyLike | undefined = step.policy;
     if (step.stripPluginOnlyAllowlist) {
-      const resolved = stripPluginOnlyAllowlist(policy, pluginGroups, coreToolNames);
+      const resolved = analyzeAllowlistByToolType(policy, pluginGroups, coreToolNames);
       if (resolved.unknownAllowlist.length > 0) {
         const entries = resolved.unknownAllowlist.join(", ");
         const gatedCoreEntries = resolved.unknownAllowlist.filter((entry) =>
@@ -122,14 +124,14 @@ export function applyToolPolicyPipeline(params: {
         );
         const otherEntries = resolved.unknownAllowlist.filter((entry) => !isKnownCoreToolId(entry));
         if (
-          !shouldSuppressUnavailableCoreToolWarning({
-            suppressUnavailableCoreToolWarning: step.suppressUnavailableCoreToolWarning === true,
+          shouldWarnAboutUnknownAllowlist({
+            suppressUnavailableCoreToolWarning: step.suppressUnavailableCoreToolWarning ?? false,
             hasGatedCoreEntries: gatedCoreEntries.length > 0,
             hasOtherEntries: otherEntries.length > 0,
           })
         ) {
           const suffix = describeUnknownAllowlistSuffix({
-            strippedAllowlist: resolved.strippedAllowlist,
+            pluginOnlyAllowlist: resolved.pluginOnlyAllowlist,
             hasGatedCoreEntries: gatedCoreEntries.length > 0,
             hasOtherEntries: otherEntries.length > 0,
           });
@@ -148,7 +150,7 @@ export function applyToolPolicyPipeline(params: {
   return filtered;
 }
 
-function shouldSuppressUnavailableCoreToolWarning(params: {
+function shouldWarnAboutUnknownAllowlist(params: {
   suppressUnavailableCoreToolWarning: boolean;
   hasGatedCoreEntries: boolean;
   hasOtherEntries: boolean;
@@ -158,18 +160,18 @@ function shouldSuppressUnavailableCoreToolWarning(params: {
     !params.hasGatedCoreEntries ||
     params.hasOtherEntries
   ) {
-    return false;
+    return true;
   }
-  return true;
+  return false;
 }
 
 function describeUnknownAllowlistSuffix(params: {
-  strippedAllowlist: boolean;
+  pluginOnlyAllowlist: boolean;
   hasGatedCoreEntries: boolean;
   hasOtherEntries: boolean;
 }): string {
-  const preface = params.strippedAllowlist
-    ? "Ignoring allowlist so core tools remain available."
+  const preface = params.pluginOnlyAllowlist
+    ? "Allowlist contains only plugin entries; core tools will not be available."
     : "";
   const detail =
     params.hasGatedCoreEntries && params.hasOtherEntries
@@ -182,4 +184,5 @@ function describeUnknownAllowlistSuffix(params: {
 
 export function resetToolPolicyWarningCacheForTest(): void {
   seenToolPolicyWarnings.clear();
+  toolPolicyWarningOrder.length = 0;
 }

src/agents/tool-policy.plugin-only-allowlist.test.ts

@@ -1,5 +1,9 @@
 import { describe, expect, it } from "vitest";
-import { stripPluginOnlyAllowlist, type PluginToolGroups } from "./tool-policy.js";
+import {
+  analyzeAllowlistByToolType,
+  buildPluginToolGroups,
+  type PluginToolGroups,
+} from "./tool-policy.js";
 
 const pluginGroups: PluginToolGroups = {
   all: ["lobster", "workflow_tool"],
@@ -7,27 +11,33 @@ const pluginGroups: PluginToolGroups = {
 };
 const coreTools = new Set(["read", "write", "exec", "session_status"]);
 
-describe("stripPluginOnlyAllowlist", () => {
-  it("strips allowlist when it only targets plugin tools", () => {
-    const policy = stripPluginOnlyAllowlist({ allow: ["lobster"] }, pluginGroups, coreTools);
-    expect(policy.policy?.allow).toBeUndefined();
+describe("analyzeAllowlistByToolType", () => {
+  it("preserves allowlist when it only targets plugin tools", () => {
+    const policy = analyzeAllowlistByToolType({ allow: ["lobster"] }, pluginGroups, coreTools);
+    expect(policy.policy?.allow).toEqual(["lobster"]);
+    expect(policy.pluginOnlyAllowlist).toBe(true);
     expect(policy.unknownAllowlist).toEqual([]);
   });
 
-  it("strips allowlist when it only targets plugin groups", () => {
-    const policy = stripPluginOnlyAllowlist({ allow: ["group:plugins"] }, pluginGroups, coreTools);
-    expect(policy.policy?.allow).toBeUndefined();
+  it("preserves allowlist when it only targets plugin groups", () => {
+    const policy = analyzeAllowlistByToolType(
+      { allow: ["group:plugins"] },
+      pluginGroups,
+      coreTools,
+    );
+    expect(policy.policy?.allow).toEqual(["group:plugins"]);
+    expect(policy.pluginOnlyAllowlist).toBe(true);
     expect(policy.unknownAllowlist).toEqual([]);
   });
 
   it('keeps allowlist when it uses "*"', () => {
-    const policy = stripPluginOnlyAllowlist({ allow: ["*"] }, pluginGroups, coreTools);
+    const policy = analyzeAllowlistByToolType({ allow: ["*"] }, pluginGroups, coreTools);
     expect(policy.policy?.allow).toEqual(["*"]);
     expect(policy.unknownAllowlist).toEqual([]);
   });
 
   it("keeps allowlist when it mixes plugin and core entries", () => {
-    const policy = stripPluginOnlyAllowlist(
+    const policy = analyzeAllowlistByToolType(
       { allow: ["lobster", "read"] },
       pluginGroups,
       coreTools,
@@ -36,16 +46,17 @@ describe("stripPluginOnlyAllowlist", () => {
     expect(policy.unknownAllowlist).toEqual([]);
   });
 
-  it("strips allowlist with unknown entries when no core tools match", () => {
+  it("preserves allowlist with unknown entries when no core tools match", () => {
     const emptyPlugins: PluginToolGroups = { all: [], byPlugin: new Map() };
-    const policy = stripPluginOnlyAllowlist({ allow: ["lobster"] }, emptyPlugins, coreTools);
-    expect(policy.policy?.allow).toBeUndefined();
+    const policy = analyzeAllowlistByToolType({ allow: ["lobster"] }, emptyPlugins, coreTools);
+    expect(policy.policy?.allow).toEqual(["lobster"]);
+    expect(policy.pluginOnlyAllowlist).toBe(false);
     expect(policy.unknownAllowlist).toEqual(["lobster"]);
   });
 
   it("keeps allowlist with core tools and reports unknown entries", () => {
     const emptyPlugins: PluginToolGroups = { all: [], byPlugin: new Map() };
-    const policy = stripPluginOnlyAllowlist(
+    const policy = analyzeAllowlistByToolType(
       { allow: ["read", "lobster"] },
       emptyPlugins,
       coreTools,
@@ -53,4 +64,19 @@ describe("stripPluginOnlyAllowlist", () => {
     expect(policy.policy?.allow).toEqual(["read", "lobster"]);
     expect(policy.unknownAllowlist).toEqual(["lobster"]);
   });
+
+  it("does not mark unavailable core entries as plugin-only", () => {
+    const policy = analyzeAllowlistByToolType({ allow: ["apply_patch"] }, pluginGroups, coreTools);
+    expect(policy.pluginOnlyAllowlist).toBe(false);
+    expect(policy.unknownAllowlist).toEqual(["apply_patch"]);
+  });
+
+  it("ignores empty plugin ids when building groups", () => {
+    const groups = buildPluginToolGroups({
+      tools: [{ name: "lobster" }],
+      toolMeta: () => ({ pluginId: "" }),
+    });
+    expect(groups.all).toEqual(["lobster"]);
+    expect(groups.byPlugin.size).toBe(0);
+  });
 });

src/agents/tool-policy.test.ts

@@ -6,6 +6,7 @@ import type { SandboxToolPolicy } from "./sandbox/types.js";
 import { TOOL_POLICY_CONFORMANCE } from "./tool-policy.conformance.js";
 import {
   applyOwnerOnlyToolPolicy,
+  collectExplicitAllowlist,
   expandToolGroups,
   isOwnerOnlyToolName,
   normalizeToolName,
@@ -133,6 +134,16 @@ describe("tool-policy", () => {
     expect(applyOwnerOnlyToolPolicy(tools, true)).toHaveLength(1);
   });
 
+  it("preserves explicit alsoAllow hints when allow is empty", () => {
+    expect(
+      collectExplicitAllowlist([
+        {
+          allow: ["*", "optional-demo"],
+        },
+      ]),
+    ).toContain("optional-demo");
+  });
+
   it("strips nodes for non-owner senders via fallback policy", () => {
     const tools = [
       {

src/agents/tool-policy.ts

@@ -77,7 +77,7 @@ export type PluginToolGroups = {
 export type AllowlistResolution = {
   policy: ToolPolicyLike | undefined;
   unknownAllowlist: string[];
-  strippedAllowlist: boolean;
+  pluginOnlyAllowlist: boolean;
 };
 
 export function collectExplicitAllowlist(policies: Array<ToolPolicyLike | undefined>): string[] {
@@ -112,7 +112,10 @@ export function buildPluginToolGroups<T extends { name: string }>(params: {
     }
     const name = normalizeToolName(tool.name);
     all.push(name);
-    const pluginId = meta.pluginId.toLowerCase();
+    const pluginId = meta.pluginId.trim().toLowerCase();
+    if (!pluginId) {
+      continue;
+    }
     const list = byPlugin.get(pluginId) ?? [];
     list.push(name);
     byPlugin.set(pluginId, list);
@@ -161,49 +164,43 @@ export function expandPolicyWithPluginGroups(
   };
 }
 
-export function stripPluginOnlyAllowlist(
+export function analyzeAllowlistByToolType(
   policy: ToolPolicyLike | undefined,
   groups: PluginToolGroups,
   coreTools: Set<string>,
 ): AllowlistResolution {
   if (!policy?.allow || policy.allow.length === 0) {
-    return { policy, unknownAllowlist: [], strippedAllowlist: false };
+    return { policy, unknownAllowlist: [], pluginOnlyAllowlist: false };
   }
   const normalized = normalizeToolList(policy.allow);
   if (normalized.length === 0) {
-    return { policy, unknownAllowlist: [], strippedAllowlist: false };
+    return { policy, unknownAllowlist: [], pluginOnlyAllowlist: false };
   }
   const pluginIds = new Set(groups.byPlugin.keys());
   const pluginTools = new Set(groups.all);
   const unknownAllowlist: string[] = [];
-  let hasCoreEntry = false;
+  let hasOnlyPluginEntries = true;
   for (const entry of normalized) {
     if (entry === "*") {
-      hasCoreEntry = true;
+      hasOnlyPluginEntries = false;
       continue;
     }
     const isPluginEntry =
       entry === "group:plugins" || pluginIds.has(entry) || pluginTools.has(entry);
     const expanded = expandToolGroups([entry]);
     const isCoreEntry = expanded.some((tool) => coreTools.has(tool));
-    if (isCoreEntry) {
-      hasCoreEntry = true;
+    if (!isPluginEntry) {
+      hasOnlyPluginEntries = false;
     }
     if (!isCoreEntry && !isPluginEntry) {
       unknownAllowlist.push(entry);
     }
   }
-  const strippedAllowlist = !hasCoreEntry;
-  // When an allowlist contains only plugin tools, we strip it to avoid accidentally
-  // disabling core tools. Users who want additive behavior should prefer `tools.alsoAllow`.
-  if (strippedAllowlist) {
-    // Note: logging happens in the caller (pi-tools/tools-invoke) after this function returns.
-    // We keep this note here for future maintainers.
-  }
+  const pluginOnlyAllowlist = hasOnlyPluginEntries;
   return {
-    policy: strippedAllowlist ? { ...policy, allow: undefined } : policy,
+    policy,
     unknownAllowlist: Array.from(new Set(unknownAllowlist)),
-    strippedAllowlist,
+    pluginOnlyAllowlist,
   };
 }