fix(everything): block SSRF to internal/metadata IPs in gzip-file-as-resource#4498
Open
olaservo wants to merge 4 commits into
Open
fix(everything): block SSRF to internal/metadata IPs in gzip-file-as-resource#4498olaservo wants to merge 4 commits into
olaservo wants to merge 4 commits into
Conversation
…resource The gzip-file-as-resource tool fetched a caller-supplied URL with only an optional domain allowlist (empty by default, treated as allow-all) and no IP-range filtering, and followed redirects without re-validation. A prompt-injection-steered URL could drive the server to fetch loopback, private, link-local, and cloud-metadata endpoints (e.g. 169.254.169.254) and return their contents to the caller. Resolve the destination host and refuse non-public IP addresses (loopback, private/RFC1918, link-local/metadata, ULA, multicast, reserved, unspecified), covering IPv4, IPv6, and IPv4-mapped IPv6, and follow redirects manually so every hop is re-validated. This applies regardless of GZIP_ALLOWED_DOMAINS, whose domain-allowlist semantics are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Unwrap deprecated IPv4-compatible IPv6 addresses (::a.b.c.d, ::/96) and classify them as IPv4, so forms like [::127.0.0.1] are refused rather than treated as public. Adds test coverage for the IPv4-compatible form and for carrier-grade NAT (100.64.0.0/10). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the everything server’s gzip-file-as-resource tool against SSRF by adding IP-range blocking (for loopback/private/link-local/metadata/reserved ranges) and by replacing automatic redirect following with manual redirects that re-validate the destination host at each hop.
Changes:
- Added host resolution + IPv4/IPv6 (incl. IPv4-mapped IPv6) classifiers to block non-public destinations for
http(s)fetches. - Implemented manual redirect following with per-hop SSRF re-validation and refusal of redirects to non-
http(s)schemes. - Updated server docs and added unit tests for blocked IP literals.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/everything/tools/gzip-file-as-resource.ts | Adds SSRF host/IP validation and guarded manual redirect following for fetches. |
| src/everything/docs/structure.md | Documents SSRF blocking behavior for the gzip tool. |
| src/everything/docs/instructions.md | Notes SSRF blocking (including across redirects) in constraints section. |
| src/everything/tests/tools.test.ts | Adds tests ensuring non-public IP literal targets are refused. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… guard Addresses Copilot review feedback on modelcontextprotocol#4498: - assertPublicHost now races the dns/promises lookup against the fetch AbortSignal via a withAbort helper, so a slow-walking resolver can no longer exceed the tool's documented timeout (DNS was previously awaited outside the AbortSignal). - Add a test that a public URL redirecting to 169.254.169.254 is refused at the second hop before any request is made to the internal target, guarding against regressions in per-hop re-validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses Copilot feedback on modelcontextprotocol#4498: restructure the guarded-redirect loop to check GZIP_MAX_REDIRECTS after detecting a redirect but before resolving/validating/following the next hop. Behavior is unchanged (at most GZIP_MAX_REDIRECTS redirects followed), but the final redirect target is no longer parsed and assigned before the loop throws. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+447
to
+453
| const isRedirect = | ||
| response.status >= 300 && | ||
| response.status < 400 && | ||
| response.headers.has("location"); | ||
| if (!isRedirect) { | ||
| return response; | ||
| } |
Comment on lines
+1221
to
+1234
| // SSRF protection: the tool must refuse to fetch non-public IP addresses. | ||
| // These use IP literals so no DNS resolution (or network) is required. | ||
| const blockedHosts: Array<[string, string]> = [ | ||
| ['loopback IPv4', 'http://127.0.0.1/secret'], | ||
| ['cloud metadata', 'http://169.254.169.254/latest/meta-data/'], | ||
| ['private 10/8', 'http://10.0.0.1/'], | ||
| ['private 192.168/16', 'http://192.168.1.1/'], | ||
| ['private 172.16/12', 'http://172.16.0.1/'], | ||
| ['unspecified', 'http://0.0.0.0/'], | ||
| ['carrier-grade NAT 100.64/10', 'http://100.64.0.1/'], | ||
| ['IPv6 loopback', 'http://[::1]/'], | ||
| ['IPv4-mapped IPv6 loopback', 'http://[::ffff:127.0.0.1]/'], | ||
| ['IPv4-compatible IPv6 loopback', 'http://[::127.0.0.1]/'], | ||
| ]; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds SSRF protection to the
everythingserver'sgzip-file-as-resourcetool. The tool fetched a caller-suppliedhttp/httpsURL and returned the (compressed) response as a resource. Its only host restriction was an optional domain allowlist (GZIP_ALLOWED_DOMAINS) that is empty by default and treated as "all domains allowed", with no IP-range filtering and automatic redirect following. Because the URL is model-produced and prompt-injection-steerable, the server could be driven to fetch loopback, private, link-local, and cloud-metadata endpoints (e.g.169.254.169.254) and return their contents to the caller.Server Details
@modelcontextprotocol/server-everything)gzip-file-as-resource)Motivation and Context
validateDataURIallowedhttp/https/dataand enforced only the domain allowlist (skipped entirely when empty), andfetchSafelycalledfetch(url)with default redirect following and no loopback/RFC1918/link-local/metadata checks.What this PR does:
assertPublicHost()plus IPv4/IPv6 classifiers that resolve the destination host and refuse non-public addresses (loopback, private/RFC1918, link-local/metadata, ULA, multicast, reserved, unspecified), covering IPv4, IPv6, and IPv4-mapped IPv6 (including the hex form the WHATWG URL parser normalizes to).fetchWithGuardedRedirects(redirect: "manual"), re-validating the host on every hop and refusing redirects to non-http(s) schemes.GZIP_ALLOWED_DOMAINS. The domain-allowlist semantics are intentionally left unchanged (empty still means "all domains"), so the tool's default demo URL keeps working; the allowlist and the SSRF guard are independent controls.How Has This Been Tested?
169.254.169.254, RFC1918 ranges,0.0.0.0, IPv6 loopback, IPv4-mapped IPv6) via IP literals (no network required).npm run build(tsc) passes; fullvitestsuite passes (115 tests, including the 8 new cases).data:URI and public-URL behavior is unchanged and still covered by tests.Breaking Changes
No client configuration changes required. Behavior only changes for requests that targeted non-public IPs, which are now refused (the intended fix).
data:URIs and public URLs are unaffected.Types of changes
Checklist
Additional context
Docs updated in
docs/instructions.mdanddocs/structure.mdto note that internal/metadata IPs are always blocked (and re-validated across redirects) independent of the allowlist. As with any resolve-then-connect guard, a narrow DNS-rebinding window remains; pinning to the validated IP could be added later.🤖 Generated with Claude Code