Skip to content

feat(detectors): Handle config field updates in base detector validator #103184

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

Merged
merged 1 commit into from
Nov 12, 2025
+61 −0
Merged

feat(detectors): Handle config field updates in base detector validator #103184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions src/sentry/workflow_engine/endpoints/validators/base/detector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,14 @@
group_validator = BaseDataConditionGroupValidator()
group_validator.update(instance.workflow_condition_group, condition_group)

# Handle config field update
if "config" in validated_data:
instance.config = validated_data.get("config", instance.config)
try:
enforce_config_schema(instance)
except JSONSchemaValidationError as error:
raise serializers.ValidationError({"config": [str(error)]})

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
Loading
flows to this location and may be exposed to an external user.
Stack trace information
Loading
flows to this location and may be exposed to an external user.
Stack trace information
Loading
flows to this location and may be exposed to an external user.
Stack trace information
Loading
flows to this location and may be exposed to an external user.
Stack trace information
Loading
flows to this location and may be exposed to an external user.

Copilot Autofix

AI 2 days ago

To fix the problem, we need to suppress the detailed exception information from being exposed to external users. Instead of passing str(error) (which may contain stack trace, schema internals, or other sensitive info) to the user via serializers.ValidationError, we should replace it with a generic error message like "Invalid configuration." At the same time, we should log the full error internally (using Python's logging facilities) so developers can still trace and debug misconfigurations. This requires adding a logger to the file (via Python's logging module) and modifying the exception handler to log error or its stack trace before raising the generic error. The required changes are limited to the update method in src/sentry/workflow_engine/endpoints/validators/base/detector.py, specifically the lines inside the config validation exception handler.

Suggested changeset 1
src/sentry/workflow_engine/endpoints/validators/base/detector.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/sentry/workflow_engine/endpoints/validators/base/detector.py b/src/sentry/workflow_engine/endpoints/validators/base/detector.py
--- a/src/sentry/workflow_engine/endpoints/validators/base/detector.py
+++ b/src/sentry/workflow_engine/endpoints/validators/base/detector.py
@@ -1,7 +1,7 @@
 import builtins
 from dataclasses import dataclass
 from typing import Any
-
+import logging
 from django.db import router, transaction
 from jsonschema import ValidationError as JSONSchemaValidationError
 from rest_framework import serializers
@@ -33,6 +33,8 @@
 from sentry.workflow_engine.types import DataConditionType
 
 
+logger = logging.getLogger(__name__)
+
 @dataclass(frozen=True)
 class DetectorQuota:
     has_exceeded: bool
@@ -139,7 +141,8 @@
                 try:
                     enforce_config_schema(instance)
                 except JSONSchemaValidationError as error:
-                    raise serializers.ValidationError({"config": [str(error)]})
+                    logger.exception("Config schema validation failed for Detector %s", instance.id)
+                    raise serializers.ValidationError({"config": ["Invalid configuration."]})
 
             instance.save()
 
EOF
@@ -1,7 +1,7 @@
import builtins
from dataclasses import dataclass
from typing import Any

import logging
from django.db import router, transaction
from jsonschema import ValidationError as JSONSchemaValidationError
from rest_framework import serializers
@@ -33,6 +33,8 @@
from sentry.workflow_engine.types import DataConditionType


logger = logging.getLogger(__name__)

@dataclass(frozen=True)
class DetectorQuota:
has_exceeded: bool
@@ -139,7 +141,8 @@
try:
enforce_config_schema(instance)
except JSONSchemaValidationError as error:
raise serializers.ValidationError({"config": [str(error)]})
logger.exception("Config schema validation failed for Detector %s", instance.id)
raise serializers.ValidationError({"config": ["Invalid configuration."]})

instance.save()

Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Member Author

@evanpurkhiser evanpurkhiser Nov 11, 2025

Choose a reason for hiding this comment

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

This is how we were doing the validation in the create as well


instance.save()

create_audit_entry(
Expand Down
53 changes: 53 additions & 0 deletions tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,59 @@ def test_update_workflows_no_changes(self) -> None:
== 0
)

def test_update_config_valid(self) -> None:
"""Test updating detector config with valid schema data"""
# Initial config
initial_config = {"detection_type": "static", "comparison_delta": None}
self.detector.config = initial_config
self.detector.save()

# Update with valid new config
updated_config = {"detection_type": "dynamic", "comparison_delta": 3600}
data = {
"config": updated_config,
}

with self.tasks():
response = self.get_success_response(
self.organization.slug,
self.detector.id,
**data,
status_code=200,
)

self.detector.refresh_from_db()
# Verify config was updated in database (snake_case)
assert self.detector.config == updated_config
# API returns camelCase
assert response.data["config"] == {
"detectionType": "dynamic",
"comparisonDelta": 3600,
}

def test_update_config_invalid_schema(self) -> None:
"""Test updating detector config with invalid schema data fails validation"""
# Config missing required field 'detection_type'
invalid_config = {"comparison_delta": 3600}
data = {
"config": invalid_config,
}

with self.tasks():
response = self.get_error_response(
self.organization.slug,
self.detector.id,
**data,
status_code=400,
)

assert "config" in response.data
assert "detection_type" in str(response.data["config"])

# Verify config was not updated
self.detector.refresh_from_db()
assert self.detector.config != invalid_config


@region_silo_test
class OrganizationDetectorDetailsDeleteTest(OrganizationDetectorDetailsBaseTest):
Expand Down
Loading