🛡️ Sentinel: [HIGH] Fix SSRF in URL lookup#31
Conversation
Adds pre-fetch DNS validation using `isPrivateIP` to block server-side requests to internal or private IP networks such as localhost and AWS metadata services. Co-authored-by: aicoder2009 <[email protected]>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Addresses a high-severity SSRF risk in /api/lookup/url by adding pre-request hostname resolution and blocking requests to private/internal IP ranges.
Changes:
- Added
dns/promiseslookup and anisPrivateIPguard before issuing the outboundfetch(). - Returns
403when the resolved IP is considered internal/private and400on DNS resolution failures. - Documented the SSRF finding and mitigation notes in
.jules/sentinel.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/app/api/lookup/url/route.ts |
Adds DNS-based SSRF filtering (private IP detection) before fetching user-supplied URLs. |
.jules/sentinel.md |
Adds a sentinel entry documenting the SSRF vulnerability and high-level prevention guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { address } = await lookup(parsedUrl.hostname); | ||
| if (isPrivateIP(address)) { |
There was a problem hiding this comment.
lookup(parsedUrl.hostname) only validates a single resolved address; hostnames with multiple A/AAAA records can still resolve to a private/internal IP on a different record. Consider resolving with { all: true } (and ideally verbatim: true) and rejecting if any returned address is private.
| const { address } = await lookup(parsedUrl.hostname); | |
| if (isPrivateIP(address)) { | |
| const addresses = await lookup(parsedUrl.hostname, { all: true, verbatim: true }); | |
| if (addresses.length === 0 || addresses.some(({ address }) => isPrivateIP(address))) { |
| // SSRF Protection: Pre-fetch DNS validation | ||
| try { | ||
| const { address } = await lookup(parsedUrl.hostname); | ||
| if (isPrivateIP(address)) { | ||
| return NextResponse.json( |
There was a problem hiding this comment.
This pre-fetch DNS check can be bypassed via redirects (fetch follows redirects by default) and DNS rebinding (fetch may resolve again later). To make the SSRF mitigation effective, disable automatic redirects and re-validate each Location hop (or use a client/dispatcher that pins the connection to the validated IP while preserving SNI/Host).
| // SSRF Protection: Pre-fetch DNS validation | ||
| try { | ||
| const { address } = await lookup(parsedUrl.hostname); |
There was a problem hiding this comment.
For SSRF hardening, explicitly allow only http:/https: URLs (reject other schemes with a clear 400) before running DNS resolution. This avoids cases like file:/empty hostname falling into the generic "Failed to resolve hostname" path.
| try { | ||
| const { address } = await lookup(parsedUrl.hostname); | ||
| if (isPrivateIP(address)) { | ||
| return NextResponse.json( | ||
| { success: false, error: 'Access to internal networks is not allowed' }, |
There was a problem hiding this comment.
Adding dns/promises lookup makes this handler (and existing unit tests) depend on real DNS/network conditions. Please mock dns/promises in unit tests and add coverage for the new SSRF behavior (e.g., 127.0.0.1/::1 rejected with 403, and lookup failures returning 400).
🚨 Severity: HIGH
💡 Vulnerability: Server-Side Request Forgery (SSRF) in
/api/lookup/url. The nativefetch()allowed querying internal networks.🎯 Impact: Attackers could access internal services, read sensitive meta-data endpoints (like
169.254.169.254on AWS), or port scan internal resources.🔧 Fix: Added
dns/promiseslookup beforefetchto validate resolved IP addresses. Reject anyisPrivateIPaddresses (127.0.0.0/8, 169.254.0.0/16, etc).✅ Verification: Ensured local queries (e.g.
http://127.0.0.1,http://[::1]) fail with403 Access to internal networks is not allowed. Existing unit tests continue to pass.PR created automatically by Jules for task 9746065586565999680 started by @aicoder2009