Skip to content
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

Wrap errors from external libraries to prevent leaking sensitive information #185

Merged
merged 2 commits into from
Jul 21, 2022

Conversation

cheukwing
Copy link
Contributor

@cheukwing cheukwing commented Jul 20, 2022

Resolves #103

Introduce a SafeError type (credits to @alovak for the idea!) which is used to wrap around external errors, preventing the returned error message from displaying sensitive information, while still allowing errors to be matched.
Use this new error type to wrap external errors in the field and encoding packages, as these operate on the potentially sensitive data.

I considered also wrapping external errors in the prefix and network packages, but since these only operate on the length part of the data, exposing their details should be okay (as long as the message is correctly formatted).

@cheukwing cheukwing requested a review from alovak as a code owner July 20, 2022 14:08
@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2022

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 51.85185% with 26 lines in your changes missing coverage. Please review.

Project coverage is 70.91%. Comparing base (92f3b34) to head (dd4a723).
Report is 120 commits behind head on master.

Files with missing lines Patch % Lines
field/binary.go 28.57% 4 Missing and 1 partial ⚠️
field/composite.go 33.33% 3 Missing and 1 partial ⚠️
utils/safe_error.go 60.00% 4 Missing ⚠️
field/numeric.go 50.00% 2 Missing and 1 partial ⚠️
field/string.go 40.00% 2 Missing and 1 partial ⚠️
message.go 40.00% 2 Missing and 1 partial ⚠️
specs/builder.go 0.00% 2 Missing ⚠️
encoding/hex.go 75.00% 1 Missing ⚠️
field/ordered_map.go 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #185      +/-   ##
==========================================
+ Coverage   70.50%   70.91%   +0.41%     
==========================================
  Files          37       38       +1     
  Lines        1773     1798      +25     
==========================================
+ Hits         1250     1275      +25     
- Misses        334      336       +2     
+ Partials      189      187       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Dakinola892 Dakinola892 left a comment

Choose a reason for hiding this comment

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

Happy with all the new errors, especially the ones with expanded context - nice work

Copy link
Member

@adamdecaf adamdecaf left a comment

Choose a reason for hiding this comment

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

I'm a fan of this, nice!

Copy link
Contributor

@alovak alovak left a comment

Choose a reason for hiding this comment

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

Great job! Thanks a lot!!!

@alovak alovak merged commit 151e28e into moov-io:master Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure sensitive information is not exposed through error messages
5 participants