Skip to content

Commit 45b07af

Browse files
gggritsoclaude
gggritso
and
claude
authored
fix(dashboards): Add backend validation for widget layout dimensions (#109826)
Replace the custom `LayoutField` with a `LayoutSerializer` using DRF's built-in `IntegerField` validators to reject invalid widget dimensions at the API level. Widgets created via external tools (e.g. LLMs) can have invalid dimensions like `w: 12` where the grid maximum is `6`. This change prevents such layouts from being persisted. The `LayoutSerializer` also lets drf-spectacular auto-generate accurate OpenAPI documentation with field types and constraints, removing the need for manual `@extend_schema_field` annotations. Frontend clamping counterpart: #109825 Refs DAIN-1266 Co-authored-by: Claude <noreply@anthropic.com>
1 parent f55826d commit 45b07af

File tree

2 files changed

+117
-30
lines changed

2 files changed

+117
-30
lines changed

‎src/sentry/api/serializers/rest_framework/dashboard.py‎

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77

88
import sentry_sdk
99
from django.db.models import Max
10-
from drf_spectacular.types import OpenApiTypes
11-
from drf_spectacular.utils import extend_schema_field, extend_schema_serializer
10+
from drf_spectacular.utils import extend_schema_serializer
1211
from rest_framework import serializers
1312

1413
from sentry import features, options
@@ -104,38 +103,30 @@ def is_table_display_type(display_type):
104103
)
105104

106105

107-
@extend_schema_field(field=OpenApiTypes.OBJECT)
108-
class LayoutField(serializers.Field):
109-
REQUIRED_KEYS = {
110-
"x",
111-
"y",
112-
"w",
113-
"h",
114-
"min_h",
115-
}
106+
MAX_WIDGET_COLS = 6
116107

117-
def to_internal_value(self, data):
118-
if data is None:
119-
return None
120108

121-
missing_keys = self.REQUIRED_KEYS - set(data.keys())
122-
if missing_keys:
123-
missing_key_str = ", ".join(sorted(snake_to_camel_case(key) for key in missing_keys))
124-
raise serializers.ValidationError(f"Missing required keys: {missing_key_str}")
109+
class WidgetLayoutSerializer(CamelSnakeSerializer[Dashboard]):
110+
"""Widget grid layout position and dimensions.
125111
126-
layout_to_store = {}
127-
for key in self.REQUIRED_KEYS:
128-
value = data.get(key)
129-
if value is None:
130-
continue
112+
The dashboard uses a 6-column grid. Required keys: x, y, w, h, minH.
113+
Constraints: x (0-5), y (>= 0), w (1-6), h (>= 1), minH (>= 1), and x + w <= 6.
114+
"""
131115

132-
if not isinstance(value, int):
133-
raise serializers.ValidationError(f"Expected number for: {key}")
134-
layout_to_store[key] = value
116+
x = serializers.IntegerField(
117+
min_value=0, max_value=MAX_WIDGET_COLS - 1, help_text="Column position (0-indexed)."
118+
)
119+
y = serializers.IntegerField(min_value=0, help_text="Row position (0-indexed).")
120+
w = serializers.IntegerField(
121+
min_value=1, max_value=MAX_WIDGET_COLS, help_text="Width in grid columns (1-6)."
122+
)
123+
h = serializers.IntegerField(min_value=1, help_text="Height in grid rows.")
124+
min_h = serializers.IntegerField(min_value=1, help_text="Minimum height in grid rows.")
135125

136-
# Store the layout with camel case dict keys because they'll be
137-
# served as camel case in outgoing responses anyways
138-
return convert_dict_key_case(layout_to_store, snake_to_camel_case)
126+
def validate(self, data):
127+
if data["x"] + data["w"] > MAX_WIDGET_COLS:
128+
raise serializers.ValidationError(f"x + w must not exceed {MAX_WIDGET_COLS}")
129+
return convert_dict_key_case(data, snake_to_camel_case)
139130

140131

141132
class DashboardWidgetQueryOnDemandSerializer(CamelSnakeSerializer[Dashboard]):
@@ -316,7 +307,7 @@ class DashboardWidgetSerializer(CamelSnakeSerializer[Dashboard]):
316307
choices=DashboardWidgetTypes.as_text_choices(), required=False
317308
)
318309
limit = serializers.IntegerField(min_value=1, required=False, allow_null=True)
319-
layout = LayoutField(required=False, allow_null=True)
310+
layout = WidgetLayoutSerializer(required=False, allow_null=True)
320311
axis_range = serializers.ChoiceField(
321312
choices=[("auto", "auto"), ("dataMin", "dataMin")],
322313
required=False,

‎tests/sentry/dashboards/endpoints/test_organization_dashboards.py‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,102 @@ def test_post_errors_if_layout_submitted_without_required_keys(self) -> None:
11451145
response = self.do_request("post", self.url, data=data)
11461146
assert response.status_code == 400, response.data
11471147

1148+
def test_post_widget_layout_rejects_width_exceeding_grid_columns(self) -> None:
1149+
data = {
1150+
"title": "Dashboard from Post",
1151+
"widgets": [
1152+
{
1153+
"displayType": "line",
1154+
"interval": "5m",
1155+
"title": "Transaction count()",
1156+
"queries": [
1157+
{
1158+
"name": "Transactions",
1159+
"fields": ["count()"],
1160+
"columns": [],
1161+
"aggregates": ["count()"],
1162+
"conditions": "event.type:transaction",
1163+
}
1164+
],
1165+
"layout": {"x": 0, "y": 0, "w": 12, "h": 2, "minH": 2},
1166+
},
1167+
],
1168+
}
1169+
response = self.do_request("post", self.url, data=data)
1170+
assert response.status_code == 400, response.data
1171+
1172+
def test_post_widget_layout_rejects_x_plus_w_exceeding_grid_columns(self) -> None:
1173+
data = {
1174+
"title": "Dashboard from Post",
1175+
"widgets": [
1176+
{
1177+
"displayType": "line",
1178+
"interval": "5m",
1179+
"title": "Transaction count()",
1180+
"queries": [
1181+
{
1182+
"name": "Transactions",
1183+
"fields": ["count()"],
1184+
"columns": [],
1185+
"aggregates": ["count()"],
1186+
"conditions": "event.type:transaction",
1187+
}
1188+
],
1189+
"layout": {"x": 4, "y": 0, "w": 4, "h": 2, "minH": 2},
1190+
},
1191+
],
1192+
}
1193+
response = self.do_request("post", self.url, data=data)
1194+
assert response.status_code == 400, response.data
1195+
1196+
def test_post_widget_layout_rejects_negative_x(self) -> None:
1197+
data = {
1198+
"title": "Dashboard from Post",
1199+
"widgets": [
1200+
{
1201+
"displayType": "line",
1202+
"interval": "5m",
1203+
"title": "Transaction count()",
1204+
"queries": [
1205+
{
1206+
"name": "Transactions",
1207+
"fields": ["count()"],
1208+
"columns": [],
1209+
"aggregates": ["count()"],
1210+
"conditions": "event.type:transaction",
1211+
}
1212+
],
1213+
"layout": {"x": -1, "y": 0, "w": 2, "h": 2, "minH": 2},
1214+
},
1215+
],
1216+
}
1217+
response = self.do_request("post", self.url, data=data)
1218+
assert response.status_code == 400, response.data
1219+
1220+
def test_post_widget_layout_rejects_zero_height(self) -> None:
1221+
data = {
1222+
"title": "Dashboard from Post",
1223+
"widgets": [
1224+
{
1225+
"displayType": "line",
1226+
"interval": "5m",
1227+
"title": "Transaction count()",
1228+
"queries": [
1229+
{
1230+
"name": "Transactions",
1231+
"fields": ["count()"],
1232+
"columns": [],
1233+
"aggregates": ["count()"],
1234+
"conditions": "event.type:transaction",
1235+
}
1236+
],
1237+
"layout": {"x": 0, "y": 0, "w": 2, "h": 0, "minH": 2},
1238+
},
1239+
],
1240+
}
1241+
response = self.do_request("post", self.url, data=data)
1242+
assert response.status_code == 400, response.data
1243+
11481244
def test_post_dashboard_with_filters(self) -> None:
11491245
project1 = self.create_project(name="foo", organization=self.organization)
11501246
project2 = self.create_project(name="bar", organization=self.organization)

0 commit comments

Comments
 (0)