mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-09 22:31:08 +00:00
feat: add model-driven permission control for subtasks (Phase 3b)
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)
This commit is contained in:
parent
8922418600
commit
22086b5b86
13 changed files with 638 additions and 7 deletions
152
packages/types/src/__tests__/task-permissions.spec.ts
Normal file
152
packages/types/src/__tests__/task-permissions.spec.ts
Normal file
|
|
@ -0,0 +1,152 @@
|
|||
import { describe, it, expect } from "vitest"
|
||||
import { mergeTaskPermissions, matchesAnyPattern, taskPermissionsSchema } from "../task-permissions.js"
|
||||
import type { TaskPermissions } from "../task-permissions.js"
|
||||
|
||||
describe("TaskPermissions", () => {
|
||||
describe("taskPermissionsSchema", () => {
|
||||
it("validates a valid permissions object", () => {
|
||||
const result = taskPermissionsSchema.safeParse({
|
||||
filePatterns: ["src/components/.*"],
|
||||
commandPatterns: ["npm test.*"],
|
||||
allowedTools: ["read_file", "write_to_file"],
|
||||
deniedTools: ["execute_command"],
|
||||
})
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it("validates an empty object", () => {
|
||||
const result = taskPermissionsSchema.safeParse({})
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it("validates partial permissions", () => {
|
||||
const result = taskPermissionsSchema.safeParse({
|
||||
filePatterns: ["src/.*"],
|
||||
})
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it("rejects non-string array values", () => {
|
||||
const result = taskPermissionsSchema.safeParse({
|
||||
filePatterns: [123],
|
||||
})
|
||||
expect(result.success).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("mergeTaskPermissions", () => {
|
||||
it("returns undefined when both are undefined", () => {
|
||||
expect(mergeTaskPermissions(undefined, undefined)).toBeUndefined()
|
||||
})
|
||||
|
||||
it("returns child when parent is undefined", () => {
|
||||
const child: TaskPermissions = { filePatterns: ["src/.*"] }
|
||||
expect(mergeTaskPermissions(undefined, child)).toEqual(child)
|
||||
})
|
||||
|
||||
it("returns parent when child is undefined", () => {
|
||||
const parent: TaskPermissions = { filePatterns: ["src/.*"] }
|
||||
expect(mergeTaskPermissions(parent, undefined)).toEqual(parent)
|
||||
})
|
||||
|
||||
it("intersects filePatterns when both defined", () => {
|
||||
const parent: TaskPermissions = { filePatterns: ["src/.*", "tests/.*"] }
|
||||
const child: TaskPermissions = { filePatterns: ["src/.*", "docs/.*"] }
|
||||
const merged = mergeTaskPermissions(parent, child)
|
||||
expect(merged?.filePatterns).toEqual(["src/.*"])
|
||||
})
|
||||
|
||||
it("intersects commandPatterns when both defined", () => {
|
||||
const parent: TaskPermissions = { commandPatterns: ["npm test.*", "npm run lint"] }
|
||||
const child: TaskPermissions = { commandPatterns: ["npm test.*", "npm run build"] }
|
||||
const merged = mergeTaskPermissions(parent, child)
|
||||
expect(merged?.commandPatterns).toEqual(["npm test.*"])
|
||||
})
|
||||
|
||||
it("intersects allowedTools when both defined", () => {
|
||||
const parent: TaskPermissions = { allowedTools: ["read_file", "write_to_file", "search_files"] }
|
||||
const child: TaskPermissions = { allowedTools: ["read_file", "execute_command"] }
|
||||
const merged = mergeTaskPermissions(parent, child)
|
||||
expect(merged?.allowedTools).toEqual(["read_file"])
|
||||
})
|
||||
|
||||
it("unions deniedTools when both defined", () => {
|
||||
const parent: TaskPermissions = { deniedTools: ["execute_command"] }
|
||||
const child: TaskPermissions = { deniedTools: ["write_to_file"] }
|
||||
const merged = mergeTaskPermissions(parent, child)
|
||||
expect(merged?.deniedTools).toEqual(["execute_command", "write_to_file"])
|
||||
})
|
||||
|
||||
it("deduplicates deniedTools in union", () => {
|
||||
const parent: TaskPermissions = { deniedTools: ["execute_command", "write_to_file"] }
|
||||
const child: TaskPermissions = { deniedTools: ["execute_command", "search_files"] }
|
||||
const merged = mergeTaskPermissions(parent, child)
|
||||
expect(merged?.deniedTools).toEqual(["execute_command", "write_to_file", "search_files"])
|
||||
})
|
||||
|
||||
it("uses parent filePatterns when child has none", () => {
|
||||
const parent: TaskPermissions = { filePatterns: ["src/.*"] }
|
||||
const child: TaskPermissions = { deniedTools: ["execute_command"] }
|
||||
const merged = mergeTaskPermissions(parent, child)
|
||||
expect(merged?.filePatterns).toEqual(["src/.*"])
|
||||
expect(merged?.deniedTools).toEqual(["execute_command"])
|
||||
})
|
||||
|
||||
it("returns empty array when intersection is empty", () => {
|
||||
const parent: TaskPermissions = { allowedTools: ["read_file"] }
|
||||
const child: TaskPermissions = { allowedTools: ["write_to_file"] }
|
||||
const merged = mergeTaskPermissions(parent, child)
|
||||
expect(merged?.allowedTools).toEqual([])
|
||||
})
|
||||
|
||||
it("handles complex nested merge scenario with exact string matching", () => {
|
||||
const grandparent: TaskPermissions = {
|
||||
filePatterns: ["src/.*"],
|
||||
commandPatterns: ["npm.*"],
|
||||
allowedTools: ["read_file", "write_to_file", "search_files"],
|
||||
deniedTools: ["execute_command"],
|
||||
}
|
||||
const parent: TaskPermissions = {
|
||||
filePatterns: ["src/components/.*"],
|
||||
allowedTools: ["read_file", "write_to_file"],
|
||||
}
|
||||
|
||||
// Intersection uses exact string matching, so "src/components/.*" (child)
|
||||
// is not equal to "src/.*" (parent) -- intersection is empty
|
||||
const merged1 = mergeTaskPermissions(grandparent, parent)
|
||||
expect(merged1?.filePatterns).toEqual([])
|
||||
// allowedTools intersection: read_file and write_to_file are in both
|
||||
expect(merged1?.allowedTools).toEqual(["read_file", "write_to_file"])
|
||||
// commandPatterns: only grandparent has them, so they pass through
|
||||
expect(merged1?.commandPatterns).toEqual(["npm.*"])
|
||||
// deniedTools: only grandparent has them, so they pass through
|
||||
expect(merged1?.deniedTools).toEqual(["execute_command"])
|
||||
})
|
||||
})
|
||||
|
||||
describe("matchesAnyPattern", () => {
|
||||
it("matches a simple regex pattern", () => {
|
||||
expect(matchesAnyPattern("src/components/Button.tsx", ["src/components/.*"])).toBe(true)
|
||||
})
|
||||
|
||||
it("does not match when no patterns match", () => {
|
||||
expect(matchesAnyPattern("tests/unit/test.ts", ["src/components/.*"])).toBe(false)
|
||||
})
|
||||
|
||||
it("matches when at least one pattern matches", () => {
|
||||
expect(matchesAnyPattern("tests/unit/test.ts", ["src/.*", "tests/.*"])).toBe(true)
|
||||
})
|
||||
|
||||
it("handles invalid regex gracefully", () => {
|
||||
expect(matchesAnyPattern("test.ts", ["[invalid"])).toBe(false)
|
||||
})
|
||||
|
||||
it("matches command patterns", () => {
|
||||
expect(matchesAnyPattern("npm test -- --coverage", ["npm test.*"])).toBe(true)
|
||||
})
|
||||
|
||||
it("does not match restricted commands", () => {
|
||||
expect(matchesAnyPattern("rm -rf /", ["npm.*", "yarn.*"])).toBe(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -20,6 +20,7 @@ export * from "./mode.js"
|
|||
export * from "./model.js"
|
||||
export * from "./provider-settings.js"
|
||||
export * from "./task.js"
|
||||
export * from "./task-permissions.js"
|
||||
export * from "./todo.js"
|
||||
export * from "./skills.js"
|
||||
export * from "./terminal.js"
|
||||
|
|
|
|||
124
packages/types/src/task-permissions.ts
Normal file
124
packages/types/src/task-permissions.ts
Normal file
|
|
@ -0,0 +1,124 @@
|
|||
import { z } from "zod"
|
||||
|
||||
/**
|
||||
* TaskPermissions defines permission boundaries that a parent task can impose
|
||||
* on a subtask created via the `new_task` tool.
|
||||
*
|
||||
* When nested subtasks are created, permissions are merged using
|
||||
* "most-restrictive-wins" semantics: a child can never grant itself
|
||||
* more access than its parent.
|
||||
*/
|
||||
|
||||
export const taskPermissionsSchema = z.object({
|
||||
/**
|
||||
* Regex patterns for allowed file paths.
|
||||
* When set, file operations (read/write) are restricted to paths matching
|
||||
* at least one of these patterns.
|
||||
*/
|
||||
filePatterns: z.array(z.string()).optional(),
|
||||
|
||||
/**
|
||||
* Regex patterns for allowed shell commands.
|
||||
* When set, command execution is restricted to commands matching
|
||||
* at least one of these patterns.
|
||||
*/
|
||||
commandPatterns: z.array(z.string()).optional(),
|
||||
|
||||
/**
|
||||
* Explicit tool allowlist. When set, only these tools may be used
|
||||
* by the subtask (in addition to always-available tools like
|
||||
* attempt_completion and ask_followup_question).
|
||||
*/
|
||||
allowedTools: z.array(z.string()).optional(),
|
||||
|
||||
/**
|
||||
* Explicit tool blocklist. These tools are denied regardless of
|
||||
* mode configuration.
|
||||
*/
|
||||
deniedTools: z.array(z.string()).optional(),
|
||||
})
|
||||
|
||||
export type TaskPermissions = z.infer<typeof taskPermissionsSchema>
|
||||
|
||||
/**
|
||||
* Merge two TaskPermissions using most-restrictive-wins semantics.
|
||||
*
|
||||
* - filePatterns / commandPatterns: if both define patterns, keep only patterns
|
||||
* present in both (intersection). If only one side defines patterns, use that.
|
||||
* - allowedTools: intersection of both lists (if both defined).
|
||||
* - deniedTools: union of both lists (most restrictive).
|
||||
*
|
||||
* @returns merged permissions, or undefined if both inputs are undefined.
|
||||
*/
|
||||
export function mergeTaskPermissions(
|
||||
parent: TaskPermissions | undefined,
|
||||
child: TaskPermissions | undefined,
|
||||
): TaskPermissions | undefined {
|
||||
if (!parent && !child) {
|
||||
return undefined
|
||||
}
|
||||
if (!parent) {
|
||||
return child
|
||||
}
|
||||
if (!child) {
|
||||
return parent
|
||||
}
|
||||
|
||||
return {
|
||||
filePatterns: intersectOptionalArrays(parent.filePatterns, child.filePatterns),
|
||||
commandPatterns: intersectOptionalArrays(parent.commandPatterns, child.commandPatterns),
|
||||
allowedTools: intersectOptionalArrays(parent.allowedTools, child.allowedTools),
|
||||
deniedTools: unionOptionalArrays(parent.deniedTools, child.deniedTools),
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a value matches at least one pattern in a list of regex patterns.
|
||||
*/
|
||||
export function matchesAnyPattern(value: string, patterns: string[]): boolean {
|
||||
return patterns.some((pattern) => {
|
||||
try {
|
||||
return new RegExp(pattern).test(value)
|
||||
} catch {
|
||||
// Invalid regex -- treat as non-match
|
||||
return false
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Intersect two optional arrays. If both are defined, return elements present
|
||||
* in both. If only one is defined, return that one. If neither, return undefined.
|
||||
*/
|
||||
function intersectOptionalArrays(a: string[] | undefined, b: string[] | undefined): string[] | undefined {
|
||||
if (!a && !b) {
|
||||
return undefined
|
||||
}
|
||||
if (!a) {
|
||||
return b
|
||||
}
|
||||
if (!b) {
|
||||
return a
|
||||
}
|
||||
|
||||
const setB = new Set(b)
|
||||
const result = a.filter((item) => setB.has(item))
|
||||
return result.length > 0 ? result : []
|
||||
}
|
||||
|
||||
/**
|
||||
* Union two optional arrays, deduplicating entries.
|
||||
*/
|
||||
function unionOptionalArrays(a: string[] | undefined, b: string[] | undefined): string[] | undefined {
|
||||
if (!a && !b) {
|
||||
return undefined
|
||||
}
|
||||
if (!a) {
|
||||
return b
|
||||
}
|
||||
if (!b) {
|
||||
return a
|
||||
}
|
||||
|
||||
return [...new Set([...a, ...b])]
|
||||
}
|
||||
|
|
@ -4,6 +4,7 @@ import { RooCodeEventName } from "./events.js"
|
|||
import type { RooCodeSettings } from "./global-settings.js"
|
||||
import type { ClineMessage, QueuedMessage, TokenUsage } from "./message.js"
|
||||
import type { ToolUsage, ToolName } from "./tool.js"
|
||||
import type { TaskPermissions } from "./task-permissions.js"
|
||||
import type { TodoItem } from "./todo.js"
|
||||
|
||||
/**
|
||||
|
|
@ -94,6 +95,9 @@ export interface CreateTaskOptions {
|
|||
/** Whether to start the task loop immediately (default: true).
|
||||
* When false, the caller must invoke `task.start()` manually. */
|
||||
startTask?: boolean
|
||||
/** Permission boundaries for the task, set by the parent via new_task tool.
|
||||
* When set, restricts what file paths, commands, and tools the task may use. */
|
||||
taskPermissions?: TaskPermissions
|
||||
}
|
||||
|
||||
export enum TaskStatus {
|
||||
|
|
|
|||
|
|
@ -551,6 +551,7 @@ export class NativeToolCallParser {
|
|||
if (partialArgs.todos !== undefined) {
|
||||
nativeArgs = {
|
||||
todos: partialArgs.todos,
|
||||
permissions: partialArgs.permissions,
|
||||
}
|
||||
}
|
||||
break
|
||||
|
|
@ -633,6 +634,7 @@ export class NativeToolCallParser {
|
|||
mode: partialArgs.mode,
|
||||
message: partialArgs.message,
|
||||
todos: partialArgs.todos,
|
||||
permissions: partialArgs.permissions,
|
||||
}
|
||||
}
|
||||
break
|
||||
|
|
@ -887,6 +889,7 @@ export class NativeToolCallParser {
|
|||
if (args.todos !== undefined) {
|
||||
nativeArgs = {
|
||||
todos: args.todos,
|
||||
permissions: args.permissions,
|
||||
} as NativeArgsFor<TName>
|
||||
}
|
||||
break
|
||||
|
|
@ -982,6 +985,7 @@ export class NativeToolCallParser {
|
|||
mode: args.mode,
|
||||
message: args.message,
|
||||
todos: args.todos,
|
||||
permissions: args.permissions,
|
||||
} as NativeArgsFor<TName>
|
||||
}
|
||||
break
|
||||
|
|
|
|||
|
|
@ -589,6 +589,7 @@ export async function presentAssistantMessage(cline: Task) {
|
|||
block.params,
|
||||
stateExperiments,
|
||||
includedTools,
|
||||
cline.taskPermissions,
|
||||
)
|
||||
} catch (error) {
|
||||
cline.consecutiveMistakeCount++
|
||||
|
|
|
|||
|
|
@ -10,6 +10,8 @@ const MESSAGE_PARAMETER_DESCRIPTION = `Initial user instructions or context for
|
|||
|
||||
const TODOS_PARAMETER_DESCRIPTION = `Optional initial todo list written as a markdown checklist; required when the workspace mandates todos`
|
||||
|
||||
const PERMISSIONS_PARAMETER_DESCRIPTION = `Optional JSON object defining permission boundaries for the subtask. Allows the parent to restrict the subtask's access. Supports: filePatterns (array of regex patterns for allowed file paths), commandPatterns (array of regex patterns for allowed commands), allowedTools (array of tool names the subtask may use), deniedTools (array of tool names the subtask may NOT use). Example: {"filePatterns":["src/components/.*"],"commandPatterns":["npm test.*"],"deniedTools":["execute_command"]}`
|
||||
|
||||
export default {
|
||||
type: "function",
|
||||
function: {
|
||||
|
|
@ -31,6 +33,10 @@ export default {
|
|||
type: ["string", "null"],
|
||||
description: TODOS_PARAMETER_DESCRIPTION,
|
||||
},
|
||||
permissions: {
|
||||
type: ["string", "null"],
|
||||
description: PERMISSIONS_PARAMETER_DESCRIPTION,
|
||||
},
|
||||
},
|
||||
required: ["mode", "message", "todos"],
|
||||
additionalProperties: false,
|
||||
|
|
|
|||
|
|
@ -39,6 +39,8 @@ import {
|
|||
TaskStatus,
|
||||
TodoItem,
|
||||
getApiProtocol,
|
||||
type TaskPermissions,
|
||||
mergeTaskPermissions,
|
||||
getModelId,
|
||||
isRetiredProvider,
|
||||
isIdleAsk,
|
||||
|
|
@ -159,6 +161,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
readonly taskId: string
|
||||
readonly rootTaskId?: string
|
||||
readonly parentTaskId?: string
|
||||
readonly taskPermissions?: TaskPermissions
|
||||
childTaskId?: string
|
||||
pendingNewTaskToolCallId?: string
|
||||
|
||||
|
|
@ -430,6 +433,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
initialTodos,
|
||||
workspacePath,
|
||||
initialStatus,
|
||||
taskPermissions,
|
||||
}: TaskOptions) {
|
||||
super()
|
||||
|
||||
|
|
@ -456,6 +460,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
this.parentTaskId = historyItem ? historyItem.parentTaskId : parentTask?.taskId
|
||||
this.childTaskId = undefined
|
||||
|
||||
// Merge task permissions with parent (most-restrictive-wins)
|
||||
this.taskPermissions = mergeTaskPermissions(parentTask?.taskPermissions, taskPermissions)
|
||||
|
||||
this.metadata = {
|
||||
task: historyItem ? historyItem.task : task,
|
||||
images: historyItem ? [] : images,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import * as vscode from "vscode"
|
||||
|
||||
import { TodoItem } from "@roo-code/types"
|
||||
import { type TaskPermissions, taskPermissionsSchema } from "@roo-code/types"
|
||||
|
||||
import { Task } from "../task/Task"
|
||||
import { getModeBySlug } from "../../shared/modes"
|
||||
|
|
@ -15,13 +16,14 @@ interface NewTaskParams {
|
|||
mode: string
|
||||
message: string
|
||||
todos?: string
|
||||
permissions?: string
|
||||
}
|
||||
|
||||
export class NewTaskTool extends BaseTool<"new_task"> {
|
||||
readonly name = "new_task" as const
|
||||
|
||||
async execute(params: NewTaskParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
|
||||
const { mode, message, todos } = params
|
||||
const { mode, message, todos, permissions: permissionsJson } = params
|
||||
const { askApproval, handleError, pushToolResult } = callbacks
|
||||
|
||||
try {
|
||||
|
|
@ -82,6 +84,33 @@ export class NewTaskTool extends BaseTool<"new_task"> {
|
|||
}
|
||||
}
|
||||
|
||||
// Parse and validate permissions if provided
|
||||
let parsedPermissions: TaskPermissions | undefined
|
||||
if (permissionsJson) {
|
||||
try {
|
||||
const raw = JSON.parse(permissionsJson)
|
||||
const result = taskPermissionsSchema.safeParse(raw)
|
||||
if (!result.success) {
|
||||
task.consecutiveMistakeCount++
|
||||
task.recordToolError("new_task")
|
||||
task.didToolFailInCurrentTurn = true
|
||||
pushToolResult(
|
||||
formatResponse.toolError(
|
||||
`Invalid permissions format: ${result.error.issues.map((i) => i.message).join(", ")}`,
|
||||
),
|
||||
)
|
||||
return
|
||||
}
|
||||
parsedPermissions = result.data
|
||||
} catch (error) {
|
||||
task.consecutiveMistakeCount++
|
||||
task.recordToolError("new_task")
|
||||
task.didToolFailInCurrentTurn = true
|
||||
pushToolResult(formatResponse.toolError("Invalid permissions: must be a valid JSON string"))
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
task.consecutiveMistakeCount = 0
|
||||
|
||||
// Un-escape one level of backslashes before '@' for hierarchical subtasks
|
||||
|
|
@ -101,6 +130,7 @@ export class NewTaskTool extends BaseTool<"new_task"> {
|
|||
mode: targetMode.name,
|
||||
content: message,
|
||||
todos: todoItems,
|
||||
...(parsedPermissions ? { permissions: parsedPermissions } : {}),
|
||||
})
|
||||
|
||||
const didApprove = await askApproval("tool", toolMessage)
|
||||
|
|
@ -115,6 +145,7 @@ export class NewTaskTool extends BaseTool<"new_task"> {
|
|||
message: unescapedMessage,
|
||||
initialTodos: todoItems,
|
||||
mode,
|
||||
permissions: parsedPermissions,
|
||||
})
|
||||
|
||||
// Reflect delegation in tool result (no pause/unpause, no wait)
|
||||
|
|
|
|||
219
src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts
Normal file
219
src/core/tools/__tests__/taskPermissionsEnforcement.spec.ts
Normal file
|
|
@ -0,0 +1,219 @@
|
|||
import { describe, it, expect } from "vitest"
|
||||
import { isToolAllowedForMode, TaskPermissionError } from "../validateToolUse"
|
||||
import type { TaskPermissions } from "@roo-code/types"
|
||||
import type { ModeConfig } from "@roo-code/types"
|
||||
|
||||
const codeMode: ModeConfig = {
|
||||
slug: "code",
|
||||
name: "Code",
|
||||
roleDefinition: "You are a coder",
|
||||
groups: ["read", "edit", "command", "mcp"],
|
||||
}
|
||||
|
||||
describe("TaskPermissions enforcement in isToolAllowedForMode", () => {
|
||||
describe("deniedTools", () => {
|
||||
it("throws TaskPermissionError when tool is in deniedTools", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
deniedTools: ["execute_command"],
|
||||
}
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"execute_command",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toThrow(TaskPermissionError)
|
||||
})
|
||||
|
||||
it("allows tools not in deniedTools", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
deniedTools: ["execute_command"],
|
||||
}
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"read_file",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("allowedTools", () => {
|
||||
it("throws TaskPermissionError when tool is not in allowedTools", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
allowedTools: ["read_file", "search_files"],
|
||||
}
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"write_to_file",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toThrow(TaskPermissionError)
|
||||
})
|
||||
|
||||
it("allows tools in allowedTools", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
allowedTools: ["read_file", "write_to_file"],
|
||||
}
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"read_file",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it("always allows ALWAYS_AVAILABLE_TOOLS even when allowedTools is set", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
allowedTools: ["read_file"],
|
||||
}
|
||||
// attempt_completion and ask_followup_question should always be allowed
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"attempt_completion",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("filePatterns", () => {
|
||||
it("throws TaskPermissionError when file path doesn't match any pattern", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
filePatterns: ["src/components/.*"],
|
||||
}
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"write_to_file",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
{ path: "src/utils/helper.ts" },
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toThrow(TaskPermissionError)
|
||||
})
|
||||
|
||||
it("allows file paths matching a pattern", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
filePatterns: ["src/components/.*"],
|
||||
}
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"write_to_file",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
{ path: "src/components/Button.tsx" },
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it("does not restrict tools without file paths", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
filePatterns: ["src/components/.*"],
|
||||
}
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"search_files",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
{ regex: "TODO" },
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("commandPatterns", () => {
|
||||
it("throws TaskPermissionError when command doesn't match any pattern", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
commandPatterns: ["npm test.*", "npm run lint"],
|
||||
}
|
||||
expect(() =>
|
||||
isToolAllowedForMode(
|
||||
"execute_command",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
{ command: "rm -rf /" },
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toThrow(TaskPermissionError)
|
||||
})
|
||||
|
||||
it("allows commands matching a pattern", () => {
|
||||
const permissions: TaskPermissions = {
|
||||
commandPatterns: ["npm test.*", "npm run lint"],
|
||||
}
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"execute_command",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
{ command: "npm test -- --coverage" },
|
||||
undefined,
|
||||
undefined,
|
||||
permissions,
|
||||
),
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("no permissions", () => {
|
||||
it("allows all tools when taskPermissions is undefined", () => {
|
||||
expect(
|
||||
isToolAllowedForMode(
|
||||
"execute_command",
|
||||
"code",
|
||||
[codeMode],
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
),
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -1,5 +1,5 @@
|
|||
import type { ToolName, ModeConfig, ExperimentId, GroupOptions, GroupEntry } from "@roo-code/types"
|
||||
import { toolNames as validToolNames } from "@roo-code/types"
|
||||
import type { ToolName, ModeConfig, ExperimentId, GroupOptions, GroupEntry, TaskPermissions } from "@roo-code/types"
|
||||
import { toolNames as validToolNames, matchesAnyPattern } from "@roo-code/types"
|
||||
import { customToolRegistry } from "@roo-code/core"
|
||||
|
||||
import { type Mode, FileRestrictionError, getModeBySlug, getGroupName } from "../../shared/modes"
|
||||
|
|
@ -37,6 +37,7 @@ export function validateToolUse(
|
|||
toolParams?: Record<string, unknown>,
|
||||
experiments?: Record<string, boolean>,
|
||||
includedTools?: string[],
|
||||
taskPermissions?: TaskPermissions,
|
||||
): void {
|
||||
// First, check if the tool name is actually a valid/known tool
|
||||
// This catches completely invalid tool names like "edit_file" that don't exist
|
||||
|
|
@ -56,6 +57,7 @@ export function validateToolUse(
|
|||
toolParams,
|
||||
experiments,
|
||||
includedTools,
|
||||
taskPermissions,
|
||||
)
|
||||
) {
|
||||
throw new Error(`Tool "${toolName}" is not allowed in ${mode} mode.`)
|
||||
|
|
@ -117,6 +119,19 @@ function doesFileMatchRegex(filePath: string, pattern: string): boolean {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Error thrown when a tool is denied by TaskPermissions.
|
||||
*/
|
||||
export class TaskPermissionError extends Error {
|
||||
constructor(
|
||||
public readonly toolName: string,
|
||||
public readonly reason: string,
|
||||
) {
|
||||
super(`Tool "${toolName}" is not allowed: ${reason}`)
|
||||
this.name = "TaskPermissionError"
|
||||
}
|
||||
}
|
||||
|
||||
export function isToolAllowedForMode(
|
||||
tool: string,
|
||||
modeSlug: string,
|
||||
|
|
@ -125,11 +140,75 @@ export function isToolAllowedForMode(
|
|||
toolParams?: Record<string, any>, // All tool parameters
|
||||
experiments?: Record<string, boolean>,
|
||||
includedTools?: string[], // Opt-in tools explicitly included (e.g., from modelInfo)
|
||||
taskPermissions?: TaskPermissions,
|
||||
): boolean {
|
||||
// Resolve alias to canonical name (e.g., "search_and_replace" → "edit")
|
||||
const resolvedTool = TOOL_ALIASES[tool] ?? tool
|
||||
const resolvedIncludedTools = includedTools?.map((t) => TOOL_ALIASES[t] ?? t)
|
||||
|
||||
// Check TaskPermissions first -- these are set by the parent task via new_task
|
||||
if (taskPermissions) {
|
||||
// Check deniedTools
|
||||
if (taskPermissions.deniedTools?.includes(resolvedTool) || taskPermissions.deniedTools?.includes(tool)) {
|
||||
throw new TaskPermissionError(tool, "This tool is denied by the parent task's permission boundaries.")
|
||||
}
|
||||
|
||||
// Check allowedTools (if set, only these tools are permitted)
|
||||
if (taskPermissions.allowedTools) {
|
||||
const isAllowed =
|
||||
taskPermissions.allowedTools.includes(resolvedTool) ||
|
||||
taskPermissions.allowedTools.includes(tool) ||
|
||||
// Always allow certain critical tools regardless of allowlist
|
||||
ALWAYS_AVAILABLE_TOOLS.includes(tool as any)
|
||||
if (!isAllowed) {
|
||||
throw new TaskPermissionError(tool, "This tool is not in the parent task's allowed tools list.")
|
||||
}
|
||||
}
|
||||
|
||||
// Check filePatterns for file-related operations
|
||||
if (taskPermissions.filePatterns && taskPermissions.filePatterns.length > 0) {
|
||||
const filePath = toolParams?.path || toolParams?.file_path
|
||||
if (filePath && typeof filePath === "string") {
|
||||
if (!matchesAnyPattern(filePath, taskPermissions.filePatterns)) {
|
||||
throw new TaskPermissionError(
|
||||
tool,
|
||||
`File "${filePath}" is outside the allowed file patterns: ${taskPermissions.filePatterns.join(", ")}`,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Check apply_patch file paths
|
||||
if (tool === "apply_patch" && typeof toolParams?.patch === "string") {
|
||||
const patchFilePaths = extractFilePathsFromPatch(toolParams.patch)
|
||||
for (const patchFilePath of patchFilePaths) {
|
||||
if (!matchesAnyPattern(patchFilePath, taskPermissions.filePatterns)) {
|
||||
throw new TaskPermissionError(
|
||||
tool,
|
||||
`File "${patchFilePath}" in patch is outside the allowed file patterns: ${taskPermissions.filePatterns.join(", ")}`,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check commandPatterns for execute_command
|
||||
if (
|
||||
taskPermissions.commandPatterns &&
|
||||
taskPermissions.commandPatterns.length > 0 &&
|
||||
resolvedTool === "execute_command"
|
||||
) {
|
||||
const command = toolParams?.command
|
||||
if (command && typeof command === "string") {
|
||||
if (!matchesAnyPattern(command, taskPermissions.commandPatterns)) {
|
||||
throw new TaskPermissionError(
|
||||
tool,
|
||||
`Command "${command}" is outside the allowed command patterns: ${taskPermissions.commandPatterns.join(", ")}`,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check tool requirements first — explicit disabling takes priority over everything,
|
||||
// including ALWAYS_AVAILABLE_TOOLS. This ensures disabledTools works consistently
|
||||
// at both the filtering layer and the execution-time validation layer.
|
||||
|
|
|
|||
|
|
@ -82,7 +82,7 @@ import { CustomModesManager } from "../config/CustomModesManager"
|
|||
import { Task } from "../task/Task"
|
||||
|
||||
import { webviewMessageHandler } from "./webviewMessageHandler"
|
||||
import type { ClineMessage, TodoItem } from "@roo-code/types"
|
||||
import type { ClineMessage, TodoItem, TaskPermissions } from "@roo-code/types"
|
||||
import { readApiMessages, saveApiMessages, saveTaskMessages, TaskHistoryStore } from "../task-persistence"
|
||||
import { readTaskMessages } from "../task-persistence/taskMessages"
|
||||
import { getNonce } from "./getNonce"
|
||||
|
|
@ -2785,8 +2785,9 @@ export class ClineProvider
|
|||
message: string
|
||||
initialTodos: TodoItem[]
|
||||
mode: string
|
||||
permissions?: TaskPermissions
|
||||
}): Promise<Task> {
|
||||
const { parentTaskId, message, initialTodos, mode } = params
|
||||
const { parentTaskId, message, initialTodos, mode, permissions } = params
|
||||
|
||||
// Metadata-driven delegation is always enabled
|
||||
|
||||
|
|
@ -2876,6 +2877,7 @@ export class ClineProvider
|
|||
const child = await this.createTask(message, undefined, parent as any, {
|
||||
initialTodos,
|
||||
initialStatus: "active",
|
||||
taskPermissions: permissions,
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -56,6 +56,7 @@ export const toolParamNames = [
|
|||
"start_line",
|
||||
"end_line",
|
||||
"todos",
|
||||
"permissions", // new_task parameter for subtask permission boundaries
|
||||
"prompt",
|
||||
"image",
|
||||
// read_file parameters (native protocol)
|
||||
|
|
@ -102,7 +103,7 @@ export type NativeToolArgs = {
|
|||
edit_file: { file_path: string; old_string: string; new_string: string; expected_replacements?: number }
|
||||
apply_patch: { patch: string }
|
||||
list_files: { path: string; recursive?: boolean }
|
||||
new_task: { mode: string; message: string; todos?: string }
|
||||
new_task: { mode: string; message: string; todos?: string; permissions?: string }
|
||||
ask_followup_question: {
|
||||
question: string
|
||||
follow_up: Array<{ text: string; mode?: string }>
|
||||
|
|
@ -240,7 +241,7 @@ export interface SwitchModeToolUse extends ToolUse<"switch_mode"> {
|
|||
|
||||
export interface NewTaskToolUse extends ToolUse<"new_task"> {
|
||||
name: "new_task"
|
||||
params: Partial<Pick<Record<ToolParamName, string>, "mode" | "message" | "todos">>
|
||||
params: Partial<Pick<Record<ToolParamName, string>, "mode" | "message" | "todos" | "permissions">>
|
||||
}
|
||||
|
||||
export interface RunSlashCommandToolUse extends ToolUse<"run_slash_command"> {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue