mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 19:10:58 +00:00
fix(coven): tighten daemon trust boundaries
This commit is contained in:
@@ -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 `<covenHome>/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
|
||||
`<covenHome>/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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
|
||||
@@ -33,6 +33,11 @@ const DEFAULT_HARNESSES: Record<string, string> = {
|
||||
};
|
||||
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<string, unknown> {
|
||||
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<string, unknown>) : {};
|
||||
@@ -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<string, string>();
|
||||
|
||||
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) {
|
||||
|
||||
Reference in New Issue
Block a user