diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b4cea9339..1e60b99df1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -412,6 +412,19 @@ This section accumulates work toward the **2.3.0** milestone before a thread view stalled the Gateway event loop that runs, streams and every other thread share. The response is unchanged. ([#7727]) +- **community:** The shared SSRF guard now returns its documented `Error: ...` + rejection for malformed URLs instead of raising `ValueError`. + `validate_public_http_url` and `validate_delegated_backend_url` called + `urlparse()` unguarded, so an authority it rejects, such as the unmatched + IPv6 bracket in `http://[::1/api`, escaped to the caller. That surfaced as an + unhandled 500 from the personal MCP router and as generic exception text in + the crawl4ai and fastCRW `web_fetch` tools. Both validators now reject any + malformed authority before the scheme check and the private-address escape + hatches, including a raw bracket in userinfo, which RFC 3986 forbids and + Python versions disagree on. This is a fail-closed contract fix plus + version-drift hardening: no host reachable through a well-formed URL becomes + blocked. ([#8079]) + ## [2.2.0] — 2026-10-29 This section accumulates work toward the **2.2.0** milestone @@ -16350,6 +16363,7 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#8069]: https://github.com/bytedance/deer-flow/issues/8069 [#8071]: https://github.com/bytedance/deer-flow/issues/8071 [#8072]: https://github.com/bytedance/deer-flow/pull/8072 +[#8079]: https://github.com/bytedance/deer-flow/pull/8079 [#8117]: https://github.com/bytedance/deer-flow/pull/8117 [#8118]: https://github.com/bytedance/deer-flow/pull/8118 [#8126]: https://github.com/bytedance/deer-flow/pull/8126 diff --git a/backend/packages/harness/deerflow/community/url_safety.py b/backend/packages/harness/deerflow/community/url_safety.py index c54863a1a4..2ff6643050 100644 --- a/backend/packages/harness/deerflow/community/url_safety.py +++ b/backend/packages/harness/deerflow/community/url_safety.py @@ -5,7 +5,7 @@ from __future__ import annotations import ipaddress import socket from collections.abc import Callable -from urllib.parse import urlparse +from urllib.parse import ParseResult, urlparse _BLOCKED_HOSTNAMES = {"localhost", "metadata.google.internal"} _IDNA_DOT_TRANSLATION = str.maketrans("\u3002\uff0e\uff61", "...") @@ -142,6 +142,33 @@ def resolve_public_addresses( return candidates +def _parse_url(value: str) -> ParseResult | None: + """Parse a URL, returning ``None`` for malformed input. + + ``urlparse`` raises ``ValueError`` for some malformed authorities (for + example an unmatched IPv6 bracket). The validators' contract is to return an + ``"Error: ..."`` string, so callers must never see that exception. + + A raw bracket in the userinfo slot is malformed per RFC 3986. Modern CPython + rejects some of those forms inside ``urlparse`` already, but which ones + differs across versions, so rejecting all of them here keeps the result + version-independent; this also blocks the ``[literal]@[IPv6]`` shape that + ``urlparse`` accepts without complaint. Mirrors the SkillScan host parser + (#8017). Non-string input is malformed too, matching + :func:`safe_public_result_url`. + """ + if not isinstance(value, str): + return None + try: + parsed = urlparse(value) + except ValueError: + return None + userinfo, _, _ = parsed.netloc.rpartition("@") + if "[" in userinfo or "]" in userinfo: + return None + return parsed + + def validate_public_http_url( url: str, *, @@ -167,7 +194,9 @@ def validate_public_http_url( crawl4ai, fastcrw, firecrawl) resolve on their own side and cannot be pinned from here. """ - parsed = urlparse(url) + parsed = _parse_url(url) + if parsed is None: + return "Error: URL could not be parsed" if parsed.scheme not in {"http", "https"} or not parsed.netloc: return "Error: Only http:// and https:// URLs are supported" @@ -216,7 +245,9 @@ def validate_delegated_backend_url( :func:`resolve_public_addresses`; from a coroutine, call it via ``asyncio.to_thread``. """ - parsed = urlparse(base_url) + parsed = _parse_url(base_url) + if parsed is None: + return "Error: Backend URL could not be parsed" if parsed.scheme not in {"http", "https"} or not parsed.netloc: return "Error: Only http:// and https:// backend URLs are supported" diff --git a/backend/tests/test_url_safety.py b/backend/tests/test_url_safety.py index 6f119059f6..dba9da820b 100644 --- a/backend/tests/test_url_safety.py +++ b/backend/tests/test_url_safety.py @@ -89,3 +89,87 @@ def test_delegated_backend_allows_private_when_isolation_confirmed(): def test_delegated_backend_rejects_non_http_scheme(): assert validate_delegated_backend_url("ftp://example.com") == "Error: Only http:// and https:// backend URLs are supported" + + +def _must_not_resolve(_host): + raise AssertionError("a malformed URL must be rejected before DNS resolution") + + +# A raw bracket in the userinfo slot is malformed per RFC 3986. urlparse() +# rejects some of these forms itself; the others are caught by the userinfo +# check in _parse_url(), which mirrors the SkillScan host parser (#8017). +# One list drives both validators so a newly discovered malformed shape is +# covered by both at once. +_MALFORMED_URLS = [ + "http://[::1/api", # unmatched IPv6 bracket: urlparse() raises ValueError + "http://[::1]@Example.COM/api", # bracketed userinfo: urlparse() raises on some versions + "http://[]@example.com/api", # empty bracketed host in userinfo: urlparse() raises + "http://[@[::1]/", # raw "[" in userinfo: urlparse() succeeds, userinfo check fires + "http://u]@[::1]/", # raw "]" in userinfo: exercises the closing-bracket half + "http://a@b[c]@[2606:4700:4700::1111]/", # the last "@" separates userinfo from host +] + + +@pytest.mark.parametrize("url", _MALFORMED_URLS) +def test_public_validator_returns_error_for_malformed_url(url): + error = validate_public_http_url(url, resolver=_must_not_resolve) + + assert error == "Error: URL could not be parsed" + + +@pytest.mark.parametrize("url", _MALFORMED_URLS) +def test_delegated_backend_validator_returns_error_for_malformed_url(url): + error = validate_delegated_backend_url(url, resolver=_must_not_resolve) + + assert error == "Error: Backend URL could not be parsed" + + +@pytest.mark.parametrize("value", [b"http://example.com", None, 123, ["http://example.com"]]) +def test_public_validator_returns_error_for_non_string_input(value): + # The signature says str, but these guards must never raise: a non-string + # value is malformed input, not an exception. Mirrors the isinstance guard + # in safe_public_result_url(). + error = validate_public_http_url(value, resolver=_must_not_resolve) + + assert error == "Error: URL could not be parsed" + + +@pytest.mark.parametrize("value", [b"http://example.com", None, 123, ["http://example.com"]]) +def test_delegated_backend_validator_returns_error_for_non_string_input(value): + error = validate_delegated_backend_url(value, resolver=_must_not_resolve) + + assert error == "Error: Backend URL could not be parsed" + + +@pytest.mark.parametrize( + ("url", "expected"), + [ + # A plain userinfo must still pass through to address classification. + ("https://user:pass@[2606:4700:4700::1111]/", None), + # ...and must not hide a private host. + ("http://user@[::1]/", "Error: Refusing to fetch a private, loopback, or metadata address"), + ], +) +def test_public_validator_keeps_legitimate_userinfo(url, expected): + assert validate_public_http_url(url, resolver=_must_not_resolve) == expected + + +def test_delegated_backend_validator_keeps_legitimate_userinfo(): + assert validate_delegated_backend_url("https://user:pass@[2606:4700:4700::1111]/", resolver=_must_not_resolve) is None + + error = validate_delegated_backend_url("http://user@[::1]/", resolver=_must_not_resolve) + + assert error is not None + assert "network_isolation_confirmed" in error + + +@pytest.mark.parametrize("url", ["http://[::1/api", "http://[@[::1]/"]) +def test_public_validator_rejects_malformed_url_before_bypassing_private_checks(url): + # Parsing must happen before the allow_private_addresses escape hatch: a + # malformed URL is not a usable URL at all. + assert validate_public_http_url(url, allow_private_addresses=True) == "Error: URL could not be parsed" + + +@pytest.mark.parametrize("url", ["http://[::1/api", "http://[@[::1]/"]) +def test_delegated_backend_validator_rejects_malformed_url_before_isolation_bypass(url): + assert validate_delegated_backend_url(url, network_isolation_confirmed=True) == "Error: Backend URL could not be parsed"