Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions .github/workflows/dependabot-campaign.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions .github/workflows/dependabot-worker.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 40 additions & 0 deletions actions/setup/js/sanitize_content.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,46 @@ describe("sanitize_content.cjs", () => {
const result = sanitizeContent('<div onclick="bad()">content</div>');
expect(result).toBe('(div onclick="bad()")content(/div)');
});

it("should strip data-* attribute with double-quoted value from allowed tag", () => {
const result = sanitizeContent('<span data-x="INJECT">text</span>');
expect(result).toBe("<span>text</span>");
});

it("should strip data-* attribute with single-quoted value from allowed tag", () => {
const result = sanitizeContent("<span data-x='INJECT'>text</span>");
expect(result).toBe("<span>text</span>");
});

it("should strip data-* attribute with unquoted value from allowed tag", () => {
const result = sanitizeContent("<span data-x=INJECT>text</span>");
expect(result).toBe("<span>text</span>");
});

it("should strip bare data-* attribute (no value) from allowed tag", () => {
const result = sanitizeContent("<details data-hidden>content</details>");
expect(result).toBe("<details>content</details>");
});

it("should strip multiple data-* attributes from a single tag", () => {
const result = sanitizeContent('<span data-a="foo" data-b="bar">text</span>');
expect(result).toBe("<span>text</span>");
});

it("should strip data-* attributes alongside other dangerous attributes", () => {
const result = sanitizeContent('<span onclick="bad()" data-x="INJECT" style="evil">text</span>');
expect(result).toBe("<span>text</span>");
});

it("should strip data-* attribute while preserving safe attributes", () => {
const result = sanitizeContent('<span title="safe" data-x="INJECT">text</span>');
expect(result).toBe('<span title="safe">text</span>');
});

it("should strip data-* attribute case-insensitively", () => {
const result = sanitizeContent('<span DATA-X="INJECT">text</span>');
expect(result).toBe("<span>text</span>");
});
});

describe("XML/HTML tag conversion: code-region awareness", () => {
Expand Down
16 changes: 11 additions & 5 deletions actions/setup/js/sanitize_content_core.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -673,10 +673,16 @@ function convertXmlTags(s) {

/**
* Strips dangerous HTML attributes from an allowed tag's content string.
* Removes on* event handler attributes (e.g. onclick, ontoggle) and style
* attributes in all quoting forms (double-quoted, single-quoted, unquoted, bare).
* Removes on* event handler attributes (e.g. onclick, ontoggle), style
* attributes, and data-* custom attributes in all quoting forms
* (double-quoted, single-quoted, unquoted, bare).
* Safe attributes such as title, class, open, lang, id, etc. are preserved.
*
* data-* attributes are stripped because they are invisible to human reviewers
* of rendered markdown (they produce no visible output) yet are delivered
* verbatim to the agent — making them a hidden injection channel equivalent
* to HTML comments or zero-width-space splits.
*
* Note: `\s+` (requiring at least one whitespace before the attribute name) is
* intentional — HTML attributes are always separated from the tag name and from
* each other by at least one whitespace character. Using `\s*` would risk false
Expand All @@ -686,13 +692,13 @@ function convertXmlTags(s) {
* @returns {string} Tag content with dangerous attributes removed
*/
function stripDangerousAttributes(tagContent) {
// Match: one-or-more whitespace-or-slash + (on* | style) + optional =value
// Match: one-or-more whitespace-or-slash + (on* | style | data-*) + optional =value
// Value forms: "...", '...', or unquoted (no whitespace / > / quote chars), or bare (no =)
// The unquoted form excludes >, whitespace, and all quote characters (', ", `) so it
// cannot consume the closing > of the tag or straddle other attribute values.
// Using [\s/]+ (instead of \s+) also strips dangerous attributes that are immediately
// preceded by a "/" with no space — e.g. the malformed <img/onerror=alert(1) src=x>.
return tagContent.replace(/[\s/]+(?:on\w+|style)(?:\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>"'`]*))?/gi, "");
return tagContent.replace(/[\s/]+(?:on\w+|style|data-[a-z0-9_-]+)(?:\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>"'`]*))?/gi, "");
}

// Convert opening tags: <tag> or <tag attr="value"> to (tag) or (tag attr="value")
Expand All @@ -705,7 +711,7 @@ function convertXmlTags(s) {
if (tagNameMatch) {
const tagName = tagNameMatch[1].toLowerCase();
if (allowedTags.includes(tagName)) {
// Strip dangerous attributes (on* event handlers and style) before preserving
// Strip dangerous attributes (on* event handlers, style, data-*) before preserving
const sanitizedContent = stripDangerousAttributes(tagContent);
return `<${sanitizedContent}>`;
}
Expand Down