fix(modes): disallow empty path for read-scoped search tools

This commit is contained in:
Hannes Rudolph 2025-12-31 15:53:15 -07:00
parent ac0f7febe4
commit 43f2fa405c
3 changed files with 73 additions and 7 deletions

View file

@ -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/)
})
})
})

View file

@ -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>

View file

@ -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)
})
})