Skip to content
102 changes: 102 additions & 0 deletions decisionModelLinter.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,87 @@ const api = {
},
],
},
// The four rules below are a temporary, "warn only" safety net for API design rules while
// deterministic checks for them are built. We will remove each one once its check is enforced
returnsUnpaginatedList: {
threshold: 0.8,
prompt:
'Does an endpoint\'s GET method return a list of objects that can keep growing, such as an organization\'s projects, members, issues, releases, or monitors, without paginating it with self.paginate(...) or a paginator class? A list inside a response object (for example {"data": [...]}) counts too. Lists with a small fixed limit, lists of fixed choices or enum values, single objects, and lists that are only built for internal use and never returned do not count.',
values: [
{
value: 'TRUE',
description:
'A GET handler returns a growing collection, at the top level of the response or nested in it, and nothing paginates or limits it.',
severity: 'warn',
report:
"List endpoints should paginate with `self.paginate(...)` and a `cursor` parameter. This endpoint returns a list that can keep growing without paginating it. If the endpoint already exists, don't add pagination here: that changes what existing callers get back, so it's a breaking change that needs a deprecation plan.",
},
{
value: 'FALSE',
description:
'Every growing list the endpoint returns is paginated, or it returns a single object, a fixed set of values, or a list with a small fixed limit.',
},
],
},
replacesMembershipWithList: {
threshold: 0.8,
prompt:
'Does a PUT, PATCH, or POST that updates an existing object accept a list of IDs, users, emails, teams, or members (for example a serializers.ListField of user IDs) and use it to replace the whole stored set, by comparing it to the current members and adding or removing the difference, clearing and re-adding, or calling set()? replayAccessMembers in organization_details.py is an example: two people editing the list at the same time overwrite each other. Lists sent once when an object is first created, lists that only add members, and lists of settings or values that are not memberships do not count.',
values: [
{
value: 'TRUE',
description:
'A request field holds a list of members, users, teams, or IDs, and its value replaces the whole stored set on an existing object.',
severity: 'warn',
report:
"A list field that replaces a whole set of members lets two concurrent edits overwrite each other. For new fields, prefer separate endpoints to add and remove one member. Don't change or remove an existing field to fix this, because that breaks its callers.",
},
{
value: 'FALSE',
description:
'No request list replaces a stored set of members, or the list is only used when creating a new object, only adds members, or is not a membership list.',
},
],
},
returnsNumericIdAsInteger: {
prompt:
"Does API response code, such as a serializer's serialize() method, a response TypedDict, or the body passed to Response(...), return a numeric ID (id, or a field ending in Id or _id, such as projectId or organization_id) as an integer instead of a string? IDs converted with str(), IDs that are already strings (UUIDs, slugs, hex event IDs), counts and other numbers that are not IDs, and code that does not build an API response do not count.",
values: [
{
value: 'TRUE',
description:
'A numeric ID is placed in an API response as an integer, for example "id": obj.id, or a response TypedDict declares an ID field as int.',
severity: 'warn',
report:
"API responses should return numeric IDs as strings, for example `str(obj.id)`. This file returns one as an integer. If the field already exists, don't change its type here, because callers expect a number. Only new fields should follow this rule.",
},
{
value: 'FALSE',
description:
'Every ID in the response is a string, or the code builds no API response.',
},
],
},
makesExistingInputStricter: {
threshold: 0.8,
prompt:
'Does the change make an endpoint or serializer that already existed reject input it used to accept? Code already existed when the lines around the change are unchanged or omitted; removed lines are not shown. That includes a (new) field line in an existing serializer that is required (required=True, or no required=False), which covers both a field becoming required and a new required field, new validation on an existing field (a validate_<field> method, validators=, min_value, max_value, max_length, a narrower choices list, or a new ValidationError raised from validate), or an allowed value or format being removed. A new endpoint or serializer where every line is new, and new optional fields, do not count.',
values: [
{
value: 'TRUE',
description:
'Input that an existing endpoint or serializer accepted before is now rejected: a field became required, a required field was added, or new validation was added to existing input.',
severity: 'warn',
report:
"Rejecting input an endpoint accepted before breaks existing callers, and the API spec often doesn't show it. Make the change backward compatible (keep the field optional, or accept the old values), or treat it as a breaking change and coordinate it.",
},
{
value: 'FALSE',
description:
'The change adds a new endpoint or serializer, adds only optional fields, or relaxes or keeps existing validation.',
},
],
},
},
} satisfies Suite;

Expand Down Expand Up @@ -333,6 +414,27 @@ const security = {
},
],
},
// A temporary, warn only safety net while a deterministic check for unscoped lookups is built.
looksUpObjectWithoutOrgScope: {
threshold: 0.8,
prompt:
"Does endpoint code fetch an object using an ID from the request (request.data, request.GET, request.query_params, or a serializer's validated_data), with Model.objects.get(...), filter(...), or get_object_or_404, without also filtering by the caller's organization or project, as in Project.objects.get(id=request.data[\"project_id\"])? Lookups that filter by organization or project (organization_id=organization.id, project__organization=organization, project_id__in=...), IDs checked through self.get_projects() or a similar access-checked helper, objects checked afterwards with has_project_access or check_object_permissions, objects the endpoint's base class already resolved from the URL (organization, project, team, group), and lookups of the caller's own user do not count.",
values: [
{
value: 'TRUE',
description:
"An object is fetched by a request-supplied ID with no filter on the caller's organization or project and no access check on the result.",
severity: 'warn',
report:
"Objects fetched by an ID from the request should also be filtered by the caller's organization or project, for example `Model.objects.get(id=..., organization_id=organization.id)`, or checked with `self.get_projects()`. Without that, one organization can read or change another's data.",
},
{
value: 'FALSE',
description:
"Every request-supplied ID is looked up within the caller's organization or project, goes through an access-checked helper, or the object is access-checked after it is fetched, or no object is fetched by a request-supplied ID.",
},
],
},
},
} satisfies Suite;

Expand Down
2 changes: 1 addition & 1 deletion src/sentry/api/endpoints/assistant.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@


class AssistantSerializer(serializers.Serializer):
guide = serializers.CharField(required=False)
guide = serializers.CharField(required=True)

Check warning on line 27 in src/sentry/api/endpoints/assistant.py

View check run for this annotation

@sentry/warden / warden: sentry-backend-bugs

Making guide required rejects valid guide_id-only requests

Setting guide required=True breaks the documented guide OR guide_id contract; clients sending only guide_id now get 400 before validate() can accept them. Keep guide required=False or require one of the two fields in validate().

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

required=True on guide rejects documented guide_id-only requests

Making guide required breaks the documented guide_id-only PUT path; keep required=False and enforce the either-or rule in validate().

Evidence
  • Hunk sets guide = serializers.CharField(required=True), so DRF rejects payloads missing guide before validate() runs.
  • AssistantEndpoint.put documents request shape as 'guide_id': <guide_id> - OR - 'guide': guide key.
  • AssistantSerializer.validate() still early-returns when only guide_id is present and errors only when neither is provided.
  • Concrete failing input: {"guide_id": <valid_id>, "status": "viewed"} previously accepted, now returns 400 guide: This field is required.

Identified by Warden · sentry-backend-bugs · MN9-U8U

guide_id = serializers.IntegerField(required=False)
status = serializers.ChoiceField(choices=list(zip(VALID_STATUSES, VALID_STATUSES)))
useful = serializers.BooleanField(required=False)
Expand Down
22 changes: 22 additions & 0 deletions src/sentry/api/endpoints/decision_lint_probe.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Temporary file for testing decision-model-lint rules. Not for merging.
from rest_framework.request import Request
from rest_framework.response import Response

from sentry.api.api_owners import ApiOwner
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import cell_silo_endpoint
from sentry.api.bases.organization import OrganizationEndpoint
from sentry.api.serializers import serialize
from sentry.models.organization import Organization
from sentry.models.project import Project


@cell_silo_endpoint
class OrganizationDecisionLintProbeEndpoint(OrganizationEndpoint):
owner = ApiOwner.UNOWNED
publish_status = {"GET": ApiPublishStatus.PRIVATE}

def get(self, request: Request, organization: Organization) -> Response:
# E. New endpoint returning a growing list without pagination -> INTRODUCED (comment)
projects = Project.objects.filter(organization_id=organization.id)
return Response(serialize(list(projects), request.user))
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def to_internal_value(self, data: Any) -> TrendType | None:
class FunctionTrendsSerializer(serializers.Serializer):
function = serializers.CharField(max_length=10)
trend = TrendTypeField()
query = serializers.CharField(required=False)
query = serializers.CharField(required=False, help_text="A search query.")
threshold = serializers.IntegerField(min_value=0, max_value=1000, default=16, required=False)


Expand Down
1 change: 1 addition & 0 deletions src/sentry/api/serializers/models/exporteddata.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ def serialize(self, obj, attrs, user, **kwargs) -> ExportedDataResponse:
return {
"id": obj.id,
"user": attrs.get("user"),
"organizationId": obj.organization_id,
"dateCreated": obj.date_added,
"dateFinished": obj.date_finished,
"dateExpired": obj.date_expired,
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/core/endpoints/organization_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def get(
visibility = request.GET.get("visibility", "visible")
if visibility not in environment_visibility_filter_options:
raise ParseError(
f"Invalid value for 'visibility', valid values are: {sorted(environment_visibility_filter_options.keys())!r}"
f"Invalid 'visibility'. Valid values are: {sorted(environment_visibility_filter_options.keys())!r}"
)
environment_projects = EnvironmentProject.objects.filter(
project__in=self.get_projects(request, organization)
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/flags/endpoints/logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class FlagLogDetailsResponse(TypedDict):
class FlagAuditLogModelSerializer(Serializer[FlagAuditLogModelSerializerResponse]):
def serialize(self, obj, attrs, user, **kwargs) -> FlagAuditLogModelSerializerResponse:
return {
"id": obj.id,
"id": obj.id, # the audit log row
"action": ActionEnum.to_string(obj.action),
"createdAt": obj.created_at.isoformat(),
"createdBy": obj.created_by,
Expand Down
Loading