diff --git a/crawl4ai/async_configs.py b/crawl4ai/async_configs.py index 3efb0d380..92ae71ed6 100644 --- a/crawl4ai/async_configs.py +++ b/crawl4ai/async_configs.py @@ -270,11 +270,46 @@ class UntrustedConfigError(ValueError): } # Upper bounds applied to attacker-influenced quantities after filtering. -_MAX_TIMEOUT_MS = 60_000 +_DEFAULT_MAX_TIMEOUT_MS = 60_000 _MAX_SCROLL_STEPS = 1000 _MAX_VIEWPORT = 4000 +def _max_timeout_ms() -> int: + """Ceiling for the untrusted timeout fields, in milliseconds. + + 60s is the right bound for a server reachable by untrusted callers, and + stays the default. An operator whose deployment is not public — a crawler + on a private network fetching pages that legitimately take minutes — can + raise it with CRAWL4AI_MAX_TIMEOUT_MS, or lower it to tighten the bound. + + Read per call rather than captured at import so the setting applies + wherever the process picked its environment up, and so a test can set it + without reloading the module. A value that is not a positive integer is + refused loudly and the default kept: a typo here would silently widen a + DoS bound, which is the one outcome worse than the timeout being fixed. + """ + raw = os.getenv("CRAWL4AI_MAX_TIMEOUT_MS") + + if raw is None or raw == "": + return _DEFAULT_MAX_TIMEOUT_MS + + try: + ceiling = int(raw) + except ValueError: + ceiling = 0 + + if ceiling <= 0: + warnings.warn( + f"CRAWL4AI_MAX_TIMEOUT_MS={raw!r} is not a positive integer; " + f"keeping the {_DEFAULT_MAX_TIMEOUT_MS}ms default.", + stacklevel=2, + ) + return _DEFAULT_MAX_TIMEOUT_MS + + return ceiling + + def _filter_untrusted_fields(type_name: str, params: dict) -> dict: """Drop non-allowlisted fields and raise on forbidden (power) fields.""" forbidden = UNTRUSTED_FORBIDDEN_FIELDS.get(type_name, set()) @@ -293,11 +328,13 @@ def _filter_untrusted_fields(type_name: str, params: dict) -> dict: def _clamp_untrusted(type_name: str, params: dict) -> dict: """Clamp attacker-influenced quantities to safe upper bounds.""" + ceiling = _max_timeout_ms() + def _cap_timeout(v): # 0 historically meant "no timeout"; treat as the cap, never unbounded. if not isinstance(v, (int, float)) or v <= 0: - return _MAX_TIMEOUT_MS - return min(int(v), _MAX_TIMEOUT_MS) + return ceiling + return min(int(v), ceiling) if type_name == "CrawlerRunConfig": for f in ("page_timeout", "wait_for_timeout", "body_visibility_timeout"): diff --git a/deploy/docker/MIGRATION.md b/deploy/docker/MIGRATION.md index bcd2097c7..c508e220f 100644 --- a/deploy/docker/MIGRATION.md +++ b/deploy/docker/MIGRATION.md @@ -170,6 +170,25 @@ limits: To keep the previous behavior exactly, set the caps you don't want to `0`. +### Timeouts from a request are capped at 60s + +`page_timeout`, `wait_for_timeout`, and `body_visibility_timeout` arriving in a +request body are clamped to 60000ms, so a client asking for more is given 60s +and its crawl fails with `Page.goto: Timeout 60000ms exceeded`. + +That bound is right for a server reachable by untrusted callers. A deployment +that is not public — a crawler on a private network fetching pages that +legitimately take minutes — can raise it: + +```bash +CRAWL4AI_MAX_TIMEOUT_MS=300000 +``` + +A request still only gets the timeout it asks for; this sets the ceiling, and +a smaller value tightens it. A value that is not a positive integer is refused +with a warning and the 60000ms default kept, so a typo cannot silently widen +the bound. + ### Error responses are generic 5xx responses return `{"error": "Internal server error", "correlation_id": "…"}`. diff --git a/tests/test_config_defaults.py b/tests/test_config_defaults.py index 3a7fba641..ad86f92f0 100644 --- a/tests/test_config_defaults.py +++ b/tests/test_config_defaults.py @@ -1,11 +1,12 @@ """Tests for BrowserConfig.set_defaults / CrawlerRunConfig.set_defaults.""" +import warnings from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock import pytest -from crawl4ai.async_configs import BrowserConfig, CrawlerRunConfig +from crawl4ai.async_configs import BrowserConfig, CrawlerRunConfig, Provenance from crawl4ai.async_crawler_strategy import AsyncPlaywrightCrawlerStrategy @@ -310,3 +311,80 @@ def test_independent_reset(self): BrowserConfig.reset_defaults() assert BrowserConfig.get_defaults() == {} assert CrawlerRunConfig.get_defaults() == {"verbose": False} + + +# ── Untrusted timeout ceiling ────────────────────────────────────────── + + +class TestMaxTimeoutCeiling: + """CRAWL4AI_MAX_TIMEOUT_MS raises (or lowers) the untrusted clamp.""" + + TIMEOUT_FIELDS = ("page_timeout", "wait_for_timeout", "body_visibility_timeout") + + @pytest.mark.parametrize("field", TIMEOUT_FIELDS) + def test_defaults_to_60s_when_unset(self, monkeypatch, field): + monkeypatch.delenv("CRAWL4AI_MAX_TIMEOUT_MS", raising=False) + + config = CrawlerRunConfig.load( + {field: 500_000}, provenance=Provenance.UNTRUSTED + ) + + assert getattr(config, field) == 60_000 + + @pytest.mark.parametrize("field", TIMEOUT_FIELDS) + def test_env_raises_the_ceiling(self, monkeypatch, field): + monkeypatch.setenv("CRAWL4AI_MAX_TIMEOUT_MS", "300000") + + config = CrawlerRunConfig.load( + {field: 300_000}, provenance=Provenance.UNTRUSTED + ) + + assert getattr(config, field) == 300_000 + + def test_a_request_over_the_raised_ceiling_is_still_clamped(self, monkeypatch): + monkeypatch.setenv("CRAWL4AI_MAX_TIMEOUT_MS", "300000") + + config = CrawlerRunConfig.load( + {"page_timeout": 900_000}, provenance=Provenance.UNTRUSTED + ) + + assert config.page_timeout == 300_000 + + def test_env_can_tighten_the_ceiling(self, monkeypatch): + monkeypatch.setenv("CRAWL4AI_MAX_TIMEOUT_MS", "5000") + + config = CrawlerRunConfig.load( + {"page_timeout": 30_000}, provenance=Provenance.UNTRUSTED + ) + + assert config.page_timeout == 5_000 + + # A typo must not silently widen a DoS bound, so the default is kept and + # the operator is told rather than left to find out under load. + @pytest.mark.parametrize("value", ["", "abc", "0", "-1", "60_000", "1e5"]) + def test_a_non_positive_integer_keeps_the_default(self, monkeypatch, value): + monkeypatch.setenv("CRAWL4AI_MAX_TIMEOUT_MS", value) + + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + config = CrawlerRunConfig.load( + {"page_timeout": 500_000}, provenance=Provenance.UNTRUSTED + ) + + assert config.page_timeout == 60_000 + + @pytest.mark.parametrize("value", ["abc", "0", "-1"]) + def test_a_bad_value_warns(self, monkeypatch, value): + monkeypatch.setenv("CRAWL4AI_MAX_TIMEOUT_MS", value) + + with pytest.warns(UserWarning, match="CRAWL4AI_MAX_TIMEOUT_MS"): + CrawlerRunConfig.load( + {"page_timeout": 1_000}, provenance=Provenance.UNTRUSTED + ) + + def test_trusted_config_is_never_clamped(self, monkeypatch): + monkeypatch.delenv("CRAWL4AI_MAX_TIMEOUT_MS", raising=False) + + config = CrawlerRunConfig(page_timeout=900_000) + + assert config.page_timeout == 900_000