From 36617bd48be5f24c2d18eea12367a58fefded956 Mon Sep 17 00:00:00 2001
From: bymle <by327@cornell.edu>
Date: Fri, 12 Jun 2026 12:19:44 +0800
Subject: [PATCH] fix(sanitizer): match attribute quotes by type to avoid
 mangling content (#1371)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

stripHiddenAttributes used the pattern `["'][^"']*["']` for each quoted
attribute, which matches an opening quote of either type and stops at the
first quote of either type. When a value contained the other quote
character — e.g. an apostrophe inside a double-quoted attribute like
`title="We'll do it"` — the match terminated at the apostrophe, so the
wrong span was removed and the surrounding text was corrupted (e.g.
`<Tooltip title="We'll do it" placement="top">` became
`<Tooltipll do it" placement="top">`).

This surfaced via the github_inline_comment MCP tool: suggestion blocks
whose code lines contain quotes were mangled before posting (#1366).

Match each quoted form per quote type (`"[^"]*"` and `'[^']*'`),
mirroring stripMarkdownLinkTitles, so a value may freely contain the
other quote character. The unquoted fallback is unchanged.

Closes #1366

Co-authored-by: bymle <229636660+bymle@users.noreply.github.com>
---
 src/github/utils/sanitizer.ts | 18 +++++++++++++-----
 test/sanitizer.test.ts        | 15 +++++++++++++++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/src/github/utils/sanitizer.ts b/src/github/utils/sanitizer.ts
index 83ee096b..0cb8408a 100644
--- a/src/github/utils/sanitizer.ts
+++ b/src/github/utils/sanitizer.ts
@@ -20,15 +20,23 @@ export function stripMarkdownLinkTitles(content: string): string {
 }
 
 export function stripHiddenAttributes(content: string): string {
-  content = content.replace(/\salt\s*=\s*["'][^"']*["']/gi, "");
+  // Quoted values are matched per quote type so that a value containing the
+  // other quote character (e.g. an apostrophe inside a double-quoted value)
+  // does not terminate the match early and mangle surrounding content (#1366).
+  content = content.replace(/\salt\s*=\s*"[^"]*"/gi, "");
+  content = content.replace(/\salt\s*=\s*'[^']*'/gi, "");
   content = content.replace(/\salt\s*=\s*[^\s>]+/gi, "");
-  content = content.replace(/\stitle\s*=\s*["'][^"']*["']/gi, "");
+  content = content.replace(/\stitle\s*=\s*"[^"]*"/gi, "");
+  content = content.replace(/\stitle\s*=\s*'[^']*'/gi, "");
   content = content.replace(/\stitle\s*=\s*[^\s>]+/gi, "");
-  content = content.replace(/\saria-label\s*=\s*["'][^"']*["']/gi, "");
+  content = content.replace(/\saria-label\s*=\s*"[^"]*"/gi, "");
+  content = content.replace(/\saria-label\s*=\s*'[^']*'/gi, "");
   content = content.replace(/\saria-label\s*=\s*[^\s>]+/gi, "");
-  content = content.replace(/\sdata-[a-zA-Z0-9-]+\s*=\s*["'][^"']*["']/gi, "");
+  content = content.replace(/\sdata-[a-zA-Z0-9-]+\s*=\s*"[^"]*"/gi, "");
+  content = content.replace(/\sdata-[a-zA-Z0-9-]+\s*=\s*'[^']*'/gi, "");
   content = content.replace(/\sdata-[a-zA-Z0-9-]+\s*=\s*[^\s>]+/gi, "");
-  content = content.replace(/\splaceholder\s*=\s*["'][^"']*["']/gi, "");
+  content = content.replace(/\splaceholder\s*=\s*"[^"]*"/gi, "");
+  content = content.replace(/\splaceholder\s*=\s*'[^']*'/gi, "");
   content = content.replace(/\splaceholder\s*=\s*[^\s>]+/gi, "");
   return content;
 }
diff --git a/test/sanitizer.test.ts b/test/sanitizer.test.ts
index a89353b7..e69a3995 100644
--- a/test/sanitizer.test.ts
+++ b/test/sanitizer.test.ts
@@ -131,6 +131,21 @@ describe("stripHiddenAttributes", () => {
       ),
     ).toBe('<img src="pic.jpg" class="image">');
   });
+
+  it("should not corrupt content when an attribute value contains the other quote type", () => {
+    // Regression for #1366: an apostrophe inside a double-quoted attribute
+    // (or a double quote inside a single-quoted attribute) must not cause the
+    // closing quote to be mismatched, which previously mangled later text.
+    expect(
+      stripHiddenAttributes(`<Tooltip title="We'll do it" placement="top">`),
+    ).toBe('<Tooltip placement="top">');
+    expect(
+      stripHiddenAttributes(`<img alt="Bob's avatar" src="pic.jpg">`),
+    ).toBe('<img src="pic.jpg">');
+    expect(stripHiddenAttributes(`<div title='say "hi"'>Content</div>`)).toBe(
+      "<div>Content</div>",
+    );
+  });
 });
 
 describe("normalizeHtmlEntities", () => {