diff --git a/extensions/coven/src/client.ts b/extensions/coven/src/client.ts index 07e59cff7f0..ba65252ac25 100644 --- a/extensions/coven/src/client.ts +++ b/extensions/coven/src/client.ts @@ -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 { diff --git a/extensions/coven/src/runtime.test.ts b/extensions/coven/src/runtime.test.ts index ea7d632a84e..72ef592a750 100644 --- a/extensions/coven/src/runtime.test.ts +++ b/extensions/coven/src/runtime.test.ts @@ -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 }); diff --git a/extensions/coven/src/runtime.ts b/extensions/coven/src/runtime.ts index a547625a808..b15f214ad7b 100644 --- a/extensions/coven/src/runtime.ts +++ b/extensions/coven/src/runtime.ts @@ -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 { await this.client.killSession(sessionId, signal); }