From ac11f0af731d41743ba02d8595f4d0fe747336e3 Mon Sep 17 00:00:00 2001 From: Bob Date: Mon, 2 Mar 2026 23:50:38 +0100 Subject: [PATCH] Security: enforce ACP sandbox inheritance for sessions_spawn (#32254) * 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> --- CHANGELOG.md | 1 + src/agents/acp-spawn.test.ts | 44 ++++++++++++++++++++ src/agents/acp-spawn.ts | 25 +++++++++++ src/agents/subagent-spawn.ts | 2 +- src/agents/system-prompt.test.ts | 20 +++++++++ src/agents/system-prompt.ts | 7 +++- src/agents/tools/sessions-spawn-tool.test.ts | 25 +++++++++++ src/agents/tools/sessions-spawn-tool.ts | 2 + 8 files changed, 124 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 083e0272fba..e16eb02ad2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ Docs: https://docs.openclaw.ai - Media understanding/audio transcription guard: skip tiny/empty audio files (<1024 bytes) before provider/CLI transcription to avoid noisy invalid-audio failures and preserve clean fallback behavior. (#8388) Thanks @Glucksberg. - OpenAI media capabilities: include `audio` in the OpenAI provider capability list so audio transcription models are eligible in media-understanding provider selection. (#12717) Thanks @openjay. - Security/Node exec approvals: preserve shell/dispatch-wrapper argv semantics during approval hardening so approved wrapper commands (for example `env sh -c ...`) cannot drift into a different runtime command shape, and add regression coverage for both approval-plan generation and approved runtime execution paths. Thanks @tdjackey for reporting. +- Security/ACP sandbox inheritance: enforce fail-closed runtime guardrails for `sessions_spawn` with `runtime="acp"` by rejecting ACP spawns from sandboxed requester sessions and rejecting `sandbox="require"` for ACP runtime, preventing sandbox-boundary bypass via host-side ACP initialization. (#32254) Thanks @dutifulbob. - Browser/Security output boundary hardening: replace check-then-rename output commits with root-bound fd-verified writes, unify install/skills canonical path-boundary checks, and add regression coverage for symlink-rebind race paths across browser output and shared fs-safe write flows. Thanks @tdjackey for reporting. - Security/Webhook request hardening: enforce auth-before-body parsing for BlueBubbles and Google Chat webhook handlers, add strict pre-auth body/time budgets for webhook auth paths (including LINE signature verification), and add shared in-flight/request guardrails plus regression tests/lint checks to prevent reintroducing unauthenticated slow-body DoS patterns. Thanks @GCXWLP for reporting. - Gateway/Security hardening: tie loopback-origin dev allowance to actual local socket clients (not Host header claims), add explicit warnings/metrics when `gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback` accepts websocket origins, harden safe-regex detection for quantified ambiguous alternation patterns (for example `(a|aa)+`), and bound large regex-evaluation inputs for session-filter and log-redaction paths. diff --git a/src/agents/acp-spawn.test.ts b/src/agents/acp-spawn.test.ts index 73b5c8bee30..732a465142d 100644 --- a/src/agents/acp-spawn.test.ts +++ b/src/agents/acp-spawn.test.ts @@ -379,4 +379,48 @@ describe("spawnAcpDirect", () => { 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(); + }); }); diff --git a/src/agents/acp-spawn.ts b/src/agents/acp-spawn.ts index 1cce4399ddc..ff475e54ebf 100644 --- a/src/agents/acp-spawn.ts +++ b/src/agents/acp-spawn.ts @@ -32,9 +32,12 @@ import { } from "../infra/outbound/session-binding-service.js"; import { normalizeAgentId } from "../routing/session-key.js"; import { normalizeDeliveryContext } from "../utils/delivery-context.js"; +import { resolveSandboxRuntimeStatus } from "./sandbox/runtime-status.js"; export const ACP_SPAWN_MODES = ["run", "session"] as const; export type SpawnAcpMode = (typeof ACP_SPAWN_MODES)[number]; +export const ACP_SPAWN_SANDBOX_MODES = ["inherit", "require"] as const; +export type SpawnAcpSandboxMode = (typeof ACP_SPAWN_SANDBOX_MODES)[number]; export type SpawnAcpParams = { task: string; @@ -43,6 +46,7 @@ export type SpawnAcpParams = { cwd?: string; mode?: SpawnAcpMode; thread?: boolean; + sandbox?: SpawnAcpSandboxMode; }; export type SpawnAcpContext = { @@ -51,6 +55,7 @@ export type SpawnAcpContext = { agentAccountId?: string; agentTo?: string; agentThreadId?: string | number; + sandboxed?: boolean; }; export type SpawnAcpResult = { @@ -228,6 +233,26 @@ export async function spawnAcpDirect( error: "ACP is disabled by policy (`acp.enabled=false`).", }; } + const sandboxMode = params.sandbox === "require" ? "require" : "inherit"; + const requesterRuntime = resolveSandboxRuntimeStatus({ + cfg, + sessionKey: ctx.agentSessionKey, + }); + const requesterSandboxed = ctx.sandboxed === true || requesterRuntime.sandboxed; + if (requesterSandboxed) { + return { + status: "forbidden", + error: + 'Sandboxed sessions cannot spawn ACP sessions because runtime="acp" runs on the host. Use runtime="subagent" from sandboxed sessions.', + }; + } + if (sandboxMode === "require") { + return { + status: "forbidden", + error: + 'sessions_spawn sandbox="require" is unsupported for runtime="acp" because ACP sessions run outside the sandbox. Use runtime="subagent" or sandbox="inherit".', + }; + } const requestThreadBinding = params.thread === true; const spawnMode = resolveSpawnMode({ diff --git a/src/agents/subagent-spawn.ts b/src/agents/subagent-spawn.ts index 5e1e76313cb..7068a057803 100644 --- a/src/agents/subagent-spawn.ts +++ b/src/agents/subagent-spawn.ts @@ -496,7 +496,7 @@ export async function spawnSubagentDirect( childSessionKey, label: label || undefined, task, - acpEnabled: cfg.acp?.enabled !== false, + acpEnabled: cfg.acp?.enabled !== false && !childRuntime.sandboxed, childDepth, maxSpawnDepth, }); diff --git a/src/agents/system-prompt.test.ts b/src/agents/system-prompt.test.ts index 2265479322b..dea5c209b49 100644 --- a/src/agents/system-prompt.test.ts +++ b/src/agents/system-prompt.test.ts @@ -286,6 +286,26 @@ describe("buildAgentSystemPrompt", () => { expect(prompt).toContain("- agents_list: List OpenClaw agent ids allowed for sessions_spawn"); }); + it("omits ACP harness spawn guidance for sandboxed sessions and shows ACP block note", () => { + const prompt = buildAgentSystemPrompt({ + workspaceDir: "/tmp/openclaw", + toolNames: ["sessions_spawn", "subagents", "agents_list", "exec"], + sandboxInfo: { + enabled: true, + }, + }); + + expect(prompt).not.toContain( + 'For requests like "do this in codex/claude code/gemini", treat it as ACP harness intent', + ); + expect(prompt).not.toContain( + 'do not call `message` with `action=thread-create`; use `sessions_spawn` (`runtime: "acp"`, `thread: true`) as the single thread creation path', + ); + expect(prompt).toContain("ACP harness spawns are blocked from sandboxed sessions"); + expect(prompt).toContain('`runtime: "acp"`'); + expect(prompt).toContain('Use `runtime: "subagent"` instead.'); + }); + it("preserves tool casing in the prompt", () => { const prompt = buildAgentSystemPrompt({ workspaceDir: "/tmp/openclaw", diff --git a/src/agents/system-prompt.ts b/src/agents/system-prompt.ts index 27d6bdef1cb..7e709195a1b 100644 --- a/src/agents/system-prompt.ts +++ b/src/agents/system-prompt.ts @@ -310,6 +310,8 @@ export function buildAgentSystemPrompt(params: { const normalizedTools = canonicalToolNames.map((tool) => tool.toLowerCase()); const availableTools = new Set(normalizedTools); const hasSessionsSpawn = availableTools.has("sessions_spawn"); + const sandboxedRuntime = params.sandboxInfo?.enabled === true; + const acpHarnessSpawnAllowed = hasSessionsSpawn && acpEnabled && !sandboxedRuntime; const externalToolSummaries = new Map(); for (const [key, value] of Object.entries(params.toolSummaries ?? {})) { const normalized = key.trim().toLowerCase(); @@ -443,7 +445,7 @@ export function buildAgentSystemPrompt(params: { "TOOLS.md does not control tool availability; it is user guidance for how to use external tools.", `For long waits, avoid rapid poll loops: use ${execToolName} with enough yieldMs or ${processToolName}(action=poll, timeout=).`, "If a task is more complex or takes longer, spawn a sub-agent. Completion is push-based: it will auto-announce when done.", - ...(hasSessionsSpawn && acpEnabled + ...(acpHarnessSpawnAllowed ? [ 'For requests like "do this in codex/claude code/gemini", treat it as ACP harness intent and call `sessions_spawn` with `runtime: "acp"`.', 'On Discord, default ACP harness requests to thread-bound persistent sessions (`thread: true`, `mode: "session"`) unless the user asks otherwise.', @@ -511,6 +513,9 @@ export function buildAgentSystemPrompt(params: { "You are running in a sandboxed runtime (tools execute in Docker).", "Some tools may be unavailable due to sandbox policy.", "Sub-agents stay sandboxed (no elevated/host access). Need outside-sandbox read/write? Don't spawn; ask first.", + hasSessionsSpawn && acpEnabled + ? 'ACP harness spawns are blocked from sandboxed sessions (`sessions_spawn` with `runtime: "acp"`). Use `runtime: "subagent"` instead.' + : "", params.sandboxInfo.containerWorkspaceDir ? `Sandbox container workdir: ${sanitizeForPromptLiteral(params.sandboxInfo.containerWorkspaceDir)}` : "", diff --git a/src/agents/tools/sessions-spawn-tool.test.ts b/src/agents/tools/sessions-spawn-tool.test.ts index a1dde4da635..db4396c78b8 100644 --- a/src/agents/tools/sessions-spawn-tool.test.ts +++ b/src/agents/tools/sessions-spawn-tool.test.ts @@ -116,6 +116,31 @@ describe("sessions_spawn tool", () => { expect(hoisted.spawnSubagentDirectMock).not.toHaveBeenCalled(); }); + it("forwards ACP sandbox options and requester sandbox context", async () => { + const tool = createSessionsSpawnTool({ + agentSessionKey: "agent:main:subagent:parent", + sandboxed: true, + }); + + await tool.execute("call-2b", { + runtime: "acp", + task: "investigate", + agentId: "codex", + sandbox: "require", + }); + + expect(hoisted.spawnAcpDirectMock).toHaveBeenCalledWith( + expect.objectContaining({ + task: "investigate", + sandbox: "require", + }), + expect.objectContaining({ + agentSessionKey: "agent:main:subagent:parent", + sandboxed: true, + }), + ); + }); + it("rejects attachments for ACP runtime", async () => { const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:main", diff --git a/src/agents/tools/sessions-spawn-tool.ts b/src/agents/tools/sessions-spawn-tool.ts index 83c61874d8c..595a0f1b0af 100644 --- a/src/agents/tools/sessions-spawn-tool.ts +++ b/src/agents/tools/sessions-spawn-tool.ts @@ -134,6 +134,7 @@ export function createSessionsSpawnTool(opts?: { cwd, mode: mode && ACP_SPAWN_MODES.includes(mode) ? mode : undefined, thread, + sandbox, }, { agentSessionKey: opts?.agentSessionKey, @@ -141,6 +142,7 @@ export function createSessionsSpawnTool(opts?: { agentAccountId: opts?.agentAccountId, agentTo: opts?.agentTo, agentThreadId: opts?.agentThreadId, + sandboxed: opts?.sandboxed, }, ); return jsonResult(result);