Centralize SSRF defense-in-depth in core.helper.ssrf_proxy and close tenant-URL bypass gaps
- Dominant language
- TypeScript
- Stars
- 156k
- Forks
- 24.6k
- Avg merge
- 22h 9m
- Merged PRs (30d)
- 610
Description
## Summary
dify already routes outbound HTTP through a network-level SSRF egress proxy (`ssrf_proxy`, squid) and `api/AGENTS.md` mandates *"Use `core.helper.ssrf_proxy` for outbound HTTP fetches."* That's a solid primary control. This issue proposes a small, focused **application-layer defense-in-depth** layer underneath it, so that tenant-supplied outbound URLs are validated even when (a) the egress proxy is not configured, or (b) a code path bypasses the proxy entirely.
I'd like to align on direction before opening a PR, since this touches a shared helper and an existing private-IP allowlist.
## Background — what already exists
- `core/helper/ssrf_proxy.py` routes outbound calls through `SSRF_PROXY_HTTP_URL` / `SSRF_PROXY_HTTPS_URL` when configured.
- #36197 added `is_safe_external_url()` to that same module — an application-layer check that rejects URLs resolving into private / loopback / link-local / multicast / reserved / unspecified ranges (covers cloud instance metadata `169.254.169.254`, Docker bridge nets, RFC1918 LANs). When a proxy is configured it skips DNS (proxy is the chokepoint); when no proxy is configured it resolves via `getaddrinfo` and requires every resolved address to be public.
- Today `is_safe_external_url()` is wired into exactly **one** call site (MCP server URL validation).
## The gap
Two residual SSRF exposures remain:
1. **No-proxy deployments are unprotected.** The squid `ssrf_proxy` is only wired up in the docker-compose deployment. `.env.example` / pip / source installs default to **no egress proxy**, so outbound requests go direct with no IP validation.
2. **Several tenant-supplied-URL code paths bypass `ssrf_proxy` via direct `httpx`.** These hit user-configured endpoints with no validation — textbook SSRF:
- `api/services/auth/watercrawl/watercrawl.py` — `httpx.get(url, ...)` (tenant-configured crawler endpoint, credential check)
- `api/services/auth/firecrawl/firecrawl.py` — `httpx.post(url, ...)`
- `api/services/auth/jina.py` — direct `httpx.Client`
- `api/core/rag/extractor/notion_extractor.py`, `api/core/rag/extractor/firecrawl/firecrawl_app.py`, `api/core/rag/extractor/watercrawl/client.py` — RAG extractors fetching tenant-configured URLs
(dify-controlled URLs such as `controllers/console/version.py`, `core/helper/marketplace.py`, `services/recommend_app/remote/*` are **out of scope** — not tenant-controlled.)
## Proposed approach (application-layer, centralized)
1. **Centralize the check in the helper.** Call `is_safe_external_url()` inside the `ssrf_proxy` client methods (`get/post/put/patch/delete/head`) before dispatch, so every existing and future caller of `ssrf_proxy` automatically gets the validation — defense-in-depth layered *under* the squid proxy, not replacing it.
2. **Migrate the tenant-URL bypass sites** listed above to `ssrf_proxy` (which also brings them in line with the existing `AGENTS.md` mandate), so they inherit the validation.
3. **Honor the existing allowlist.** dify already supports `SSRF_PROXY_ALLOW_PRIVATE_IPS` / `SSRF_PROXY_ALLOW_PRIVATE_DOMAINS` (see `docker/ssrf_proxy/docker-entrypoint.sh`) for self-hosters who intentionally target internal services. The application-layer check **must respect the same allowlist** so it doesn't break those deployments. (Note: `is_safe_external_url()` currently rejects private ranges unconditionally — honoring the allowlist would be a small follow-up enhancement to it.)
## Out of scope (deliberately deferred)
- **DNS rebinding (TOCTOU).** Application-layer IP validation can't fully close the gap between validation and connect. The complete fix is validating the *connected* IP via a custom transport (or relying on the squid proxy). Acknowledged as a residual risk; this issue takes the high-value 90%.
- **CI lint enforcement** of "no direct `httpx` for outbound" — possible follow-up, not part of this change.
## Questions for maintainers
1. Do you want application-layer SSRF defense-in-depth inside `ssrf_proxy`, given the squid proxy is the primary control? (Rationale: it protects no-proxy deployments and acts as a hard stop if the proxy is misconfigured.)
2. Is "validate centrally in the helper + migrate the bypassing call sites" the shape you'd accept, vs. validating at each call site individually?
3. For allowlist parity — OK to extend `is_safe_external_url()` to read `SSRF_PROXY_ALLOW_PRIVATE_IPS` / `SSRF_PROXY_ALLOW_PRIVATE_DOMAINS`?
4. Any performance concern with one `getaddrinfo` per outbound call in no-proxy deployments? (Negligible vs. the network call itself, and skipped entirely when a proxy is configured.)
Happy to open the PR once we agree on the shape. Related: #36197.
Contributor guide
Research direction
Start by reading core/helper/ssrf_proxy.py, especially is_safe_external_url() and the client methods, then inspect the tenant-URL call sites listed in the issue. The work is done when validation is centralized, those bypasses use ssrf_proxy, and the existing private-IP and private-domain allowlists are honored without covering the explicitly out-of-scope paths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- docker, python
- Domain
- backend, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100