fix(offline-download): block cloud metadata endpoints#2487
Open
jyxjjj wants to merge 1 commit into
Open
Conversation
- Add shared validation for cloud metadata endpoint URLs - Reuse validation in offline download entry and HTTP backends - Cover direct, DNS, private URL, and redirect validation cases Co-authored-by: Codex <267193182+codex@users.noreply.github.com> Signed-off-by: jyxjjj <16695261+jyxjjj@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds shared offline-download URL validation to block cloud metadata endpoint access, then wires it into task creation and HTTP-based download paths.
Changes:
- Added metadata endpoint validation and redirect-aware HTTP client helper.
- Added tests for direct IP, DNS resolution, private/public URLs, and redirect checks.
- Integrated validation into
tool.AddURL,SimpleHttp.Run, andTransmission.AddURL.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
internal/offline_download/tool/metadata_url.go |
Adds shared URL validation and redirect-checking HTTP client wrapper. |
internal/offline_download/tool/metadata_url_test.go |
Adds unit tests for metadata endpoint blocking behavior. |
internal/offline_download/tool/add.go |
Validates URLs before creating offline download tasks. |
internal/offline_download/http/client.go |
Applies validation and redirect checks to SimpleHttp downloads. |
internal/offline_download/transmission/client.go |
Applies validation and redirect checks to Transmission torrent URL fetching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+76
to
+79
| addrs, err := lookupIPAddr(ctx, host) | ||
| if err != nil { | ||
| return err | ||
| } |
Comment on lines
+47
to
+49
| if err := ValidateOfflineDownloadURL(ctx, args.URL); err != nil { | ||
| return nil, err | ||
| } |
Comment on lines
+50
to
+54
| func NewOfflineDownloadHTTPClient(base http.Client) *http.Client { | ||
| client := base | ||
| previousCheckRedirect := client.CheckRedirect | ||
| client.CheckRedirect = func(req *http.Request, via []*http.Request) error { | ||
| if err := ValidateOfflineDownloadURL(req.Context(), req.URL.String()); err != nil { |
Comment on lines
+47
to
+49
| if err := ValidateOfflineDownloadURL(ctx, args.URL); err != nil { | ||
| return nil, err | ||
| } |
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 / 描述
Add shared validation for offline download URLs to block access to cloud metadata endpoints before URLs enter specific download backends.
This change rejects
169.254.169.254, including URL forms with schemes, ports, DNS names resolving to this IP, and redirect targets pointing to this IP.The validation is reused by:
tool.AddURLSimpleHttpTransmissionIt intentionally does not block ordinary private network or LAN addresses.
Motivation and Context / 背景
Offline download backends may fetch user-provided URLs directly. Without centralized validation, a URL can target cloud metadata endpoints such as
169.254.169.254, or reach them through DNS resolution or HTTP redirects.This PR limits only the high-risk cloud metadata endpoint case while preserving compatibility with user self-hosted private network services.
Relates to security hardening for offline download URL handling.
How Has This Been Tested? / 测试
Tested with:
Covered cases:
169.254.169.254is rejected169.254.169.254is rejected169.254.169.254is rejectedNote:
go test ./internal/offline_download/...still fails because of existing unrelatedfmt.Errorfvet issues in115,115_open, andpikpak.Checklist / 检查清单
我已阅读 CONTRIBUTING 文档。
go fmtor prettier.我已使用
go fmt或 prettier 格式化提交的代码。我已为此 PR 添加了适当的标签（如无权限或需要的标签不存在，请在描述中说明，管理员将后续处理）。
我已在适当情况下使用"Request review"功能请求相关代码作者进行审查。