Skip to content

Commit f5a947e

Browse files
ihor-sokoliukclaudecodex
committed
security(url-reader): enforce size limit via streaming body cap (SEC-022)
web_url_read's size limit was advisory only: the HEAD Content-Length preflight is non-fatal, so a server using chunked encoding, a failing/absent HEAD, or a GET body larger than its reported Content-Length could make the unbounded `await response.text()` buffer the whole body into memory (DoS). Read the response body via a bounded stream that counts decompressed bytes and cancels the reader once URL_READ_MAX_CONTENT_LENGTH_BYTES is exceeded, returning the existing content-too-large message before any conversion or cache write. The same bounded read caps the !response.ok error-body snippet. HEAD preflight is kept as a cheap early-out; the streaming cap is authoritative. Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
1 parent 14575ea commit f5a947e

4 files changed

Lines changed: 255 additions & 3 deletions

File tree

CONFIGURATION.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Self-hosting SearXNG with JSON output enabled remains the recommended setup. The
5757
| Variable | Required | Default | Description |
5858
|---|---|---|---|
5959
| `URL_READ_MAX_CHARS` | No || Default maximum characters returned by `web_url_read` when the caller omits `maxLength`. Explicit `maxLength` always wins. Invalid values are ignored. |
60-
| `URL_READ_MAX_CONTENT_LENGTH_BYTES` | No | `5242880` | Maximum `Content-Length` allowed by the `web_url_read` HEAD preflight before downloading a page. Invalid values fall back to the default. HEAD failures are non-fatal and the GET proceeds. |
60+
| `URL_READ_MAX_CONTENT_LENGTH_BYTES` | No | `5242880` | Maximum decompressed response-body bytes `web_url_read` will read while streaming a page. A HEAD `Content-Length` preflight may reject oversized pages before GET, but the streaming cap is authoritative. Invalid values fall back to the default. |
6161
| `CACHE_TTL_MS` | No | `86400000` | URL cache TTL in milliseconds. Invalid or non-positive values fall back to the default (24 hours). |
6262
| `CACHE_MAX_ENTRIES` | No | `500` | Maximum number of cached URLs. When the cache exceeds this size, the least frequently used entry is evicted, with oldest entry used as the tie-breaker. Invalid or non-positive values fall back to the default. |
6363

@@ -129,6 +129,8 @@ For direct URL-reader requests without a proxy, DNS answers are validated before
129129

130130
When a URL-reader proxy is configured (`URL_READER_HTTP_PROXY`, `URL_READER_HTTPS_PROXY`, `HTTP_PROXY`, or `HTTPS_PROXY`), the proxy performs DNS resolution. Client-side DNS-answer validation cannot inspect proxied resolutions, so proxied deployments should rely on proxy, firewall, and egress controls.
131131

132+
`URL_READ_MAX_CONTENT_LENGTH_BYTES` is enforced while streaming the response body, including chunked responses and responses whose GET body is larger than the HEAD `Content-Length` value. The limit is measured after transparent response decompression.
133+
132134
Set `MCP_HTTP_ALLOW_PRIVATE_URLS=true` only when internal URL reads are intentional for your deployment. This also allows hostnames that DNS-resolve to private/internal addresses.
133135

134136

SECURITY.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,10 @@ The server auto-detects system CA bundles on Linux and macOS for outbound HTTPS
9494

9595
The `web_url_read` tool manually follows redirects (up to 5 hops). Each intermediate URL is validated against the private-IP blocklist before the request is made. On the direct no-proxy path, each redirect hop also goes through DNS-answer validation before connecting.
9696

97+
### URL Reader Size Limits
98+
99+
`web_url_read` enforces `URL_READ_MAX_CONTENT_LENGTH_BYTES` while streaming the response body. The HEAD `Content-Length` check remains as a cheap early rejection path, but the streaming cap is authoritative and also applies when the server omits `Content-Length`, uses chunked transfer encoding, or sends more data than it reported. The cap is measured after undici's transparent Content-Encoding decompression, which bounds the in-memory content size used for HTML-to-Markdown conversion.
100+
97101
## Deployment Recommendations
98102

99103
### Minimal / Local

__tests__/unit/url-reader.test.ts

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,194 @@ async function runTests() {
392392
}
393393
}, results);
394394

395+
await testFunction('Streaming GET body without Content-Length is capped', async () => {
396+
const mockServer = createMockServer();
397+
urlCache.clear();
398+
envManager.set('URL_READ_MAX_CONTENT_LENGTH_BYTES', '64');
399+
400+
const seenMethods: string[] = [];
401+
const { url, close } = await startHttpServer((req, res) => {
402+
seenMethods.push(req.method || 'UNKNOWN');
403+
if (req.method === 'HEAD') {
404+
res.writeHead(200);
405+
res.end();
406+
return;
407+
}
408+
409+
res.writeHead(200, { 'content-type': 'text/html; charset=utf-8' });
410+
res.write('<html><body><h1>Chunked</h1>');
411+
res.write('x'.repeat(128));
412+
res.end('</body></html>');
413+
});
414+
415+
try {
416+
const result = await fetchAndConvertToMarkdown(mockServer as any, url);
417+
assert.ok(result.includes('Content too large'), `Expected content-too-large message, got: ${result}`);
418+
assert.deepEqual(seenMethods, ['HEAD', 'GET']);
419+
} finally {
420+
await close();
421+
envManager.restore();
422+
urlCache.clear();
423+
}
424+
}, results);
425+
426+
await testFunction('Streaming GET body with understated HEAD Content-Length is capped', async () => {
427+
const mockServer = createMockServer();
428+
urlCache.clear();
429+
envManager.set('URL_READ_MAX_CONTENT_LENGTH_BYTES', '64');
430+
431+
const seenMethods: string[] = [];
432+
const { url, close } = await startHttpServer((req, res) => {
433+
seenMethods.push(req.method || 'UNKNOWN');
434+
if (req.method === 'HEAD') {
435+
res.writeHead(200, { 'content-length': '10' });
436+
res.end();
437+
return;
438+
}
439+
440+
res.writeHead(200, { 'content-type': 'text/html; charset=utf-8' });
441+
res.write('<html><body><h1>Understated</h1>');
442+
res.write('x'.repeat(128));
443+
res.end('</body></html>');
444+
});
445+
446+
try {
447+
const result = await fetchAndConvertToMarkdown(mockServer as any, url);
448+
assert.ok(result.includes('Content too large'), `Expected content-too-large message, got: ${result}`);
449+
assert.deepEqual(seenMethods, ['HEAD', 'GET']);
450+
} finally {
451+
await close();
452+
envManager.restore();
453+
urlCache.clear();
454+
}
455+
}, results);
456+
457+
await testFunction('Streaming GET body just under limit is returned in full', async () => {
458+
const mockServer = createMockServer();
459+
urlCache.clear();
460+
envManager.set('URL_READ_MAX_CONTENT_LENGTH_BYTES', '128');
461+
462+
const html = '<html><body><h1>Within Limit</h1><p>Complete body.</p></body></html>';
463+
const { url, close } = await startHttpServer((req, res) => {
464+
if (req.method === 'HEAD') {
465+
res.writeHead(200);
466+
res.end();
467+
return;
468+
}
469+
470+
res.writeHead(200, { 'content-type': 'text/html; charset=utf-8' });
471+
res.end(html);
472+
});
473+
474+
try {
475+
assert.ok(Buffer.byteLength(html, 'utf8') < 128, 'Test body must stay below configured cap');
476+
const result = await fetchAndConvertToMarkdown(mockServer as any, url);
477+
assert.ok(result.includes('Within Limit'), `Expected converted content, got: ${result}`);
478+
assert.ok(result.includes('Complete body'), `Expected complete converted body, got: ${result}`);
479+
} finally {
480+
await close();
481+
envManager.restore();
482+
urlCache.clear();
483+
}
484+
}, results);
485+
486+
await testFunction('Over-limit streaming GET result is not cached', async () => {
487+
const mockServer = createMockServer();
488+
urlCache.clear();
489+
envManager.set('URL_READ_MAX_CONTENT_LENGTH_BYTES', '64');
490+
491+
let headCount = 0;
492+
let getCount = 0;
493+
const { url, close } = await startHttpServer((req, res) => {
494+
if (req.method === 'HEAD') {
495+
headCount++;
496+
res.writeHead(200);
497+
res.end();
498+
return;
499+
}
500+
501+
getCount++;
502+
res.writeHead(200, { 'content-type': 'text/html; charset=utf-8' });
503+
res.write('<html><body><h1>Not cached</h1>');
504+
res.write('x'.repeat(128));
505+
res.end('</body></html>');
506+
});
507+
508+
try {
509+
const first = await fetchAndConvertToMarkdown(mockServer as any, url);
510+
const second = await fetchAndConvertToMarkdown(mockServer as any, url);
511+
512+
assert.ok(first.includes('Content too large'), `Expected first read to be capped, got: ${first}`);
513+
assert.ok(second.includes('Content too large'), `Expected second read to be capped, got: ${second}`);
514+
assert.equal(headCount, 2, 'Second over-limit read should repeat HEAD instead of using cache');
515+
assert.equal(getCount, 2, 'Second over-limit read should re-fetch instead of using cache');
516+
} finally {
517+
await close();
518+
envManager.restore();
519+
urlCache.clear();
520+
}
521+
}, results);
522+
523+
await testFunction('Oversized HTTP error response body is capped', async () => {
524+
const mockServer = createMockServer();
525+
urlCache.clear();
526+
envManager.set('URL_READ_MAX_CONTENT_LENGTH_BYTES', '64');
527+
528+
let chunksWritten = 0;
529+
let responseClosed = false;
530+
let resolveResponseClosed: () => void = () => {};
531+
const responseClosedPromise = new Promise<void>((resolve) => {
532+
resolveResponseClosed = resolve;
533+
});
534+
const totalChunks = 100;
535+
const { url, close } = await startHttpServer((req, res) => {
536+
if (req.method === 'HEAD') {
537+
res.writeHead(500);
538+
res.end();
539+
return;
540+
}
541+
542+
res.writeHead(500, { 'content-type': 'text/plain; charset=utf-8' });
543+
res.on('close', () => {
544+
responseClosed = true;
545+
resolveResponseClosed();
546+
});
547+
548+
const writeNext = () => {
549+
if (res.destroyed || chunksWritten >= totalChunks) {
550+
res.end();
551+
return;
552+
}
553+
554+
chunksWritten++;
555+
res.write('x'.repeat(32));
556+
setImmediate(writeNext);
557+
};
558+
559+
writeNext();
560+
});
561+
562+
try {
563+
await fetchAndConvertToMarkdown(mockServer as any, url);
564+
assert.fail('Expected server error');
565+
} catch (error: any) {
566+
assert.ok(
567+
error.message.includes('Website Error (500)') || error.name === 'MCPSearXNGError',
568+
`Expected server error, got: ${error.message}`,
569+
);
570+
await Promise.race([
571+
responseClosedPromise,
572+
new Promise<void>((resolve) => setTimeout(resolve, 50)),
573+
]);
574+
assert.ok(responseClosed, 'Expected response stream to be closed');
575+
assert.ok(chunksWritten < totalChunks, `Expected capped error-body read, wrote all ${chunksWritten} chunks`);
576+
} finally {
577+
await close();
578+
envManager.restore();
579+
urlCache.clear();
580+
}
581+
}, results);
582+
395583
await testFunction('Invalid URL_READ_MAX_CONTENT_LENGTH_BYTES falls back to default cap', async () => {
396584
const mockServer = createMockServer();
397585
urlCache.clear();

src/url-reader.ts

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ interface PaginationOptions {
2626
readHeadings?: boolean;
2727
}
2828

29+
type BoundedBodyReadResult =
30+
| { exceeded: false; text: string; bytesRead: number }
31+
| { exceeded: true; bytesRead: number };
32+
2933
const REDIRECT_STATUS_CODES = new Set([301, 302, 303, 307, 308]);
3034
const MAX_REDIRECTS = 5;
3135
export const DEFAULT_MAX_CONTENT_LENGTH_BYTES = 5 * 1024 * 1024;
@@ -224,6 +228,53 @@ function createContentTooLargeMessage(contentLength: number, maxBytes: number):
224228
);
225229
}
226230

231+
function concatenateChunks(chunks: Uint8Array[], totalBytes: number): Uint8Array {
232+
const result = new Uint8Array(totalBytes);
233+
let offset = 0;
234+
235+
for (const chunk of chunks) {
236+
result.set(chunk, offset);
237+
offset += chunk.byteLength;
238+
}
239+
240+
return result;
241+
}
242+
243+
async function readResponseBodyWithLimit(response: Response, maxBytes: number): Promise<BoundedBodyReadResult> {
244+
if (response.body === null) {
245+
return { exceeded: false, text: "", bytesRead: 0 };
246+
}
247+
248+
const reader = response.body.getReader();
249+
const chunks: Uint8Array[] = [];
250+
let bytesRead = 0;
251+
252+
try {
253+
while (true) {
254+
const { done, value } = await reader.read();
255+
if (done) {
256+
break;
257+
}
258+
if (!value) {
259+
continue;
260+
}
261+
262+
bytesRead += value.byteLength;
263+
if (bytesRead > maxBytes) {
264+
await reader.cancel();
265+
return { exceeded: true, bytesRead };
266+
}
267+
268+
chunks.push(value);
269+
}
270+
} finally {
271+
reader.releaseLock();
272+
}
273+
274+
const bodyBytes = concatenateChunks(chunks, bytesRead);
275+
return { exceeded: false, text: new TextDecoder("utf-8").decode(bodyBytes), bytesRead };
276+
}
277+
227278
export async function fetchAndConvertToMarkdown(
228279
mcpServer: McpServer,
229280
url: string,
@@ -345,7 +396,10 @@ export async function fetchAndConvertToMarkdown(
345396
if (!response.ok) {
346397
let responseBody: string;
347398
try {
348-
responseBody = await response.text();
399+
const bodyRead = await readResponseBodyWithLimit(response, maxContentLengthBytes);
400+
responseBody = bodyRead.exceeded
401+
? createContentTooLargeMessage(bodyRead.bytesRead, maxContentLengthBytes)
402+
: bodyRead.text;
349403
} catch {
350404
responseBody = '[Could not read response body]';
351405
}
@@ -357,7 +411,11 @@ export async function fetchAndConvertToMarkdown(
357411
// Retrieve HTML content
358412
let htmlContent: string;
359413
try {
360-
htmlContent = await response.text();
414+
const bodyRead = await readResponseBodyWithLimit(response, maxContentLengthBytes);
415+
if (bodyRead.exceeded) {
416+
return createContentTooLargeMessage(bodyRead.bytesRead, maxContentLengthBytes);
417+
}
418+
htmlContent = bodyRead.text;
361419
} catch (error: any) {
362420
throw createContentError(
363421
`Failed to read website content: ${error.message || 'Unknown error reading content'}`,

0 commit comments

Comments
 (0)