fix: Control default deny list via an env var#5862
fix: Control default deny list via an env var#5862christopherholland-workday merged 2 commits intomainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the network security configuration by making the default HTTP deny list optional. Previously hardcoded, this list can now be disabled via an environment variable, addressing a limitation for self-hosted Flowise instances that require local network access. This change improves deployment flexibility while maintaining a secure-by-default posture. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces the HTTP_SECURITY_CHECK environment variable to control the default HTTP deny list, offering flexibility for self-hosted instances requiring local network access while maintaining a secure default. However, the current deny list implementation has security weaknesses. Hostnames in HTTP_DENY_LIST are not effectively blocked as the check only compares resolved IP addresses. Furthermore, the system is vulnerable to SSRF bypasses using IPv4-mapped IPv6 addresses, which are not covered by the deny list or its matching logic.
| return [...new Set([...DEFAULT_DENY_LIST, ...customList])] | ||
| } | ||
| return DEFAULT_DENY_LIST | ||
| return customList |
There was a problem hiding this comment.
The getHttpDenyList function returns a list that can contain hostnames, as documented in the updated JSDoc (line 33). However, the isDeniedIP function only compares the target IP address against the entries in the list. It does not check the target hostname against the entries, nor does it resolve the entries in the list to IP addresses. This means that any hostname added to HTTP_DENY_LIST will not be effectively blocked unless it is also an IP address or CIDR range. This is particularly dangerous when HTTP_SECURITY_CHECK is set to false, as the default IP-based protections are removed and the user may rely on the custom deny list for security.
Remediation: Update the security check logic to either resolve hostnames in the deny list to IP addresses or check the target hostname against the deny list before resolution.
In a previous fix we hardcoded a default deny list of endpoints that should not be accessible: #5708
This is a little too restricting for users hosting Flowise themselves. This PR controls the default deny list via a new environment variable, which defaults to
trueand must explicitly be set tofalseto allow calls to local endpoints.Testing
HTTP_SECURITY_CHECK=false(access granted, ignore that the call fail, it still hit the endpoint which is what we are fixing):HTTP_SECURITY_CHECK=trueand the env var missing and access was denied