fix: address markdown image review feedback

obviyus · obviyus · commit d25b493c7f79 · 2026-03-07T19:46:41.000+05:30
diff --git a/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatMarkdownPreprocessor.swift b/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatMarkdownPreprocessor.swift
@@ -40,24 +40,22 @@ enum ChatMarkdownPreprocessor {
         if matches.isEmpty { return Result(cleaned: self.normalize(withoutTimestamps), images: []) }
 
         var images: [InlineImage] = []
-        var cleaned = withoutTimestamps
+        let cleaned = NSMutableString(string: withoutTimestamps)
 
         for match in matches.reversed() {
             guard match.numberOfRanges >= 3 else { continue }
             let label = ns.substring(with: match.range(at: 1))
             let source = ns.substring(with: match.range(at: 2))
 
-            let start = cleaned.index(cleaned.startIndex, offsetBy: match.range.location)
-            let end = cleaned.index(start, offsetBy: match.range.length)
             if let inlineImage = self.inlineImage(label: label, source: source) {
                 images.append(inlineImage)
-                cleaned.replaceSubrange(start..<end, with: "")
+                cleaned.replaceCharacters(in: match.range, with: "")
             } else {
-                cleaned.replaceSubrange(start..<end, with: self.fallbackImageLabel(label))
+                cleaned.replaceCharacters(in: match.range, with: self.fallbackImageLabel(label))
             }
         }
 
-        return Result(cleaned: self.normalize(cleaned), images: images.reversed())
+        return Result(cleaned: self.normalize(cleaned as String), images: images.reversed())
     }
 
     private static func inlineImage(label: String, source: String) -> InlineImage? {
diff --git a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ChatMarkdownPreprocessorTests.swift b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ChatMarkdownPreprocessorTests.swift
@@ -42,6 +42,15 @@ struct ChatMarkdownPreprocessorTests {
         #expect(result.images.isEmpty)
     }
 
+    @Test func handlesUnicodeBeforeRemoteMarkdownImages() {
+        let markdown = "🙂![Leak](https://example.com/image.png)"
+
+        let result = ChatMarkdownPreprocessor.preprocess(markdown: markdown)
+
+        #expect(result.cleaned == "🙂Leak")
+        #expect(result.images.isEmpty)
+    }
+
     @Test func stripsInboundUntrustedContextBlocks() {
         let markdown = """
         Conversation info (untrusted metadata):
diff --git a/src/auto-reply/reply/export-html/template.js b/src/auto-reply/reply/export-html/template.js
@@ -665,6 +665,10 @@
     return div.innerHTML;
   }
 
+  function escapeHtmlAttr(text) {
+    return escapeHtml(text).replaceAll('"', "&quot;").replaceAll("'", "&#39;");
+  }
+
   // Validate image fields before interpolating data URLs.
   const SAFE_IMAGE_MIME_RE = /^image\/(png|jpeg|gif|webp|svg\+xml|bmp|tiff|avif)$/i;
   const SAFE_BASE64_RE = /^[A-Za-z0-9+/]+={0,2}$/;
@@ -1725,7 +1729,7 @@
     if (!INLINE_DATA_IMAGE_RE.test(href)) {
       return escapeHtml(label);
     }
-    return `<img src="${escapeHtml(href)}" alt="${escapeHtml(label)}">`;
+    return `<img src="${escapeHtmlAttr(href)}" alt="${escapeHtmlAttr(label)}">`;
   }
 
   // Configure marked with syntax highlighting and HTML escaping for text
diff --git a/src/auto-reply/reply/export-html/template.security.test.ts b/src/auto-reply/reply/export-html/template.security.test.ts
@@ -284,4 +284,38 @@ describe("export html security hardening", () => {
     expect(messages?.textContent).toContain("exfil");
     expect(messages?.querySelector(`img[src="${dataImage}"]`)).toBeTruthy();
   });
+
+  it("escapes markdown data-image attributes", () => {
+    const dataImage = "data:image/png;base64,AAAA";
+    const session: SessionData = {
+      header: { id: "session-5", timestamp: now() },
+      entries: [
+        {
+          id: "1",
+          parentId: null,
+          timestamp: now(),
+          type: "message",
+          message: {
+            role: "assistant",
+            content: [
+              {
+                type: "text",
+                text: `![x" onerror="alert(1)](${dataImage})`,
+              },
+            ],
+          },
+        },
+      ],
+      leafId: "1",
+      systemPrompt: "",
+      tools: [],
+    };
+
+    const { document } = renderTemplate(session);
+    const img = document.querySelector("#messages img");
+    expect(img).toBeTruthy();
+    expect(img?.getAttribute("onerror")).toBeNull();
+    expect(img?.getAttribute("alt")).toBe('x" onerror="alert(1)');
+    expect(img?.getAttribute("src")).toBe(dataImage);
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -665,6 +665,10 @@`
`665`	`665`	`return div.innerHTML;`
`666`	`666`	`}`
`667`	`667`
	`668`	`+ function escapeHtmlAttr(text) {`
	`669`	`+ return escapeHtml(text).replaceAll('"', """).replaceAll("'", "'");`
	`670`	`+ }`
	`671`	`+`
`668`	`672`	`// Validate image fields before interpolating data URLs.`
`669`	`673`	`const SAFE_IMAGE_MIME_RE = /^image\/(png\|jpeg\|gif\|webp\|svg\+xml\|bmp\|tiff\|avif)$/i;`
`670`	`674`	`const SAFE_BASE64_RE = /^[A-Za-z0-9+/]+={0,2}$/;`
`@@ -1725,7 +1729,7 @@`
`1725`	`1729`	`if (!INLINE_DATA_IMAGE_RE.test(href)) {`
`1726`	`1730`	`return escapeHtml(label);`
`1727`	`1731`	`}`
`1728`		- return `<img src="${escapeHtml(href)}" alt="${escapeHtml(label)}">`;
	`1732`	+ return `<img src="${escapeHtmlAttr(href)}" alt="${escapeHtmlAttr(label)}">`;
`1729`	`1733`	`}`
`1730`	`1734`
`1731`	`1735`	`// Configure marked with syntax highlighting and HTML escaping for text`