fix(tool-policy): include alsoAllow entries in plugin tool allowlist for subagents
Problem:
Plugin tools registered via the plugin system were silently unavailable
in subagent sessions even when explicitly listed in
tools.subagents.tools.alsoAllow. Users following the documented alsoAllow
pattern would add the config, observe no error, but find their subagents
unable to call the plugin tools they expected.
Root cause (three places):
1. resolveSubagentToolPolicyForSession() in pi-tools.policy.ts returned
{ allow, deny } and dropped alsoAllow off the object entirely. When
only alsoAllow was configured (no explicit allow list), allow remained
undefined and alsoAllow entries were lost before reaching
collectExplicitAllowlist().
2. collectExplicitAllowlist() in tool-policy.ts only iterated policy.allow.
The ToolPolicyLike type also lacked the alsoAllow field entirely.
3. expandPolicyWithPluginGroups() reconstructed the policy as { allow, deny }
and silently discarded alsoAllow — a latent footgun for callers that
expect the transformed policy to still carry the field.
Fix:
- sandbox/types.ts: add alsoAllow to SandboxToolPolicy so the field is
recognized throughout the policy pipeline at the type level.
- tool-policy.ts (ToolPolicyLike): add alsoAllow field.
- tool-policy.ts (collectExplicitAllowlist): merge policy.allow and
policy.alsoAllow before iterating so both sources contribute to the
plugin tool allowlist passed to resolvePluginTools().
- tool-policy.ts (expandPolicyWithPluginGroups): pass alsoAllow through
the expansion so the field is not silently dropped on transformed policies.
- pi-tools.policy.ts (resolveSubagentToolPolicyForSession): preserve
alsoAllow on the returned object. allow stays undefined in the
alsoAllow-only case (correct for access gating via filterToolsByPolicy),
but alsoAllow is no longer discarded before collectExplicitAllowlist().
- tool-policy.test.ts: add six unit tests directly exercising
collectExplicitAllowlist with allow-only, alsoAllow-only, both, empty,
undefined, and multi-policy inputs.
Impact:
With this fix, the documented config pattern works as intended:
tools:
subagents:
tools:
alsoAllow: ["openrag_search"]
All 65 existing and new tool-policy and pi-tools.policy tests pass.
No behavior change for callers that only use policy.allow.
This commit is contained in:
parent
5e417b44e1
commit
292eae6f9c
@ -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) {
|
||||
|
||||
@ -6,6 +6,7 @@ export type { SandboxDockerConfig } from "./types.docker.js";
|
||||
|
||||
export type SandboxToolPolicy = {
|
||||
allow?: string[];
|
||||
alsoAllow?: string[];
|
||||
deny?: string[];
|
||||
};
|
||||
|
||||
|
||||
@ -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"]);
|
||||
});
|
||||
});
|
||||
|
||||
@ -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),
|
||||
};
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user