mirror of https://github.com/openclaw/openclaw.git
fix(security): harden ACP prompt size guardrails
This commit is contained in:
parent
ebcf19746f
commit
63e39d7f57
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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" },
|
||||
|
|
|
|||
|
|
@ -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`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<string, unknown> = {},
|
||||
): 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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<PromptResponse>((resolve, reject) => {
|
||||
this.pendingPrompts.set(params.sessionId, {
|
||||
sessionId: params.sessionId,
|
||||
|
|
|
|||
Loading…
Reference in New Issue