Add SSRF protection to read_file URL fetches (#587, #560)

package.json

@@ -107,6 +107,7 @@
     "file-type": "^21.1.1",
     "glob": "^10.3.10",
     "highlight.js": "^11.11.1",
+    "ipaddr.js": "^1.9.1",
     "isbinaryfile": "^5.0.4",
     "markdown-it": "^14.1.0",
     "md-to-pdf": "^5.2.5",

src/tools/filesystem.ts

@@ -11,6 +11,9 @@ import { getFileHandler, TextFileHandler } from '../utils/files/index.js';
 import type { ReadOptions, FileResult, PdfPageItem } from '../utils/files/base.js';
 import { isPdfFile } from "./mime-types.js";
 import { parsePdfToMarkdown, editPdf, PdfOperations, PdfMetadata, parseMarkdownToPdf } from './pdf/index.js';
+import { fetchUrlValidated } from '../utils/urlSafety.js';
+
+export { assertUrlIsFetchable } from '../utils/urlSafety.js';
 import { isBinaryFile } from 'isbinaryfile';
 
 // CONSTANTS SECTION - Consolidate all timeouts and thresholds
@@ -364,9 +367,9 @@ export async function readFileFromUrl(url: string): Promise<FileResult> {
     const timeoutId = setTimeout(() => controller.abort(), FILE_OPERATION_TIMEOUTS.URL_FETCH);
 
     try {
-        const response = await fetch(url, {
-            signal: controller.signal
-        });
+        // SSRF guard: validates the URL (and every redirect hop) before any
+        // request leaves the process.
+        const { response, finalUrl: currentUrl } = await fetchUrlValidated(url, controller.signal);
 
         // Clear the timeout since fetch completed
         clearTimeout(timeoutId);
@@ -378,12 +381,15 @@ export async function readFileFromUrl(url: string): Promise<FileResult> {
         // Get MIME type from Content-Type header or infer from URL
         const contentType = response.headers.get('content-type') || 'text/plain';
         const isImage = isImageFile(contentType);
-        const isPdf = isPdfFile(contentType) || url.toLowerCase().endsWith('.pdf');
+        const isPdf = isPdfFile(contentType) || currentUrl.toLowerCase().endsWith('.pdf');
 
         // NEW: Add PDF handling before image check
         if (isPdf) {
-            // Use URL directly - pdfreader handles URL downloads internally
-            const pdfResult = await parsePdfToMarkdown(url);
+            // Parse the bytes from the request we already validated. Handing the URL
+            // to the parser would download it a second time, resolving DNS again and
+            // bypassing the checks above.
+            const pdfBytes = new Uint8Array(await response.arrayBuffer());
+            const pdfResult = await parsePdfToMarkdown(pdfBytes);
 
             return {
                 content: "",

src/tools/pdf/markdown.ts

@@ -5,6 +5,7 @@ import { mdToPdf } from 'md-to-pdf';
 import type { PageRange } from './lib/pdf2md.js';
 import { PdfParseResult, pdf2md } from './lib/pdf2md.js';
 import { CONFIG_FILE } from '../../config.js';
+import { fetchUrlValidated } from '../../utils/urlSafety.js';
 
 const isUrl = (source: string): boolean =>
     source.startsWith('http://') || source.startsWith('https://');
@@ -259,9 +260,14 @@ export function ensureChromeAvailable(): void {
     });
 }
 
-async function loadPdfToBuffer(source: string): Promise<Buffer | ArrayBuffer> {
+async function loadPdfToBuffer(source: string | Uint8Array): Promise<Buffer | ArrayBuffer | Uint8Array> {
+    if (typeof source !== 'string') {
+        return source;
+    }
     if (isUrl(source)) {
-        const response = await fetch(source);
+        // Same SSRF guard as read_file's URL path — this parser must not be a
+        // way to fetch URLs that the guard would refuse.
+        const { response } = await fetchUrlValidated(source);
         return await response.arrayBuffer();
     } else {
         return await fs.readFile(source);
@@ -270,8 +276,13 @@ async function loadPdfToBuffer(source: string): Promise<Buffer | ArrayBuffer> {
 
 /**
  * Convert PDF to Markdown using @opendocsg/pdf2md
+ *
+ * Callers that already downloaded the PDF should pass the bytes rather than the
+ * URL — it avoids a second fetch, and the original request's validation stays
+ * authoritative. URL sources are fetched through the same SSRF guard as
+ * read_file.
  */
-export async function parsePdfToMarkdown(source: string, pageNumbers: number[] | PageRange = []): Promise<PdfParseResult> {
+export async function parsePdfToMarkdown(source: string | Uint8Array, pageNumbers: number[] | PageRange = []): Promise<PdfParseResult> {
     try {
         const data = await loadPdfToBuffer(source);
 

src/utils/urlSafety.ts

@@ -0,0 +1,170 @@
+import dns from 'dns/promises';
+import http from 'http';
+import https from 'https';
+import type { LookupFunction } from 'net';
+import ipaddr from 'ipaddr.js';
+import fetch from 'cross-fetch';
+
+// Follow at most this many redirects when reading a URL, re-validating each hop.
+const MAX_URL_REDIRECTS = 5;
+
+/**
+ * Returns true if an IP address is not publicly routable — loopback, private,
+ * link-local (including the 169.254.169.254 cloud metadata endpoint), unique
+ * local, CGNAT, multicast, or otherwise reserved. Anything that isn't a valid
+ * IP is treated as blocked so a malformed value can't slip through.
+ *
+ * Uses ipaddr.js for CIDR-correct classification across IPv4 and IPv6: only the
+ * `unicast` range is publicly routable, everything else is refused. IPv4-mapped
+ * IPv6 (e.g. `::ffff:127.0.0.1` and its hex form `::ffff:7f00:1`) is unwrapped
+ * to its IPv4 address first so the underlying range is what gets checked.
+ */
+function isBlockedAddress(ip: string): boolean {
+    let addr: ipaddr.IPv4 | ipaddr.IPv6;
+    try {
+        addr = ipaddr.parse(ip.split('%')[0]); // drop any IPv6 zone id
+    } catch {
+        return true;
+    }
+    if (addr.kind() === 'ipv6' && (addr as ipaddr.IPv6).isIPv4MappedAddress()) {
+        addr = (addr as ipaddr.IPv6).toIPv4Address();
+    }
+    return addr.range() !== 'unicast';
+}
+
+/** An address that passed the SSRF checks, in `dns.lookup` shape. */
+export interface ValidatedAddress {
+    address: string;
+    family: number;
+}
+
+/**
+ * Guards against SSRF before a URL is fetched. Allows only http(s), and rejects
+ * hosts that are — or that resolve to — non-public addresses. Every resolved
+ * address is checked, so a hostname pointing at an internal IP is blocked too.
+ *
+ * @param rawUrl The URL about to be fetched
+ * @returns The addresses the host was validated as — connect to these rather
+ *          than resolving the hostname again, or the second lookup can be
+ *          rebound to an internal address after the check passed
+ * @throws Error if the URL is not safe to fetch
+ */
+export async function assertUrlIsFetchable(rawUrl: string): Promise<ValidatedAddress[]> {
+    let parsed: URL;
+    try {
+        parsed = new URL(rawUrl);
+    } catch {
+        throw new Error(`Invalid URL: ${rawUrl}`);
+    }
+
+    if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
+        throw new Error(`URL protocol not allowed: ${parsed.protocol} — only http and https can be read`);
+    }
+
+    const hostname = parsed.hostname.replace(/^\[|\]$/g, ''); // strip IPv6 brackets
+
+    if (ipaddr.isValid(hostname)) {
+        if (isBlockedAddress(hostname)) {
+            throw new Error(`Refusing to fetch a URL that targets a non-public address: ${hostname}`);
+        }
+        return [{ address: hostname, family: ipaddr.parse(hostname.split('%')[0]).kind() === 'ipv6' ? 6 : 4 }];
+    }
+
+    let resolved: Array<{ address: string; family: number }>;
+    try {
+        resolved = await dns.lookup(hostname, { all: true });
+    } catch {
+        throw new Error(`Could not resolve host: ${hostname}`);
+    }
+    if (resolved.length === 0) {
+        throw new Error(`Could not resolve host: ${hostname}`);
+    }
+    for (const { address } of resolved) {
+        if (isBlockedAddress(address)) {
+            throw new Error(
+                `Refusing to fetch a URL that resolves to a non-public address: ${hostname} -> ${address}`
+            );
+        }
+    }
+    return resolved.map(({ address, family }) => ({ address, family }));
+}
+
+/**
+ * An http(s) agent that connects only to the given pre-validated addresses
+ * instead of resolving the hostname again. The socket-level lookup re-resolving
+ * the name is what makes DNS rebinding work: a host can pass the guard and then
+ * serve an internal address to the connection's own lookup. TLS SNI and
+ * certificate checks still run against the original hostname.
+ */
+function pinnedAgentFor(url: URL, addresses: ValidatedAddress[]): http.Agent {
+    const lookup: LookupFunction = (_hostname, options, callback) => {
+        if (options.all) {
+            callback(null, addresses.map(({ address, family }) => ({ address, family })));
+        } else {
+            callback(null, addresses[0].address, addresses[0].family);
+        }
+    };
+    return url.protocol === 'https:' ? new https.Agent({ lookup }) : new http.Agent({ lookup });
+}
+
+/**
+ * Releases a response we are not going to return (a redirect hop), so its
+ * unread body does not hold the connection open. Handles both body shapes:
+ * a WHATWG stream (`cancel`) and node-fetch's Node stream (`destroy`).
+ */
+function discardResponseBody(response: Awaited<ReturnType<typeof fetch>>): void {
+    const body = response.body as { cancel?: () => Promise<void>; destroy?: () => void } | null;
+    if (body && typeof body.cancel === 'function') {
+        body.cancel().catch(() => {});
+    } else if (body && typeof body.destroy === 'function') {
+        body.destroy();
+    }
+}
+
+/**
+ * Fetches a URL with the SSRF guard applied to the initial request and to every
+ * redirect hop — a public URL must not be able to redirect us onto an internal
+ * address. Each hop's socket connects to the addresses the guard validated
+ * (never re-resolving the hostname), closing the DNS-rebinding window between
+ * check and connect. All code that downloads a user-supplied URL must go
+ * through this (or pass already-downloaded bytes onward) rather than calling
+ * fetch directly.
+ *
+ * @param fetchImpl Overridable for hermetic tests; defaults to cross-fetch
+ * @returns The final response and the URL it actually came from
+ */
+export async function fetchUrlValidated(
+    url: string,
+    signal?: AbortSignal,
+    fetchImpl: typeof fetch = fetch
+): Promise<{ response: Awaited<ReturnType<typeof fetch>>; finalUrl: string }> {
+    let currentUrl = url;
+    let response!: Awaited<ReturnType<typeof fetch>>;
+    for (let hop = 0; ; hop++) {
+        const addresses = await assertUrlIsFetchable(currentUrl);
+        const parsed = new URL(currentUrl);
+        response = await fetchImpl(currentUrl, {
+            signal,
+            redirect: 'manual',
+            // cross-fetch is node-fetch on Node, which connects through this
+            // agent; runtimes with WHATWG fetch ignore the extra field.
+            agent: pinnedAgentFor(parsed, addresses)
+        } as RequestInit);
+
+        if (response.status < 300 || response.status >= 400) {
+            break;
+        }
+
+        const location = response.headers.get('location');
+        if (!location) {
+            break; // redirect status without a target — treat as final
+        }
+        discardResponseBody(response); // this hop's body is never read
+        if (hop >= MAX_URL_REDIRECTS) {
+            throw new Error(`Too many redirects while fetching URL: ${url}`);
+        }
+        currentUrl = new URL(location, currentUrl).toString();
+    }
+
+    return { response, finalUrl: currentUrl };
+}

test/test-read-url-ssrf.js

@@ -0,0 +1,127 @@
+import assert from 'assert';
+import { assertUrlIsFetchable, readFileFromUrl } from '../dist/tools/filesystem.js';
+import { fetchUrlValidated } from '../dist/utils/urlSafety.js';
+import { parsePdfToMarkdown } from '../dist/tools/pdf/index.js';
+
+/**
+ * Regression tests for #587 / #560: read_file with isUrl fetched arbitrary URLs
+ * with no SSRF protection, so a URL like http://169.254.169.254/… could read
+ * cloud instance metadata or reach internal services.
+ *
+ * assertUrlIsFetchable() now rejects non-http(s) schemes and any host that is
+ * (or resolves to) a non-public address. These cases short-circuit before any
+ * network access, so the test is hermetic — no outbound requests are made.
+ */
+
+let passed = 0;
+const ok = (msg) => { passed++; console.log(`✓ ${msg}`); };
+
+// A minimal single-page PDF, inlined so the PDF check needs no fixture or network.
+const MINIMAL_PDF_BASE64 =
+  'JVBERi0xLjQKMSAwIG9iago8PCAvVHlwZSAvQ2F0YWxvZyAvUGFnZXMgMiAwIFIgPj4KZW5kb2JqCjIgMCBvYmoK' +
+  'PDwgL1R5cGUgL1BhZ2VzIC9LaWRzIFszIDAgUl0gL0NvdW50IDEgPj4KZW5kb2JqCjMgMCBvYmoKPDwgL1R5cGUg' +
+  'L1BhZ2UgL1BhcmVudCAyIDAgUiAvTWVkaWFCb3ggWzAgMCAyMDAgMjAwXSAvUmVzb3VyY2VzIDw8IC9Gb250IDw8' +
+  'IC9GMSA0IDAgUiA+PiA+PiAvQ29udGVudHMgNSAwIFIgPj4KZW5kb2JqCjQgMCBvYmoKPDwgL1R5cGUgL0ZvbnQg' +
+  'L1N1YnR5cGUgL1R5cGUxIC9CYXNlRm9udCAvSGVsdmV0aWNhID4+CmVuZG9iago1IDAgb2JqCjw8IC9MZW5ndGgg' +
+  'MzMgPj4Kc3RyZWFtCkJUIC9GMSAyNCBUZiAyMCAxMDAgVGQgKEhpKSBUaiBFVAplbmRzdHJlYW0KZW5kb2JqCnhy' +
+  'ZWYKMCA2CjAwMDAwMDAwMDAgNjU1MzUgZiAKMDAwMDAwMDAwOSAwMDAwMCBuIAowMDAwMDAwMDU4IDAwMDAwIG4g' +
+  'CjAwMDAwMDAxMTUgMDAwMDAgbiAKMDAwMDAwMDI0MSAwMDAwMCBuIAowMDAwMDAwMzExIDAwMDAwIG4gCnRyYWls' +
+  'ZXIKPDwgL1NpemUgNiAvUm9vdCAxIDAgUiA+PgpzdGFydHhyZWYKMzk0CiUlRU9GCg==';
+
+const BLOCKED_URLS = [
+  'http://169.254.169.254/latest/meta-data/iam/security-credentials/', // AWS metadata
+  'http://[fd00::1]/',                                                  // IPv6 unique-local
+  'http://127.0.0.1:8080/admin',                                        // loopback
+  'http://localhost/internal',                                         // resolves to loopback
+  'http://10.0.0.1/',                                                   // private
+  'http://192.168.1.1/',                                               // private
+  'http://172.16.5.4/',                                                // private
+  'http://[::1]/',                                                     // IPv6 loopback
+  'http://[fe90::1]/',                                                 // IPv6 link-local (fe80::/10, not just fe80::)
+  'http://[::ffff:7f00:1]/',                                           // IPv4-mapped loopback in hex form (127.0.0.1)
+  'file:///etc/passwd',                                                // non-http scheme
+  'ftp://example.com/secret',                                          // non-http scheme
+];
+
+async function run() {
+  for (const url of BLOCKED_URLS) {
+    await assert.rejects(
+      () => assertUrlIsFetchable(url),
+      (err) => err instanceof Error && /not allowed|non-public|Invalid URL|resolve/i.test(err.message),
+      `assertUrlIsFetchable should reject ${url}`
+    );
+  }
+  ok(`assertUrlIsFetchable rejects all ${BLOCKED_URLS.length} SSRF / non-http URLs`);
+
+  // readFile(isUrl) routes through the same guard, so the metadata PoC is
+  // blocked before any request leaves the process.
+  await assert.rejects(
+    () => readFileFromUrl('http://169.254.169.254/latest/meta-data/'),
+    /non-public/i,
+    'readFileFromUrl should refuse the cloud metadata endpoint'
+  );
+  ok('readFileFromUrl refuses the cloud metadata endpoint');
+
+  // A public address with a valid scheme passes validation. An IP literal is
+  // used so the check needs no outbound DNS and stays hermetic; no request is
+  // made — we only assert the guard itself does not reject it.
+  await assert.doesNotReject(
+    () => assertUrlIsFetchable('https://8.8.8.8/file.txt'),
+    'a public https URL must pass the guard'
+  );
+  ok('a public https address passes the guard');
+
+  // readFileFromUrl hands the PDF parser the bytes it already fetched through the
+  // guard; passing the URL made the parser download it a second time, re-resolving
+  // DNS outside these checks. That requires the parser to accept bytes — it used
+  // to treat every source as a path or URL.
+  const parsed = await parsePdfToMarkdown(Buffer.from(MINIMAL_PDF_BASE64, 'base64'));
+  assert.strictEqual(parsed.metadata.totalPages, 1, 'byte input should parse as a PDF');
+  ok('parsePdfToMarkdown accepts already-fetched bytes');
+
+  // A public URL must not be able to redirect us onto an internal address.
+  // The fetch is stubbed (fetchUrlValidated takes the implementation as its
+  // third parameter precisely so this stays hermetic): the stub answers the
+  // public entry URL with a 302 pointing at cloud metadata, and records every
+  // request it is asked to make. An IP-literal entry URL keeps validation off
+  // the real DNS resolver.
+  const requested = [];
+  let capturedInit;
+  const redirectingFetch = async (url, init) => {
+    requested.push(String(url));
+    capturedInit = init;
+    return {
+      status: 302,
+      headers: { get: (name) => (name.toLowerCase() === 'location' ? 'http://169.254.169.254/latest/meta-data/' : null) },
+      body: null,
+    };
+  };
+  await assert.rejects(
+    () => fetchUrlValidated('https://8.8.8.8/entry', undefined, redirectingFetch),
+    /non-public/i,
+    'a redirect onto the metadata endpoint must be rejected'
+  );
+  assert.deepStrictEqual(
+    requested,
+    ['https://8.8.8.8/entry'],
+    'the internal redirect target must never be requested'
+  );
+  ok('redirect from a public URL to the metadata endpoint is blocked before any request to it');
+
+  // The connection must go to the address that passed validation, not wherever
+  // the hostname resolves at connect time (DNS rebinding). The fetch options
+  // carry an agent whose lookup answers from the validated set; ask it directly
+  // and confirm it returns the pinned address without touching the resolver.
+  assert.ok(capturedInit && capturedInit.agent, 'fetch should be given a pinning agent');
+  const pinned = await new Promise((resolve, reject) => {
+    capturedInit.agent.options.lookup('rebind.example', {}, (err, address, family) => {
+      if (err) reject(err); else resolve({ address, family });
+    });
+  });
+  assert.strictEqual(pinned.address, '8.8.8.8', 'socket lookup must return the validated address');
+  ok('socket-level lookup is pinned to the validated address');
+}
+
+run()
+  .then(() => { console.log(`\nPASS (${passed}/6)`); process.exit(0); })
+  .catch((e) => { console.error(`\nFAIL: ${e.message}`); process.exit(1); });