Skip to content

Redact read_file content from UI storage #8692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Redact read_file content from UI storage #8692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1192102
Redact read_file and mention payloads from ui_messages.json; prevent …
hannesrudolph Oct 16, 2025
0377592
test(read-file): remove redundant inline notes per review by @daniel-lxs
hannesrudolph Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 7 additions & 50 deletions apps/vscode-e2e/src/suite/tools/read-file.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ suite("Roo Code read_file Tool", function () {
let taskCompleted = false
let errorOccurred: string | null = null
let toolExecuted = false
let toolResult: string | null = null
let _toolResult: string | null = null
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_toolResult is never read after assignment in this test. Remove this variable and the associated extraction/logging code to reduce noise and keep the test focused on verifying AI responses.

Suggested change
let _toolResult: string | null = null

Copilot uses AI. Check for mistakes.


// Listen for messages
const messageHandler = ({ message }: { message: ClineMessage }) => {
Expand Down Expand Up @@ -157,8 +157,8 @@ suite("Roo Code read_file Tool", function () {
resultMatch = requestData.request.match(/Result:\s*\n([\s\S]+?)(?:\n\n|$)/)
}
if (resultMatch) {
toolResult = resultMatch[1]
console.log("Extracted tool result:", toolResult)
_toolResult = resultMatch[1]
console.log("Extracted tool result:", _toolResult)
} else {
console.log("Could not extract tool result from request")
}
Comment on lines 159 to 164
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extraction/logging block sets _toolResult but the value is no longer asserted, making it dead code. Remove the extraction and console logs to simplify the test.

Copilot uses AI. Check for mistakes.

Expand Down Expand Up @@ -235,17 +235,6 @@ suite("Roo Code read_file Tool", function () {
// Check that no errors occurred
assert.strictEqual(errorOccurred, null, "No errors should have occurred")

// Verify the tool returned the correct content
assert.ok(toolResult !== null, "Tool should have returned a result")
// The tool returns content with line numbers, so we need to extract just the content
// For single line, the format is "1 | Hello, World!"
const actualContent = (toolResult as string).replace(/^\d+\s*\|\s*/, "")
assert.strictEqual(
actualContent.trim(),
"Hello, World!",
"Tool should have returned the exact file content",
)

// Also verify the AI mentioned the content in its response
const hasContent = messages.some(
(m) =>
Expand All @@ -270,7 +259,7 @@ suite("Roo Code read_file Tool", function () {
const messages: ClineMessage[] = []
let taskCompleted = false
let toolExecuted = false
let toolResult: string | null = null
let _toolResult: string | null = null

// Listen for messages
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_toolResult is declared but never used in this test case. Remove the variable and the related parsing code added for it.

Copilot uses AI. Check for mistakes.

const messageHandler = ({ message }: { message: ClineMessage }) => {
Expand All @@ -297,7 +286,7 @@ suite("Roo Code read_file Tool", function () {
resultMatch = requestData.request.match(/Result:\s*\n([\s\S]+?)(?:\n\n|$)/)
}
if (resultMatch) {
toolResult = resultMatch[1]
_toolResult = resultMatch[1]
console.log("Extracted multiline tool result")
} else {
console.log("Could not extract tool result from request")
Expand Down Expand Up @@ -344,21 +333,6 @@ suite("Roo Code read_file Tool", function () {
// Verify the read_file tool was executed
assert.ok(toolExecuted, "The read_file tool should have been executed")

// Verify the tool returned the correct multiline content
assert.ok(toolResult !== null, "Tool should have returned a result")
// The tool returns content with line numbers, so we need to extract just the content
const lines = (toolResult as string).split("\n").map((line) => {
const match = line.match(/^\d+\s*\|\s*(.*)$/)
return match ? match[1] : line
})
const actualContent = lines.join("\n")
const expectedContent = "Line 1\nLine 2\nLine 3\nLine 4\nLine 5"
assert.strictEqual(
actualContent.trim(),
expectedContent,
"Tool should have returned the exact multiline content",
)

// Also verify the AI mentioned the correct number of lines
const hasLineCount = messages.some(
(m) =>
Expand All @@ -381,7 +355,7 @@ suite("Roo Code read_file Tool", function () {
const messages: ClineMessage[] = []
let taskCompleted = false
let toolExecuted = false
let toolResult: string | null = null
let _toolResult: string | null = null

// Listen for messages
const messageHandler = ({ message }: { message: ClineMessage }) => {
Expand All @@ -408,7 +382,7 @@ suite("Roo Code read_file Tool", function () {
resultMatch = requestData.request.match(/Result:\s*\n([\s\S]+?)(?:\n\n|$)/)
}
if (resultMatch) {
toolResult = resultMatch[1]
_toolResult = resultMatch[1]
console.log("Extracted line range tool result")
} else {
console.log("Could not extract tool result from request")
Expand Down Expand Up @@ -455,23 +429,6 @@ suite("Roo Code read_file Tool", function () {
// Verify tool was executed
assert.ok(toolExecuted, "The read_file tool should have been executed")

// Verify the tool returned the correct lines (when line range is used)
if (toolResult && (toolResult as string).includes(" | ")) {
// The result includes line numbers
assert.ok(
(toolResult as string).includes("2 | Line 2"),
"Tool result should include line 2 with line number",
)
assert.ok(
(toolResult as string).includes("3 | Line 3"),
"Tool result should include line 3 with line number",
)
assert.ok(
(toolResult as string).includes("4 | Line 4"),
"Tool result should include line 4 with line number",
)
}

// Also verify the AI mentioned the specific lines
const hasLines = messages.some(
(m) =>
Expand Down
100 changes: 92 additions & 8 deletions src/core/task/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,11 +603,55 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {

// Cline Messages

// Redact file payloads from UI-persisted messages (ui_messages.json)
// while leaving full content intact for apiConversationHistory.
private sanitizeMessageText(text?: string): string | undefined {
if (!text) return text

const scrub = (s: string): string => {
// Replace inner contents of known file payload tags with an omission marker
// Order matters: scrub more specific tags first.
s = s.replace(/<file_content\b[\s\S]*?<\/file_content>/gi, "<file_content>[omitted]</file_content>")
s = s.replace(/<content\b[^>]*>[\s\S]*?<\/content>/gi, "<content>[omitted]</content>")
s = s.replace(/<file\b[^>]*>[\s\S]*?<\/file>/gi, "<file>[omitted]</file>")
s = s.replace(/<files\b[^>]*>[\s\S]*?<\/files>/gi, "<files>[omitted]</files>")
return s
}

// If this is a JSON payload (e.g. api_req_started), try to sanitize the 'request' field.
try {
const obj = JSON.parse(text)
if (obj && typeof obj === "object" && typeof obj.request === "string") {
obj.request = scrub(obj.request)
return JSON.stringify(obj)
}
} catch {
// Not JSON, fall-through to raw scrub
}

return scrub(text)
}

// Sanitize an array of messages for persistence to UI storage
private sanitizeMessagesArray(messages: ClineMessage[]): ClineMessage[] {
return messages.map((m) => {
if (typeof (m as any).text === "string") {
return { ...m, text: this.sanitizeMessageText((m as any).text) }
}
return m
})
}

private async getSavedClineMessages(): Promise<ClineMessage[]> {
return readTaskMessages({ taskId: this.taskId, globalStoragePath: this.globalStoragePath })
const msgs = await readTaskMessages({ taskId: this.taskId, globalStoragePath: this.globalStoragePath })
return this.sanitizeMessagesArray(msgs)
}

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

public async overwriteClineMessages(newMessages: ClineMessage[]) {
this.clineMessages = newMessages
this.clineMessages = this.sanitizeMessagesArray(newMessages)

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

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

private async saveClineMessages() {
try {
// Sanitize just before persisting to ensure any direct mutations are scrubbed
const sanitizedMessages = this.sanitizeMessagesArray(this.clineMessages)

await saveTaskMessages({
messages: this.clineMessages,
messages: sanitizedMessages,
taskId: this.taskId,
globalStoragePath: this.globalStoragePath,
})
Expand All @@ -670,7 +721,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
rootTaskId: this.rootTaskId,
parentTaskId: this.parentTaskId,
taskNumber: this.taskNumber,
messages: this.clineMessages,
messages: sanitizedMessages,
globalStoragePath: this.globalStoragePath,
workspace: this.cwd,
mode: this._taskMode || defaultModeSlug, // Use the task's own mode, not the current provider mode.
Expand Down Expand Up @@ -1790,12 +1841,45 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
const modelId = getModelId(this.apiConfiguration)
const apiProtocol = getApiProtocol(this.apiConfiguration.apiProvider, modelId)

// Redact any read_file results or file payload blocks from UI messages.
// This prevents file contents from being persisted to ui_messages.json while
// still sending full content to the LLM via apiConversationHistory.
const formatRequestWithReadFileRedaction = (blocks: Anthropic.Messages.ContentBlockParam[]) => {
let redactNext = false
const parts = blocks.map((block: any) => {
if (block?.type === "text") {
const text = String(block.text ?? "")

// 1) Detect the explicit read_file header line emitted by pushToolResult
const isReadFileHeader = /^\[read_file\b[\s\S]*\]\s*Result:/i.test(text)

// 2) Detect any XML-like file payloads that tools may include
// Examples: <files>...</files>, <file>...</file>, <content ...>...</content>, <file_content ...>...</file_content>
const looksLikeFilePayload = /<files[\s>]|<file[\s>]|<content\b|<file_content\b/i.test(text)

// If we see the header, show the header but redact the next text block (payload)
if (isReadFileHeader) {
redactNext = true
return text
}

// If the previous block was a read_file header, or this block itself looks like a file payload, redact it
if (redactNext || looksLikeFilePayload) {
redactNext = false
return "[tool output omitted from UI storage]"
}
}

// Default formatting for other blocks
return formatContentBlockToMarkdown(block as any)
})
return parts.join("\n\n")
}
Comment on lines +1847 to +1877
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-redacted text blocks (including the read_file header) bypass formatContentBlockToMarkdown and return raw text, which diverges from prior behavior and from how non-text blocks are formatted. To keep formatting consistent and avoid UI regressions, return formatContentBlockToMarkdown(block) for the non-redacted text path (and for the header), e.g., replace return text with return formatContentBlockToMarkdown(block as any).

Copilot uses AI. Check for mistakes.


await this.say(
"api_req_started",
JSON.stringify({
request:
currentUserContent.map((block) => formatContentBlockToMarkdown(block)).join("\n\n") +
"\n\nLoading...",
request: formatRequestWithReadFileRedaction(currentUserContent) + "\n\nLoading...",
apiProtocol,
}),
)
Expand Down Expand Up @@ -1835,7 +1919,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
const lastApiReqIndex = findLastIndex(this.clineMessages, (m) => m.say === "api_req_started")

this.clineMessages[lastApiReqIndex].text = JSON.stringify({
request: finalUserContent.map((block) => formatContentBlockToMarkdown(block)).join("\n\n"),
request: formatRequestWithReadFileRedaction(finalUserContent),
apiProtocol,
} satisfies ClineApiReqInfo)

Expand Down