Skip to content

fix: preserve template delimiters inside fenced code blocks in safe-output sanitizer #28736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 8 commits into from
Apr 27, 2026
+729 −78
Merged

fix: preserve template delimiters inside fenced code blocks in safe-output sanitizer #28736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5059ec3
Initial plan
Copilot Apr 27, 2026
0d9457a
fix: preserve template delimiters inside fenced code blocks in safe-o…
Copilot Apr 27, 2026
fe01828
fix: remove typeof core checks from neutralizeTemplateDelimiters
Copilot Apr 27, 2026
f9cf5e1
test: add extensive unit + fuzz tests for parser (getFencedCodeRanges…
Copilot Apr 27, 2026
f8a9047
fix: address all PR review comments on parser tests and core guards
Copilot Apr 27, 2026
fd30d3e
fix: remove all typeof core guards from sanitize_content_core.cjs
Copilot Apr 27, 2026
b8ae0f0
fix: add global.core setup to sanitize_title.test.cjs
Copilot Apr 27, 2026
d371126
Merge branch 'main' into copilot/fix-safe-output-sanitizer
pelikhan Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions actions/setup/js/sanitize_content.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2416,11 +2416,37 @@ describe("sanitize_content.cjs", () => {
expect(mockCore.warning).not.toHaveBeenCalledWith(expect.stringContaining("Template-like syntax detected"));
});

it("should escape template delimiters in code blocks", () => {
// Template delimiters should still be escaped even in code blocks
// This is defense-in-depth - we escape everywhere
it("should preserve template delimiters inside inline code spans", () => {
// Template delimiters inside inline code spans must NOT be escaped –
// code content is reproduced verbatim.
const result = sanitizeContent("`code with {{ var }}`");
expect(result).toBe("`code with \\{\\{ var }}`");
expect(result).toBe("`code with {{ var }}`");
});

it("should preserve template delimiters inside fenced code blocks", () => {
// Template delimiters inside fenced code blocks must NOT be escaped.
const input = "Text before\n```\n{{ template_var }}\n```\nText after";
const result = sanitizeContent(input);
expect(result).toContain("{{ template_var }}");
expect(result).not.toContain("\\{\\{");
});

it("should preserve template delimiters inside GitHub suggestion blocks", () => {
// Suggestion blocks are fenced code blocks – their content is applied literally
// as a patch, so template delimiters must not be escaped.
const input = "Review comment\n```suggestion\nRefer to [Advanced {{fleet-server}} options](/ref.md).\n```";
const result = sanitizeContent(input);
expect(result).toContain("{{fleet-server}}");
expect(result).not.toContain("\\{\\{");
});

it("should still escape template delimiters outside code blocks", () => {
// Template delimiters in regular prose must still be escaped.
const input = "Outside: {{ var }}\n```\nInside: {{ safe }}\n```\nAlso outside: {{ other }}";
const result = sanitizeContent(input);
expect(result).toContain("\\{\\{ var }}");
expect(result).toContain("\\{\\{ other }}");
expect(result).toContain("{{ safe }}"); // inside fence – preserved
});

it("should handle real-world GitHub Actions template expressions", () => {
Expand Down
144 changes: 71 additions & 73 deletions actions/setup/js/sanitize_content_core.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,8 @@ function sanitizeUrlProtocols(s) {
const domainLower = domain.toLowerCase();
const sanitized = sanitizeDomainName(domainLower);
const truncated = domainLower.length > 12 ? domainLower.substring(0, 12) + "..." : domainLower;
if (typeof core !== "undefined" && core.info) {
core.info(`Redacted URL: ${truncated}`);
}
if (typeof core !== "undefined" && core.debug) {
core.debug(`Redacted URL (full): ${match}`);
}
core.info(`Redacted URL: ${truncated}`);
core.debug(`Redacted URL (full): ${match}`);
addRedactedDomain(domainLower);
// Return sanitized domain format
return sanitized ? `(${sanitized}/redacted)` : "(redacted)";
Expand All @@ -224,12 +220,8 @@ function sanitizeUrlProtocols(s) {
const protocol = protocolMatch[1] + ":";
// Truncate the matched URL for logging (keep first 12 chars + "...")
const truncated = match.length > 12 ? match.substring(0, 12) + "..." : match;
if (typeof core !== "undefined" && core.info) {
core.info(`Redacted URL: ${truncated}`);
}
if (typeof core !== "undefined" && core.debug) {
core.debug(`Redacted URL (full): ${match}`);
}
core.info(`Redacted URL: ${truncated}`);
core.debug(`Redacted URL (full): ${match}`);
addRedactedDomain(protocol);
}
return "(redacted)";
Expand Down Expand Up @@ -288,12 +280,8 @@ function sanitizeUrlDomains(s, allowed) {
// Redact the domain but preserve the protocol and structure for debugging
const sanitized = sanitizeDomainName(hostname);
const truncated = hostname.length > 12 ? hostname.substring(0, 12) + "..." : hostname;
if (typeof core !== "undefined" && core.info) {
core.info(`Redacted URL: ${truncated}`);
}
if (typeof core !== "undefined" && core.debug) {
core.debug(`Redacted URL (full): ${match}`);
}
core.info(`Redacted URL: ${truncated}`);
core.debug(`Redacted URL (full): ${match}`);
addRedactedDomain(hostname);
// Return sanitized domain format
return sanitized ? `(${sanitized}/redacted)` : "(redacted)";
Expand Down Expand Up @@ -356,9 +344,7 @@ function neutralizeAllMentions(s) {
// 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
if (typeof core !== "undefined" && core.info) {
core.info(`Escaped mention: @${p2} (not in allowed list)`);
}
core.info(`Escaped mention: @${p2} (not in allowed list)`);
return `${p1}\`@${p2}\``;
});
}
Expand Down Expand Up @@ -744,69 +730,87 @@ function neutralizeBotTriggers(s, maxBotMentions = MAX_BOT_TRIGGER_REFERENCES) {
* template syntax, but this prevents issues if content is later processed by
* template engines (Jinja2, Liquid, ERB, JavaScript template literals).
*
* Fenced code blocks (including GitHub suggestion blocks) and inline code spans are
* preserved verbatim so that legitimate source content inside code regions is not altered.
*
* @param {string} s - The string to process
* @returns {string} The string with escaped template delimiters
* @returns {string} The string with escaped template delimiters (outside code regions)
*/
function neutralizeTemplateDelimiters(s) {
if (!s || typeof s !== "string") {
return "";
}

let result = s;
let templatesDetected = false;
// Track which template types were detected (outside code regions) for deduped logging.
const detectedTypes = new Set();

// Escape Jinja2/Liquid double curly braces: {{ ... }}
// Replace {{ with \{\{ to prevent template evaluation
if (/\{\{/.test(result)) {
templatesDetected = true;
if (typeof core !== "undefined" && core.info) {
core.info("Template syntax detected: Jinja2/Liquid double braces {{");
/**
* Escapes template delimiters in a plain-text segment (no fenced blocks or inline code).
* @param {string} text - Plain text to escape
* @returns {string} Text with template delimiters escaped
*/
function escapeInText(text) {
let result = text;

// Escape Jinja2/Liquid double curly braces: {{ ... }}
// Replace {{ with \{\{ to prevent template evaluation
if (/\{\{/.test(result)) {
if (!detectedTypes.has("jinja2")) {
detectedTypes.add("jinja2");
core.info("Template syntax detected: Jinja2/Liquid double braces {{");
}
result = result.replace(/\{\{/g, "\\{\\{");
}
result = result.replace(/\{\{/g, "\\{\\{");
}

// Escape ERB delimiters: <%= ... %>
// Replace <%= with \<%= to prevent ERB evaluation
if (/<%=/.test(result)) {
templatesDetected = true;
if (typeof core !== "undefined" && core.info) {
core.info("Template syntax detected: ERB delimiter <%=");
// Escape ERB delimiters: <%= ... %>
Comment on lines +757 to +765
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neutralizeTemplateDelimiters now calls core.info / core.warning unconditionally. This will throw a ReferenceError if core is not present in the global scope (e.g., when importing sanitize_content_core.cjs in a plain Node context without setupGlobals()/shim.cjs). Other functions in this file still guard core access via typeof core !== "undefined", so this introduces an inconsistency and a potential runtime regression when template patterns are present. Consider restoring the typeof core !== "undefined" && core.* guards here, or consistently referencing global.core with a safe fallback/no-op logger.

Copilot uses AI. Check for mistakes.
// Replace <%= with \<%= to prevent ERB evaluation
if (/<%=/.test(result)) {
if (!detectedTypes.has("erb")) {
detectedTypes.add("erb");
core.info("Template syntax detected: ERB delimiter <%=");
}
result = result.replace(/<%=/g, "\\<%=");
}
result = result.replace(/<%=/g, "\\<%=");
}

// Escape JavaScript template literal delimiters: ${ ... }
// Replace ${ with \$\{ to prevent template literal evaluation
if (/\$\{/.test(result)) {
templatesDetected = true;
if (typeof core !== "undefined" && core.info) {
core.info("Template syntax detected: JavaScript template literal ${");
// Escape JavaScript template literal delimiters: ${ ... }
// Replace ${ with \$\{ to prevent template literal evaluation
if (/\$\{/.test(result)) {
if (!detectedTypes.has("js")) {
detectedTypes.add("js");
core.info("Template syntax detected: JavaScript template literal ${");
}
result = result.replace(/\$\{/g, "\\$\\{");
}
result = result.replace(/\$\{/g, "\\$\\{");
}

// Escape Jinja2 comment delimiters: {# ... #}
// Replace {# with \{\# to prevent Jinja2 comment evaluation
if (/\{#/.test(result)) {
templatesDetected = true;
if (typeof core !== "undefined" && core.info) {
core.info("Template syntax detected: Jinja2 comment {#");
// Escape Jinja2 comment delimiters: {# ... #}
// Replace {# with \{\# to prevent Jinja2 comment evaluation
if (/\{#/.test(result)) {
if (!detectedTypes.has("jinja2comment")) {
detectedTypes.add("jinja2comment");
core.info("Template syntax detected: Jinja2 comment {#");
}
result = result.replace(/\{#/g, "\\{\\#");
}
result = result.replace(/\{#/g, "\\{\\#");
}

// Escape Jekyll raw blocks: {% raw %} and {% endraw %}
// Replace {% with \{\% to prevent Jekyll directive evaluation
if (/\{%/.test(result)) {
templatesDetected = true;
if (typeof core !== "undefined" && core.info) {
core.info("Template syntax detected: Jekyll/Liquid directive {%");
// Escape Jekyll raw blocks: {% raw %} and {% endraw %}
// Replace {% with \{\% to prevent Jekyll directive evaluation
if (/\{%/.test(result)) {
if (!detectedTypes.has("jekyll")) {
detectedTypes.add("jekyll");
core.info("Template syntax detected: Jekyll/Liquid directive {%");
}
result = result.replace(/\{%/g, "\\{\\%");
}
result = result.replace(/\{%/g, "\\{\\%");

return result;
}

// Apply escaping only to non-code regions (skip fenced code blocks and inline code spans).
// This preserves the verbatim content of suggestion blocks and other code fences.
const result = applyToNonCodeRegions(s, escapeInText);

// Log a summary warning if any template patterns were detected
if (templatesDetected && typeof core !== "undefined" && core.warning) {
if (detectedTypes.size > 0) {
core.warning(
"Template-like syntax detected and escaped. " +
"This is a defense-in-depth measure to prevent potential template injection " +
Expand All @@ -831,19 +835,15 @@ function buildAllowedGitHubReferences() {
}

if (allowedRefsEnv === "") {
if (typeof core !== "undefined" && core.info) {
core.info("GitHub reference filtering: all references will be escaped (GH_AW_ALLOWED_GITHUB_REFS is empty)");
}
core.info("GitHub reference filtering: all references will be escaped (GH_AW_ALLOWED_GITHUB_REFS is empty)");
return []; // Empty array means escape all references
}

const refs = allowedRefsEnv
.split(",")
.map(ref => ref.trim().toLowerCase())
.filter(ref => ref);
if (typeof core !== "undefined" && core.info) {
core.info(`GitHub reference filtering: allowed repos = ${refs.join(", ")}`);
}
core.info(`GitHub reference filtering: allowed repos = ${refs.join(", ")}`);
return refs;
}

Expand Down Expand Up @@ -903,9 +903,7 @@ function neutralizeGitHubReferences(s, allowedRepos) {
const refText = owner && repo ? `${owner}/${repo}#${issueNum}` : `#${issueNum}`;

// Log when a reference is escaped
if (typeof core !== "undefined" && core.info) {
core.info(`Escaped GitHub reference: ${refText} (not in allowed list)`);
}
core.info(`Escaped GitHub reference: ${refText} (not in allowed list)`);

return `${prefix}\`${refText}\``;
}
Expand Down
Loading
Loading