feat: complete Phase 3b - permission control with prompt guidance and UI visibility#12339
Closed
roomote-v0[bot] wants to merge 6 commits into
Closed
feat: complete Phase 3b - permission control with prompt guidance and UI visibility#12339roomote-v0[bot] wants to merge 6 commits into
roomote-v0[bot] wants to merge 6 commits into
Conversation
Adds an optional `permissions` parameter to the `new_task` tool, allowing the Orchestrator (or any parent task) to dynamically set permission boundaries for subtasks: - New `TaskPermissions` type with filePatterns, commandPatterns, allowedTools, and deniedTools - Permission merging with most-restrictive-wins semantics for nested subtask delegation - Runtime enforcement in validateToolUse() for all permission types - Full test coverage for merging logic and enforcement Addresses Issue #12330 (Phase 3b)
…pattern merging 1. NativeToolCallParser: Remove permissions from update_todo_list cases (was erroneously added to wrong tool case, should only be on new_task) 2. deniedTools: Exempt ALWAYS_AVAILABLE_TOOLS (attempt_completion, etc.) from deniedTools check, matching the existing allowedTools behavior. Prevents parent from trapping subtask by denying completion tools. 3. Pattern merging: Replace broken exact-string intersection with layered enforcement. filePatterns/commandPatterns from parent and child are kept as separate layers (AND between layers, OR within each layer). This correctly handles narrowing: parent ["src/.*"] + child ["src/components/.*"] now allows only files matching BOTH patterns, instead of producing an empty intersection.
…ema level, simplify validation code 1. Anchor regex patterns in matchesAnyPattern with ^(?:...)$ wrapping so patterns like "src/.*" require full-path matching instead of substring matching. Prevents "evil/src/foo" from matching a "src/.*" permission. 2. Add regex validation at schema level (regexString refinement) so invalid patterns are rejected at parse time rather than silently failing at runtime. 3. Simplify duplicate file/command pattern validation in validateToolUse by unifying layered and flat code paths into a single branch that falls back to wrapping flat patterns as a single layer. 4. Remove unused matchesAnyPattern import from validateToolUse.ts. 5. Add tests for anchoring behavior, pre-anchored patterns, and invalid regex rejection at schema level.
1. Persist taskPermissions in HistoryItem so permissions survive task
restarts. Added taskPermissions field to historyItemSchema, included
it in taskMetadata output, and restored it in the Task constructor
when loading from history.
2. Add ReDoS mitigation for model-provided regex patterns:
- isSafeRegex() heuristic rejects nested quantifiers like (a+)+
and overlapping alternations in repeated groups like (a|a)+
- Max pattern length capped at 200 characters
- Both checks enforced at schema validation time via Zod refinements
- 11 new tests covering ReDoS detection and persistence round-trips
- Enhance Orchestrator customInstructions with guidance on using the permissions parameter (filePatterns, commandPatterns, allowedTools, deniedTools) including example use cases and most-restrictive-wins semantics explanation - Add permission boundaries display in the ChatRow newTask approval message so users can see what restrictions are being set before approving subtask creation - Add i18n translation keys for permission display - Add 8 new tests across packages/types and webview-ui
This was referenced May 12, 2026
|
@roomote can you pick the commit |
5 tasks
Contributor
Author
|
Closing as part of PR restructuring per issue #12330. This work will be combined into a new Phase 2+3 PR with proper stacked branching. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related GitHub Issue
Addresses: #12330
Description
This PR is the single final PR for Phase 3b of Issue #12330, consolidating all permission control work. It supersedes #12337 (now closed) and includes everything needed for model-driven permission control for subtasks.
What this includes (from #12337 core + prompt/UI additions):
TaskPermissionstype (packages/types/src/task-permissions.ts): Defines four permission boundary fields:filePatterns- regex patterns for allowed file pathscommandPatterns- regex patterns for allowed shell commandsallowedTools- explicit tool allowlistdeniedTools- explicit tool blocklistMost-restrictive-wins merging (
mergeTaskPermissions): When nested subtasks are created, permissions are merged so a child can never grant itself more access than its parent.Runtime enforcement in
validateToolUse(): All permission types are checked at tool execution time with clearTaskPermissionErrormessages.ReDoS mitigation -
isSafeRegex()heuristic rejects dangerous patterns.Permission persistence -
taskPermissionssaved tohistory_item.jsonand restored on task reopen.Orchestrator prompt guidance - New instruction in Orchestrator mode explaining when/how to use each permission type.
Permission visibility in approval UI - ChatRow shows permission boundaries when approving subtask creation.
i18n translations for permission display labels.
Optional by default: When
permissionsis omitted fromnew_task, behavior is identical to today -- zero friction for normal users.Test Procedure
task-permissions.spec.ts)taskPermissionsEnforcement.spec.ts)orchestrator-permissions-prompt.spec.ts)ChatRow.permissions.spec.tsx)Pre-Submission Checklist
This PR attempts to address Issue #12330 Phase 3b. Feedback and guidance are welcome.
Interactively review PR in Roo Code Cloud