fix: protect URL downloads against SSRF with pinned DNS - #10125
Open
banned2054 wants to merge 2 commits into
Open
banned2054 wants to merge 2 commits into
banned2054 wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. This changes the outbound-request trust boundary for all protected downloads: a validation or redirect/DNS-pinning mistake could let attacker-controlled URLs reach private services and feed their responses into the application. Reverting would stop future requests but would not undo any internal data already fetched or processed.
🔴 [blocking] 自建私网 T2I 端点在
|
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.
Fixes #10122
URL-based file and image downloads could reach private services through a direct URL, a DNS response, or a redirect. This change validates each destination before requesting it and pins the validated DNS results to the connection.
Modifications / 改动点
Protect
download_fileanddownload_image_by_urlin the shared download layer: accept HTTP(S) URLs without embedded credentials and reject non-public IPv4/IPv6 destinations by default.Use yarl hostname normalization, a resolver with no unchecked fallback, and explicit redirect validation. Apply the same policy to GET, POST, and TLS fallback requests.
Preserve access to explicitly configured self-hosted T2I endpoints. For Telegram voice/audio downloads, permit custom private file services while restricting the initial request and every redirect to the configured scheme, host, and port.
Document the proxy compatibility change in both English and Chinese configuration guides.
This is NOT a breaking change. / 这不是一个破坏性变更。
Compatibility: Protected downloads now connect directly with
trust_env=False. They no longer use HTTP(S) environment proxies, because proxy-side DNS resolution would bypass the pinned destination. Deployments requiring a proxy need a directly reachable download URL or an appropriate application-level mirror. Existing callers downloading private URLs must explicitly opt into trusted private destinations.Screenshots or Test Results / 运行截图或测试结果
Verification on Windows using the project virtual environment:
The warning is the existing
audioopdeprecation warning. Tests cover blocked IP/DNS destinations, IDNA normalization, redirect validation, trusted-origin restrictions, proxy settings, POST handling, TLS fallback behavior, and Telegram integration. A real aiohttp connector test uses a mocked hostname and a local server with a same-origin redirect, asserting one DNS lookup per validation step and no additional lookup when either connection is opened.Checklist / 检查清单
Summary by Sourcery
Protect outbound media downloads against SSRF while preserving controlled access for trusted self-hosted services.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: