Keep the SSRF guard on redirects and at connect time for all outbound calls - #826
Merged
Merged
Conversation
… calls Outbound calls to user-supplied URLs ran assertSafeOutboundUrl once and then let axios/fetch follow redirects and re-resolve the host unchecked. - common/guarded-http.util: ssrfGuardedAxiosOptions() (agents that check the address at connect time + a beforeRedirect hook for scheme and literal IPs), ssrfGuardedAxios() for soap, ssrfGuardedFetch() with manual redirects for the MCP client transport. - Applied to the REST, GraphQL, SOAP and MCP engines, the OAuth2, LOGIN_TOKEN and MCP OAuth services, and the OpenAPI, Postman, GraphQL and WSDL importers. The WSDL importer had no check at all. - The operator's HTTP(S)_PROXY host is exempt from the connect-time check. - The instance-wide SSRF allowlist routes are self-hosted only; in cloud the list comes from SSRF_ALLOWED_HOSTS, the settings card is hidden and the connection test no longer suggests allowlisting.
The MCP transport calls fetch for each message of a session; checking the starting URL there added one DNS lookup per message. The engine already checks that URL once per call, as every caller does.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #821 (merged); replaces #823, which closed when its stacked base branch was deleted.
Outbound calls to URLs that come from users ran
assertSafeOutboundUrlonce and then handed the URL to axios or fetch, which follow redirects and resolve the host again on their own. This keeps the guard on for the whole request:common/guarded-http.util.tsssrfGuardedAxiosOptions(): http/https agents whoselookupchecks the address at connect time, plus abeforeRedirecthook for the scheme and literal-IP targets (Node connects to an IP without callinglookup). Same keep-alive settings as Node's global agents.ssrfGuardedAxios(): an axios instance with those options, forsoap, so WSDL/XSD imports go through it too.ssrfGuardedFetch(): follows redirects by hand and checks every target (fetch semantics kept: 307/308 keep the body, 301/302/303 become GET, Authorization/Cookie dropped across origins).HTTP_PROXY/HTTPS_PROXYis exempt from the connect-time check, so self-hosted setups behind a private proxy keep working. The connector proxy (CONNECTOR_PROXY_URL) keeps its own agent; redirects through it still get the scheme/IP check.GET/PUT /api/admin/settings/ssrf-allowed-hostsare now self-hosted only (SelfHostedOnlyGuard). The list is instance-wide, and in cloud every sign-up is the ADMIN of its own workspace. Cloud keeps reading the existing DB entries andSSRF_ALLOWED_HOSTS. The settings card is hidden in cloud, and the connection test no longer suggests allowlisting there.Nothing changes when the guard is off (
SSRF_GUARD=disabled, and under jest by default).Tests:
guarded-http.util.spec.ts(10 cases, real sockets: redirect to an internal IP, redirect to a name resolving inward, allowed redirects, guard off, fetch method/body/header rules, redirect cap, proxy exemption),rest.engine.redirect.spec.ts(a REST tool call and an OpenAPI URL import against a server that redirects inward; both fail on main and pass here),site-settings.controller.spec.ts(the allowlist routes carry the guard). Removing thebeforeRedirecthook or the agents each fails a test. Full backend suite passes locally, backend and frontend typecheck clean.Since #823:
ssrfGuardedFetchchecks redirect targets only. The MCP transport calls it once per message, and re-checking the starting URL there cost one DNS lookup per message; the engine already checks that URL once per call. Measured before/after on axios calls: no difference (0.11-0.12 ms p50 locally, ~72 ms to a public HTTPS host either way).