diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index 8383d9a4e1..20ea726c67 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -76,6 +76,16 @@ export function resolveAgentTimeoutMs(timeoutSeconds: number | null | undefined) return process.env.ROO_CLI_RUNTIME === "1" ? 0 : requestedAgentTimeout } +async function commandWorkingDirectoryError(workingDirectory: string): Promise { + try { + await fs.access(workingDirectory) + const stats = await fs.stat(workingDirectory) + return stats.isDirectory() ? undefined : `Working directory '${workingDirectory}' is not a directory.` + } catch { + return `Working directory '${workingDirectory}' does not exist.` + } +} + // Fire-and-forget: some call sites are synchronous terminal callbacks that cannot await, // and postMessageToWebview swallows its own errors, so void is enough. function postCommandExecutionStatus(provider: ClineProvider | undefined, status: CommandExecutionStatus): void { @@ -127,10 +137,20 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { pushToolResult(formatResponse.toolError(parseError.message)) return } - const provider = await task.providerRef.deref() let dcgBlocked = false if (provider?.contextProxy.getValue("destructiveCommandGuardEnabled") === true) { + const workingDirectory = customCwd + ? path.isAbsolute(customCwd) + ? customCwd + : path.resolve(task.cwd, customCwd) + : task.cwd + const workingDirectoryError = await commandWorkingDirectoryError(workingDirectory) + if (workingDirectoryError) { + task.didToolFailInCurrentTurn = true + pushToolResult(formatResponse.toolError(workingDirectoryError)) + return + } const { ensureDcgInstalled, runDcg } = await import("../../services/destructive-command-guard") // Resolve through the managed installer on use so an extension update // automatically installs the newly pinned and verified DCG version. @@ -138,11 +158,8 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> { if (!binaryPath) { throw new Error(t("common:errors.destructiveCommandGuard.unavailable")) } - const workingDirectory = customCwd - ? path.isAbsolute(customCwd) - ? customCwd - : path.resolve(task.cwd, customCwd) - : task.cwd + // Use the same validated directory as terminal execution. A missing cwd + // also surfaces as spawn ENOENT and can be mistaken for a missing binary. const dcgResult = await runDcg(binaryPath, canonicalCommand, workingDirectory) dcgBlocked = dcgResult.decision === "deny" if (dcgResult.decision === "deny") { @@ -272,7 +289,6 @@ export async function executeCommandInTerminal( // Convert milliseconds back to seconds for display purposes. const commandExecutionTimeoutSeconds = commandExecutionTimeout / 1000 let workingDir: string - if (!customCwd) { workingDir = task.cwd } else if (path.isAbsolute(customCwd)) { @@ -280,11 +296,9 @@ export async function executeCommandInTerminal( } else { workingDir = path.resolve(task.cwd, customCwd) } - - try { - await fs.access(workingDir) - } catch (error) { - return [false, `Working directory '${workingDir}' does not exist.`] + const workingDirectoryError = await commandWorkingDirectoryError(workingDir) + if (workingDirectoryError) { + return [false, workingDirectoryError] } let runInBackground = false diff --git a/src/core/tools/__tests__/executeCommand.spec.ts b/src/core/tools/__tests__/executeCommand.spec.ts index 7146fa930b..e3ef938ffe 100644 --- a/src/core/tools/__tests__/executeCommand.spec.ts +++ b/src/core/tools/__tests__/executeCommand.spec.ts @@ -3,15 +3,20 @@ // import * as path from "path" import * as fs from "fs/promises" +import type { Stats } from "fs" import { ExecuteCommandOptions } from "../ExecuteCommandTool" import { TerminalRegistry } from "../../../integrations/terminal/TerminalRegistry" import { Terminal } from "../../../integrations/terminal/Terminal" import { ExecaTerminal } from "../../../integrations/terminal/ExecaTerminal" import type { RooTerminalCallbacks } from "../../../integrations/terminal/types" +const filesystemMocks = vitest.hoisted(() => ({ + access: vitest.fn(), + stat: vitest.fn(), +})) -// Mock fs to control directory existence checks -vitest.mock("fs/promises") +// ExecuteCommandTool uses the default import; this suite configures named imports. +vitest.mock("fs/promises", () => ({ ...filesystemMocks, default: filesystemMocks })) // Mock TerminalRegistry to control terminal creation vitest.mock("../../../integrations/terminal/TerminalRegistry") @@ -33,8 +38,9 @@ describe("executeCommand", () => { beforeEach(() => { vitest.clearAllMocks() - // Mock fs.access to simulate directory existence - ;(fs.access as any).mockResolvedValue(undefined) + // Mock filesystem checks to simulate an accessible directory. + vitest.mocked(fs.access).mockResolvedValue(undefined) + vitest.mocked(fs.stat).mockResolvedValue({ isDirectory: () => true } as Stats) // Create mock provider mockProvider = { @@ -268,6 +274,22 @@ describe("executeCommand", () => { expect(result).toBe(`Working directory '${nonExistentCwd}' does not exist.`) expect(TerminalRegistry.getOrCreateTerminal).not.toHaveBeenCalled() }) + + it("should return error when custom working directory is an existing file", async () => { + const filePath = "/existing/file.txt" + vitest.mocked(fs.stat).mockResolvedValueOnce({ isDirectory: () => false } as Stats) + + const [rejected, result] = await executeCommandInTerminal(mockTask, { + executionId: "test-123", + command: "echo test", + customCwd: filePath, + terminalShellIntegrationDisabled: false, + }) + + expect(rejected).toBe(false) + expect(result).toBe(`Working directory '${filePath}' is not a directory.`) + expect(TerminalRegistry.getOrCreateTerminal).not.toHaveBeenCalled() + }) }) describe("Terminal Provider Selection", () => { diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index a856b180ca..03be2af1d6 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -1,6 +1,8 @@ // npx vitest run src/core/tools/__tests__/executeCommandTool.spec.ts import type { ToolUsage } from "@roo-code/types" +import fs from "fs/promises" +import type { Stats } from "fs" import * as vscode from "vscode" import { Task } from "../../task/Task" @@ -19,6 +21,7 @@ vitest.mock("execa", () => ({ vitest.mock("fs/promises", () => ({ default: { access: vitest.fn().mockResolvedValue(undefined), + stat: vitest.fn().mockResolvedValue({ isDirectory: () => true }), }, })) @@ -356,6 +359,52 @@ describe("executeCommandTool", () => { expect(mockRunDcg).toHaveBeenCalledWith("/test/storage/dcg", "echo test", "/test/workspace") }) + it("rejects a missing working directory before starting DCG", async () => { + const provider = await mockCline.providerRef.deref() + provider.context = { globalStorageUri: { fsPath: "/test/storage" } } + provider.contextProxy.getValue.mockReturnValue(true) + mockToolUse.params.cwd = "/missing/remote/workspace" + mockToolUse.nativeArgs = { command: "echo test", cwd: "/missing/remote/workspace" } + vi.mocked(fs.access).mockRejectedValueOnce(new Error("ENOENT")) + + await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, { + askApproval: mockAskApproval as unknown as AskApproval, + handleError: mockHandleError as unknown as HandleError, + pushToolResult: mockPushToolResult as unknown as PushToolResult, + }) + + expect(mockPushToolResult).toHaveBeenCalledWith( + formatResponse.toolError("Working directory '/missing/remote/workspace' does not exist."), + ) + expect(mockEnsureDcgInstalled).not.toHaveBeenCalled() + expect(mockRunDcg).not.toHaveBeenCalled() + expect(mockAskApproval).not.toHaveBeenCalled() + expect(mockCline.didToolFailInCurrentTurn).toBe(true) + }) + + it("rejects an existing file used as the working directory", async () => { + const provider = await mockCline.providerRef.deref() + provider.context = { globalStorageUri: { fsPath: "/test/storage" } } + provider.contextProxy.getValue.mockReturnValue(true) + mockToolUse.params.cwd = "/remote/workspace/file.txt" + mockToolUse.nativeArgs = { command: "echo test", cwd: "/remote/workspace/file.txt" } + vi.mocked(fs.stat).mockResolvedValueOnce({ isDirectory: () => false } as Stats) + await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, { + askApproval: mockAskApproval as unknown as AskApproval, + handleError: mockHandleError as unknown as HandleError, + pushToolResult: mockPushToolResult as unknown as PushToolResult, + }) + + expect(fs.access).toHaveBeenCalledWith("/remote/workspace/file.txt") + expect(mockPushToolResult).toHaveBeenCalledWith( + formatResponse.toolError("Working directory '/remote/workspace/file.txt' is not a directory."), + ) + expect(mockEnsureDcgInstalled).not.toHaveBeenCalled() + expect(mockRunDcg).not.toHaveBeenCalled() + expect(mockAskApproval).not.toHaveBeenCalled() + expect(mockCline.didToolFailInCurrentTurn).toBe(true) + }) + it("fails closed when the DCG install or update fails", async () => { const provider = await mockCline.providerRef.deref() provider.context = { globalStorageUri: { fsPath: "/test/storage" } } diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 93741e9174..548ef81c88 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -946,7 +946,7 @@ }, "core/tools/__tests__/executeCommand.spec.ts": { "@typescript-eslint/no-explicit-any": { - "count": 12 + "count": 11 } }, "core/tools/__tests__/executeCommandTool.spec.ts": { diff --git a/src/services/destructive-command-guard/__tests__/runner.spec.ts b/src/services/destructive-command-guard/__tests__/runner.spec.ts index 3dcfd805e2..ddf991f5f6 100644 --- a/src/services/destructive-command-guard/__tests__/runner.spec.ts +++ b/src/services/destructive-command-guard/__tests__/runner.spec.ts @@ -138,8 +138,9 @@ describe("runDcg", () => { const result = runDcg("/dcg", "echo test", "/workspace") child.emit("error", new Error("ENOENT")) - await expect(result).rejects.toThrow("Unable to start DCG: ENOENT") - expect(warnSpy).toHaveBeenCalledWith("[DCG]", "Unable to start DCG: ENOENT") + const message = "Unable to start DCG executable '/dcg' in working directory '/workspace': ENOENT" + await expect(result).rejects.toThrow(message) + expect(warnSpy).toHaveBeenCalledWith("[DCG]", message) }) it("rejects excessive output and kills the process", async () => { diff --git a/src/services/destructive-command-guard/runner.ts b/src/services/destructive-command-guard/runner.ts index 3e3b3cab45..6dd8d4663b 100644 --- a/src/services/destructive-command-guard/runner.ts +++ b/src/services/destructive-command-guard/runner.ts @@ -55,7 +55,13 @@ export function runDcg(binaryPath: string, command: string, cwd: string): Promis const timer = setTimeout(() => fail(new Error("DCG evaluation timed out")), DCG_RUN_TIMEOUT_MS) child.stdout?.on("data", (chunk: Buffer) => (stdout = appendOutputOrFail(stdout, chunk))) child.stderr?.on("data", (chunk: Buffer) => (stderr = appendOutputOrFail(stderr, chunk))) - child.on("error", (error) => fail(new Error(`Unable to start DCG: ${error.message}`))) + child.on("error", (error) => + fail( + new Error( + `Unable to start DCG executable '${binaryPath}' in working directory '${cwd}': ${error.message}`, + ), + ), + ) child.on("close", (code, signal) => { if (settled) return if (signal || (code !== 0 && code !== 1)) {