Skip to content

Commit 69f5f14

Browse files
mrubensmrubens
committed
Add logic to prevent auto-approving edits of configuration files
1 parent c0876d0 commit 69f5f14

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+468
-32
lines changed

‎packages/types/src/global-settings.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export const globalSettingsSchema = z.object({
3636
alwaysAllowReadOnlyOutsideWorkspace: z.boolean().optional(),
3737
alwaysAllowWrite: z.boolean().optional(),
3838
alwaysAllowWriteOutsideWorkspace: z.boolean().optional(),
39+
alwaysAllowWriteProtected: z.boolean().optional(),
3940
writeDelayMs: z.number().optional(),
4041
alwaysAllowBrowser: z.boolean().optional(),
4142
alwaysApproveResubmit: z.boolean().optional(),
@@ -177,6 +178,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
177178
alwaysAllowReadOnlyOutsideWorkspace: false,
178179
alwaysAllowWrite: true,
179180
alwaysAllowWriteOutsideWorkspace: false,
181+
alwaysAllowWriteProtected: false,
180182
writeDelayMs: 1000,
181183
alwaysAllowBrowser: true,
182184
alwaysApproveResubmit: true,

‎packages/types/src/message.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ export const clineMessageSchema = z.object({
153153
checkpoint: z.record(z.string(), z.unknown()).optional(),
154154
progressStatus: toolProgressStatusSchema.optional(),
155155
contextCondense: contextCondenseSchema.optional(),
156+
isProtected: z.boolean().optional(),
156157
})
157158

158159
export type ClineMessage = z.infer<typeof clineMessageSchema>

‎src/core/assistant-message/presentAssistantMessage.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,15 @@ export async function presentAssistantMessage(cline: Task) {
261261
type: ClineAsk,
262262
partialMessage?: string,
263263
progressStatus?: ToolProgressStatus,
264+
isProtected?: boolean,
264265
) => {
265-
const { response, text, images } = await cline.ask(type, partialMessage, false, progressStatus)
266+
const { response, text, images } = await cline.ask(
267+
type,
268+
partialMessage,
269+
false,
270+
progressStatus,
271+
isProtected || false,
272+
)
266273

267274
if (response !== "yesButtonClicked") {
268275
// Handle both messageResponse and noButtonClicked with text.

‎src/core/prompts/responses.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Anthropic } from "@anthropic-ai/sdk"
22
import * as path from "path"
33
import * as diff from "diff"
44
import { RooIgnoreController, LOCK_TEXT_SYMBOL } from "../ignore/RooIgnoreController"
5+
import { RooProtectedController } from "../protect/RooProtectedController"
56

67
export const formatResponse = {
78
toolDenied: () => `The user denied this operation.`,
@@ -95,6 +96,7 @@ Otherwise, if you have not completed the task and do not need additional informa
9596
didHitLimit: boolean,
9697
rooIgnoreController: RooIgnoreController | undefined,
9798
showRooIgnoredFiles: boolean,
99+
rooProtectedController?: RooProtectedController,
98100
): string => {
99101
const sorted = files
100102
.map((file) => {
@@ -143,7 +145,13 @@ Otherwise, if you have not completed the task and do not need additional informa
143145
// Otherwise, mark it with a lock symbol
144146
rooIgnoreParsed.push(LOCK_TEXT_SYMBOL + " " + filePath)
145147
} else {
146-
rooIgnoreParsed.push(filePath)
148+
// Check if file is write-protected (only for non-ignored files)
149+
const isWriteProtected = rooProtectedController?.isWriteProtected(absoluteFilePath) || false
150+
if (isWriteProtected) {
151+
rooIgnoreParsed.push("🛡️ " + filePath)
152+
} else {
153+
rooIgnoreParsed.push(filePath)
154+
}
147155
}
148156
}
149157
}

‎src/core/protect/RooProtectedController.ts‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import path from "path"
2+
import ignore, { Ignore } from "ignore"
3+
4+
export const SHIELD_SYMBOL = "\u{1F6E1}"
5+
6+
/**
7+
* Controls write access to Roo configuration files by enforcing protection patterns.
8+
* Prevents auto-approved modifications to sensitive Roo configuration files.
9+
*/
10+
export class RooProtectedController {
11+
private cwd: string
12+
private ignoreInstance: Ignore
13+
14+
// Predefined list of protected Roo configuration patterns
15+
private static readonly PROTECTED_PATTERNS = [
16+
".rooignore",
17+
".roo/**",
18+
".rooprotected", // For future use
19+
".roo*", // Any file starting with .roo
20+
]
21+
22+
constructor(cwd: string) {
23+
this.cwd = cwd
24+
// Initialize ignore instance with protected patterns
25+
this.ignoreInstance = ignore()
26+
this.ignoreInstance.add(RooProtectedController.PROTECTED_PATTERNS)
27+
}
28+
29+
/**
30+
* Check if a file is write-protected
31+
* @param filePath - Path to check (relative to cwd)
32+
* @returns true if file is write-protected, false otherwise
33+
*/
34+
isWriteProtected(filePath: string): boolean {
35+
try {
36+
// Normalize path to be relative to cwd and use forward slashes
37+
const absolutePath = path.resolve(this.cwd, filePath)
38+
const relativePath = path.relative(this.cwd, absolutePath).toPosix()
39+
40+
// Use ignore library to check if file matches any protected pattern
41+
return this.ignoreInstance.ignores(relativePath)
42+
} catch (error) {
43+
// If there's an error processing the path, err on the side of caution
44+
// Ignore is designed to work with relative file paths, so will throw error for paths outside cwd
45+
console.error(`Error checking protection for ${filePath}:`, error)
46+
return false
47+
}
48+
}
49+
50+
/**
51+
* Get set of write-protected files from a list
52+
* @param paths - Array of paths to filter (relative to cwd)
53+
* @returns Set of protected file paths
54+
*/
55+
getProtectedFiles(paths: string[]): Set<string> {
56+
const protectedFiles = new Set<string>()
57+
58+
for (const filePath of paths) {
59+
if (this.isWriteProtected(filePath)) {
60+
protectedFiles.add(filePath)
61+
}
62+
}
63+
64+
return protectedFiles
65+
}
66+
67+
/**
68+
* Filter an array of paths, marking which ones are protected
69+
* @param paths - Array of paths to check (relative to cwd)
70+
* @returns Array of objects with path and protection status
71+
*/
72+
annotatePathsWithProtection(paths: string[]): Array<{ path: string; isProtected: boolean }> {
73+
return paths.map((filePath) => ({
74+
path: filePath,
75+
isProtected: this.isWriteProtected(filePath),
76+
}))
77+
}
78+
79+
/**
80+
* Get display message for protected file operations
81+
*/
82+
getProtectionMessage(): string {
83+
return "This is a Roo configuration file and requires approval for modifications"
84+
}
85+
86+
/**
87+
* Get formatted instructions about protected files for the LLM
88+
* @returns Formatted instructions about file protection
89+
*/
90+
getInstructions(): string {
91+
const patterns = RooProtectedController.PROTECTED_PATTERNS.join(", ")
92+
return `# Protected Files\n\n(The following Roo configuration file patterns are write-protected and always require approval for modifications, regardless of autoapproval settings. When using list_files, you'll notice a ${SHIELD_SYMBOL} next to files that are write-protected.)\n\nProtected patterns: ${patterns}`
93+
}
94+
95+
/**
96+
* Get the list of protected patterns (for testing/debugging)
97+
*/
98+
static getProtectedPatterns(): readonly string[] {
99+
return RooProtectedController.PROTECTED_PATTERNS
100+
}
101+
}

‎src/core/protect/__tests__/RooProtectedController.spec.ts‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import path from "path"
2+
import { RooProtectedController } from "../RooProtectedController"
3+
4+
describe("RooProtectedController", () => {
5+
const TEST_CWD = "/test/workspace"
6+
let controller: RooProtectedController
7+
8+
beforeEach(() => {
9+
controller = new RooProtectedController(TEST_CWD)
10+
})
11+
12+
describe("isWriteProtected", () => {
13+
it("should protect .rooignore file", () => {
14+
expect(controller.isWriteProtected(".rooignore")).toBe(true)
15+
})
16+
17+
it("should protect files in .roo directory", () => {
18+
expect(controller.isWriteProtected(".roo/config.json")).toBe(true)
19+
expect(controller.isWriteProtected(".roo/settings/user.json")).toBe(true)
20+
expect(controller.isWriteProtected(".roo/modes/custom.json")).toBe(true)
21+
})
22+
23+
it("should protect roo.json file", () => {
24+
expect(controller.isWriteProtected("roo.json")).toBe(true)
25+
})
26+
27+
it("should protect .rooprotected file", () => {
28+
expect(controller.isWriteProtected(".rooprotected")).toBe(true)
29+
})
30+
31+
it("should protect files starting with .roo", () => {
32+
expect(controller.isWriteProtected(".roosettings")).toBe(true)
33+
expect(controller.isWriteProtected(".rooconfig")).toBe(true)
34+
})
35+
36+
it("should not protect regular files", () => {
37+
expect(controller.isWriteProtected("src/index.ts")).toBe(false)
38+
expect(controller.isWriteProtected("package.json")).toBe(false)
39+
expect(controller.isWriteProtected("README.md")).toBe(false)
40+
})
41+
42+
it("should not protect files that contain 'roo' but don't start with .roo", () => {
43+
expect(controller.isWriteProtected("src/roo-utils.ts")).toBe(false)
44+
expect(controller.isWriteProtected("config/roo.config.js")).toBe(false)
45+
})
46+
47+
it("should handle nested paths correctly", () => {
48+
expect(controller.isWriteProtected("src/.roo/config.json")).toBe(true) // .roo/** matches anywhere
49+
expect(controller.isWriteProtected("nested/.rooignore")).toBe(true) // .rooignore matches anywhere by default
50+
})
51+
52+
it("should handle absolute paths by converting to relative", () => {
53+
const absolutePath = path.join(TEST_CWD, ".rooignore")
54+
expect(controller.isWriteProtected(absolutePath)).toBe(true)
55+
})
56+
57+
it("should handle paths with different separators", () => {
58+
expect(controller.isWriteProtected(".roo\\config.json")).toBe(true)
59+
expect(controller.isWriteProtected(".roo/config.json")).toBe(true)
60+
})
61+
})
62+
63+
describe("getProtectedFiles", () => {
64+
it("should return set of protected files from a list", () => {
65+
const files = ["src/index.ts", ".rooignore", "package.json", ".roo/config.json", "README.md"]
66+
67+
const protectedFiles = controller.getProtectedFiles(files)
68+
69+
expect(protectedFiles).toEqual(new Set([".rooignore", ".roo/config.json", "roo.json"]))
70+
})
71+
72+
it("should return empty set when no files are protected", () => {
73+
const files = ["src/index.ts", "package.json", "README.md"]
74+
75+
const protectedFiles = controller.getProtectedFiles(files)
76+
77+
expect(protectedFiles).toEqual(new Set())
78+
})
79+
})
80+
81+
describe("annotatePathsWithProtection", () => {
82+
it("should annotate paths with protection status", () => {
83+
const files = ["src/index.ts", ".rooignore", ".roo/config.json", "package.json"]
84+
85+
const annotated = controller.annotatePathsWithProtection(files)
86+
87+
expect(annotated).toEqual([
88+
{ path: "src/index.ts", isProtected: false },
89+
{ path: ".rooignore", isProtected: true },
90+
{ path: ".roo/config.json", isProtected: true },
91+
{ path: "package.json", isProtected: false },
92+
])
93+
})
94+
})
95+
96+
describe("getProtectionMessage", () => {
97+
it("should return appropriate protection message", () => {
98+
const message = controller.getProtectionMessage()
99+
expect(message).toBe("This is a Roo configuration file and requires approval for modifications")
100+
})
101+
})
102+
103+
describe("getInstructions", () => {
104+
it("should return formatted instructions about protected files", () => {
105+
const instructions = controller.getInstructions()
106+
107+
expect(instructions).toContain("# Protected Files")
108+
expect(instructions).toContain("write-protected")
109+
expect(instructions).toContain(".rooignore")
110+
expect(instructions).toContain(".roo/**")
111+
expect(instructions).toContain("roo.json")
112+
expect(instructions).toContain("\u{1F6E1}") // Shield symbol
113+
})
114+
})
115+
116+
describe("getProtectedPatterns", () => {
117+
it("should return the list of protected patterns", () => {
118+
const patterns = RooProtectedController.getProtectedPatterns()
119+
120+
expect(patterns).toEqual([".rooignore", ".roo/**", ".rooprotected", ".roo*"])
121+
})
122+
})
123+
})

‎src/core/task/Task.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ import { SYSTEM_PROMPT } from "../prompts/system"
6565
import { ToolRepetitionDetector } from "../tools/ToolRepetitionDetector"
6666
import { FileContextTracker } from "../context-tracking/FileContextTracker"
6767
import { RooIgnoreController } from "../ignore/RooIgnoreController"
68+
import { RooProtectedController } from "../protect/RooProtectedController"
6869
import { type AssistantMessageContent, parseAssistantMessage, presentAssistantMessage } from "../assistant-message"
6970
import { truncateConversationIfNeeded } from "../sliding-window"
7071
import { ClineProvider } from "../webview/ClineProvider"
@@ -144,6 +145,7 @@ export class Task extends EventEmitter<ClineEvents> {
144145

145146
toolRepetitionDetector: ToolRepetitionDetector
146147
rooIgnoreController?: RooIgnoreController
148+
rooProtectedController?: RooProtectedController
147149
fileContextTracker: FileContextTracker
148150
urlContentFetcher: UrlContentFetcher
149151
terminalProcess?: RooTerminalProcess
@@ -223,6 +225,7 @@ export class Task extends EventEmitter<ClineEvents> {
223225
this.taskNumber = -1
224226

225227
this.rooIgnoreController = new RooIgnoreController(this.cwd)
228+
this.rooProtectedController = new RooProtectedController(this.cwd)
226229
this.fileContextTracker = new FileContextTracker(provider, this.taskId)
227230

228231
this.rooIgnoreController.initialize().catch((error) => {
@@ -406,6 +409,7 @@ export class Task extends EventEmitter<ClineEvents> {
406409
text?: string,
407410
partial?: boolean,
408411
progressStatus?: ToolProgressStatus,
412+
isProtected?: boolean,
409413
): Promise<{ response: ClineAskResponse; text?: string; images?: string[] }> {
410414
// If this Cline instance was aborted by the provider, then the only
411415
// thing keeping us alive is a promise still running in the background,
@@ -433,6 +437,7 @@ export class Task extends EventEmitter<ClineEvents> {
433437
lastMessage.text = text
434438
lastMessage.partial = partial
435439
lastMessage.progressStatus = progressStatus
440+
lastMessage.isProtected = isProtected
436441
// TODO: Be more efficient about saving and posting only new
437442
// data or one whole message at a time so ignore partial for
438443
// saves, and only post parts of partial message instead of
@@ -444,7 +449,7 @@ export class Task extends EventEmitter<ClineEvents> {
444449
// state.
445450
askTs = Date.now()
446451
this.lastMessageTs = askTs
447-
await this.addToClineMessages({ ts: askTs, type: "ask", ask: type, text, partial })
452+
await this.addToClineMessages({ ts: askTs, type: "ask", ask: type, text, partial, isProtected })
448453
throw new Error("Current ask promise was ignored (#2)")
449454
}
450455
} else {
@@ -471,6 +476,7 @@ export class Task extends EventEmitter<ClineEvents> {
471476
lastMessage.text = text
472477
lastMessage.partial = false
473478
lastMessage.progressStatus = progressStatus
479+
lastMessage.isProtected = isProtected
474480
await this.saveClineMessages()
475481
this.updateClineMessage(lastMessage)
476482
} else {
@@ -480,7 +486,7 @@ export class Task extends EventEmitter<ClineEvents> {
480486
this.askResponseImages = undefined
481487
askTs = Date.now()
482488
this.lastMessageTs = askTs
483-
await this.addToClineMessages({ ts: askTs, type: "ask", ask: type, text })
489+
await this.addToClineMessages({ ts: askTs, type: "ask", ask: type, text, isProtected })
484490
}
485491
}
486492
} else {
@@ -490,7 +496,7 @@ export class Task extends EventEmitter<ClineEvents> {
490496
this.askResponseImages = undefined
491497
askTs = Date.now()
492498
this.lastMessageTs = askTs
493-
await this.addToClineMessages({ ts: askTs, type: "ask", ask: type, text })
499+
await this.addToClineMessages({ ts: askTs, type: "ask", ask: type, text, isProtected })
494500
}
495501

496502
await pWaitFor(() => this.askResponse !== undefined || this.lastMessageTs !== askTs, { interval: 100 })

‎src/core/tools/insertContentTool.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ export async function insertContentTool(
6666
return
6767
}
6868

69+
// Check if file is write-protected
70+
const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false
71+
6972
const absolutePath = path.resolve(cline.cwd, relPath)
7073
const fileExists = await fileExistsAtPath(absolutePath)
7174

@@ -124,10 +127,11 @@ export async function insertContentTool(
124127
...sharedMessageProps,
125128
diff,
126129
lineNumber: lineNumber,
130+
isProtected: isWriteProtected,
127131
} satisfies ClineSayTool)
128132

129133
const didApprove = await cline
130-
.ask("tool", completeMessage, false)
134+
.ask("tool", completeMessage, isWriteProtected)
131135
.then((response) => response.response === "yesButtonClicked")
132136

133137
if (!didApprove) {

0 commit comments

Comments
 (0)