🛡️ Sentinel: [HIGH] Fix Host-Header SSRF in bulk lookup API#146
🛡️ Sentinel: [HIGH] Fix Host-Header SSRF in bulk lookup API#146aicoder2009 wants to merge 1 commit into
Conversation
Removed unsafe loopback fetch() calls dependent on the user-controllable Host header (`request.nextUrl.origin`) in the bulk lookup API. Instead, directly imports and invokes the internal Next.js route handlers with a safe, synthetic `NextRequest` object (`http://localhost`). This eliminates the possibility of manipulating the host header to execute unintended Server-Side Request Forgery (SSRF) requests to internal infrastructure. Co-authored-by: aicoder2009 <127642633+aicoder2009@users.noreply.github.com>
|
👋 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.
|
📝 WalkthroughWalkthroughThe PR mitigates a Host-Header SSRF vulnerability in the bulk lookup API by replacing unsafe fetch-based loopback requests with direct handler invocations using synthetic ChangesSSRF Vulnerability Mitigation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR mitigates a Host-header SSRF vulnerability in the bulk lookup API by removing loopback fetch() calls that relied on request.nextUrl.origin (user-controlled via Host) and instead directly invoking the internal lookup route handlers with a fixed-origin synthetic NextRequest.
Changes:
- Replaced loopback HTTP
fetch()calls in the bulk lookup route with direct imports/invocations of the URL/DOI/ISBN routePOSThandlers. - Constructed a synthetic
NextRequestbound tohttp://localhostfor internal handler calls to eliminateHostheader influence. - Updated bulk lookup tests to mock the imported handlers instead of mocking
global.fetch.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/app/api/lookup/bulk/route.ts | Removes loopback fetch(request.nextUrl.origin + ...) and directly calls internal lookup handlers to prevent Host-header SSRF. |
| src/app/api/lookup/bulk/route.test.ts | Updates tests to mock and assert direct handler invocation instead of mocking fetch. |
| .jules/sentinel.md | Documents the Host-header SSRF vulnerability pattern and the prevention approach used here. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/api/lookup/bulk/route.ts (1)
57-60: ⚡ Quick winsrc/app/api/lookup/bulk/route.ts: Content-Type for synthetic JSON request
- In this repo’s existing API tests,
new NextRequest(...)is constructed with a JSON-stringifiedbodywithout settingContent-Type, indicatingreq.json()parsing doesn’t rely on it.- Add
Content-Type: application/jsononly if the downstream lookup handlers explicitly readreq.headers.get("content-type").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/lookup/bulk/route.ts` around lines 57 - 60, The synthetic NextRequest constructed as syntheticRequest = new NextRequest(...) is missing an explicit Content-Type header; if any downstream lookup handlers read req.headers.get("content-type") (or branch logic depends on that header) then update the NextRequest init to include headers: { "content-type": "application/json" } so req.json() consumers see the correct MIME type, otherwise leave the existing construction unchanged—check the lookup handler functions that consume the request to decide.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/app/api/lookup/bulk/route.ts`:
- Around line 57-60: The synthetic NextRequest constructed as syntheticRequest =
new NextRequest(...) is missing an explicit Content-Type header; if any
downstream lookup handlers read req.headers.get("content-type") (or branch logic
depends on that header) then update the NextRequest init to include headers: {
"content-type": "application/json" } so req.json() consumers see the correct
MIME type, otherwise leave the existing construction unchanged—check the lookup
handler functions that consume the request to decide.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 29b7c13f-39fc-4703-bc79-9424f11462c3
📒 Files selected for processing (3)
.jules/sentinel.mdsrc/app/api/lookup/bulk/route.test.tssrc/app/api/lookup/bulk/route.ts
🛡️ Sentinel: [HIGH] Fix Host-Header SSRF in bulk lookup API
🚨 Severity: HIGH
💡 Vulnerability: The
/api/lookup/bulk/route.tsAPI endpoint previously executed bulk requests by performing a loopbackfetch()against its own dynamically generated origin (request.nextUrl.origin). This relies directly on the HTTPHostheader provided by the user.🎯 Impact: An attacker could manipulate the
Hostheader to point to internal IP addresses or private network endpoints. The Next.js server would then execute thefetchrequest against that internal target, leading to a Server-Side Request Forgery (SSRF) vulnerability.🔧 Fix: Refactored the route to directly import the internal
POSThandlers (url/route.ts,doi/route.ts,isbn/route.ts) and invoke them directly using a safe, syntheticNextRequestbound tohttp://localhost. This eliminates the loopback HTTP request entirely and removes any reliance on theHostheader.✅ Verification: Verified by updating tests (
src/app/api/lookup/bulk/route.test.ts) to mock the direct POST handler invocations instead ofglobal.fetch, ensuring the core logic remains covered and regressions are avoided.PR created automatically by Jules for task 8579163770110542801 started by @aicoder2009
Summary by CodeRabbit
Bug Fixes
New Features