mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 08:50:43 +00:00
* 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:
@@ -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.
|
||||
|
||||
@@ -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"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 }) => {
|
||||
|
||||
Reference in New Issue
Block a user