From 63e39d7f57ac4ad4a5e38d17e7394ae7c4dd0b9c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 19 Feb 2026 15:40:46 +0100 Subject: [PATCH] fix(security): harden ACP prompt size guardrails --- CHANGELOG.md | 1 + src/acp/client.test.ts | 22 +++++++ src/acp/event-mapper.ts | 3 +- src/acp/translator.session-rate-limit.test.ts | 61 ++++++++++++++++++- src/acp/translator.ts | 12 ++-- 5 files changed, 89 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b617545fad..d5b60cc28ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai - Security/Plugins/Hooks: enforce runtime/package path containment with realpath checks so `openclaw.extensions`, `openclaw.hooks`, and hook handler modules cannot escape their trusted roots via traversal or symlinks. - Security/Discord: centralize trusted sender checks for moderation actions in message-action dispatch, share moderation command parsing across handlers, and clarify permission helpers with explicit any/all semantics. - Security/ACP: harden ACP bridge session management with duplicate-session refresh, idle-session reaping, oldest-idle soft-cap eviction, and burst rate limiting on session creation to reduce local DoS risk without disrupting normal IDE usage. +- Security/ACP: bound ACP prompt text payloads to 2 MiB before gateway forwarding, account for join separator bytes during pre-concatenation size checks, and avoid stale active-run session state when oversized prompts are rejected. Thanks @aether-ai-agent for reporting. - Security/Plugins/Hooks: add optional `--pin` for npm plugin/hook installs, persist resolved npm metadata (`name`, `version`, `spec`, integrity, shasum, timestamp), warn/confirm on integrity drift during updates, and extend `openclaw security audit` to flag unpinned specs, missing integrity metadata, and install-record version drift. - Security/Plugins: harden plugin discovery by blocking unsafe candidates (root escapes, world-writable paths, suspicious ownership), add startup warnings when `plugins.allow` is empty with discoverable non-bundled plugins, and warn on loaded plugins without install/load-path provenance. - Refactor/Plugins: extract shared plugin path-safety utilities, split discovery safety checks into typed reasoned guards, precompute provenance matchers during plugin load, and switch ownership tests to injected uid inputs. diff --git a/src/acp/client.test.ts b/src/acp/client.test.ts index def03c7e8ce..e258942f2cb 100644 --- a/src/acp/client.test.ts +++ b/src/acp/client.test.ts @@ -153,6 +153,28 @@ describe("acp event mapper", () => { expect(text).toBe("Hello\nFile contents\n[Resource link (Spec)] https://example.com"); }); + it("counts newline separators toward prompt byte limits", () => { + expect(() => + extractTextFromPrompt( + [ + { type: "text", text: "a" }, + { type: "text", text: "b" }, + ], + 2, + ), + ).toThrow(/maximum allowed size/i); + + expect( + extractTextFromPrompt( + [ + { type: "text", text: "a" }, + { type: "text", text: "b" }, + ], + 3, + ), + ).toBe("a\nb"); + }); + it("extracts image blocks into gateway attachments", () => { const attachments = extractAttachmentsFromPrompt([ { type: "image", data: "abc", mimeType: "image/png" }, diff --git a/src/acp/event-mapper.ts b/src/acp/event-mapper.ts index 5ba8b02db63..83f4ba07b2d 100644 --- a/src/acp/event-mapper.ts +++ b/src/acp/event-mapper.ts @@ -27,7 +27,8 @@ export function extractTextFromPrompt(prompt: ContentBlock[], maxBytes?: number) if (blockText !== undefined) { // Guard: reject before allocating the full concatenated string if (maxBytes !== undefined) { - totalBytes += Buffer.byteLength(blockText, "utf-8"); + const separatorBytes = parts.length > 0 ? 1 : 0; // "\n" added by join() between blocks + totalBytes += separatorBytes + Buffer.byteLength(blockText, "utf-8"); if (totalBytes > maxBytes) { throw new Error(`Prompt exceeds maximum allowed size of ${maxBytes} bytes`); } diff --git a/src/acp/translator.session-rate-limit.test.ts b/src/acp/translator.session-rate-limit.test.ts index f8e31914be6..fa1ce258c7f 100644 --- a/src/acp/translator.session-rate-limit.test.ts +++ b/src/acp/translator.session-rate-limit.test.ts @@ -2,6 +2,7 @@ import type { AgentSideConnection, LoadSessionRequest, NewSessionRequest, + PromptRequest, } from "@agentclientprotocol/sdk"; import { describe, expect, it, vi } from "vitest"; import type { GatewayClient } from "../gateway/client.js"; @@ -14,9 +15,11 @@ function createConnection(): AgentSideConnection { } as unknown as AgentSideConnection; } -function createGateway(): GatewayClient { +function createGateway( + request: GatewayClient["request"] = vi.fn(async () => ({ ok: true })) as GatewayClient["request"], +): GatewayClient { return { - request: vi.fn(async () => ({ ok: true })), + request, } as unknown as GatewayClient; } @@ -37,6 +40,18 @@ function createLoadSessionRequest(sessionId: string, cwd = "/tmp"): LoadSessionR } as unknown as LoadSessionRequest; } +function createPromptRequest( + sessionId: string, + text: string, + meta: Record = {}, +): PromptRequest { + return { + sessionId, + prompt: [{ type: "text", text }], + _meta: meta, + } as unknown as PromptRequest; +} + describe("acp session creation rate limit", () => { it("rate limits excessive newSession bursts", async () => { const sessionStore = createInMemorySessionStore(); @@ -76,3 +91,45 @@ describe("acp session creation rate limit", () => { sessionStore.clearAllSessionsForTest(); }); }); + +describe("acp prompt size hardening", () => { + it("rejects oversized prompt blocks without leaking active runs", async () => { + const request = vi.fn(async () => ({ ok: true })); + const sessionStore = createInMemorySessionStore(); + const agent = new AcpGatewayAgent(createConnection(), createGateway(request), { + sessionStore, + }); + const sessionId = "prompt-limit-oversize"; + await agent.loadSession(createLoadSessionRequest(sessionId)); + + await expect( + agent.prompt(createPromptRequest(sessionId, "a".repeat(2 * 1024 * 1024 + 1))), + ).rejects.toThrow(/maximum allowed size/i); + expect(request).not.toHaveBeenCalledWith("chat.send", expect.anything(), expect.anything()); + const session = sessionStore.getSession(sessionId); + expect(session?.activeRunId).toBeNull(); + expect(session?.abortController).toBeNull(); + + sessionStore.clearAllSessionsForTest(); + }); + + it("rejects oversize final messages from cwd prefix without leaking active runs", async () => { + const request = vi.fn(async () => ({ ok: true })); + const sessionStore = createInMemorySessionStore(); + const agent = new AcpGatewayAgent(createConnection(), createGateway(request), { + sessionStore, + }); + const sessionId = "prompt-limit-prefix"; + await agent.loadSession(createLoadSessionRequest(sessionId)); + + await expect( + agent.prompt(createPromptRequest(sessionId, "a".repeat(2 * 1024 * 1024))), + ).rejects.toThrow(/maximum allowed size/i); + expect(request).not.toHaveBeenCalledWith("chat.send", expect.anything(), expect.anything()); + const session = sessionStore.getSession(sessionId); + expect(session?.activeRunId).toBeNull(); + expect(session?.abortController).toBeNull(); + + sessionStore.clearAllSessionsForTest(); + }); +}); diff --git a/src/acp/translator.ts b/src/acp/translator.ts index e90437fa7a5..6d01cdb5214 100644 --- a/src/acp/translator.ts +++ b/src/acp/translator.ts @@ -259,10 +259,6 @@ export class AcpGatewayAgent implements Agent { this.sessionStore.cancelActiveRun(params.sessionId); } - const abortController = new AbortController(); - const runId = randomUUID(); - this.sessionStore.setActiveRun(params.sessionId, runId, abortController); - const meta = parseSessionMeta(params._meta); // Pass MAX_PROMPT_BYTES so extractTextFromPrompt rejects oversized content // block-by-block, before the full string is ever assembled in memory (CWE-400) @@ -274,11 +270,13 @@ export class AcpGatewayAgent implements Agent { // Defense-in-depth: also check the final assembled message (includes cwd prefix) if (Buffer.byteLength(message, "utf-8") > MAX_PROMPT_BYTES) { - throw new Error( - `Prompt exceeds maximum allowed size of ${MAX_PROMPT_BYTES} bytes`, - ); + throw new Error(`Prompt exceeds maximum allowed size of ${MAX_PROMPT_BYTES} bytes`); } + const abortController = new AbortController(); + const runId = randomUUID(); + this.sessionStore.setActiveRun(params.sessionId, runId, abortController); + return new Promise((resolve, reject) => { this.pendingPrompts.set(params.sessionId, { sessionId: params.sessionId,