ACP: honor per-agent allowlists for spawned sessions

This commit is contained in:
Vincent Koc 2026-03-14 20:05:10 -07:00
parent db20141993
commit 924406472a
2 changed files with 170 additions and 24 deletions

View File

@ -4,6 +4,23 @@ import type { SessionBindingRecord } from "../infra/outbound/session-binding-ser
function createDefaultSpawnConfig(): OpenClawConfig {
return {
agents: {
list: [
{
id: "main",
default: true,
subagents: {
allowAgents: ["codex"],
},
},
{
id: "research",
subagents: {
allowAgents: ["codex"],
},
},
],
},
acp: {
enabled: true,
backend: "acpx",
@ -82,35 +99,25 @@ function buildSessionBindingServiceMock() {
};
}
vi.mock("../config/config.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../config/config.js")>();
return {
...actual,
loadConfig: () => hoisted.state.cfg,
};
});
vi.mock("../config/config.js", () => ({
loadConfig: () => hoisted.state.cfg,
}));
vi.mock("../gateway/call.js", () => ({
callGateway: (opts: unknown) => hoisted.callGatewayMock(opts),
}));
vi.mock("../config/sessions.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../config/sessions.js")>();
return {
...actual,
loadSessionStore: (storePath: string) => hoisted.loadSessionStoreMock(storePath),
resolveStorePath: (store: unknown, opts: unknown) => hoisted.resolveStorePathMock(store, opts),
};
});
vi.mock("../config/sessions.js", () => ({
loadSessionStore: (storePath: string) => hoisted.loadSessionStoreMock(storePath),
resolveStorePath: (store: unknown, opts: unknown) => hoisted.resolveStorePathMock(store, opts),
resolveAgentMainSessionKey: ({ agentId }: { agentId: string }) => `agent:${agentId}:main`,
canonicalizeMainSessionAlias: ({ sessionKey }: { sessionKey: string }) => sessionKey,
}));
vi.mock("../config/sessions/transcript.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../config/sessions/transcript.js")>();
return {
...actual,
resolveSessionTranscriptFile: (params: unknown) =>
hoisted.resolveSessionTranscriptFileMock(params),
};
});
vi.mock("../config/sessions/transcript.js", () => ({
resolveSessionTranscriptFile: (params: unknown) =>
hoisted.resolveSessionTranscriptFileMock(params),
}));
vi.mock("../acp/control-plane/manager.js", () => {
return {
@ -650,6 +657,21 @@ describe("spawnAcpDirect", () => {
hoisted.state.cfg = {
...hoisted.state.cfg,
agents: {
list: [
{
id: "main",
default: true,
subagents: {
allowAgents: ["codex"],
},
},
{
id: "research",
subagents: {
allowAgents: ["codex"],
},
},
],
defaults: {
heartbeat: {
every: "30m",
@ -728,6 +750,21 @@ describe("spawnAcpDirect", () => {
hoisted.state.cfg = {
...hoisted.state.cfg,
agents: {
list: [
{
id: "main",
default: true,
subagents: {
allowAgents: ["codex"],
},
},
{
id: "research",
subagents: {
allowAgents: ["codex"],
},
},
],
defaults: {
heartbeat: {
every: "30m",
@ -762,6 +799,21 @@ describe("spawnAcpDirect", () => {
scope: "global",
},
agents: {
list: [
{
id: "main",
default: true,
subagents: {
allowAgents: ["codex"],
},
},
{
id: "research",
subagents: {
allowAgents: ["codex"],
},
},
],
defaults: {
heartbeat: {
every: "30m",
@ -791,7 +843,21 @@ describe("spawnAcpDirect", () => {
hoisted.state.cfg = {
...hoisted.state.cfg,
agents: {
list: [{ id: "main", heartbeat: { every: "30m" } }, { id: "research" }],
list: [
{
id: "main",
heartbeat: { every: "30m" },
subagents: {
allowAgents: ["codex"],
},
},
{
id: "research",
subagents: {
allowAgents: ["codex"],
},
},
],
},
};
@ -819,6 +885,9 @@ describe("spawnAcpDirect", () => {
{
id: "research",
heartbeat: { every: "0m" },
subagents: {
allowAgents: ["codex"],
},
},
],
},
@ -1041,4 +1110,44 @@ describe("spawnAcpDirect", () => {
expect(hoisted.callGatewayMock).not.toHaveBeenCalled();
expect(hoisted.startAcpSpawnParentStreamRelayMock).not.toHaveBeenCalled();
});
it("forbids ACP cross-agent spawning when the requester allowlist does not include the target", async () => {
hoisted.state.cfg = {
...createDefaultSpawnConfig(),
acp: {
enabled: true,
backend: "acpx",
allowedAgents: ["ops"],
},
agents: {
list: [
{
id: "main",
subagents: {
allowAgents: ["research"],
},
},
{
id: "ops",
},
],
},
};
const result = await spawnAcpDirect(
{
task: "do thing",
agentId: "ops",
},
{
agentSessionKey: "agent:main:subagent:parent",
},
);
expect(result).toEqual({
status: "forbidden",
error: "agentId is not allowed for sessions_spawn (allowed: research)",
});
expect(hoisted.initializeSessionMock).not.toHaveBeenCalled();
});
});

View File

@ -256,6 +256,32 @@ function normalizeOptionalAgentId(value: string | undefined | null): string | un
return normalizeAgentId(trimmed);
}
function resolveCrossAgentAllowlistError(params: {
cfg: OpenClawConfig;
requesterAgentId?: string;
targetAgentId: string;
}): string | undefined {
const requesterAgentId = normalizeOptionalAgentId(params.requesterAgentId);
if (!requesterAgentId || requesterAgentId === params.targetAgentId) {
return undefined;
}
const allowAgents =
resolveAgentConfig(params.cfg, requesterAgentId)?.subagents?.allowAgents ?? [];
const allowAny = allowAgents.some((value) => value.trim() === "*");
const allowSet = new Set(
allowAgents
.filter((value) => value.trim() && value.trim() !== "*")
.map((value) => normalizeAgentId(value).toLowerCase()),
);
if (allowAny || allowSet.has(params.targetAgentId.toLowerCase())) {
return undefined;
}
const allowedText = allowSet.size > 0 ? Array.from(allowSet).join(", ") : "none";
return `agentId is not allowed for sessions_spawn (allowed: ${allowedText})`;
}
function summarizeError(err: unknown): string {
if (err instanceof Error) {
return err.message;
@ -506,6 +532,17 @@ export async function spawnAcpDirect(
};
}
const targetAgentId = targetAgentResult.agentId;
const crossAgentAllowlistError = resolveCrossAgentAllowlistError({
cfg,
requesterAgentId,
targetAgentId,
});
if (crossAgentAllowlistError) {
return {
status: "forbidden",
error: crossAgentAllowlistError,
};
}
const agentPolicyError = resolveAcpAgentPolicyError(cfg, targetAgentId);
if (agentPolicyError) {
return {