feat(aci): error detector #82533
feat(aci): error detector #82533
Conversation
github-actions
bot
added
the
Scope: Backend
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| try: | ||
| FingerprintingRules.from_config_string(value) | ||
| except InvalidFingerprintingConfig as e: | ||
| raise serializers.ValidationError(str(e)) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 4 days ago
To fix the problem, we should avoid exposing the detailed exception message to the end user. Instead, we can log the detailed exception message on the server for debugging purposes and raise a generic ValidationError message to the user. This approach ensures that sensitive information is not exposed while still allowing developers to diagnose issues.
- Modify the
validate_fingerprinting_rulesmethod to log the exception message and raise a generic error message. - Add necessary imports for logging if not already present.
| @@ -9,2 +9,5 @@ | ||
|
|
||
| import logging | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| @@ -25,3 +28,4 @@ | ||
| except InvalidFingerprintingConfig as e: | ||
| raise serializers.ValidationError(str(e)) | ||
| logger.error(f"Invalid fingerprinting config: {e}") | ||
| raise serializers.ValidationError("Invalid fingerprinting configuration provided.") | ||
|
|
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #82533 +/- ##
==========================================
- Coverage 80.43% 80.43% -0.01%
==========================================
Files 7319 7320 +1
Lines 322657 322709 +52
Branches 21048 21048
==========================================
+ Hits 259529 259567 +38
- Misses 62721 62735 +14
Partials 407 407 |
| @@ -126,11 +128,30 @@ def get_attrs( | |||
| for group, serialized in zip(condition_groups, serialize(condition_groups, user=user)) | |||
| } | |||
|
|
|||
| filtered_item_list = [item for item in item_list if item.type == ErrorGroupType.slug] | |||
| project_ids = [item.project_id for item in filtered_item_list] | |||
There was a problem hiding this comment.
the ids are guaranteed to be unique b/c 1 error detector per project right?
There was a problem hiding this comment.
yes, but that is a good point.
| if item.id in configs: | ||
| attrs[item]["config"] = configs[item.id] | ||
| else: | ||
| attrs[item]["config"] = item.config |
There was a problem hiding this comment.
for all other detectors, will this just be empty?
There was a problem hiding this comment.
it's possible, but it will have whatever shape defined by detector_config_schema for the group type
sentry/src/sentry/issues/grouptype.py
Line 274 in 228d727
There was a problem hiding this comment.
^ as an example. but for the error detector i'm overwriting it since i'm not migrating the project options rn
| **kwargs, | ||
| ) -> Detector: | ||
| if name is None: | ||
| name = petname.generate(2, " ", letters=10).title() | ||
| if config is None: | ||
| config = {} |
There was a problem hiding this comment.
need to explicitly set this as a dictionary, the receiver does not think the Django db_default={} is the same as {}
Basically the same thing as #81950 sans migration
Adds the following for error detectors:
configMissing:
For ACI-28