diff --git a/docs/tools/acp-agents-setup.md b/docs/tools/acp-agents-setup.md index 81efbeaa451..4a242fc7e6c 100644 --- a/docs/tools/acp-agents-setup.md +++ b/docs/tools/acp-agents-setup.md @@ -236,12 +236,14 @@ fails, OpenClaw falls back to the configured direct ACP backend (`acpx` by default) instead of breaking existing ACP behavior. For path safety, `~` in `covenHome` and `socketPath` expands to the current -user home directory. Relative Coven paths resolve from the OpenClaw workspace, -not from the process working directory. `socketPath` must stay inside -`covenHome`; use the default `/coven.sock` unless your Coven daemon -uses a different socket filename in the same home directory. Keep `covenHome` -owned by the OpenClaw user and not group/world-writable; OpenClaw rejects -symlinked, shared-writable, or non-socket Coven socket paths before connecting. +user home directory, and configured Coven paths must be absolute after that +expansion. OpenClaw rejects workspace-relative Coven daemon paths because the +daemon socket is a local user trust anchor, not repository-controlled state. +`socketPath` must stay inside `covenHome`; use the default +`/coven.sock` unless your Coven daemon uses a different socket +filename in the same home directory. Keep `covenHome` owned by the OpenClaw user +and not group/world-writable; OpenClaw rejects symlinked, shared-writable, or +non-socket Coven socket paths before connecting. The default harness mapping sends common ACP agent ids such as `codex`, `claude`, `gemini`, and `opencode` to the matching Coven harness id. Override diff --git a/extensions/coven/src/client.test.ts b/extensions/coven/src/client.test.ts index 13a5e085c58..89367f58e54 100644 --- a/extensions/coven/src/client.test.ts +++ b/extensions/coven/src/client.test.ts @@ -145,6 +145,14 @@ describe("createCovenClient", () => { ).rejects.toThrow(/covenHome must not be a symlink/); }); + it("rejects missing socket roots with a validation error", async () => { + const covenHome = path.join(tmpDir, "missing-coven"); + + await expect( + createCovenClient(path.join(covenHome, "coven.sock"), { socketRoot: covenHome }).health(), + ).rejects.toThrow(/covenHome must exist/); + }); + it("rejects a group or world writable socket root", async () => { if (process.platform === "win32") { return; diff --git a/extensions/coven/src/client.ts b/extensions/coven/src/client.ts index baafb6b9b3d..f77a462c236 100644 --- a/extensions/coven/src/client.ts +++ b/extensions/coven/src/client.ts @@ -99,6 +99,22 @@ function lstatIfExists(filePath: string): fs.Stats | null { } } +function statExistingPath(filePath: string, label: string): fs.Stats { + try { + return fs.statSync(filePath); + } catch { + throw new Error(`${label} must exist`); + } +} + +function realpathExistingPath(filePath: string, label: string): string { + try { + return fs.realpathSync.native(filePath); + } catch { + throw new Error(`${label} must exist`); + } +} + function validateSocketPathForUse(socketPath: string, socketRoot: string | undefined): void { if (!socketRoot) { return; @@ -107,21 +123,24 @@ function validateSocketPathForUse(socketPath: string, socketRoot: string | undef if (socketRootLstat?.isSymbolicLink()) { throw new Error("Coven covenHome must not be a symlink"); } - const socketRootStat = fs.statSync(socketRoot); + const socketRootStat = statExistingPath(socketRoot, "Coven covenHome"); validateSocketOwnerAndMode(socketRootStat, "Coven covenHome"); const socketStat = lstatIfExists(socketPath); if (socketStat?.isSymbolicLink()) { throw new Error("Coven socketPath must not be a symlink"); } - const resolvedSocketStat = fs.statSync(socketPath); + const resolvedSocketStat = statExistingPath(socketPath, "Coven socketPath"); if (!resolvedSocketStat.isSocket()) { throw new Error("Coven socketPath must be a Unix socket"); } validateSocketOwnerAndMode(resolvedSocketStat, "Coven socketPath"); - const realSocketRoot = fs.realpathSync.native(socketRoot); - const realSocketDir = fs.realpathSync.native(path.dirname(socketPath)); + const realSocketRoot = realpathExistingPath(socketRoot, "Coven covenHome"); + const realSocketDir = realpathExistingPath( + path.dirname(socketPath), + "Coven socketPath directory", + ); if (!pathIsInside(realSocketRoot, realSocketDir)) { throw new Error("Coven socketPath must stay inside covenHome"); } diff --git a/extensions/coven/src/config.test.ts b/extensions/coven/src/config.test.ts index e249ee2854e..eb6848a13d5 100644 --- a/extensions/coven/src/config.test.ts +++ b/extensions/coven/src/config.test.ts @@ -28,18 +28,16 @@ describe("resolveCovenPluginConfig", () => { expect(resolved.socketPath).toBe(path.join(os.homedir(), ".coven", "coven.sock")); }); - it("resolves relative Coven paths from the workspace instead of process cwd", () => { - const resolved = resolveCovenPluginConfig({ - rawConfig: { - covenHome: ".coven", - socketPath: ".coven/coven.sock", - }, - workspaceDir: "/repo", - }); - - expect(resolved.workspaceDir).toBe("/repo"); - expect(resolved.covenHome).toBe("/repo/.coven"); - expect(resolved.socketPath).toBe("/repo/.coven/coven.sock"); + it("rejects relative Coven paths instead of trusting workspace contents", () => { + expect(() => + resolveCovenPluginConfig({ + rawConfig: { + covenHome: ".coven", + socketPath: ".coven/coven.sock", + }, + workspaceDir: "/repo", + }), + ).toThrow(/covenHome must be absolute/); }); it("rejects socket paths outside covenHome", () => { diff --git a/extensions/coven/src/config.ts b/extensions/coven/src/config.ts index cdc4db88aa8..49e590a7c58 100644 --- a/extensions/coven/src/config.ts +++ b/extensions/coven/src/config.ts @@ -54,9 +54,12 @@ function expandTilde(raw: string): string { return trimmed; } -function resolveConfiguredPath(raw: string, baseDir: string): string { +function resolveConfiguredPath(raw: string, label: "covenHome" | "socketPath"): string { const expanded = expandTilde(raw); - return path.isAbsolute(expanded) ? path.resolve(expanded) : path.resolve(baseDir, expanded); + if (!path.isAbsolute(expanded)) { + throw new Error(`Coven ${label} must be absolute`); + } + return path.resolve(expanded); } function pathIsInside(parent: string, child: string): boolean { @@ -80,24 +83,24 @@ function lstatIfExists(filePath: string): fs.Stats | null { } } -function resolveCovenHome(raw: string | undefined, baseDir: string): string { +function resolveCovenHome(raw: string | undefined): string { const fromConfig = raw?.trim(); if (fromConfig) { - return resolveConfiguredPath(fromConfig, baseDir); + return resolveConfiguredPath(fromConfig, "covenHome"); } const fromEnv = process.env.COVEN_HOME?.trim(); if (fromEnv) { - return resolveConfiguredPath(fromEnv, baseDir); + return resolveConfiguredPath(fromEnv, "covenHome"); } return path.join(os.homedir(), ".coven"); } -function resolveSocketPath(covenHome: string, raw: string | undefined, baseDir: string): string { +function resolveSocketPath(covenHome: string, raw: string | undefined): string { if (lstatIfExists(covenHome)?.isSymbolicLink()) { throw new Error("Coven covenHome must not be a symlink"); } const socketPath = raw?.trim() - ? resolveConfiguredPath(raw, baseDir) + ? resolveConfiguredPath(raw, "socketPath") : path.join(covenHome, "coven.sock"); if (!pathIsInside(covenHome, socketPath)) { throw new Error("Coven socketPath must stay inside covenHome"); @@ -138,10 +141,10 @@ export function resolveCovenPluginConfig(params: { } const config = parsed.data as CovenPluginConfig; const workspaceDir = path.resolve(params.workspaceDir ?? process.cwd()); - const covenHome = resolveCovenHome(config.covenHome, workspaceDir); + const covenHome = resolveCovenHome(config.covenHome); return { covenHome, - socketPath: resolveSocketPath(covenHome, config.socketPath, workspaceDir), + socketPath: resolveSocketPath(covenHome, config.socketPath), workspaceDir, fallbackBackend: normalizeBackendId(config.fallbackBackend), pollIntervalMs: config.pollIntervalMs ?? DEFAULT_POLL_INTERVAL_MS, diff --git a/extensions/coven/src/runtime.test.ts b/extensions/coven/src/runtime.test.ts index 8195e3a0f83..b982d1ccde2 100644 --- a/extensions/coven/src/runtime.test.ts +++ b/extensions/coven/src/runtime.test.ts @@ -18,7 +18,7 @@ const baseConfig: ResolvedCovenPluginConfig = { socketPath: "", workspaceDir: "", fallbackBackend: "acpx", - pollIntervalMs: 1, + pollIntervalMs: 25, harnesses: {}, }; @@ -394,6 +394,72 @@ describe("CovenAcpRuntime", () => { ); }); + it("clamps malformed runtime poll intervals before sleeping", async () => { + const sleep = vi.fn(async () => undefined); + const client = fakeClient({ + listEvents: vi + .fn() + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([ + event({ + id: "event-1", + kind: "exit", + payloadJson: JSON.stringify({ status: "completed", exitCode: 0 }), + }), + ]), + getSession: vi.fn(async () => session({ status: "running" })), + }); + const runtime = new CovenAcpRuntime({ + config: { ...config, pollIntervalMs: 0 }, + client, + sleep, + }); + const handle = await runtime.ensureSession({ + sessionKey: "agent:codex:test", + agent: "codex", + mode: "oneshot", + cwd: workspaceDir, + }); + + await collect( + runtime.runTurn({ handle, text: "Fix tests", mode: "prompt", requestId: "req-1" }), + ); + + expect(sleep).toHaveBeenCalledWith(25, undefined); + }); + + it("bounds daemon events processed during one poll cycle", async () => { + const client = fakeClient({ + listEvents: vi.fn(async () => + Array.from({ length: 600 }, (_, index) => + event({ + id: `event-${index}`, + kind: "output", + payloadJson: JSON.stringify({ data: `line-${index}\n` }), + }), + ), + ), + getSession: vi.fn(async () => session({ status: "completed", exitCode: 0 })), + }); + const runtime = new CovenAcpRuntime({ config, client }); + const handle = await runtime.ensureSession({ + sessionKey: "agent:codex:test", + agent: "codex", + mode: "oneshot", + cwd: workspaceDir, + }); + + const events = await collect( + runtime.runTurn({ handle, text: "Fix tests", mode: "prompt", requestId: "req-1" }), + ); + + const outputEvents = events.filter((item) => item.type === "text_delta"); + expect(outputEvents).toHaveLength(500); + expect(outputEvents).not.toContainEqual( + expect.objectContaining({ type: "text_delta", text: "line-500\n" }), + ); + }); + it("converts Coven polling failures into controlled terminal events", async () => { const client = fakeClient({ listEvents: vi.fn(async () => { @@ -450,6 +516,17 @@ describe("CovenAcpRuntime", () => { ).toContainEqual(expect.objectContaining({ type: "done", stopReason: "completed" })); }); + it("drops oversized daemon event payloads before parsing", () => { + expect( + __testing.eventToRuntimeEvents( + event({ + kind: "output", + payloadJson: JSON.stringify({ data: "x".repeat(64_001) }), + }), + ), + ).toEqual([]); + }); + it("rejects oversized Coven runtime session metadata", () => { expect(__testing.decodeRuntimeSessionName(`coven:${"a".repeat(2_049)}`)).toBeNull(); }); diff --git a/extensions/coven/src/runtime.ts b/extensions/coven/src/runtime.ts index df7780e075f..d13cb8f456a 100644 --- a/extensions/coven/src/runtime.ts +++ b/extensions/coven/src/runtime.ts @@ -33,6 +33,11 @@ const DEFAULT_HARNESSES: Record = { }; const HEALTH_CHECK_TIMEOUT_MS = 5_000; const MAX_COVEN_PROMPT_BYTES = 500_000; +const MIN_POLL_INTERVAL_MS = 25; +const MAX_POLL_INTERVAL_MS = 10_000; +const DEFAULT_POLL_INTERVAL_MS = 250; +const MAX_EVENTS_PER_POLL = 500; +const MAX_EVENT_PAYLOAD_BYTES = 64_000; const MAX_TRACKED_EVENT_IDS = 10_000; const MAX_RUNTIME_SESSION_NAME_BYTES = 2_048; const MAX_STATUS_FIELD_CHARS = 256; @@ -114,6 +119,9 @@ function titleFromPrompt(prompt: string): string { } function parsePayload(event: CovenEventRecord): Record { + if (Buffer.byteLength(event.payloadJson, "utf8") > MAX_EVENT_PAYLOAD_BYTES) { + return {}; + } try { const parsed = JSON.parse(event.payloadJson) as unknown; return typeof parsed === "object" && parsed !== null ? (parsed as Record) : {}; @@ -163,6 +171,13 @@ function boundedCovenPrompt(input: string): string { return input; } +function normalizePollIntervalMs(value: number): number { + if (!Number.isFinite(value)) { + return DEFAULT_POLL_INTERVAL_MS; + } + return Math.min(MAX_POLL_INTERVAL_MS, Math.max(MIN_POLL_INTERVAL_MS, value)); +} + function normalizeStopReason(value: unknown): string { const normalized = typeof value === "string" ? sanitizeStatusText(value).toLowerCase() : "completed"; @@ -242,7 +257,10 @@ export class CovenAcpRuntime implements AcpRuntime { private readonly activeSessionIdsBySessionKey = new Map(); constructor(params: CovenAcpRuntimeParams) { - this.config = params.config; + this.config = { + ...params.config, + pollIntervalMs: normalizePollIntervalMs(params.config.pollIntervalMs), + }; this.logger = params.logger; this.client = params.client ?? @@ -333,10 +351,15 @@ export class CovenAcpRuntime implements AcpRuntime { ? events.findIndex((event) => event.id === lastSeenEventId) : -1; const nextEvents = cursorIndex >= 0 ? events.slice(cursorIndex + 1) : events; + let processedEvents = 0; for (const event of nextEvents) { if (seenEventIds.has(event.id)) { continue; } + if (processedEvents >= MAX_EVENTS_PER_POLL) { + break; + } + processedEvents += 1; seenEventIds.add(event.id); seenEventQueue.push(event.id); while (seenEventQueue.length > MAX_TRACKED_EVENT_IDS) {