diff --git a/docs/tools/acp-agents-setup.md b/docs/tools/acp-agents-setup.md index 41f4150ff8a..7f71fdb8bb8 100644 --- a/docs/tools/acp-agents-setup.md +++ b/docs/tools/acp-agents-setup.md @@ -218,7 +218,7 @@ Minimal opt-in config: config: { // Optional. Defaults to COVEN_HOME or ~/.coven. covenHome: "~/.coven", - // Optional. Defaults to /coven.sock. + // Optional. Defaults to /coven.sock; overrides must resolve to that path. socketPath: "~/.coven/coven.sock", // Optional. Defaults to false; enable only when direct ACP fallback is acceptable. allowFallback: false, @@ -242,16 +242,18 @@ For path safety, `~` in `covenHome` and `socketPath` expands to the current 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 private (`0700`); OpenClaw rejects symlinked, shared-accessible, -shared-writable, or non-socket Coven socket paths before connecting. +`socketPath` must resolve to `/coven.sock`; OpenClaw does not allow +arbitrary Coven socket filenames because the daemon socket is the local trust +anchor. Keep `covenHome` owned by the OpenClaw user and private (`0700`); +OpenClaw rejects symlinked, shared-accessible, 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 +The default harness mapping sends known ACP agent ids such as `codex`, `claude`, +`gemini`, and `opencode` to explicitly authorized Coven harness ids. Unknown +ACP agent ids are rejected instead of being forwarded as harness names. Override `plugins.entries.coven.config.harnesses` only when your local Coven install uses -custom harness names. +custom harness names, and keep `acp.allowedAgents` aligned with the intended +chat-exposed harness set. ### Automatic dependency install diff --git a/extensions/coven/openclaw.plugin.json b/extensions/coven/openclaw.plugin.json index 6a80d47e178..0bbcbd404bc 100644 --- a/extensions/coven/openclaw.plugin.json +++ b/extensions/coven/openclaw.plugin.json @@ -13,7 +13,7 @@ }, "socketPath": { "type": "string", - "description": "Path to the Coven daemon Unix socket. Defaults to /coven.sock and must stay inside covenHome." + "description": "Path to the Coven daemon Unix socket. Defaults to /coven.sock; overrides must resolve to that fixed socket filename." }, "allowFallback": { "type": "boolean", @@ -30,7 +30,7 @@ "harnesses": { "type": "object", "additionalProperties": { "type": "string" }, - "description": "Map OpenClaw ACP agent ids to Coven harness ids." + "description": "Explicitly map additional OpenClaw ACP agent ids to authorized Coven harness ids. Unknown agent ids are rejected." } } } diff --git a/extensions/coven/package.json b/extensions/coven/package.json index 7f0247eb409..2fdf9518fc9 100644 --- a/extensions/coven/package.json +++ b/extensions/coven/package.json @@ -1,5 +1,5 @@ { - "name": "@openclaw/coven-runtime", + "name": "@openclaw/coven", "version": "2026.4.26", "private": true, "description": "OpenClaw Coven ACP runtime bridge", diff --git a/extensions/coven/src/client.test.ts b/extensions/coven/src/client.test.ts index efd8f79c366..75aac1e4994 100644 --- a/extensions/coven/src/client.test.ts +++ b/extensions/coven/src/client.test.ts @@ -82,6 +82,14 @@ describe("createCovenClient", () => { ); }); + it("rejects oversized event cursors before building the events URL", () => { + expect(() => + createCovenClient("/tmp/coven.sock").listEvents("session-1", { + afterEventId: "e".repeat(257), + }), + ).toThrow(/event id is invalid/); + }); + it("wraps invalid daemon JSON in a typed API error", async () => { await withServer( (_req, res) => { @@ -179,4 +187,28 @@ describe("createCovenClient", () => { /must be a Unix socket/, ); }); + + it("rejects socket path overrides even when they are inside covenHome", async () => { + const covenHome = path.join(tmpDir, ".coven"); + await fs.mkdir(covenHome); + await fs.chmod(covenHome, 0o700); + const socketPath = path.join(covenHome, "other.sock"); + const server = http.createServer((_req, res) => { + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify({ ok: true, daemon: null })); + }); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(socketPath, () => resolve()); + }); + try { + await expect( + createCovenClient(socketPath, { socketRoot: covenHome }).health(), + ).rejects.toThrow(/socketPath must be \/coven\.sock/); + } finally { + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); + } + }); }); diff --git a/extensions/coven/src/client.ts b/extensions/coven/src/client.ts index 1e7ea3fa504..90b5104473b 100644 --- a/extensions/coven/src/client.ts +++ b/extensions/coven/src/client.ts @@ -94,6 +94,9 @@ 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; +const DEFAULT_SOCKET_FILENAME = "coven.sock"; +const SAFE_QUERY_ID_REGEX = /^[A-Za-z0-9._:-]+$/; +const MAX_QUERY_ID_CHARS = 256; function statExistingPath(filePath: string, label: string): fs.Stats { try { @@ -145,6 +148,10 @@ function validateSocketPathForUse( const socketRootStat = statExistingPath(socketRoot, "Coven covenHome"); validateSocketOwnerAndMode(socketRootStat, "Coven covenHome"); validatePrivateDirectory(socketRootStat, "Coven covenHome"); + const expectedSocketPath = path.resolve(socketRoot, DEFAULT_SOCKET_FILENAME); + if (path.resolve(socketPath) !== expectedSocketPath) { + throw new Error("Coven socketPath must be /coven.sock"); + } const socketStat = lstatIfExists(socketPath); if (socketStat?.isSymbolicLink()) { @@ -174,6 +181,14 @@ function validateSocketPathForUse( return fingerprintSocket(resolvedSocketStat); } +function requireSafeQueryId(input: string, label: string): string { + const value = input.trim(); + if (!value || value.length > MAX_QUERY_ID_CHARS || !SAFE_QUERY_ID_REGEX.test(value)) { + throw new Error(`${label} is invalid`); + } + return value; +} + function validateSocketOwnerAndMode(stat: fs.Stats, label: string): void { if (process.platform === "win32") { return; @@ -375,10 +390,12 @@ export function createCovenClient( }); }, listEvents(sessionId, options, signal) { - const params = new URLSearchParams({ sessionId }); + const params = new URLSearchParams({ + sessionId: requireSafeQueryId(sessionId, "Coven session id"), + }); const afterEventId = options?.afterEventId?.trim(); if (afterEventId) { - params.set("afterEventId", afterEventId); + params.set("afterEventId", requireSafeQueryId(afterEventId, "Coven event id")); } return requestJson({ socketPath, diff --git a/extensions/coven/src/config.test.ts b/extensions/coven/src/config.test.ts index 56be19f82d5..2e2a61b60ca 100644 --- a/extensions/coven/src/config.test.ts +++ b/extensions/coven/src/config.test.ts @@ -52,6 +52,18 @@ describe("resolveCovenPluginConfig", () => { ).toThrow(/socketPath must stay inside covenHome/); }); + it("rejects alternate socket filenames inside covenHome", () => { + expect(() => + resolveCovenPluginConfig({ + rawConfig: { + covenHome: "~/.coven", + socketPath: "~/.coven/other.sock", + }, + workspaceDir: "/repo", + }), + ).toThrow(/socketPath overrides are not supported/); + }); + it("rejects socket paths that are symlinks", async () => { const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-coven-config-")); const covenHome = path.join(workspaceDir, ".coven"); diff --git a/extensions/coven/src/config.ts b/extensions/coven/src/config.ts index 316be806752..befcd77c4b0 100644 --- a/extensions/coven/src/config.ts +++ b/extensions/coven/src/config.ts @@ -25,6 +25,7 @@ export type ResolvedCovenPluginConfig = { const DEFAULT_FALLBACK_BACKEND = "acpx"; const DEFAULT_POLL_INTERVAL_MS = 250; +const DEFAULT_SOCKET_FILENAME = "coven.sock"; const nonEmptyString = z.string().trim().min(1); @@ -81,12 +82,14 @@ 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, "socketPath") - : path.join(covenHome, "coven.sock"); + const defaultSocketPath = path.join(covenHome, DEFAULT_SOCKET_FILENAME); + const socketPath = raw?.trim() ? resolveConfiguredPath(raw, "socketPath") : defaultSocketPath; if (!pathIsInside(covenHome, socketPath)) { throw new Error("Coven socketPath must stay inside covenHome"); } + if (socketPath !== defaultSocketPath) { + throw new Error("Coven socketPath overrides are not supported"); + } const socketStat = lstatIfExists(socketPath); if (socketStat?.isSymbolicLink()) { throw new Error("Coven socketPath must not be a symlink"); diff --git a/extensions/coven/src/runtime.test.ts b/extensions/coven/src/runtime.test.ts index 6f457e13f8d..f499020babc 100644 --- a/extensions/coven/src/runtime.test.ts +++ b/extensions/coven/src/runtime.test.ts @@ -226,6 +226,49 @@ describe("CovenAcpRuntime", () => { ]); }); + it("rejects unknown ACP agent ids instead of forwarding them as Coven harness names", async () => { + const client = fakeClient(); + const runtime = new CovenAcpRuntime({ config, client }); + + await expect( + runtime.ensureSession({ + sessionKey: "agent:attacker:test", + agent: "attacker-harness", + mode: "oneshot", + cwd: workspaceDir, + }), + ).rejects.toThrow(/Unknown or unauthorized ACP agent/); + expect(client.health).not.toHaveBeenCalled(); + }); + + it("allows explicit configured agent-to-harness mappings", async () => { + const client = fakeClient(); + const runtime = new CovenAcpRuntime({ + config: { ...config, harnesses: { assistant: "codex" } }, + client, + }); + const handle = await runtime.ensureSession({ + sessionKey: "agent:assistant:test", + agent: "assistant", + mode: "oneshot", + cwd: workspaceDir, + }); + + await collect( + runtime.runTurn({ + handle, + text: "Fix tests", + mode: "prompt", + requestId: "req-1", + }), + ); + + expect(client.launchSession).toHaveBeenCalledWith( + expect.objectContaining({ harness: "codex" }), + undefined, + ); + }); + it("sanitizes daemon-controlled harness fields in start status", async () => { const client = fakeClient({ launchSession: vi.fn(async () => @@ -471,6 +514,37 @@ describe("CovenAcpRuntime", () => { ); }); + it("fails and kills the Coven session when the daemon returns an unsafe event id", async () => { + const client = fakeClient({ + listEvents: vi.fn(async () => [ + event({ + id: "e".repeat(257), + kind: "output", + payloadJson: JSON.stringify({ data: "hello\n" }), + }), + ]), + killSession: vi.fn(async () => undefined), + }); + 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(client.killSession).toHaveBeenCalledWith("session-1", undefined); + expect(events).toEqual([ + expect.objectContaining({ type: "status", text: "coven session session-1 started (codex)" }), + expect.objectContaining({ type: "status", text: "coven session polling failed" }), + expect.objectContaining({ type: "done", stopReason: "error" }), + ]); + }); + it("clamps malformed runtime poll intervals before sleeping", async () => { const sleep = vi.fn(async () => undefined); const client = fakeClient({ @@ -594,6 +668,18 @@ describe("CovenAcpRuntime", () => { ).toContainEqual(expect.objectContaining({ type: "done", stopReason: "completed" })); }); + it("guards daemon exitCode types before rendering terminal status text", () => { + expect( + __testing.terminalStatusEvent( + session({ status: "completed", exitCode: "\u001b[31m1" as unknown as number }), + ), + ).toEqual({ + type: "status", + text: "coven session completed", + tag: "session_info_update", + }); + }); + it("drops oversized daemon event payloads before parsing", () => { expect( __testing.eventToRuntimeEvents( diff --git a/extensions/coven/src/runtime.ts b/extensions/coven/src/runtime.ts index ae44e094bd9..0bb62011d14 100644 --- a/extensions/coven/src/runtime.ts +++ b/extensions/coven/src/runtime.ts @@ -45,6 +45,7 @@ const MAX_RUNTIME_AGENT_CHARS = 128; const MAX_RUNTIME_MODE_CHARS = 32; const MAX_STATUS_FIELD_CHARS = 256; const MAX_SESSION_ID_CHARS = 128; +const MAX_EVENT_ID_CHARS = 256; const SAFE_SESSION_ID_REGEX = /^[A-Za-z0-9._:-]+$/; type CovenRuntimeSessionState = { @@ -191,6 +192,14 @@ function requireSafeSessionId(input: string): string { return value; } +function requireSafeEventId(input: string): string { + const value = input.trim(); + if (!value || value.length > MAX_EVENT_ID_CHARS || !SAFE_SESSION_ID_REGEX.test(value)) { + throw new Error("Coven event id is invalid"); + } + return value; +} + function boundedCovenPrompt(input: string): string { if (Buffer.byteLength(input, "utf8") > MAX_COVEN_PROMPT_BYTES) { throw new Error("Coven prompt exceeded size limit"); @@ -286,6 +295,8 @@ export class CovenAcpRuntime implements AcpRuntime { async ensureSession( input: Parameters[0], ): Promise { + const agent = normalizeAgentId(input.agent); + this.resolveHarness(agent); if (!(await this.isCovenAvailable())) { if (!this.config.allowFallback) { throw new AcpRuntimeError( @@ -295,7 +306,6 @@ export class CovenAcpRuntime implements AcpRuntime { } return await this.ensureFallbackSession(input); } - const agent = normalizeAgentId(input.agent); return { sessionKey: input.sessionKey, backend: COVEN_BACKEND_ID, @@ -322,6 +332,7 @@ export class CovenAcpRuntime implements AcpRuntime { } const cwd = this.resolveWorkspaceCwd(input.handle.cwd); + const harness = this.resolveHarness(state.agent); let session: CovenSessionRecord; let sessionId: string; try { @@ -330,7 +341,7 @@ export class CovenAcpRuntime implements AcpRuntime { { projectRoot: this.config.workspaceDir, cwd, - harness: this.resolveHarness(state.agent), + harness, prompt, title: titleFromPrompt(prompt), }, @@ -380,18 +391,19 @@ export class CovenAcpRuntime implements AcpRuntime { throw new Error("Coven daemon returned too many events"); } for (const event of events) { - if (seenEventIds.has(event.id)) { + const eventId = requireSafeEventId(event.id); + if (seenEventIds.has(eventId)) { continue; } - seenEventIds.add(event.id); - seenEventQueue.push(event.id); + seenEventIds.add(eventId); + seenEventQueue.push(eventId); while (seenEventQueue.length > MAX_TRACKED_EVENT_IDS) { const removed = seenEventQueue.shift(); if (removed) { seenEventIds.delete(removed); } } - lastSeenEventId = event.id; + lastSeenEventId = eventId; for (const runtimeEvent of eventToRuntimeEvents(event)) { yield runtimeEvent; if (runtimeEvent.type === "done") { @@ -522,7 +534,14 @@ export class CovenAcpRuntime implements AcpRuntime { private resolveHarness(agent: string): string { const normalized = normalizeAgentId(agent); - return this.config.harnesses[normalized] ?? DEFAULT_HARNESSES[normalized] ?? normalized; + const harness = this.config.harnesses[normalized] ?? DEFAULT_HARNESSES[normalized]; + if (!harness) { + throw new AcpRuntimeError( + "ACP_INVALID_RUNTIME_OPTION", + `Unknown or unauthorized ACP agent "${normalized}" for Coven backend.`, + ); + } + return harness; } private getFallbackRuntime(backendId = this.config.fallbackBackend): AcpRuntime | null { @@ -618,5 +637,6 @@ export const __testing = { normalizeStopReason, sanitizeStatusField, sanitizeTerminalText, + terminalStatusEvent, titleFromPrompt, }; diff --git a/src/proxy-capture/runtime.test.ts b/src/proxy-capture/runtime.test.ts index 50cad7bc2cc..e14888dcf3e 100644 --- a/src/proxy-capture/runtime.test.ts +++ b/src/proxy-capture/runtime.test.ts @@ -87,6 +87,28 @@ describe("debug proxy runtime", () => { expect(events.some((event) => event.kind === "response")).toBe(true); }); + it("reinstalls ambient global fetch capture when fetch changes after initialization", async () => { + globalThis.fetch = vi.fn(async () => ({ status: 200 }) as Response) as typeof fetch; + + initializeDebugProxyCapture("test"); + const replacementFetch = vi.fn(async () => ({ status: 204 }) as Response) as typeof fetch; + globalThis.fetch = replacementFetch; + initializeDebugProxyCapture("test"); + + await globalThis.fetch("https://api.minimax.io/anthropic/messages", { + method: "POST", + headers: { "content-type": "application/json" }, + body: '{"input":"hello"}', + }); + finalizeDebugProxyCapture(); + + expect(replacementFetch).toHaveBeenCalledTimes(1); + const events = storeState.events.filter((event) => event.sessionId === "runtime-test-session"); + expect(events.some((event) => event.host === "api.minimax.io")).toBe(true); + expect(events.some((event) => event.kind === "request")).toBe(true); + expect(events.some((event) => event.kind === "response" && event.status === 204)).toBe(true); + }); + it("redacts sensitive request and response headers before persistence", async () => { initializeDebugProxyCapture("test"); captureHttpExchange({ diff --git a/src/proxy-capture/runtime.ts b/src/proxy-capture/runtime.ts index 33b3ed8f8bc..0211af64d0b 100644 --- a/src/proxy-capture/runtime.ts +++ b/src/proxy-capture/runtime.ts @@ -41,6 +41,7 @@ const SENSITIVE_CAPTURE_HEADER_NAME_FRAGMENTS = [ type GlobalFetchPatchedState = { originalFetch: typeof globalThis.fetch; + patchedFetch: typeof globalThis.fetch; }; type GlobalFetchPatchTarget = typeof globalThis & { @@ -134,15 +135,16 @@ function installDebugProxyGlobalFetchPatch(settings: DebugProxySettings): void { return; } const patched = globalThis as GlobalFetchPatchTarget; - if (patched[DEBUG_PROXY_FETCH_PATCH_KEY]) { + const existing = patched[DEBUG_PROXY_FETCH_PATCH_KEY]; + if (existing && globalThis.fetch === existing.patchedFetch) { return; } - const originalFetch = globalThis.fetch.bind(globalThis); - patched[DEBUG_PROXY_FETCH_PATCH_KEY] = { originalFetch }; - globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const originalFetch = globalThis.fetch; + const callOriginalFetch = originalFetch.bind(globalThis); + const patchedFetch = (async (input: RequestInfo | URL, init?: RequestInit) => { const url = resolveUrlString(input); try { - const response = await originalFetch(input, init); + const response = await callOriginalFetch(input, init); if (url && /^https?:/i.test(url)) { captureHttpExchange({ url, @@ -199,6 +201,8 @@ function installDebugProxyGlobalFetchPatch(settings: DebugProxySettings): void { throw error; } }) as typeof globalThis.fetch; + patched[DEBUG_PROXY_FETCH_PATCH_KEY] = { originalFetch, patchedFetch }; + globalThis.fetch = patchedFetch; } function uninstallDebugProxyGlobalFetchPatch(): void { @@ -207,12 +211,15 @@ function uninstallDebugProxyGlobalFetchPatch(): void { if (!state) { return; } - globalThis.fetch = state.originalFetch; + if (globalThis.fetch === state.patchedFetch) { + globalThis.fetch = state.originalFetch; + } delete patched[DEBUG_PROXY_FETCH_PATCH_KEY]; } export function isDebugProxyGlobalFetchPatchInstalled(): boolean { - return Boolean((globalThis as GlobalFetchPatchTarget)[DEBUG_PROXY_FETCH_PATCH_KEY]); + const state = (globalThis as GlobalFetchPatchTarget)[DEBUG_PROXY_FETCH_PATCH_KEY]; + return Boolean(state && globalThis.fetch === state.patchedFetch); } export function initializeDebugProxyCapture(mode: string, resolved?: DebugProxySettings): void { diff --git a/test/vitest/vitest.unit-fast-paths.mjs b/test/vitest/vitest.unit-fast-paths.mjs index ecd4951b34f..ca921c5a29b 100644 --- a/test/vitest/vitest.unit-fast-paths.mjs +++ b/test/vitest/vitest.unit-fast-paths.mjs @@ -96,7 +96,6 @@ export const forcedUnitFastTestFiles = [ "src/pairing/allow-from-store-read.test.ts", "src/pairing/pairing-store.test.ts", "src/plugin-sdk/memory-host-events.test.ts", - "src/proxy-capture/runtime.test.ts", "src/proxy-capture/store.sqlite.test.ts", "src/security/audit-exec-surface.test.ts", "src/security/audit-extra.async.test.ts",