Fix safe-output inline-backtick neutralization (#54103)

* Fix safe-output backtick neutralization

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b591adab-6031-4a22-ac49-0755015a5014

* Bound safe-output code-span fallback

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

* Align mention boundaries and fence regression

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot-Session: b591adab-6031-4a22-ac49-0755015a5014

Author: 4 people

.changeset/patch-fix-safe-output-inline-backticks.md

@@ -36,6 +36,23 @@
  * @module markdown_code_region_balancer
  */
 
+/**
+ * Matches a CommonMark fenced-code delimiter line.
+ * @param {string} line
+ * @returns {RegExpMatchArray | null}
+ */
+function matchFenceLine(line) {
+  const match = line.match(/^( {0,3})(`{3,}|~{3,})([^`~\s]*)?(.*)$/);
+  if (!match) {
+    return null;
+  }
+  const infoString = `${match[3] || ""}${match[4] || ""}`;
+  if (match[2][0] === "`" && infoString.includes("`")) {
+    return null;
+  }
+  return match;
+}
+
 /**
  * Balance markdown code regions by attempting to fix mismatched fences.
  *
@@ -106,7 +123,7 @@ function balanceCodeRegions(markdown) {
   for (let i = 0; i < lines.length; i++) {
     if (isInXmlComment(i)) continue;
 
-    const fenceMatch = lines[i].match(/^(\s*)(`{3,}|~{3,})([^`~\s]*)?(.*)$/);
+    const fenceMatch = matchFenceLine(lines[i]);
     if (fenceMatch) {
       fences.push({
         lineIndex: i,
@@ -340,7 +357,7 @@ function isBalanced(markdown) {
   let openingFence = null;
 
   for (const line of lines) {
-    const fenceMatch = line.match(/^(\s*)(`{3,}|~{3,})([^`~\s]*)?(.*)$/);
+    const fenceMatch = matchFenceLine(line);
 
     if (fenceMatch) {
       const fence = fenceMatch[2];
@@ -392,7 +409,7 @@ function countCodeRegions(markdown) {
   let openingFence = null;
 
   for (const line of lines) {
-    const fenceMatch = line.match(/^(\s*)(`{3,}|~{3,})([^`~\s]*)?(.*)$/);
+    const fenceMatch = matchFenceLine(line);
 
     if (fenceMatch) {
       const fence = fenceMatch[2];

actions/setup/js/markdown_code_region_balancer.cjs

@@ -29,6 +29,18 @@ More content.`;
         expect(balancer.balanceCodeRegions(input)).toBe(input);
       });
 
+      it("should not treat backticks in an info string as a fenced block", () => {
+        const input = "```x```@octocat";
+        expect(balancer.balanceCodeRegions(input)).toBe(input);
+        expect(balancer.isBalanced(input)).toBe(true);
+      });
+
+      it("should not treat a four-space-indented fence as a fenced block", () => {
+        const input = "    ```\n@octocat\n    ```";
+        expect(balancer.balanceCodeRegions(input)).toBe(input);
+        expect(balancer.isBalanced(input)).toBe(true);
+      });
+
       it("should not modify properly balanced code blocks", () => {
         const input = `# Title
 

actions/setup/js/markdown_code_region_balancer.test.cjs

@@ -14,6 +14,7 @@ const {
   buildAllowedGitHubReferences,
   getCurrentRepoSlug,
   applyURLSanitizationPolicy,
+  createRenderSafeCodeSpanWrapper,
   neutralizeCommands,
   neutralizeGitHubReferences,
   removeXmlComments,
@@ -112,9 +113,6 @@ function sanitizeContent(content, maxLengthOrOptions) {
   // removeXmlComments.
   sanitized = applyToNonCodeRegions(sanitized, neutralizeMarkdownLinkTitles);
 
-  // Neutralize @mentions with selective filtering (custom logic for allowed aliases)
-  sanitized = neutralizeMentions(sanitized, allowedAliasesLowercase);
-
   // Convert XML tags – skip code blocks and inline code
   sanitized = applyToNonCodeRegions(sanitized, convertXmlTags);
 
@@ -124,6 +122,10 @@ function sanitizeContent(content, maxLengthOrOptions) {
   // Apply truncation limits (shared with core)
   sanitized = applyTruncation(sanitized, maxLength);
 
+  // Neutralize mentions after truncation so the length boundary cannot split an
+  // inserted code-span delimiter and reactivate a mention.
+  sanitized = neutralizeMentions(sanitized, allowedAliasesLowercase);
+
   // Neutralize GitHub references if restrictions are configured
   sanitized = neutralizeGitHubReferences(sanitized, allowedGitHubRefs);
 
@@ -186,17 +188,20 @@ function sanitizeContent(content, maxLengthOrOptions) {
    * @returns {string} Processed string
    */
   function neutralizeMentions(s, allowedLowercase) {
-    return s.replace(/(^|[^\w`])@([A-Za-z0-9](?:[A-Za-z0-9_-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, (_m, p1, p2) => {
-      // Check if this mention is in the allowed aliases list (case-insensitive)
-      const isAllowed = allowedLowercase.includes(p2.toLowerCase());
-      if (isAllowed) {
-        return `${p1}@${p2}`; // Keep the original mention
-      }
-      // Log when a mention is escaped
-      if (typeof core !== "undefined" && core.info) {
-        core.info(`Escaped mention: @${p2} (not in allowed list)`);
-      }
-      return `${p1}\`@${p2}\``; // Neutralize the mention
+    const wrapInCodeSpan = createRenderSafeCodeSpanWrapper(s);
+    return applyToNonCodeRegions(s, (segment, regionBefore = "", regionAfter = "") => {
+      return segment.replace(/(^|[^A-Za-z0-9])@([A-Za-z0-9](?:[A-Za-z0-9_-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, (match, prefix, alias, offset) => {
+        const isAllowed = allowedLowercase.includes(alias.toLowerCase());
+        if (isAllowed) {
+          return `${prefix}@${alias}`;
+        }
+        if (typeof core !== "undefined" && core.info) {
+          core.info(`Escaped mention: @${alias} (not in allowed list)`);
+        }
+        const before = prefix || (offset === 0 ? regionBefore : "");
+        const after = segment[offset + match.length] || (offset + match.length === segment.length ? regionAfter : "");
+        return `${prefix}${wrapInCodeSpan(`@${alias}`, before, after)}`;
+      });
     });
   }
 }

actions/setup/js/sanitize_content.cjs

@@ -101,6 +101,11 @@ describe("sanitize_content.cjs", () => {
       const result = sanitizeContent("/smoke-copilot-sdk run tests");
       expect(result).toBe("`/smoke-copilot-sdk` run tests");
     });
+
+    it("should keep commands neutralized when the body contains attacker backticks", () => {
+      const result = sanitizeContent("/bot run ` later");
+      expect(result).toBe("``/bot`` run ` later");
+    });
   });
 
   describe("@mention neutralization", () => {
@@ -139,6 +144,42 @@ describe("sanitize_content.cjs", () => {
       expect(result).toBe("Hello `@user_name_test`");
     });
 
+    it("should use a distinct delimiter for mentions after unmatched backticks", () => {
+      expect(sanitizeContent("note ` @octocat done")).toBe("note ` ``@octocat`` done");
+    });
+
+    it("should preserve mentions already contained by matched attacker backticks", () => {
+      expect(sanitizeContent("start ` mid @octocat end ` tail")).toBe("start ` mid @octocat end ` tail");
+    });
+
+    it("should choose a delimiter length absent from the complete input", () => {
+      expect(sanitizeContent("one ` two `` @octocat done")).toBe("one ` two `` ```@octocat``` done");
+    });
+
+    it("should neutralize mentions adjacent to unmatched backticks", () => {
+      expect(sanitizeContent("`@octocat")).toBe("` ``@octocat``");
+      expect(sanitizeContent("``@octocat`")).toBe("`` ```@octocat``` `");
+      expect(sanitizeContent("@octocat`")).toBe("``@octocat`` `");
+    });
+
+    it("should preserve mentions inside matched code spans", () => {
+      expect(sanitizeContent("`@octocat`")).toBe("`@octocat`");
+    });
+
+    it("should separate neutralized mentions from adjacent matched code spans", () => {
+      expect(sanitizeContent("`x`@octocat @other")).toBe("`x` ``@octocat`` ``@other``");
+      expect(sanitizeContent("@octocat`x`")).toBe("``@octocat`` `x`");
+    });
+
+    it("should not treat inline code with trailing prose as a fenced block", () => {
+      expect(sanitizeContent("```x```@octocat")).toBe("```x``` `@octocat`");
+    });
+
+    it("should neutralize a mention after truncating its alias", () => {
+      expect(sanitizeContent("123456 @octocat", 10)).toBe("123456 `@oc`\n[Content truncated due to length]");
+      expect(sanitizeContent("123456 @octocat", { maxLength: 10, allowedAliases: ["author"] })).toBe("123456 `@oc`\n[Content truncated due to length]");
+    });
+
     it("should neutralize @mentions with underscores and hyphens", () => {
       const result = sanitizeContent("Hello @user-name_test");
       expect(result).toBe("Hello `@user-name_test`");
@@ -1755,6 +1796,21 @@ describe("sanitize_content.cjs", () => {
       // The 12th entry (11th unquoted) is wrapped
       expect(result).toContain("`fixes #12`");
     });
+
+    it("should keep excess bot triggers neutralized when preceded by attacker backticks", () => {
+      const result = sanitizeContent("prefix ` fixes #1", { maxBotMentions: 0 });
+      expect(result).toBe("prefix ` ``fixes #1``");
+    });
+
+    it("should neutralize excess bot triggers adjacent to unmatched backticks", () => {
+      expect(sanitizeContent("`fixes #1", { maxBotMentions: 0 })).toBe("` ``fixes #1``");
+      expect(sanitizeContent("fixes #1`", { maxBotMentions: 0 })).toBe("``fixes #1`` `");
+    });
+
+    it("should separate excess bot triggers from adjacent matched code spans", () => {
+      expect(sanitizeContent("`x`fixes #1", { maxBotMentions: 0 })).toBe("`x` ``fixes #1``");
+      expect(sanitizeContent("```x```fixes #1", { maxBotMentions: 0 })).toBe("```x``` `fixes #1`");
+    });
   });
 
   describe("GitHub reference neutralization", () => {
@@ -1782,6 +1838,30 @@ describe("sanitize_content.cjs", () => {
       expect(result).toBe("See issue #123 and `other/repo#456`");
     });
 
+    it("should keep restricted references neutralized when preceded by attacker backticks", () => {
+      process.env.GITHUB_REPOSITORY = "myorg/myrepo";
+      process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo";
+
+      const result = sanitizeContent("see ` other/repo#1337 now");
+      expect(result).toBe("see ` ``other/repo#1337`` now");
+    });
+
+    it("should neutralize restricted references adjacent to unmatched backticks", () => {
+      process.env.GITHUB_REPOSITORY = "myorg/myrepo";
+      process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo";
+
+      expect(sanitizeContent("`other/repo#1337")).toBe("` ``other/repo#1337``");
+      expect(sanitizeContent("other/repo#1337`")).toBe("``other/repo#1337`` `");
+    });
+
+    it("should separate restricted references from adjacent matched code spans", () => {
+      process.env.GITHUB_REPOSITORY = "myorg/myrepo";
+      process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo";
+
+      expect(sanitizeContent("`x`other/repo#1337")).toBe("`x` ``other/repo#1337``");
+      expect(sanitizeContent("```x```other/repo#1337")).toBe("```x``` `other/repo#1337`");
+    });
+
     it("should allow current repo references with 'repo' keyword", () => {
       process.env.GITHUB_REPOSITORY = "myorg/myrepo";
       process.env.GH_AW_ALLOWED_GITHUB_REFS = "repo";
@@ -2111,7 +2191,7 @@ describe("sanitize_content.cjs", () => {
 
     it("should handle nested backticks", () => {
       const result = sanitizeContent("Already `@user` and @other");
-      expect(result).toBe("Already `@user` and `@other`");
+      expect(result).toBe("Already `@user` and ``@other``");
     });
   });