Skip to content
This repository was archived by the owner on May 15, 2026. It is now read-only.

Commit d9a172d

Browse files
committed
fix: harden task permissions - anchor regex patterns, validate at schema 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 parent 6c51a5d commit d9a172d

3 files changed

Lines changed: 61 additions & 49 deletions

File tree

packages/types/src/__tests__/task-permissions.spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,20 @@ describe("TaskPermissions", () => {
3838
})
3939
expect(result.success).toBe(false)
4040
})
41+
42+
it("rejects invalid regex patterns in filePatterns", () => {
43+
const result = taskPermissionsSchema.safeParse({
44+
filePatterns: ["[invalid"],
45+
})
46+
expect(result.success).toBe(false)
47+
})
48+
49+
it("rejects invalid regex patterns in commandPatterns", () => {
50+
const result = taskPermissionsSchema.safeParse({
51+
commandPatterns: ["(unclosed"],
52+
})
53+
expect(result.success).toBe(false)
54+
})
4155
})
4256

4357
describe("toTaskPermissions", () => {
@@ -191,6 +205,19 @@ describe("TaskPermissions", () => {
191205
it("does not match restricted commands", () => {
192206
expect(matchesAnyPattern("rm -rf /", ["npm.*", "yarn.*"])).toBe(false)
193207
})
208+
209+
it("anchors patterns so substrings do not match", () => {
210+
// "src/.*" should NOT match a path that merely contains "src/" as a substring
211+
expect(matchesAnyPattern("evil/src/components/foo.ts", ["src/.*"])).toBe(false)
212+
// But should still match paths that start with src/
213+
expect(matchesAnyPattern("src/components/foo.ts", ["src/.*"])).toBe(true)
214+
})
215+
216+
it("respects pre-anchored patterns (starting with ^)", () => {
217+
// A pattern already starting with ^ should not be double-wrapped
218+
expect(matchesAnyPattern("src/foo.ts", ["^src/.*$"])).toBe(true)
219+
expect(matchesAnyPattern("evil/src/foo.ts", ["^src/.*$"])).toBe(false)
220+
})
194221
})
195222

196223
describe("matchesAllPatternLayers", () => {

packages/types/src/task-permissions.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,34 @@ import { z } from "zod"
99
* more access than its parent.
1010
*/
1111

12+
/** Zod refinement that rejects strings which are not valid regular expressions. */
13+
const regexString = z.string().refine(
14+
(val) => {
15+
try {
16+
new RegExp(val)
17+
return true
18+
} catch {
19+
return false
20+
}
21+
},
22+
{ message: "Invalid regular expression" },
23+
)
24+
1225
export const taskPermissionsSchema = z.object({
1326
/**
1427
* Regex patterns for allowed file paths.
1528
* When set, file operations (read/write) are restricted to paths matching
16-
* at least one of these patterns.
29+
* at least one of these patterns. Patterns are automatically anchored
30+
* (wrapped in `^(?:...)$`) at runtime so they match the full path.
1731
*/
18-
filePatterns: z.array(z.string()).optional(),
32+
filePatterns: z.array(regexString).optional(),
1933

2034
/**
2135
* Regex patterns for allowed shell commands.
2236
* When set, command execution is restricted to commands matching
23-
* at least one of these patterns.
37+
* at least one of these patterns. Patterns are automatically anchored.
2438
*/
25-
commandPatterns: z.array(z.string()).optional(),
39+
commandPatterns: z.array(regexString).optional(),
2640

2741
/**
2842
* Explicit tool allowlist. When set, only these tools may be used
@@ -174,7 +188,10 @@ function collectPatternLayers(
174188
export function matchesAnyPattern(value: string, patterns: string[]): boolean {
175189
return patterns.some((pattern) => {
176190
try {
177-
return new RegExp(pattern).test(value)
191+
// Anchor patterns so they must match the entire value, not a substring.
192+
// This prevents "src/.*" from matching "evil/src/foo".
193+
const anchored = pattern.startsWith("^") ? pattern : `^(?:${pattern})$`
194+
return new RegExp(anchored).test(value)
178195
} catch {
179196
// Invalid regex -- treat as non-match
180197
return false

src/core/tools/validateToolUse.ts

Lines changed: 12 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ToolName, ModeConfig, ExperimentId, GroupOptions, GroupEntry, TaskPermissions } from "@roo-code/types"
2-
import { toolNames as validToolNames, matchesAnyPattern, matchesAllPatternLayers } from "@roo-code/types"
2+
import { toolNames as validToolNames, matchesAllPatternLayers } from "@roo-code/types"
33
import { customToolRegistry } from "@roo-code/core"
44

55
import { type Mode, FileRestrictionError, getModeBySlug, getGroupName } from "../../shared/modes"
@@ -170,9 +170,12 @@ export function isToolAllowedForMode(
170170
}
171171
}
172172

173-
// Check filePatterns using layered enforcement (AND between layers, OR within each layer).
174-
// Falls back to flat filePatterns if no layers are present.
175-
const filePatternLayers = taskPermissions._filePatternLayers
173+
// Check filePatterns -- use layered enforcement when available (AND between
174+
// layers, OR within each layer), fall back to flat filePatterns as a single layer.
175+
const filePatternLayers =
176+
taskPermissions._filePatternLayers ??
177+
(taskPermissions.filePatterns?.length ? [taskPermissions.filePatterns] : undefined)
178+
176179
if (filePatternLayers && filePatternLayers.length > 0) {
177180
const filePath = toolParams?.path || toolParams?.file_path
178181
if (filePath && typeof filePath === "string") {
@@ -193,55 +196,20 @@ export function isToolAllowedForMode(
193196
}
194197
}
195198
}
196-
} else if (taskPermissions.filePatterns && taskPermissions.filePatterns.length > 0) {
197-
// Fallback for non-merged permissions (single layer)
198-
const filePath = toolParams?.path || toolParams?.file_path
199-
if (filePath && typeof filePath === "string") {
200-
if (!matchesAnyPattern(filePath, taskPermissions.filePatterns)) {
201-
throw new TaskPermissionError(
202-
tool,
203-
`File "${filePath}" is outside the allowed file patterns: ${taskPermissions.filePatterns.join(", ")}`,
204-
)
205-
}
206-
}
207-
208-
if (tool === "apply_patch" && typeof toolParams?.patch === "string") {
209-
const patchFilePaths = extractFilePathsFromPatch(toolParams.patch)
210-
for (const patchFilePath of patchFilePaths) {
211-
if (!matchesAnyPattern(patchFilePath, taskPermissions.filePatterns)) {
212-
throw new TaskPermissionError(
213-
tool,
214-
`File "${patchFilePath}" in patch is outside the allowed file patterns: ${taskPermissions.filePatterns.join(", ")}`,
215-
)
216-
}
217-
}
218-
}
219199
}
220200

221-
// Check commandPatterns using layered enforcement
222-
const commandPatternLayers = taskPermissions._commandPatternLayers
201+
// Check commandPatterns -- same layered approach as filePatterns.
202+
const commandPatternLayers =
203+
taskPermissions._commandPatternLayers ??
204+
(taskPermissions.commandPatterns?.length ? [taskPermissions.commandPatterns] : undefined)
205+
223206
if (commandPatternLayers && commandPatternLayers.length > 0 && resolvedTool === "execute_command") {
224207
const command = toolParams?.command
225208
if (command && typeof command === "string") {
226209
if (!matchesAllPatternLayers(command, commandPatternLayers)) {
227210
throw new TaskPermissionError(tool, `Command "${command}" is outside the allowed command patterns.`)
228211
}
229212
}
230-
} else if (
231-
taskPermissions.commandPatterns &&
232-
taskPermissions.commandPatterns.length > 0 &&
233-
resolvedTool === "execute_command"
234-
) {
235-
// Fallback for non-merged permissions (single layer)
236-
const command = toolParams?.command
237-
if (command && typeof command === "string") {
238-
if (!matchesAnyPattern(command, taskPermissions.commandPatterns)) {
239-
throw new TaskPermissionError(
240-
tool,
241-
`Command "${command}" is outside the allowed command patterns: ${taskPermissions.commandPatterns.join(", ")}`,
242-
)
243-
}
244-
}
245213
}
246214
}
247215

0 commit comments

Comments
 (0)