diff --git a/packages/types/src/__tests__/task-permissions.spec.ts b/packages/types/src/__tests__/task-permissions.spec.ts index cea98cff38e..15e8839f2b7 100644 --- a/packages/types/src/__tests__/task-permissions.spec.ts +++ b/packages/types/src/__tests__/task-permissions.spec.ts @@ -5,7 +5,9 @@ import { matchesAllPatternLayers, taskPermissionsSchema, toTaskPermissions, + isSafeRegex, } from "../task-permissions.js" +import { historyItemSchema } from "../history.js" import type { TaskPermissions } from "../task-permissions.js" describe("TaskPermissions", () => { @@ -246,4 +248,174 @@ describe("TaskPermissions", () => { expect(matchesAllPatternLayers("docs/readme.md", layers)).toBe(false) }) }) + + describe("isSafeRegex", () => { + it("allows simple patterns", () => { + expect(isSafeRegex("src/.*")).toBe(true) + expect(isSafeRegex("^foo$")).toBe(true) + expect(isSafeRegex("[a-z]+")).toBe(true) + expect(isSafeRegex("npm\\s+test.*")).toBe(true) + expect(isSafeRegex("src/components/.*\\.tsx?")).toBe(true) + }) + + it("rejects nested quantifiers (a+)+", () => { + expect(isSafeRegex("(a+)+")).toBe(false) + }) + + it("rejects nested quantifiers (a*)*", () => { + expect(isSafeRegex("(a*)*")).toBe(false) + }) + + it("rejects nested quantifiers (.*)+", () => { + expect(isSafeRegex("(.*)+")).toBe(false) + }) + + it("rejects nested quantifiers with braces (a{1,})+", () => { + expect(isSafeRegex("(a{1,})+")).toBe(false) + }) + + it("rejects star-height > 1 patterns like (.+)+", () => { + expect(isSafeRegex("(.+)+")).toBe(false) + }) + + it("rejects star-height > 1 patterns like (\\w+)*", () => { + expect(isSafeRegex("(\\w+)*")).toBe(false) + }) + + it("rejects patterns that exceed max length", () => { + const longPattern = "a".repeat(501) + expect(isSafeRegex(longPattern)).toBe(false) + }) + + it("allows patterns at max length", () => { + const maxPattern = "a".repeat(500) + expect(isSafeRegex(maxPattern)).toBe(true) + }) + }) + + describe("schema rejects unsafe regex", () => { + it("rejects nested quantifiers in filePatterns", () => { + const result = taskPermissionsSchema.safeParse({ + filePatterns: ["(a+)+"], + }) + expect(result.success).toBe(false) + }) + + it("rejects nested quantifiers in commandPatterns", () => { + const result = taskPermissionsSchema.safeParse({ + commandPatterns: ["(.*)+"], + }) + expect(result.success).toBe(false) + }) + + it("allows safe regex in schema", () => { + const result = taskPermissionsSchema.safeParse({ + filePatterns: ["src/components/.*\\.tsx?"], + commandPatterns: ["npm\\s+test.*"], + }) + expect(result.success).toBe(true) + }) + }) + + describe("matchesAnyPattern skips unsafe patterns at runtime", () => { + it("skips unsafe pattern and does not match", () => { + // (a+)+ is a ReDoS-prone pattern -- should be skipped + expect(matchesAnyPattern("aaa", ["(a+)+"])).toBe(false) + }) + + it("still matches safe patterns alongside unsafe ones", () => { + // The safe pattern "aaa" should still work + expect(matchesAnyPattern("aaa", ["(a+)+", "aaa"])).toBe(true) + }) + }) + + describe("HistoryItem taskPermissions persistence", () => { + it("accepts a HistoryItem with taskPermissions", () => { + const result = historyItemSchema.safeParse({ + id: "test-task", + number: 1, + ts: Date.now(), + task: "Test task", + tokensIn: 100, + tokensOut: 50, + totalCost: 0.01, + taskPermissions: { + filePatterns: ["src/.*"], + commandPatterns: ["npm test.*"], + allowedTools: ["read_file"], + deniedTools: ["execute_command"], + }, + }) + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.taskPermissions).toBeDefined() + expect(result.data.taskPermissions?.filePatterns).toEqual(["src/.*"]) + } + }) + + it("accepts a HistoryItem without taskPermissions", () => { + const result = historyItemSchema.safeParse({ + id: "test-task", + number: 1, + ts: Date.now(), + task: "Test task", + tokensIn: 100, + tokensOut: 50, + totalCost: 0.01, + }) + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.taskPermissions).toBeUndefined() + } + }) + + it("rejects HistoryItem with invalid taskPermissions regex", () => { + const result = historyItemSchema.safeParse({ + id: "test-task", + number: 1, + ts: Date.now(), + task: "Test task", + tokensIn: 100, + tokensOut: 50, + totalCost: 0.01, + taskPermissions: { + filePatterns: ["[invalid"], + }, + }) + expect(result.success).toBe(false) + }) + + it("rejects HistoryItem with unsafe ReDoS patterns", () => { + const result = historyItemSchema.safeParse({ + id: "test-task", + number: 1, + ts: Date.now(), + task: "Test task", + tokensIn: 100, + tokensOut: 50, + totalCost: 0.01, + taskPermissions: { + filePatterns: ["(a+)+"], + }, + }) + expect(result.success).toBe(false) + }) + + it("round-trips permissions through toTaskPermissions", () => { + const input = { + filePatterns: ["src/.*"], + commandPatterns: ["npm.*"], + allowedTools: ["read_file"], + deniedTools: ["execute_command"], + } + const internal = toTaskPermissions(input) + expect(internal._filePatternLayers).toEqual([["src/.*"]]) + expect(internal._commandPatternLayers).toEqual([["npm.*"]]) + // Serializable fields survive round-trip + expect(internal.filePatterns).toEqual(input.filePatterns) + expect(internal.commandPatterns).toEqual(input.commandPatterns) + expect(internal.allowedTools).toEqual(input.allowedTools) + expect(internal.deniedTools).toEqual(input.deniedTools) + }) + }) }) diff --git a/packages/types/src/history.ts b/packages/types/src/history.ts index a60d1a75b65..facf3c6d763 100644 --- a/packages/types/src/history.ts +++ b/packages/types/src/history.ts @@ -1,5 +1,7 @@ import { z } from "zod" +import { taskPermissionsSchema } from "./task-permissions.js" + /** * HistoryItem */ @@ -26,6 +28,9 @@ export const historyItemSchema = z.object({ awaitingChildId: z.string().optional(), // Child currently awaited (set when delegated) completedByChildId: z.string().optional(), // Child that completed and resumed this parent completionResultSummary: z.string().optional(), // Summary from completed child + /** Persisted task permissions set by parent via new_task tool. + * Allows permissions to survive VS Code restarts for subtasks. */ + taskPermissions: taskPermissionsSchema.optional(), }) export type HistoryItem = z.infer diff --git a/packages/types/src/task-permissions.ts b/packages/types/src/task-permissions.ts index 61f5001ffe9..4330f120ae7 100644 --- a/packages/types/src/task-permissions.ts +++ b/packages/types/src/task-permissions.ts @@ -9,18 +9,67 @@ import { z } from "zod" * more access than its parent. */ -/** Zod refinement that rejects strings which are not valid regular expressions. */ -const regexString = z.string().refine( - (val) => { - try { - new RegExp(val) - return true - } catch { - return false - } - }, - { message: "Invalid regular expression" }, -) +/** + * Maximum allowed length for a regex pattern string. + * Keeps patterns short to limit the surface area for ReDoS. + */ +const MAX_REGEX_LENGTH = 500 + +/** + * Heuristic check for ReDoS-prone regex patterns. + * + * Detects common dangerous constructs: + * - Nested quantifiers: `(a+)+`, `(a*)*`, `(a+)*`, `(a{1,})+` etc. + * - Overlapping alternation with quantifiers that can cause exponential backtracking + * + * This is a conservative heuristic -- it may reject some safe patterns, + * but it will catch the most common ReDoS vectors. + * + * @returns `true` if the pattern appears safe, `false` if it looks dangerous. + */ +export function isSafeRegex(pattern: string): boolean { + if (pattern.length > MAX_REGEX_LENGTH) { + return false + } + + // Detect nested quantifiers: a group/char-class followed by a quantifier, + // itself followed by another quantifier. + // e.g., (a+)+ (\w+)* [a-z]+* (.+){2,}+ + // Pattern: something quantified inside a group, then the group is quantified again. + // We look for `)` followed by a quantifier, preceded by content that also has a quantifier. + const nestedQuantifierRe = /(\+|\*|\?|\{[0-9,]+\})\s*\)(\+|\*|\?|\{[0-9,]+\})/ + if (nestedQuantifierRe.test(pattern)) { + return false + } + + // Detect star-height > 1 patterns like (.+)+ (.*)* (.+)* + const starHeightRe = /\(([^)]*(\+|\*|\{[0-9,]+\})[^)]*)\)(\+|\*|\{[0-9,]+\})/ + if (starHeightRe.test(pattern)) { + return false + } + + return true +} + +/** Zod refinement that rejects strings which are not valid regular expressions + * or that contain potentially dangerous ReDoS patterns. */ +const regexString = z + .string() + .refine( + (val) => { + try { + new RegExp(val) + return true + } catch { + return false + } + }, + { message: "Invalid regular expression" }, + ) + .refine((val) => isSafeRegex(val), { + message: + "Regex pattern is potentially unsafe (nested quantifiers or excessive length). Simplify the pattern to avoid ReDoS risk.", + }) export const taskPermissionsSchema = z.object({ /** @@ -188,6 +237,11 @@ function collectPatternLayers( export function matchesAnyPattern(value: string, patterns: string[]): boolean { return patterns.some((pattern) => { try { + // Skip patterns that fail the safety heuristic at runtime + // (belt-and-suspenders: Zod schema also checks at parse time) + if (!isSafeRegex(pattern)) { + return false + } // Anchor patterns so they must match the entire value, not a substring. // This prevents "src/.*" from matching "evil/src/foo". const anchored = pattern.startsWith("^") ? pattern : `^(?:${pattern})$` diff --git a/src/core/task-persistence/taskMetadata.ts b/src/core/task-persistence/taskMetadata.ts index 4b771269713..771353bb554 100644 --- a/src/core/task-persistence/taskMetadata.ts +++ b/src/core/task-persistence/taskMetadata.ts @@ -1,7 +1,7 @@ import NodeCache from "node-cache" import getFolderSize from "get-folder-size" -import type { ClineMessage, HistoryItem } from "@roo-code/types" +import type { ClineMessage, HistoryItem, TaskPermissionsInput } from "@roo-code/types" import { combineApiRequests } from "../../shared/combineApiRequests" import { combineCommandSequences } from "../../shared/combineCommandSequences" @@ -25,6 +25,8 @@ export type TaskMetadataOptions = { apiConfigName?: string /** Initial status for the task (e.g., "active" for child tasks) */ initialStatus?: "active" | "delegated" | "completed" + /** Task permissions set by parent via new_task tool, persisted for restart survival */ + taskPermissions?: TaskPermissionsInput } export async function taskMetadata({ @@ -38,6 +40,7 @@ export async function taskMetadata({ mode, apiConfigName, initialStatus, + taskPermissions, }: TaskMetadataOptions) { const taskDir = await getTaskDirectoryPath(globalStoragePath, id) @@ -112,6 +115,7 @@ export async function taskMetadata({ mode, ...(typeof apiConfigName === "string" && apiConfigName.length > 0 ? { apiConfigName } : {}), ...(initialStatus && { status: initialStatus }), + ...(taskPermissions && { taskPermissions }), } return { historyItem, tokenUsage } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 80c70841c0c..93ebd9b38ca 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1193,6 +1193,17 @@ export class Task extends EventEmitter implements TaskLike { mode: this._taskMode || defaultModeSlug, // Use the task's own mode, not the current provider mode. apiConfigName: this._taskApiConfigName, // Use the task's own provider profile, not the current provider profile. initialStatus: this.initialStatus, + // Persist the serializable (input) permission fields so they survive restarts. + // Internal layer fields (_filePatternLayers, _commandPatternLayers) are + // recomputed from the flat fields via toTaskPermissions() on restore. + taskPermissions: this.taskPermissions + ? { + filePatterns: this.taskPermissions.filePatterns, + commandPatterns: this.taskPermissions.commandPatterns, + allowedTools: this.taskPermissions.allowedTools, + deniedTools: this.taskPermissions.deniedTools, + } + : undefined, }) // Emit token/tool usage updates using debounced function diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6fd4ed20cff..f571cbc954e 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -82,7 +82,7 @@ import { CustomModesManager } from "../config/CustomModesManager" import { Task } from "../task/Task" import { webviewMessageHandler } from "./webviewMessageHandler" -import type { ClineMessage, TodoItem, TaskPermissions } from "@roo-code/types" +import { toTaskPermissions, type ClineMessage, type TodoItem, type TaskPermissions } from "@roo-code/types" import { readApiMessages, saveApiMessages, saveTaskMessages, TaskHistoryStore } from "../task-persistence" import { readTaskMessages } from "../task-persistence/taskMessages" import { getNonce } from "./getNonce" @@ -953,6 +953,10 @@ export class ClineProvider startTask: options?.startTask ?? true, // Preserve the status from the history item to avoid overwriting it when the task saves messages initialStatus: historyItem.status, + // Restore persisted task permissions from history so they survive VS Code restarts. + // toTaskPermissions() converts the flat input fields back into the internal + // representation with pattern layers for runtime enforcement. + taskPermissions: historyItem.taskPermissions ? toTaskPermissions(historyItem.taskPermissions) : undefined, }) if (isRehydratingCurrentTask) {