mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-11 22:51:26 +00:00
Fix: Enforce file restrictions for all editing tools (#10896)
Co-authored-by: Roo Code <roomote@roocode.com>
This commit is contained in:
roomote[bot]
GitHub
parent
5c1c16c16e
commit
f6006c998d
2 changed files with 300 additions and 2 deletions
|
|
@ -62,7 +62,46 @@ export function validateToolUse(
|
|||
}
|
||||
}
|
||||
|
||||
const EDIT_OPERATION_PARAMS = ["diff", "content", "operations", "search", "replace", "args", "line"] as const
|
||||
const EDIT_OPERATION_PARAMS = [
|
||||
"diff",
|
||||
"content",
|
||||
"operations",
|
||||
"search",
|
||||
"replace",
|
||||
"args",
|
||||
"line",
|
||||
"patch", // Used by apply_patch
|
||||
"old_string", // Used by search_replace and edit_file
|
||||
"new_string", // Used by search_replace and edit_file
|
||||
] as const
|
||||
|
||||
// Markers used in apply_patch format to identify file operations
|
||||
const PATCH_FILE_MARKERS = ["*** Add File: ", "*** Delete File: ", "*** Update File: "] as const
|
||||
|
||||
/**
|
||||
* Extract file paths from apply_patch content.
|
||||
* The patch format uses markers like "*** Add File: path", "*** Delete File: path", "*** Update File: path"
|
||||
* @param patchContent The patch content string
|
||||
* @returns Array of file paths found in the patch
|
||||
*/
|
||||
function extractFilePathsFromPatch(patchContent: string): string[] {
|
||||
const filePaths: string[] = []
|
||||
const lines = patchContent.split("\n")
|
||||
|
||||
for (const line of lines) {
|
||||
for (const marker of PATCH_FILE_MARKERS) {
|
||||
if (line.startsWith(marker)) {
|
||||
const path = line.substring(marker.length).trim()
|
||||
if (path) {
|
||||
filePaths.push(path)
|
||||
}
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return filePaths
|
||||
}
|
||||
|
||||
function getGroupOptions(group: GroupEntry): GroupOptions | undefined {
|
||||
return Array.isArray(group) ? group[1] : undefined
|
||||
|
|
@ -155,7 +194,7 @@ export function isToolAllowedForMode(
|
|||
|
||||
// For the edit group, check file regex if specified
|
||||
if (groupName === "edit" && options.fileRegex) {
|
||||
const filePath = toolParams?.path
|
||||
const filePath = toolParams?.path || toolParams?.file_path
|
||||
// Check if this is an actual edit operation (not just path-only for streaming)
|
||||
const isEditOperation = EDIT_OPERATION_PARAMS.some((param) => toolParams?.[param])
|
||||
|
||||
|
|
@ -164,6 +203,22 @@ export function isToolAllowedForMode(
|
|||
throw new FileRestrictionError(mode.name, options.fileRegex, options.description, filePath, tool)
|
||||
}
|
||||
|
||||
// Handle apply_patch: extract file paths from patch content and validate each
|
||||
if (tool === "apply_patch" && typeof toolParams?.patch === "string") {
|
||||
const patchFilePaths = extractFilePathsFromPatch(toolParams.patch)
|
||||
for (const patchFilePath of patchFilePaths) {
|
||||
if (!doesFileMatchRegex(patchFilePath, options.fileRegex)) {
|
||||
throw new FileRestrictionError(
|
||||
mode.name,
|
||||
options.fileRegex,
|
||||
options.description,
|
||||
patchFilePath,
|
||||
tool,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Native-only: multi-file edits provide structured params; no legacy XML args parsing.
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -273,6 +273,249 @@ describe("isToolAllowedForMode", () => {
|
|||
}),
|
||||
).toThrow(/Markdown files only/)
|
||||
})
|
||||
|
||||
it("applies restrictions to apply_patch (custom tool)", () => {
|
||||
// Test that apply_patch respects file restrictions when included
|
||||
// Note: apply_patch only accepts { patch: string } - file paths are embedded in patch content
|
||||
const patchResult = isToolAllowedForMode(
|
||||
"apply_patch",
|
||||
"markdown-editor",
|
||||
customModes,
|
||||
undefined,
|
||||
{
|
||||
patch: "*** Begin Patch\n*** Update File: test.md\n@@ \n-old\n+new\n*** End Patch",
|
||||
},
|
||||
undefined,
|
||||
["apply_patch"], // Include custom tool
|
||||
)
|
||||
expect(patchResult).toBe(true)
|
||||
|
||||
// Test apply_patch with non-matching file (file path embedded in patch content)
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"apply_patch",
|
||||
"markdown-editor",
|
||||
customModes,
|
||||
undefined,
|
||||
{
|
||||
patch: "*** Begin Patch\n*** Update File: test.js\n@@ \n-old\n+new\n*** End Patch",
|
||||
},
|
||||
undefined,
|
||||
["apply_patch"], // Include custom tool
|
||||
),
|
||||
).toThrow(FileRestrictionError)
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"apply_patch",
|
||||
"markdown-editor",
|
||||
customModes,
|
||||
undefined,
|
||||
{
|
||||
patch: "*** Begin Patch\n*** Update File: test.js\n@@ \n-old\n+new\n*** End Patch",
|
||||
},
|
||||
undefined,
|
||||
["apply_patch"], // Include custom tool
|
||||
),
|
||||
).toThrow(/\\.md\$/)
|
||||
})
|
||||
|
||||
it("applies restrictions to search_replace (custom tool)", () => {
|
||||
// Test that search_replace respects file restrictions when included
|
||||
const searchReplaceResult = isToolAllowedForMode(
|
||||
"search_replace",
|
||||
"markdown-editor",
|
||||
customModes,
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.md",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["search_replace"], // Include custom tool
|
||||
)
|
||||
expect(searchReplaceResult).toBe(true)
|
||||
|
||||
// Test search_replace with non-matching file
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"search_replace",
|
||||
"markdown-editor",
|
||||
customModes,
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.js",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["search_replace"], // Include custom tool
|
||||
),
|
||||
).toThrow(FileRestrictionError)
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"search_replace",
|
||||
"markdown-editor",
|
||||
customModes,
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.js",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["search_replace"], // Include custom tool
|
||||
),
|
||||
).toThrow(/\\.md\$/)
|
||||
})
|
||||
|
||||
it("applies restrictions to edit_file (custom tool)", () => {
|
||||
// Test that edit_file respects file restrictions when included
|
||||
const editFileResult = isToolAllowedForMode(
|
||||
"edit_file",
|
||||
"markdown-editor",
|
||||
customModes,
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.md",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["edit_file"], // Include custom tool
|
||||
)
|
||||
expect(editFileResult).toBe(true)
|
||||
|
||||
// Test edit_file with non-matching file
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"edit_file",
|
||||
"markdown-editor",
|
||||
customModes,
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.js",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["edit_file"], // Include custom tool
|
||||
),
|
||||
).toThrow(FileRestrictionError)
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"edit_file",
|
||||
"markdown-editor",
|
||||
customModes,
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.js",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["edit_file"], // Include custom tool
|
||||
),
|
||||
).toThrow(/\\.md\$/)
|
||||
})
|
||||
|
||||
it("applies restrictions to all editing tools in architect mode (custom tools)", () => {
|
||||
// Test apply_patch in architect mode
|
||||
// Note: apply_patch only accepts { patch: string } - file paths are embedded in patch content
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"apply_patch",
|
||||
"architect",
|
||||
[],
|
||||
undefined,
|
||||
{
|
||||
patch: "*** Begin Patch\n*** Update File: test.md\n@@ \n-old\n+new\n*** End Patch",
|
||||
},
|
||||
undefined,
|
||||
["apply_patch"], // Include custom tool
|
||||
),
|
||||
).toBe(true)
|
||||
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"apply_patch",
|
||||
"architect",
|
||||
[],
|
||||
undefined,
|
||||
{
|
||||
patch: "*** Begin Patch\n*** Update File: test.js\n@@ \n-old\n+new\n*** End Patch",
|
||||
},
|
||||
undefined,
|
||||
["apply_patch"], // Include custom tool
|
||||
),
|
||||
).toThrow(FileRestrictionError)
|
||||
|
||||
// Test search_replace in architect mode
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"search_replace",
|
||||
"architect",
|
||||
[],
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.md",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["search_replace"], // Include custom tool
|
||||
),
|
||||
).toBe(true)
|
||||
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"search_replace",
|
||||
"architect",
|
||||
[],
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.js",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["search_replace"], // Include custom tool
|
||||
),
|
||||
).toThrow(FileRestrictionError)
|
||||
|
||||
// Test edit_file in architect mode
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"edit_file",
|
||||
"architect",
|
||||
[],
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.md",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["edit_file"], // Include custom tool
|
||||
),
|
||||
).toBe(true)
|
||||
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"edit_file",
|
||||
"architect",
|
||||
[],
|
||||
undefined,
|
||||
{
|
||||
file_path: "test.js",
|
||||
old_string: "old text",
|
||||
new_string: "new text",
|
||||
},
|
||||
undefined,
|
||||
["edit_file"], // Include custom tool
|
||||
),
|
||||
).toThrow(FileRestrictionError)
|
||||
})
|
||||
})
|
||||
|
||||
it("handles non-existent modes", () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue