Limit search_files to only look within the workspace (#4642)

Author: mrubens

src/core/tools/__tests__/searchFilesTool.spec.ts

@@ -0,0 +1,300 @@
+import path from "path"
+import { describe, it, expect, beforeEach, vi, type Mock, type MockedFunction } from "vitest"
+import { searchFilesTool } from "../searchFilesTool"
+import { Task } from "../../task/Task"
+import { SearchFilesToolUse } from "../../../shared/tools"
+import { isPathOutsideWorkspace } from "../../../utils/pathUtils"
+import { regexSearchFiles } from "../../../services/ripgrep"
+import { RooIgnoreController } from "../../ignore/RooIgnoreController"
+
+// Mock dependencies
+vi.mock("../../../utils/pathUtils", () => ({
+	isPathOutsideWorkspace: vi.fn(),
+}))
+
+vi.mock("../../../services/ripgrep", () => ({
+	regexSearchFiles: vi.fn(),
+}))
+
+vi.mock("../../../utils/path", () => ({
+	getReadablePath: vi.fn((cwd: string, relPath: string) => relPath),
+}))
+
+vi.mock("../../ignore/RooIgnoreController")
+
+vi.mock("../../../i18n", () => ({
+	t: vi.fn((key: string, params?: any) => {
+		if (key === "tools:searchFiles.workspaceBoundaryError") {
+			return `Cannot search outside workspace. Path '${params?.path}' is outside the current workspace.`
+		}
+		return key
+	}),
+}))
+
+const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction<typeof isPathOutsideWorkspace>
+const mockedRegexSearchFiles = regexSearchFiles as MockedFunction<typeof regexSearchFiles>
+
+describe("searchFilesTool", () => {
+	let mockTask: Partial<Task>
+	let mockAskApproval: Mock
+	let mockHandleError: Mock
+	let mockPushToolResult: Mock
+	let mockRemoveClosingTag: Mock
+
+	beforeEach(() => {
+		vi.clearAllMocks()
+
+		mockTask = {
+			cwd: "/workspace",
+			consecutiveMistakeCount: 0,
+			recordToolError: vi.fn(),
+			sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
+			say: vi.fn().mockResolvedValue(undefined),
+			rooIgnoreController: new RooIgnoreController("/workspace"),
+		}
+
+		mockAskApproval = vi.fn().mockResolvedValue(true)
+		mockHandleError = vi.fn()
+		mockPushToolResult = vi.fn()
+		mockRemoveClosingTag = vi.fn((tag: string, value: string | undefined) => value || "")
+
+		mockedRegexSearchFiles.mockResolvedValue("Search results")
+	})
+
+	describe("workspace boundary validation", () => {
+		it("should allow search within workspace", async () => {
+			const block: SearchFilesToolUse = {
+				type: "tool_use",
+				name: "search_files",
+				params: {
+					path: "src",
+					regex: "test",
+					file_pattern: "*.ts",
+				},
+				partial: false,
+			}
+
+			mockedIsPathOutsideWorkspace.mockReturnValue(false)
+
+			await searchFilesTool(
+				mockTask as Task,
+				block,
+				mockAskApproval,
+				mockHandleError,
+				mockPushToolResult,
+				mockRemoveClosingTag,
+			)
+
+			expect(mockedIsPathOutsideWorkspace).toHaveBeenCalledWith(path.resolve("/workspace", "src"))
+			expect(mockedRegexSearchFiles).toHaveBeenCalled()
+			expect(mockPushToolResult).toHaveBeenCalledWith("Search results")
+		})
+
+		it("should block search outside workspace", async () => {
+			const block: SearchFilesToolUse = {
+				type: "tool_use",
+				name: "search_files",
+				params: {
+					path: "../external",
+					regex: "test",
+					file_pattern: "*.ts",
+				},
+				partial: false,
+			}
+
+			mockedIsPathOutsideWorkspace.mockReturnValue(true)
+
+			await searchFilesTool(
+				mockTask as Task,
+				block,
+				mockAskApproval,
+				mockHandleError,
+				mockPushToolResult,
+				mockRemoveClosingTag,
+			)
+
+			expect(mockedIsPathOutsideWorkspace).toHaveBeenCalledWith(path.resolve("/workspace", "../external"))
+			expect(mockedRegexSearchFiles).not.toHaveBeenCalled()
+			expect(mockTask.say).toHaveBeenCalledWith(
+				"error",
+				"Cannot search outside workspace. Path '../external' is outside the current workspace.",
+			)
+			expect(mockPushToolResult).toHaveBeenCalledWith(
+				"Cannot search outside workspace. Path '../external' is outside the current workspace.",
+			)
+			expect(mockTask.consecutiveMistakeCount).toBe(1)
+			expect(mockTask.recordToolError).toHaveBeenCalledWith("search_files")
+		})
+
+		it("should block search with absolute path outside workspace", async () => {
+			const block: SearchFilesToolUse = {
+				type: "tool_use",
+				name: "search_files",
+				params: {
+					path: "/etc/passwd",
+					regex: "root",
+				},
+				partial: false,
+			}
+
+			mockedIsPathOutsideWorkspace.mockReturnValue(true)
+
+			await searchFilesTool(
+				mockTask as Task,
+				block,
+				mockAskApproval,
+				mockHandleError,
+				mockPushToolResult,
+				mockRemoveClosingTag,
+			)
+
+			expect(mockedIsPathOutsideWorkspace).toHaveBeenCalledWith(path.resolve("/workspace", "/etc/passwd"))
+			expect(mockedRegexSearchFiles).not.toHaveBeenCalled()
+			expect(mockTask.say).toHaveBeenCalledWith(
+				"error",
+				"Cannot search outside workspace. Path '/etc/passwd' is outside the current workspace.",
+			)
+			expect(mockPushToolResult).toHaveBeenCalledWith(
+				"Cannot search outside workspace. Path '/etc/passwd' is outside the current workspace.",
+			)
+		})
+
+		it("should handle relative paths that resolve outside workspace", async () => {
+			const block: SearchFilesToolUse = {
+				type: "tool_use",
+				name: "search_files",
+				params: {
+					path: "../../..",
+					regex: "sensitive",
+				},
+				partial: false,
+			}
+
+			mockedIsPathOutsideWorkspace.mockReturnValue(true)
+
+			await searchFilesTool(
+				mockTask as Task,
+				block,
+				mockAskApproval,
+				mockHandleError,
+				mockPushToolResult,
+				mockRemoveClosingTag,
+			)
+
+			expect(mockedIsPathOutsideWorkspace).toHaveBeenCalledWith(path.resolve("/workspace", "../../.."))
+			expect(mockedRegexSearchFiles).not.toHaveBeenCalled()
+			expect(mockTask.say).toHaveBeenCalledWith(
+				"error",
+				"Cannot search outside workspace. Path '../../..' is outside the current workspace.",
+			)
+			expect(mockPushToolResult).toHaveBeenCalledWith(
+				"Cannot search outside workspace. Path '../../..' is outside the current workspace.",
+			)
+		})
+	})
+
+	describe("existing functionality", () => {
+		beforeEach(() => {
+			mockedIsPathOutsideWorkspace.mockReturnValue(false)
+		})
+
+		it("should handle missing path parameter", async () => {
+			const block: SearchFilesToolUse = {
+				type: "tool_use",
+				name: "search_files",
+				params: {
+					regex: "test",
+				},
+				partial: false,
+			}
+
+			await searchFilesTool(
+				mockTask as Task,
+				block,
+				mockAskApproval,
+				mockHandleError,
+				mockPushToolResult,
+				mockRemoveClosingTag,
+			)
+
+			expect(mockTask.sayAndCreateMissingParamError).toHaveBeenCalledWith("search_files", "path")
+			expect(mockedRegexSearchFiles).not.toHaveBeenCalled()
+		})
+
+		it("should handle missing regex parameter", async () => {
+			const block: SearchFilesToolUse = {
+				type: "tool_use",
+				name: "search_files",
+				params: {
+					path: "src",
+				},
+				partial: false,
+			}
+
+			await searchFilesTool(
+				mockTask as Task,
+				block,
+				mockAskApproval,
+				mockHandleError,
+				mockPushToolResult,
+				mockRemoveClosingTag,
+			)
+
+			expect(mockTask.sayAndCreateMissingParamError).toHaveBeenCalledWith("search_files", "regex")
+			expect(mockedRegexSearchFiles).not.toHaveBeenCalled()
+		})
+
+		it("should handle partial blocks", async () => {
+			const block: SearchFilesToolUse = {
+				type: "tool_use",
+				name: "search_files",
+				params: {
+					path: "src",
+					regex: "test",
+				},
+				partial: true,
+			}
+
+			const mockAsk = vi.fn()
+			mockTask.ask = mockAsk
+
+			await searchFilesTool(
+				mockTask as Task,
+				block,
+				mockAskApproval,
+				mockHandleError,
+				mockPushToolResult,
+				mockRemoveClosingTag,
+			)
+
+			expect(mockAsk).toHaveBeenCalled()
+			expect(mockedRegexSearchFiles).not.toHaveBeenCalled()
+		})
+
+		it("should handle user rejection", async () => {
+			const block: SearchFilesToolUse = {
+				type: "tool_use",
+				name: "search_files",
+				params: {
+					path: "src",
+					regex: "test",
+				},
+				partial: false,
+			}
+
+			mockAskApproval.mockResolvedValue(false)
+
+			await searchFilesTool(
+				mockTask as Task,
+				block,
+				mockAskApproval,
+				mockHandleError,
+				mockPushToolResult,
+				mockRemoveClosingTag,
+			)
+
+			expect(mockedRegexSearchFiles).toHaveBeenCalled()
+			expect(mockPushToolResult).not.toHaveBeenCalled()
+		})
+	})
+})

src/core/tools/searchFilesTool.ts

@@ -4,7 +4,9 @@ import { Task } from "../task/Task"
 import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../../shared/tools"
 import { ClineSayTool } from "../../shared/ExtensionMessage"
 import { getReadablePath } from "../../utils/path"
+import { isPathOutsideWorkspace } from "../../utils/pathUtils"
 import { regexSearchFiles } from "../../services/ripgrep"
+import { t } from "../../i18n"
 
 export async function searchFilesTool(
 	cline: Task,
@@ -49,6 +51,17 @@ export async function searchFilesTool(
 
 			const absolutePath = path.resolve(cline.cwd, relDirPath)
 
+			// Check if path is outside workspace
+			if (isPathOutsideWorkspace(absolutePath)) {
+				const userErrorMessage = t("tools:searchFiles.workspaceBoundaryError", { path: relDirPath })
+				const llmErrorMessage = `Cannot search outside workspace. Path '${relDirPath}' is outside the current workspace.`
+				cline.consecutiveMistakeCount++
+				cline.recordToolError("search_files")
+				await cline.say("error", userErrorMessage)
+				pushToolResult(llmErrorMessage)
+				return
+			}
+
 			const results = await regexSearchFiles(
 				cline.cwd,
 				absolutePath,

src/i18n/locales/ca/tools.json

@@ -7,5 +7,8 @@
 	"toolRepetitionLimitReached": "Roo sembla estar atrapat en un bucle, intentant la mateixa acció ({{toolName}}) repetidament. Això podria indicar un problema amb la seva estratègia actual. Considera reformular la tasca, proporcionar instruccions més específiques o guiar-lo cap a un enfocament diferent.",
 	"codebaseSearch": {
 		"approval": "Cercant '{{query}}' a la base de codi..."
+	},
+	"searchFiles": {
+		"workspaceBoundaryError": "No es pot cercar fora de l'espai de treball. El camí '{{path}}' està fora de l'espai de treball actual."
 	}
 }

src/i18n/locales/de/tools.json

@@ -7,5 +7,8 @@
 	"toolRepetitionLimitReached": "Roo scheint in einer Schleife festzustecken und versucht wiederholt dieselbe Aktion ({{toolName}}). Dies könnte auf ein Problem mit der aktuellen Strategie hindeuten. Überlege dir, die Aufgabe umzuformulieren, genauere Anweisungen zu geben oder Roo zu einem anderen Ansatz zu führen.",
 	"codebaseSearch": {
 		"approval": "Suche nach '{{query}}' im Codebase..."
+	},
+	"searchFiles": {
+		"workspaceBoundaryError": "Kann nicht außerhalb des Arbeitsbereichs suchen. Pfad '{{path}}' liegt außerhalb des aktuellen Arbeitsbereichs."
 	}
 }

src/i18n/locales/en/tools.json

@@ -7,5 +7,8 @@
 	"toolRepetitionLimitReached": "Roo appears to be stuck in a loop, attempting the same action ({{toolName}}) repeatedly. This might indicate a problem with its current strategy. Consider rephrasing the task, providing more specific instructions, or guiding it towards a different approach.",
 	"codebaseSearch": {
 		"approval": "Searching for '{{query}}' in codebase..."
+	},
+	"searchFiles": {
+		"workspaceBoundaryError": "Cannot search outside workspace. Path '{{path}}' is outside the current workspace."
 	}
 }

src/i18n/locales/es/tools.json

@@ -7,5 +7,8 @@
 	"toolRepetitionLimitReached": "Roo parece estar atrapado en un bucle, intentando la misma acción ({{toolName}}) repetidamente. Esto podría indicar un problema con su estrategia actual. Considera reformular la tarea, proporcionar instrucciones más específicas o guiarlo hacia un enfoque diferente.",
 	"codebaseSearch": {
 		"approval": "Buscando '{{query}}' en la base de código..."
+	},
+	"searchFiles": {
+		"workspaceBoundaryError": "No se puede buscar fuera del espacio de trabajo. La ruta '{{path}}' está fuera del espacio de trabajo actual."
 	}
 }

src/i18n/locales/fr/tools.json

@@ -7,5 +7,8 @@
 	"toolRepetitionLimitReached": "Roo semble être bloqué dans une boucle, tentant la même action ({{toolName}}) de façon répétée. Cela pourrait indiquer un problème avec sa stratégie actuelle. Envisage de reformuler la tâche, de fournir des instructions plus spécifiques ou de le guider vers une approche différente.",
 	"codebaseSearch": {
 		"approval": "Recherche de '{{query}}' dans la base de code..."
+	},
+	"searchFiles": {
+		"workspaceBoundaryError": "Impossible de rechercher en dehors de l'espace de travail. Le chemin '{{path}}' est en dehors de l'espace de travail actuel."
 	}
 }

src/i18n/locales/hi/tools.json

@@ -7,5 +7,8 @@
 	"toolRepetitionLimitReached": "Roo एक लूप में फंसा हुआ लगता है, बार-बार एक ही क्रिया ({{toolName}}) को दोहरा रहा है। यह उसकी वर्तमान रणनीति में किसी समस्या का संकेत हो सकता है। कार्य को पुनः परिभाषित करने, अधिक विशिष्ट निर्देश देने, या उसे एक अलग दृष्टिकोण की ओर मार्गदर्शित करने पर विचार करें।",
 	"codebaseSearch": {
 		"approval": "कोडबेस में '{{query}}' खोज रहा है..."
+	},
+	"searchFiles": {
+		"workspaceBoundaryError": "वर्कस्पेस के बाहर खोज नहीं की जा सकती। पथ '{{path}}' वर्तमान वर्कस्पेस के बाहर है।"
 	}
 }

src/i18n/locales/it/tools.json

@@ -7,5 +7,8 @@
 	"toolRepetitionLimitReached": "Roo sembra essere bloccato in un ciclo, tentando ripetutamente la stessa azione ({{toolName}}). Questo potrebbe indicare un problema con la sua strategia attuale. Considera di riformulare l'attività, fornire istruzioni più specifiche o guidarlo verso un approccio diverso.",
 	"codebaseSearch": {
 		"approval": "Ricerca di '{{query}}' nella base di codice..."
+	},
+	"searchFiles": {
+		"workspaceBoundaryError": "Impossibile cercare al di fuori dell'area di lavoro. Il percorso '{{path}}' è al di fuori dell'area di lavoro corrente."
 	}
 }