From d8c2a0f1a20589a93cee0c9e648d2d719468c20c Mon Sep 17 00:00:00 2001 From: Roo Code Date: Thu, 16 Oct 2025 23:07:05 +0000 Subject: [PATCH] fix: prevent read_file contents from being persisted in ui_messages.json - Add sanitization function to strip large file contents from messages - Sanitize messages before saving to ui_messages.json - Add backward compatibility to purge contents from existing messages - Add comprehensive tests for sanitization logic - Extract constants for better maintainability Fixes #8690 --- .../__tests__/sanitizeMessages.spec.ts | 251 ++++++++++++++++++ src/core/task-persistence/sanitizeMessages.ts | 86 ++++++ src/core/task-persistence/taskMessages.ts | 10 +- 3 files changed, 345 insertions(+), 2 deletions(-) create mode 100644 src/core/task-persistence/__tests__/sanitizeMessages.spec.ts create mode 100644 src/core/task-persistence/sanitizeMessages.ts diff --git a/src/core/task-persistence/__tests__/sanitizeMessages.spec.ts b/src/core/task-persistence/__tests__/sanitizeMessages.spec.ts new file mode 100644 index 0000000000..d741020ad5 --- /dev/null +++ b/src/core/task-persistence/__tests__/sanitizeMessages.spec.ts @@ -0,0 +1,251 @@ +import { describe, it, expect } from "vitest" +import { sanitizeMessagesForUIStorage, purgeFileContentsFromMessages } from "../sanitizeMessages" +import type { ClineMessage } from "@roo-code/types" + +describe("sanitizeMessages", () => { + describe("sanitizeMessagesForUIStorage", () => { + it("should leave non-tool messages unchanged", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "say", + say: "text", + text: "This is a regular text message", + }, + { + ts: 1234567891, + type: "ask", + ask: "followup", + text: "What would you like to do next?", + }, + ] + + const sanitized = sanitizeMessagesForUIStorage(messages) + expect(sanitized).toEqual(messages) + }) + + it("should strip content from single readFile tool messages", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "ask", + ask: "tool", + text: JSON.stringify({ + tool: "readFile", + path: "/path/to/file.ts", + content: + "const longFileContent = 'This is a very long file content that should be stripped because it is over 100 characters long and takes up unnecessary space in storage';", + }), + }, + ] + + const sanitized = sanitizeMessagesForUIStorage(messages) + const parsedText = JSON.parse(sanitized[0].text!) + + expect(parsedText.tool).toBe("readFile") + expect(parsedText.path).toBe("/path/to/file.ts") + expect(parsedText.content).toBe("[content stripped for storage]") + }) + + it("should keep short content in readFile messages", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "ask", + ask: "tool", + text: JSON.stringify({ + tool: "readFile", + path: "/path/to/file.ts", + content: "short content", + }), + }, + ] + + const sanitized = sanitizeMessagesForUIStorage(messages) + const parsedText = JSON.parse(sanitized[0].text!) + + expect(parsedText.content).toBe("short content") + }) + + it("should sanitize batchFiles in readFile tool messages", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "ask", + ask: "tool", + text: JSON.stringify({ + tool: "readFile", + batchFiles: [ + { + path: "/path/to/file1.ts", + lineSnippet: "(lines 1-100)", + isOutsideWorkspace: false, + key: "file1.ts (lines 1-100)", + content: "/full/path/to/file1.ts", + }, + { + path: "/path/to/file2.ts", + lineSnippet: "(lines 1-50)", + isOutsideWorkspace: true, + key: "file2.ts (lines 1-50)", + content: "/full/path/to/file2.ts", + }, + ], + }), + }, + ] + + const sanitized = sanitizeMessagesForUIStorage(messages) + const parsedText = JSON.parse(sanitized[0].text!) + + expect(parsedText.tool).toBe("readFile") + expect(parsedText.batchFiles).toHaveLength(2) + expect(parsedText.batchFiles[0].path).toBe("/path/to/file1.ts") + expect(parsedText.batchFiles[0].lineSnippet).toBe("(lines 1-100)") + expect(parsedText.batchFiles[0].content).toBeUndefined() + expect(parsedText.batchFiles[1].path).toBe("/path/to/file2.ts") + expect(parsedText.batchFiles[1].content).toBeUndefined() + }) + + it("should handle messages without text field", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "say", + say: "checkpoint_saved", + }, + ] + + const sanitized = sanitizeMessagesForUIStorage(messages) + expect(sanitized).toEqual(messages) + }) + + it("should handle messages with non-JSON text", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "say", + say: "text", + text: "This is not JSON", + }, + ] + + const sanitized = sanitizeMessagesForUIStorage(messages) + expect(sanitized).toEqual(messages) + }) + + it("should handle messages with malformed JSON", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "say", + say: "text", + text: '{"broken": json', + }, + ] + + const sanitized = sanitizeMessagesForUIStorage(messages) + expect(sanitized).toEqual(messages) + }) + + it("should preserve other tool messages unchanged", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "ask", + ask: "tool", + text: JSON.stringify({ + tool: "writeFile", + path: "/path/to/file.ts", + content: "new file content", + }), + }, + ] + + const sanitized = sanitizeMessagesForUIStorage(messages) + expect(sanitized).toEqual(messages) + }) + + it("should handle mixed message types", () => { + const messages: ClineMessage[] = [ + { + ts: 1, + type: "say", + say: "text", + text: "Regular message", + }, + { + ts: 2, + type: "ask", + ask: "tool", + text: JSON.stringify({ + tool: "readFile", + path: "/file.ts", + content: "a".repeat(200), // Long content + }), + }, + { + ts: 3, + type: "ask", + ask: "tool", + text: JSON.stringify({ + tool: "writeFile", + path: "/other.ts", + content: "write content", + }), + }, + ] + + const sanitized = sanitizeMessagesForUIStorage(messages) + + expect(sanitized[0]).toEqual(messages[0]) + + const readFileMsg = JSON.parse(sanitized[1].text!) + expect(readFileMsg.content).toBe("[content stripped for storage]") + + expect(sanitized[2]).toEqual(messages[2]) + }) + }) + + describe("purgeFileContentsFromMessages", () => { + it("should use the same sanitization logic", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "ask", + ask: "tool", + text: JSON.stringify({ + tool: "readFile", + path: "/path/to/file.ts", + content: "x".repeat(150), + }), + }, + ] + + const purged = purgeFileContentsFromMessages(messages) + const parsedText = JSON.parse(purged[0].text!) + + expect(parsedText.content).toBe("[content stripped for storage]") + }) + + it("should handle already sanitized messages gracefully", () => { + const messages: ClineMessage[] = [ + { + ts: 1234567890, + type: "ask", + ask: "tool", + text: JSON.stringify({ + tool: "readFile", + path: "/path/to/file.ts", + content: "[content stripped for storage]", + }), + }, + ] + + const purged = purgeFileContentsFromMessages(messages) + const parsedText = JSON.parse(purged[0].text!) + + expect(parsedText.content).toBe("[content stripped for storage]") + }) + }) +}) diff --git a/src/core/task-persistence/sanitizeMessages.ts b/src/core/task-persistence/sanitizeMessages.ts new file mode 100644 index 0000000000..606433c92b --- /dev/null +++ b/src/core/task-persistence/sanitizeMessages.ts @@ -0,0 +1,86 @@ +import type { ClineMessage } from "@roo-code/types" +import type { ClineSayTool } from "../../shared/ExtensionMessage" + +// Constants for sanitization +const CONTENT_TRUNCATION_LENGTH = 100 +const STRIPPED_CONTENT_MARKER = "[content stripped for storage]" + +/** + * Sanitizes messages for storage in ui_messages.json by removing large file contents + * from read_file tool messages while preserving essential metadata. + * This prevents storage bloat and UI performance issues. + * + * @param messages - Array of ClineMessage objects to sanitize + * @returns Sanitized copy of messages with file contents stripped + */ +export function sanitizeMessagesForUIStorage(messages: ClineMessage[]): ClineMessage[] { + return messages.map((message) => { + // Only process messages with text content + if (!message.text || typeof message.text !== "string") { + return message + } + + // Try to parse as JSON to check if it's a tool message + try { + const parsed = JSON.parse(message.text) + + // Check if this is a readFile tool message + if (parsed.tool === "readFile") { + const sanitized = sanitizeReadFileMessage(parsed) + return { + ...message, + text: JSON.stringify(sanitized), + } + } + + return message + } catch { + // Not JSON or parsing failed, return as-is + return message + } + }) +} + +/** + * Sanitizes a read_file tool message by removing file contents while preserving metadata + */ +function sanitizeReadFileMessage(toolMessage: any): any { + const sanitized: any = { + ...toolMessage, + } + + // Handle single file reads with content field + if ("content" in sanitized) { + // Keep the path but replace content with a placeholder + if (typeof sanitized.content === "string" && sanitized.content.length > CONTENT_TRUNCATION_LENGTH) { + sanitized.content = STRIPPED_CONTENT_MARKER + } + } + + // Handle batch file reads + if (sanitized.batchFiles && Array.isArray(sanitized.batchFiles)) { + sanitized.batchFiles = sanitized.batchFiles.map((file: any) => { + const sanitizedFile = { ...file } + // Remove the actual file content, keep only metadata + // Add type checking for content field + if ("content" in sanitizedFile && typeof sanitizedFile.content === "string") { + delete sanitizedFile.content + } + return sanitizedFile + }) + } + + return sanitized +} + +/** + * Purges file contents from existing messages during rehydration. + * This is used for backward compatibility to clean up already-saved messages + * that contain full file contents. + * + * @param messages - Array of ClineMessage objects from storage + * @returns Messages with file contents purged + */ +export function purgeFileContentsFromMessages(messages: ClineMessage[]): ClineMessage[] { + return sanitizeMessagesForUIStorage(messages) +} diff --git a/src/core/task-persistence/taskMessages.ts b/src/core/task-persistence/taskMessages.ts index 63a2eefbaa..331e766142 100644 --- a/src/core/task-persistence/taskMessages.ts +++ b/src/core/task-persistence/taskMessages.ts @@ -8,6 +8,7 @@ import { fileExistsAtPath } from "../../utils/fs" import { GlobalFileNames } from "../../shared/globalFileNames" import { getTaskDirectoryPath } from "../../utils/storage" +import { sanitizeMessagesForUIStorage, purgeFileContentsFromMessages } from "./sanitizeMessages" export type ReadTaskMessagesOptions = { taskId: string @@ -23,7 +24,10 @@ export async function readTaskMessages({ const fileExists = await fileExistsAtPath(filePath) if (fileExists) { - return JSON.parse(await fs.readFile(filePath, "utf8")) + const messages = JSON.parse(await fs.readFile(filePath, "utf8")) + // Purge file contents from existing messages for backward compatibility + // This handles tasks that were saved before the sanitization was implemented + return purgeFileContentsFromMessages(messages) } return [] @@ -38,5 +42,7 @@ export type SaveTaskMessagesOptions = { export async function saveTaskMessages({ messages, taskId, globalStoragePath }: SaveTaskMessagesOptions) { const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId) const filePath = path.join(taskDir, GlobalFileNames.uiMessages) - await safeWriteJson(filePath, messages) + // Sanitize messages before saving to prevent storage bloat + const sanitizedMessages = sanitizeMessagesForUIStorage(messages) + await safeWriteJson(filePath, sanitizedMessages) }