feat: resolve SSL issue #508, implement SSRF protection & security te…#3171
Closed
Tomo1912 wants to merge 0 commit intomodelcontextprotocol:mainfrom
Closed
feat: resolve SSL issue #508, implement SSRF protection & security te…#3171Tomo1912 wants to merge 0 commit intomodelcontextprotocol:mainfrom
Tomo1912 wants to merge 0 commit intomodelcontextprotocol:mainfrom
Conversation
12 tasks
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
2 participants
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
This PR addresses Issue #508 by adding an SSL verification toggle for the fetch server. Since this was a highly requested fix for internal testing environments, I also took the opportunity to harden the server's security against SSRF and resource abuse.
Server Details
Server: fetch
Changes to: SSL config, URL validation, and error handling
Motivation and Context
Users on internal networks with self-signed certificates currently can't use the fetch tool. This PR introduces a secure way to bypass SSL check via env vars, while adding a safety layer to ensure this doesn't expose internal infrastructure.
How Has This Been Tested?
I've implemented a new test suite (tests/test_security.py) with 71 test cases.
Scenarios tested: SSL toggle functionality, private/reserved IP blocking (SSRF protection), and response size limits (5MB) to prevent memory exhaustion.
Pass rate: 100% of tests are passing locally.
Breaking Changes
No. Default behavior remains SSL_VERIFY=true.
Types of changes
[x] Bug fix
[x] New feature
[x] Documentation update
Checklist
[x] I have read the MCP Protocol Documentation
[x] My changes follow MCP security best practices
[x] I have updated the server's documentation (SECURITY_AUDIT.md)
[x] New and existing tests pass locally
[x] I have documented all new environment variables
Additional context
The core of the security fix is the new is_safe_url logic. It prevents the server from being used to scan internal networks or cloud metadata endpoints. I've also added a 10s timeout to keep the server responsive under load.