Use secureFetch in WebScraperTool#5678
Use secureFetch in WebScraperTool#5678christopherholland-workday wants to merge 1 commit intomainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly replaces node-fetch with the custom secureFetch method in WebScraperTool to add SSRF protection, which is a good security improvement. My review includes a suggestion to simplify the call to secureFetch for better code clarity and to remove redundant options.
| private async scrapeSingleUrl(url: string): Promise<Omit<ScrapedPageData, 'url'> & { foundLinks: string[] }> { | ||
| try { | ||
| const response = await fetch(url, { timeout: this.timeoutMs, redirect: 'follow', follow: 5 }) | ||
| const response = await secureFetch(url, { timeout: this.timeoutMs, redirect: 'follow', follow: 5 }) |
There was a problem hiding this comment.
The secureFetch function handles redirects manually and overrides the redirect option. The follow option is also not used by secureFetch's internal logic. To avoid confusion and improve clarity, these properties can be removed from the call. secureFetch defaults to a maximum of 5 redirects, which preserves the original behavior of follow: 5.
| const response = await secureFetch(url, { timeout: this.timeoutMs, redirect: 'follow', follow: 5 }) | |
| const response = await secureFetch(url, { timeout: this.timeoutMs }) |
|
The relevant changes are included inPR #5653, specifically in the commit feat(WebScraperTool.ts): swap fetch to secureFetch |
2 participants
This code was using
node-fetchinstead of the custom-builtsecureFetchmethod.secureFetchhas SSRF protection built in and should be the standard used in Flowise when making fetch calls.