Fix diff escaping issues (#2694)

* Fix diff escaping issues

* Potential fix for code scanning alert no. 75: Double escaping or unescaping

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Author: mrubens

src/core/tools/applyDiffTool.ts

@@ -11,7 +11,7 @@ import path from "path"
 import fs from "fs/promises"
 import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
 import { telemetryService } from "../../services/telemetry/TelemetryService"
-
+import { unescapeHtmlEntities } from "../../utils/text-normalization"
 export async function applyDiffTool(
 	cline: Cline,
 	block: ToolUse,
@@ -21,7 +21,11 @@ export async function applyDiffTool(
 	removeClosingTag: RemoveClosingTag,
 ) {
 	const relPath: string | undefined = block.params.path
-	const diffContent: string | undefined = block.params.diff
+	let diffContent: string | undefined = block.params.diff
+
+	if (diffContent && !cline.api.getModel().id.includes("claude")) {
+		diffContent = unescapeHtmlEntities(diffContent)
+	}
 
 	const sharedMessageProps: ClineSayTool = {
 		tool: "appliedDiff",

src/core/tools/executeCommandTool.ts

@@ -2,6 +2,7 @@ import { Cline } from "../Cline"
 import { ToolUse } from "../assistant-message"
 import { AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "./types"
 import { formatResponse } from "../prompts/responses"
+import { unescapeHtmlEntities } from "../../utils/text-normalization"
 
 export async function executeCommandTool(
 	cline: Cline,
@@ -32,8 +33,8 @@ export async function executeCommandTool(
 				return
 			}
 
-			// unescape html entities (e.g. &lt; -> <)
-			command = command.replace(/&lt;/g, "<").replace(/&gt;/g, ">").replace(/&amp;/g, "&")
+			// Unescape HTML entities
+			command = unescapeHtmlEntities(command)
 
 			cline.consecutiveMistakeCount = 0
 

src/core/tools/writeToFileTool.ts

@@ -14,6 +14,7 @@ import { isPathOutsideWorkspace } from "../../utils/pathUtils"
 import { everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
 import delay from "delay"
 import { detectCodeOmission } from "../../integrations/editor/detect-omission"
+import { unescapeHtmlEntities } from "../../utils/text-normalization"
 
 export async function writeToFileTool(
 	cline: Cline,
@@ -60,13 +61,7 @@ export async function writeToFileTool(
 	}
 
 	if (!cline.api.getModel().id.includes("claude")) {
-		// it seems not just llama models are doing cline, but also gemini and potentially others
-		if (newContent.includes(">") || newContent.includes("<") || newContent.includes(""")) {
-			newContent = newContent
-				.replace(/>/g, ">")
-				.replace(/&lt;/g, "<")
-				.replace(/&quot;/g, '"')
-		}
+		newContent = unescapeHtmlEntities(newContent)
 	}
 
 	// Determine if the path is outside the workspace

src/utils/__tests__/text-normalization.test.ts

@@ -1,4 +1,4 @@
-import { normalizeString } from "../text-normalization"
+import { normalizeString, unescapeHtmlEntities } from "../text-normalization"
 
 describe("Text normalization utilities", () => {
 	describe("normalizeString", () => {
@@ -30,4 +30,50 @@ describe("Text normalization utilities", () => {
 			expect(normalizeString(input)).toBe('Let\'s test this-with some "fancy" punctuation... and spaces')
 		})
 	})
+
+	describe("unescapeHtmlEntities", () => {
+		test("unescapes basic HTML entities", () => {
+			expect(unescapeHtmlEntities("&lt;div&gt;Hello&lt;/div&gt;")).toBe("<div>Hello</div>")
+		})
+
+		test("unescapes ampersand entity", () => {
+			expect(unescapeHtmlEntities("This &amp; that")).toBe("This & that")
+		})
+
+		test("unescapes quote entities", () => {
+			expect(unescapeHtmlEntities("&quot;quoted&quot; and &#39;single-quoted&#39;")).toBe(
+				"\"quoted\" and 'single-quoted'",
+			)
+		})
+
+		test("unescapes apostrophe entity", () => {
+			expect(unescapeHtmlEntities("Don&apos;t worry")).toBe("Don't worry")
+		})
+
+		test("handles mixed content with multiple entity types", () => {
+			expect(
+				unescapeHtmlEntities(
+					"&lt;a href=&quot;https://example.com?param1=value&amp;param2=value&quot;&gt;Link&lt;/a&gt;",
+				),
+			).toBe('<a href="https://example.com?param1=value&param2=value">Link</a>')
+		})
+
+		test("handles mixed content with apostrophe entities", () => {
+			expect(
+				unescapeHtmlEntities(
+					"&lt;div&gt;Don&apos;t forget that Tom&amp;Jerry&apos;s show is at 3 o&apos;clock&lt;/div&gt;",
+				),
+			).toBe("<div>Don't forget that Tom&Jerry's show is at 3 o'clock</div>")
+		})
+
+		test("returns original string when no entities are present", () => {
+			const original = "Plain text without entities"
+			expect(unescapeHtmlEntities(original)).toBe(original)
+		})
+
+		test("handles empty or undefined input", () => {
+			expect(unescapeHtmlEntities("")).toBe("")
+			expect(unescapeHtmlEntities(undefined as unknown as string)).toBe(undefined)
+		})
+	})
 })

src/utils/text-normalization.ts

@@ -75,3 +75,21 @@ export function normalizeString(str: string, options: NormalizeOptions = DEFAULT
 
 	return normalized
 }
+
+/**
+ * Unescapes common HTML entities in a string
+ *
+ * @param text The string containing HTML entities to unescape
+ * @returns The unescaped string with HTML entities converted to their literal characters
+ */
+export function unescapeHtmlEntities(text: string): string {
+	if (!text) return text
+
+	return text
+		.replace(/&lt;/g, "<")
+		.replace(/&gt;/g, ">")
+		.replace(/&quot;/g, '"')
+		.replace(/&#39;/g, "'")
+		.replace(/&apos;/g, "'")
+		.replace(/&amp;/g, "&")
+}