🛡️ Sentinel: [CRITICAL] Fix SSRF via Host header in bulk lookup#189
🛡️ Sentinel: [CRITICAL] Fix SSRF via Host header in bulk lookup#189aicoder2009 wants to merge 1 commit into
Conversation
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 bulk lookup API now invokes the URL, DOI, and ISBN lookup route handlers directly with ChangesBulk lookup direct invocation and SSRF note
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed due to a network error. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/app/api/lookup/bulk/route.ts`:
- Around line 40-43: The bulk lookup branch for bare www. inputs forwards a
scheme-less value into the URL lookup, causing the downstream validation in the
URL handler to reject it. Update the dispatch logic in the bulk route’s URL
detection block so that when trimmedItem matches www. it is normalized to a full
URL with a scheme before assigning handlerBody, while leaving already-schemed
http/https inputs unchanged. Use the existing urlLookup path and handlerBody
assignment as the place to make this normalization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e12183c-b0b4-4daf-b088-e002c77c6ede
📒 Files selected for processing (3)
.jules/sentinel.mdsrc/app/api/lookup/bulk/route.test.tssrc/app/api/lookup/bulk/route.ts
| if (trimmedItem.match(/^(https?:\/\/|www\.)/i)) { | ||
| apiEndpoint = "/api/lookup/url"; | ||
| body = { url: trimmedItem }; | ||
| handler = urlLookup; | ||
| handlerUrl = "http://localhost/api/lookup/url"; | ||
| handlerBody = { url: trimmedItem }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Bare www. inputs will fail the URL lookup.
The detector matches www. prefixes but forwards the raw trimmedItem as the url body. The URL handler validates with new URL(url), which throws on a scheme-less value like www.example.com, so these items return "Invalid URL format" instead of being looked up. Normalize the scheme before dispatching.
🐛 Proposed fix to prepend a scheme for www. inputs
if (trimmedItem.match(/^(https?:\/\/|www\.)/i)) {
handler = urlLookup;
handlerUrl = "http://localhost/api/lookup/url";
- handlerBody = { url: trimmedItem };
+ handlerBody = {
+ url: /^https?:\/\//i.test(trimmedItem) ? trimmedItem : `https://${trimmedItem}`,
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (trimmedItem.match(/^(https?:\/\/|www\.)/i)) { | |
| apiEndpoint = "/api/lookup/url"; | |
| body = { url: trimmedItem }; | |
| handler = urlLookup; | |
| handlerUrl = "http://localhost/api/lookup/url"; | |
| handlerBody = { url: trimmedItem }; | |
| if (trimmedItem.match(/^(https?:\/\/|www\.)/i)) { | |
| handler = urlLookup; | |
| handlerUrl = "http://localhost/api/lookup/url"; | |
| handlerBody = { | |
| url: /^https?:\/\//i.test(trimmedItem) ? trimmedItem : `https://${trimmedItem}`, | |
| }; |
🤖 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 40 - 43, The bulk lookup
branch for bare www. inputs forwards a scheme-less value into the URL lookup,
causing the downstream validation in the URL handler to reject it. Update the
dispatch logic in the bulk route’s URL detection block so that when trimmedItem
matches www. it is normalized to a full URL with a scheme before assigning
handlerBody, while leaving already-schemed http/https inputs unchanged. Use the
existing urlLookup path and handlerBody assignment as the place to make this
normalization.
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: The bulk lookup API (
src/app/api/lookup/bulk/route.ts) was vulnerable to Server-Side Request Forgery (SSRF) and Host Header Injection. It dynamically derived the internal base URL for sub-requests usingrequest.nextUrl.origin, which trusts the HTTPHostorX-Forwarded-Hostheader, and then performed loopbackfetchcalls to itself.🎯 Impact: An attacker could craft a malicious request with a modified
Hostheader, causing the backend server to make internal network requests to arbitrary attacker-controlled domains, potentially exposing internal data, leaking tokens, or conducting reconnaissance on the internal network.🔧 Fix: Refactored the endpoint to bypass network traversal entirely. The route now directly imports and invokes the targeted Next.js route handler functions (
urlLookup,doiLookup,isbnLookup) by passing a syntheticNextRequestobject, ensuring the execution stays strictly server-side and internal.✅ Verification: Unit tests (
route.test.ts) were fully updated to mock the imported internal module handlers instead ofglobal.fetch. Test execution (pnpm test) succeeds, confirming the route properly parses logic and calls the correct handlers without HTTP fetches.PR created automatically by Jules for task 17207393750535372451 started by @aicoder2009
Summary by CodeRabbit
Bug Fixes
Tests