* Security: enforce ACP sandbox inheritance in sessions_spawn * fix: add changelog attribution for ACP sandbox inheritance (#32254) (thanks @dutifulbob) --------- Co-authored-by: Onur <2453968+osolmaz@users.noreply.github.com>
427 lines
12 KiB
TypeScript
427 lines
12 KiB
TypeScript
import { beforeEach, describe, expect, it, vi } from "vitest";
|
|
import type { OpenClawConfig } from "../config/config.js";
|
|
import type { SessionBindingRecord } from "../infra/outbound/session-binding-service.js";
|
|
|
|
function createDefaultSpawnConfig(): OpenClawConfig {
|
|
return {
|
|
acp: {
|
|
enabled: true,
|
|
backend: "acpx",
|
|
allowedAgents: ["codex"],
|
|
},
|
|
session: {
|
|
mainKey: "main",
|
|
scope: "per-sender",
|
|
},
|
|
channels: {
|
|
discord: {
|
|
threadBindings: {
|
|
enabled: true,
|
|
spawnAcpSessions: true,
|
|
},
|
|
},
|
|
},
|
|
};
|
|
}
|
|
|
|
const hoisted = vi.hoisted(() => {
|
|
const callGatewayMock = vi.fn();
|
|
const sessionBindingCapabilitiesMock = vi.fn();
|
|
const sessionBindingBindMock = vi.fn();
|
|
const sessionBindingUnbindMock = vi.fn();
|
|
const sessionBindingResolveByConversationMock = vi.fn();
|
|
const sessionBindingListBySessionMock = vi.fn();
|
|
const closeSessionMock = vi.fn();
|
|
const initializeSessionMock = vi.fn();
|
|
const state = {
|
|
cfg: createDefaultSpawnConfig(),
|
|
};
|
|
return {
|
|
callGatewayMock,
|
|
sessionBindingCapabilitiesMock,
|
|
sessionBindingBindMock,
|
|
sessionBindingUnbindMock,
|
|
sessionBindingResolveByConversationMock,
|
|
sessionBindingListBySessionMock,
|
|
closeSessionMock,
|
|
initializeSessionMock,
|
|
state,
|
|
};
|
|
});
|
|
|
|
function buildSessionBindingServiceMock() {
|
|
return {
|
|
touch: vi.fn(),
|
|
bind(input: unknown) {
|
|
return hoisted.sessionBindingBindMock(input);
|
|
},
|
|
unbind(input: unknown) {
|
|
return hoisted.sessionBindingUnbindMock(input);
|
|
},
|
|
getCapabilities(params: unknown) {
|
|
return hoisted.sessionBindingCapabilitiesMock(params);
|
|
},
|
|
resolveByConversation(ref: unknown) {
|
|
return hoisted.sessionBindingResolveByConversationMock(ref);
|
|
},
|
|
listBySession(targetSessionKey: string) {
|
|
return hoisted.sessionBindingListBySessionMock(targetSessionKey);
|
|
},
|
|
};
|
|
}
|
|
|
|
vi.mock("../config/config.js", async (importOriginal) => {
|
|
const actual = await importOriginal<typeof import("../config/config.js")>();
|
|
return {
|
|
...actual,
|
|
loadConfig: () => hoisted.state.cfg,
|
|
};
|
|
});
|
|
|
|
vi.mock("../gateway/call.js", () => ({
|
|
callGateway: (opts: unknown) => hoisted.callGatewayMock(opts),
|
|
}));
|
|
|
|
vi.mock("../acp/control-plane/manager.js", () => {
|
|
return {
|
|
getAcpSessionManager: () => ({
|
|
initializeSession: (params: unknown) => hoisted.initializeSessionMock(params),
|
|
closeSession: (params: unknown) => hoisted.closeSessionMock(params),
|
|
}),
|
|
};
|
|
});
|
|
|
|
vi.mock("../infra/outbound/session-binding-service.js", async (importOriginal) => {
|
|
const actual =
|
|
await importOriginal<typeof import("../infra/outbound/session-binding-service.js")>();
|
|
return {
|
|
...actual,
|
|
getSessionBindingService: () => buildSessionBindingServiceMock(),
|
|
};
|
|
});
|
|
|
|
const { spawnAcpDirect } = await import("./acp-spawn.js");
|
|
|
|
function createSessionBindingCapabilities() {
|
|
return {
|
|
adapterAvailable: true,
|
|
bindSupported: true,
|
|
unbindSupported: true,
|
|
placements: ["current", "child"] as const,
|
|
};
|
|
}
|
|
|
|
function createSessionBinding(overrides?: Partial<SessionBindingRecord>): SessionBindingRecord {
|
|
return {
|
|
bindingId: "default:child-thread",
|
|
targetSessionKey: "agent:codex:acp:s1",
|
|
targetKind: "session",
|
|
conversation: {
|
|
channel: "discord",
|
|
accountId: "default",
|
|
conversationId: "child-thread",
|
|
parentConversationId: "parent-channel",
|
|
},
|
|
status: "active",
|
|
boundAt: Date.now(),
|
|
metadata: {
|
|
agentId: "codex",
|
|
boundBy: "system",
|
|
},
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
function expectResolvedIntroTextInBindMetadata(): void {
|
|
const callWithMetadata = hoisted.sessionBindingBindMock.mock.calls.find(
|
|
(call: unknown[]) =>
|
|
typeof (call[0] as { metadata?: { introText?: unknown } } | undefined)?.metadata
|
|
?.introText === "string",
|
|
);
|
|
const introText =
|
|
(callWithMetadata?.[0] as { metadata?: { introText?: string } } | undefined)?.metadata
|
|
?.introText ?? "";
|
|
expect(introText.includes("session ids: pending (available after the first reply)")).toBe(false);
|
|
}
|
|
|
|
describe("spawnAcpDirect", () => {
|
|
beforeEach(() => {
|
|
hoisted.state.cfg = createDefaultSpawnConfig();
|
|
|
|
hoisted.callGatewayMock.mockReset().mockImplementation(async (argsUnknown: unknown) => {
|
|
const args = argsUnknown as { method?: string };
|
|
if (args.method === "sessions.patch") {
|
|
return { ok: true };
|
|
}
|
|
if (args.method === "agent") {
|
|
return { runId: "run-1" };
|
|
}
|
|
if (args.method === "sessions.delete") {
|
|
return { ok: true };
|
|
}
|
|
return {};
|
|
});
|
|
|
|
hoisted.closeSessionMock.mockReset().mockResolvedValue({
|
|
runtimeClosed: true,
|
|
metaCleared: false,
|
|
});
|
|
hoisted.initializeSessionMock.mockReset().mockImplementation(async (argsUnknown: unknown) => {
|
|
const args = argsUnknown as {
|
|
sessionKey: string;
|
|
agent: string;
|
|
mode: "persistent" | "oneshot";
|
|
cwd?: string;
|
|
};
|
|
const runtimeSessionName = `${args.sessionKey}:runtime`;
|
|
const cwd = typeof args.cwd === "string" ? args.cwd : undefined;
|
|
return {
|
|
runtime: {
|
|
close: vi.fn().mockResolvedValue(undefined),
|
|
},
|
|
handle: {
|
|
sessionKey: args.sessionKey,
|
|
backend: "acpx",
|
|
runtimeSessionName,
|
|
...(cwd ? { cwd } : {}),
|
|
agentSessionId: "codex-inner-1",
|
|
backendSessionId: "acpx-1",
|
|
},
|
|
meta: {
|
|
backend: "acpx",
|
|
agent: args.agent,
|
|
runtimeSessionName,
|
|
...(cwd ? { runtimeOptions: { cwd }, cwd } : {}),
|
|
identity: {
|
|
state: "pending",
|
|
source: "ensure",
|
|
acpxSessionId: "acpx-1",
|
|
agentSessionId: "codex-inner-1",
|
|
lastUpdatedAt: Date.now(),
|
|
},
|
|
mode: args.mode,
|
|
state: "idle",
|
|
lastActivityAt: Date.now(),
|
|
},
|
|
};
|
|
});
|
|
|
|
hoisted.sessionBindingCapabilitiesMock
|
|
.mockReset()
|
|
.mockReturnValue(createSessionBindingCapabilities());
|
|
hoisted.sessionBindingBindMock
|
|
.mockReset()
|
|
.mockImplementation(
|
|
async (input: {
|
|
targetSessionKey: string;
|
|
conversation: { accountId: string };
|
|
metadata?: Record<string, unknown>;
|
|
}) =>
|
|
createSessionBinding({
|
|
targetSessionKey: input.targetSessionKey,
|
|
conversation: {
|
|
channel: "discord",
|
|
accountId: input.conversation.accountId,
|
|
conversationId: "child-thread",
|
|
parentConversationId: "parent-channel",
|
|
},
|
|
metadata: {
|
|
boundBy:
|
|
typeof input.metadata?.boundBy === "string" ? input.metadata.boundBy : "system",
|
|
agentId: "codex",
|
|
webhookId: "wh-1",
|
|
},
|
|
}),
|
|
);
|
|
hoisted.sessionBindingResolveByConversationMock.mockReset().mockReturnValue(null);
|
|
hoisted.sessionBindingListBySessionMock.mockReset().mockReturnValue([]);
|
|
hoisted.sessionBindingUnbindMock.mockReset().mockResolvedValue([]);
|
|
});
|
|
|
|
it("spawns ACP session, binds a new thread, and dispatches initial task", async () => {
|
|
const result = await spawnAcpDirect(
|
|
{
|
|
task: "Investigate flaky tests",
|
|
agentId: "codex",
|
|
mode: "session",
|
|
thread: true,
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
agentChannel: "discord",
|
|
agentAccountId: "default",
|
|
agentTo: "channel:parent-channel",
|
|
agentThreadId: "requester-thread",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("accepted");
|
|
expect(result.childSessionKey).toMatch(/^agent:codex:acp:/);
|
|
expect(result.runId).toBe("run-1");
|
|
expect(result.mode).toBe("session");
|
|
expect(hoisted.sessionBindingBindMock).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
targetKind: "session",
|
|
placement: "child",
|
|
}),
|
|
);
|
|
expectResolvedIntroTextInBindMetadata();
|
|
|
|
const agentCall = hoisted.callGatewayMock.mock.calls
|
|
.map((call: unknown[]) => call[0] as { method?: string; params?: Record<string, unknown> })
|
|
.find((request) => request.method === "agent");
|
|
expect(agentCall?.params?.sessionKey).toMatch(/^agent:codex:acp:/);
|
|
expect(agentCall?.params?.to).toBe("channel:child-thread");
|
|
expect(agentCall?.params?.threadId).toBe("child-thread");
|
|
expect(agentCall?.params?.deliver).toBe(true);
|
|
expect(hoisted.initializeSessionMock).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
sessionKey: expect.stringMatching(/^agent:codex:acp:/),
|
|
agent: "codex",
|
|
mode: "persistent",
|
|
}),
|
|
);
|
|
});
|
|
|
|
it("includes cwd in ACP thread intro banner when provided at spawn time", async () => {
|
|
const result = await spawnAcpDirect(
|
|
{
|
|
task: "Check workspace",
|
|
agentId: "codex",
|
|
cwd: "/home/bob/clawd",
|
|
mode: "session",
|
|
thread: true,
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
agentChannel: "discord",
|
|
agentAccountId: "default",
|
|
agentTo: "channel:parent-channel",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("accepted");
|
|
expect(hoisted.sessionBindingBindMock).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
metadata: expect.objectContaining({
|
|
introText: expect.stringContaining("cwd: /home/bob/clawd"),
|
|
}),
|
|
}),
|
|
);
|
|
});
|
|
|
|
it("rejects disallowed ACP agents", async () => {
|
|
hoisted.state.cfg = {
|
|
...hoisted.state.cfg,
|
|
acp: {
|
|
enabled: true,
|
|
backend: "acpx",
|
|
allowedAgents: ["claudecode"],
|
|
},
|
|
};
|
|
|
|
const result = await spawnAcpDirect(
|
|
{
|
|
task: "hello",
|
|
agentId: "codex",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
},
|
|
);
|
|
|
|
expect(result).toMatchObject({
|
|
status: "forbidden",
|
|
});
|
|
});
|
|
|
|
it("requires an explicit ACP agent when no config default exists", async () => {
|
|
const result = await spawnAcpDirect(
|
|
{
|
|
task: "hello",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("error");
|
|
expect(result.error).toContain("set `acp.defaultAgent`");
|
|
});
|
|
|
|
it("fails fast when Discord ACP thread spawn is disabled", async () => {
|
|
hoisted.state.cfg = {
|
|
...hoisted.state.cfg,
|
|
channels: {
|
|
discord: {
|
|
threadBindings: {
|
|
enabled: true,
|
|
spawnAcpSessions: false,
|
|
},
|
|
},
|
|
},
|
|
};
|
|
|
|
const result = await spawnAcpDirect(
|
|
{
|
|
task: "hello",
|
|
agentId: "codex",
|
|
thread: true,
|
|
mode: "session",
|
|
},
|
|
{
|
|
agentChannel: "discord",
|
|
agentAccountId: "default",
|
|
agentTo: "channel:parent-channel",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("error");
|
|
expect(result.error).toContain("spawnAcpSessions=true");
|
|
});
|
|
|
|
it("forbids ACP spawn from sandboxed requester sessions", async () => {
|
|
hoisted.state.cfg = {
|
|
...hoisted.state.cfg,
|
|
agents: {
|
|
defaults: {
|
|
sandbox: { mode: "all" },
|
|
},
|
|
},
|
|
};
|
|
|
|
const result = await spawnAcpDirect(
|
|
{
|
|
task: "hello",
|
|
agentId: "codex",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:subagent:parent",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("forbidden");
|
|
expect(result.error).toContain("Sandboxed sessions cannot spawn ACP sessions");
|
|
expect(hoisted.callGatewayMock).not.toHaveBeenCalled();
|
|
expect(hoisted.initializeSessionMock).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('forbids sandbox="require" for runtime=acp', async () => {
|
|
const result = await spawnAcpDirect(
|
|
{
|
|
task: "hello",
|
|
agentId: "codex",
|
|
sandbox: "require",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("forbidden");
|
|
expect(result.error).toContain('sandbox="require"');
|
|
expect(hoisted.callGatewayMock).not.toHaveBeenCalled();
|
|
expect(hoisted.initializeSessionMock).not.toHaveBeenCalled();
|
|
});
|
|
});
|