refactor: unify tool policy allow merging

This commit is contained in:
Peter Steinberger
2026-03-09 04:14:46 +00:00
parent 26e76f9a61
commit 1b837a6b24
9 changed files with 212 additions and 170 deletions

View File

@@ -187,7 +187,7 @@ describe("resolveEffectiveToolPolicy", () => {
},
} as OpenClawConfig;
const result = resolveEffectiveToolPolicy({ config: cfg });
expect(result.profileAlsoAllow).toEqual(["exec", "process"]);
expect(result.profiles.primary.alsoAllow).toEqual(["exec", "process"]);
});
it("implicitly re-exposes read, write, and edit when tools.fs is configured", () => {
@@ -198,7 +198,7 @@ describe("resolveEffectiveToolPolicy", () => {
},
} as OpenClawConfig;
const result = resolveEffectiveToolPolicy({ config: cfg });
expect(result.profileAlsoAllow).toEqual(["read", "write", "edit"]);
expect(result.profiles.primary.alsoAllow).toEqual(["read", "write", "edit"]);
});
it("merges explicit alsoAllow with implicit tool-section exposure", () => {
@@ -210,7 +210,7 @@ describe("resolveEffectiveToolPolicy", () => {
},
} as OpenClawConfig;
const result = resolveEffectiveToolPolicy({ config: cfg });
expect(result.profileAlsoAllow).toEqual(["web_search", "exec", "process"]);
expect(result.profiles.primary.alsoAllow).toEqual(["web_search", "exec", "process"]);
});
it("uses agent tool sections when resolving implicit exposure", () => {
@@ -230,6 +230,6 @@ describe("resolveEffectiveToolPolicy", () => {
},
} as OpenClawConfig;
const result = resolveEffectiveToolPolicy({ config: cfg, agentId: "coder" });
expect(result.profileAlsoAllow).toEqual(["read", "write", "edit"]);
expect(result.profiles.primary.alsoAllow).toEqual(["read", "write", "edit"]);
});
});

View File

@@ -2,7 +2,7 @@ import { getChannelDock } from "../channels/dock.js";
import { DEFAULT_SUBAGENT_MAX_SPAWN_DEPTH } from "../config/agent-limits.js";
import type { OpenClawConfig } from "../config/config.js";
import { resolveChannelGroupToolsPolicy } from "../config/group-policy.js";
import type { AgentToolsConfig } from "../config/types.tools.js";
import type { ToolPolicyConfig } from "../config/types.tools.js";
import { normalizeAgentId } from "../routing/session-key.js";
import { resolveThreadParentSessionKey } from "../sessions/session-key-utils.js";
import { normalizeMessageChannel } from "../utils/message-channel.js";
@@ -11,6 +11,11 @@ import { compileGlobPatterns, matchesAnyGlobPattern } from "./glob-pattern.js";
import type { AnyAgentTool } from "./pi-tools.types.js";
import { pickSandboxToolPolicy } from "./sandbox-tool-policy.js";
import type { SandboxToolPolicy } from "./sandbox.js";
import {
mergeAlsoAllowIntoAllowlist,
resolveProfileAlsoAllow,
resolveProviderProfileAlsoAllow,
} from "./tool-policy-also-allow.js";
import { expandToolGroups, normalizeToolName } from "./tool-policy.js";
function makeToolPolicyMatcher(policy: SandboxToolPolicy) {
@@ -99,7 +104,7 @@ export function resolveSubagentToolPolicy(cfg?: OpenClawConfig, depth?: number):
...baseDeny.filter((toolName) => !explicitAllow.has(normalizeToolName(toolName))),
...(Array.isArray(configured?.deny) ? configured.deny : []),
];
const mergedAllow = allow && alsoAllow ? Array.from(new Set([...allow, ...alsoAllow])) : allow;
const mergedAllow = mergeAlsoAllowIntoAllowlist({ allow, alsoAllow });
return { allow: mergedAllow, deny };
}
@@ -118,11 +123,9 @@ export function filterToolsByPolicy(tools: AnyAgentTool[], policy?: SandboxToolP
return tools.filter((tool) => matcher(tool.name));
}
type ToolPolicyConfig = {
allow?: string[];
export type ResolvedToolProfileScope = {
id?: string;
alsoAllow?: string[];
deny?: string[];
profile?: string;
};
function normalizeProviderKey(value: string): string {
@@ -197,36 +200,19 @@ function resolveProviderToolPolicy(params: {
return undefined;
}
function resolveExplicitProfileAlsoAllow(tools?: OpenClawConfig["tools"]): string[] | undefined {
return Array.isArray(tools?.alsoAllow) ? tools.alsoAllow : undefined;
}
function hasExplicitToolSection(section: unknown): boolean {
return section !== undefined && section !== null;
}
function resolveImplicitProfileAlsoAllow(params: {
globalTools?: OpenClawConfig["tools"];
agentTools?: AgentToolsConfig;
}): string[] | undefined {
const implicit = new Set<string>();
if (
hasExplicitToolSection(params.agentTools?.exec) ||
hasExplicitToolSection(params.globalTools?.exec)
) {
implicit.add("exec");
implicit.add("process");
}
if (
hasExplicitToolSection(params.agentTools?.fs) ||
hasExplicitToolSection(params.globalTools?.fs)
) {
implicit.add("read");
implicit.add("write");
implicit.add("edit");
}
return implicit.size > 0 ? Array.from(implicit) : undefined;
}
export type ResolvedToolPolicyContext = {
agentId?: string;
profiles: {
primary: ResolvedToolProfileScope;
provider: ResolvedToolProfileScope;
};
sandboxPolicies: {
global?: SandboxToolPolicy;
globalProvider?: SandboxToolPolicy;
agent?: SandboxToolPolicy;
agentProvider?: SandboxToolPolicy;
};
};
export function resolveEffectiveToolPolicy(params: {
config?: OpenClawConfig;
@@ -234,7 +220,7 @@ export function resolveEffectiveToolPolicy(params: {
agentId?: string;
modelProvider?: string;
modelId?: string;
}) {
}): ResolvedToolPolicyContext {
const explicitAgentId =
typeof params.agentId === "string" && params.agentId.trim()
? normalizeAgentId(params.agentId)
@@ -258,30 +244,27 @@ export function resolveEffectiveToolPolicy(params: {
modelProvider: params.modelProvider,
modelId: params.modelId,
});
const explicitProfileAlsoAllow =
resolveExplicitProfileAlsoAllow(agentTools) ?? resolveExplicitProfileAlsoAllow(globalTools);
const implicitProfileAlsoAllow = resolveImplicitProfileAlsoAllow({ globalTools, agentTools });
const profileAlsoAllow =
explicitProfileAlsoAllow || implicitProfileAlsoAllow
? Array.from(
new Set([...(explicitProfileAlsoAllow ?? []), ...(implicitProfileAlsoAllow ?? [])]),
)
: undefined;
return {
agentId,
globalPolicy: pickSandboxToolPolicy(globalTools),
globalProviderPolicy: pickSandboxToolPolicy(providerPolicy),
agentPolicy: pickSandboxToolPolicy(agentTools),
agentProviderPolicy: pickSandboxToolPolicy(agentProviderPolicy),
profile,
providerProfile: agentProviderPolicy?.profile ?? providerPolicy?.profile,
// alsoAllow is applied at the profile stage (to avoid being filtered out early).
profileAlsoAllow,
providerProfileAlsoAllow: Array.isArray(agentProviderPolicy?.alsoAllow)
? agentProviderPolicy?.alsoAllow
: Array.isArray(providerPolicy?.alsoAllow)
? providerPolicy?.alsoAllow
: undefined,
profiles: {
primary: {
id: profile,
alsoAllow: resolveProfileAlsoAllow({ globalTools, agentTools }),
},
provider: {
id: agentProviderPolicy?.profile ?? providerPolicy?.profile,
alsoAllow: resolveProviderProfileAlsoAllow({
providerPolicy,
agentProviderPolicy,
}),
},
},
sandboxPolicies: {
global: pickSandboxToolPolicy(globalTools),
globalProvider: pickSandboxToolPolicy(providerPolicy),
agent: pickSandboxToolPolicy(agentTools),
agentProvider: pickSandboxToolPolicy(agentProviderPolicy),
},
};
}

View File

@@ -1,6 +1,10 @@
import { codingTools, createReadTool, readTool } from "@mariozechner/pi-coding-agent";
import type { OpenClawConfig } from "../config/config.js";
import type { ToolLoopDetectionConfig } from "../config/types.tools.js";
import type {
AgentToolsConfig,
ToolLoopDetectionConfig,
ToolsConfig,
} from "../config/types.tools.js";
import { resolveMergedSafeBinProfileFixtures } from "../infra/exec-safe-bin-runtime-policy.js";
import { logWarn } from "../logger.js";
import { getPluginToolMeta } from "../plugins/tools.js";
@@ -129,32 +133,33 @@ function isApplyPatchAllowedForModel(params: {
});
}
function resolveExecConfig(params: { cfg?: OpenClawConfig; agentId?: string }) {
function resolveToolSectionConfig<T>(
params: { cfg?: OpenClawConfig; agentId?: string },
select: (tools: ToolsConfig | AgentToolsConfig | undefined) => T | undefined,
): {
global?: T;
agent?: T;
} {
const cfg = params.cfg;
const globalExec = cfg?.tools?.exec;
const agentExec =
cfg && params.agentId ? resolveAgentConfig(cfg, params.agentId)?.tools?.exec : undefined;
return {
host: agentExec?.host ?? globalExec?.host,
security: agentExec?.security ?? globalExec?.security,
ask: agentExec?.ask ?? globalExec?.ask,
node: agentExec?.node ?? globalExec?.node,
pathPrepend: agentExec?.pathPrepend ?? globalExec?.pathPrepend,
safeBins: agentExec?.safeBins ?? globalExec?.safeBins,
safeBinTrustedDirs: agentExec?.safeBinTrustedDirs ?? globalExec?.safeBinTrustedDirs,
global: select(cfg?.tools),
agent:
cfg && params.agentId ? select(resolveAgentConfig(cfg, params.agentId)?.tools) : undefined,
};
}
function resolveExecConfig(params: { cfg?: OpenClawConfig; agentId?: string }) {
const { global: globalExec, agent: agentExec } = resolveToolSectionConfig(
params,
(tools) => tools?.exec,
);
return {
...globalExec,
...agentExec,
safeBinProfiles: resolveMergedSafeBinProfileFixtures({
global: globalExec,
local: agentExec,
}),
backgroundMs: agentExec?.backgroundMs ?? globalExec?.backgroundMs,
timeoutSec: agentExec?.timeoutSec ?? globalExec?.timeoutSec,
approvalRunningNoticeMs:
agentExec?.approvalRunningNoticeMs ?? globalExec?.approvalRunningNoticeMs,
cleanupMs: agentExec?.cleanupMs ?? globalExec?.cleanupMs,
notifyOnExit: agentExec?.notifyOnExit ?? globalExec?.notifyOnExit,
notifyOnExitEmptySuccess:
agentExec?.notifyOnExitEmptySuccess ?? globalExec?.notifyOnExitEmptySuccess,
applyPatch: agentExec?.applyPatch ?? globalExec?.applyPatch,
};
}
@@ -162,11 +167,7 @@ export function resolveToolLoopDetectionConfig(params: {
cfg?: OpenClawConfig;
agentId?: string;
}): ToolLoopDetectionConfig | undefined {
const global = params.cfg?.tools?.loopDetection;
const agent =
params.agentId && params.cfg
? resolveAgentConfig(params.cfg, params.agentId)?.tools?.loopDetection
: undefined;
const { global, agent } = resolveToolSectionConfig(params, (tools) => tools?.loopDetection);
if (!agent) {
return global;
@@ -258,23 +259,14 @@ export function createOpenClawCodingTools(options?: {
}): AnyAgentTool[] {
const execToolName = "exec";
const sandbox = options?.sandbox?.enabled ? options.sandbox : undefined;
const {
agentId,
globalPolicy,
globalProviderPolicy,
agentPolicy,
agentProviderPolicy,
profile,
providerProfile,
profileAlsoAllow,
providerProfileAlsoAllow,
} = resolveEffectiveToolPolicy({
const policyContext = resolveEffectiveToolPolicy({
config: options?.config,
sessionKey: options?.sessionKey,
agentId: options?.agentId,
modelProvider: options?.modelProvider,
modelId: options?.modelId,
});
const { agentId } = policyContext;
const groupPolicy = resolveGroupToolPolicy({
config: options?.config,
sessionKey: options?.sessionKey,
@@ -289,13 +281,13 @@ export function createOpenClawCodingTools(options?: {
senderUsername: options?.senderUsername,
senderE164: options?.senderE164,
});
const profilePolicy = resolveToolProfilePolicy(profile);
const providerProfilePolicy = resolveToolProfilePolicy(providerProfile);
const profilePolicyWithAlsoAllow = mergeAlsoAllowPolicy(profilePolicy, profileAlsoAllow);
const profilePolicyWithAlsoAllow = mergeAlsoAllowPolicy(
resolveToolProfilePolicy(policyContext.profiles.primary.id),
policyContext.profiles.primary.alsoAllow,
);
const providerProfilePolicyWithAlsoAllow = mergeAlsoAllowPolicy(
providerProfilePolicy,
providerProfileAlsoAllow,
resolveToolProfilePolicy(policyContext.profiles.provider.id),
policyContext.profiles.provider.alsoAllow,
);
// Prefer sessionKey for process isolation scope to prevent cross-session process visibility/killing.
// Fallback to agentId if no sessionKey is available (e.g. legacy or global contexts).
@@ -311,10 +303,10 @@ export function createOpenClawCodingTools(options?: {
const allowBackground = isToolAllowedByPolicies("process", [
profilePolicyWithAlsoAllow,
providerProfilePolicyWithAlsoAllow,
globalPolicy,
globalProviderPolicy,
agentPolicy,
agentProviderPolicy,
policyContext.sandboxPolicies.global,
policyContext.sandboxPolicies.globalProvider,
policyContext.sandboxPolicies.agent,
policyContext.sandboxPolicies.agentProvider,
groupPolicy,
sandbox?.tools,
subagentPolicy,
@@ -491,12 +483,12 @@ export function createOpenClawCodingTools(options?: {
sandboxed: !!sandbox,
config: options?.config,
pluginToolAllowlist: collectExplicitAllowlist([
profilePolicy,
providerProfilePolicy,
globalPolicy,
globalProviderPolicy,
agentPolicy,
agentProviderPolicy,
profilePolicyWithAlsoAllow,
providerProfilePolicyWithAlsoAllow,
policyContext.sandboxPolicies.global,
policyContext.sandboxPolicies.globalProvider,
policyContext.sandboxPolicies.agent,
policyContext.sandboxPolicies.agentProvider,
groupPolicy,
sandbox?.tools,
subagentPolicy,
@@ -530,13 +522,13 @@ export function createOpenClawCodingTools(options?: {
steps: [
...buildDefaultToolPolicyPipelineSteps({
profilePolicy: profilePolicyWithAlsoAllow,
profile,
profile: policyContext.profiles.primary.id,
providerProfilePolicy: providerProfilePolicyWithAlsoAllow,
providerProfile,
globalPolicy,
globalProviderPolicy,
agentPolicy,
agentProviderPolicy,
providerProfile: policyContext.profiles.provider.id,
globalPolicy: policyContext.sandboxPolicies.global,
globalProviderPolicy: policyContext.sandboxPolicies.globalProvider,
agentPolicy: policyContext.sandboxPolicies.agent,
agentProviderPolicy: policyContext.sandboxPolicies.agentProvider,
groupPolicy,
agentId,
}),

View File

@@ -1,4 +1,5 @@
import type { SandboxToolPolicy } from "./sandbox/types.js";
import { mergeAlsoAllowIntoAllowlist } from "./tool-policy-also-allow.js";
type SandboxToolPolicyConfig = {
allow?: string[];
@@ -6,29 +7,17 @@ type SandboxToolPolicyConfig = {
deny?: string[];
};
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) {
return Array.from(new Set(["*", ...extra]));
}
return Array.from(new Set([...base, ...extra]));
}
export function pickSandboxToolPolicy(
config?: SandboxToolPolicyConfig,
): SandboxToolPolicy | undefined {
if (!config) {
return undefined;
}
const allow = Array.isArray(config.allow)
? unionAllow(config.allow, config.alsoAllow)
: Array.isArray(config.alsoAllow) && config.alsoAllow.length > 0
? unionAllow(undefined, config.alsoAllow)
: undefined;
const allow = mergeAlsoAllowIntoAllowlist({
allow: Array.isArray(config.allow) ? config.allow : undefined,
alsoAllow: config.alsoAllow,
assumeAllowAll: true,
});
const deny = Array.isArray(config.deny) ? config.deny : undefined;
if (!allow && !deny) {
return undefined;

View File

@@ -0,0 +1,6 @@
export const CONFIGURED_TOOL_SECTION_EXPOSURES = {
exec: ["exec", "process"],
fs: ["read", "write", "edit"],
} as const;
export type ConfiguredToolSectionId = keyof typeof CONFIGURED_TOOL_SECTION_EXPOSURES;

View File

@@ -0,0 +1,74 @@
import type { AgentToolsConfig, ToolPolicyConfig, ToolsConfig } from "../config/types.tools.js";
import { CONFIGURED_TOOL_SECTION_EXPOSURES } from "./tool-config-exposure.js";
type AlsoAllowConfig = Pick<ToolPolicyConfig, "alsoAllow">;
export function mergeUniqueToolNames(...lists: Array<string[] | undefined>): string[] | undefined {
const merged: string[] = [];
for (const list of lists) {
if (!Array.isArray(list)) {
continue;
}
for (const raw of list) {
const trimmed = typeof raw === "string" ? raw.trim() : "";
if (trimmed) {
merged.push(trimmed);
}
}
}
return merged.length > 0 ? Array.from(new Set(merged)) : undefined;
}
export function mergeAlsoAllowIntoAllowlist(params: {
allow?: string[];
alsoAllow?: string[];
assumeAllowAll?: boolean;
}): string[] | undefined {
if (!Array.isArray(params.alsoAllow) || params.alsoAllow.length === 0) {
return params.allow;
}
if (!Array.isArray(params.allow) || params.allow.length === 0) {
return params.assumeAllowAll ? mergeUniqueToolNames(["*"], params.alsoAllow) : params.allow;
}
return mergeUniqueToolNames(params.allow, params.alsoAllow);
}
export function resolveExplicitAlsoAllow(...configs: Array<AlsoAllowConfig | undefined>) {
for (const config of configs) {
if (Array.isArray(config?.alsoAllow) && config.alsoAllow.length > 0) {
return config.alsoAllow;
}
}
return undefined;
}
export function resolveImplicitToolSectionAlsoAllow(params: {
globalTools?: ToolsConfig;
agentTools?: AgentToolsConfig;
}): string[] | undefined {
const exposures: string[][] = [];
if (params.agentTools?.exec != null || params.globalTools?.exec != null) {
exposures.push([...CONFIGURED_TOOL_SECTION_EXPOSURES.exec]);
}
if (params.agentTools?.fs != null || params.globalTools?.fs != null) {
exposures.push([...CONFIGURED_TOOL_SECTION_EXPOSURES.fs]);
}
return mergeUniqueToolNames(...exposures);
}
export function resolveProfileAlsoAllow(params: {
globalTools?: ToolsConfig;
agentTools?: AgentToolsConfig;
}) {
return mergeUniqueToolNames(
resolveExplicitAlsoAllow(params.agentTools, params.globalTools),
resolveImplicitToolSectionAlsoAllow(params),
);
}
export function resolveProviderProfileAlsoAllow(params: {
providerPolicy?: ToolPolicyConfig;
agentProviderPolicy?: ToolPolicyConfig;
}) {
return resolveExplicitAlsoAllow(params.agentProviderPolicy, params.providerPolicy);
}

View File

@@ -1,3 +1,4 @@
import { mergeAlsoAllowIntoAllowlist } from "./tool-policy-also-allow.js";
import {
expandToolGroups,
normalizeToolList,
@@ -198,8 +199,12 @@ export function mergeAlsoAllowPolicy<TPolicy extends { allow?: string[] }>(
policy: TPolicy | undefined,
alsoAllow?: string[],
): TPolicy | undefined {
if (!policy?.allow || !Array.isArray(alsoAllow) || alsoAllow.length === 0) {
const allow = mergeAlsoAllowIntoAllowlist({
allow: policy?.allow,
alsoAllow,
});
if (allow === policy?.allow) {
return policy;
}
return { ...policy, allow: Array.from(new Set([...policy.allow, ...alsoAllow])) };
return { ...policy, allow } as TPolicy;
}

View File

@@ -350,6 +350,7 @@ describe("POST /tools/invoke", () => {
expect(resProfile.status).toBe(200);
const profileBody = await resProfile.json();
expect(profileBody.ok).toBe(true);
expect(lastCreateOpenClawToolsContext?.pluginToolAllowlist).toContain("agents_list");
cfg = {
...cfg,
@@ -360,6 +361,7 @@ describe("POST /tools/invoke", () => {
expect(resImplicit.status).toBe(200);
const implicitBody = await resImplicit.json();
expect(implicitBody.ok).toBe(true);
expect(lastCreateOpenClawToolsContext?.pluginToolAllowlist).toContain("agents_list");
});
it("routes tools invoke before plugin HTTP handlers", async () => {

View File

@@ -216,24 +216,15 @@ export async function handleToolsInvokeHttpRequest(
const agentTo = getHeader(req, "x-openclaw-message-to")?.trim() || undefined;
const agentThreadId = getHeader(req, "x-openclaw-thread-id")?.trim() || undefined;
const {
agentId,
globalPolicy,
globalProviderPolicy,
agentPolicy,
agentProviderPolicy,
profile,
providerProfile,
profileAlsoAllow,
providerProfileAlsoAllow,
} = resolveEffectiveToolPolicy({ config: cfg, sessionKey });
const profilePolicy = resolveToolProfilePolicy(profile);
const providerProfilePolicy = resolveToolProfilePolicy(providerProfile);
const profilePolicyWithAlsoAllow = mergeAlsoAllowPolicy(profilePolicy, profileAlsoAllow);
const policyContext = resolveEffectiveToolPolicy({ config: cfg, sessionKey });
const { agentId } = policyContext;
const profilePolicyWithAlsoAllow = mergeAlsoAllowPolicy(
resolveToolProfilePolicy(policyContext.profiles.primary.id),
policyContext.profiles.primary.alsoAllow,
);
const providerProfilePolicyWithAlsoAllow = mergeAlsoAllowPolicy(
providerProfilePolicy,
providerProfileAlsoAllow,
resolveToolProfilePolicy(policyContext.profiles.provider.id),
policyContext.profiles.provider.alsoAllow,
);
const groupPolicy = resolveGroupToolPolicy({
config: cfg,
@@ -256,12 +247,12 @@ export async function handleToolsInvokeHttpRequest(
allowMediaInvokeCommands: true,
config: cfg,
pluginToolAllowlist: collectExplicitAllowlist([
profilePolicy,
providerProfilePolicy,
globalPolicy,
globalProviderPolicy,
agentPolicy,
agentProviderPolicy,
profilePolicyWithAlsoAllow,
providerProfilePolicyWithAlsoAllow,
policyContext.sandboxPolicies.global,
policyContext.sandboxPolicies.globalProvider,
policyContext.sandboxPolicies.agent,
policyContext.sandboxPolicies.agentProvider,
groupPolicy,
subagentPolicy,
]),
@@ -276,13 +267,13 @@ export async function handleToolsInvokeHttpRequest(
steps: [
...buildDefaultToolPolicyPipelineSteps({
profilePolicy: profilePolicyWithAlsoAllow,
profile,
profile: policyContext.profiles.primary.id,
providerProfilePolicy: providerProfilePolicyWithAlsoAllow,
providerProfile,
globalPolicy,
globalProviderPolicy,
agentPolicy,
agentProviderPolicy,
providerProfile: policyContext.profiles.provider.id,
globalPolicy: policyContext.sandboxPolicies.global,
globalProviderPolicy: policyContext.sandboxPolicies.globalProvider,
agentPolicy: policyContext.sandboxPolicies.agent,
agentProviderPolicy: policyContext.sandboxPolicies.agentProvider,
groupPolicy,
agentId,
}),