diff --git a/.changeset/patch-fix-safe-output-inline-backticks.md b/.changeset/patch-fix-safe-output-inline-backticks.md new file mode 100644 index 00000000000..d5855697171 --- /dev/null +++ b/.changeset/patch-fix-safe-output-inline-backticks.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Prevent attacker-controlled inline backticks from reactivating mentions, GitHub references, bot triggers, and commands neutralized in safe outputs diff --git a/actions/setup/js/markdown_code_region_balancer.cjs b/actions/setup/js/markdown_code_region_balancer.cjs index e9414442f4f..6e743cfbb54 100644 --- a/actions/setup/js/markdown_code_region_balancer.cjs +++ b/actions/setup/js/markdown_code_region_balancer.cjs @@ -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]; diff --git a/actions/setup/js/markdown_code_region_balancer.test.cjs b/actions/setup/js/markdown_code_region_balancer.test.cjs index 987a5e11487..8cd1cc62a69 100644 --- a/actions/setup/js/markdown_code_region_balancer.test.cjs +++ b/actions/setup/js/markdown_code_region_balancer.test.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 diff --git a/actions/setup/js/sanitize_content.cjs b/actions/setup/js/sanitize_content.cjs index bb291c04a1a..62bc5f359d6 100644 --- a/actions/setup/js/sanitize_content.cjs +++ b/actions/setup/js/sanitize_content.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)}`; + }); }); } } diff --git a/actions/setup/js/sanitize_content.test.cjs b/actions/setup/js/sanitize_content.test.cjs index a4fc629ca92..19664d207a3 100644 --- a/actions/setup/js/sanitize_content.test.cjs +++ b/actions/setup/js/sanitize_content.test.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``"); }); }); diff --git a/actions/setup/js/sanitize_content_core.cjs b/actions/setup/js/sanitize_content_core.cjs index 94481a132ce..938593acf6a 100644 --- a/actions/setup/js/sanitize_content_core.cjs +++ b/actions/setup/js/sanitize_content_core.cjs @@ -548,7 +548,34 @@ function sanitizeUrlDomains(s, allowed) { } /** - * Neutralizes commands at the start of text by wrapping them in backticks. + * Creates a code-span wrapper whose delimiter length does not occur in the input. + * This prevents surrounding attacker-controlled backticks from pairing with the + * sanitizer's delimiters and exposing the wrapped token in rendered Markdown. + * @param {string} s - The complete string being processed + * @returns {(text: string, before?: string, after?: string) => string} A render-safe inline-code wrapper + */ +function createRenderSafeCodeSpanWrapper(s) { + const usedDelimiterLengths = new Set(Array.from(s.matchAll(/`+/g), match => match[0].length)); + let delimiterLength = 1; + while (usedDelimiterLengths.has(delimiterLength)) { + delimiterLength++; + } + // Do not let an attacker inflate every replacement by supplying progressively + // longer backtick runs. HTML code elements provide the same inert rendering + // without a delimiter whose length depends on the input. + if (delimiterLength > 16) { + return text => `${text.replace(/[&<>"']/g, character => ({ "&": "&", "<": "<", ">": ">", '"': """, "'": "'" })[character])}`; + } + const delimiter = "`".repeat(delimiterLength); + return (text, before = "", after = "") => { + const leadingSeparator = before.endsWith("`") ? " " : ""; + const trailingSeparator = after.startsWith("`") ? " " : ""; + return `${leadingSeparator}${delimiter}${text}${delimiter}${trailingSeparator}`; + }; +} + +/** + * Neutralizes commands at the start of text by wrapping them in a render-safe code span. * Reads all command names from GH_AW_COMMANDS (JSON array). * @param {string} s - The string to process * @returns {string} The string with neutralized commands @@ -575,10 +602,11 @@ function neutralizeCommands(s) { const leadingWhitespace = s.match(/^\s*/)?.[0] ?? ""; const remainder = s.slice(leadingWhitespace.length); + const wrapInCodeSpan = createRenderSafeCodeSpanWrapper(s); const matchedCommand = resolveMatchedCommand(remainder, commandNames); if (matchedCommand) { const escapedCommand = matchedCommand.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - return s.replace(new RegExp(`^(\\s*)/(${escapedCommand})\\b`, "i"), "$1`/$2`"); + return s.replace(new RegExp(`^(\\s*)/(${escapedCommand})\\b`, "i"), (match, whitespace, command, offset, input) => `${whitespace}${wrapInCodeSpan(`/${command}`, "", input[offset + match.length])}`); } for (const name of commandNames) { @@ -586,7 +614,7 @@ function neutralizeCommands(s) { continue; } const escapedCommand = name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const result = s.replace(new RegExp(`^(\\s*)/(${escapedCommand})\\b`, "i"), "$1`/$2`"); + const result = s.replace(new RegExp(`^(\\s*)/(${escapedCommand})\\b`, "i"), (match, whitespace, command, offset, input) => `${whitespace}${wrapInCodeSpan(`/${command}`, "", input[offset + match.length])}`); if (result !== s) { return result; } @@ -596,20 +624,22 @@ function neutralizeCommands(s) { } /** - * Neutralizes ALL @mentions by wrapping them in backticks + * Neutralizes ALL @mentions by wrapping them in render-safe code spans. * This is the core version without any filtering * @param {string} s - The string to process * @returns {string} The string with neutralized mentions */ function neutralizeAllMentions(s) { - // Replace @name or @org/team outside code with `@name` - // No filtering - all mentions are neutralized - // Changed [^\w`] to [^A-Za-z0-9`] to include underscore as a valid preceding character - // This prevents bypass patterns like "test_@user" from escaping sanitization - return s.replace(/(^|[^A-Za-z0-9`])@([A-Za-z0-9](?:[A-Za-z0-9_-]{0,37}[A-Za-z0-9])?(?:\/[A-Za-z0-9._-]+)?)/g, (m, p1, p2) => { - // Log when a mention is escaped to help debug issues - core.info(`Escaped mention: @${p2} (not in allowed list)`); - return `${p1}\`@${p2}\``; + const wrapInCodeSpan = createRenderSafeCodeSpanWrapper(s); + return applyToNonCodeRegions(s, (segment, regionBefore = "", regionAfter = "") => { + // No filtering - all mentions outside matched code regions are neutralized. + // Use an explicit ASCII class so underscores before mentions remain covered. + 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) => { + 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)}`; + }); }); } @@ -641,8 +671,11 @@ function getFencedCodeRanges(s) { const lineEnd = i < lines.length - 1 ? lineContentEnd + 1 : lineContentEnd; if (!inBlock) { - const m = trimmed.match(/^(`{3,}|~{3,})/); - if (m) { + // CommonMark permits at most three leading spaces. Backtick fence info + // strings cannot contain backticks; such lines may instead contain an + // inline code span followed by prose that still requires sanitization. + const m = line.match(/^ {0,3}(`{3,}|~{3,})(.*)$/); + if (m && (m[1][0] !== "`" || !m[2].includes("`"))) { inBlock = true; blockStart = pos; fenceChar = m[1][0]; @@ -651,8 +684,8 @@ function getFencedCodeRanges(s) { } else { // A closing fence: same character, at least as long, only whitespace after const fc = fenceChar === "`" ? "\\`" : "~"; - const closingRegex = new RegExp(`^[${fc}]{${fenceLen},}\\s*$`); - if (closingRegex.test(trimmed)) { + const closingRegex = new RegExp(`^ {0,3}[${fc}]{${fenceLen},}\\s*$`); + if (closingRegex.test(line)) { ranges.push([blockStart, lineEnd]); inBlock = false; blockStart = -1; @@ -678,11 +711,13 @@ function getFencedCodeRanges(s) { * non-code text; inline code spans are preserved verbatim. * * @param {string} text - The text to process (should not contain fenced code blocks) - * @param {(s: string) => string} fn - Transformation to apply to non-code portions + * @param {(s: string, before?: string, after?: string) => string} fn - Transformation to apply to non-code portions + * @param {string} [outerBefore] - Character immediately before this text segment + * @param {string} [outerAfter] - Character immediately after this text segment * @returns {string} The processed text */ -function applyFnOutsideInlineCode(text, fn) { - if (!text) return fn(text || ""); +function applyFnOutsideInlineCode(text, fn, outerBefore = "", outerAfter = "") { + if (!text) return fn(text || "", outerBefore, outerAfter); const parts = []; let i = 0; @@ -727,7 +762,8 @@ function applyFnOutsideInlineCode(text, fn) { if (closeIdx !== -1) { // Valid inline code span found: apply fn to the text before it, then keep the code span if (textStart < btStart) { - parts.push(fn(text.slice(textStart, btStart))); + const before = textStart > 0 ? text[textStart - 1] : outerBefore; + parts.push(fn(text.slice(textStart, btStart), before, text[btStart])); } parts.push(text.slice(btStart, closeIdx + btCount)); textStart = closeIdx + btCount; @@ -738,7 +774,8 @@ function applyFnOutsideInlineCode(text, fn) { // Apply fn to any remaining non-code text if (textStart < text.length) { - parts.push(fn(text.slice(textStart))); + const before = textStart > 0 ? text[textStart - 1] : outerBefore; + parts.push(fn(text.slice(textStart), before, outerAfter)); } return parts.join(""); @@ -752,7 +789,7 @@ function applyFnOutsideInlineCode(text, fn) { * Falls back to applying fn to the entire string if any parsing error occurs. * * @param {string} s - Markdown content to process - * @param {(s: string) => string} fn - Transformation to apply outside code regions + * @param {(s: string, before?: string, after?: string) => string} fn - Transformation to apply outside code regions * @returns {string} The content with the transformation applied only outside code regions */ function applyToNonCodeRegions(s, fn) { @@ -774,7 +811,7 @@ function applyToNonCodeRegions(s, fn) { for (const [start, end] of codeRanges) { if (pos < start) { // Non-code text before this code block: protect inline code spans - parts.push(applyFnOutsideInlineCode(s.slice(pos, start), fn)); + parts.push(applyFnOutsideInlineCode(s.slice(pos, start), fn, pos > 0 ? s[pos - 1] : "", s[start])); } // Fenced code block: preserve verbatim parts.push(s.slice(start, end)); @@ -783,13 +820,13 @@ function applyToNonCodeRegions(s, fn) { // Non-code text after the last code block if (pos < s.length) { - parts.push(applyFnOutsideInlineCode(s.slice(pos), fn)); + parts.push(applyFnOutsideInlineCode(s.slice(pos), fn, pos > 0 ? s[pos - 1] : "", "")); } return parts.join(""); } catch (_e) { // Fallback: apply fn to the entire string (conservative – redacts more, never less) - return fn(s); + return fn(s, "", ""); } } @@ -1021,7 +1058,7 @@ function convertXmlTags(s) { const MAX_BOT_TRIGGER_REFERENCES = 10; /** - * Neutralizes bot trigger phrases by wrapping them in backticks. + * Neutralizes bot trigger phrases by wrapping them in render-safe code spans. * The first `maxBotMentions` unquoted trigger references are left unchanged; * any occurrences beyond that threshold are wrapped in backticks. * Already-quoted entries are never re-quoted. @@ -1030,20 +1067,19 @@ const MAX_BOT_TRIGGER_REFERENCES = 10; * @returns {string} The string with excess bot triggers neutralized */ function neutralizeBotTriggers(s, maxBotMentions = MAX_BOT_TRIGGER_REFERENCES) { - // Match unquoted bot trigger phrases like "fixes #123", "closes #asdfs", etc. - // The negative lookbehind (? { - count++; - if (count <= maxBotMentions) { - return match; - } - return `\`${action} #${ref}\``; + const wrapInCodeSpan = createRenderSafeCodeSpanWrapper(s); + return applyToNonCodeRegions(s, (segment, regionBefore = "", regionAfter = "") => { + return segment.replace(pattern, (match, action, ref, offset) => { + count++; + if (count <= maxBotMentions) { + return match; + } + const before = segment[offset - 1] || (offset === 0 ? regionBefore : ""); + const after = segment[offset + match.length] || (offset + match.length === segment.length ? regionAfter : ""); + return wrapInCodeSpan(`${action} #${ref}`, before, after); + }); }); } @@ -1186,7 +1222,8 @@ function getCurrentRepoSlug() { } /** - * Neutralizes GitHub references (#123 or owner/repo#456) by wrapping them in backticks + * Neutralizes GitHub references (#123 or owner/repo#456) by wrapping them in + * render-safe code spans * if they reference repositories not in the allowed list. * Supports wildcard patterns (e.g., "myorg/*", "*") via isRepoAllowed(). * @param {string} s - The string to process @@ -1200,38 +1237,35 @@ function neutralizeGitHubReferences(s, allowedRepos) { } const currentRepo = getCurrentRepoSlug(); + const wrapInCodeSpan = createRenderSafeCodeSpanWrapper(s); // Expand the special "repo" keyword to the current repo slug and build a Set for isRepoAllowed() const allowedSet = new Set(allowedRepos.map(r => (r === "repo" ? currentRepo : r))); - // Match GitHub references: - // - #123 (current repo reference) - // - owner/repo#456 (cross-repo reference) - // - GH-123 (GitHub shorthand) - // Must not be inside backticks or code blocks - return s.replace(/(^|[^\w`])(?:([a-z0-9](?:[a-z0-9-]{0,38}[a-z0-9])?)\/([a-z0-9._-]+))?#(\w+)/gi, (match, prefix, owner, repo, issueNum) => { - let targetRepo; - - if (owner && repo) { - // Cross-repo reference: owner/repo#123 - targetRepo = `${owner}/${repo}`.toLowerCase(); - } else { - // Current repo reference: #123 - targetRepo = currentRepo; - } + return applyToNonCodeRegions(s, (segment, regionBefore = "", regionAfter = "") => { + // Match #123 and owner/repo#456 outside matched code regions. + return segment.replace(/(^|[^\w])(?:([a-z0-9](?:[a-z0-9-]{0,38}[a-z0-9])?)\/([a-z0-9._-]+))?#(\w+)/gi, (match, prefix, owner, repo, issueNum, offset) => { + let targetRepo; - // Check if this repo is allowed using isRepoAllowed (supports wildcard patterns) - if (isRepoAllowed(targetRepo, allowedSet)) { - return match; // Keep the original reference - } else { - // Escape the reference - const refText = owner && repo ? `${owner}/${repo}#${issueNum}` : `#${issueNum}`; + if (owner && repo) { + // Cross-repo reference: owner/repo#123 + targetRepo = `${owner}/${repo}`.toLowerCase(); + } else { + // Current repo reference: #123 + targetRepo = currentRepo; + } - // Log when a reference is escaped - core.info(`Escaped GitHub reference: ${refText} (not in allowed list)`); + // Check if this repo is allowed using isRepoAllowed (supports wildcard patterns) + if (isRepoAllowed(targetRepo, allowedSet)) { + return match; // Keep the original reference + } - return `${prefix}\`${refText}\``; - } + const refText = owner && repo ? `${owner}/${repo}#${issueNum}` : `#${issueNum}`; + core.info(`Escaped GitHub reference: ${refText} (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(refText, before, after)}`; + }); }); } @@ -1533,9 +1567,6 @@ function sanitizeContentCore(content, maxLength, maxBotMentions) { // Must run before mention neutralization for the same ordering reason as removeXmlComments. sanitized = applyToNonCodeRegions(sanitized, neutralizeMarkdownLinkTitles); - // Neutralize ALL @mentions (no filtering in core version) - sanitized = neutralizeAllMentions(sanitized); - // Convert XML tags to parentheses format – skip code blocks and inline code so that // type parameters (e.g. VBuffer) and code containing angle brackets are preserved sanitized = applyToNonCodeRegions(sanitized, convertXmlTags); @@ -1546,6 +1577,10 @@ function sanitizeContentCore(content, maxLength, maxBotMentions) { // Apply truncation limits sanitized = applyTruncation(sanitized, maxLength); + // Neutralize ALL @mentions after truncation so a length boundary cannot split + // an inserted code-span delimiter and reactivate the mention. + sanitized = neutralizeAllMentions(sanitized); + // Neutralize GitHub references if restrictions are configured sanitized = neutralizeGitHubReferences(sanitized, allowedGitHubRefs); @@ -1601,6 +1636,8 @@ module.exports = { sanitizeUrlProtocols, sanitizeUrlDomains, applyURLSanitizationPolicy, + createRenderSafeCodeSpanWrapper, + getFencedCodeRanges, neutralizeCommands, neutralizeGitHubReferences, removeXmlComments, diff --git a/actions/setup/js/sanitize_content_core_parser.test.cjs b/actions/setup/js/sanitize_content_core_parser.test.cjs index d5b99e64bbc..1071a72a02c 100644 --- a/actions/setup/js/sanitize_content_core_parser.test.cjs +++ b/actions/setup/js/sanitize_content_core_parser.test.cjs @@ -15,6 +15,7 @@ describe("sanitize_content_core.cjs – parser internals", () => { let getFencedCodeRanges; let applyFnOutsideInlineCode; let applyToNonCodeRegions; + let createRenderSafeCodeSpanWrapper; beforeEach(async () => { // Set up a minimal stub so code that calls core.* doesn't throw. @@ -34,12 +35,21 @@ describe("sanitize_content_core.cjs – parser internals", () => { getFencedCodeRanges = mod.getFencedCodeRanges ?? null; applyFnOutsideInlineCode = mod.applyFnOutsideInlineCode ?? null; applyToNonCodeRegions = mod.applyToNonCodeRegions; + createRenderSafeCodeSpanWrapper = mod.createRenderSafeCodeSpanWrapper; }); afterEach(() => { delete global.core; }); + it("uses bounded HTML code elements when all short delimiters are present", () => { + const input = Array.from({ length: 17 }, (_, index) => "`".repeat(index + 1)).join(" "); + const wrap = createRenderSafeCodeSpanWrapper(input); + const result = wrap("@user"); + expect(result).toBe("@user"); + expect(result.length).toBeLessThan(32); + }); + // --------------------------------------------------------------------------- // getFencedCodeRanges – only if exported directly // --------------------------------------------------------------------------- @@ -130,6 +140,11 @@ describe("sanitize_content_core.cjs – parser internals", () => { expect(result).toContain("content"); // preserved, not uppercased }); + it("does not treat a four-space-indented fence as a code block", () => { + const input = " ```\n@user\n ```"; + expect(getFencedCodeRanges(input)).toEqual([]); + }); + it("backtick fence is not closed by tilde fence", () => { const input = "a\n```\ncontent\n~~~\nstill in backtick block\n```\nb"; const result = applyToNonCodeRegions(input, s => s.toUpperCase());