scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-09-07 08:26:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: extend read group fileRegex enforcement to search_files, codebase_search, list_files

Browse source 
The read group's fileRegex restriction was only enforced on read_file, allowing
orchestrator mode to bypass read restrictions via other tools that access paths.

Changes:
- Extended validateToolUse to restrict path param of search_files, codebase_search,
  and list_files when read group has fileRegex
- Added tests verifying orchestrator mode cannot access arbitrary directories
- Tests pass for both allowed (.roo/skills) and restricted (src, node_modules) paths
This commit is contained in:
Hannes Rudolph 2025-12-31 14:53:02 -07:00
parent 11c88f698c
commit ac0f7febe4
2 changed files with 132 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

72
src/core/tools/validateToolUse.ts
View file

@ -282,28 +282,64 @@ export function isToolAllowedForMode(
}
// Handle XML args parameter (used by MULTI_FILE_APPLY_DIFF experiment)
if (toolParams?.args && typeof toolParams.args === "string") {
const xmlPaths = extractPathsFromXmlArgs(toolParams.args)
for (const extractedPath of xmlPaths) {
if (!doesFileMatchRegex(extractedPath, options.fileRegex)) {
throw new FileRestrictionError(
mode.name,
options.fileRegex,
options.description,
extractedPath,
tool,
)
}
if (toolParams?.args && typeof toolParams.args === "string") {
const xmlPaths = extractPathsFromXmlArgs(toolParams.args)
for (const extractedPath of xmlPaths) {
if (!doesFileMatchRegex(extractedPath, options.fileRegex)) {
throw new FileRestrictionError(
mode.name,
options.fileRegex,
options.description,
extractedPath,
tool,
)
}
}
}
}
// For the read group, optionally restrict read_file paths if specified
if (groupName === "read" && options.fileRegex && tool === "read_file") {
const readPaths = extractReadFilePaths(toolParams)
for (const p of readPaths) {
if (!doesFileMatchRegex(p, options.fileRegex)) {
throw new FileRestrictionError(mode.name, options.fileRegex, options.description, p, tool, "read")
// For the read group, optionally restrict paths if fileRegex is specified
if (groupName === "read" && options.fileRegex) {
// Restrict read_file to only read files matching the pattern
if (tool === "read_file") {
const readPaths = extractReadFilePaths(toolParams)
for (const p of readPaths) {
if (!doesFileMatchRegex(p, options.fileRegex)) {
throw new FileRestrictionError(
mode.name,
options.fileRegex,
options.description,
p,
tool,
"read",
)
}
}
}
// 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) {
// 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>
const dirPattern = options.fileRegex
.replace(/\[\^[^\]]*\]\+[\\\\/]?[^$]*\$$/, "") // Remove [^\/]+\/SKILL\.md$ part
.replace(/\$$/, "") // Remove trailing $ if present
const dirRegex = dirPattern ? `${dirPattern}($|[\\\\/])` : options.fileRegex
if (!doesFileMatchRegex(pathParam, dirRegex) && !doesFileMatchRegex(pathParam + "/", dirRegex)) {
throw new FileRestrictionError(
mode.name,
options.fileRegex,
options.description,
pathParam,
tool,
"read",
)
}
}
}
}

78
src/shared/__tests__/modes.spec.ts
View file

@ -498,6 +498,84 @@ describe("FileRestrictionError", () => {
}),
).toThrow(FileRestrictionError)
})
it("restricts search_files to skills directories", () => {
// Should allow searching within .roo/skills directories
expect(
isToolAllowedForMode("search_files", "orchestrator", [], undefined, {
path: ".roo/skills",
regex: ".*",
}),
).toBe(true)
expect(
isToolAllowedForMode("search_files", "orchestrator", [], undefined, {
path: ".roo/skills-code",
regex: ".*",
}),
).toBe(true)
// Should reject searching outside skills directories
expect(() =>
isToolAllowedForMode("search_files", "orchestrator", [], undefined, {
path: "src",
regex: ".*",
}),
).toThrow(FileRestrictionError)
})
it("restricts codebase_search to skills directories", () => {
// Should allow searching within .roo/skills directories
expect(
isToolAllowedForMode("codebase_search", "orchestrator", [], undefined, {
path: ".roo/skills",
query: "test",
}),
).toBe(true)
// Should reject searching outside skills directories
expect(() =>
isToolAllowedForMode("codebase_search", "orchestrator", [], undefined, {
path: "src/core",
query: "test",
}),
).toThrow(FileRestrictionError)
})
it("restricts list_files to skills directories", () => {
// Should allow listing within .roo/skills directories
expect(
isToolAllowedForMode("list_files", "orchestrator", [], undefined, {
path: ".roo/skills",
recursive: true,
}),
).toBe(true)
// Should reject listing outside skills directories
expect(() =>
isToolAllowedForMode("list_files", "orchestrator", [], undefined, {
path: "node_modules",
recursive: false,
}),
).toThrow(FileRestrictionError)
})
it("allows empty path for search tools (workspace root)", () => {
// Empty path should be allowed (searches entire workspace which is allowed)
expect(
isToolAllowedForMode("search_files", "orchestrator", [], undefined, {
path: "",
regex: ".*",
}),
).toBe(true)
expect(
isToolAllowedForMode("codebase_search", "orchestrator", [], undefined, {
path: "",
query: "test",
}),
).toBe(true)
})
})
describe("getFullModeDetails", () => {