fix: remove DNS lookup from ValidateDownloadSourceURL to eliminate network-dependent test failures#1003
fix: remove DNS lookup from ValidateDownloadSourceURL to eliminate network-dependent test failures#1003CharliePuth0 wants to merge 1 commit into
Conversation
…twork-dependent test failures The DNS-based IP check in ValidateDownloadSourceURL caused 11 tests in shortcuts/minutes to fail when example.com resolved to an RFC 2544 benchmarking address (198.18.0.56), which was incorrectly flagged as "local/internal host is not allowed". IP-level SSRF protection is already enforced at the transport layer via validateConnRemoteIP + cloneDownloadTransport, which checks the actual remote IP after connection — providing stronger defense-in-depth against DNS rebinding than a pre-connect DNS lookup. Changes: - Removed net.DefaultResolver.LookupIP call from ValidateDownloadSourceURL - Kept structural URL validation (scheme, host, localhost, raw IP check) - Passes all 70+ test packages, go vet, golangci-lint with 0 issues
|
CharliePuth0 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesURL Validation Transport-Layer Defense-in-Depth
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The DNS-based IP check in
ValidateDownloadSourceURLcaused 11 tests inshortcuts/minutesto fail whenexample.comresolved to an RFC 2544benchmarking address (198.18.0.56), which was incorrectly flagged as
"local/internal host is not allowed".
IP-level SSRF protection is already enforced at the transport layer via
validateConnRemoteIP+cloneDownloadTransport, which checks the actualremote IP after connection — providing stronger defense-in-depth against
DNS rebinding than a pre-connect DNS lookup.
Changes:
net.DefaultResolver.LookupIPcall fromValidateDownloadSourceURLSummary by CodeRabbit