Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 26 additions & 12 deletions src/core/tools/ExecuteCommandTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@
return process.env.ROO_CLI_RUNTIME === "1" ? 0 : requestedAgentTimeout
}

async function commandWorkingDirectoryError(workingDirectory: string): Promise<string | undefined> {
try {
await fs.access(workingDirectory)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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 {
Expand Down Expand Up @@ -127,22 +137,29 @@
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

Check warning on line 150 in src/core/tools/ExecuteCommandTool.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/tools/ExecuteCommandTool.ts:150: Survived BooleanLiteral mutant (replacement: false). See the job summary for the complete list and resolution guidance.
pushToolResult(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.
const binaryPath = await ensureDcgInstalled(provider.context.globalStorageUri.fsPath)
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") {
Expand Down Expand Up @@ -272,19 +289,16 @@
// 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)) {
workingDir = customCwd
} 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) {

Check warning on line 300 in src/core/tools/ExecuteCommandTool.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/tools/ExecuteCommandTool.ts:300: Survived ConditionalExpression mutant (replacement: false). See the job summary for the complete list and resolution guidance.
return [false, workingDirectoryError]

Check warning on line 301 in src/core/tools/ExecuteCommandTool.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/tools/ExecuteCommandTool.ts:301: 2 mutation test gaps; example: NoCoverage ArrayDeclaration mutant (replacement: []). See the job summary for the complete list and resolution guidance.
}

let runInBackground = false
Expand Down
14 changes: 10 additions & 4 deletions src/core/tools/__tests__/executeCommand.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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 = {
Expand Down
47 changes: 47 additions & 0 deletions src/core/tools/__tests__/executeCommandTool.spec.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -19,6 +21,7 @@ vitest.mock("execa", () => ({
vitest.mock("fs/promises", () => ({
default: {
access: vitest.fn().mockResolvedValue(undefined),
stat: vitest.fn().mockResolvedValue({ isDirectory: () => true }),
},
}))

Expand Down Expand Up @@ -356,6 +359,50 @@ 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(
"Working directory '/missing/remote/workspace' does not exist.",
)
expect(mockEnsureDcgInstalled).not.toHaveBeenCalled()
expect(mockRunDcg).not.toHaveBeenCalled()
expect(mockAskApproval).not.toHaveBeenCalled()
})

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(
"Working directory '/remote/workspace/file.txt' is not a directory.",
)
expect(mockEnsureDcgInstalled).not.toHaveBeenCalled()
expect(mockRunDcg).not.toHaveBeenCalled()
expect(mockAskApproval).not.toHaveBeenCalled()
})

it("fails closed when the DCG install or update fails", async () => {
const provider = await mockCline.providerRef.deref()
provider.context = { globalStorageUri: { fsPath: "/test/storage" } }
Expand Down
2 changes: 1 addition & 1 deletion src/eslint-suppressions.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
8 changes: 7 additions & 1 deletion src/services/destructive-command-guard/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Loading