From 0eddda971402f267a11dc111cd86e8e763d9b509 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Jul 2026 08:39:31 +0000 Subject: [PATCH 1/5] Initial plan From b965f415f14dcae9dbe65deb699d788c6732ea4e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Jul 2026 08:50:16 +0000 Subject: [PATCH 2/5] fix: preserve https autolinks in safe-output sanitization Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../setup/js/collect_ndjson_output.test.cjs | 14 +++++++++ actions/setup/js/sanitize_content.test.cjs | 14 +++++++-- actions/setup/js/sanitize_content_core.cjs | 29 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/collect_ndjson_output.test.cjs b/actions/setup/js/collect_ndjson_output.test.cjs index a774b468b83..044e7fb53d7 100644 --- a/actions/setup/js/collect_ndjson_output.test.cjs +++ b/actions/setup/js/collect_ndjson_output.test.cjs @@ -195,6 +195,20 @@ describe("collect_ndjson_output.cjs", () => { const parsedOutput = JSON.parse(outputCall[1]); (expect(parsedOutput.items).toHaveLength(2), expect(parsedOutput.items[0].type).toBe("create_issue"), expect(parsedOutput.items[1].type).toBe("add_comment"), expect(parsedOutput.errors).toHaveLength(0)); }), + it("should preserve Slack mrkdwn links in custom safe-job string inputs", async () => { + const testFile = "/tmp/gh-aw/test-ndjson-output.txt"; + const slackText = "Tracking issue: "; + const ndjsonContent = JSON.stringify({ type: "post_to_slack", text: slackText }); + fs.writeFileSync(testFile, ndjsonContent); + process.env.GH_AW_SAFE_OUTPUTS = testFile; + fs.writeFileSync("/tmp/gh-aw/safeoutputs/config.json", JSON.stringify({ post_to_slack: { inputs: { text: { type: "string", required: true } } } })); + await eval(`(async () => { ${collectScript}; await main(); })()`); + const setOutputCalls = mockCore.setOutput.mock.calls, + outputCall = setOutputCalls.find(call => "output" === call[0]); + expect(outputCall).toBeDefined(); + const parsedOutput = JSON.parse(outputCall[1]); + (expect(parsedOutput.errors).toHaveLength(0), expect(parsedOutput.items).toEqual([{ type: "post_to_slack", text: slackText }])); + }), it("should reject items with unexpected output types", async () => { const testFile = "/tmp/gh-aw/test-ndjson-output.txt", ndjsonContent = '{"type": "create_issue", "title": "Test Issue", "body": "Test body"}\n{"type": "unexpected-type", "data": "some data"}'; diff --git a/actions/setup/js/sanitize_content.test.cjs b/actions/setup/js/sanitize_content.test.cjs index 09fc29a0be1..82b3d04c628 100644 --- a/actions/setup/js/sanitize_content.test.cjs +++ b/actions/setup/js/sanitize_content.test.cjs @@ -441,9 +441,9 @@ describe("sanitize_content.cjs", () => { }); it("should move title into link text for inline link with angle-bracket URL", () => { - // Note: convertXmlTags runs after neutralizeMarkdownLinkTitles and converts to (url) + // Angle-bracket HTTPS autolinks are preserved so CommonMark/Slack link syntax survives sanitization. const result = sanitizeContent('[click here]( "injected payload")'); - expect(result).toBe("[click here (injected payload)]((https://github.com/path))"); + expect(result).toBe("[click here (injected payload)]()"); }); it("should move multiple link titles into link text in the same content", () => { @@ -581,6 +581,16 @@ describe("sanitize_content.cjs", () => { expect(result).not.toContain("onerror"); }); + it("should preserve HTTPS angle-bracket autolinks", () => { + const input = "Tracking issue: "; + expect(sanitizeContent(input)).toBe(input); + }); + + it("should preserve Slack mrkdwn links on allowed HTTPS domains", () => { + const input = "Tracking issue: "; + expect(sanitizeContent(input)).toBe(input); + }); + it("should handle CDATA sections", () => { const result = sanitizeContent("alert('xss')]]>"); expect(result).toBe("(![CDATA[(script)alert('xss')(/script)]])"); diff --git a/actions/setup/js/sanitize_content_core.cjs b/actions/setup/js/sanitize_content_core.cjs index 48a5ff77cb1..8255b8289df 100644 --- a/actions/setup/js/sanitize_content_core.cjs +++ b/actions/setup/js/sanitize_content_core.cjs @@ -266,6 +266,8 @@ function sanitizeUrlProtocols(s) { * @returns {string} The string with unknown domains redacted */ function sanitizeUrlDomains(s, allowed) { + const angleBracketHttpsAutolinkRegex = /|]*)?(?:\|([^<>]*))?>/gi; + // Match HTTPS URLs with optional port and path // This regex is designed to: // 1. Match https:// URIs with explicit protocol @@ -318,6 +320,14 @@ function sanitizeUrlDomains(s, allowed) { } } + // First pass: handle angle-bracket autolinks and Slack mrkdwn links as a unit so + // later generic URL matching does not consume the trailing ">" or "|label". + s = s.replace(angleBracketHttpsAutolinkRegex, (match, hostnameWithPort, path = "") => { + const url = `https://${hostnameWithPort}${path || ""}`; + const filtered = applyDomainFilter(url, hostnameWithPort); + return filtered === url ? match : filtered; + }); + // First pass: handle explicit https:// URLs s = s.replace(httpsUrlRegex, (match, hostnameWithPort) => applyDomainFilter(match, hostnameWithPort)); @@ -722,6 +732,22 @@ function convertXmlTags(s) { "ul", ]; + /** + * Slack/CommonMark autolinks use angle brackets but are not HTML tags. + * Preserve HTTPS forms here so downstream URL filtering can inspect them + * without convertXmlTags rewriting them to parentheses first. + * + * Supported forms: + * + * + * + * @param {string} tagContent + * @returns {boolean} + */ + function isHttpsAngleBracketAutolink(tagContent) { + return /^https:\/\/[^\s<>|]+(?:\|[^<>]*)?$/i.test(tagContent); + } + // First, process CDATA sections specially - convert tags inside them and the CDATA markers s = s.replace(//g, (match, content) => { // Convert tags inside CDATA content @@ -767,6 +793,9 @@ function convertXmlTags(s) { // Convert self-closing tags: or to (tag/) or (tag /) // But preserve allowed safe tags (with dangerous attributes stripped) return s.replace(/<(\/?[A-Za-z!][^>]*?)>/g, (match, tagContent) => { + if (isHttpsAngleBracketAutolink(tagContent)) { + return match; + } // Extract tag name from the content (handle closing tags and attributes) const tagNameMatch = tagContent.match(/^\/?\s*([A-Za-z][A-Za-z0-9]*)/); if (tagNameMatch) { From fb2851afca0567307748e02247510b7e4fea518c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Jul 2026 12:21:45 +0000 Subject: [PATCH 3/5] fix: tighten isHttpsAngleBracketAutolink to reject IPv6 and userinfo authorities; add fuzz seeds Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/sanitize_content.test.cjs | 12 ++++++++++++ actions/setup/js/sanitize_content_core.cjs | 9 ++++++++- pkg/workflow/sanitize_output_fuzz_test.go | 12 +++++++++++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/sanitize_content.test.cjs b/actions/setup/js/sanitize_content.test.cjs index 82b3d04c628..0c3f1e57825 100644 --- a/actions/setup/js/sanitize_content.test.cjs +++ b/actions/setup/js/sanitize_content.test.cjs @@ -591,6 +591,18 @@ describe("sanitize_content.cjs", () => { expect(sanitizeContent(input)).toBe(input); }); + it("should not preserve IPv6 angle-bracket links", () => { + // IPv6 authority is not a simple [\w.-]+ hostname — treat as unknown tag + const result = sanitizeContent(""); + expect(result).not.toContain(" { + // Userinfo (user@host) must not bypass domain filtering + const result = sanitizeContent(""); + expect(result).not.toContain(" { const result = sanitizeContent("alert('xss')]]>"); expect(result).toBe("(![CDATA[(script)alert('xss')(/script)]])"); diff --git a/actions/setup/js/sanitize_content_core.cjs b/actions/setup/js/sanitize_content_core.cjs index 8255b8289df..9ad8d196741 100644 --- a/actions/setup/js/sanitize_content_core.cjs +++ b/actions/setup/js/sanitize_content_core.cjs @@ -745,7 +745,14 @@ function convertXmlTags(s) { * @returns {boolean} */ function isHttpsAngleBracketAutolink(tagContent) { - return /^https:\/\/[^\s<>|]+(?:\|[^<>]*)?$/i.test(tagContent); + // Only match simple hostname forms that sanitizeUrlDomains can parse and + // filter. The hostname must consist of word characters, dots, and hyphens + // ([\w.-]+), which excludes: + // - IPv6 literals e.g. https://[2001:db8::1]/ + // - Userinfo forms e.g. https://user@evil.example/ + // Those forms fall through to the generic tag-conversion path and are + // wrapped in parentheses instead of being preserved as autolinks. + return /^https:\/\/[\w.-]+(?::\d+)?(\/[^\s<>|]*)?(?:\|[^<>]*)?$/i.test(tagContent); } // First, process CDATA sections specially - convert tags inside them and the CDATA markers diff --git a/pkg/workflow/sanitize_output_fuzz_test.go b/pkg/workflow/sanitize_output_fuzz_test.go index 372e0b458bc..8ca11710ac1 100644 --- a/pkg/workflow/sanitize_output_fuzz_test.go +++ b/pkg/workflow/sanitize_output_fuzz_test.go @@ -125,7 +125,17 @@ func FuzzSanitizeOutput(f *testing.F) { f.Add("https://evil.com/]]>"); expect(result).toBe("(![CDATA[(script)alert('xss')(/script)]])"); diff --git a/actions/setup/js/sanitize_content_core.cjs b/actions/setup/js/sanitize_content_core.cjs index 9ad8d196741..ad8abd677ec 100644 --- a/actions/setup/js/sanitize_content_core.cjs +++ b/actions/setup/js/sanitize_content_core.cjs @@ -13,6 +13,32 @@ const SAFE_OUTPUTS_URLS_ENV = "GH_AW_SAFE_OUTPUTS_URLS"; const SAFE_OUTPUTS_URLS_ALLOWED_ONLY = "allowed-only"; const SAFE_OUTPUTS_URLS_ALLOWED_OR_CODE_REGION = "allowed-or-code-region"; +/** + * Matches angle-bracket HTTPS autolinks in CommonMark and Slack mrkdwn form: + * + * + * + * Capture groups: + * 1: hostname with optional port ([\w.-]+(?::\d+)?) + * 2: optional path (\/[^\s<>|]*) + * 3: optional Slack label ([^<>]*) + * + * The path intentionally stops at "|" so that the Slack label is captured + * separately. A bare "|" inside a URL path or query string (valid per + * RFC 3986 but uncommon — browsers percent-encode it as %7C) is therefore + * treated as the Slack label separator. This is a known limitation; use + * %7C in URLs where a literal "|" must be preserved verbatim. + * + * The strict hostname pattern ([\w.-]+) deliberately excludes IPv6 literals + * and userinfo forms so that those fall through to tag-conversion instead of + * being preserved as autolinks. + * + * Defined at module level (not inside sanitizeUrlDomains) so the RegExp object + * is allocated once and reused across calls. String.prototype.replace resets + * lastIndex automatically, so the /g flag is safe here. + */ +const angleBracketHttpsAutolinkRegex = /|]*)?(?:\|([^<>]*))?>/gi; + /** * Module-level set to collect redacted URL domains across sanitization calls. * @type {string[]} @@ -266,8 +292,6 @@ function sanitizeUrlProtocols(s) { * @returns {string} The string with unknown domains redacted */ function sanitizeUrlDomains(s, allowed) { - const angleBracketHttpsAutolinkRegex = /|]*)?(?:\|([^<>]*))?>/gi; - // Match HTTPS URLs with optional port and path // This regex is designed to: // 1. Match https:// URIs with explicit protocol @@ -320,15 +344,26 @@ function sanitizeUrlDomains(s, allowed) { } } - // First pass: handle angle-bracket autolinks and Slack mrkdwn links as a unit so + // Pass 1: handle angle-bracket autolinks and Slack mrkdwn links as a unit so // later generic URL matching does not consume the trailing ">" or "|label". - s = s.replace(angleBracketHttpsAutolinkRegex, (match, hostnameWithPort, path = "") => { - const url = `https://${hostnameWithPort}${path || ""}`; + // + // Capture groups from angleBracketHttpsAutolinkRegex: + // 1: hostnameWithPort + // 2: optional path (stops before "|") + // 3: optional Slack label (everything after "|" up to ">", may contain spaces) + // + // If the domain is disallowed the label text is preserved alongside the + // redacted URL so that the author's visible link text is not silently lost. + s = s.replace(angleBracketHttpsAutolinkRegex, (match, hostnameWithPort, path = "", label = "") => { + const url = `https://${hostnameWithPort}${path}`; const filtered = applyDomainFilter(url, hostnameWithPort); - return filtered === url ? match : filtered; + if (filtered === url) return match; // allowed — preserve original angle-bracket form + // Disallowed — redact URL but preserve label text if present so the author's + // visible link text is not silently lost. + return label ? `${filtered}|${label}` : filtered; }); - // First pass: handle explicit https:// URLs + // Pass 2: handle remaining explicit https:// URLs s = s.replace(httpsUrlRegex, (match, hostnameWithPort) => applyDomainFilter(match, hostnameWithPort)); // Second pass: handle protocol-relative URLs (//hostname/path). @@ -752,6 +787,9 @@ function convertXmlTags(s) { // - Userinfo forms e.g. https://user@evil.example/ // Those forms fall through to the generic tag-conversion path and are // wrapped in parentheses instead of being preserved as autolinks. + // + // (\/[^\s<>|]*)? — optional path (mirrors angleBracketHttpsAutolinkRegex) + // (?:\|[^<>]*)? — optional Slack label, may contain spaces return /^https:\/\/[\w.-]+(?::\d+)?(\/[^\s<>|]*)?(?:\|[^<>]*)?$/i.test(tagContent); } From f71cd0822e2bdc70a709a8b87f5bdcaf841b385c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Jul 2026 13:29:28 +0000 Subject: [PATCH 5/5] fix: drop unnecessary /i flag from angleBracketHttpsAutolinkRegex and isHttpsAngleBracketAutolink Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- actions/setup/js/sanitize_content_core.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/sanitize_content_core.cjs b/actions/setup/js/sanitize_content_core.cjs index ad8abd677ec..f3effeb5796 100644 --- a/actions/setup/js/sanitize_content_core.cjs +++ b/actions/setup/js/sanitize_content_core.cjs @@ -37,7 +37,7 @@ const SAFE_OUTPUTS_URLS_ALLOWED_OR_CODE_REGION = "allowed-or-code-region"; * is allocated once and reused across calls. String.prototype.replace resets * lastIndex automatically, so the /g flag is safe here. */ -const angleBracketHttpsAutolinkRegex = /|]*)?(?:\|([^<>]*))?>/gi; +const angleBracketHttpsAutolinkRegex = /|]*)?(?:\|([^<>]*))?>/g; /** * Module-level set to collect redacted URL domains across sanitization calls. @@ -790,7 +790,7 @@ function convertXmlTags(s) { // // (\/[^\s<>|]*)? — optional path (mirrors angleBracketHttpsAutolinkRegex) // (?:\|[^<>]*)? — optional Slack label, may contain spaces - return /^https:\/\/[\w.-]+(?::\d+)?(\/[^\s<>|]*)?(?:\|[^<>]*)?$/i.test(tagContent); + return /^https:\/\/[\w.-]+(?::\d+)?(\/[^\s<>|]*)?(?:\|[^<>]*)?$/.test(tagContent); } // First, process CDATA sections specially - convert tags inside them and the CDATA markers