fix: reorder removeXmlComments before mention neutralization to prevent HTML comment bypass (#25462)

Author: Copilot

actions/setup/js/sanitize_content.cjs

@@ -88,12 +88,15 @@ function sanitizeContent(content, maxLengthOrOptions) {
   // Neutralize commands at the start of text
   sanitized = neutralizeCommands(sanitized);
 
+  // Remove XML comments before mention neutralization to prevent bypass: if removeXmlComments
+  // ran after neutralizeMentions, a comment like <!-- @user payload --> would first become
+  // <!-- `@user` payload --> and applyFnOutsideInlineCode would split at the backtick boundary,
+  // preventing the full <!--...--> pattern from being matched.
+  sanitized = applyToNonCodeRegions(sanitized, removeXmlComments);
+
   // Neutralize @mentions with selective filtering (custom logic for allowed aliases)
   sanitized = neutralizeMentions(sanitized, allowedAliasesLowercase);
 
-  // Remove XML comments – skip code blocks and inline code
-  sanitized = applyToNonCodeRegions(sanitized, removeXmlComments);
-
   // Convert XML tags – skip code blocks and inline code
   sanitized = applyToNonCodeRegions(sanitized, convertXmlTags);
 

actions/setup/js/sanitize_content.test.cjs

@@ -289,6 +289,23 @@ describe("sanitize_content.cjs", () => {
       const result = sanitizeContent("Hello <!-- multi\nline\ncomment --> world");
       expect(result).toBe("Hello  world");
     });
+
+    it("should remove XML comments containing @mentions (regression: bypass via backtick wrapping)", () => {
+      // If removeXmlComments ran after neutralizeMentions, the @mention would be wrapped in
+      // backticks first, splitting the <!--...--> pattern and causing it to survive sanitization.
+      const result = sanitizeContent("<!-- @exploituser injected payload -->");
+      expect(result).toBe("");
+    });
+
+    it("should remove XML comments containing multiple @mentions", () => {
+      const result = sanitizeContent("<!-- @attacker1 and @attacker2 payload -->");
+      expect(result).toBe("");
+    });
+
+    it("should remove XML comments with @mentions mixed with surrounding text", () => {
+      const result = sanitizeContent("before <!-- @exploituser payload --> after");
+      expect(result).toBe("before  after");
+    });
   });
 
   describe("XML/HTML tag conversion", () => {

actions/setup/js/sanitize_content_core.cjs

@@ -1035,12 +1035,15 @@ function sanitizeContentCore(content, maxLength, maxBotMentions) {
   // Neutralize commands at the start of text (e.g., /bot-name)
   sanitized = neutralizeCommands(sanitized);
 
+  // Remove XML comments before mention neutralization to prevent bypass: if removeXmlComments
+  // ran after neutralizeAllMentions, a comment like <!-- @user payload --> would first become
+  // <!-- `@user` payload --> and applyFnOutsideInlineCode would split at the backtick boundary,
+  // preventing the full <!--...--> pattern from being matched.
+  sanitized = applyToNonCodeRegions(sanitized, removeXmlComments);
+
   // Neutralize ALL @mentions (no filtering in core version)
   sanitized = neutralizeAllMentions(sanitized);
 
-  // Remove XML comments – skip code blocks and inline code to avoid altering code content
-  sanitized = applyToNonCodeRegions(sanitized, removeXmlComments);
-
   // Convert XML tags to parentheses format – skip code blocks and inline code so that
   // type parameters (e.g. VBuffer<float32>) and code containing angle brackets are preserved
   sanitized = applyToNonCodeRegions(sanitized, convertXmlTags);