Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@
**Vulnerability:** The application was using the `marked` library to parse Markdown content into HTML (in `src/app/docs/changelog/page.tsx` and `src/lib/docs.ts`) and subsequently rendering it using `dangerouslySetInnerHTML` without proper sanitization.
**Learning:** `marked` does not sanitize HTML by default. While this may seem safe for trusted inputs (like internal docs or GitHub releases), if malicious input manages to enter these sources, it leads directly to an XSS vulnerability.
**Prevention:** The output of `marked` (or any markdown parser) must always be wrapped with `DOMPurify.sanitize()` (using `isomorphic-dompurify` for SSR) before being passed to `dangerouslySetInnerHTML`.
## 2025-02-28 - [SSRF via Host-header controlled loopback requests]
**Vulnerability:** The bulk lookup API (`src/app/api/lookup/bulk/route.ts`) was deriving the base URL for internal API requests dynamically using `request.nextUrl.origin`, which relies on the `Host` or `X-Forwarded-Host` HTTP header, and then performing `fetch` requests to itself.
**Learning:** This constitutes a Server-Side Request Forgery (SSRF) and potential Host Header Injection vulnerability. If an attacker controls the `Host` header (e.g., passing a malicious domain), the application will make server-side HTTP requests to the attacker's server, leaking data or exposing internal network capabilities.
**Prevention:** When a Next.js server-side route needs to invoke logic from another internal route, never use `fetch` with dynamically derived hostnames. Instead, directly import and invoke the exported route handler functions (e.g., `POST(syntheticRequest)`) to bypass network traversal entirely and guarantee the call executes internally.
73 changes: 43 additions & 30 deletions src/app/api/lookup/bulk/route.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { POST } from './route';
import { NextRequest } from 'next/server';
import { NextRequest, NextResponse } from 'next/server';

global.fetch = vi.fn();
vi.mock('@/app/api/lookup/url/route', () => ({
POST: vi.fn(),
}));

vi.mock('@/app/api/lookup/doi/route', () => ({
POST: vi.fn(),
}));

vi.mock('@/app/api/lookup/isbn/route', () => ({
POST: vi.fn(),
}));

import { POST as urlLookup } from '@/app/api/lookup/url/route';
import { POST as doiLookup } from '@/app/api/lookup/doi/route';
import { POST as isbnLookup } from '@/app/api/lookup/isbn/route';

function makeRequest(body: object) {
return new NextRequest('http://localhost/api/lookup/bulk', {
Expand Down Expand Up @@ -54,57 +68,53 @@ describe('Bulk Lookup API', () => {
});

it('routes URLs to /api/lookup/url', async () => {
(global.fetch as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
ok: true,
json: async () => ({ data: { title: 'Example Page' } }),
});
(urlLookup as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
NextResponse.json({ data: { title: 'Example Page' } }, { status: 200 })
);
const response = await POST(makeRequest({ items: ['https://example.com'] }));
const data = await response.json();
expect(data.results[0].success).toBe(true);
expect(data.results[0].data.title).toBe('Example Page');
const [url] = (global.fetch as ReturnType<typeof vi.fn>).mock.calls[0] as [string];
expect(url).toContain('/api/lookup/url');
expect(urlLookup).toHaveBeenCalledTimes(1);
});

it('routes DOIs to /api/lookup/doi', async () => {
(global.fetch as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
ok: true,
json: async () => ({ data: { title: 'Research Article' } }),
});
(doiLookup as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
NextResponse.json({ data: { title: 'Research Article' } }, { status: 200 })
);
const response = await POST(makeRequest({ items: ['10.1000/xyz123'] }));
const data = await response.json();
expect(data.results[0].success).toBe(true);
const [url] = (global.fetch as ReturnType<typeof vi.fn>).mock.calls[0] as [string];
expect(url).toContain('/api/lookup/doi');
expect(doiLookup).toHaveBeenCalledTimes(1);
});

it('routes ISBNs to /api/lookup/isbn', async () => {
(global.fetch as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
ok: true,
json: async () => ({ data: { title: 'Book Title' } }),
});
(isbnLookup as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
NextResponse.json({ data: { title: 'Book Title' } }, { status: 200 })
);
const response = await POST(makeRequest({ items: ['9780316769174'] }));
const data = await response.json();
expect(data.results[0].success).toBe(true);
const [url] = (global.fetch as ReturnType<typeof vi.fn>).mock.calls[0] as [string];
expect(url).toContain('/api/lookup/isbn');
expect(isbnLookup).toHaveBeenCalledTimes(1);
});

it('marks item as failed when sub-request fails', async () => {
(global.fetch as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
ok: false,
json: async () => ({ error: 'Not found' }),
});
(doiLookup as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
NextResponse.json({ error: 'Not found' }, { status: 404 })
);
const response = await POST(makeRequest({ items: ['10.1000/nonexistent'] }));
const data = await response.json();
expect(data.results[0].success).toBe(false);
expect(data.results[0].error).toBe('Not found');
});

it('returns summary counts', async () => {
(global.fetch as ReturnType<typeof vi.fn>)
.mockResolvedValueOnce({ ok: true, json: async () => ({ data: { title: 'A' } }) })
.mockResolvedValueOnce({ ok: false, json: async () => ({ error: 'fail' }) });
(urlLookup as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
NextResponse.json({ data: { title: 'A' } }, { status: 200 })
);
(doiLookup as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
NextResponse.json({ error: 'fail' }, { status: 404 })
);
const response = await POST(
makeRequest({ items: ['https://success.com', '10.1000/fail'] })
);
Expand All @@ -115,9 +125,12 @@ describe('Bulk Lookup API', () => {
});

it('handles mixed item types in one batch', async () => {
(global.fetch as ReturnType<typeof vi.fn>)
.mockResolvedValueOnce({ ok: true, json: async () => ({ data: { title: 'URL result' } }) })
.mockResolvedValueOnce({ ok: true, json: async () => ({ data: { title: 'DOI result' } }) });
(urlLookup as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
NextResponse.json({ data: { title: 'URL result' } }, { status: 200 })
);
(doiLookup as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
NextResponse.json({ data: { title: 'DOI result' } }, { status: 200 })
);
const response = await POST(
makeRequest({ items: ['https://example.com', '10.1000/abc'] })
);
Expand Down
31 changes: 18 additions & 13 deletions src/app/api/lookup/bulk/route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { NextRequest, NextResponse } from "next/server";
import { POST as urlLookup } from "@/app/api/lookup/url/route";
import { POST as doiLookup } from "@/app/api/lookup/doi/route";
import { POST as isbnLookup } from "@/app/api/lookup/isbn/route";

interface LookupResult {
input: string;
Expand All @@ -22,28 +25,30 @@ export async function POST(request: NextRequest) {
}

// Refactored to process lookups concurrently for performance improvement
const baseUrl = request.nextUrl.origin;

const lookupPromises = items.map(async (item) => {
const trimmedItem = item.trim();
if (!trimmedItem) {
return { input: item, success: false, error: "Empty input" };
}

try {
let apiEndpoint: string;
let body: object;
let handler: (req: NextRequest) => Promise<NextResponse>;
let handlerUrl: string;
let handlerBody: object;

// Detect input type
if (trimmedItem.match(/^(https?:\/\/|www\.)/i)) {
apiEndpoint = "/api/lookup/url";
body = { url: trimmedItem };
handler = urlLookup;
handlerUrl = "http://localhost/api/lookup/url";
handlerBody = { url: trimmedItem };
Comment on lines 40 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
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.

} else if (trimmedItem.match(/^10\.\d{4,}/)) {
apiEndpoint = "/api/lookup/doi";
body = { doi: trimmedItem };
handler = doiLookup;
handlerUrl = "http://localhost/api/lookup/doi";
handlerBody = { doi: trimmedItem };
} else if (trimmedItem.match(/^(97[89])?\d{9}[\dXx]$/)) {
apiEndpoint = "/api/lookup/isbn";
body = { isbn: trimmedItem };
handler = isbnLookup;
handlerUrl = "http://localhost/api/lookup/isbn";
handlerBody = { isbn: trimmedItem };
} else {
return {
input: trimmedItem,
Expand All @@ -52,13 +57,13 @@ export async function POST(request: NextRequest) {
};
}

// Make the API call
const response = await fetch(`${baseUrl}${apiEndpoint}`, {
const syntheticRequest = new NextRequest(handlerUrl, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify(body),
body: JSON.stringify(handlerBody),
});

const response = await handler(syntheticRequest);
const data = await response.json();

if (response.ok && data.data) {
Expand Down
Loading