idea(ssrf): enforce SSRF protection at TCP connection boundary with tcp_module - #3875
idea(ssrf): enforce SSRF protection at TCP connection boundary with tcp_module#3875ruslandoga wants to merge 2 commits into
tcp_module#3875Conversation
| adapter_config( | ||
| Keyword.get(opts, :http2, true), | ||
| opts[:pool_name], | ||
| Keyword.get(opts, :ssrf, false) |
There was a problem hiding this comment.
🟠 Severity: HIGH
:ssrf defaults to false, and current HttpBased.Client callers (Axiom’s domain, OTLP’s endpoint, and Sentry’s DSN host) do not pass it. Their user-configurable URLs therefore use ordinary Finch pools, allowing requests and credentials to reach private services despite this PR’s SSRF boundary.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Change the default value of the :ssrf option from false to true in the new/1 function so that all HttpBased.Client callers (including Axiom, OTLP, and Sentry adaptors whose URLs are user-configurable) route through the SSRF-safe Logflare.FinchSSRF Finch pool by default. Callers that explicitly need to bypass SSRF protection (e.g., for trusted, hardcoded internal endpoints) can opt out by passing ssrf: false explicitly. Additionally, update the corresponding @doc comment on line 61 to reflect the new default (Defaults to \true`.`).
⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.
| Keyword.get(opts, :ssrf, false) | |
| Keyword.get(opts, :ssrf, true) |
tcp_module
tcp_moduletcp_module
Summary
FinchSSRFpool in both networking modes and route Webhook plus opt-in HTTP-based clients through it without allowing pool overrides.Testing
mix test test/logflare/utils/ssrf_test.exs test/logflare/utils/ssrf/tcp_test.exs test/logflare/networking_test.exs test/logflare/backends/adaptor/http_based/client_test.exs test/logflare/backends/adaptor/webhook_adaptor_test.exs(8 doctests, 63 tests, 0 failures).