From 43f2fa405cb45637f3a36fd034a438a660393817 Mon Sep 17 00:00:00 2001
From: Hannes Rudolph <hrudolph@gmail.com>
Date: Wed, 31 Dec 2025 15:53:15 -0700
Subject: [PATCH] fix(modes): disallow empty path for read-scoped search tools

---
 .../tools/__tests__/validateToolUse.spec.ts   | 51 +++++++++++++++++++
 src/core/tools/validateToolUse.ts             | 19 ++++++-
 src/shared/__tests__/modes.spec.ts            | 10 ++--
 3 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/src/core/tools/__tests__/validateToolUse.spec.ts b/src/core/tools/__tests__/validateToolUse.spec.ts
index ebaebab00e..e90c1544e0 100644
--- a/src/core/tools/__tests__/validateToolUse.spec.ts
+++ b/src/core/tools/__tests__/validateToolUse.spec.ts
@@ -244,5 +244,56 @@ describe("mode-validator", () => {
 		it("handles undefined requirements gracefully", () => {
 			expect(() => validateToolUse("apply_diff", codeMode, [], undefined)).not.toThrow()
 		})
+
+		it("enforces read fileRegex restrictions for directory tools when path is empty/missing", () => {
+			const skillFileRegex = "(^|.*[\\\\/])\\.roo[\\\\/]skills(-[a-zA-Z0-9-]+)?[\\\\/][^\\\\/]+[\\\\/]SKILL\\.md$"
+
+			const customModes: ModeConfig[] = [
+				{
+					slug: "skills-reader",
+					name: "Skills Reader",
+					roleDefinition: "Read skill definition files only",
+					groups: [["read", { fileRegex: skillFileRegex }]] as const,
+				},
+			]
+
+			// Empty string path must not be treated as an unrestricted root operation.
+			expect(() => validateToolUse("list_files", "skills-reader", customModes, undefined, { path: "" })).toThrow(
+				/can only read files matching pattern/,
+			)
+			expect(() =>
+				validateToolUse("search_files", "skills-reader", customModes, undefined, { path: "", regex: ".*" }),
+			).toThrow(/can only read files matching pattern/)
+			expect(() =>
+				validateToolUse("codebase_search", "skills-reader", customModes, undefined, { query: "x", path: "" }),
+			).toThrow(/can only read files matching pattern/)
+
+			// Omitted path must also be rejected under read fileRegex.
+			expect(() =>
+				validateToolUse("codebase_search", "skills-reader", customModes, undefined, { query: "x" }),
+			).toThrow(/can only read files matching pattern/)
+
+			// Allowed directory should be accepted.
+			expect(() =>
+				validateToolUse("list_files", "skills-reader", customModes, undefined, { path: ".roo/skills" }),
+			).not.toThrow()
+			expect(() =>
+				validateToolUse("search_files", "skills-reader", customModes, undefined, {
+					path: ".roo/skills",
+					regex: ".*",
+				}),
+			).not.toThrow()
+			expect(() =>
+				validateToolUse("codebase_search", "skills-reader", customModes, undefined, {
+					query: "x",
+					path: ".roo/skills",
+				}),
+			).not.toThrow()
+
+			// Disallowed directory should be rejected.
+			expect(() =>
+				validateToolUse("list_files", "skills-reader", customModes, undefined, { path: "src" }),
+			).toThrow(/can only read files matching pattern/)
+		})
 	})
 })
diff --git a/src/core/tools/validateToolUse.ts b/src/core/tools/validateToolUse.ts
index cadbd3c932..9047eadcfe 100644
--- a/src/core/tools/validateToolUse.ts
+++ b/src/core/tools/validateToolUse.ts
@@ -320,8 +320,23 @@ export function isToolAllowedForMode(
 			// Restrict search_files, codebase_search, and list_files path parameter
 			// These tools operate on directories, so we derive a directory pattern from the fileRegex
 			if (["search_files", "codebase_search", "list_files"].includes(tool)) {
-				const pathParam = toolParams?.path
-				if (typeof pathParam === "string" && pathParam.trim().length > 0) {
+				const rawPathParam = toolParams?.path
+
+				// IMPORTANT: When read group fileRegex is configured, we must not allow an empty/omitted
+				// path to be interpreted as an unrestricted workspace-root operation.
+				if (typeof rawPathParam !== "string" || rawPathParam.trim().length === 0) {
+					throw new FileRestrictionError(
+						mode.name,
+						options.fileRegex,
+						options.description,
+						typeof rawPathParam === "string" ? rawPathParam : "(missing path)",
+						tool,
+						"read",
+					)
+				}
+
+				const pathParam = rawPathParam.trim()
+				if (pathParam.length > 0) {
 					// Derive directory pattern: strip trailing file pattern (e.g., SKILL.md$) and match directory prefix
 					// For pattern like: (^|.*[\\/])\.roo[\\/]skills(-[a-zA-Z0-9-]+)?[\\/][^\\/]+[\\/]SKILL\.md$
 					// Directory should be within: .roo/skills or .roo/skills-<mode>
diff --git a/src/shared/__tests__/modes.spec.ts b/src/shared/__tests__/modes.spec.ts
index 1ad08cd172..8c87c89358 100644
--- a/src/shared/__tests__/modes.spec.ts
+++ b/src/shared/__tests__/modes.spec.ts
@@ -561,20 +561,20 @@ describe("FileRestrictionError", () => {
 		})
 
 		it("allows empty path for search tools (workspace root)", () => {
-			// Empty path should be allowed (searches entire workspace which is allowed)
-			expect(
+			// Empty path must not be treated as an unrestricted workspace-root operation when fileRegex is set.
+			expect(() =>
 				isToolAllowedForMode("search_files", "orchestrator", [], undefined, {
 					path: "",
 					regex: ".*",
 				}),
-			).toBe(true)
+			).toThrow(FileRestrictionError)
 
-			expect(
+			expect(() =>
 				isToolAllowedForMode("codebase_search", "orchestrator", [], undefined, {
 					path: "",
 					query: "test",
 				}),
-			).toBe(true)
+			).toThrow(FileRestrictionError)
 		})
 	})