Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
43 changes: 40 additions & 3 deletions src/sentry/overwatch/endpoints/overwatch_rpc.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import hashlib
import hmac
import logging
from copy import deepcopy
from typing import Any

import sentry_sdk
Expand All @@ -19,8 +20,11 @@
HIDE_AI_FEATURES_DEFAULT,
ObjectStatus,
)
from sentry.integrations.services.integration import integration_service
from sentry.models.organization import Organization
from sentry.models.repository import Repository
from sentry.prevent.models import PreventAIConfiguration
from sentry.prevent.types.config import PREVENT_AI_CONFIG_DEFAULT
from sentry.silo.base import SiloMode

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -98,7 +102,7 @@ class PreventPrReviewResolvedConfigsEndpoint(Endpoint):
"""
Returns the resolved config for a Sentry organization.

GET /prevent/pr-review/configs/resolved?sentryOrgId={orgId}
GET /prevent/pr-review/configs/resolved?sentryOrgId={orgId}&gitOrgName={gitOrgName}&provider={provider}
"""

publish_status = {
Expand All @@ -115,9 +119,42 @@ def get(self, request: Request) -> Response:
):
raise PermissionDenied

# TODO: Fetch config from PreventAIConfiguration model
sentry_org_id_str = request.GET.get("sentryOrgId")
if not sentry_org_id_str:
raise ParseError("Missing required query parameter: sentryOrgId")
try:
sentry_org_id = int(sentry_org_id_str)
if sentry_org_id <= 0:
raise ParseError("sentryOrgId must be a positive integer")
except ValueError:
raise ParseError("sentryOrgId must be a valid integer")

git_org_name = request.GET.get("gitOrgName")
if not git_org_name:
raise ParseError("Missing required query parameter: gitOrgName")
provider = request.GET.get("provider")
if not provider:
raise ParseError("Missing required query parameter: provider")

github_org_integrations = integration_service.get_organization_integrations(
organization_id=sentry_org_id,
providers=[provider],
status=ObjectStatus.ACTIVE,
name=git_org_name,
)
if not github_org_integrations:
return Response({"detail": "GitHub integration not found"}, status=404)

config = PreventAIConfiguration.objects.filter(
organization_id=sentry_org_id,
integration_id=github_org_integrations[0].integration_id,
).first()
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: String ID Mismatch in Database Queries

The sentry_org_id query parameter is retrieved as a string but used directly in database queries and service calls expecting an integer. This prevents matching records from being found, causing the endpoint to incorrectly report missing integrations or configurations.

Fix in Cursor Fix in Web


response_data: dict[str, Any] = deepcopy(PREVENT_AI_CONFIG_DEFAULT)
if config:
response_data["organization"][git_org_name] = config.data

return Response(data={})
return Response(data=response_data)


@region_silo_endpoint
Expand Down
10 changes: 5 additions & 5 deletions src/sentry/prevent/endpoints/pr_review_github_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from sentry.integrations.types import IntegrationProviderSlug
from sentry.models.organization import Organization
from sentry.prevent.models import PreventAIConfiguration
from sentry.prevent.types.config import ORG_CONFIG_SCHEMA, PREVENT_AI_CONFIG_GITHUB_DEFAULT
from sentry.prevent.types.config import ORG_CONFIG_SCHEMA, PREVENT_AI_CONFIG_DEFAULT


class PreventAIConfigPermission(OrganizationPermission):
Expand Down Expand Up @@ -59,9 +59,9 @@ def get(
integration_id=github_org_integrations[0].integration_id,
).first()

response_data: dict[str, Any] = deepcopy(PREVENT_AI_CONFIG_GITHUB_DEFAULT)
response_data: dict[str, Any] = deepcopy(PREVENT_AI_CONFIG_DEFAULT)
if config:
response_data["github_organization"][git_organization_name] = config.data
response_data["organization"][git_organization_name] = config.data

return Response(response_data, status=200)

Expand Down Expand Up @@ -102,7 +102,7 @@ def put(
},
)

response_data: dict[str, Any] = deepcopy(PREVENT_AI_CONFIG_GITHUB_DEFAULT)
response_data["github_organization"][git_organization_name] = request.data
response_data: dict[str, Any] = deepcopy(PREVENT_AI_CONFIG_DEFAULT)
response_data["organization"][git_organization_name] = request.data

return Response(response_data, status=200)
4 changes: 2 additions & 2 deletions src/sentry/prevent/types/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
"additionalProperties": False,
}

PREVENT_AI_CONFIG_GITHUB_DEFAULT = {
PREVENT_AI_CONFIG_DEFAULT = {
"schema_version": "v1",
"default_org_config": {
"org_defaults": {
Expand Down Expand Up @@ -119,5 +119,5 @@
},
"repo_overrides": {},
},
"github_organization": {},
"organization": {},
}
240 changes: 237 additions & 3 deletions tests/sentry/overwatch/endpoints/test_overwatch_rpc.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,37 @@
import hashlib
import hmac
from copy import deepcopy
from unittest.mock import patch

from django.urls import reverse

from sentry.constants import ObjectStatus
from sentry.prevent.models import PreventAIConfiguration
from sentry.prevent.types.config import PREVENT_AI_CONFIG_DEFAULT
from sentry.silo.base import SiloMode
from sentry.testutils.cases import APITestCase
from sentry.testutils.silo import assume_test_silo_mode

VALID_ORG_CONFIG = {
"schema_version": "v1",
"org_defaults": {
"bug_prediction": {
"enabled": True,
"sensitivity": "medium",
"triggers": {"on_command_phrase": True, "on_ready_for_review": True},
},
"test_generation": {
"enabled": False,
"triggers": {"on_command_phrase": True, "on_ready_for_review": False},
},
"vanilla": {
"enabled": False,
"sensitivity": "medium",
"triggers": {"on_command_phrase": True, "on_ready_for_review": False},
},
},
"repo_overrides": {},
}


class TestPreventPrReviewResolvedConfigsEndpoint(APITestCase):
Expand All @@ -29,14 +55,222 @@ def test_requires_auth(self):
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_success_returns_default_config(self):
def test_missing_sentry_org_id_returns_400(self):
"""Test that missing sentryOrgId parameter returns 400."""
url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
auth = self._auth_header_for_get(url, {}, "test-secret")
resp = self.client.get(url, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 400
assert "sentryOrgId" in resp.data["detail"]

@patch(
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_invalid_sentry_org_id_returns_400(self):
"""Test that invalid sentryOrgId (non-integer) returns 400."""
url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
params = {"sentryOrgId": "not-a-number", "gitOrgName": "test-org", "provider": "github"}
auth = self._auth_header_for_get(url, params, "test-secret")
resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 400
assert "must be a valid integer" in resp.data["detail"]

@patch(
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_negative_sentry_org_id_returns_400(self):
"""Test that negative sentryOrgId returns 400."""
url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
params = {"sentryOrgId": "-123", "gitOrgName": "test-org", "provider": "github"}
auth = self._auth_header_for_get(url, params, "test-secret")
resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 400
assert "must be a positive integer" in resp.data["detail"]

@patch(
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_zero_sentry_org_id_returns_400(self):
"""Test that zero sentryOrgId returns 400."""
url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
params = {"sentryOrgId": "0", "gitOrgName": "test-org", "provider": "github"}
auth = self._auth_header_for_get(url, params, "test-secret")
resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 400
assert "must be a positive integer" in resp.data["detail"]

@patch(
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_missing_git_org_name_returns_400(self):
"""Test that missing gitOrgName parameter returns 400."""
url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
params = {"sentryOrgId": "123"}
auth = self._auth_header_for_get(url, params, "test-secret")
resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 400
assert "gitOrgName" in resp.data["detail"]

@patch(
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_missing_provider_returns_400(self):
"""Test that missing provider parameter returns 400."""
url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
params = {"sentryOrgId": "123", "gitOrgName": "test-org"}
auth = self._auth_header_for_get(url, params, "test-secret")
resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 400
assert "provider" in resp.data["detail"]

@patch(
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_returns_default_when_no_config(self):
"""Test that default config is returned when no configuration exists."""
org = self.create_organization()
git_org_name = "test-github-org"

with assume_test_silo_mode(SiloMode.CONTROL):
self.create_integration(
organization=org,
provider="github",
name=git_org_name,
external_id=f"github:{git_org_name}",
status=ObjectStatus.ACTIVE,
)

url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
params = {
"sentryOrgId": str(org.id),
"gitOrgName": git_org_name,
"provider": "github",
}
auth = self._auth_header_for_get(url, params, "test-secret")
resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 200
assert resp.data == PREVENT_AI_CONFIG_DEFAULT
assert resp.data["organization"] == {}

@patch(
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_returns_config_when_exists(self):
"""Test that saved configuration is returned when it exists."""
org = self.create_organization()
git_org_name = "test-github-org"

with assume_test_silo_mode(SiloMode.CONTROL):
integration = self.create_integration(
organization=org,
provider="github",
name=git_org_name,
external_id=f"github:{git_org_name}",
status=ObjectStatus.ACTIVE,
)

PreventAIConfiguration.objects.create(
organization_id=org.id,
integration_id=integration.id,
data=VALID_ORG_CONFIG,
)

url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
params = {"sentryOrgId": str(org.id)}
params = {
"sentryOrgId": str(org.id),
"gitOrgName": git_org_name,
"provider": "github",
}
auth = self._auth_header_for_get(url, params, "test-secret")
resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 200
assert resp.data == {}
assert resp.data["organization"][git_org_name] == VALID_ORG_CONFIG

@patch(
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_returns_404_when_integration_not_found(self):
"""Test that 404 is returned when GitHub integration doesn't exist."""
org = self.create_organization()

url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
params = {
"sentryOrgId": str(org.id),
"gitOrgName": "nonexistent-org",
"provider": "github",
}
auth = self._auth_header_for_get(url, params, "test-secret")
resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 404
assert resp.data["detail"] == "GitHub integration not found"

@patch(
"sentry.overwatch.endpoints.overwatch_rpc.settings.OVERWATCH_RPC_SHARED_SECRET",
["test-secret"],
)
def test_config_with_repo_overrides(self):
"""Test that configuration with repo overrides is properly retrieved."""
org = self.create_organization()
git_org_name = "test-github-org"

with assume_test_silo_mode(SiloMode.CONTROL):
integration = self.create_integration(
organization=org,
provider="github",
name=git_org_name,
external_id=f"github:{git_org_name}",
status=ObjectStatus.ACTIVE,
)

config_with_overrides = deepcopy(VALID_ORG_CONFIG)
config_with_overrides["repo_overrides"] = {
"my-repo": {
"bug_prediction": {
"enabled": True,
"sensitivity": "high",
"triggers": {"on_command_phrase": True, "on_ready_for_review": False},
},
"test_generation": {
"enabled": True,
"triggers": {"on_command_phrase": True, "on_ready_for_review": True},
},
"vanilla": {
"enabled": False,
"sensitivity": "low",
"triggers": {"on_command_phrase": False, "on_ready_for_review": False},
},
}
}

PreventAIConfiguration.objects.create(
organization_id=org.id,
integration_id=integration.id,
data=config_with_overrides,
)

url = reverse("sentry-api-0-prevent-pr-review-configs-resolved")
params = {
"sentryOrgId": str(org.id),
"gitOrgName": git_org_name,
"provider": "github",
}
auth = self._auth_header_for_get(url, params, "test-secret")
resp = self.client.get(url, params, HTTP_AUTHORIZATION=auth)
assert resp.status_code == 200
assert (
resp.data["organization"][git_org_name]["repo_overrides"]["my-repo"]["bug_prediction"][
"sensitivity"
]
== "high"
)


class TestPreventPrReviewSentryOrgEndpoint(APITestCase):
Expand Down
Loading
Loading