fix(security): stop implicit tool grants from config sections (#47487) (#75055)

* 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
This commit is contained in:
Alex Knight
2026-04-30 22:19:26 +10:00
committed by GitHub
parent 58a0b077c1
commit 4aa08e9d79
6 changed files with 109 additions and 28 deletions

View File

@@ -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.

View File

@@ -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"]);
});
});

View File

@@ -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

View File

@@ -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);
});

View File

@@ -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,

View File

@@ -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 }) => {