mirror of
https://github.com/moltbot/moltbot.git
synced 2026-04-26 16:06:16 +00:00
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>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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<string, string>();
|
||||
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=<ms>).`,
|
||||
"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)}`
|
||||
: "",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user