langgenius / langgenius/dify

Centralize SSRF defense-in-depth in core.helper.ssrf_proxy and close tenant-URL bypass gaps

Open
#38,064 5 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.