From 00ca2f3bbdc11428e08c87aae5ff912575f008e2 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 31 Jul 2025 11:52:58 +0100 Subject: [PATCH] prevent invalid http redirect --- .../components/nodes/agentflow/HTTP/HTTP.ts | 10 +- .../nodes/tools/RequestsDelete/core.ts | 8 +- .../nodes/tools/RequestsGet/core.ts | 8 +- .../nodes/tools/RequestsPost/core.ts | 8 +- .../nodes/tools/RequestsPut/core.ts | 8 +- packages/components/src/httpSecurity.ts | 169 ++++++++++++++++++ 6 files changed, 181 insertions(+), 30 deletions(-) diff --git a/packages/components/nodes/agentflow/HTTP/HTTP.ts b/packages/components/nodes/agentflow/HTTP/HTTP.ts index 960119d05..f7635ce03 100644 --- a/packages/components/nodes/agentflow/HTTP/HTTP.ts +++ b/packages/components/nodes/agentflow/HTTP/HTTP.ts @@ -1,9 +1,9 @@ import { ICommonObject, INode, INodeData, INodeParams } from '../../../src/Interface' -import axios, { AxiosRequestConfig, Method, ResponseType } from 'axios' +import { AxiosRequestConfig, Method, ResponseType } from 'axios' import FormData from 'form-data' import * as querystring from 'querystring' import { getCredentialData, getCredentialParam } from '../../../src/utils' -import { checkDenyList } from '../../../src/httpSecurity' +import { secureAxiosRequest } from '../../../src/httpSecurity' class HTTP_Agentflow implements INode { label: string @@ -293,8 +293,6 @@ class HTTP_Agentflow implements INode { // Build final URL with query parameters const finalUrl = queryString ? `${url}${url.includes('?') ? '&' : '?'}${queryString}` : url - await checkDenyList(finalUrl) - // Prepare request config const requestConfig: AxiosRequestConfig = { method: method as Method, @@ -331,8 +329,8 @@ class HTTP_Agentflow implements INode { } } - // Make the HTTP request - const response = await axios(requestConfig) + // Make the secure HTTP request that validates all URLs in redirect chains + const response = await secureAxiosRequest(requestConfig) // Process response based on response type let responseData diff --git a/packages/components/nodes/tools/RequestsDelete/core.ts b/packages/components/nodes/tools/RequestsDelete/core.ts index 049091c09..a60dee959 100644 --- a/packages/components/nodes/tools/RequestsDelete/core.ts +++ b/packages/components/nodes/tools/RequestsDelete/core.ts @@ -1,7 +1,6 @@ import { z } from 'zod' -import fetch from 'node-fetch' import { DynamicStructuredTool } from '../OpenAPIToolkit/core' -import { checkDenyList } from '../../../src/httpSecurity' +import { secureFetch } from '../../../src/httpSecurity' export const desc = `Use this when you need to execute a DELETE request to remove data from a website.` @@ -166,11 +165,8 @@ export class RequestsDeleteTool extends DynamicStructuredTool { finalUrl = url.toString() } - // Check if URL is allowed by security policy - await checkDenyList(finalUrl) - try { - const res = await fetch(finalUrl, { + const res = await secureFetch(finalUrl, { method: 'DELETE', headers: requestHeaders }) diff --git a/packages/components/nodes/tools/RequestsGet/core.ts b/packages/components/nodes/tools/RequestsGet/core.ts index 179c4377f..931a494e8 100644 --- a/packages/components/nodes/tools/RequestsGet/core.ts +++ b/packages/components/nodes/tools/RequestsGet/core.ts @@ -1,7 +1,6 @@ import { z } from 'zod' -import fetch from 'node-fetch' import { DynamicStructuredTool } from '../OpenAPIToolkit/core' -import { checkDenyList } from '../../../src/httpSecurity' +import { secureFetch } from '../../../src/httpSecurity' export const desc = `Use this when you need to execute a GET request to get data from a website.` @@ -166,11 +165,8 @@ export class RequestsGetTool extends DynamicStructuredTool { finalUrl = url.toString() } - // Check if URL is allowed by security policy - await checkDenyList(finalUrl) - try { - const res = await fetch(finalUrl, { + const res = await secureFetch(finalUrl, { headers: requestHeaders }) diff --git a/packages/components/nodes/tools/RequestsPost/core.ts b/packages/components/nodes/tools/RequestsPost/core.ts index e680b26ec..bbffe9379 100644 --- a/packages/components/nodes/tools/RequestsPost/core.ts +++ b/packages/components/nodes/tools/RequestsPost/core.ts @@ -1,7 +1,6 @@ import { z } from 'zod' -import fetch from 'node-fetch' import { DynamicStructuredTool } from '../OpenAPIToolkit/core' -import { checkDenyList } from '../../../src/httpSecurity' +import { secureFetch } from '../../../src/httpSecurity' export const desc = `Use this when you want to execute a POST request to create or update a resource.` @@ -127,10 +126,7 @@ export class RequestsPostTool extends DynamicStructuredTool { ...this.headers } - // Check if URL is allowed by security policy - await checkDenyList(inputUrl) - - const res = await fetch(inputUrl, { + const res = await secureFetch(inputUrl, { method: 'POST', headers: requestHeaders, body: JSON.stringify(inputBody) diff --git a/packages/components/nodes/tools/RequestsPut/core.ts b/packages/components/nodes/tools/RequestsPut/core.ts index de9d19365..28003f9d0 100644 --- a/packages/components/nodes/tools/RequestsPut/core.ts +++ b/packages/components/nodes/tools/RequestsPut/core.ts @@ -1,7 +1,6 @@ import { z } from 'zod' -import fetch from 'node-fetch' import { DynamicStructuredTool } from '../OpenAPIToolkit/core' -import { checkDenyList } from '../../../src/httpSecurity' +import { secureFetch } from '../../../src/httpSecurity' export const desc = `Use this when you want to execute a PUT request to update or replace a resource.` @@ -127,10 +126,7 @@ export class RequestsPutTool extends DynamicStructuredTool { ...this.headers } - // Check if URL is allowed by security policy - await checkDenyList(inputUrl) - - const res = await fetch(inputUrl, { + const res = await secureFetch(inputUrl, { method: 'PUT', headers: requestHeaders, body: JSON.stringify(inputBody) diff --git a/packages/components/src/httpSecurity.ts b/packages/components/src/httpSecurity.ts index d52825350..d729f8da5 100644 --- a/packages/components/src/httpSecurity.ts +++ b/packages/components/src/httpSecurity.ts @@ -1,5 +1,7 @@ import * as ipaddr from 'ipaddr.js' import dns from 'dns/promises' +import axios, { AxiosRequestConfig, AxiosResponse } from 'axios' +import fetch, { RequestInit, Response } from 'node-fetch' /** * Checks if an IP address is in the deny list @@ -50,3 +52,170 @@ export async function checkDenyList(url: string): Promise { } } } + +/** + * Makes a secure HTTP request that validates all URLs in redirect chains against the deny list + * @param config - Axios request configuration + * @param maxRedirects - Maximum number of redirects to follow (default: 5) + * @returns Promise + * @throws Error if any URL in the redirect chain is denied + */ +export async function secureAxiosRequest(config: AxiosRequestConfig, maxRedirects: number = 5): Promise { + let currentUrl = config.url + let redirectCount = 0 + let currentConfig = { ...config, maxRedirects: 0 } // Disable automatic redirects + + // Validate the initial URL + if (currentUrl) { + await checkDenyList(currentUrl) + } + + while (redirectCount <= maxRedirects) { + try { + // Update the URL in config for subsequent requests + currentConfig.url = currentUrl + + const response = await axios(currentConfig) + + // If it's a successful response (not a redirect), return it + if (response.status < 300 || response.status >= 400) { + return response + } + + // Handle redirect + const location = response.headers.location + if (!location) { + // No location header, but it's a redirect status - return the response + return response + } + + redirectCount++ + + if (redirectCount > maxRedirects) { + throw new Error('Too many redirects') + } + + // Resolve the redirect URL (handle relative URLs) + const redirectUrl = new URL(location, currentUrl).toString() + + // Validate the redirect URL against the deny list + await checkDenyList(redirectUrl) + + // Update current URL for next iteration + currentUrl = redirectUrl + + // For redirects, we only need to preserve certain headers and change method if needed + if (response.status === 301 || response.status === 302 || response.status === 303) { + // For 303, or when redirecting POST requests, change to GET + if ( + response.status === 303 || + (currentConfig.method && ['POST', 'PUT', 'PATCH'].includes(currentConfig.method.toUpperCase())) + ) { + currentConfig.method = 'GET' + delete currentConfig.data + } + } + } catch (error) { + // If it's not a redirect-related error from axios, propagate it + if (error.response && error.response.status >= 300 && error.response.status < 400) { + // This is a redirect response that axios couldn't handle automatically + // Continue with our manual redirect handling + const response = error.response + const location = response.headers.location + + if (!location) { + return response + } + + redirectCount++ + + if (redirectCount > maxRedirects) { + throw new Error('Too many redirects') + } + + const redirectUrl = new URL(location, currentUrl).toString() + await checkDenyList(redirectUrl) + currentUrl = redirectUrl + + // Handle method changes for redirects + if (response.status === 301 || response.status === 302 || response.status === 303) { + if ( + response.status === 303 || + (currentConfig.method && ['POST', 'PUT', 'PATCH'].includes(currentConfig.method.toUpperCase())) + ) { + currentConfig.method = 'GET' + delete currentConfig.data + } + } + continue + } + + // For other errors, re-throw + throw error + } + } + + throw new Error('Too many redirects') +} + +/** + * Makes a secure fetch request that validates all URLs in redirect chains against the deny list + * @param url - URL to fetch + * @param init - Fetch request options + * @param maxRedirects - Maximum number of redirects to follow (default: 5) + * @returns Promise + * @throws Error if any URL in the redirect chain is denied + */ +export async function secureFetch(url: string, init?: RequestInit, maxRedirects: number = 5): Promise { + let currentUrl = url + let redirectCount = 0 + let currentInit = { ...init, redirect: 'manual' as const } // Disable automatic redirects + + // Validate the initial URL + await checkDenyList(currentUrl) + + while (redirectCount <= maxRedirects) { + const response = await fetch(currentUrl, currentInit) + + // If it's a successful response (not a redirect), return it + if (response.status < 300 || response.status >= 400) { + return response + } + + // Handle redirect + const location = response.headers.get('location') + if (!location) { + // No location header, but it's a redirect status - return the response + return response + } + + redirectCount++ + + if (redirectCount > maxRedirects) { + throw new Error('Too many redirects') + } + + // Resolve the redirect URL (handle relative URLs) + const redirectUrl = new URL(location, currentUrl).toString() + + // Validate the redirect URL against the deny list + await checkDenyList(redirectUrl) + + // Update current URL for next iteration + currentUrl = redirectUrl + + // Handle method changes for redirects according to HTTP specs + if (response.status === 301 || response.status === 302 || response.status === 303) { + // For 303, or when redirecting POST/PUT/PATCH requests, change to GET + if (response.status === 303 || (currentInit.method && ['POST', 'PUT', 'PATCH'].includes(currentInit.method.toUpperCase()))) { + currentInit = { + ...currentInit, + method: 'GET', + body: undefined + } + } + } + } + + throw new Error('Too many redirects') +}