diff --git a/deploy/docker/api.py b/deploy/docker/api.py index 7c90fb5ba..581494a2a 100644 --- a/deploy/docker/api.py +++ b/deploy/docker/api.py @@ -1,7 +1,7 @@ import os import json import asyncio -from typing import List, Tuple, Dict +from typing import List, Tuple, Dict, NamedTuple from functools import partial from uuid import uuid4 from datetime import datetime, timezone @@ -18,6 +18,7 @@ from crawl4ai import ( AsyncWebCrawler, CrawlerRunConfig, + CrawlResult, LLMExtractionStrategy, CacheMode, BrowserConfig, @@ -647,14 +648,119 @@ async def stream_results(crawler: AsyncWebCrawler, results_gen: AsyncGenerator) await _dispose_crawler(crawler) -def _normalize_and_validate_seeds(urls: List[str]) -> List[str]: +class _SeedBatch(NamedTuple): + """The outcome of normalizing and destination-checking one batch of seeds. + + `crawlable` are the seeds to fetch, in the caller's order. `refused` holds + `(url, detail)` for each seed the destination check turned away, in the + caller's order, where `detail` is the message the 400 used to carry. + """ + crawlable: List[str] + refused: List[Tuple[str, str]] + + +def _normalize_and_validate_seeds(urls: List[str]) -> _SeedBatch: """Prefix bare hosts with https:// and SSRF-validate every seed URL's destination. Shared by the streaming and non-streaming crawl handlers so a - new entry point cannot silently skip the destination check.""" - urls = [('https://' + url) if not url.startswith(('http://', 'https://')) and not url.startswith(("raw:", "raw://")) else url for url in urls] + new entry point cannot silently skip the destination check. + + A refused seed does not fail the whole request. It is reported back as one + failed result among the batch, the way a robots.txt refusal already is, so + one bad URL in a sitemap-derived list cannot take a batch down (#2288). The + detail is carried through verbatim, which is what keeps this from becoming + a resolution oracle: an internal address and a name that does not resolve + both still produce the same opaque "URL blocked", and the caller already + knows the hostname it sent. A request with nothing left to crawl is still a + rejected request, and keeps the 400 (see _require_crawlable_seeds).""" + crawlable: List[str] = [] + refused: List[Tuple[str, str]] = [] for url in urls: - validate_url_destination(url) - return urls + url = ('https://' + url) if not url.startswith(('http://', 'https://')) and not url.startswith(("raw:", "raw://")) else url + try: + validate_url_destination(url) + except HTTPException as e: + refused.append((url, e.detail)) + continue + crawlable.append(url) + if refused: + # One line an operator can grep, so a refused seed is visible server-side + # and not just inferable from the response. The URLs are the caller's own + # and come straight back to it in the failed results. Each is truncated + # because a seed can be arbitrarily long, and a scanner probing blocked + # URLs should not be able to write megabytes into the log. + shown = ", ".join(url[:200] for url, _detail in refused[:5]) + if len(refused) > 5: + shown += f", +{len(refused) - 5} more" + logger.warning( + "[seeds] %d of %d seed(s) refused by the destination check, " + "reported as failed results: %s", + len(refused), len(urls), shown, + ) + return _SeedBatch(crawlable, refused) + + +def _require_crawlable_seeds(batch: _SeedBatch) -> None: + """Refuse a request that has no crawlable seed left. + + With every seed refused there is no batch to report per-URL failures in, so + this stays the 400 it has always been. That also keeps a single blocked URL + a 400, which is the contract the /md, /llm and /crawl/stream callers (and + the existing SSRF tests) rely on. + """ + if batch.refused and not batch.crawlable: + raise HTTPException(status_code=400, detail=batch.refused[0][1]) + + +def _refused_seed_result(url: str, detail: str): + """A refused seed as a failed CrawlResult. + + Shaped like the robots.txt refusal in async_webcrawler.arun() (success + False, 403, a marker header) so a caller handles one refused seed and one + robots-refused seed the same way. Built as a real CrawlResult so the key + set matches every other entry in `results` -- the SDK client rebuilds each + one with `CrawlResult(**result)`, so a partial dict would break it. + """ + return CrawlResult( + url=url, + html="", + success=False, + status_code=403, + error_message=detail, + response_headers={"X-Egress-Status": "Blocked by egress policy"}, + ) + + +def _append_refused_results(results: List, batch: _SeedBatch) -> List: + """Add one failed result per refused seed to `results`. + + Appended, not spliced into place: MemoryAdaptiveDispatcher.run_urls returns + in completion order, so `results` never lined up with the caller's `urls` to + begin with. Callers therefore already have to match a result to its seed by + `result.url` -- which is exactly the field that makes a refused seed + identifiable, and the thing the 400 gave them no way to recover. + """ + if not batch.refused: + return results + return list(results) + [ + _refused_seed_result(url, detail) for url, detail in batch.refused + ] + + +async def _prepend_refused_results(results_gen, batch: _SeedBatch): + """Yield one failed result per refused seed, then the crawled results. + + A refusal is known before the first byte is fetched, so it goes first. A + stream is completion-ordered like the batch path, so a caller matches + results to seeds by `result.url` here too. + + The results are built before the first yield on purpose: this generator is + consumed inside the response, where an exception would truncate a 200 the + client had already been given, with no error frame and no completion marker. + """ + for result in [_refused_seed_result(url, detail) for url, detail in batch.refused]: + yield result + async for result in results_gen: + yield result async def handle_crawl_request( @@ -684,7 +790,12 @@ async def handle_crawl_request( peak_mem_mb = start_mem_mb try: - urls = _normalize_and_validate_seeds(urls) + seeds = _normalize_and_validate_seeds(urls) + _require_crawlable_seeds(seeds) + # A refused seed comes back as a failed result rather than a 400, so the + # rest of this handler only ever sees the seeds it may actually fetch. + seed_count = len(urls) + urls = seeds.crawlable browser_config = BrowserConfig.load(browser_config, provenance=Provenance.UNTRUSTED) crawler_config = CrawlerRunConfig.load(crawler_config, provenance=Provenance.UNTRUSTED) from egress_broker import enforce_egress @@ -722,8 +833,14 @@ async def handle_crawl_request( base_config = config["crawler"]["base_config"] - # Build the config(s) to pass to arun/arun_many - if crawler_configs and len(urls) > 1: + # Build the config(s) to pass to arun/arun_many. + # A caller that sent per-URL crawler_configs still means them when a + # seed is refused, so the branch keys off the ORIGINAL url count, not + # the number that survived. The list itself is not filtered or + # reordered: the dispatcher pairs a config to a URL by url_matcher + # (see BaseDispatcher.select_config), not by position. + per_url_configs = bool(crawler_configs) and seed_count > 1 + if per_url_configs: # Per-URL config list: deserialize each and apply base_config config_list = [CrawlerRunConfig.load(cc, provenance=Provenance.UNTRUSTED) for cc in crawler_configs] for cfg in config_list: @@ -746,9 +863,15 @@ async def handle_crawl_request( effective_config = crawler_config results = [] - func = getattr(crawler, "arun" if len(urls) == 1 else "arun_many") + # arun_many unless exactly one seed is left, or the caller sent per-URL + # configs (arun_many is what pairs them to their URLs). The `!= 1` shape + # is deliberate: /crawl and /crawl/stream reject an empty urls list, but + # CrawlJobPayload.urls has no min_length, so arun_many([]) is still + # reachable here and must not become an arun(urls[0]) IndexError. + use_many = len(urls) != 1 or per_url_configs + func = getattr(crawler, "arun_many" if use_many else "arun") partial_func = partial(func, - urls[0] if len(urls) == 1 else urls, + urls if use_many else urls[0], config=effective_config, dispatcher=dispatcher) # Optional per-crawl wall-clock deadline (config limits.wall_clock_s; 0 = none). @@ -763,6 +886,9 @@ async def handle_crawl_request( if not isinstance(results, list): results = [results] + # Report one failed result for each seed the destination check refused. + results = _append_refused_results(results, seeds) + end_mem_mb = _get_memory_mb() # <--- Get memory after end_time = time.time() @@ -900,7 +1026,10 @@ async def handle_stream_crawl_request( # SSRF guard: validate every seed URL's destination before fetching, # mirroring handle_crawl_request. The streaming path previously skipped # this, leaving /crawl/stream (and /crawl with stream=true) unguarded. - urls = _normalize_and_validate_seeds(urls) + seeds = _normalize_and_validate_seeds(urls) + _require_crawlable_seeds(seeds) + seed_count = len(urls) + urls = seeds.crawlable browser_config = BrowserConfig.load( browser_config, provenance=Provenance.UNTRUSTED ) @@ -917,13 +1046,16 @@ async def handle_stream_crawl_request( clamp_deep_crawl(crawler_config) crawler_config.stream = True - # Deep crawl streaming supports exactly one start URL - if crawler_config.deep_crawl_strategy is not None and len(urls) != 1: + # Deep crawl streaming supports exactly one start URL. Keyed off the + # caller's own count: a request for several seeds is refused for that + # reason whether or not one of them was also refused by egress, and the + # deep-crawl limit is not this handler's business to relax. + if crawler_config.deep_crawl_strategy is not None and seed_count != 1: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=( "Deep crawling with stream currently supports exactly one URL per request. " - f"Received {len(urls)} URLs." + f"Received {seed_count} URLs." ), ) @@ -948,7 +1080,9 @@ async def handle_stream_crawl_request( hooks_info = {'status': hooks_status} # Deep crawl with single URL: use arun() which returns an async generator - # mirroring the Python library's streaming behavior + # mirroring the Python library's streaming behavior. + # The guard above rejects any request that did not send exactly one seed, + # so reaching here means seed_count == 1 and therefore len(urls) == 1. if crawler_config.deep_crawl_strategy is not None and len(urls) == 1: results_gen = await crawler.arun( urls[0], @@ -968,6 +1102,13 @@ async def handle_stream_crawl_request( dispatcher=dispatcher ) + # Yield the refused seeds first, so a streamed batch reports a refusal + # exactly like the non-streaming one does instead of dropping it. Their + # position in the stream is not the caller's `urls` position, because a + # stream's order is completion order, not request order. + if seeds.refused: + results_gen = _prepend_refused_results(results_gen, seeds) + return crawler, results_gen, hooks_info except (UntrustedConfigError, HookValidationError) as e: diff --git a/deploy/docker/tests/test_security_seed_batch.py b/deploy/docker/tests/test_security_seed_batch.py new file mode 100644 index 000000000..2f3010ac2 --- /dev/null +++ b/deploy/docker/tests/test_security_seed_batch.py @@ -0,0 +1,643 @@ +""" +#2288: one refused seed must not take a whole /crawl batch down. + +`_normalize_and_validate_seeds` used to raise on the first seed the destination +check turned away, so a single internal address -- or a single name that does +not resolve, e.g. a dead domain in a stale sitemap -- failed the whole request +with a 400 and no results at all. The caller could not tell WHICH seed it was, +so its only recovery was to split the batch and retry. + +A refused seed is now reported as one failed result among the batch, shaped like +the robots.txt refusal `AsyncWebCrawler.arun()` already returns. The opaque +message is carried through verbatim, so this does not become a resolution +oracle: an internal address and an NXDOMAIN still produce the same "URL +blocked", and the caller already knows the hostname it sent. + +A request with nothing crawlable left is still a 400 -- there is no batch to +report per-URL failures in, and that keeps a single blocked URL a 400 for the +/md, /llm and /crawl/stream callers. + +Behavioral tests run fully offline: `seeds_dns` models a resolver that fails +for specific hosts, and the crawler pool is mocked, so no browser is launched. +""" + +import asyncio +import ipaddress +import json +import socket + +import pytest +from fastapi import HTTPException + +import api +from api import ( + _append_refused_results, + _normalize_and_validate_seeds, + _prepend_refused_results, + _refused_seed_result, + _require_crawlable_seeds, + _SeedBatch, +) +from egress_broker import EgressBlocked + +pytestmark = pytest.mark.posture + +BLOCKED_DETAIL = "URL blocked (SSRF protection): URL blocked" +DEAD_HOST = "https://no-such-host-12345.example/" +GOOD_HOST = "https://example.com/" +INTERNAL = "http://169.254.169.254/latest/meta-data/" + + +# ─────────────────────────── fixtures ─────────────────────────── + +class _SeedsDns: + """Offline resolver for the seed-batch tests. + + Modelled on the real thing in the three ways these tests depend on: + + * a literal IP resolves to itself, so a literal internal address is + refused by the real `is_global` rule rather than by the fixture + (conftest's `offline_dns` answers every host with one public IP, which + would quietly allow `http://169.254.169.254/`); + * a host in `dead` fails to resolve, modelling the NXDOMAIN of a stale + sitemap entry; + * a host in `answers` resolves to the given IP, so a hostname can be made + internal; + * anything else resolves to one public address, so no test here can reach + the network by accident. + """ + + PUBLIC = "93.184.216.34" + + def __init__(self): + self.dead = set() + self.answers = {} + + def add_dead(self, *hosts): + self.dead.update(hosts) + + def answer(self, host, ip): + self.answers[host] = ip + + def getaddrinfo(self, host, port, *args, **kwargs): + if host in self.dead: + raise socket.gaierror(-2, "Name or service not known") + try: + # A literal address (v4 or v6) resolves to itself, which is what + # lets the real is_global rule reject http://[::1]/ and friends. + literal = ipaddress.ip_address(host) + except ValueError: + literal = None + if literal is not None: + ip, family = str(literal), ( + socket.AF_INET6 if literal.version == 6 else socket.AF_INET + ) + else: + ip, family = self.answers.get(host, self.PUBLIC), socket.AF_INET + return [(family, socket.SOCK_STREAM, 6, "", (ip, port or 0))] + + +@pytest.fixture +def seeds_dns(monkeypatch): + resolver = _SeedsDns() + monkeypatch.setattr(socket, "getaddrinfo", resolver.getaddrinfo) + return resolver + + +@pytest.fixture +def refuse_dns(seeds_dns): + """A resolver under which the classic internal targets are refused.""" + seeds_dns.answer("internal.example", "169.254.169.254") + seeds_dns.answer("ten.example", "10.0.0.1") + seeds_dns.answer("lan.example", "192.168.0.1") + return seeds_dns + + +def _crawled(url, success=True, error=None): + """A real CrawlResult, so model_dump()/serialization is exercised for real.""" + from crawl4ai.models import CrawlResult + + return CrawlResult( + url=url, + html="
hi" if success else "", + success=success, + status_code=200 if success else 500, + error_message=error, + ) + + +@pytest.fixture +def pooled_crawler(monkeypatch): + """Mock the pooled crawler so /crawl needs no browser. + + Returns (patch, calls) where `calls` records the urls/config each arun* + received, which is how the tests check the refused seed was never handed to + the crawler at all. + """ + import crawler_pool + + calls = {"arun": [], "arun_many": []} + + class Pooled: + active_requests = 0 + # _dispose_crawler branches on this; a non-PDF strategy takes the + # release_crawler path, which is mocked. + crawler_strategy = None + + async def arun(self, url, config=None, dispatcher=None): + calls["arun"].append(url) + return _crawled(url) + + async def arun_many(self, urls=None, config=None, dispatcher=None, **kw): + urls = list(urls or []) + calls["arun_many"].append(urls) + return [_crawled(u) for u in urls] + + crawler = Pooled() + + async def get_crawler(*a, **kw): + return crawler + + async def release_crawler(*a, **kw): + return None + + monkeypatch.setattr(crawler_pool, "get_crawler", get_crawler) + monkeypatch.setattr(crawler_pool, "release_crawler", release_crawler) + return crawler, calls + + +def _auth(): + from auth import create_access_token + + return {"Authorization": f"Bearer {create_access_token({'sub': 'u@x.com'})}"} + + +def _stock_crawl_config(): + """The `config` dict handle_crawl_request reads, as config.yml supplies it.""" + return { + "crawler": { + "memory_threshold_percent": 90, + "rate_limiter": {"enabled": False, "base_delay": [0.1, 0.3]}, + "base_config": {}, + }, + "limits": {"wall_clock_s": 0}, + } + + +# ───────────────── seed partitioning (unit) ───────────────── + +class TestSeedPartitioning: + def test_one_refused_seed_does_not_reject_the_batch(self, refuse_dns): + """The reported bug: an internal seed must not raise for its neighbours.""" + batch = _normalize_and_validate_seeds([GOOD_HOST, INTERNAL]) + + assert batch.crawlable == [GOOD_HOST] + assert batch.refused == [(INTERNAL, BLOCKED_DETAIL)] + + def test_unresolvable_seed_is_refused_not_fatal(self, seeds_dns): + """A dead domain from a stale sitemap is a per-URL failure, not a 400.""" + seeds_dns.add_dead("no-such-host-12345.example") + batch = _normalize_and_validate_seeds([GOOD_HOST, DEAD_HOST]) + + assert batch.crawlable == [GOOD_HOST] + assert batch.refused == [(DEAD_HOST, BLOCKED_DETAIL)] + + def test_refusal_message_is_identical_for_internal_and_nxdomain( + self, refuse_dns, seeds_dns + ): + """The no-oracle property: one message, so a refusal says nothing about + why the host was refused beyond what the caller already knows.""" + seeds_dns.add_dead("no-such-host-12345.example") + internal = _normalize_and_validate_seeds([INTERNAL]) + dead = _normalize_and_validate_seeds([DEAD_HOST]) + + assert internal.refused[0][1] == dead.refused[0][1] == BLOCKED_DETAIL + assert "169.254" not in internal.refused[0][1] + assert "no-such-host" not in dead.refused[0][1] + + def test_every_refused_seed_is_collected_not_just_the_first(self, refuse_dns): + """Validation must not stop at the first refusal.""" + batch = _normalize_and_validate_seeds( + [GOOD_HOST, "http://10.0.0.1/", "http://192.168.0.1/", GOOD_HOST + "b"] + ) + + assert batch.crawlable == [GOOD_HOST, GOOD_HOST + "b"] + assert [url for url, _ in batch.refused] == [ + "http://10.0.0.1/", + "http://192.168.0.1/", + ] + + def test_nothing_refused_is_a_passthrough(self, seeds_dns): + """Unchanged behaviour for a clean batch: same seeds, no refusals.""" + batch = _normalize_and_validate_seeds([GOOD_HOST, "https://iana.org/"]) + + assert batch.refused == [] + assert batch.crawlable == [GOOD_HOST, "https://iana.org/"] + + def test_bare_host_is_normalized_before_it_is_validated(self, refuse_dns): + """The https:// prefix is applied first, so 'localhost' is refused as + https://localhost and never reached the checker scheme-less.""" + batch = _normalize_and_validate_seeds(["internal.example"]) + + assert batch.crawlable == [] + assert batch.refused == [("https://internal.example", BLOCKED_DETAIL)] + + def test_raw_urls_stay_crawlable(self, seeds_dns): + """raw: is inline HTML with no network fetch, so it is never refused.""" + batch = _normalize_and_validate_seeds(["raw:hello
", "raw://hi
"]) + + assert batch.crawlable == ["raw:hello
", "raw://hi
"] + assert batch.refused == [] + + def test_refused_url_echoes_the_normalized_form(self, refuse_dns): + """The refused record carries the normalized URL, because that is the + string the caller has to recognise in the response.""" + batch = _normalize_and_validate_seeds(["internal.example"]) + + assert batch.refused[0][0] == "https://internal.example" + + +# ───────────────── all-refused stays a 400 ───────────────── + +class TestRequireCrawlableSeeds: + def test_all_refused_raises_the_same_400(self, refuse_dns): + batch = _normalize_and_validate_seeds([INTERNAL, "http://10.0.0.1/"]) + + with pytest.raises(HTTPException) as raised: + _require_crawlable_seeds(batch) + + assert raised.value.status_code == 400 + assert raised.value.detail == BLOCKED_DETAIL + + def test_partially_refused_does_not_raise(self, refuse_dns): + _require_crawlable_seeds(_normalize_and_validate_seeds([GOOD_HOST, INTERNAL])) + + def test_clean_batch_does_not_raise(self, seeds_dns): + _require_crawlable_seeds(_normalize_and_validate_seeds([GOOD_HOST])) + + def test_empty_batch_does_not_raise(self): + """No seeds is a request-shape error handled by the route, not a refusal.""" + _require_crawlable_seeds(_SeedBatch([], [])) + + +# ───────────────── the failed result's shape ───────────────── + +class TestRefusedResultShape: + def test_matches_the_robots_txt_refusal(self): + """Same contract as the robots.txt refusal in async_webcrawler.arun(): + success False, 403, the reason in error_message.""" + result = _refused_seed_result(INTERNAL, BLOCKED_DETAIL) + + assert result.url == INTERNAL + assert result.success is False + assert result.status_code == 403 + assert result.error_message == BLOCKED_DETAIL + assert result.response_headers == {"X-Egress-Status": "Blocked by egress policy"} + + def test_serializes_with_the_same_keys_as_a_crawled_result(self): + """A refused seed is a CrawlResult, not a hand-rolled dict, so a client + reading result['links'] / result['media'] does not hit a KeyError.""" + dumped = _refused_seed_result(INTERNAL, BLOCKED_DETAIL).model_dump() + crawled = _crawled(GOOD_HOST).model_dump() + + assert set(dumped) == set(crawled) + assert dumped["links"] == {} and dumped["media"] == {} + + def test_is_json_serializable_for_the_response(self): + payload = _refused_seed_result(INTERNAL, BLOCKED_DETAIL).model_dump() + assert json.loads(json.dumps(payload, default=str))["url"] == INTERNAL + + def test_survives_the_docker_clients_reconstruction(self): + """Crawl4aiDockerClient.crawl does `CrawlResult(**r)` on every result, so + a refused seed has to reconstruct into a CrawlResult like any other.""" + from crawl4ai.models import CrawlResult + + dumped = _refused_seed_result(INTERNAL, BLOCKED_DETAIL).model_dump() + rebuilt = CrawlResult(**dumped) + + assert rebuilt.success is False + assert rebuilt.status_code == 403 + assert rebuilt.error_message == BLOCKED_DETAIL + + +class TestAppendRefusedResults: + def test_appends_one_failure_per_refused_seed(self): + crawled = [_crawled(GOOD_HOST)] + batch = _SeedBatch([GOOD_HOST], [(INTERNAL, BLOCKED_DETAIL)]) + + out = _append_refused_results(crawled, batch) + + assert len(out) == 2 + assert out[0].url == GOOD_HOST + assert out[1].url == INTERNAL + assert out[1].success is False + + def test_does_not_mutate_the_crawlers_list(self): + crawled = [_crawled(GOOD_HOST)] + _append_refused_results(crawled, _SeedBatch([GOOD_HOST], [(INTERNAL, BLOCKED_DETAIL)])) + + assert len(crawled) == 1 + + def test_no_refusals_returns_the_crawlers_list_untouched(self): + """Documented fast path: with nothing refused the crawler's own list is + handed back, so a clean batch is not copied.""" + crawled = [_crawled(GOOD_HOST)] + + out = _append_refused_results(crawled, _SeedBatch([GOOD_HOST], [])) + + assert out is crawled + + +class TestPrependRefusedResults: + @pytest.mark.asyncio + async def test_refusals_are_yielded_then_the_crawled_results(self): + async def gen(): + yield _crawled(GOOD_HOST) + + batch = _SeedBatch([GOOD_HOST], [(INTERNAL, BLOCKED_DETAIL)]) + out = [r async for r in _prepend_refused_results(gen(), batch)] + + assert [r.url for r in out] == [INTERNAL, GOOD_HOST] + assert out[0].success is False + + +# ───────────────── behavioral: /crawl ───────────────── + +class TestCrawlBatchBehavioral: + def test_reported_repro_returns_200_with_one_failed_result( + self, stock_client, seeds_dns, pooled_crawler + ): + """The issue's exact input, as a client.""" + seeds_dns.add_dead("no-such-host-12345.example") + _crawler, calls = pooled_crawler + + r = stock_client.post( + "/crawl", json={"urls": [GOOD_HOST, DEAD_HOST]}, headers=_auth() + ) + + assert r.status_code == 200, r.text[:300] + body = r.json() + assert body["success"] is True + by_url = {res["url"]: res for res in body["results"]} + assert set(by_url) == {GOOD_HOST, DEAD_HOST} + assert by_url[GOOD_HOST]["success"] is True + assert by_url[DEAD_HOST]["success"] is False + assert by_url[DEAD_HOST]["error_message"] == BLOCKED_DETAIL + + def test_the_refused_seed_is_never_handed_to_the_crawler( + self, stock_client, refuse_dns, pooled_crawler + ): + """The SSRF check is a gate, not a filter applied after the fetch: the + refused URL must not appear in the crawler's work at all.""" + _crawler, calls = pooled_crawler + + r = stock_client.post( + "/crawl", + json={"urls": [GOOD_HOST, "http://10.0.0.1/", "https://iana.org/"]}, + headers=_auth(), + ) + + assert r.status_code == 200, r.text[:300] + crawled = calls["arun_many"] + calls["arun"] + assert crawled == [[GOOD_HOST, "https://iana.org/"]] + assert "http://10.0.0.1/" not in crawled[0] + + def test_an_internal_address_still_fails_and_still_blocks_the_fetch( + self, stock_client, refuse_dns, pooled_crawler + ): + """A refused seed in a batch is a failed result, not a silent success and + not a fetch of the internal address.""" + _crawler, calls = pooled_crawler + + r = stock_client.post( + "/crawl", json={"urls": [GOOD_HOST, INTERNAL]}, headers=_auth() + ) + + assert r.status_code == 200, r.text[:300] + blocked = [res for res in r.json()["results"] if res["url"] == INTERNAL] + assert len(blocked) == 1 + assert blocked[0]["success"] is False + assert blocked[0]["error_message"] == BLOCKED_DETAIL + # One crawlable seed left, so arun was used -- and the refused URL was + # not the argument. + assert calls["arun"] == [GOOD_HOST] + assert calls["arun_many"] == [] + + def test_single_refused_url_still_400(self, stock_client, refuse_dns, pooled_crawler): + """The one-URL contract the other endpoints and clients rely on.""" + r = stock_client.post("/crawl", json={"urls": [INTERNAL]}, headers=_auth()) + + assert r.status_code == 400, f"got {r.status_code}: {r.text[:200]}" + assert BLOCKED_DETAIL in r.text + + def test_batch_where_every_seed_is_refused_still_400( + self, stock_client, refuse_dns, pooled_crawler + ): + _crawler, calls = pooled_crawler + + r = stock_client.post( + "/crawl", + json={"urls": [INTERNAL, "http://10.0.0.1/"]}, + headers=_auth(), + ) + + assert r.status_code == 400, f"got {r.status_code}: {r.text[:200]}" + assert calls["arun"] == [] and calls["arun_many"] == [] + + def test_clean_batch_is_unaffected(self, stock_client, seeds_dns, pooled_crawler): + r = stock_client.post( + "/crawl", json={"urls": [GOOD_HOST, "https://iana.org/"]}, headers=_auth() + ) + + assert r.status_code == 200, r.text[:300] + results = r.json()["results"] + assert len(results) == 2 + assert all(res["success"] for res in results) + + def test_empty_url_list_is_still_an_empty_success(self, pooled_crawler): + """CrawlJobPayload.urls has no min_length and /crawl/job has no emptiness + guard, so an empty list reaches the handler. It must keep returning the + empty success it always did, not IndexError on urls[0] into a 500.""" + _crawler, calls = pooled_crawler + + result = asyncio.run( + api.handle_crawl_request( + urls=[], + browser_config={}, + crawler_config={}, + config=_stock_crawl_config(), + ) + ) + + assert result["success"] is True + assert result["results"] == [] + assert calls["arun"] == [] + assert calls["arun_many"] == [[]] + + def test_per_url_crawler_configs_survive_a_refused_seed( + self, stock_client, refuse_dns, monkeypatch + ): + """A caller that sent per-URL configs still means them when a seed is + refused. The list reaches arun_many unfiltered, because the dispatcher + pairs a config to a URL by url_matcher and not by position -- filtering + it by the surviving seeds would drop the config that matches. + """ + from crawl4ai.async_dispatcher import BaseDispatcher + + seen = {} + + class ConfigCrawler: + active_requests = 0 + crawler_strategy = None + + async def arun_many(self, urls=None, config=None, dispatcher=None, **kw): + seen["urls"] = list(urls or []) + seen["config"] = config + return [_crawled(u) for u in seen["urls"]] + + crawler = ConfigCrawler() + + async def get_crawler(*a, **kw): + return crawler + + async def release_crawler(*a, **kw): + return None + + import crawler_pool + + monkeypatch.setattr(crawler_pool, "get_crawler", get_crawler) + monkeypatch.setattr(crawler_pool, "release_crawler", release_crawler) + + r = stock_client.post( + "/crawl", + json={ + "urls": [GOOD_HOST, INTERNAL, "https://iana.org/"], + "crawler_configs": [ + {"type": "CrawlerRunConfig", + "params": {"screenshot": True, "url_matcher": "*example.com*"}}, + {"type": "CrawlerRunConfig", + "params": {"word_count_threshold": 5, "url_matcher": "*iana.org*"}}, + # A catch-all, so one config is not needed per URL. + {"type": "CrawlerRunConfig", "params": {}}, + ], + }, + headers=_auth(), + ) + + assert r.status_code == 200, r.text[:300] + assert seen["urls"] == [GOOD_HOST, "https://iana.org/"] + # Unfiltered: all three survive, including the ones that only matched a + # refused seed's position would have shifted. + assert len(seen["config"]) == 3 + # And the pairing the dispatcher will do still resolves per URL. + assert BaseDispatcher.select_config(None, GOOD_HOST, seen["config"]).screenshot is True + assert BaseDispatcher.select_config( + None, "https://iana.org/", seen["config"] + ).word_count_threshold == 5 + + +# ───────────────── behavioral: /crawl/stream ───────────────── + +def _stream_results(response): + """Parse an NDJSON crawl stream into its result objects. + + The stream is a result object per line, then a trailing + {"status": "completed"} marker. + """ + results = [] + for line in response.iter_lines(): + if not line: + continue + obj = json.loads(line) + if isinstance(obj, dict) and "url" in obj: + results.append(obj) + return results + + +class TestStreamBatchBehavioral: + def test_refused_seed_is_streamed_as_a_failed_result( + self, stock_client, refuse_dns, monkeypatch + ): + """The streaming path shares the seed check, so it must report the + refusal the same way instead of 400-ing the whole stream.""" + import crawler_pool + + seen = {} + + class Pooled: + active_requests = 0 + crawler_strategy = None + + async def arun_many(self, urls=None, config=None, dispatcher=None, **kw): + urls = list(urls or []) + seen["urls"] = urls + + async def gen(): + for url in urls: + yield _crawled(url) + + return gen() + + crawler = Pooled() + + async def get_crawler(*a, **kw): + return crawler + + async def release_crawler(*a, **kw): + return None + + monkeypatch.setattr(crawler_pool, "get_crawler", get_crawler) + monkeypatch.setattr(crawler_pool, "release_crawler", release_crawler) + + with stock_client.stream( + "POST", "/crawl/stream", json={"urls": [INTERNAL, GOOD_HOST]}, + headers=_auth(), + ) as r: + assert r.status_code == 200, r.read().decode()[:300] + results = _stream_results(r) + + by_url = {res["url"]: res for res in results} + # The gate is a gate: the refused seed is never handed to the crawler, + # so it is absent from the stream and present only as a failed result. + assert seen["urls"] == [GOOD_HOST] + assert INTERNAL in by_url, f"refusal missing from stream: {results}" + assert by_url[INTERNAL]["success"] is False + assert by_url[INTERNAL]["error_message"] == BLOCKED_DETAIL + assert by_url[INTERNAL]["status_code"] == 403 + assert by_url[GOOD_HOST]["success"] is True + + def test_single_refused_url_still_400s_the_stream( + self, stock_client, refuse_dns + ): + r = stock_client.post("/crawl/stream", json={"urls": [INTERNAL]}, headers=_auth()) + + assert r.status_code == 400, f"got {r.status_code}: {r.text[:200]}" + assert BLOCKED_DETAIL in r.text + + +# ───────────────── the guard itself still guards ───────────────── + +class TestNoOracleRegression: + def test_refused_detail_never_leaks_the_address_or_host(self, refuse_dns): + """Belt and braces on the property the issue depends on: whatever the + reason, the refused detail is the one opaque string.""" + for url in ( + INTERNAL, + "http://10.0.0.1/", + "http://192.168.1.1/", + "http://localhost:8080/", + "http://[::1]/", + "http://host.docker.internal:3000/", + ): + batch = _normalize_and_validate_seeds([url]) + assert batch.refused, f"{url} was not refused" + detail = batch.refused[0][1] + assert detail == BLOCKED_DETAIL, f"{url} -> {detail}" + + def test_egress_blocked_still_raises_for_the_direct_call(self, refuse_dns): + """The broker's own contract is untouched: resolve_and_pin still raises + rather than returning a verdict, so no other caller can be softened.""" + from egress_broker import resolve_and_pin + + with pytest.raises(EgressBlocked): + resolve_and_pin(INTERNAL) diff --git a/tests/test_docker_pdf_crawler_pairing.py b/tests/test_docker_pdf_crawler_pairing.py index 2749d6407..7f64d1f2f 100644 --- a/tests/test_docker_pdf_crawler_pairing.py +++ b/tests/test_docker_pdf_crawler_pairing.py @@ -54,7 +54,12 @@ def pool_mock(api, monkeypatch): pooled.active_requests = 1 # release_crawler decrements this int mock = AsyncMock(return_value=pooled) monkeypatch.setattr(crawler_pool, "get_crawler", mock) - monkeypatch.setattr(api, "_normalize_and_validate_seeds", lambda urls: urls) + # Pass every seed through: these tests are about the PDF crawler pairing, so + # the destination check is stubbed out rather than exercised. The stub must + # still return a _SeedBatch (see _normalize_and_validate_seeds). + monkeypatch.setattr( + api, "_normalize_and_validate_seeds", lambda urls: api._SeedBatch(list(urls), []) + ) monkeypatch.setattr(egress_broker, "enforce_egress", lambda _: None) monkeypatch.setattr(governor, "clamp_deep_crawl", lambda _: None) return mock diff --git a/tests/test_issue_2127_docker_pdf.py b/tests/test_issue_2127_docker_pdf.py index 952f1daa2..6802ebd1d 100644 --- a/tests/test_issue_2127_docker_pdf.py +++ b/tests/test_issue_2127_docker_pdf.py @@ -37,7 +37,11 @@ async def test_stream_handler_preserves_requested_scraping_strategy(monkeypatch) crawler = MagicMock() crawler.arun_many = AsyncMock(return_value=MagicMock()) crawler.start = AsyncMock() - monkeypatch.setattr(api, "_normalize_and_validate_seeds", lambda urls: urls) + monkeypatch.setattr( + api, + "_normalize_and_validate_seeds", + lambda urls: api._SeedBatch(list(urls), []), + ) monkeypatch.setattr(egress_broker, "enforce_egress", lambda _: None) monkeypatch.setattr(governor, "clamp_deep_crawl", lambda _: None) monkeypatch.setattr(api, "AsyncWebCrawler", MagicMock(return_value=crawler))