Merge 292eae6f9c850f21f6ce6b2ca11059092ac97da4 into 598f1826d8b2bc969aace2c6459824737667218c

This commit is contained in:
J. Gavin Ray 2026-03-20 20:15:40 -07:00 committed by GitHub
commit 973cc5b49c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 54 additions and 6 deletions

View File

@ -89,7 +89,7 @@ export function resolveSubagentToolPolicy(cfg?: OpenClawConfig, depth?: number):
...(Array.isArray(configured?.deny) ? configured.deny : []),
];
const mergedAllow = allow && alsoAllow ? Array.from(new Set([...allow, ...alsoAllow])) : allow;
return { allow: mergedAllow, deny };
return { allow: mergedAllow, alsoAllow, deny };
}
export function resolveSubagentToolPolicyForSession(
@ -109,8 +109,13 @@ export function resolveSubagentToolPolicyForSession(
),
...(Array.isArray(configured?.deny) ? configured.deny : []),
];
// Merge allow + alsoAllow when both are present (keeps allow list accurate for
// access gating). When only alsoAllow is set, preserve it on the returned
// object so collectExplicitAllowlist() can build the plugin tool allowlist —
// callers that only gate on `allow` remain unaffected because allow stays
// undefined, but the alsoAllow entries are no longer silently dropped.
const mergedAllow = allow && alsoAllow ? Array.from(new Set([...allow, ...alsoAllow])) : allow;
return { allow: mergedAllow, deny };
return { allow: mergedAllow, alsoAllow, deny };
}
export function filterToolsByPolicy(tools: AnyAgentTool[], policy?: SandboxToolPolicy) {

View File

@ -6,6 +6,7 @@ export type { SandboxDockerConfig } from "./types.docker.js";
export type SandboxToolPolicy = {
allow?: string[];
alsoAllow?: string[];
deny?: string[];
};

View File

@ -5,6 +5,7 @@ import type { SandboxToolPolicy } from "./sandbox/types.js";
import { TOOL_POLICY_CONFORMANCE } from "./tool-policy.conformance.js";
import {
applyOwnerOnlyToolPolicy,
collectExplicitAllowlist,
expandToolGroups,
isOwnerOnlyToolName,
normalizeToolName,
@ -225,3 +226,38 @@ describe("resolveSandboxToolPolicyForAgent", () => {
expect(resolved.deny).toEqual(["image"]);
});
});
describe("collectExplicitAllowlist", () => {
it("collects entries from allow", () => {
expect(collectExplicitAllowlist([{ allow: ["read", "write"] }])).toEqual(["read", "write"]);
});
it("collects entries from alsoAllow when allow is absent", () => {
expect(collectExplicitAllowlist([{ alsoAllow: ["openrag_search"] }])).toEqual([
"openrag_search",
]);
});
it("collects entries from both allow and alsoAllow", () => {
expect(
collectExplicitAllowlist([{ allow: ["read"], alsoAllow: ["openrag_search"] }]),
).toEqual(["read", "openrag_search"]);
});
it("returns empty array when policy has neither allow nor alsoAllow", () => {
expect(collectExplicitAllowlist([{ deny: ["exec"] }])).toEqual([]);
});
it("returns empty array for undefined policies", () => {
expect(collectExplicitAllowlist([undefined, undefined])).toEqual([]);
});
it("collects alsoAllow across multiple policies", () => {
expect(
collectExplicitAllowlist([
{ alsoAllow: ["openrag_search"] },
{ alsoAllow: ["custom_tool"] },
]),
).toEqual(["openrag_search", "custom_tool"]);
});
});

View File

@ -58,6 +58,7 @@ export function applyOwnerOnlyToolPolicy(tools: AnyAgentTool[], senderIsOwner: b
export type ToolPolicyLike = {
allow?: string[];
alsoAllow?: string[];
deny?: string[];
};
@ -75,10 +76,14 @@ export type AllowlistResolution = {
export function collectExplicitAllowlist(policies: Array<ToolPolicyLike | undefined>): string[] {
const entries: string[] = [];
for (const policy of policies) {
if (!policy?.allow) {
continue;
}
for (const value of policy.allow) {
// Collect both `allow` and `alsoAllow` entries. `alsoAllow` is the additive
// form used when a profile is set and the user wants to extend it with
// additional tools (e.g. plugin tools for subagents) without replacing the
// full allow list. Without including `alsoAllow` here, plugin tools listed
// in `tools.subagents.tools.alsoAllow` are silently ignored and never reach
// subagent sessions.
const sources = [...(policy?.allow ?? []), ...(policy?.alsoAllow ?? [])];
for (const value of sources) {
if (typeof value !== "string") {
continue;
}
@ -149,6 +154,7 @@ export function expandPolicyWithPluginGroups(
}
return {
allow: expandPluginGroups(policy.allow, groups),
alsoAllow: expandPluginGroups(policy.alsoAllow, groups),
deny: expandPluginGroups(policy.deny, groups),
};
}