-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat(modes): add regex scoped read permissions per mode + orchestrator skills read allowlist #10411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Status: All previously flagged items are resolved in 43f2fa4.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
|
Fixed: read_file fileRegex enforcement now also validates when toolParams.files is a JSON string. Added tests for both allowed/blocked JSON-string payloads.\n\nChanges:\n- src/core/tools/validateToolUse.ts\n- src/core/tools/tests/validateToolUse.spec.ts |
4e46243 to
2490135
Compare
…e_search, list_files 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
src/core/tools/validateToolUse.ts
Outdated
| // 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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a read-group fileRegex is present, the path restriction for search_files/codebase_search/list_files is skipped for empty string paths, which effectively allows Orchestrator to search from workspace root (and read non-skill files) by passing path: "". If the intent is that Orchestrator is constrained to skills only, consider validating empty paths too (e.g., treat "" as "." and enforce) or rejecting empty path when fileRegex is set.
Fix it with Roo Code or mention @roomote and request a fix.
Summary
fileRegexon thereadgroup for theread_filetool (native{ files: [...] }payloads).FileRestrictionErrorto supportreadvseditwording.SKILL.mdfiles, supporting both:.roo/skills(-<mode>)/<skill>/SKILL.md/Users/<user>/.roo/skills(-<mode>)/<skill>/SKILL.md/C:\\Users\\<user>\\.roo\\skills(-<mode>)\\<skill>\\SKILL.mdKey changes
src/core/assistant-message/presentAssistantMessage.tssrc/core/tools/validateToolUse.ts<location>uses absolute paths:src/core/prompts/sections/skills.tspackages/types/src/mode.tsTests
cd src && npx vitest run shared/__tests__/modes.spec.tsNotes
fileRegexon thereadgroup.