-
Notifications
You must be signed in to change notification settings - Fork 10
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.
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
Fix/security vulnerability #5692
base: main
Are you sure you want to change the base?
Conversation
090afc3
to
9788049
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5692 +/- ##
==========================================
- Coverage 96.36% 96.34% -0.02%
==========================================
Files 1010 1010
Lines 23954 23966 +12
Branches 2155 2155
==========================================
+ Hits 23084 23091 +7
- Misses 703 707 +4
- Partials 167 168 +1 ☔ View full report in Codecov by Sentry. |
9788049
to
7b64d54
Compare
Can you add some tests to check that the detailed error message is still being logged correctly (as well, as the generic exceptions you have now)? |
If you look at the original code, there was no error loggin at all. So, I was adding it for the purpose of help us pinpoint problems more effectively if they occur later |
@ince-dbt yep I realise the logging is new, but would be good to have tests for it |
Description of change
This PR aims to fix a number of alerts recently flags by codescanning happen here
As per recommendation, when handling errors or exceptions in a secure system, it is crucial to provide generic messages to users rather than exposing detailed technical information. This approach is backed by various security standards and best practices (such as OWASP, ISO/IEC 27001, NIST, and PCI DSS) to prevent potential attackers from gaining insights into the internal workings of a system.
Checklist
Has this branch been rebased on top of the current
main
branch?Explanation
The branch should not be stale or have conflicts at the time reviews are requested.
Is the CircleCI build passing?
General points
Other things to check
fixtures/test_data.yaml
is maintained when updating modelsSee docs/CONTRIBUTING.md for more guidelines.