Skip to content

fix(dashboards): validate threshold JSON shape #129434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joshuarli wants to merge 2 commits into
base: master
Choose a base branch
from
+87 −25
Draft

fix(dashboards): validate threshold JSON shape #129434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a04d52
fix(dashboards): validate threshold JSON shape
joshuarli Oct 31, 2026
bd45f8c
test(dashboards): annotate invalid threshold cases
joshuarli Oct 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 44 additions & 25 deletions src/sentry/api/serializers/rest_framework/dashboard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,46 +897,65 @@ def validate(self, data):
{"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) or isinstance(max_value, bool):
raise serializers.ValidationError(
{"thresholds": {max_key: "Maximum values must be integers."}}
)
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(
Expand Down
43 changes: 43 additions & 0 deletions tests/sentry/dashboards/endpoints/test_organization_dashboard_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2118,6 +2118,49 @@ 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 integers.",
),
(
{"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_invalid_threshold_time_window(self) -> None:
data = {
"title": "Dashboard",
Expand Down
Loading