diff --git a/docs/tools/acp-agents-setup.md b/docs/tools/acp-agents-setup.md index f5e69df14f1..81efbeaa451 100644 --- a/docs/tools/acp-agents-setup.md +++ b/docs/tools/acp-agents-setup.md @@ -239,7 +239,9 @@ 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. +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 bb771a31f12..13a5e085c58 100644 --- a/extensions/coven/src/client.test.ts +++ b/extensions/coven/src/client.test.ts @@ -50,6 +50,23 @@ describe("createCovenClient", () => { ); }); + it("validates a real socket inside the configured socket root", async () => { + await withServer( + (_req, res) => { + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify({ ok: true, daemon: null })); + }, + async (socketPath) => { + await expect( + createCovenClient(socketPath, { socketRoot: tmpDir }).health(), + ).resolves.toEqual({ + ok: true, + daemon: null, + }); + }, + ); + }); + it("sends the event cursor when listing events", async () => { await withServer( (req, res) => { @@ -87,6 +104,25 @@ describe("createCovenClient", () => { ); }); + it("rejects request bodies above the request size limit", async () => { + await withServer( + (_req, res) => { + res.end("{}"); + }, + async (socketPath) => { + await expect( + createCovenClient(socketPath).launchSession({ + projectRoot: "/repo", + cwd: "/repo", + harness: "codex", + prompt: "x".repeat(1_000_001), + title: "Large prompt", + }), + ).rejects.toThrow(/request exceeded size limit/); + }, + ); + }); + it("revalidates socket paths before connecting", async () => { const covenHome = path.join(tmpDir, ".coven"); await fs.mkdir(covenHome); @@ -108,4 +144,28 @@ describe("createCovenClient", () => { createCovenClient(path.join(symlinkHome, "coven.sock"), { socketRoot: symlinkHome }).health(), ).rejects.toThrow(/covenHome must not be a symlink/); }); + + it("rejects a group or world writable socket root", async () => { + if (process.platform === "win32") { + return; + } + const covenHome = path.join(tmpDir, ".coven"); + await fs.mkdir(covenHome); + await fs.chmod(covenHome, 0o777); + + await expect( + createCovenClient(path.join(covenHome, "coven.sock"), { socketRoot: covenHome }).health(), + ).rejects.toThrow(/covenHome must not be group or world writable/); + }); + + it("rejects socket paths that are not Unix sockets", async () => { + const covenHome = path.join(tmpDir, ".coven"); + await fs.mkdir(covenHome); + const socketPath = path.join(covenHome, "coven.sock"); + await fs.writeFile(socketPath, ""); + + await expect(createCovenClient(socketPath, { socketRoot: covenHome }).health()).rejects.toThrow( + /must be a Unix socket/, + ); + }); }); diff --git a/extensions/coven/src/client.ts b/extensions/coven/src/client.ts index efc25ae420e..baafb6b9b3d 100644 --- a/extensions/coven/src/client.ts +++ b/extensions/coven/src/client.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import http from "node:http"; +import net from "node:net"; import path from "node:path"; export type CovenSessionRecord = { @@ -82,6 +83,7 @@ export class CovenApiError extends Error { } const DEFAULT_REQUEST_TIMEOUT_MS = 10_000; +const MAX_REQUEST_BYTES = 1_000_000; const MAX_RESPONSE_BYTES = 1_000_000; function pathIsInside(parent: string, child: string): boolean { @@ -101,13 +103,23 @@ function validateSocketPathForUse(socketPath: string, socketRoot: string | undef if (!socketRoot) { return; } - if (lstatIfExists(socketRoot)?.isSymbolicLink()) { + const socketRootLstat = lstatIfExists(socketRoot); + if (socketRootLstat?.isSymbolicLink()) { throw new Error("Coven covenHome must not be a symlink"); } + const socketRootStat = fs.statSync(socketRoot); + 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); + 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)); if (!pathIsInside(realSocketRoot, realSocketDir)) { @@ -115,14 +127,54 @@ function validateSocketPathForUse(socketPath: string, socketRoot: string | undef } } +function validateSocketOwnerAndMode(stat: fs.Stats, label: string): void { + if (process.platform === "win32") { + return; + } + const currentUid = typeof process.getuid === "function" ? process.getuid() : null; + if (currentUid != null && stat.uid !== currentUid) { + throw new Error(`${label} must be owned by the current user`); + } + if ((stat.mode & 0o022) !== 0) { + throw new Error(`${label} must not be group or world writable`); + } +} + +function serializeRequestBody(body: unknown): { text: string; byteLength: number } { + if (body === undefined) { + return { text: "", byteLength: 0 }; + } + const text = JSON.stringify(body) ?? ""; + const byteLength = Buffer.byteLength(text, "utf8"); + if (byteLength > MAX_REQUEST_BYTES) { + throw new Error("Coven API request exceeded size limit"); + } + return { text, byteLength }; +} + +function errorToError(error: unknown): Error { + return error instanceof Error ? error : new Error(String(error)); +} + +function socketThatFailsWith(error: unknown): net.Socket { + const socket = new net.Socket(); + queueMicrotask(() => socket.destroy(errorToError(error))); + return socket; +} + function requestOverSocket(options: RequestOptions): Promise { return new Promise((resolve, reject) => { if (options.signal?.aborted) { reject(options.signal.reason ?? new Error("request aborted")); return; } + let requestBody = ""; + let requestBodyBytes = 0; try { validateSocketPathForUse(options.socketPath, options.socketRoot); + const serialized = serializeRequestBody(options.body); + requestBody = serialized.text; + requestBodyBytes = serialized.byteLength; } catch (error) { reject(error); return; @@ -141,10 +193,16 @@ function requestOverSocket(options: RequestOptions): Promise { fn(); }; - const requestBody = options.body === undefined ? "" : JSON.stringify(options.body); const req = http.request( { - socketPath: options.socketPath, + createConnection: () => { + try { + validateSocketPathForUse(options.socketPath, options.socketRoot); + return net.createConnection({ path: options.socketPath }); + } catch (error) { + return socketThatFailsWith(error); + } + }, method: options.method, path: options.path, headers: { @@ -153,7 +211,7 @@ function requestOverSocket(options: RequestOptions): Promise { ...(requestBody ? { "Content-Type": "application/json", - "Content-Length": Buffer.byteLength(requestBody), + "Content-Length": requestBodyBytes, } : {}), }, diff --git a/extensions/coven/src/runtime.test.ts b/extensions/coven/src/runtime.test.ts index e1f9215fb8d..8195e3a0f83 100644 --- a/extensions/coven/src/runtime.test.ts +++ b/extensions/coven/src/runtime.test.ts @@ -205,6 +205,65 @@ describe("CovenAcpRuntime", () => { ]); }); + it("sanitizes daemon-controlled session fields in start status", async () => { + const client = fakeClient({ + launchSession: vi.fn(async () => + session({ + id: "\u001b]0;spoof\u0007session-1\r", + harness: "\u001b[31mcodex\u001b[0m", + }), + ), + }); + 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", + }), + ); + + expect(events).toContainEqual( + expect.objectContaining({ type: "status", text: "coven session session-1 started (codex)" }), + ); + }); + + it("falls back without launching Coven when prompts exceed the Coven request limit", async () => { + const fallback = fallbackRuntime(); + registerAcpRuntimeBackend({ id: "acpx", runtime: fallback }); + const client = fakeClient(); + 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: "x".repeat(500_001), + mode: "prompt", + requestId: "req-1", + }), + ); + + expect(client.launchSession).not.toHaveBeenCalled(); + expect(events).toEqual([ + expect.objectContaining({ type: "text_delta", text: "direct fallback\n" }), + expect.objectContaining({ type: "done", stopReason: "complete" }), + ]); + }); + it("ignores cwd embedded in runtimeSessionName when launching Coven sessions", async () => { const client = fakeClient(); const runtime = new CovenAcpRuntime({ config, client }); diff --git a/extensions/coven/src/runtime.ts b/extensions/coven/src/runtime.ts index 550451af344..df7780e075f 100644 --- a/extensions/coven/src/runtime.ts +++ b/extensions/coven/src/runtime.ts @@ -32,8 +32,10 @@ const DEFAULT_HARNESSES: Record = { opencode: "opencode", }; const HEALTH_CHECK_TIMEOUT_MS = 5_000; +const MAX_COVEN_PROMPT_BYTES = 500_000; const MAX_TRACKED_EVENT_IDS = 10_000; const MAX_RUNTIME_SESSION_NAME_BYTES = 2_048; +const MAX_STATUS_FIELD_CHARS = 256; type CovenRuntimeSessionState = { agent: string; @@ -150,6 +152,17 @@ function sanitizeStatusText(input: string): string { return sanitizeTerminalText(input).replace(/\s+/g, " ").trim(); } +function sanitizeStatusField(input: string, fallback = "unknown"): string { + return sanitizeStatusText(input).slice(0, MAX_STATUS_FIELD_CHARS) || fallback; +} + +function boundedCovenPrompt(input: string): string { + if (Buffer.byteLength(input, "utf8") > MAX_COVEN_PROMPT_BYTES) { + throw new Error("Coven prompt exceeded size limit"); + } + return input; +} + function normalizeStopReason(value: unknown): string { const normalized = typeof value === "string" ? sanitizeStatusText(value).toLowerCase() : "completed"; @@ -172,8 +185,9 @@ function eventToRuntimeEvents(event: CovenEventRecord): AcpRuntimeEvent[] { return text ? [{ type: "text_delta", text, stream: "output", tag: "agent_message_chunk" }] : []; } if (event.kind === "exit") { - const status = sanitizeStatusText( + const status = sanitizeStatusField( typeof payload.status === "string" ? payload.status : "completed", + "completed", ); const exitCode = typeof payload.exitCode === "number" ? payload.exitCode : null; return [ @@ -199,7 +213,7 @@ function sessionIsTerminal(session: CovenSessionRecord): boolean { } function terminalStatusEvent(session: CovenSessionRecord): AcpRuntimeEvent { - const status = sanitizeStatusText(session.status); + const status = sanitizeStatusField(session.status, "completed"); return { type: "status", text: `coven session ${status}${session.exitCode == null ? "" : ` exitCode=${session.exitCode}`}`, @@ -272,13 +286,14 @@ export class CovenAcpRuntime implements AcpRuntime { const cwd = this.resolveWorkspaceCwd(input.handle.cwd); let session: CovenSessionRecord; try { + const prompt = boundedCovenPrompt(input.text); session = await this.client.launchSession( { projectRoot: this.config.workspaceDir, cwd, harness: this.resolveHarness(state.agent), - prompt: input.text, - title: titleFromPrompt(input.text), + prompt, + title: titleFromPrompt(prompt), }, input.signal, ); @@ -295,7 +310,7 @@ export class CovenAcpRuntime implements AcpRuntime { this.activeSessionIdsBySessionKey.set(input.handle.sessionKey, session.id); yield { type: "status", - text: `coven session ${session.id} started (${session.harness})`, + text: `coven session ${sanitizeStatusField(session.id)} started (${sanitizeStatusField(session.harness)})`, tag: "session_info_update", }; @@ -384,14 +399,17 @@ export class CovenAcpRuntime implements AcpRuntime { return { summary: "coven runtime ready" }; } const session = await this.client.getSession(sessionId, input.signal); + const status = sanitizeStatusField(session.status, "completed"); + const harness = sanitizeStatusField(session.harness); + const title = sanitizeStatusField(session.title, "untitled"); return { - summary: `${sanitizeStatusText(session.status)} ${sanitizeStatusText(session.harness)} ${sanitizeStatusText(session.title)}`, + summary: `${status} ${harness} ${title}`, backendSessionId: session.id, agentSessionId: session.id, details: { - projectRoot: sanitizeStatusText(session.projectRoot), - harness: sanitizeStatusText(session.harness), - status: sanitizeStatusText(session.status), + projectRoot: sanitizeStatusField(session.projectRoot), + harness, + status, exitCode: session.exitCode, }, }; @@ -534,6 +552,7 @@ export const __testing = { encodeRuntimeSessionName, eventToRuntimeEvents, normalizeStopReason, + sanitizeStatusField, sanitizeTerminalText, titleFromPrompt, };