mirror of
https://github.com/langgenius/dify.git
synced 2026-09-24 23:22:26 +08:00
fix(api): make SSRF-blocked error message actionable (#39867)
Co-authored-by: Taranum01 <50813317+Taranum01@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Taranum01
autofix-ci[bot]
parent
8a2cf65ec5
commit
83b3a8fd88
@@ -19,6 +19,7 @@ from core.helper.ssrf_proxy import (
|
||||
max_retries_exceeded_error,
|
||||
request_error,
|
||||
)
|
||||
from core.tools.errors import ToolSSRFError
|
||||
|
||||
|
||||
@patch("core.helper.ssrf_proxy._get_ssrf_client", autospec=True)
|
||||
@@ -360,3 +361,96 @@ def test_graphon_ssrf_proxy_wraps_module_requests(method_name: str) -> None:
|
||||
assert wrapped.status_code == 200
|
||||
assert wrapped.url == "https://example.com/resource"
|
||||
assert wrapped.content == b"ok"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Squid-blocked 403 regression tests (issue #38443)
|
||||
# ---------------------------------------------------------------------------
|
||||
# When the SSRF proxy denies a request to a private/internal network address,
|
||||
# Squid returns 401/403 with itself identified in the Server or Via header.
|
||||
# The Python client must raise ToolSSRFError with a message that tells the
|
||||
# user exactly which env var to set, instead of just "blocked by SSRF
|
||||
# protection" (the pre-#38443 message gave no actionable guidance).
|
||||
|
||||
|
||||
def _build_squid_blocked_response(status_code: int = 403) -> MagicMock:
|
||||
"""Construct a mock httpx.Response that looks like Squid's ACL deny."""
|
||||
response = MagicMock()
|
||||
response.status_code = status_code
|
||||
response.headers = {"server": "squid/4.10", "via": "1.1 squid (squid/4.10)"}
|
||||
return response
|
||||
|
||||
|
||||
@patch("core.helper.ssrf_proxy._get_ssrf_client", autospec=True)
|
||||
def test_squid_block_raises_actionable_tool_ssrf_error(mock_get_client) -> None:
|
||||
"""A 403 from Squid must raise ToolSSRFError whose message tells the user
|
||||
exactly which env var to set. Pre-#38443 the message had no remediation
|
||||
hint, so users hit dead ends when their internal API was blocked."""
|
||||
mock_client = MagicMock()
|
||||
mock_client.send.return_value = _build_squid_blocked_response(status_code=403)
|
||||
mock_get_client.return_value = mock_client
|
||||
|
||||
with pytest.raises(ToolSSRFError) as exc_info:
|
||||
make_request("GET", "http://172.21.0.5/api/health")
|
||||
|
||||
msg = str(exc_info.value)
|
||||
assert "172.21.0.5" in msg, f"URL should appear in the error, got: {msg!r}"
|
||||
assert "SSRF_PROXY_ALLOW_PRIVATE_IPS" in msg, "Error must tell the user which env var to set; got: " + msg
|
||||
# The remediation hint must include a concrete example, otherwise users
|
||||
# still have to grep the squid config to figure out the syntax.
|
||||
assert "172.21.0.0/16" in msg
|
||||
# And it must point to the issue so maintainers can find context.
|
||||
assert "issues/38443" in msg
|
||||
|
||||
|
||||
@patch("core.helper.ssrf_proxy._get_ssrf_client", autospec=True)
|
||||
def test_squid_401_via_header_also_triggers_actionable_error(mock_get_client) -> None:
|
||||
"""Squid can return 401 with only the Via header set (no Server header)
|
||||
on some configurations. The detection must work for both."""
|
||||
mock_client = MagicMock()
|
||||
response = MagicMock()
|
||||
response.status_code = 401
|
||||
# Server header absent — only Via identifies Squid.
|
||||
response.headers = {"server": "", "via": "1.1 squid (squid/4.10)"}
|
||||
mock_client.send.return_value = response
|
||||
mock_get_client.return_value = mock_client
|
||||
|
||||
with pytest.raises(ToolSSRFError) as exc_info:
|
||||
make_request("GET", "http://10.0.0.1/internal")
|
||||
|
||||
assert "SSRF_PROXY_ALLOW_PRIVATE_IPS" in str(exc_info.value)
|
||||
|
||||
|
||||
@patch("core.helper.ssrf_proxy._get_ssrf_client", autospec=True)
|
||||
def test_non_squid_403_is_not_treated_as_ssrf_block(mock_get_client) -> None:
|
||||
"""A 403 from the *target server* (not Squid) must NOT be re-raised as
|
||||
a ToolSSRFError — that would mislead the user into editing SSRF config
|
||||
when the real problem is application-level authorization on the target.
|
||||
Pre-#38443 we didn't have this guard at all; the new wording only changes
|
||||
the Squid path, so verify we don't accidentally widen it."""
|
||||
mock_client = MagicMock()
|
||||
response = MagicMock()
|
||||
response.status_code = 403
|
||||
response.headers = {"server": "nginx/1.21", "via": "1.1 varnish"}
|
||||
mock_client.send.return_value = response
|
||||
mock_get_client.return_value = mock_client
|
||||
|
||||
# Should return the response, not raise.
|
||||
returned = make_request("GET", "http://public.example.com/admin")
|
||||
assert returned.status_code == 403
|
||||
|
||||
|
||||
@patch("core.helper.ssrf_proxy._get_ssrf_client", autospec=True)
|
||||
def test_squid_block_with_internal_10_x_url_mentions_allowlist(mock_get_client) -> None:
|
||||
"""Real-world repro from #38443: 10.x.x.x internal API blocked. The error
|
||||
message must still point at SSRF_PROXY_ALLOW_PRIVATE_IPS, not just say
|
||||
"private address" without telling the user what to do."""
|
||||
mock_client = MagicMock()
|
||||
mock_client.send.return_value = _build_squid_blocked_response(status_code=403)
|
||||
mock_get_client.return_value = mock_client
|
||||
|
||||
with pytest.raises(ToolSSRFError) as exc_info:
|
||||
make_request("POST", "http://10.0.0.42/v1/chat/completions")
|
||||
|
||||
assert "10.0.0.42" in str(exc_info.value)
|
||||
assert "SSRF_PROXY_ALLOW_PRIVATE_IPS" in str(exc_info.value)
|
||||
|
||||
Reference in New Issue
Block a user