From b3a01b396a8b97bb88a7587f92bd723f1ab0eaf2 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:04:49 +0000 Subject: [PATCH] fix(dashboards): validate threshold JSON shape --- .../serializers/rest_framework/dashboard.py | 69 ++++++++++++------- .../test_organization_dashboard_details.py | 67 ++++++++++++++++++ 2 files changed, 111 insertions(+), 25 deletions(-) diff --git a/src/sentry/api/serializers/rest_framework/dashboard.py b/src/sentry/api/serializers/rest_framework/dashboard.py index 524f9a8490b1..c7b8d33549c0 100644 --- a/src/sentry/api/serializers/rest_framework/dashboard.py +++ b/src/sentry/api/serializers/rest_framework/dashboard.py @@ -909,46 +909,65 @@ def validate(self, data: dict[str, Any]) -> dict[str, Any]: {"thresholds": "Heatmap widgets do not support thresholds."} ) - if thresholds: + if thresholds is not None: + if not isinstance(thresholds, dict): + raise serializers.ValidationError({"thresholds": "Must be an object."}) + max_values = thresholds.get("max_values") allowed_max_keys = [key.value for key in ThresholdMaxKeys] - if max_values: - for i in range(len(max_values)): - max_key = f"max{i + 1}" - - if max_key not in allowed_max_keys: + if max_values is not None: + if not isinstance(max_values, dict): + raise serializers.ValidationError( + {"thresholds": {"max_values": "Must be an object."}} + ) + if max_values: + invalid_keys = set(max_values) - set(allowed_max_keys) + if invalid_keys: raise serializers.ValidationError( - {"thresholds": f"Invalid maximum key {max_key}"} + {"thresholds": f"Invalid maximum key {sorted(invalid_keys)[0]}"} ) - if max_values.get(max_key): - if max_values.get(max_key) < 0: + for max_key, max_value in max_values.items(): + if not isinstance(max_value, (int, float)) or isinstance(max_value, bool): + raise serializers.ValidationError( + {"thresholds": {max_key: "Maximum values must be numbers."}} + ) + if max_value < 0: raise serializers.ValidationError( {"thresholds": {max_key: "Maximum values can not be negative"}} ) - elif i > 0: - prev_max_key = f"max{i}" - if max_values.get(prev_max_key) and max_values.get( - prev_max_key - ) >= max_values.get(max_key): - raise serializers.ValidationError( - { - "thresholds": { - max_key: "Maximum value must be greater than minimum." - } - } - ) - if len(max_values) < len(ThresholdMaxKeys): - for key in allowed_max_keys: - if max_values.get(key) is None: + for prev_max_key, max_key in zip( + allowed_max_keys, allowed_max_keys[1:], strict=False + ): + if ( + prev_max_key in max_values + and max_key in max_values + and max_values[prev_max_key] >= max_values[max_key] + ): raise serializers.ValidationError( { "thresholds": { - key: "Must set all threshold maximums or none at all." + max_key: "Maximum value must be greater than minimum." } } ) + + if len(max_values) < len(ThresholdMaxKeys): + for key in allowed_max_keys: + if key not in max_values: + raise serializers.ValidationError( + { + "thresholds": { + key: "Must set all threshold maximums or none at all." + } + } + ) + + unit = thresholds.get("unit") + if unit is not None and not isinstance(unit, str): + raise serializers.ValidationError({"thresholds": {"unit": "Must be a string."}}) + preferred_polarity = thresholds.get("preferred_polarity") if preferred_polarity is not None and preferred_polarity not in ("+", "-", ""): raise serializers.ValidationError( diff --git a/tests/sentry/dashboards/endpoints/test_organization_dashboard_details.py b/tests/sentry/dashboards/endpoints/test_organization_dashboard_details.py index b40cd7eee8cd..a09c09d59c71 100644 --- a/tests/sentry/dashboards/endpoints/test_organization_dashboard_details.py +++ b/tests/sentry/dashboards/endpoints/test_organization_dashboard_details.py @@ -2158,6 +2158,73 @@ def test_update_widget_with_invalid_preferred_polarity(self) -> None: == "Must be '+', '-', or empty string." ) + def test_update_widget_with_invalid_thresholds(self) -> None: + cases: tuple[tuple[object, str], ...] = ( + ([], "Must be an object."), + ({"max_values": []}, "Must be an object."), + ( + {"max_values": {"max1": 100, "max3": 300}}, + "Invalid maximum key max3", + ), + ( + {"max_values": {"max1": "100", "max2": 200}}, + "Maximum values must be numbers.", + ), + ( + {"max_values": {"max1": 100, "max2": 200}, "unit": 5}, + "Must be a string.", + ), + ) + for thresholds, message in cases: + with self.subTest(thresholds=thresholds): + data = { + "title": "Dashboard", + "widgets": [ + { + "id": str(self.widget_1.id), + "title": "Big Number with Invalid Thresholds", + "displayType": "big_number", + "thresholds": thresholds, + "queries": [ + { + "name": "", + "fields": ["count()"], + "columns": [], + "aggregates": ["count()"], + "conditions": "", + } + ], + }, + ], + } + response = self.do_request("put", self.url(self.dashboard.id), data=data) + assert response.status_code == 400, response.data + assert message in str(response.data) + + def test_update_widget_with_decimal_thresholds(self) -> None: + data = { + "title": "Dashboard", + "widgets": [ + { + "id": str(self.widget_1.id), + "title": "Big Number with Decimal Thresholds", + "displayType": "big_number", + "thresholds": {"max_values": {"max1": 0.1, "max2": 0.25}, "unit": None}, + "queries": [ + { + "name": "", + "fields": ["count()"], + "columns": [], + "aggregates": ["count()"], + "conditions": "", + } + ], + }, + ], + } + response = self.do_request("put", self.url(self.dashboard.id), data=data) + assert response.status_code == 200, response.data + def test_update_widget_with_invalid_threshold_time_window(self) -> None: data = { "title": "Dashboard",