mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 14:10:51 +00:00
fix(coven): harden daemon trust errors
This commit is contained in:
@@ -216,7 +216,7 @@ Minimal opt-in config:
|
||||
coven: {
|
||||
enabled: true,
|
||||
config: {
|
||||
// Optional. Defaults to COVEN_HOME or ~/.coven.
|
||||
// Optional. Defaults to ~/.coven. Environment variables are not used for this trust anchor.
|
||||
covenHome: "~/.coven",
|
||||
// Optional. Defaults to <covenHome>/coven.sock; overrides must resolve to that path.
|
||||
socketPath: "~/.coven/coven.sock",
|
||||
@@ -246,7 +246,9 @@ daemon socket is a local user trust anchor, not repository-controlled state.
|
||||
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.
|
||||
Coven socket paths before connecting. The Coven backend currently requires Unix
|
||||
socket validation and fails closed on Windows rather than trusting a socket path
|
||||
whose owner and permissions cannot be validated by this plugin.
|
||||
|
||||
The default harness mapping sends known ACP agent ids such as `codex`, `claude`,
|
||||
`gemini`, and `opencode` to explicitly authorized Coven harness ids. Unknown
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
"properties": {
|
||||
"covenHome": {
|
||||
"type": "string",
|
||||
"description": "Path to COVEN_HOME. Defaults to COVEN_HOME or ~/.coven."
|
||||
"description": "Path to the Coven daemon state directory. Defaults to ~/.coven; environment variables are not used for this trust anchor."
|
||||
},
|
||||
"socketPath": {
|
||||
"type": "string",
|
||||
|
||||
@@ -3,7 +3,7 @@ import http from "node:http";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { CovenApiError, createCovenClient } from "./client.js";
|
||||
import { __testing, CovenApiError, createCovenClient } from "./client.js";
|
||||
|
||||
let tmpDir: string;
|
||||
|
||||
@@ -211,4 +211,14 @@ describe("createCovenClient", () => {
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("fails closed instead of bypassing socket validation on Windows", () => {
|
||||
expect(() =>
|
||||
__testing.validateSocketPathForUse(
|
||||
path.join(tmpDir, ".coven", "coven.sock"),
|
||||
path.join(tmpDir, ".coven"),
|
||||
"win32",
|
||||
),
|
||||
).toThrow(/not supported on Windows/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -137,17 +137,19 @@ function socketFingerprintMatches(left: SocketFingerprint, right: SocketFingerpr
|
||||
function validateSocketPathForUse(
|
||||
socketPath: string,
|
||||
socketRoot: string | undefined,
|
||||
platform: NodeJS.Platform = process.platform,
|
||||
): SocketFingerprint | null {
|
||||
if (!socketRoot) {
|
||||
return null;
|
||||
}
|
||||
validateSocketPlatform(platform);
|
||||
const socketRootLstat = lstatIfExists(socketRoot);
|
||||
if (socketRootLstat?.isSymbolicLink()) {
|
||||
throw new Error("Coven covenHome must not be a symlink");
|
||||
}
|
||||
const socketRootStat = statExistingPath(socketRoot, "Coven covenHome");
|
||||
validateSocketOwnerAndMode(socketRootStat, "Coven covenHome");
|
||||
validatePrivateDirectory(socketRootStat, "Coven covenHome");
|
||||
validateSocketOwnerAndMode(socketRootStat, "Coven covenHome", platform);
|
||||
validatePrivateDirectory(socketRootStat, "Coven covenHome", platform);
|
||||
const expectedSocketPath = path.resolve(socketRoot, DEFAULT_SOCKET_FILENAME);
|
||||
if (path.resolve(socketPath) !== expectedSocketPath) {
|
||||
throw new Error("Coven socketPath must be <covenHome>/coven.sock");
|
||||
@@ -161,7 +163,7 @@ function validateSocketPathForUse(
|
||||
if (!resolvedSocketStat.isSocket()) {
|
||||
throw new Error("Coven socketPath must be a Unix socket");
|
||||
}
|
||||
validateSocketOwnerAndMode(resolvedSocketStat, "Coven socketPath");
|
||||
validateSocketOwnerAndMode(resolvedSocketStat, "Coven socketPath", platform);
|
||||
|
||||
const realSocketRoot = realpathExistingPath(socketRoot, "Coven covenHome");
|
||||
const realSocketDir = realpathExistingPath(
|
||||
@@ -169,8 +171,8 @@ function validateSocketPathForUse(
|
||||
"Coven socketPath directory",
|
||||
);
|
||||
const socketDirStat = statExistingPath(path.dirname(socketPath), "Coven socketPath directory");
|
||||
validateSocketOwnerAndMode(socketDirStat, "Coven socketPath directory");
|
||||
validatePrivateDirectory(socketDirStat, "Coven socketPath directory");
|
||||
validateSocketOwnerAndMode(socketDirStat, "Coven socketPath directory", platform);
|
||||
validatePrivateDirectory(socketDirStat, "Coven socketPath directory", platform);
|
||||
if (!pathIsInside(realSocketRoot, realSocketDir)) {
|
||||
throw new Error("Coven socketPath must stay inside covenHome");
|
||||
}
|
||||
@@ -181,6 +183,12 @@ function validateSocketPathForUse(
|
||||
return fingerprintSocket(resolvedSocketStat);
|
||||
}
|
||||
|
||||
function validateSocketPlatform(platform: NodeJS.Platform): void {
|
||||
if (platform === "win32") {
|
||||
throw new Error("Coven Unix socket validation is not supported on Windows");
|
||||
}
|
||||
}
|
||||
|
||||
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)) {
|
||||
@@ -189,10 +197,12 @@ function requireSafeQueryId(input: string, label: string): string {
|
||||
return value;
|
||||
}
|
||||
|
||||
function validateSocketOwnerAndMode(stat: fs.Stats, label: string): void {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
function validateSocketOwnerAndMode(
|
||||
stat: fs.Stats,
|
||||
label: string,
|
||||
platform: NodeJS.Platform,
|
||||
): void {
|
||||
validateSocketPlatform(platform);
|
||||
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`);
|
||||
@@ -202,10 +212,8 @@ function validateSocketOwnerAndMode(stat: fs.Stats, label: string): void {
|
||||
}
|
||||
}
|
||||
|
||||
function validatePrivateDirectory(stat: fs.Stats, label: string): void {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
function validatePrivateDirectory(stat: fs.Stats, label: string, platform: NodeJS.Platform): void {
|
||||
validateSocketPlatform(platform);
|
||||
if (!stat.isDirectory()) {
|
||||
throw new Error(`${label} must be a directory`);
|
||||
}
|
||||
@@ -426,3 +434,7 @@ export function createCovenClient(
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export const __testing = {
|
||||
validateSocketPathForUse,
|
||||
};
|
||||
|
||||
@@ -105,7 +105,7 @@ describe("resolveCovenPluginConfig", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("uses COVEN_HOME with tilde expansion for the default socket path", () => {
|
||||
it("ignores COVEN_HOME when resolving the socket trust anchor", () => {
|
||||
process.env.COVEN_HOME = "~/.custom-coven";
|
||||
|
||||
const resolved = resolveCovenPluginConfig({
|
||||
@@ -113,8 +113,8 @@ describe("resolveCovenPluginConfig", () => {
|
||||
workspaceDir: "/repo",
|
||||
});
|
||||
|
||||
expect(resolved.covenHome).toBe(path.join(os.homedir(), ".custom-coven"));
|
||||
expect(resolved.socketPath).toBe(path.join(os.homedir(), ".custom-coven", "coven.sock"));
|
||||
expect(resolved.covenHome).toBe(path.join(os.homedir(), ".coven"));
|
||||
expect(resolved.socketPath).toBe(path.join(os.homedir(), ".coven", "coven.sock"));
|
||||
expect(resolved.allowFallback).toBe(false);
|
||||
});
|
||||
|
||||
|
||||
@@ -71,10 +71,6 @@ function resolveCovenHome(raw: string | undefined): string {
|
||||
if (fromConfig) {
|
||||
return resolveConfiguredPath(fromConfig, "covenHome");
|
||||
}
|
||||
const fromEnv = process.env.COVEN_HOME?.trim();
|
||||
if (fromEnv) {
|
||||
return resolveConfiguredPath(fromEnv, "covenHome");
|
||||
}
|
||||
return path.join(os.homedir(), ".coven");
|
||||
}
|
||||
|
||||
|
||||
@@ -327,6 +327,39 @@ describe("CovenAcpRuntime", () => {
|
||||
).rejects.toThrow(/session id is invalid/);
|
||||
expect(handle.backendSessionId).toBeUndefined();
|
||||
expect(handle.agentSessionId).toBeUndefined();
|
||||
expect(client.killSession).toHaveBeenCalledWith("\u001b]0;spoof\u0007session-1\r", undefined);
|
||||
});
|
||||
|
||||
it("kills an already-launched Coven session before falling back on unsafe session ids", async () => {
|
||||
const fallback = fallbackRuntime();
|
||||
registerAcpRuntimeBackend({ id: "acpx", runtime: fallback });
|
||||
const client = fakeClient({
|
||||
launchSession: vi.fn(async () => session({ id: "bad\nsession" })),
|
||||
killSession: vi.fn(async () => undefined),
|
||||
});
|
||||
const runtime = new CovenAcpRuntime({ config: { ...config, allowFallback: true }, 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("bad\nsession", undefined);
|
||||
expect(handle.backend).toBe("acpx");
|
||||
expect(events).toEqual([
|
||||
expect.objectContaining({ type: "text_delta", text: "direct fallback\n" }),
|
||||
expect.objectContaining({ type: "done", stopReason: "complete" }),
|
||||
]);
|
||||
});
|
||||
|
||||
it("fails closed without launching Coven when prompts exceed the Coven request limit", async () => {
|
||||
@@ -639,6 +672,29 @@ describe("CovenAcpRuntime", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("sanitizes Coven polling errors before logging", async () => {
|
||||
const logger = { warn: vi.fn(), info: vi.fn(), error: vi.fn(), debug: vi.fn() };
|
||||
const client = fakeClient({
|
||||
listEvents: vi.fn(async () => {
|
||||
throw new Error("\u001b]0;spoof\u0007bad\r\njson");
|
||||
}),
|
||||
killSession: vi.fn(async () => undefined),
|
||||
});
|
||||
const runtime = new CovenAcpRuntime({ config, client, logger });
|
||||
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(logger.warn).toHaveBeenCalledWith("coven polling failed: Error: bad json");
|
||||
});
|
||||
|
||||
it("strips terminal escape and control characters from Coven output", () => {
|
||||
expect(
|
||||
__testing.sanitizeTerminalText(
|
||||
@@ -841,7 +897,7 @@ describe("CovenAcpRuntime", () => {
|
||||
config,
|
||||
client: fakeClient({
|
||||
launchSession: vi.fn(async () => {
|
||||
throw new Error("launch failed");
|
||||
throw new Error("\u001b]0;spoof\u0007launch\r\nfailed");
|
||||
}),
|
||||
}),
|
||||
});
|
||||
@@ -854,6 +910,22 @@ describe("CovenAcpRuntime", () => {
|
||||
|
||||
await expect(
|
||||
collect(runtime.runTurn({ handle, text: "Fix tests", mode: "prompt", requestId: "req-1" })),
|
||||
).rejects.toThrow(/fallback is disabled/);
|
||||
).rejects.toThrow(/Error: launch failed/);
|
||||
});
|
||||
|
||||
it("sanitizes Coven doctor error details", async () => {
|
||||
const runtime = new CovenAcpRuntime({
|
||||
config,
|
||||
client: fakeClient({
|
||||
health: vi.fn(async () => {
|
||||
throw new Error("\u001b[31moffline\r\nnow");
|
||||
}),
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(runtime.doctor()).resolves.toMatchObject({
|
||||
ok: false,
|
||||
details: ["Error: offline now"],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -184,6 +184,11 @@ function sanitizeStatusField(input: string, fallback = "unknown"): string {
|
||||
return sanitizeStatusText(input).slice(0, MAX_STATUS_FIELD_CHARS) || fallback;
|
||||
}
|
||||
|
||||
function sanitizeErrorText(error: unknown): string {
|
||||
const raw = error instanceof Error ? `${error.name}: ${error.message}` : String(error);
|
||||
return sanitizeStatusField(raw, "unknown error");
|
||||
}
|
||||
|
||||
function requireSafeSessionId(input: string): string {
|
||||
const value = input.trim();
|
||||
if (!value || value.length > MAX_SESSION_ID_CHARS || !SAFE_SESSION_ID_REGEX.test(value)) {
|
||||
@@ -333,7 +338,7 @@ export class CovenAcpRuntime implements AcpRuntime {
|
||||
|
||||
const cwd = this.resolveWorkspaceCwd(input.handle.cwd);
|
||||
const harness = this.resolveHarness(state.agent);
|
||||
let session: CovenSessionRecord;
|
||||
let session: CovenSessionRecord | undefined;
|
||||
let sessionId: string;
|
||||
try {
|
||||
const prompt = boundedCovenPrompt(input.text);
|
||||
@@ -347,17 +352,38 @@ export class CovenAcpRuntime implements AcpRuntime {
|
||||
},
|
||||
input.signal,
|
||||
);
|
||||
sessionId = requireSafeSessionId(session.id);
|
||||
} catch (error) {
|
||||
const safeError = sanitizeErrorText(error);
|
||||
if (!this.config.allowFallback) {
|
||||
throw new AcpRuntimeError(
|
||||
"ACP_TURN_FAILED",
|
||||
`Coven launch failed and fallback is disabled: ${String(error)}`,
|
||||
`Coven launch failed and fallback is disabled: ${safeError}`,
|
||||
{ cause: error },
|
||||
);
|
||||
}
|
||||
this.logger?.warn(
|
||||
`coven launch failed; falling back to ${this.config.fallbackBackend}: ${String(error)}`,
|
||||
`coven launch failed; falling back to ${this.config.fallbackBackend}: ${safeError}`,
|
||||
);
|
||||
yield* this.runFallbackFromCovenHandle(input, state);
|
||||
return;
|
||||
}
|
||||
try {
|
||||
if (!session) {
|
||||
throw new Error("Coven launch did not return a session");
|
||||
}
|
||||
sessionId = requireSafeSessionId(session.id);
|
||||
} catch (error) {
|
||||
await this.killLaunchedSessionBestEffort(session?.id);
|
||||
const safeError = sanitizeErrorText(error);
|
||||
if (!this.config.allowFallback) {
|
||||
throw new AcpRuntimeError(
|
||||
"ACP_TURN_FAILED",
|
||||
`Coven launch failed and fallback is disabled: ${safeError}`,
|
||||
{ cause: error },
|
||||
);
|
||||
}
|
||||
this.logger?.warn(
|
||||
`coven launch failed; falling back to ${this.config.fallbackBackend}: ${safeError}`,
|
||||
);
|
||||
yield* this.runFallbackFromCovenHandle(input, state);
|
||||
return;
|
||||
@@ -425,7 +451,7 @@ export class CovenAcpRuntime implements AcpRuntime {
|
||||
await this.killActiveSession(sessionId).catch(() => undefined);
|
||||
throw input.signal.reason ?? error;
|
||||
}
|
||||
this.logger?.warn(`coven polling failed: ${String(error)}`);
|
||||
this.logger?.warn(`coven polling failed: ${sanitizeErrorText(error)}`);
|
||||
await this.killActiveSession(sessionId).catch(() => undefined);
|
||||
this.activeSessionIdsBySessionKey.delete(input.handle.sessionKey);
|
||||
yield { type: "status", text: "coven session polling failed", tag: "session_info_update" };
|
||||
@@ -482,7 +508,7 @@ export class CovenAcpRuntime implements AcpRuntime {
|
||||
ok: false,
|
||||
code: "COVEN_UNAVAILABLE",
|
||||
message: "Coven daemon is not reachable; direct ACP fallback remains available.",
|
||||
details: [String(error)],
|
||||
details: [sanitizeErrorText(error)],
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -628,6 +654,13 @@ export class CovenAcpRuntime implements AcpRuntime {
|
||||
private async killActiveSession(sessionId: string, signal?: AbortSignal): Promise<void> {
|
||||
await this.client.killSession(sessionId, signal);
|
||||
}
|
||||
|
||||
private async killLaunchedSessionBestEffort(sessionId: string | undefined): Promise<void> {
|
||||
if (!sessionId) {
|
||||
return;
|
||||
}
|
||||
await this.client.killSession(sessionId, undefined).catch(() => undefined);
|
||||
}
|
||||
}
|
||||
|
||||
export const __testing = {
|
||||
@@ -635,6 +668,7 @@ export const __testing = {
|
||||
encodeRuntimeSessionName,
|
||||
eventToRuntimeEvents,
|
||||
normalizeStopReason,
|
||||
sanitizeErrorText,
|
||||
sanitizeStatusField,
|
||||
sanitizeTerminalText,
|
||||
terminalStatusEvent,
|
||||
|
||||
Reference in New Issue
Block a user