fix(sanitize): switch sanitizeUrlProtocols to allowlist for ://-scheme URLs by Copilot · Pull Request #31715 · github/gh-aw

Copilot · 2026-05-12T14:49:57Z

Bug Fix

What was the bug?

sanitizeUrlProtocols() used a hardcoded blocklist of :// schemes (http, ftp, file, ssh, git). Any unlisted scheme — ws://, wss://, smb://, irc://, ldap://, ldaps://, rtsp://, feed://, etc. — passed through the full sanitizeContentCore pipeline unchanged and was written verbatim to GitHub. In a prompt-injection scenario this enables active markdown links to attacker-controlled endpoints (e.g. smb:// triggers NTLM credential leaks on Windows via Responder).

How did you fix it?

Step 1 — allowlist for :// schemes (sanitize_content_core.cjs)

Replace the explicit blocklist with an allowlist: redact every protocol:// URL that is not https://. Two anchors prevent corrupting valid https:// URLs:

(?<![a-z0-9]) (negative lookbehind) — stops the regex from matching a suffix of https:// (e.g. ttps://) mid-string.
(?!https://) (negative lookahead) — explicitly skips the https:// scheme, which is forwarded to sanitizeUrlDomains for domain-level filtering.

// Before — explicit blocklist, incomplete:
normalized.replace(/((?:http|ftp|file|ssh|git):\/\/([\w.-]*)(?:[^\s]*)|...)/gi, ...)

// After — allowlist, catches everything except https://:
normalized.replace(/(?<![a-z0-9])(?!https:\/\/)([a-z][a-z0-9+.-]*)(:\/\/)([\w.-]*)([^\s]*)/gi, ...)

Step 2 — extend the single-colon blocklist

Add magnet to the existing data|javascript|vbscript|about|mailto|tel list to cover magnet:?xt=… URLs. A fully general single-colon pattern is intentionally avoided to prevent false positives on YAML key:value, C++ std::, and Windows C:\ paths.

The two steps are implemented as separate replace calls for clarity.

Testing

10 new regression tests added covering ws://, wss://, smb://, irc://, ldap://, ldaps://, rtsp://, magnet:, the https:// pass-through, and a guard against the "ttps:// suffix" regression.

Co-authored-by: szabta89 <1330202+szabta89@users.noreply.github.com>

… single-colon blocklist The previous `sanitizeUrlProtocols` regex matched only a fixed set of `://` protocols (http, ftp, file, ssh, git), leaving ws://, wss://, smb://, irc://, ldap://, ldaps://, rtsp://, feed://, and any future schemes able to pass through unredacted. Switch the `://` branch to an allowlist: redact **every** protocol:// URL except https:// (which is handled downstream by sanitizeUrlDomains for domain-level filtering). Two anchors keep https:// safe: * Negative lookbehind `(?<![a-z0-9])` – prevents the regex from matching a suffix of "https://" (e.g. "ttps://") when scanning inside an existing HTTPS URL. * Negative lookahead `(?!https://)` – explicitly excludes the https:// scheme at position 0. Also add `magnet` to the single-colon scheme blocklist to cover `magnet:?xt=…` URLs. Adds 10 new regression tests covering ws://, wss://, smb://, irc://, ldap://, ldaps://, rtsp://, magnet:, the https:// pass-through, and the "ttps://" suffix regression guard. Co-authored-by: szabta89 <1330202+szabta89@users.noreply.github.com>

…ce calls Addresses code-review feedback: the combined regex was hard to read. Step 1 (allowlist) handles protocol:// URLs; step 2 (blocklist) handles single-colon schemes. Logic is unchanged; all 467 tests still pass. Co-authored-by: szabta89 <1330202+szabta89@users.noreply.github.com>

Copilot

Pull request overview

This PR tightens URL protocol sanitization to prevent non-HTTPS scheme:// links from passing through unredacted, and adds regression tests for newly covered schemes.

Changes:

Switch sanitizeUrlProtocols() from a :// scheme blocklist to an allowlist approach that only permits https://.
Extend single-colon scheme blocking to include magnet:.
Add protocol sanitization regression tests for several scheme:// URLs and magnet:; update two Dependabot workflow lockfiles to derive the Docker socket path from DOCKER_HOST.

Show a summary per file

File	Description
actions/setup/js/sanitize_content_core.cjs	Changes protocol sanitization to redact all non-HTTPS `scheme://` URLs and adds `magnet:` to the single-colon blocklist.
actions/setup/js/sanitize_content.test.cjs	Adds regression tests for new protocol sanitization behavior.
.github/workflows/dependabot-worker.lock.yml	Updates MCP gateway Docker socket handling to respect `DOCKER_HOST`.
.github/workflows/dependabot-campaign.lock.yml	Updates MCP gateway Docker socket handling to respect `DOCKER_HOST`.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 4/4 changed files
Comments generated: 3

+    const sanitized = sanitizeDomainName(domainLower);
+    const truncated = domainLower.length > 12 ? domainLower.substring(0, 12) + "..." : domainLower;
+    core.info(`Redacted URL: ${truncated}`);
+    core.debug(`Redacted URL (full): ${fullMatch}`);
+    addRedactedDomain(domainLower);


+          case "${DOCKER_HOST:-}" in
+            unix://* ) DOCKER_SOCK_PATH="${DOCKER_HOST#unix://}" ;;
+            /* ) DOCKER_SOCK_PATH="$DOCKER_HOST" ;;
+            * ) DOCKER_SOCK_PATH=/var/run/docker.sock ;;
+          esac


+          case "${DOCKER_HOST:-}" in
+            unix://* ) DOCKER_SOCK_PATH="${DOCKER_HOST#unix://}" ;;
+            /* ) DOCKER_SOCK_PATH="$DOCKER_HOST" ;;
+            * ) DOCKER_SOCK_PATH=/var/run/docker.sock ;;
+          esac


pelikhan · 2026-05-12T15:49:59Z

@copilot merge main, recompile, review all comments

…url-protocols Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

…edaction When the domain capture group is empty (e.g. `file:///etc/passwd` or bare `http://`), the previous code called `addRedactedDomain("")` and logged an empty truncated string. Now the empty-domain branch uses the scheme name (e.g. `file://`) as the redacted-domain token and returns `(redacted)` directly. Adds a regression test for `file:///etc/passwd`. Also merges origin/main. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot · 2026-05-12T15:58:21Z

@copilot merge main, recompile, review all comments

Done — merged main, typechecked, and addressed all review comments:

Empty-domain guard (file:///path, bare http://): the Step 1 callback now detects an empty domain capture, uses scheme + "://" as the addRedactedDomain token, and returns (redacted) directly — no empty-string entry is recorded. Regression test added. (commit fix(sanitize): guard empty-domain case)
Workflow file comments: the dependabot-*.lock.yml changes were present on the branch before my commits (they are part of the upstream branch state, not introduced by this fix). They are unrelated to the sanitization change.

…Protocols The comment said "skip addRedactedDomain" but the code called it. Updated the comment to accurately describe that the scheme token (e.g. "file://") is used as the redacted-domain entry. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Initial plan

3b2ec4e

Copilot AI assigned Copilot and szabta89 May 12, 2026

Copilot started work on behalf of szabta89 May 12, 2026 14:53 View session

Copilot AI linked an issue May 12, 2026 that may be closed by this pull request

sanitizeUrlProtocols incomplete — ws://, wss://, smb://, irc://, ldap:// bypass safe-output URL sanitization #31710

Closed

Copilot AI and others added 3 commits May 12, 2026 15:02

chore: planning sanitizeUrlProtocols fix

c5cc316

Co-authored-by: szabta89 <1330202+szabta89@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix incomplete URL sanitization for additional protocols~~ fix(sanitize): switch sanitizeUrlProtocols to allowlist for ://-scheme URLs May 12, 2026

Copilot AI requested a review from szabta89 May 12, 2026 15:12

Copilot finished work on behalf of szabta89 May 12, 2026 15:12

pelikhan marked this pull request as ready for review May 12, 2026 15:42

Copilot AI review requested due to automatic review settings May 12, 2026 15:43

Copilot started reviewing on behalf of pelikhan May 12, 2026 15:44 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Copilot started work on behalf of pelikhan May 12, 2026 15:50 View session

Copilot AI and others added 2 commits May 12, 2026 15:56

Merge remote-tracking branch 'origin/main' into copilot/fix-sanitize-…

aedfe42

…url-protocols Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot finished work on behalf of pelikhan May 12, 2026 16:00

Copilot AI requested a review from pelikhan May 12, 2026 16:00

pelikhan merged commit 8eb7e09 into main May 12, 2026

pelikhan deleted the copilot/fix-sanitize-url-protocols branch May 12, 2026 16:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(sanitize): switch sanitizeUrlProtocols to allowlist for ://-scheme URLs#31715

fix(sanitize): switch sanitizeUrlProtocols to allowlist for ://-scheme URLs#31715
pelikhan merged 7 commits into
mainfrom
copilot/fix-sanitize-url-protocols

Copilot AI commented May 12, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

pelikhan commented May 12, 2026

Uh oh!

Copilot AI commented May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Copilot AI commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug Fix

What was the bug?

How did you fix it?

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

pelikhan commented May 12, 2026

Uh oh!

Copilot AI commented May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented May 12, 2026 •

edited

Loading