diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index 4011f802e9..b08ef9b6e6 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -5,6 +5,7 @@ import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" import { getReadablePath } from "../../utils/path" +import { isPathOutsideWorkspace } from "../../utils/pathUtils" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { fileExistsAtPath } from "../../utils/fs" @@ -138,11 +139,13 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { // Check if file is write-protected const isWriteProtected = task.rooProtectedController?.isWriteProtected(relPath) || false + const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", path: getReadablePath(task.cwd, relPath), diff: diffContent, + isOutsideWorkspace, } if (isPreventFocusDisruptionEnabled) { @@ -332,10 +335,14 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { return } + // relPath is guaranteed non-null after hasPathStabilized + const absolutePath = path.resolve(task.cwd, relPath!) + const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", path: getReadablePath(task.cwd, relPath), diff: diffContent, + isOutsideWorkspace: isPathOutsideWorkspace(absolutePath), } let toolProgressStatus diff --git a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts index bf7b3f5351..7c372645d6 100644 --- a/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts @@ -26,6 +26,10 @@ vi.mock("../../../utils/path", () => ({ getReadablePath: vi.fn().mockReturnValue("test/file.txt"), })) +vi.mock("../../../utils/pathUtils", () => ({ + isPathOutsideWorkspace: vi.fn().mockReturnValue(false), +})) + vi.mock("../../prompts/responses", () => ({ formatResponse: { toolError: vi.fn((message: string) => `Error: ${message}`), diff --git a/src/core/tools/__tests__/applyDiffTool.outsideWorkspace.spec.ts b/src/core/tools/__tests__/applyDiffTool.outsideWorkspace.spec.ts new file mode 100644 index 0000000000..cbaa2433ca --- /dev/null +++ b/src/core/tools/__tests__/applyDiffTool.outsideWorkspace.spec.ts @@ -0,0 +1,236 @@ +// npx vitest run core/tools/__tests__/applyDiffTool.outsideWorkspace.spec.ts + +import fs from "fs/promises" +import path from "path" + +import type { ClineSayTool } from "@roo-code/types" + +import type { ToolUse } from "../../../shared/tools" +import { isPathOutsideWorkspace } from "../../../utils/pathUtils" +import { checkAutoApproval } from "../../auto-approval" +import { baseState } from "../../auto-approval/__tests__/fixtures" +import type { Task } from "../../task/Task" +import type { ToolCallbacks } from "../BaseTool" +import { applyDiffTool } from "../ApplyDiffTool" + +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { instance: { captureDiffApplicationError: vi.fn() } }, +})) + +vi.mock("fs/promises", () => ({ + default: { + readFile: vi.fn(), + }, +})) + +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockResolvedValue(true), +})) + +vi.mock("../../../utils/path", () => ({ + getReadablePath: vi.fn((_cwd: string, relPath?: string) => relPath ?? ""), +})) + +vi.mock("../../../utils/pathUtils", () => ({ + isPathOutsideWorkspace: vi.fn(), +})) + +vi.mock("../../prompts/responses", () => ({ + formatResponse: { + toolError: vi.fn((message: string) => `Error: ${message}`), + rooIgnoreError: vi.fn((filePath: string) => `Access denied: ${filePath}`), + createPrettyPatch: vi.fn(() => "mock-diff"), + }, +})) + +vi.mock("../../diff/stats", () => ({ + sanitizeUnifiedDiff: vi.fn((diff: string) => diff), + computeDiffStats: vi.fn(() => ({ additions: 1, deletions: 1 })), +})) + +vi.mock("vscode", () => ({ + window: { showWarningMessage: vi.fn() }, + env: { openExternal: vi.fn() }, + Uri: { parse: vi.fn() }, + workspace: { + get workspaceFolders() { + return [{ uri: { fsPath: path.resolve(CWD) } }] + }, + }, +})) + +const CWD = "/workspace" +const INSIDE_PATH = "src/index.ts" +const OUTSIDE_PATH = "/home/me/.zshrc" +const DIFF = "<<<<<<< SEARCH\nold\n=======\nnew\n>>>>>>> REPLACE" + +const cases = [ + { name: "a file outside the workspace", relPath: OUTSIDE_PATH, isOutside: true }, + { name: "a file inside the workspace", relPath: INSIDE_PATH, isOutside: false }, +] + +const approvalPaths = [ + { name: "diff view", preventFocusDisruption: false }, + { name: "preventFocusDisruption", preventFocusDisruption: true }, +] + +describe("ApplyDiffTool isOutsideWorkspace", () => { + let askApproval: ReturnType> + + function createTask(preventFocusDisruption = false): Task { + const task = { + cwd: CWD, + consecutiveMistakeCount: 0, + consecutiveMistakeCountForApplyDiff: new Map(), + didEditFile: false, + api: { getModel: () => ({ id: "claude-test" }) }, + rooIgnoreController: { validateAccess: vi.fn().mockReturnValue(true) }, + rooProtectedController: { isWriteProtected: vi.fn().mockReturnValue(false) }, + providerRef: { + deref: vi.fn().mockReturnValue({ + getState: vi.fn().mockResolvedValue({ + diagnosticsEnabled: false, + writeDelayMs: 0, + experiments: { preventFocusDisruption }, + }), + }), + }, + diffStrategy: { + getName: () => "test", + applyDiff: vi.fn().mockResolvedValue({ success: true, content: "new\n" }), + }, + diffViewProvider: { + editType: undefined, + originalContent: "", + open: vi.fn().mockResolvedValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + reset: vi.fn().mockResolvedValue(undefined), + revertChanges: vi.fn().mockResolvedValue(undefined), + saveChanges: vi.fn().mockResolvedValue(undefined), + saveDirectly: vi.fn().mockResolvedValue(undefined), + scrollToFirstDiff: vi.fn(), + pushToolWriteResult: vi.fn().mockResolvedValue("Changes successfully applied."), + }, + fileContextTracker: { trackFileContext: vi.fn().mockResolvedValue(undefined) }, + say: vi.fn().mockResolvedValue(undefined), + ask: vi.fn().mockResolvedValue(undefined), + recordToolError: vi.fn(), + processQueuedMessages: vi.fn(), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing param error"), + } + // The double provides only the Task members that ApplyDiffTool.execute and handlePartial read. + return task as unknown as Task + } + + async function runExecute(relPath: string, isOutside: boolean, preventFocusDisruption = false): Promise { + vi.mocked(isPathOutsideWorkspace).mockReturnValue(isOutside) + + await applyDiffTool.execute({ path: relPath, diff: DIFF }, createTask(preventFocusDisruption), { + askApproval, + handleError: vi.fn().mockResolvedValue(undefined), + pushToolResult: vi.fn(), + toolCallId: "tool-call-1", + }) + + expect(askApproval).toHaveBeenCalledTimes(1) + return String(askApproval.mock.calls[0][1]) + } + + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(isPathOutsideWorkspace).mockReset() + applyDiffTool.resetPartialState() + vi.mocked(fs.readFile).mockResolvedValue("old\n") + askApproval = vi.fn().mockResolvedValue(true) + }) + + describe.each(approvalPaths)("approval payload ($name)", ({ preventFocusDisruption }) => { + it.each(cases)("reports isOutsideWorkspace for $name", async ({ relPath, isOutside }) => { + const payload = JSON.parse(await runExecute(relPath, isOutside, preventFocusDisruption)) as ClineSayTool + + expect(isPathOutsideWorkspace).toHaveBeenCalledWith(path.resolve(CWD, relPath)) + expect(payload.tool).toBe("appliedDiff") + expect(payload.isOutsideWorkspace).toBe(isOutside) + }) + }) + + it.each(cases)("reports isOutsideWorkspace in the partial preview for $name", async ({ relPath, isOutside }) => { + vi.mocked(isPathOutsideWorkspace).mockReturnValue(isOutside) + const task = createTask() + const block: ToolUse<"apply_diff"> = { + type: "tool_use", + name: "apply_diff", + params: { path: relPath, diff: DIFF }, + partial: true, + } + + // The first call records the path; the second call sees it stable and asks. + await applyDiffTool.handlePartial(task, block) + await applyDiffTool.handlePartial(task, block) + + expect(task.ask).toHaveBeenCalledTimes(1) + const payload = JSON.parse(String(vi.mocked(task.ask).mock.calls[0][1])) as ClineSayTool + expect(isPathOutsideWorkspace).toHaveBeenCalledWith(path.resolve(CWD, relPath)) + expect(payload.isOutsideWorkspace).toBe(isOutside) + }) + + describe("with the real workspace check", () => { + it.each([ + { + name: "a ../ path that resolves outside", + relPath: "../outside/config.json", + isOutside: true, + expected: "ask", + }, + { name: "a path inside", relPath: "src/index.ts", isOutside: false, expected: "approve" }, + ])("marks $name the workspace and returns $expected", async ({ relPath, isOutside, expected }) => { + const actual = await vi.importActual("../../../utils/pathUtils") + vi.mocked(isPathOutsideWorkspace).mockImplementation(actual.isPathOutsideWorkspace) + + await applyDiffTool.execute({ path: relPath, diff: DIFF }, createTask(), { + askApproval, + handleError: vi.fn().mockResolvedValue(undefined), + pushToolResult: vi.fn(), + toolCallId: "tool-call-1", + }) + + const text = String(askApproval.mock.calls[0][1]) + const payload = JSON.parse(text) as ClineSayTool + expect(payload.isOutsideWorkspace).toBe(isOutside) + + const result = await checkAutoApproval({ + state: { ...baseState, alwaysAllowWrite: true }, + cwd: CWD, + ask: "tool", + text, + }) + expect(result).toEqual({ decision: expected }) + }) + }) + + describe("auto-approval decision with alwaysAllowWrite on", () => { + it.each([ + { ...cases[0], allowOutside: false, expected: "ask" }, + { ...cases[0], allowOutside: true, expected: "approve" }, + { ...cases[1], allowOutside: false, expected: "approve" }, + ])( + "returns $expected for $name when alwaysAllowWriteOutsideWorkspace is $allowOutside", + async ({ relPath, isOutside, allowOutside, expected }) => { + const text = await runExecute(relPath, isOutside) + + const result = await checkAutoApproval({ + state: { + ...baseState, + alwaysAllowWrite: true, + alwaysAllowWriteOutsideWorkspace: allowOutside, + }, + cwd: CWD, + ask: "tool", + text, + }) + + expect(result).toEqual({ decision: expected }) + }, + ) + }) +})