feat(detectors): Handle config field updates in base detector validator #103184
Conversation
Previously, the base detector validator update() method did not handle config field updates at all. This adds proper support for updating the config field with JSON schema validation using enforce_config_schema(). Adds tests to verify: - Valid config updates are applied correctly - Invalid config updates that violate the schema are rejected with appropriate error messages
github-actions
bot
added
the
Scope: Backend
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 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
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day 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.
| @@ -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() | ||
|
|
There was a problem hiding this comment.
This is how we were doing the validation in the create as well
evanpurkhiser
changed the title
saponifi3d
left a comment
saponifi3d
left a comment
There was a problem hiding this comment.
🎉 - now configs update. thanks!
evanpurkhiser
deleted the
evanpurkhiser/ref-detectors-handle-config-field-updates-in-base-detector-validator
branch
…or (#103184) Previously, the base detector validator update() method did not handle config field updates at all. This adds proper support for updating the config field with JSON schema validation using enforce_config_schema(). Adds tests to verify: - Valid config updates are applied correctly - Invalid config updates that violate the schema are rejected with appropriate error messages
Previously, the base detector validator update() method did not handle config field updates at all. This adds proper support for updating the config field with JSON schema validation using enforce_config_schema().
Adds tests to verify: