Skip to content

Commit 470b435

Browse files
authored
Validate Prefect pagination URLs (#23354)
* Fix Prefect event pagination host validation * Add changelog * Rename changelog * Change changelog
1 parent 339fbaf commit 470b435

3 files changed

Lines changed: 72 additions & 2 deletions

File tree

prefect/changelog.d/23354.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Adds validation for event pagination URLs.

prefect/datadog_checks/prefect/check.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from functools import cached_property
99
from json import JSONDecodeError
1010
from typing import TYPE_CHECKING, Any, Literal
11+
from urllib.parse import urlparse
1112

1213
from requests.exceptions import ConnectionError, HTTPError, InvalidURL, Timeout
1314

@@ -667,6 +668,9 @@ class PrefectClient:
667668
def __init__(self, url: str, http: RequestsWrapper, log: CheckLoggingAdapter):
668669
self.http_exceptions = (HTTPError, InvalidURL, ConnectionError, Timeout, JSONDecodeError)
669670
self.url = url
671+
parsed_url = urlparse(url)
672+
self.base_url_scheme = parsed_url.scheme
673+
self.base_url_netloc = parsed_url.netloc
670674
self.http = http
671675
self.log = log
672676

@@ -689,6 +693,11 @@ def post(self, endpoint: str, payload: dict | None = None) -> Any:
689693
except self.http_exceptions:
690694
raise
691695

696+
def check_pagination_url(self, next_page: str):
697+
parsed_next_page = urlparse(next_page)
698+
if parsed_next_page.scheme != self.base_url_scheme or parsed_next_page.netloc != self.base_url_netloc:
699+
raise InvalidURL(f'Invalid next_page URL with unexpected host: {next_page}')
700+
692701
def paginate_filter(self, endpoint: str, payload: dict | None = None) -> list[dict]:
693702
"""Implements pagination for /filter endpoints using limit/offset loop."""
694703
payload = dict(payload) if payload else {}
@@ -728,9 +737,10 @@ def paginate_events(self, endpoint: str, payload: dict | None = None) -> list[di
728737
events.extend(response.get("events", []))
729738
if not response.get("next_page"):
730739
break
731-
732740
try:
733-
response = self.get(response.get("next_page"), pagination=True)
741+
next_page = response.get("next_page")
742+
self.check_pagination_url(next_page)
743+
response = self.get(next_page, pagination=True)
734744
except self.http_exceptions as e:
735745
self.log.error("Could not collect next page of events: %s, data is incomplete", e)
736746
return events

prefect/tests/test_unit.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
from datetime import datetime, timezone
22
from typing import Callable, NamedTuple
3+
from unittest.mock import Mock
34

45
import pytest
6+
from requests.exceptions import InvalidURL
57

68
from datadog_checks.base.stubs.aggregator import AggregatorStub
79
from datadog_checks.dev.utils import get_metadata_metrics
810
from datadog_checks.prefect import PrefectCheck
11+
from datadog_checks.prefect.check import PrefectClient
912

1013
WP1_TAGS = ["work_pool_id:wp-1", "work_pool_name:default-pool", "work_pool_type:process"]
1114
WP2_TAGS = ["work_pool_id:wp-2", "work_pool_name:paused-pool", "work_pool_type:docker"]
@@ -600,6 +603,62 @@ class EventCase(NamedTuple):
600603
pytestmark = [pytest.mark.usefixtures("mock_prefect_client"), pytest.mark.unit]
601604

602605

606+
class MockResponse:
607+
def __init__(self, payload: dict):
608+
self.payload = payload
609+
610+
def raise_for_status(self):
611+
return None
612+
613+
def json(self) -> dict:
614+
return self.payload
615+
616+
617+
def test_paginate_events_rejects_external_next_page():
618+
http = Mock()
619+
http.post.return_value = MockResponse(
620+
{
621+
"events": [{"id": "event-1"}],
622+
"next_page": "http://attacker.example/evil",
623+
}
624+
)
625+
log = Mock()
626+
client = PrefectClient("http://prefect.local/api", http, log)
627+
628+
events = client.paginate_events("/events/filter", {})
629+
630+
assert events == [{"id": "event-1"}]
631+
http.get.assert_not_called()
632+
log.error.assert_called_once()
633+
args = log.error.call_args[0]
634+
assert args[0] == "Could not collect next page of events: %s, data is incomplete"
635+
assert isinstance(args[1], InvalidURL)
636+
assert str(args[1]) == "Invalid next_page URL with unexpected host: http://attacker.example/evil"
637+
638+
639+
def test_paginate_events_allows_same_host_absolute_next_page():
640+
http = Mock()
641+
http.post.return_value = MockResponse(
642+
{
643+
"events": [{"id": "event-1"}],
644+
"next_page": "http://prefect.local/api/events/filter?page=2",
645+
}
646+
)
647+
http.get.return_value = MockResponse(
648+
{
649+
"events": [{"id": "event-2"}],
650+
"next_page": None,
651+
}
652+
)
653+
log = Mock()
654+
client = PrefectClient("http://prefect.local/api", http, log)
655+
656+
events = client.paginate_events("/events/filter", {})
657+
658+
assert events == [{"id": "event-1"}, {"id": "event-2"}]
659+
http.get.assert_called_once_with("http://prefect.local/api/events/filter?page=2")
660+
661+
603662
@pytest.fixture()
604663
def ready_check(check: PrefectCheck, dd_run_check: Callable, aggregator, mocker) -> PrefectCheck:
605664
mocker.patch(

0 commit comments

Comments
 (0)