[BUG] ChatFlowTool using wrong URL depending on infrastructure setup about flowise HOT 9 CLOSED

Sounds great! Good find, I didn't know about that http header.

I'll submit a pull request as soon as I have some free time.

Thanks!!

from flowise.

if it's helpful, i've seen applications deal with the x-forwarded headers by having allow lists in the app config. it could specify the allowed hosts and a default host, the forwarded protocol, and the forwarded port. that way it has enough info to determine a safe way to build a full url. it also specify a list of proxy addresses/hostnames that are known to be valid. strip those away from the x-forwarded-for header and if you have one address left, it should be the address of the original requester (i.e. the end user). that might make it easier to deal with proxy setups, vs "number of proxies".

from flowise.

We're getting the baseURL like this: ${req.protocol}://${req.get('host')}
https://github.com/FlowiseAI/Flowise/blob/main/packages/server/src/utils/buildChatflow.ts#L57

Does the req.protocol changed behind load balancer?

from flowise.

I saw that code.

The host header is passed from the LB to Flowise because its part of the request context, but the protocol is not. The protocol depends on the communication to Flowise, in this case from the LB. Communication between the browser and the LB is unknown to Flowise.

That's when things get mixed up.

In environments without any middleware, like testing locally, current logic works just fine.

from flowise.

you are right, req.protocol becomes http when its behind a proxy/load balancer.

it seems like we can get the protocol from header x-forwarded-proto: https://stackoverflow.com/questions/9024783/how-to-force-node-js-express-js-to-https-when-it-is-running-behind-an-aws-load-b

what we can do is first check if the hearder exsits, if yes use that as priority over req.protocol. And we can also add an additional field to the node for user to specify the full URL base path like http://localhost:3000/api/v1

from flowise.

There's one thing that I been thinking about since our conversation yesterday and is making me reconsider the current approach entirely.

The current approach can lead to potential security risks. The Host header can be modified by the user and a malicious one could use this to make the Flowise backend call an endpoint of their own.

One way to reproduce this is:

• Run Flowise locally
• Install a Chrome extension that allows to modify headers or use Postman to the call to the prediction endpoint directly
• Open Flowise in Chrome or call the prediction endpoint: http://localhost:3000, injecting a Host header like google.com
• Trigger flow by sending a message

The outcome is that the backend will try to call http://google.com/api/v1/prediction/chatflowid. Instead of google.com an attacker could execute something on their own servers.

Not sure how this can be exploited because I'm not a security expert. But by returning a specific answer to the agent, attacker might be able to leak information or induce unexpected behaviors.

So I have a few questions:

• Why are we calling the ChatFlow with an HTTP request instead of triggering the flow directly in code?
• If necessary still to do it in an HTTP request, why are we not just calling http://localhost:{port} as the default? I'd imagine this is agnostic of the infrastructure and will work in every scenario. We could also still offer the option to change the URL base path to something different if there is a reason to go through a LB or something.

from flowise.

what if we do something like this:

export const getBaseURL = (protocol: string, host: string, headers: ICommonObject) => {
    // Check if apps behind load balancer
    if (headers['x-forwarded-proto'] && headers['x-forwarded-proto'] === "http" && !host.includes('localhost')) {
        protocol = 'https'
    }
    if (headers['X-Forwarded-Proto'] && headers['X-Forwarded-Proto'] === "http" && !host.includes('localhost')) {
        protocol = 'https'
    }

    return `${protocol}://${host}`
}

Then use it like:

const baseURL = getBaseURL(req.protocol, req.get('host') ?? '', req.headers)

Theoretically we should just call the utilBuildChatflow, but components level nodes don't have access to server level functions

from flowise.

I'd try not make any assumptions in code about the infrastructure configuration. A load balancer could also be accessible through HTTP. I submitted a PR that should be generic enough to solve the original issue. I'd release this changes to fix current flow which is broken.

Regarding my concern about security issues, as long as we use req.get('host') users will be able to redirect the call to other services. So I'd suggest to avoid using and trusting the host header.

I can open a new issue for this topic if it makes more sense.

from flowise.

if it's helpful, i've seen applications deal with the x-forwarded headers by having allow lists in the app config. it could specify the allowed hosts and a default host, the forwarded protocol, and the forwarded port. that way it has enough info to determine a safe way to build a full url. it also specify a list of proxy addresses/hostnames that are known to be valid. strip those away from the x-forwarded-for header and if you have one address left, it should be the address of the original requester (i.e. the end user). that might make it easier to deal with proxy setups, vs "number of proxies".

Thanks! This approach works if we think having something dynamic is better. Another approach I've seen is the one I shared in my first message, letting the user specify the host in the app config and having a default like 127.0.0.1. I'm not sure if the baseURL is being used by other nodes or functionalities, so I don't know if either of each approach is better.

What I still don't see is why the ChatFlowTool needs to handle it dynamically, because it's just an "internal" communication that needs to happen. For this particular case, I'd default to 127.0.0.1:port and let the user optionally override it in the node, instead of using the baseURL that comes from the context.

from flowise.