Server-Side Request Forgery (SSRF) protection#271
Server-Side Request Forgery (SSRF) protection#271sergey-tihon merged 7 commits intofsprojects:masterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds SSRF (Server-Side Request Forgery) protection to SwaggerProvider by blocking HTTP URLs and localhost/private IP addresses by default. A new SsrfProtection parameter (default: true) allows developers to disable these protections for local development and testing.
Key changes:
- Added SSRF validation that blocks HTTP, localhost, and private IP ranges when protection is enabled
- Disabled credential leakage by setting
UseDefaultCredentials = falseunconditionally - Added timeout configuration for HTTP client requests
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/SwaggerProvider.DesignTime/Utils.fs |
Implements SSRF protection validation logic and enforces secure HTTP client configuration |
src/SwaggerProvider.DesignTime/Provider.SwaggerClient.fs |
Adds SsrfProtection parameter to SwaggerClientProvider with documentation |
src/SwaggerProvider.DesignTime/Provider.OpenApiClient.fs |
Adds SsrfProtection parameter to OpenApiClientProvider with documentation |
tests/SwaggerProvider.ProviderTests/v3/Swashbuckle.ReturnControllers.Tests.fs |
Updates test to disable SSRF protection for localhost testing |
tests/SwaggerProvider.ProviderTests/v2/Swashbuckle.ReturnControllers.Tests.fs |
Updates test to disable SSRF protection for localhost testing |
docs/SwaggerClientProvider.md |
Documents the new security feature and usage examples |
docs/OpenApiClientProvider.md |
Documents the new security feature and usage examples |
README.md |
Adds security notice about SSRF protection |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not(isNull contentType) then | ||
| let mediaType = contentType.MediaType.ToLowerInvariant() | ||
|
|
||
| if | ||
| not( | ||
| mediaType.Contains "json" | ||
| || mediaType.Contains "yaml" | ||
| || mediaType.Contains "text" | ||
| || mediaType.Contains "application/octet-stream" | ||
| ) | ||
| then | ||
| failwithf "Invalid Content-Type for schema: %s. Expected JSON or YAML." mediaType |
There was a problem hiding this comment.
The HTTP response path includes Content-Type validation (lines 97-111), but the same validation is missing from the HTTPS response path when ignoreSsrfProtection is true (lines 140-184). This inconsistency could allow invalid content types in development mode. Consider extracting this validation into a reusable function and applying it consistently across both paths.
There was a problem hiding this comment.
Does it make sense to unify validation? Otherwise in may work in development mode and then fail when you switch to https because of inconsistent validation.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
CI fails, the multiformat tests, maybe doesn't pass the content validation now. I struggle to build the test solution in my local machine. |
|
Some documentation to anywhere how to build the test-project now would be nice, it needs some server: |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add Server-Side Request Forgery (SSRF) protection.
https://owasp.org/Top10/A10_2021-Server-Side_Request_Forgery_%28SSRF%29/
This is a potentially breaking change, but an important security update:
use handler = new HttpClientHandler(UseDefaultCredentials = false)to avoid machine level credentials to be exposed.