Skip to content

Commit 1192102

Browse files
committed
Redact read_file and mention payloads from ui_messages.json; prevent storing file contents in clineMessages (Fixes #8690)
1 parent 270dce5 commit 1192102

File tree

2 files changed

+102
-55
lines changed

2 files changed

+102
-55
lines changed

apps/vscode-e2e/src/suite/tools/read-file.test.ts

Lines changed: 10 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ suite("Roo Code read_file Tool", function () {
127127
let taskCompleted = false
128128
let errorOccurred: string | null = null
129129
let toolExecuted = false
130-
let toolResult: string | null = null
130+
let _toolResult: string | null = null
131131

132132
// Listen for messages
133133
const messageHandler = ({ message }: { message: ClineMessage }) => {
@@ -157,8 +157,8 @@ suite("Roo Code read_file Tool", function () {
157157
resultMatch = requestData.request.match(/Result:\s*\n([\s\S]+?)(?:\n\n|$)/)
158158
}
159159
if (resultMatch) {
160-
toolResult = resultMatch[1]
161-
console.log("Extracted tool result:", toolResult)
160+
_toolResult = resultMatch[1]
161+
console.log("Extracted tool result:", _toolResult)
162162
} else {
163163
console.log("Could not extract tool result from request")
164164
}
@@ -235,16 +235,7 @@ suite("Roo Code read_file Tool", function () {
235235
// Check that no errors occurred
236236
assert.strictEqual(errorOccurred, null, "No errors should have occurred")
237237

238-
// Verify the tool returned the correct content
239-
assert.ok(toolResult !== null, "Tool should have returned a result")
240-
// The tool returns content with line numbers, so we need to extract just the content
241-
// For single line, the format is "1 | Hello, World!"
242-
const actualContent = (toolResult as string).replace(/^\d+\s*\|\s*/, "")
243-
assert.strictEqual(
244-
actualContent.trim(),
245-
"Hello, World!",
246-
"Tool should have returned the exact file content",
247-
)
238+
// Note: read_file tool result content is redacted from clineMessages; verify via AI response instead.
248239

249240
// Also verify the AI mentioned the content in its response
250241
const hasContent = messages.some(
@@ -270,7 +261,7 @@ suite("Roo Code read_file Tool", function () {
270261
const messages: ClineMessage[] = []
271262
let taskCompleted = false
272263
let toolExecuted = false
273-
let toolResult: string | null = null
264+
let _toolResult: string | null = null
274265

275266
// Listen for messages
276267
const messageHandler = ({ message }: { message: ClineMessage }) => {
@@ -297,7 +288,7 @@ suite("Roo Code read_file Tool", function () {
297288
resultMatch = requestData.request.match(/Result:\s*\n([\s\S]+?)(?:\n\n|$)/)
298289
}
299290
if (resultMatch) {
300-
toolResult = resultMatch[1]
291+
_toolResult = resultMatch[1]
301292
console.log("Extracted multiline tool result")
302293
} else {
303294
console.log("Could not extract tool result from request")
@@ -344,20 +335,7 @@ suite("Roo Code read_file Tool", function () {
344335
// Verify the read_file tool was executed
345336
assert.ok(toolExecuted, "The read_file tool should have been executed")
346337

347-
// Verify the tool returned the correct multiline content
348-
assert.ok(toolResult !== null, "Tool should have returned a result")
349-
// The tool returns content with line numbers, so we need to extract just the content
350-
const lines = (toolResult as string).split("\n").map((line) => {
351-
const match = line.match(/^\d+\s*\|\s*(.*)$/)
352-
return match ? match[1] : line
353-
})
354-
const actualContent = lines.join("\n")
355-
const expectedContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5"
356-
assert.strictEqual(
357-
actualContent.trim(),
358-
expectedContent,
359-
"Tool should have returned the exact multiline content",
360-
)
338+
// Note: read_file tool result content is redacted from clineMessages; verify via AI response instead.
361339

362340
// Also verify the AI mentioned the correct number of lines
363341
const hasLineCount = messages.some(
@@ -381,7 +359,7 @@ suite("Roo Code read_file Tool", function () {
381359
const messages: ClineMessage[] = []
382360
let taskCompleted = false
383361
let toolExecuted = false
384-
let toolResult: string | null = null
362+
let _toolResult: string | null = null
385363

386364
// Listen for messages
387365
const messageHandler = ({ message }: { message: ClineMessage }) => {
@@ -408,7 +386,7 @@ suite("Roo Code read_file Tool", function () {
408386
resultMatch = requestData.request.match(/Result:\s*\n([\s\S]+?)(?:\n\n|$)/)
409387
}
410388
if (resultMatch) {
411-
toolResult = resultMatch[1]
389+
_toolResult = resultMatch[1]
412390
console.log("Extracted line range tool result")
413391
} else {
414392
console.log("Could not extract tool result from request")
@@ -455,22 +433,7 @@ suite("Roo Code read_file Tool", function () {
455433
// Verify tool was executed
456434
assert.ok(toolExecuted, "The read_file tool should have been executed")
457435

458-
// Verify the tool returned the correct lines (when line range is used)
459-
if (toolResult && (toolResult as string).includes(" | ")) {
460-
// The result includes line numbers
461-
assert.ok(
462-
(toolResult as string).includes("2 | Line 2"),
463-
"Tool result should include line 2 with line number",
464-
)
465-
assert.ok(
466-
(toolResult as string).includes("3 | Line 3"),
467-
"Tool result should include line 3 with line number",
468-
)
469-
assert.ok(
470-
(toolResult as string).includes("4 | Line 4"),
471-
"Tool result should include line 4 with line number",
472-
)
473-
}
436+
// Note: read_file tool result content is redacted from clineMessages; verify via AI response instead.
474437

475438
// Also verify the AI mentioned the specific lines
476439
const hasLines = messages.some(

src/core/task/Task.ts

Lines changed: 92 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -603,11 +603,55 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
603603

604604
// Cline Messages
605605

606+
// Redact file payloads from UI-persisted messages (ui_messages.json)
607+
// while leaving full content intact for apiConversationHistory.
608+
private sanitizeMessageText(text?: string): string | undefined {
609+
if (!text) return text
610+
611+
const scrub = (s: string): string => {
612+
// Replace inner contents of known file payload tags with an omission marker
613+
// Order matters: scrub more specific tags first.
614+
s = s.replace(/<file_content\b[\s\S]*?<\/file_content>/gi, "<file_content>[omitted]</file_content>")
615+
s = s.replace(/<content\b[^>]*>[\s\S]*?<\/content>/gi, "<content>[omitted]</content>")
616+
s = s.replace(/<file\b[^>]*>[\s\S]*?<\/file>/gi, "<file>[omitted]</file>")
617+
s = s.replace(/<files\b[^>]*>[\s\S]*?<\/files>/gi, "<files>[omitted]</files>")
618+
return s
619+
}
620+
621+
// If this is a JSON payload (e.g. api_req_started), try to sanitize the 'request' field.
622+
try {
623+
const obj = JSON.parse(text)
624+
if (obj && typeof obj === "object" && typeof obj.request === "string") {
625+
obj.request = scrub(obj.request)
626+
return JSON.stringify(obj)
627+
}
628+
} catch {
629+
// Not JSON, fall-through to raw scrub
630+
}
631+
632+
return scrub(text)
633+
}
634+
635+
// Sanitize an array of messages for persistence to UI storage
636+
private sanitizeMessagesArray(messages: ClineMessage[]): ClineMessage[] {
637+
return messages.map((m) => {
638+
if (typeof (m as any).text === "string") {
639+
return { ...m, text: this.sanitizeMessageText((m as any).text) }
640+
}
641+
return m
642+
})
643+
}
644+
606645
private async getSavedClineMessages(): Promise<ClineMessage[]> {
607-
return readTaskMessages({ taskId: this.taskId, globalStoragePath: this.globalStoragePath })
646+
const msgs = await readTaskMessages({ taskId: this.taskId, globalStoragePath: this.globalStoragePath })
647+
return this.sanitizeMessagesArray(msgs)
608648
}
609649

610650
private async addToClineMessages(message: ClineMessage) {
651+
// Sanitize any UI-persisted text before storing
652+
if (typeof message.text === "string") {
653+
message.text = this.sanitizeMessageText(message.text)
654+
}
611655
this.clineMessages.push(message)
612656
const provider = this.providerRef.deref()
613657
await provider?.postStateToWebview()
@@ -625,7 +669,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
625669
}
626670

627671
public async overwriteClineMessages(newMessages: ClineMessage[]) {
628-
this.clineMessages = newMessages
672+
this.clineMessages = this.sanitizeMessagesArray(newMessages)
629673

630674
// If deletion or history truncation leaves a condense_context as the last message,
631675
// ensure the next API call suppresses previous_response_id so the condensed context is respected.
@@ -643,6 +687,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
643687
}
644688

645689
private async updateClineMessage(message: ClineMessage) {
690+
// Ensure any updates are also sanitized before persisting/posting
691+
if (typeof message.text === "string") {
692+
message.text = this.sanitizeMessageText(message.text)
693+
}
646694
const provider = this.providerRef.deref()
647695
await provider?.postMessageToWebview({ type: "messageUpdated", clineMessage: message })
648696
this.emit(RooCodeEventName.Message, { action: "updated", message })
@@ -659,8 +707,11 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
659707

660708
private async saveClineMessages() {
661709
try {
710+
// Sanitize just before persisting to ensure any direct mutations are scrubbed
711+
const sanitizedMessages = this.sanitizeMessagesArray(this.clineMessages)
712+
662713
await saveTaskMessages({
663-
messages: this.clineMessages,
714+
messages: sanitizedMessages,
664715
taskId: this.taskId,
665716
globalStoragePath: this.globalStoragePath,
666717
})
@@ -670,7 +721,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
670721
rootTaskId: this.rootTaskId,
671722
parentTaskId: this.parentTaskId,
672723
taskNumber: this.taskNumber,
673-
messages: this.clineMessages,
724+
messages: sanitizedMessages,
674725
globalStoragePath: this.globalStoragePath,
675726
workspace: this.cwd,
676727
mode: this._taskMode || defaultModeSlug, // Use the task's own mode, not the current provider mode.
@@ -1790,12 +1841,45 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
17901841
const modelId = getModelId(this.apiConfiguration)
17911842
const apiProtocol = getApiProtocol(this.apiConfiguration.apiProvider, modelId)
17921843

1844+
// Redact any read_file results or file payload blocks from UI messages.
1845+
// This prevents file contents from being persisted to ui_messages.json while
1846+
// still sending full content to the LLM via apiConversationHistory.
1847+
const formatRequestWithReadFileRedaction = (blocks: Anthropic.Messages.ContentBlockParam[]) => {
1848+
let redactNext = false
1849+
const parts = blocks.map((block: any) => {
1850+
if (block?.type === "text") {
1851+
const text = String(block.text ?? "")
1852+
1853+
// 1) Detect the explicit read_file header line emitted by pushToolResult
1854+
const isReadFileHeader = /^\[read_file\b[\s\S]*\]\s*Result:/i.test(text)
1855+
1856+
// 2) Detect any XML-like file payloads that tools may include
1857+
// Examples: <files>...</files>, <file>...</file>, <content ...>...</content>, <file_content ...>...</file_content>
1858+
const looksLikeFilePayload = /<files[\s>]|<file[\s>]|<content\b|<file_content\b/i.test(text)
1859+
1860+
// If we see the header, show the header but redact the next text block (payload)
1861+
if (isReadFileHeader) {
1862+
redactNext = true
1863+
return text
1864+
}
1865+
1866+
// If the previous block was a read_file header, or this block itself looks like a file payload, redact it
1867+
if (redactNext || looksLikeFilePayload) {
1868+
redactNext = false
1869+
return "[tool output omitted from UI storage]"
1870+
}
1871+
}
1872+
1873+
// Default formatting for other blocks
1874+
return formatContentBlockToMarkdown(block as any)
1875+
})
1876+
return parts.join("\n\n")
1877+
}
1878+
17931879
await this.say(
17941880
"api_req_started",
17951881
JSON.stringify({
1796-
request:
1797-
currentUserContent.map((block) => formatContentBlockToMarkdown(block)).join("\n\n") +
1798-
"\n\nLoading...",
1882+
request: formatRequestWithReadFileRedaction(currentUserContent) + "\n\nLoading...",
17991883
apiProtocol,
18001884
}),
18011885
)
@@ -1835,7 +1919,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
18351919
const lastApiReqIndex = findLastIndex(this.clineMessages, (m) => m.say === "api_req_started")
18361920

18371921
this.clineMessages[lastApiReqIndex].text = JSON.stringify({
1838-
request: finalUserContent.map((block) => formatContentBlockToMarkdown(block)).join("\n\n"),
1922+
request: formatRequestWithReadFileRedaction(finalUserContent),
18391923
apiProtocol,
18401924
} satisfies ClineApiReqInfo)
18411925

0 commit comments

Comments
 (0)