Skip to content
12 changes: 4 additions & 8 deletions src/sentry/api/bases/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,16 +394,12 @@ def get_projects(
:return: A list of Project objects, or raises PermissionDenied. When project_ids or project_slugs
are explicitly provided, the returned list is guaranteed non-empty (or PermissionDenied is raised).

NOTE: Passing both project_ids and project_slugs raises ``ParseError``.
"""
qs = Project.objects.filter(organization_id=organization.id, status=ObjectStatus.ACTIVE)
if project_slugs and project_ids:
raise ParseError(detail="Cannot query for both ids and slugs")

if project_ids:
requested_projects = ParsedProjectIdOrSlugParams(ids=project_ids, slugs=set())
elif project_slugs:
requested_projects = ParsedProjectIdOrSlugParams(ids=set(), slugs=set(project_slugs))
if project_ids or project_slugs:
requested_projects = ParsedProjectIdOrSlugParams(
ids=project_ids or set(), slugs=set(project_slugs or ())
)
else:
requested_projects = self.get_requested_project_params_unchecked(request)
ids = requested_projects.ids
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,29 @@
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.helpers.projects import ProjectIdOrSlug, ProjectIdOrSlugField
from sentry.api.paginator import OffsetPaginator
from sentry.api.serializers import serialize
from sentry.constants import ALL_ACCESS_PROJECTS_SLUG
from sentry.models.organization import Organization
from sentry.models.release_threshold.release_threshold import ReleaseThreshold


class ReleaseThresholdIndexGETData(TypedDict, total=False):
environment: list[str]
project: list[int]
project: list[ProjectIdOrSlug]


class ReleaseThresholdIndexGETValidator(serializers.Serializer[ReleaseThresholdIndexGETData]):
environment = serializers.ListField(
required=False, allow_empty=True, child=serializers.CharField()
)
project = serializers.ListField(
required=True, allow_empty=False, child=serializers.IntegerField()
)
project = serializers.ListField(required=True, allow_empty=False, child=ProjectIdOrSlugField())
Comment thread
sentry[bot] marked this conversation as resolved.

def validate_project(self, value: list[ProjectIdOrSlug]) -> list[ProjectIdOrSlug]:
if ALL_ACCESS_PROJECTS_SLUG in value:
raise serializers.ValidationError("Invalid project")
return value


@cell_silo_endpoint
Expand All @@ -39,7 +44,7 @@ class ReleaseThresholdIndexEndpoint(OrganizationEndpoint):

def get(self, request: Request, organization: Organization) -> HttpResponse:
validator = ReleaseThresholdIndexGETValidator(
data=request.query_params,
data=self.get_query_params_without_empty_project_params(request),
)
if not validator.is_valid():
return Response(validator.errors, status=400)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
get_errors_counts_timeseries_by_project_and_release,
get_new_issue_counts,
)
from sentry.api.helpers.projects import (
ProjectIdOrSlug,
ProjectIdOrSlugField,
parse_id_or_slug_params,
)
from sentry.api.serializers import serialize
from sentry.apidocs.constants import RESPONSE_BAD_REQUEST
from sentry.apidocs.examples.release_threshold_examples import ReleaseThresholdExamples
Expand Down Expand Up @@ -56,6 +61,7 @@ class ReleaseThresholdStatusIndexData(TypedDict, total=False):
environment: list[str]
projectSlug: list[str]
release: list[str]
project: list[ProjectIdOrSlug]


class ReleaseThresholdStatusIndexSerializer(
Expand All @@ -82,7 +88,7 @@ class ReleaseThresholdStatusIndexSerializer(
projectSlug = serializers.ListField(
required=False,
allow_empty=True,
child=serializers.CharField(),
child=serializers.CharField(allow_blank=True),
help_text=("A list of project slugs to filter your results by."),
)
release = serializers.ListField(
Expand All @@ -91,6 +97,12 @@ class ReleaseThresholdStatusIndexSerializer(
child=serializers.CharField(),
help_text=("A list of release versions to filter your results by."),
)
project = serializers.ListField(
required=False,
allow_empty=True,
child=ProjectIdOrSlugField(),
help_text=("A list of project IDs or slugs to filter your results by."),
)

def validate(self, data: ReleaseThresholdStatusIndexData) -> ReleaseThresholdStatusIndexData:
if data["start"] >= data["end"]:
Expand Down Expand Up @@ -136,27 +148,36 @@ def get(
# NOTE: start/end parameters determine window to query for releases
# This is NOT the window to query snuba for event data - nor the individual threshold windows
# ========================================================================
serializer = ReleaseThresholdStatusIndexSerializer(
data=request.query_params,
)
query_params = self.get_query_params_with_project_slug_precedence(request)

serializer = ReleaseThresholdStatusIndexSerializer(data=query_params)
if not serializer.is_valid():
return Response(as_validation_errors(serializer), status=400)

environments_list = serializer.validated_data.get(
"environment"
) # list of environment names
project_slug_list = serializer.validated_data.get("projectSlug")
releases_list = serializer.validated_data.get("release") # list of release versions
validated_data = serializer.validated_data
environments_list = validated_data.get("environment") # list of environment names
releases_list = validated_data.get("release") # list of release versions

project_ids: set[int] | None = None
project_slugs = {slug for slug in validated_data.get("projectSlug", []) if slug} or None
if project_slugs is None:
requested_project = parse_id_or_slug_params(validated_data.get("project", []))
project_ids = requested_project.ids or None
project_slugs = requested_project.slugs or None

try:
filter_params = self.get_filter_params(
request, organization, date_filter_optional=True, project_slugs=project_slug_list
request,
organization,
date_filter_optional=True,
project_ids=project_ids,
project_slugs=project_slugs,
)
except NoProjects:
raise NoProjects("No projects available")

# Use validated project IDs from get_filter_params instead of raw user input.
# The raw project_slug_list could contain slugs for projects the user doesn't
# have access to, bypassing the permission checks in get_projects().
# Use project IDs from get_filter_params instead of raw project filters so
# project access is checked before fetching threshold data.
validated_project_ids = set(filter_params["project_id"])

start: datetime | None = filter_params["start"]
Expand Down Expand Up @@ -203,7 +224,7 @@ def get(
"Fetched releases",
extra={
"results": len(queryset),
"project_slugs": project_slug_list,
"project_slugs": project_slugs,
"releases": releases_list,
"environments": environments_list,
},
Expand Down
57 changes: 48 additions & 9 deletions src/sentry/releases/endpoints/organization_release_details.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import sentry_sdk
from django.db.models import Q
from drf_spectacular.utils import extend_schema, extend_schema_serializer
from rest_framework.exceptions import ParseError
from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_serializer
from rest_framework.exceptions import ParseError, ValidationError
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.serializers import ListField
Expand All @@ -17,6 +17,10 @@
get_stats_period_detail,
)
from sentry.api.exceptions import ConflictError, InvalidRepository, ResourceDoesNotExist
from sentry.api.helpers.projects import (
PROJECT_ID_OR_SLUG_SCHEMA,
ProjectIdOrSlugField,
)
from sentry.api.serializers import serialize
from sentry.api.serializers.rest_framework import (
ReleaseHeadCommitSerializer,
Expand All @@ -38,6 +42,7 @@
as_validation_errors,
)
from sentry.apidocs.utils import inline_sentry_response_serializer
from sentry.constants import ALL_ACCESS_PROJECT_ID, ALL_ACCESS_PROJECTS_SLUG
from sentry.models.activity import Activity
from sentry.models.organization import Organization
from sentry.models.release import Release, ReleaseStatus
Expand Down Expand Up @@ -314,7 +319,21 @@ class OrganizationReleaseDetailsEndpoint(
parameters=[
GlobalParams.ORG_ID_OR_SLUG,
ReleaseParams.VERSION,
ReleaseParams.PROJECT_ID,
OpenApiParameter(
name="project_id",
location="query",
required=False,
type=str,
deprecated=True,
description="Deprecated. Use project instead.",
),
OpenApiParameter(
name="project",
location="query",
required=False,
type=PROJECT_ID_OR_SLUG_SCHEMA,
description="The project ID or slug to filter by. Overrides project_id when both are sent.",
),
ReleaseParams.HEALTH,
ReleaseParams.ADOPTION_STAGES,
ReleaseParams.SUMMARY_STATS_PERIOD,
Expand All @@ -340,7 +359,7 @@ def get(
"""
# Dictionary responsible for storing selected project meta data
current_project_meta = {}
project_id = request.GET.get("project")
project_id = request.GET.get("project") or request.GET.get("project_id")

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.

Bug: The call to get_filter_params omits the validated project ID, causing get_adjacent_releases_to_current_release to query for releases across all accessible projects instead of the specified one.
Severity: MEDIUM

Suggested Fix

Pass the validated project information to get_filter_params. The call should be updated to self.get_filter_params(request, organization, project_ids=project_ids, project_slugs=project_slugs) where project_ids and project_slugs are derived from the validated project object earlier in the function.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/releases/endpoints/organization_release_details.py#L362

Potential issue: When a `project` or `project_id` is specified in the request, the
endpoint correctly identifies the single project. However, the subsequent call to
`self.get_filter_params(request, organization)` does not pass this project context. As a
result, `filter_params` is populated with all projects the user has access to. This
incorrect set of projects is then used by `get_adjacent_releases_to_current_release`,
causing it to compute adjacent releases across multiple projects instead of being scoped
to the single project from the request parameters. This leads to incorrect next/previous
release navigation on the release details page.

with_health = request.GET.get("health") == "1"
with_adoption_stages = request.GET.get("adoptionStages") == "1"
summary_stats_period = request.GET.get("summaryStatsPeriod") or "14d"
Expand All @@ -362,15 +381,30 @@ def get(
if not self.has_release_permission(request, organization, release):
raise ResourceDoesNotExist

# Validate project access when project_id is provided
# Validate project access when a project identifier is provided.
project = None
project_ids: set[int] | None = None
project_slugs: set[str] | None = None
if project_id:
try:
project_id_int = int(project_id)
except ValueError:
project_id_or_slug = ProjectIdOrSlugField().run_validation(project_id)
except ValidationError:
raise ParseError(detail="Invalid project")

if isinstance(project_id_or_slug, int):
if project_id_or_slug == ALL_ACCESS_PROJECT_ID:
raise ParseError(detail="Invalid project")
project_ids = {project_id_or_slug}
else:
if project_id_or_slug == ALL_ACCESS_PROJECTS_SLUG:
raise ParseError(detail="Invalid project")
project_slugs = {project_id_or_slug}

validated_projects = self.get_projects(
request, organization, project_ids={project_id_int}
request,
organization,
project_ids=project_ids,
project_slugs=project_slugs,
)
if not validated_projects:
raise ResourceDoesNotExist
Expand All @@ -395,7 +429,12 @@ def get(

# Get prev and next release to current release
try:
filter_params = self.get_filter_params(request, organization)
filter_params = self.get_filter_params(
request,
organization,
project_ids=project_ids,
project_slugs=project_slugs,
)
current_project_meta.update(
{
**self.get_adjacent_releases_to_current_release(
Expand Down
13 changes: 13 additions & 0 deletions tests/sentry/api/bases/test_organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,19 @@ def test_project_param_with_mixed_ids_and_slugs(self) -> None:

assert {p.id for p in result} == {self.project_1.id, self.project_2.id}

def test_explicit_project_ids_and_slugs(self) -> None:
self.create_team_membership(user=self.user, team=self.team_3)
request = self.build_request()

result = self.endpoint.get_projects(
request,
self.org,
project_ids={self.project_1.id},
project_slugs={self.project_2.slug},
)

assert {p.id for p in result} == {self.project_1.id, self.project_2.id}

def test_project_param_with_nonexistent_slug(self) -> None:
self.create_team_membership(user=self.user, team=self.team_1)
request = self.build_request(project=["nonexistent-slug"])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,64 @@ def test_get_success_project_slug_filter(self) -> None:
r2_keys = [k for k, v in data.items() if k.split("-")[1] == self.release2.version]
assert len(r2_keys) == 0

def test_get_success_project_param_slug_filter(self) -> None:
now = datetime.now(UTC)
yesterday = now - timedelta(hours=24)

project_slug_response = self.get_success_response(
self.organization.slug, start=yesterday, end=now, projectSlug=[self.project2.slug]
)
project_response = self.get_success_response(
self.organization.slug, start=yesterday, end=now, project=[self.project2.slug]
)

assert project_response.data == project_slug_response.data

def test_get_success_project_param_mixed_id_and_slug_filter(self) -> None:
now = datetime.now(UTC)
yesterday = now - timedelta(hours=24)

response = self.get_success_response(
self.organization.slug,
start=yesterday,
end=now,
project=[str(self.project1.id), self.project2.slug],
)

assert set(response.data.keys()) == {
f"{self.project1.slug}-{self.release1.version}",
f"{self.project1.slug}-{self.release2.version}",
f"{self.project2.slug}-{self.release1.version}",
}

def test_get_success_project_slug_takes_precedence_over_project_id(self) -> None:
now = datetime.now(UTC)
yesterday = now - timedelta(hours=24)

response = self.get_success_response(
self.organization.slug,
start=yesterday,
end=now,
projectSlug=[self.project2.slug],
project=[str(self.project1.id)],
)

assert set(response.data.keys()) == {f"{self.project2.slug}-{self.release1.version}"}

def test_get_success_empty_project_slug_falls_back_to_project_filter(self) -> None:
now = datetime.now(UTC)
yesterday = now - timedelta(hours=24)

response = self.get_success_response(
self.organization.slug,
start=yesterday,
end=now,
projectSlug=[""],
project=[self.project2.slug],
)

assert set(response.data.keys()) == {f"{self.project2.slug}-{self.release1.version}"}

@patch(
"sentry.api.endpoints.release_thresholds.release_threshold_status_index.fetch_sessions_data"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ def setUp(self) -> None:
def test_get_invalid_project(self) -> None:
self.get_error_response(self.organization.slug, project="foo bar")

def test_get_all_projects_slug_is_invalid(self) -> None:
response = self.get_error_response(self.organization.slug, project="$all")

assert response.status_code == 400

def test_get_no_project(self) -> None:
self.get_error_response(self.organization.slug)

Expand Down Expand Up @@ -49,6 +54,21 @@ def test_get_valid_project(self) -> None:
assert created_threshold["environment"]["id"] == str(self.canary_environment.id)
assert created_threshold["environment"]["name"] == self.canary_environment.name

def test_get_valid_project_slug(self) -> None:
ReleaseThreshold.objects.create(
threshold_type=0,
trigger_type=0,
value=100,
window_in_seconds=1800,
project=self.project,
environment=self.canary_environment,
)

response = self.get_success_response(self.organization.slug, project=self.project.slug)

assert len(response.data) == 1
assert response.data[0]["project"]["id"] == str(self.project.id)

def test_get_invalid_environment(self) -> None:
self.get_error_response(self.organization.slug, environment="foo bar", project="-1")

Expand Down
Loading
Loading