From 4aa08e9d79466323d6f259f42123937e8f925a09 Mon Sep 17 00:00:00 2001 From: Alex Knight Date: Thu, 30 Apr 2026 22:19:26 +1000 Subject: [PATCH] fix(security): stop implicit tool grants from config sections (#47487) (#75055) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(security): stop implicit tool grants from config sections (#47487) Configured tool sections (tools.exec, tools.fs) no longer implicitly widen restrictive profiles (messaging, minimal). Previously, having a tools.exec section anywhere in config — even just safety settings like security: "allowlist" — would automatically add exec and process to the profile's allowed tools, defeating the purpose of the restrictive profile. The same pattern existed in tool-fs-policy.ts where tools.fs presence would add read/write/edit to the profile allowlist for root expansion. Changes: - pi-tools.policy.ts: Stop merging implicit grants into profileAlsoAllow. Renamed resolveImplicitProfileAlsoAllow → detectImplicitProfileGrants and use it only for a startup warning that tells users to add explicit alsoAllow entries. - tool-fs-policy.ts: Remove the implicit read/write/edit grant from resolveEffectiveToolFsRootExpansionAllowed when tools.fs is present. Root expansion now requires actual read access via profile or alsoAllow. - Updated 4 existing tests and added 3 new regression tests. Migration: users who relied on tools.exec or tools.fs implicitly granting access under a restrictive profile should add explicit alsoAllow entries: tools: profile: "messaging" alsoAllow: ["exec", "process"] # was implicit, now required exec: { security: "allowlist" } Fixes #47487 * fix: address tool policy review feedback --- CHANGELOG.md | 1 + src/agents/pi-tools.policy.test.ts | 51 +++++++++++++++++++++++++----- src/agents/pi-tools.policy.ts | 47 +++++++++++++++++++++------ src/agents/tool-fs-policy.test.ts | 17 ++++++++-- src/agents/tool-fs-policy.ts | 7 +--- src/media/local-roots.test.ts | 14 +++++++- 6 files changed, 109 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e8f3ac1868..6503968e174 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Docs: https://docs.openclaw.ai ### Changes +- Security/tools: configured tool sections (`tools.exec`, `tools.fs`) no longer implicitly widen restrictive profiles (`messaging`, `minimal`). Users who need those tools under a restricted profile must add explicit `alsoAllow` entries; a startup warning identifies affected configs. Fixes #47487. Thanks @amknight. - Agents/commitments: add opt-in inferred follow-up commitments with hidden batched extraction, per-agent/per-channel scoping, heartbeat delivery, CLI management, a simple `commitments.enabled`/`commitments.maxPerDay` config, and heartbeat-interval due-time clamping so magical check-ins do not echo immediately. (#74189) Thanks @vignesh07. - Messages/queue: make `steer` drain all pending Pi steering messages at the next model boundary, keep legacy one-at-a-time steering as `queue`, and add a dedicated steering queue docs page. Thanks @vincentkoc. - Messages/queue: default active-run queueing to `steer` with a 500ms followup fallback debounce, and document the queue modes, precedence, and drop policies on the command queue page. Thanks @vincentkoc. diff --git a/src/agents/pi-tools.policy.test.ts b/src/agents/pi-tools.policy.test.ts index 8a121cd3537..ef107d4e810 100644 --- a/src/agents/pi-tools.policy.test.ts +++ b/src/agents/pi-tools.policy.test.ts @@ -504,7 +504,7 @@ describe("resolveEffectiveToolPolicy", () => { ).toBeUndefined(); }); - it("implicitly re-exposes exec and process when tools.exec is configured", () => { + it("does not implicitly re-expose exec when tools.exec is configured (#47487)", () => { const cfg = { tools: { profile: "messaging", @@ -512,10 +512,10 @@ describe("resolveEffectiveToolPolicy", () => { }, } as OpenClawConfig; const result = resolveEffectiveToolPolicy({ config: cfg }); - expect(result.profileAlsoAllow).toEqual(["exec", "process"]); + expect(result.profileAlsoAllow).toBeUndefined(); }); - it("implicitly re-exposes read, write, and edit when tools.fs is configured", () => { + it("does not implicitly re-expose fs tools when tools.fs is configured (#47487)", () => { const cfg = { tools: { profile: "messaging", @@ -523,10 +523,10 @@ describe("resolveEffectiveToolPolicy", () => { }, } as OpenClawConfig; const result = resolveEffectiveToolPolicy({ config: cfg }); - expect(result.profileAlsoAllow).toEqual(["read", "write", "edit"]); + expect(result.profileAlsoAllow).toBeUndefined(); }); - it("merges explicit alsoAllow with implicit tool-section exposure", () => { + it("explicit alsoAllow works without implicit widening (#47487)", () => { const cfg = { tools: { profile: "messaging", @@ -535,10 +535,10 @@ describe("resolveEffectiveToolPolicy", () => { }, } as OpenClawConfig; const result = resolveEffectiveToolPolicy({ config: cfg }); - expect(result.profileAlsoAllow).toEqual(["web_search", "exec", "process"]); + expect(result.profileAlsoAllow).toEqual(["web_search"]); }); - it("uses agent tool sections when resolving implicit exposure", () => { + it("does not implicitly re-expose fs tools from agent tool sections (#47487)", () => { const cfg = { tools: { profile: "messaging", @@ -555,6 +555,41 @@ describe("resolveEffectiveToolPolicy", () => { }, } as OpenClawConfig; const result = resolveEffectiveToolPolicy({ config: cfg, agentId: "coder" }); - expect(result.profileAlsoAllow).toEqual(["read", "write", "edit"]); + expect(result.profileAlsoAllow).toBeUndefined(); + }); + + it("global tools.exec does not widen agent messaging profile (#47487)", () => { + const cfg = { + tools: { + exec: { security: "allowlist" }, + }, + agents: { + list: [ + { + id: "messenger", + tools: { + profile: "messaging", + alsoAllow: ["image"], + }, + }, + ], + }, + } as OpenClawConfig; + const result = resolveEffectiveToolPolicy({ config: cfg, agentId: "messenger" }); + expect(result.profileAlsoAllow).toEqual(["image"]); + expect(result.profileAlsoAllow).not.toContain("exec"); + expect(result.profileAlsoAllow).not.toContain("process"); + }); + + it("explicit alsoAllow with exec still grants exec under messaging profile", () => { + const cfg = { + tools: { + profile: "messaging", + alsoAllow: ["exec", "process"], + exec: { host: "sandbox" }, + }, + } as OpenClawConfig; + const result = resolveEffectiveToolPolicy({ config: cfg }); + expect(result.profileAlsoAllow).toEqual(["exec", "process"]); }); }); diff --git a/src/agents/pi-tools.policy.ts b/src/agents/pi-tools.policy.ts index 105e0f747d6..b3cc2791d8d 100644 --- a/src/agents/pi-tools.policy.ts +++ b/src/agents/pi-tools.policy.ts @@ -4,6 +4,7 @@ import { DEFAULT_SUBAGENT_MAX_SPAWN_DEPTH } from "../config/agent-limits.js"; import { resolveChannelGroupToolsPolicy } from "../config/group-policy.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { AgentToolsConfig } from "../config/types.tools.js"; +import { logWarn } from "../logger.js"; import { normalizeAgentId } from "../routing/session-key.js"; import { parseRawSessionConversationRef, @@ -26,7 +27,11 @@ import { type SubagentSessionRole, } from "./subagent-capabilities.js"; import { isToolAllowedByPolicies, isToolAllowedByPolicyName } from "./tool-policy-match.js"; -import { normalizeToolName } from "./tool-policy.js"; +import { + mergeAlsoAllowPolicy, + normalizeToolName, + resolveToolProfilePolicy, +} from "./tool-policy.js"; /** * Tools always denied for sub-agents regardless of depth. @@ -367,7 +372,9 @@ function hasExplicitToolSection(section: unknown): boolean { return section !== undefined && section !== null; } -function resolveImplicitProfileAlsoAllow(params: { +/** Detect tool config sections that previously widened profiles implicitly. + * Used only for migration warnings — not merged into profileAlsoAllow. #47487 */ +function detectImplicitProfileGrants(params: { globalTools?: OpenClawConfig["tools"]; agentTools?: AgentToolsConfig; }): string[] | undefined { @@ -422,13 +429,33 @@ export function resolveEffectiveToolPolicy(params: { }); const explicitProfileAlsoAllow = resolveExplicitProfileAlsoAllow(agentTools) ?? resolveExplicitProfileAlsoAllow(globalTools); - const implicitProfileAlsoAllow = resolveImplicitProfileAlsoAllow({ globalTools, agentTools }); - const profileAlsoAllow = - explicitProfileAlsoAllow || implicitProfileAlsoAllow - ? Array.from( - new Set([...(explicitProfileAlsoAllow ?? []), ...(implicitProfileAlsoAllow ?? [])]), - ) - : undefined; + + // Warn affected users about removed implicit grants (#47487), but only when + // the active profile/explicit alsoAllow do not already grant those tools. + if (profile) { + const implicitGrants = detectImplicitProfileGrants({ globalTools, agentTools }); + if (implicitGrants) { + const profilePolicy = mergeAlsoAllowPolicy( + resolveToolProfilePolicy(profile), + explicitProfileAlsoAllow, + ); + const uncovered = implicitGrants.filter( + (toolName) => !isToolAllowedByPolicyName(toolName, profilePolicy), + ); + if (uncovered.length > 0) { + logWarn( + `tools policy: profile "${profile}"${agentId ? ` (agent "${agentId}")` : ""} has ` + + `configured tool sections (tools.exec / tools.fs) that no longer implicitly widen ` + + `the profile. Add alsoAllow: [${uncovered.map((t) => `"${t}"`).join(", ")}] ` + + `explicitly if these tools should be available. See #47487.`, + ); + } + } + } + + const profileAlsoAllow = explicitProfileAlsoAllow + ? Array.from(new Set(explicitProfileAlsoAllow)) + : undefined; return { agentId, globalPolicy: pickSandboxToolPolicy(globalTools), @@ -437,7 +464,7 @@ export function resolveEffectiveToolPolicy(params: { agentProviderPolicy: pickSandboxToolPolicy(agentProviderPolicy), profile, providerProfile: agentProviderPolicy?.profile ?? providerPolicy?.profile, - // alsoAllow is applied at the profile stage (to avoid being filtered out early). + // alsoAllow is applied at the profile stage to avoid early filtering. profileAlsoAllow, providerProfileAlsoAllow: Array.isArray(agentProviderPolicy?.alsoAllow) ? agentProviderPolicy?.alsoAllow diff --git a/src/agents/tool-fs-policy.test.ts b/src/agents/tool-fs-policy.test.ts index 1c68481a042..b28e98c5abd 100644 --- a/src/agents/tool-fs-policy.test.ts +++ b/src/agents/tool-fs-policy.test.ts @@ -64,23 +64,34 @@ describe("resolveEffectiveToolFsRootExpansionAllowed", () => { expect(resolveEffectiveToolFsRootExpansionAllowed({ cfg, agentId: "main" })).toBe(false); }); - it("re-enables root expansion when tools.fs explicitly allows non-workspace reads", () => { + it("does not re-enable root expansion from tools.fs alone under messaging profile (#47487)", () => { const cfg: OpenClawConfig = { tools: { profile: "messaging", fs: { workspaceOnly: false }, }, }; - expect(resolveEffectiveToolFsRootExpansionAllowed({ cfg, agentId: "main" })).toBe(true); + expect(resolveEffectiveToolFsRootExpansionAllowed({ cfg, agentId: "main" })).toBe(false); }); - it("treats an explicit tools.fs block as a filesystem opt-in", () => { + it("does not treat an explicit tools.fs block as a filesystem opt-in (#47487)", () => { const cfg: OpenClawConfig = { tools: { profile: "messaging", fs: {}, }, }; + expect(resolveEffectiveToolFsRootExpansionAllowed({ cfg, agentId: "main" })).toBe(false); + }); + + it("re-enables root expansion when alsoAllow explicitly includes read (#47487)", () => { + const cfg: OpenClawConfig = { + tools: { + profile: "messaging", + alsoAllow: ["read"], + fs: { workspaceOnly: false }, + }, + }; expect(resolveEffectiveToolFsRootExpansionAllowed({ cfg, agentId: "main" })).toBe(true); }); diff --git a/src/agents/tool-fs-policy.ts b/src/agents/tool-fs-policy.ts index 42e90ce7ff9..5d23b1858d4 100644 --- a/src/agents/tool-fs-policy.ts +++ b/src/agents/tool-fs-policy.ts @@ -46,15 +46,10 @@ export function resolveEffectiveToolFsRootExpansionAllowed(params: { const profile = agentTools?.profile ?? globalTools?.profile; const profileAlsoAllow = new Set(agentTools?.alsoAllow ?? globalTools?.alsoAllow ?? []); const fsConfig = resolveToolFsConfig(params); - const hasExplicitFsConfig = agentTools?.fs !== undefined || globalTools?.fs !== undefined; if (fsConfig.workspaceOnly === true) { return false; } - if (hasExplicitFsConfig) { - profileAlsoAllow.add("read"); - profileAlsoAllow.add("write"); - profileAlsoAllow.add("edit"); - } + // tools.fs presence does not grant access; require profile or alsoAllow (#47487). const profilePolicy = mergeAlsoAllowPolicy( resolveToolProfilePolicy(profile), profileAlsoAllow.size > 0 ? Array.from(profileAlsoAllow) : undefined, diff --git a/src/media/local-roots.test.ts b/src/media/local-roots.test.ts index 25f617d909d..386f78a8ef8 100644 --- a/src/media/local-roots.test.ts +++ b/src/media/local-roots.test.ts @@ -154,7 +154,7 @@ describe("local media roots", () => { shouldContainPictures: false, }, { - name: "widens media roots again when messaging-profile agents explicitly enable filesystem tools", + name: "does not widen media roots when messaging-profile agents only configure filesystem guards", stateDir: path.join("/tmp", "openclaw-messaging-fs-media-roots-state"), cfg: { tools: { @@ -162,6 +162,18 @@ describe("local media roots", () => { fs: { workspaceOnly: false }, }, }, + shouldContainPictures: false, + }, + { + name: "widens media roots when messaging-profile agents explicitly allow reads", + stateDir: path.join("/tmp", "openclaw-messaging-read-media-roots-state"), + cfg: { + tools: { + profile: "messaging", + alsoAllow: ["read"] as string[], + fs: { workspaceOnly: false }, + }, + }, shouldContainPictures: true, }, ] as const)("$name", ({ stateDir, cfg, shouldContainPictures }) => {