fix(coven): guard runtime controls

This commit is contained in:
Val Alexander
2026-04-27 11:20:46 -05:00
parent c589394450
commit 59100ca4ab
3 changed files with 100 additions and 9 deletions

View File

@@ -132,6 +132,10 @@ function validateSocketPathForUse(socketPath: string, socketRoot: string | undef
if (!pathIsInside(realSocketRoot, realSocketDir)) {
throw new Error("Coven socketPath must stay inside covenHome");
}
const realSocketPath = realpathExistingPath(socketPath, "Coven socketPath");
if (!pathIsInside(realSocketRoot, realSocketPath)) {
throw new Error("Coven socketPath must stay inside covenHome");
}
}
function validateSocketOwnerAndMode(stat: fs.Stats, label: string): void {

View File

@@ -576,6 +576,74 @@ describe("CovenAcpRuntime", () => {
}
});
it("rejects Coven cwd paths that are not directories", async () => {
const filePath = path.join(workspaceDir, "not-a-directory");
await fs.writeFile(filePath, "not a directory");
const runtime = new CovenAcpRuntime({ config, client: fakeClient() });
const handle = await runtime.ensureSession({
sessionKey: "agent:codex:test",
agent: "codex",
mode: "oneshot",
cwd: filePath,
});
await expect(
collect(
runtime.runTurn({
handle,
text: "Fix tests",
mode: "prompt",
requestId: "req-1",
}),
),
).rejects.toThrow(/cwd must be a directory/);
});
it("does not trust persisted backendSessionId without an active tracked Coven session", async () => {
const client = fakeClient();
const runtime = new CovenAcpRuntime({ config, client });
const handle: AcpRuntimeHandle = {
sessionKey: "agent:codex:test",
backend: "coven",
runtimeSessionName: __testing.encodeRuntimeSessionName({
agent: "codex",
mode: "prompt",
}),
cwd: workspaceDir,
backendSessionId: "attacker-session",
};
await expect(runtime.getStatus({ handle })).resolves.toEqual({
summary: "coven runtime ready",
});
await expect(runtime.cancel({ handle })).resolves.toBeUndefined();
await expect(runtime.close({ handle, reason: "user" })).resolves.toBeUndefined();
expect(client.getSession).not.toHaveBeenCalledWith("attacker-session", undefined);
expect(client.killSession).not.toHaveBeenCalledWith("attacker-session", undefined);
});
it("rejects backendSessionId values that conflict with the active tracked Coven session", async () => {
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 turn = runtime.runTurn({ handle, text: "Fix tests", mode: "prompt", requestId: "req-1" });
const iterator = turn[Symbol.asyncIterator]();
await iterator.next();
handle.backendSessionId = "attacker-session";
await expect(runtime.getStatus({ handle })).rejects.toThrow(/does not match/);
await expect(runtime.cancel({ handle })).rejects.toThrow(/does not match/);
await expect(runtime.close({ handle, reason: "user" })).rejects.toThrow(/does not match/);
expect(client.getSession).not.toHaveBeenCalledWith("attacker-session", undefined);
expect(client.killSession).not.toHaveBeenCalledWith("attacker-session", undefined);
await iterator.return?.();
});
it("preserves direct fallback when Coven launch fails after detection", async () => {
const fallback = fallbackRuntime();
registerAcpRuntimeBackend({ id: "acpx", runtime: fallback });

View File

@@ -1,3 +1,4 @@
import fs from "node:fs";
import path from "node:path";
import {
AcpRuntimeError,
@@ -419,9 +420,7 @@ export class CovenAcpRuntime implements AcpRuntime {
? await fallback.getStatus(input)
: { summary: `fallback backend ${input.handle.backend} active` };
}
const sessionId =
input.handle.backendSessionId ??
this.activeSessionIdsBySessionKey.get(input.handle.sessionKey);
const sessionId = this.getTrackedSessionId(input.handle);
if (!sessionId) {
return { summary: "coven runtime ready" };
}
@@ -463,9 +462,7 @@ export class CovenAcpRuntime implements AcpRuntime {
await this.requireFallbackRuntime(input.handle.backend).cancel(input);
return;
}
const sessionId =
input.handle.backendSessionId ??
this.activeSessionIdsBySessionKey.get(input.handle.sessionKey);
const sessionId = this.getTrackedSessionId(input.handle);
if (sessionId) {
await this.killActiveSession(sessionId);
}
@@ -476,9 +473,7 @@ export class CovenAcpRuntime implements AcpRuntime {
await this.requireFallbackRuntime(input.handle.backend).close(input);
return;
}
const sessionId =
input.handle.backendSessionId ??
this.activeSessionIdsBySessionKey.get(input.handle.sessionKey);
const sessionId = this.getTrackedSessionId(input.handle);
if (sessionId && input.reason !== "oneshot-complete") {
await this.killActiveSession(sessionId).catch(() => undefined);
}
@@ -566,9 +561,33 @@ export class CovenAcpRuntime implements AcpRuntime {
if (!workspaceReal || !cwdReal || !pathIsInside(workspaceReal, cwdReal)) {
throw new AcpRuntimeError("ACP_SESSION_INIT_FAILED", "Coven cwd is outside workspace.");
}
try {
if (!fs.statSync(cwdReal).isDirectory()) {
throw new AcpRuntimeError("ACP_SESSION_INIT_FAILED", "Coven cwd must be a directory.");
}
} catch (error) {
if (error instanceof AcpRuntimeError) {
throw error;
}
throw new AcpRuntimeError("ACP_SESSION_INIT_FAILED", "Coven cwd must be a directory.");
}
return cwdReal;
}
private getTrackedSessionId(handle: AcpRuntimeHandle): string | undefined {
const tracked = this.activeSessionIdsBySessionKey.get(handle.sessionKey);
if (!tracked) {
return undefined;
}
if (handle.backendSessionId && handle.backendSessionId !== tracked) {
throw new AcpRuntimeError(
"ACP_INVALID_RUNTIME_OPTION",
"Coven session handle does not match this runtime session.",
);
}
return tracked;
}
private async killActiveSession(sessionId: string, signal?: AbortSignal): Promise<void> {
await this.client.killSession(sessionId, signal);
}