diff --git a/src/everything/__tests__/tools.test.ts b/src/everything/__tests__/tools.test.ts index a50bbd6592..030c729412 100644 --- a/src/everything/__tests__/tools.test.ts +++ b/src/everything/__tests__/tools.test.ts @@ -1217,5 +1217,199 @@ describe('Tools', () => { handler!({ name: 'test.gz', data: 'ftp://example.com/file.txt', outputType: 'resource' }) ).rejects.toThrow('Unsupported URL protocol'); }); + + it('should re-check GZIP_ALLOWED_DOMAINS on redirect targets', async () => { + vi.stubEnv('GZIP_ALLOWED_DOMAINS', 'allowed.example'); + vi.resetModules(); + + const fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const href = String(input); + const redirect = init?.redirect ?? 'follow'; + if (href === 'https://allowed.example/file') { + if (redirect === 'follow') { + // Pre-fix behavior would auto-follow into a non-allowed host. + return new Response('SECRET_FROM_PRIVATE_HOST', { status: 200 }); + } + return new Response(null, { + status: 302, + headers: { Location: 'http://127.0.0.1/secret' }, + }); + } + if (href === 'http://127.0.0.1/secret') { + return new Response('SECRET_FROM_PRIVATE_HOST', { status: 200 }); + } + throw new Error(`unexpected fetch: ${href}`); + }); + vi.stubGlobal('fetch', fetchMock); + + const { registerGZipFileAsResourceTool: registerFresh } = await import( + '../tools/gzip-file-as-resource.js' + ); + + const mockServer = { + registerTool: vi.fn(), + registerResource: vi.fn(), + } as unknown as McpServer; + + let handler: Function | null = null; + (mockServer.registerTool as any).mockImplementation( + (name: string, config: any, h: Function) => { + handler = h; + } + ); + + registerFresh(mockServer); + + await expect( + handler!({ + name: 'test.gz', + data: 'https://allowed.example/file', + outputType: 'resource', + }) + ).rejects.toThrow('not in the allowed domains list'); + + const fetched = fetchMock.mock.calls.map((c) => String(c[0])); + expect(fetched).toEqual(['https://allowed.example/file']); + expect(fetched).not.toContain('http://127.0.0.1/secret'); + + vi.unstubAllGlobals(); + vi.unstubAllEnvs(); + }); + + it('re-checks the allowlist when redirect hops change address representation', async () => { + vi.stubEnv('GZIP_ALLOWED_DOMAINS', 'allowed.example'); + vi.resetModules(); + + const redirect302 = (location: string) => + new Response(null, { status: 302, headers: { Location: location } }); + + // Each scenario: an allowed start URL whose redirect chain ends at a + // destination that is not in GZIP_ALLOWED_DOMAINS. The representation + // changes per hop while the policy decision must not. + const scenarios: Array<{ + name: string; + start: string; + hops: Record; + forbiddenMarker: string; + }> = [ + { + name: 'hostname to IPv4 literal', + start: 'https://allowed.example/to-ipv4', + hops: { 'https://allowed.example/to-ipv4': 'https://93.184.216.34/secret' }, + forbiddenMarker: '93.184.216.34', + }, + { + name: 'hostname to IPv6 literal', + start: 'https://allowed.example/to-ipv6', + hops: { + 'https://allowed.example/to-ipv6': + 'https://[2606:2800:220:1:248:1893:25c8:1946]/secret', + }, + forbiddenMarker: '2606:2800', + }, + { + name: 'repeated redirects across permitted then non-permitted hosts', + start: 'https://allowed.example/chain', + hops: { + 'https://allowed.example/chain': 'https://allowed.example/hop2', + 'https://allowed.example/hop2': 'http://169.254.169.254/latest/meta-data', + }, + forbiddenMarker: '169.254.169.254', + }, + { + name: 'relative Location followed by absolute cross-origin redirect', + start: 'https://allowed.example/rel', + hops: { + 'https://allowed.example/rel': '/same-origin', + 'https://allowed.example/same-origin': 'https://evil.example/steal', + }, + forbiddenMarker: 'evil.example', + }, + { + name: 'userinfo authority trick (allowed.example@evil.example)', + start: 'https://allowed.example/userinfo', + hops: { + 'https://allowed.example/userinfo': 'https://allowed.example@evil.example/steal', + }, + forbiddenMarker: 'evil.example', + }, + ]; + + const fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const href = String(input); + const redirect = init?.redirect ?? 'follow'; + for (const scenario of scenarios) { + const location = scenario.hops[href]; + if (location !== undefined) { + if (redirect === 'follow') { + // Pre-fix behavior would auto-follow into the non-allowed host. + return new Response('SECRET_FROM_BYPASSED_HOST', { status: 200 }); + } + return redirect302(location); + } + } + if (href === 'https://allowed.example/final') { + return new Response('PUBLIC_CONTENT', { status: 200 }); + } + // Any fetch of a forbidden destination means the allowlist was bypassed. + return new Response('SECRET_FROM_BYPASSED_HOST', { status: 200 }); + }); + vi.stubGlobal('fetch', fetchMock); + + const { registerGZipFileAsResourceTool: registerFresh } = await import( + '../tools/gzip-file-as-resource.js' + ); + + const mockServer = { + registerTool: vi.fn(), + registerResource: vi.fn(), + } as unknown as McpServer; + + let handler: Function | null = null; + (mockServer.registerTool as any).mockImplementation( + (name: string, config: any, h: Function) => { + handler = h; + } + ); + + registerFresh(mockServer); + + for (const scenario of scenarios) { + fetchMock.mockClear(); + await expect( + handler!({ + name: 'test.gz', + data: scenario.start, + outputType: 'resource', + }) + ).rejects.toThrow('not in the allowed domains list'); + + const fetched = fetchMock.mock.calls.map((c) => String(c[0])); + expect( + fetched.some((u) => u.includes(scenario.forbiddenMarker)), + `${scenario.name}: forbidden destination must never be fetched` + ).toBe(false); + } + + // Positive control: a relative redirect that stays on the allowed host + // must still succeed. + fetchMock.mockClear(); + const okHops = scenarios[3].hops; + okHops['https://allowed.example/stay'] = '/final'; + scenarios[3].hops = okHops; + await expect( + handler!({ + name: 'test.gz', + data: 'https://allowed.example/stay', + outputType: 'resource', + }) + ).resolves.toBeTruthy(); + expect( + fetchMock.mock.calls.map((c) => String(c[0])) + ).toContain('https://allowed.example/final'); + + vi.unstubAllGlobals(); + vi.unstubAllEnvs(); + }); }); }); diff --git a/src/everything/tools/gzip-file-as-resource.ts b/src/everything/tools/gzip-file-as-resource.ts index 3dd6fdae4a..09d007907c 100644 --- a/src/everything/tools/gzip-file-as-resource.ts +++ b/src/everything/tools/gzip-file-as-resource.ts @@ -17,11 +17,18 @@ const GZIP_MAX_FETCH_TIME_MILLIS = Number( process.env.GZIP_MAX_FETCH_TIME_MILLIS ?? String(30 * 1000) ); +const MAX_REDIRECTS = 10; +const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]); + // Comma-separated list of allowed domains. Empty means all domains are allowed. -const GZIP_ALLOWED_DOMAINS = (process.env.GZIP_ALLOWED_DOMAINS ?? "") - .split(",") - .map((d) => d.trim().toLowerCase()) - .filter((d) => d.length > 0); +// Read at call time so operators (and tests) can set GZIP_ALLOWED_DOMAINS without +// relying on module-load ordering. +function getAllowedDomains(): string[] { + return (process.env.GZIP_ALLOWED_DOMAINS ?? "") + .split(",") + .map((d) => d.trim().toLowerCase()) + .filter((d) => d.length > 0); +} // Tool input schema const GZipFileAsResourceSchema = z.object({ @@ -125,6 +132,36 @@ export const registerGZipFileAsResourceTool = (server: McpServer) => { }); }; +/** + * Asserts that a URL uses an allowed protocol and, when configured, an allowed domain. + * Applied to the initial URL and every redirect hop so GZIP_ALLOWED_DOMAINS cannot be + * bypassed by an allowed host that redirects elsewhere. + */ +export function assertFetchUrlAllowed(url: URL): void { + if ( + url.protocol !== "http:" && + url.protocol !== "https:" && + url.protocol !== "data:" + ) { + throw new Error( + `Unsupported URL protocol for ${url.href}. Only http, https, and data URLs are supported.` + ); + } + const allowedDomains = getAllowedDomains(); + if ( + allowedDomains.length > 0 && + (url.protocol === "http:" || url.protocol === "https:") + ) { + const domain = url.hostname.toLowerCase(); + const domainAllowed = allowedDomains.some((allowedDomain) => { + return domain === allowedDomain || domain.endsWith(`.${allowedDomain}`); + }); + if (!domainAllowed) { + throw new Error(`Domain ${domain} is not in the allowed domains list.`); + } + } +} + /** * Validates a given data URI to ensure it follows the appropriate protocols and rules. * @@ -133,30 +170,14 @@ export const registerGZipFileAsResourceTool = (server: McpServer) => { * @throws {Error} If the data URI does not use a supported protocol or does not meet allowed domains criteria. */ function validateDataURI(dataUri: string): URL { - // Validate Inputs - const url = new URL(dataUri); + let url: URL; try { - if ( - url.protocol !== "http:" && - url.protocol !== "https:" && - url.protocol !== "data:" - ) { - throw new Error( - `Unsupported URL protocol for ${dataUri}. Only http, https, and data URLs are supported.` - ); - } - if ( - GZIP_ALLOWED_DOMAINS.length > 0 && - (url.protocol === "http:" || url.protocol === "https:") - ) { - const domain = url.hostname; - const domainAllowed = GZIP_ALLOWED_DOMAINS.some((allowedDomain) => { - return domain === allowedDomain || domain.endsWith(`.${allowedDomain}`); - }); - if (!domainAllowed) { - throw new Error(`Domain ${domain} is not in the allowed domains list.`); - } - } + url = new URL(dataUri); + } catch { + throw new Error(`Error processing file ${dataUri}: Invalid URL`); + } + try { + assertFetchUrlAllowed(url); } catch (error) { throw new Error( `Error processing file ${dataUri}: ${ @@ -169,6 +190,7 @@ function validateDataURI(dataUri: string): URL { /** * Fetches data safely from a given URL while ensuring constraints on maximum byte size and timeout duration. + * Redirects are followed manually so each hop is re-checked against the same protocol/domain rules. * * @param {URL} url The URL to fetch data from. * @param {Object} options An object containing options for the fetch operation. @@ -191,57 +213,87 @@ async function fetchSafely( ); try { - // Fetch the data - const response = await fetch(url, { signal: controller.signal }); - if (!response.body) { - throw new Error("No response body"); - } + let current = url; + for (let hop = 0; hop <= MAX_REDIRECTS; hop++) { + assertFetchUrlAllowed(current); + + const response = await fetch(current, { + signal: controller.signal, + redirect: "manual", + }); - // Note: we can't trust the Content-Length header: a malicious or clumsy server could return much more data than advertised. - // We check it here for early bail-out, but we still need to monitor actual bytes read below. - const contentLengthHeader = response.headers.get("content-length"); - if (contentLengthHeader != null) { - const contentLength = parseInt(contentLengthHeader, 10); - if (contentLength > maxBytes) { - throw new Error( - `Content-Length for ${url} exceeds max of ${maxBytes}: ${contentLength}` - ); + if (REDIRECT_STATUSES.has(response.status)) { + const location = response.headers.get("location"); + if (!location) { + throw new Error( + `Redirect from ${current.href} missing Location header` + ); + } + if (response.body) { + try { + await response.body.cancel(); + } catch { + // ignore cancel errors + } + } + current = new URL(location, current); + continue; } - } - // Read the fetched data from the response body - const reader = response.body.getReader(); - const chunks = []; - let totalSize = 0; + if (!response.body) { + throw new Error("No response body"); + } + + // Note: we can't trust the Content-Length header: a malicious or clumsy server could return much more data than advertised. + // We check it here for early bail-out, but we still need to monitor actual bytes read below. + const contentLengthHeader = response.headers.get("content-length"); + if (contentLengthHeader != null) { + const contentLength = parseInt(contentLengthHeader, 10); + if (contentLength > maxBytes) { + throw new Error( + `Content-Length for ${current.href} exceeds max of ${maxBytes}: ${contentLength}` + ); + } + } - // Read chunks until done - try { - while (true) { - const { done, value } = await reader.read(); - if (done) break; + // Read the fetched data from the response body + const reader = response.body.getReader(); + const chunks = []; + let totalSize = 0; - totalSize += value.length; + // Read chunks until done + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; - if (totalSize > maxBytes) { - reader.cancel(); - throw new Error(`Response from ${url} exceeds ${maxBytes} bytes`); + totalSize += value.length; + + if (totalSize > maxBytes) { + reader.cancel(); + throw new Error( + `Response from ${current.href} exceeds ${maxBytes} bytes` + ); + } + + chunks.push(value); } + } finally { + reader.releaseLock(); + } - chunks.push(value); + // Combine chunks into a single buffer + const buffer = new Uint8Array(totalSize); + let offset = 0; + for (const chunk of chunks) { + buffer.set(chunk, offset); + offset += chunk.length; } - } finally { - reader.releaseLock(); - } - // Combine chunks into a single buffer - const buffer = new Uint8Array(totalSize); - let offset = 0; - for (const chunk of chunks) { - buffer.set(chunk, offset); - offset += chunk.length; + return buffer.buffer; } - return buffer.buffer; + throw new Error(`Too many redirects fetching ${url.href}`); } finally { clearTimeout(timeout); }